This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
Summary:
FIDO U2F's specification says that when the wrong security key responds to a
signature, or when an already-registered key exists, that the UA should return
error code 4, DEVICE_INELIGIBLE. We used to do that, but adjusted some things
for WebAuthn and now we don't. This changes the soft token to return that at
the appropriate times, and updates the expectations of U2F.cpp that it should
use InvalidStateError as the signal to reutrn DEVICE_INELIGIBLE.
Also, note that WebAuthn's specification says that if any authenticator returns
"InvalidStateError" that it should be propagated, as it indicates that the
authenticator obtained user consent and failed to complete its job [1].
This change to the Soft Token affects the WebAuthn tests, but in a good way.
Reading the WebAuthn spec, we should not be returning NotAllowedError when there
is consent from the user via the token (which the softtoken always deliveres).
As such, this adjusts the affected WebAuthn tests, and adds a couple useful
checks to test_webauthn_get_assertion.html for future purposes.
[1] https://w3c.github.io/webauthn/#createCredential section 5.1.3 "Create a new
credential", Step 20, Note 2: "If any authenticator returns an error status
equivalent to "InvalidStateError"..."
Test Plan: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2fc930f7fc8eea69b1ebc96748fe95e150a92a4
Reviewers: ttaubert
Bug #: 1460767
Differential Revision: https://phabricator.services.mozilla.com/D1269
--HG--
extra : transplant_source : M%5B%93%81%29%7E%B2%E8%24%05%A6%96%8BUN%C9%FB%3E%B3h
The old name no longer makes sense, since it no longer exports an spawn_task
symbol, and add_task is what we really care about.
MozReview-Commit-ID: IE7B8Czv8DH
--HG--
rename : testing/mochitest/tests/SimpleTest/SpawnTask.js => testing/mochitest/tests/SimpleTest/AddTask.js
extra : rebase_source : 03bca5aa69a7625a49b4455a6c96ce4c59de3a5a
This patch support already-enrolled U2F devices at Google Accounts by adding a
hard-coded "OK" into the U2F EvaluateAppID method, per the intent-to-ship [1].
This adds no tests, as this is not testable in our infrastructure. It will
require cooporation with Google Accounts to validate.
[1] https://groups.google.com/d/msg/mozilla.dev.platform/Uiu3fwnA2xw/201ynAiPAQAJ
MozReview-Commit-ID: 1YLd5sfeTKv
--HG--
extra : rebase_source : 96bfb92819be2c6e549dae0a5df0525587f894b8
This patch support already-enrolled U2F devices at Google Accounts by adding a
hard-coded "OK" into the U2F EvaluateAppID method, per the intent-to-ship [1].
This adds no tests, as this is not testable in our infrastructure. It will
require cooporation with Google Accounts to validate.
[1] https://groups.google.com/d/msg/mozilla.dev.platform/Uiu3fwnA2xw/201ynAiPAQAJ
MozReview-Commit-ID: 1YLd5sfeTKv
--HG--
extra : rebase_source : bfdb407cec61c4f4e5efaf85d1590fe287aaea4c
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
Tooru Fujisawa
Jeff Gilbert
Miko Mynttinen
arthur.iakab
Emilio Cobos Álvarez
Tim Taubert
J.C. Jones
Kris Maglione
Dorel Luca
Bevis Tseng
Andrew McCreight
Andrea Marchesini
Phil Ringnalda