Summary:
With both managers storing transaction infos in `Maybe<Info> mTransaction` now,
it occurred to me that we can't actually assert that
`mTransaction.isSome() == true` when we receive a message.
At least with the U2F API the request could be cancelled (and mTransaction
cleared) while there's a pending completion message. For WebAuthn it probably
doesn't hurt to handle this properly either.
(As a bonus, I snuck in the removal of an unused enum.)
Reviewers: jcj
Reviewed By: jcj
Bug #: 1410428
Differential Revision: https://phabricator.services.mozilla.com/D145
Summary:
This patch aims to clean up the WebAuthnManager's state machine, especially
to make cancellation of transactions clearer. To fix bug 1403818, we'll have to
later introduce a unique id that is forwarded to the U2FTokenManager.
There are multiple stages of cancellation/cleanup after a transaction was
started. All of the places where we previously called Cancel() or
MaybeClearTransaction() are listed below:
[stage 1] ClearTransaction
This is the most basic stage, we only clean up what information we have about
the current transaction. This means that the request was completed successfully.
It is used at the end of FinishMakeCredential() and FinishGetAssertion().
[stage 2] RejectTransaction
The second stage will reject the transaction promise we returned to the caller.
Then it will call ClearTransaction, i.e. stage 1. It is used when one of the
two Finish*() functions aborts before completion, or when the parent process
sends a RequestAborted message.
[stage 2b] MaybeRejectTransaction
This is the same as stage 2, but will only run if there's an active transaction.
It is used by ~WebAuthnManager() to reject and clean up when we the manager
goes away.
[stage 3] CancelTransaction
The third stage sends a "Cancel" message to the parent process before rejecting
the transaction promise (stage 2) and cleaning up (stage 1). It's used by
HandleEvent(), i.e. the document becomes inactive.
[stage 3b] MaybeCancelTransaction
This is the same as stage 3, but will only run if there's an active transaction.
it is used at the top of MakeCredential() and GetAssertion() so that any
active transaction is cancelled before we handle a new request.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1409434
Differential Revision: https://phabricator.services.mozilla.com/D132
Summary:
We can simplify and reduce the {WebAuthn,U2F}Manager code by removing these
methods and sending messages directly from closures.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1409357
Differential Revision: https://phabricator.services.mozilla.com/D131
The WD-06 (and later) WebAuthn specs choose to move to integer algorithm
identifiers for the signatures [1], with a handful of algorithms identified [2].
U2F devices only support ES256 (e.g., COSE ID "-7"), so that's all that is
implemented here.
Note that the spec also now requires that we accept empty lists of parameters,
and in that case, the RP says they aren't picky, so this changes what happens
when the parameter list is empty (but still aborts when the list is non-empty
but doesn't have anything we can use) [3].
There's a follow-on to move parameter-validation logic into the U2FTokenManager
in Bug 1409220.
[1] https://w3c.github.io/webauthn/#dictdef-publickeycredentialparameters
[2] https://w3c.github.io/webauthn/#alg-identifier
[3] https://w3c.github.io/webauthn/#createCredential bullet #12
MozReview-Commit-ID: KgL7mQ9u1uq
--HG--
extra : rebase_source : 2a1767805779a9f8049102723011193f113f0713
The WebAuthnRequest.h file is no longer used, and it appears we forgot to
clean it up.
MozReview-Commit-ID: 8Cgh40YxGiY
--HG--
extra : rebase_source : 81b84d0365f8a0766d84962a2f628b6025c135e2
This covers these renames:
* In CollectedClientData, hashAlg => hashAlgorithm
* In CollectedClientData, tokenBinding => tokenBindingId
* In MakePublicKeyCredentialOptions, parameters => pubKeyCredParams
* In MakePublicKeyCredentialOptions, excludeList => excludeCredentials
* In PublicKeyCredentialRequestOptions, allowList => allowCredentials
* Transport (WebAuthnTransport in Gecko) => AuthenticatorTransport
MozReview-Commit-ID: 3FdRnkosy83
--HG--
extra : rebase_source : 22f124c781b03837ad0cd4be4edf34527e3b9d38
This covers these renames:
* In PublicKeyCredentialParameters, algorithm => alg
* MakeCredentialOptions => MakePublicKeyCredentialOptions
* PublicKeyCredentialEntity => PublicKeyCredentialRpEntity
* Attachment => AuthenticatorAttachment
It sets a default excludeList and allowList for the make / get options.
It adds the method isPlatformAuthenticatorAvailable which is incomplete and
not callable, to be completed in Bug 1406468.
Adds type PublicKeyCredentialRpEntity.
Adds "userId" to AuthenticatorAssertionResponse.
Adds "id" as a buffer source to PublicKeyCredentialUserEntity and as a
DOMString to PublicKeyCredentialRpEntity, refactoring out the "id" field
from the parent PublicKeyCredentialEntity.
It also adds a simple enforcement per spec 4.4.3 "User Account Parameters for
Credential Generation" that the new user ID buffer, if set, be no more than
64 bytes long. I mostly added it here so I could adjust the tests all at once
in this commit.
MozReview-Commit-ID: IHUdGVoWocq
--HG--
extra : rebase_source : bc1793f74700b2785d2bf2099c0dba068f717a59
WebAuthn has added a flag UV to indicate the user was biometrically verified. We
have to make sure not to set that flag for U2F. Turns out we already do that,
but let's add the constant and such.
Ref: https://w3c.github.io/webauthn/#authenticator-data
MozReview-Commit-ID: 6Qtjdkverls
--HG--
extra : rebase_source : 660348596b917d8f461b19298e01dbe19410b63f
Summary: It seems like a good idea to call AssertIsOnBackgroundThread() in the WebAuthnTransactionParent and U2FTransactionParent methods. They should never be called on any other thread. (Other BPImpls are doing the same.)
Reviewers: jcj
Reviewed By: jcj
Bug #: 1407179
Differential Revision: https://phabricator.services.mozilla.com/D105
There's an intermittent which might be spurious because ASN.1 signatures might
sometimes be less than 70 bytes, but the actual floor is probably 68 (32 + 32
+ 4).
It's a sanity check, so I've adjusted it down and also am now emitting the
offending key bytes if this triggers again.
MozReview-Commit-ID: 1wwU9Q3BUPF
--HG--
extra : rebase_source : 2877deb770f8bf4bcf31dae40f75016892dc9d53
The Web Authentication types, by spec, return ArrayBuffer objects, while we
were returning a concrete Uint8Array. This is a fairly straightforward change
to add functionality to CryptoBuffer and the WebIDL types, however it's a
substantial change to the tests.
Frankly, the tests just could use another pass of clean-up now, since this is
a lot of relative ugliness added in. I refactored tab_webauthn_success.html
pretty heavily -- since it was also fairly ugly to start -- but I decided to go
with a lighter touch on the other tests.
MozReview-Commit-ID: 9vb1wdLo3SI
--HG--
rename : dom/webauthn/tests/browser/frame_webauthn_success.html => dom/webauthn/tests/browser/tab_webauthn_success.html
extra : rebase_source : bd2bc326c6bb5e00929b14c7aae66eba335c0605
The NS_LITERAL_STRING macro creates a temporary nsLiteralString to encapsulate the char16_t string literal and its length, but AssignLiteral() can determine the char16_t string literal's length at compile-time without nsLiteralString.
MozReview-Commit-ID: 6vgQiU8zN3o
--HG--
extra : rebase_source : 1b536b92ef43f610db057ace6f108620e8d8b4d5
extra : source : 336e21386d5eeb16f1c9893c29377f23b67cc4b0
This should be an easy solution. We can't stop the sign() or register()
runloop from calling the callback, so we need the callback to simply return
early when the U2FHIDTokenManager shuts down.
Bug #: 1400940
Differential Revision: https://phabricator.services.mozilla.com/D67
The runloop seems like a good candidate for moving into its own crate.
I wasn't sure whether we want it under the Mozilla org on GitHub, so I pushed
it to ttaubert/rust-runloop for a start. Moving the repository to mozilla/*
is easy, and we'd just need to bump the crate version with the updated
repository, if you think we should.
Bug #: 1400559
Differential Revision: https://phabricator.services.mozilla.com/D62
--HG--
rename : dom/webauthn/u2f-hid-rs/src/runloop.rs => third_party/rust/runloop/src/lib.rs
One cannot use #[cfg(target_os)] checks in build.rs.
Build scripts can be used to generate code so the target
is set to the host platform when they are compiled.
Having this setting exported an unconditional link
depencency whenever the host was macOS, which broke
cross-compiling, in particular for fennec builds
targetting Android.
Instead, declare the IOKit dependency on the `extern`
block which imports the symbol inside macOS-specific
code. That way final link still works, but the extra
dependency is only enabled when appropriate for the
final target, like the other platform-dependent code.
Summary: We're currently using the thread_rng to derive a cmd byte for the U2F protocol fuzzers. That of course should rather be derived deterministically from the input handed to the fuzzing target.
Bug #: 1400513
Differential Revision: https://phabricator.services.mozilla.com/D61
The algorithm names provided to the WebAuthn methods have to either be a
string, or (potentially) a WebCrypto object. Right now we only work with
strings, but there's no good reason to assert that, we can just let the
action fail.
This patch removes the assert to help out the fuzzing team.
MozReview-Commit-ID: 9dc8m0a2gZK
--HG--
extra : rebase_source : 649a7f4928679405fe445ac533eee2cfccaedd25
FreeBSD isn't currently support for FIDO U2F support, similar to Android, so
this patch [1] from Jan Beich <jbeich@FreeBSD.org> treats Android and FreeBSD
the same. With luck, someone will add in the platform support for both, soon!
[1] https://github.com/jcjones/u2f-hid-rs/pull/44
MozReview-Commit-ID: DU7Rco2NLb3
--HG--
rename : dom/webauthn/u2f-hid-rs/src/android/mod.rs => dom/webauthn/u2f-hid-rs/src/stub/mod.rs
Now that there are actual hardware devices, this test can't be run: it
depended on there being a deliberately-erroring implementation of WebAuthn
which would instantly reject promises. Fortunately, this test was really more
a test that telemetry scalars work properly than really the functionality
of WebAuthn.
Sadly, I don't see any way to re-enable this test without adding a new test-
only pref to the tree, which doesn't seem worth it for the telemetry.
So this patch removes the offending test completely which was backed out in
https://hg.mozilla.org/integration/mozilla-inbound/rev/c115eec567a6 .
MozReview-Commit-ID: LiLuQHbPU1z
The nsIU2FToken and its implementors are no longer needed; the soft token was
re-implemented into dom/webauthn/U2FSoftTokenManager.cpp during the WebAuthn
implementation. When the dom/u2f/ code changed to the implementation from
WebAuthn, the old synchronous version became dead code.
This patch removes the dead code.
MozReview-Commit-ID: 2yDD0tccgZr
--HG--
extra : rebase_source : 0f14d8de8f62599a41c13aa4d8fc9cdbc1fd79c7
- This patch reworks the U2F module to asynchronously call U2FManager,
which in turn handles constructing and managing the U2FTokenManager
via IPC.
- Add U2FTransaction{Parent,Child} implementations to mirror similar ones for
WebAuthn
- Rewrite all tests to compensate for U2F executing asynchronously now.
- Used async tasks, used the manifest parameters for scheme, and generally
made these cleaner.
- The mochitest "pref =" functionality from Bug 1328830 doesn't support Android
yet, causing breakage on Android. Rework the tests to go back to the old way
of using iframes to test U2F.
NOTE TO REVIEWERS:
Since this is huge, I recommend the following:
keeler - please review U2F.cpp/h, the tests, and the security-prefs.js. Most
of the U2F logic is still in U2F.cpp like before, but there's been
some reworking of how it is called.
ttaubert - please review U2FManager, the Transaction classes, build changes,
and the changes to nsGlobalWindow. All of these should be very
similar to the WebAuthn code it's patterned off.
MozReview-Commit-ID: C1ZN2ch66Rm
--HG--
extra : rebase_source : 5a2c52b0340c13f471af5040b998eb7e661b1981
This is a change to permit interacting with the U2FTokenManager from
the dom/U2F context in addition to the dom/WebAuthn one.
MozReview-Commit-ID: BvP5BY2wVYi
--HG--
extra : rebase_source : 0ca9cb1e72cb688b901484ec6bf2602d15131478
In Bug 1380421 we reverted some behavior that required Web Authentication's
RP ID to be domain string to permit it to be an origin, too, for interop
testing. That is no longer needed, so this patch resumes enforcement that
RP ID be a domain string.
It also adds a needed test that the RP ID hash is calculated correctly.
MozReview-Commit-ID: 8dDjzo5kQKP
--HG--
extra : rebase_source : 65cd7b9f3a6ecfc58805daf102f33966c9b19b98
Replace it with NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION, because it
has been the same for a while.
MozReview-Commit-ID: 5agRGFyUry1
--HG--
extra : rebase_source : 5388c56b2f6905c6ef969150f0c5b77bf247624d
WD-05 changed the "hashAlg" parameter of the CollectedClientData definition
from using an internally-defined "S256" string to identify the sha256 digest
to the definition in WebCrypto [1]. This only appears once, hard-coded in
WebAuthn (since U2F only supports SHA-256), so we need to change that one
instance.
[1] https://www.w3.org/TR/WebCryptoAPI/#sha-registration
MozReview-Commit-ID: 8de2CIGBBGA
--HG--
extra : rebase_source : e54c0d1f3f9551be48c3a72444edf62c45c647c4
WebAuthn operations that are in-flight with authenticators must be cancelled
when switching tabs.
There's an Issue [1] opened with the WebAuthn spec for this already, but the
language is _not_ in spec. Still, it's necessary for security, spec or not.
This also matches how Chromium handles U2F operations during a tab switch.
[1] https://github.com/w3c/webauthn/issues/316
MozReview-Commit-ID: 6Qh9oC4pqys
--HG--
extra : rebase_source : ad1665b8140f74b1291f17994285e6146c4ec468
This patch intends to obtain a 1) rough sense of the percentage of telemetry
population using Web Authentication over time, and 2) whether or not the
Authentication request was successful or errored out as a scalar. It also tracks
3) how long it takes for requests to complete as a histogram.
It counts Register (enrollment) and Sign (login) separately as we would
anticipate there being far, far more Sign uses than Register.
MozReview-Commit-ID: 8DFyKAG8XJw
--HG--
extra : rebase_source : 0c168b32b995ffffda804538d2b92009d4dc38c5
The WebAuthn WD-05 version of the specification defines the Origin field [1]
of the CollectedClientData as being set to the RP ID [2][3].
Note there is some ambiguity in the specification, as [1] says
CollectedClientData.Origin is the document's origin, while the
algorithms [2] and [3] set it to RP ID.
I'm going to stick with the algorithm's definition for this patch; it's
simple to revert when we move to WD-06 (Bug 1384776).
[1] https://www.w3.org/TR/webauthn/#dom-collectedclientdata-origin
[2] https://www.w3.org/TR/webauthn/#createCredential
[3] https://www.w3.org/TR/webauthn/#getAssertion
MozReview-Commit-ID: LW918sIg5wH
--HG--
extra : rebase_source : 799f5fa8878614c45d0def07d01d6c1c0c6e9824
Tim Taubert
Sebastian Hengst
J.C. Jones
Chris Peterson
Ralph Giles
Andrew McCreight
Wes Kocher
Carsten "Tomcat" Book