Summary:
Add support for PublicKeyCredentialRequestOptions.userVerification. For now
this basically means that we'll abort the operation with NotAllowed, as we
don't support user verification yet.
Pass PublicKeyCredentialDescriptor.transports through to the token manager
implementations. The softoken will ignore those and pretend to support all
transports defined by the spec. The USB HID token will check for the "usb"
transport and either ignore credentials accordingly, or abort the operation.
Note: The `UserVerificationRequirement` in WebIDL is defined at https://w3c.github.io/webauthn/#assertion-options
Reviewers: jcj, smaug
Reviewed By: jcj, smaug
Bug #: 1406467
Differential Revision: https://phabricator.services.mozilla.com/D338
--HG--
extra : amend_source : 314cadb3bc40bbbee2a414bc5f13caed55f9d720
Use the [SecureContext] webidl notation to hide the powerful "window.u2f"
feature and its interface when not loaded in a secure context.
MozReview-Commit-ID: 7en8b5ieI85
Summary:
Ensure that transactions are cleared before U2FCallbacks are called, to allow
reentrancy from microtask checkpoints.
Move the two possible callbacks into U2FTransaction so we have nicer invariants
and know that there's a callback as long as we have a transaction.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1422661
Differential Revision: https://phabricator.services.mozilla.com/D329
--HG--
extra : amend_source : 7097f38199a5bc4a215377e4f1a64079cf6d6a24
Summary: We can probably abstract more stuff in the future, but this seems like a good start.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1396907
Differential Revision: https://phabricator.services.mozilla.com/D323
Summary:
We currently have a single WebAuthnManager instance per process that's shared
between all CredentialContainers. That way the nsPIDOMWindowInner parent has
to be tracked by the transaction, as multiple containers could kick off
requests simultaneously.
This patch lets us we have one WebAuthnManager instance per each
CredentialsContainer and thus each nsPIDOMWindowInner. This matches the current
U2F implementation where there is one instance per parent window too.
This somewhat simplifies the communication diagram (at least in my head), as
each U2F/WebAuthnManager instance also has their own TransactionChild/Parent
pair for IPC protocol communication. The manager and child/parent pair are
destroyed when the window is.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1421616
Differential Revision: https://phabricator.services.mozilla.com/D305
Summary:
We currently have a single WebAuthnManager instance per process that's shared
between all CredentialContainers. That way the nsPIDOMWindowInner parent has
to be tracked by the transaction, as multiple containers could kick off
requests simultaneously.
This patch lets us we have one WebAuthnManager instance per each
CredentialsContainer and thus each nsPIDOMWindowInner. This matches the current
U2F implementation where there is one instance per parent window too.
This somewhat simplifies the communication diagram (at least in my head), as
each U2F/WebAuthnManager instance also has their own TransactionChild/Parent
pair for IPC protocol communication. The manager and child/parent pair are
destroyed when the window is.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1421616
Differential Revision: https://phabricator.services.mozilla.com/D305
Summary:
This patch fixes the reported leak of U2FTransactionChild instances in the
content process by introducing a WebAuthnTransactionChildBase class that both
WebAuthnTransactionChild and U2FTransactionChild inherit from.
This base class is responsible for proper refcounting. In
BackgroundChildImpl::DeallocPWebAuthnTransactionChild() we currently always
cast to WebAuthnTransactionChild, that will work only for the WebAuthn API. We
can now cast to WebAuthnTransactionChildBase to make this work for U2F as well.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1412408
Differential Revision: https://phabricator.services.mozilla.com/D179
This was automatically generated by the script modeline.py.
MozReview-Commit-ID: BgulzkGteAL
--HG--
extra : rebase_source : a4b9d16a4c06c4e85d7d85f485221b1e4ebdfede
Summary:
We currently call ChildActor.send__delete() when clearing an active transaction
and thereby destroy the child actor. If that happens, e.g. due to a tab switch,
while a message is in the IPC buffer waiting to be delivered, we crash.
This patch creates the child actor lazily as before, but keeps it around until
the WebAuthnManager goes away, which will be at process shutdown.
Each transaction now has a unique id, that the parent process will include in
any of the ConfirmRegister, ConfirmSign, or Abort messages. That way we can
easily ignore stale messages that were in the buffer while we started a new
transaction or cancelled the current one.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1403818
Differential Revision: https://phabricator.services.mozilla.com/D149
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 U2FManager'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 FinishRegister() and FinishSign().
[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 ~U2FManager() 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 Register() and Sign() so that any active transaction
is cancelled before we handle a new request. It's also used by U2F::Cancel()
as long as bug 1410346 isn't fixed.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1410345
Differential Revision: https://phabricator.services.mozilla.com/D144
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
Summary:
Both files declare a few methods as public that we can make private. Let's
seize the chance to rearrange declarations such that they reflect the message
model better.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1409135
Differential Revision: https://phabricator.services.mozilla.com/D128
--HG--
extra : amend_source : 8f7a9d92ec81253847c44d92c71ea00cc8753bd1
Summary:
We currently allow sending a "Cancel" message from the child to abort a running
transaction, e.g. when the user switches away from the currently active tab.
We have a message with the same name "Cancel" sent by the parent when the
transaction is aborted due to failure somewhere in the token manager.
This patch renames abort messages from the parent to "Abort" to clarify the
purpose of the message.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1409116
Differential Revision: https://phabricator.services.mozilla.com/D127
--HG--
extra : amend_source : ee6767965ad928033eb23b258aacf54bbaf57d2d
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
In Comment 8 of Bug 1244959 [1], Brad Hill argues that instead of leaving our
U2F Facet support completely half-way, that we could use the Public Suffix logic
introduced into HTML for W3C Web Authentication (the method named
IsRegistrableDomainSuffixOfOrEqualTo) to scope the FIDO AppID to an eTLD+1
hierarchy. This is a deviation from the FIDO specification, but doesn't break
anything that currently works with our U2F implementation, and theoretically
enables sites that otherwise need an external FacetID fetch which we aren't
implementing.
The downside to this is that it's then Firefox-specific behavior. But since this
isn't a shipped feature, we have more room to experiment. As an additional
bonus, it encourages U2F sites to use the upcoming Web Authentication security
model, which will help them prepare to adopt the newer standard.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1244959#c8
MozReview-Commit-ID: DzNVhHT9qRL
--HG--
extra : rebase_source : 262e2ddbec325e0391d346473f27ae2738490da1
There's an intermittent on the call attestationCert.verify() to test the self-
signed cert from our not-shipped software U2F implementation. Collection of the
intermittents shows these certs are fine, and should verify correctly, but they
don't. The bug must be in pki.js, which is out-of-scope as we only use it for
mochitests.
This patch removes the offending call to xxxx.verify(), because it doesn't
really matter whether the self-signed-cert looks OK to pki.js; we just need
the public key from inside it to proceed with the rest of the tests.
As an example of a so-called "invalid" self-signed cert that failed, we have:
https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-central&job_id=134282931&lineNumber=2673
-----BEGIN CERTIFICATE-----
MIIBMTCB2aADAgECAgUA55x6LTAKBggqhkjOPQQDAjAhMR8wHQYDVQQDExZGaXJl
Zm94IFUyRiBTb2Z0IFRva2VuMB4XDTE3MDkzMDE5MjIzMloXDTE3MTAwMjE5MjIz
MlowITEfMB0GA1UEAxMWRmlyZWZveCBVMkYgU29mdCBUb2tlbjBZMBMGByqGSM49
AgEGCCqGSM49AwEHA0IABIWu4L8ky7s8I7qVv+JwMRHpippH4b6h7rN0jlKpFbHK
hnEwaCPLrTx04Eh9xT4GK9JWuuP759hnAxsWD5wk0H0wCgYIKoZIzj0EAwIDRwAw
RAIgRIeRcn6LkwU8VOmX+mdQ3jUQrUOp5f2xH/qBECGi5EcCIADBjsm/EDKkAwLZ
pGdX7+N+kgf9No4uuLV4dsNVJ1pa
-----END CERTIFICATE-----
There's nothing wrong with this cert, actually. Checking it with OpenSSL shows
all OK:
openssl verify -purpose any -CAfile /tmp/cert2.pem /tmp/cert2.pem
/tmp/cert2.pem: OK
So this intermittent is a bug outside of our U2F and U2F test soft token code.
MozReview-Commit-ID: K142toVWtcv
--HG--
extra : rebase_source : 3c31a407e27cd5c6e7a1a4f1287f17f56f80daaa
There's an intermittent that is showing up now that test_register_sign.html
checks state.attestationCert.verify(); to ensure hte SoftToken's certificate
is valid. This patch prints the offending certificate when it's encountered,
to help diagnose the root cause.
MozReview-Commit-ID: 4QSobq9fBGK
I wasn't sure what the right behavior for the U2F API is when `.sign()`
or `.register()` is called but there's an ongoing request that wasn't fulfilled
yet.
I think it makes sense to deny the request (as we currently do) when a request
of the same type is currently active. When however sign() -> register() or
vice-versa is called then we should cancel the previous request and start
the new one. From what I understand from reading the spec we definitely should
call the callback before starting the new request.
Bug #: 1401019
Differential Revision: https://phabricator.services.mozilla.com/D70
- 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 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 prefs and scheme,
and generally made these cleaner.
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 : transplant_source : %EA%98%D2%87C%FD%CC%A5%3D%B5%9B%1C%DA%A5J%CD%05%94%13%0D
We have a minimum requirement of VS 2015 for Windows builds, which supports
the z length modifier for format specifiers. So we don't need SizePrintfMacros.h
any more, and can just use %zu and friends directly everywhere.
MozReview-Commit-ID: 6s78RvPFMzv
--HG--
extra : rebase_source : 009ea39eb4dac1c927aa03e4f97d8ab673de8a0e
This change disables the test dom/u2f/tests/test_multiple_keys.html, as it
is being prompted by some mis-use of the IPC system. All IPC for this component,
U2F, is being reworked currently in Bug 1323339, so fixing this really falls to
that bug. Bug 1347374 is filed to re-enable this test after the IPC rework.
MozReview-Commit-ID: BQCk7Muz53c
This is a cheezy fix to u2f/tests/frame_multiple_keys.html to try and fix
the ongoing intermittents. It's cheesy because it changes this from a
HTTPS-scheme test to HTTP, which is kind of a negative change, and shouldn't
do anything to help. It does, however, make this test look identical to the
other u2f tests which _don't_ have this intermittent issue. If this commit
fixes the problem then I'll know more about this strange failure case.
MozReview-Commit-ID: JXXFshJ6AGq
--HG--
extra : rebase_source : 83b09430d36904bba49037060f615f1b7a2d9078
This change includes the FIDO "App ID" as part of the function used to generate
the wrapping key used in the NSS-based U2F soft token, cryptographically binding
the "Key Handle" to the site that Key Handle is intended for.
This is a breaking change with existing registered U2F keys, but since our soft
token is hidden behind a pref, it does not attempt to be backward-compatible.
- Updated for rbarnes' and qdot's reviews comments. Thanks!
- Made more strict in size restrictions, and added a version field
to help us be this strict.
- Bugfix for an early unprotected buffer use (Thanks again rbarnes!)
- Fix a sneaky memory leak re: CryptoBuffer.ToSECItem
MozReview-Commit-ID: Jf6gNPauT4Y
--HG--
extra : rebase_source : 4ff5898e93e4a0a75576e5e54035a1cb6dd952d7
This change includes the FIDO "App ID" as part of the function used to generate
the wrapping key used in the NSS-based U2F soft token, cryptographically binding
the "Key Handle" to the site that Key Handle is intended for.
This is a breaking change with existing registered U2F keys, but since our soft
token is hidden behind a pref, it does not attempt to be backward-compatible.
- Updated for rbarnes' and qdot's reviews comments. Thanks!
- Made more strict in size restrictions, and added a version field
to help us be this strict.
- Bugfix for an early unprotected buffer use (Thanks again rbarnes!)
MozReview-Commit-ID: Jf6gNPauT4Y
--HG--
extra : rebase_source : 52d10287d10698292e1480e04f580f6f8b4847cb
Tim Taubert
J.C. Jones
Bevis Tseng
Andrew McCreight
Andrea Marchesini
Phil Ringnalda
Kartikaya Gupta
Bill McCloskey
Carsten "Tomcat" Book
Iris Hsiao
Daniel Holbert