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
- 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
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
If there's a second token (say, USB anyone?) that fails early, U2F.cpp's
U2FStatus object should not be told to "stop" unless it's actually done.
So basically, in the promise failures for U2F::Sign and U2F::Register, don't
call Stop - let the stop come implicitly when no tokens respond correctly.
This changes U2FStatus to be used the same way WebAuthn does its WebAuthnRequest
object, for the same purpose.
- Review updates from Keeler; thanks!
MozReview-Commit-ID: HaTKopFakDB
--HG--
extra : rebase_source : f55918f76117abb0f120b21a742c3705c2640225
Add a test that U2F supports signing with multiple keys.
Reorder the WaitGroupDone calls to ensure they always fire, even if there
are multiple actions in flight.
Also, change the status lanbda captures to capture by reference, and disable
the copy constructor that would let the by-value work. Interestingly, the
compiler is choosing by-reference by default -- at least logs show that the
behavior is correct without this change, but still - this is the right thing to
do.
Updated: Fix the unit tests and write a README that explains why they have to
use an iframe, while WebAuthn tests do not.
MozReview-Commit-ID: AqSyxU5N4yu
--HG--
extra : rebase_source : b8f18973891ba63ac364203dece65a0689f46ee5
This uses the new mochitest "scheme" option from Bug 1286312. This cannot land
until after Bug 1286312 does.
For now, you can test locally by adding
--setpref dom.securecontext.whitelist=mochi.test
to your command line, such as:
~/hg/mozilla-central/mach mochitest \
--setpref dom.securecontext.whitelist=mochi.test ./dom/u2f/tests/
Updated:
Review fixes (thanks keeler!)
MozReview-Commit-ID: 7jTxF3Mrtcg
--HG--
extra : rebase_source : 72c24bdc028e440705598c694f3c4119d5304d83
Add more debugging information to signing operations for the NSS Soft Token.
Bugfixes in WebAuthentication.cpp:
- Calculate ArrayBuffer/View before using.
- Fix an instance where we should return NotSupportedError.
- Fix several instances where we should return Out Of Memory.
- Fix a MozPromise assertion that occurs in GetAssertion if you coerce an early
return.
- Mark all constructors explicit.
MozReview-Commit-ID: DQWHqZIlau9
--HG--
extra : rebase_source : f0f2bdde650e61c90b9b47c20c2427f1340f2d97
- Breaks compatibility with non-e10s windows, as the underlying USB
implementation from Bug 1298838 won't support non-e10s either.
- Now that U2F doesn't support non-e10s, disable tests if we're not in
e10s mode.
MozReview-Commit-ID: 5F2323xtXEC
--HG--
extra : transplant_source : v%1Fl%C0%2AJ%26k4%89/%95v%89%12%87%94Y%3Cs
This is reworking the U2F tests to do two things:
1) Don't run all the tests in one big frame; that makes it hard to tell
what test is actually dying in Treeherder.
2) Fix the obvious possible test races with the async functions which could be
causing the intermittent
- Review updates per keeler
- Change inappropriate uses of 'var' to 'let' in u2futil.js (kudos, keeler)
- Rework frame_no_token.html to follow the same pattern as the others
- Catch unexpected messages on the u2f testing harness
- Update 2: Go back to a pre-set number of expected async tests.
MozReview-Commit-ID: 6uLt5O1lUa3
--HG--
rename : dom/u2f/tests/test_frame_appid_facet.html => dom/u2f/tests/frame_appid_facet.html
rename : dom/u2f/tests/test_frame_appid_facet_insecure.html => dom/u2f/tests/frame_appid_facet_insecure.html
rename : dom/u2f/tests/test_frame_appid_facet_subdomain.html => dom/u2f/tests/frame_appid_facet_subdomain.html
rename : dom/u2f/tests/test_frame_register.html => dom/u2f/tests/frame_register.html
rename : dom/u2f/tests/test_frame_register_sign.html => dom/u2f/tests/frame_register_sign.html
extra : rebase_source : 1255bd8ba17a141c3c8205a277c06c483540de90
- The u2futil.js script's verifySignature method was causing an intermittent
in test_frame_register_sign.html due to incomplete ASN.1 decoding. Since
we're calready pulling in an ASN.1 parsing library, this changes that code to
do a complete parse and santizize, which should cover all cases.
MozReview-Commit-ID: 9kDWT2KUFdq
--HG--
extra : transplant_source : %A9CD%CD%E7E%11s%0A%82ls%5B%7B%80jQ%FC%FE%0B
- Move the AppID/FacetID algorithm into its own (potentially reentrant) method
to facilitate Bug 1244959
- Change the Register and Sign operations to be Runnables so that in the future
they can be executed after (future) remote fetches
- Clean up error handling
- Remove unnecessary remote-load Facet test files; we'll re-add some form of
them when the remote load algorithm is completed
MozReview-Commit-ID: 4K1q6ovzhgf
--HG--
extra : transplant_source : /%7F/%96o1%3E%5E%17%20%A2%D0%AA%10%21%88%19%D9%B3%C9
extra : histedit_source : 4d3c61294951920a22e1f1eb7846a2a03f7cd2f0
Work on the FacetID/AppID algorithm showed this patch had incorrect usage of
the eTLD+1 checking, so this patch removes those checks until the more
sophisticated algorithm lands in Bug 1244959.
MozReview-Commit-ID: 2k6N5AU0J68
--HG--
extra : transplant_source : %B7n%17%00%DF%AB%F4OG%7E%D1%F0p%B1%AC%9Bq%C9%2B%D0
- Add an ephemeral self-signed Attestation Cert to NSSToken
- A new one is generated at each call to Register; this is allowed by the
protocol, and avoids fingerprinting if the NSSToken is in use.
- This now passes at https://u2fdemo.appspot.com/
MozReview-Commit-ID: Aq61MuX9oSD
--HG--
extra : transplant_source : %C1%00n6%22%01%E7q%B4/%D8-%C5W%D4%E6%86%14%25%C2
- Merge in test changes from Bug 1255784.
- Remove the unnecessary mutex
- Stop doing direct memory work in NSS Token
- Clean up direct memory work in ContentParent
- In order to store persistent crypto parameters, the NSSToken had to move
onto the main thread and be interfaced with via IDL/IPDL.
- Support Register/Sign via NSS using a long-lived secret key
- Rename the softtoken/usbtoken "enable" prefs, because of hierarchy issues
with the WebIDL Pref shadowing.
- Also orders the includes on nsNSSModule.cpp
- Attestation Certificates are in Part 2.
Updates per keeler review comments:
- Use //-style comments everywhere
- Refactor the PrivateKeyFromKeyHandle method
- Rename the logging and fix extraneous NS_WARN_IF/logging combinations
- Other updates from review
April 11-12:
- Correct usage of the "usageCount" flag for PK11_UnwrapPrivKey
- Rebase up to latest
April 15:
- Rebase to latest
MozReview-Commit-ID: 6T8jNmwFvHJ
--HG--
extra : transplant_source : w%26%CES%2Cu%04%3EAl%04%2Cb%E2v%C9%08%3A%CC%F4
J.C. Jones
Phil Ringnalda
Kyle Machulis
Florian Quèze
Andrea Marchesini
J.C. Jones