Bug 1335899 - Tolerate token failures in U2F.cpp r=keeler
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
--- a/dom/u2f/U2F.cpp
+++ b/dom/u2f/U2F.cpp
@@ -715,19 +715,19 @@ U2FRegisterRunnable::Run()
registerTask->Execute()->Then(mAbstractMainThread, __func__,
[&status, this] (nsString aResponse) {
if (!status->IsStopped()) {
status->Stop(ErrorCode::OK, aResponse);
}
status->WaitGroupDone();
},
[&status, this] (ErrorCode aErrorCode) {
- if (!status->IsStopped()) {
- status->Stop(aErrorCode);
- }
+ // Ignore the failing error code, as we only want the first success.
+ // U2F devices don't provide much for error codes anyway, so if
+ // they all fail we'll return DEVICE_INELIGIBLE.
status->WaitGroupDone();
});
}
}
// Wait until the first key is successfuly generated
status->WaitGroupWait();
@@ -901,19 +901,19 @@ U2FSignRunnable::Run()
signTask->Execute()->Then(mAbstractMainThread, __func__,
[&status, this] (nsString aResponse) {
if (!status->IsStopped()) {
status->Stop(ErrorCode::OK, aResponse);
}
status->WaitGroupDone();
},
[&status, this] (ErrorCode aErrorCode) {
- if (!status->IsStopped()) {
- status->Stop(aErrorCode);
- }
+ // Ignore the failing error code, as we only want the first success.
+ // U2F devices don't provide much for error codes anyway, so if
+ // they all fail we'll return DEVICE_INELIGIBLE.
status->WaitGroupDone();
});
}
}
// Wait for the authenticators to finish
status->WaitGroupWait();
--- a/dom/u2f/tests/frame_multiple_keys.html
+++ b/dom/u2f/tests/frame_multiple_keys.html
@@ -47,16 +47,27 @@ window.u2f.register(window.location.orig
registerSecondKey();
});
// Get the second key...
// It's OK to repeat the regRequest; not material for this test
function registerSecondKey() {
window.u2f.register(window.location.origin, [regRequest], [], function(aRegResponse) {
testState.key2 = keyHandleFromRegResponse(aRegResponse);
+
+ registerWithInvalidAndValidKey();
+ });
+}
+
+function registerWithInvalidAndValidKey() {
+ window.u2f.register(window.location.origin, [regRequest],
+ [invalidKey, testState.key1], function(aRegResponse) {
+ // Expect a failure response since key1 is already registered
+ local_is(aRegResponse.errorCode, 4, "The register should have skipped since there was a valid key");
+
testSignSingleKey();
});
}
// It should also work with just one key
function testSignSingleKey() {
window.u2f.sign(window.location.origin, bytesToBase64UrlSafe(challenge),
[testState.key1], function(aSignResponse) {
@@ -69,15 +80,42 @@ function testSignSingleKey() {
function testSignDual() {
// It's OK to sign with either one
window.u2f.sign(window.location.origin, bytesToBase64UrlSafe(challenge),
[testState.key1, testState.key2], function(aSignResponse) {
local_is(aSignResponse.errorCode, 0, "The signing did not error with two keys");
local_isnot(aSignResponse.clientData, undefined, "The signing provided clientData with two keys");
+ testSignWithInvalidKey();
+ });
+}
+
+// Just a key that came from a random profile; syntactically valid but not
+// unwrappable.
+var invalidKey = {
+ "version": "U2F_V2",
+ "keyHandle": "rQdreHgHrmKfsnGPAElEP9yfTx6eq2eU3_Y8n0RRsGKML0DY2d1_a8_-sOtxDr3"
+};
+
+function testSignWithInvalidKey() {
+ window.u2f.sign(window.location.origin, bytesToBase64UrlSafe(challenge),
+ [invalidKey, testState.key2], function(aSignResponse) {
+ local_is(aSignResponse.errorCode, 0, "The signing did not error when given an invalid key");
+ local_isnot(aSignResponse.clientData, undefined, "The signing provided clientData even when given an invalid key");
+
+ testSignWithInvalidKeyReverse();
+ });
+}
+
+function testSignWithInvalidKeyReverse() {
+ window.u2f.sign(window.location.origin, bytesToBase64UrlSafe(challenge),
+ [testState.key2, invalidKey], function(aSignResponse) {
+ local_is(aSignResponse.errorCode, 0, "The signing did not error when given an invalid key");
+ local_isnot(aSignResponse.clientData, undefined, "The signing provided clientData even when given an invalid key");
+
local_completeTest();
});
}
</script>
</body>
</html>
\ No newline at end of file