Skip to content
Permalink
Browse files
[WebAuthn] Handle security keys with a full key store
https://bugs.webkit.org/show_bug.cgi?id=247339
rdar://100241655

Reviewed by Brent Fulgham.

* LayoutTests/http/wpt/webauthn/public-key-credential-create-failure-hid.https-expected.txt:
* LayoutTests/http/wpt/webauthn/public-key-credential-create-failure-hid.https.html:
* LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https.html:
* LayoutTests/http/wpt/webauthn/public-key-credential-create-success-local.https-expected.txt:
* LayoutTests/http/wpt/webauthn/public-key-credential-create-success-nfc.https-expected.txt:
* LayoutTests/http/wpt/webauthn/public-key-credential-create-success-nfc.https.html:
* LayoutTests/http/wpt/webauthn/resources/util.js:
* Source/WebCore/Modules/webauthn/fido/AuthenticatorGetInfoResponse.cpp:
(fido::encodeAsCBOR):
* Source/WebCore/Modules/webauthn/fido/AuthenticatorGetInfoResponse.h:
* Source/WebCore/Modules/webauthn/fido/DeviceResponseConverter.cpp:
(fido::readCTAPGetInfoResponse):
* Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h:
* Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm:
(WebKit::AuthenticatorPresenterCoordinator::updatePresenter):
* Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:
(WebKit::wkWebAuthenticationPanelUpdate):
* Source/WebKit/UIProcess/WebAuthentication/WebAuthenticationFlags.h:
* Source/WebKit/UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp:
(WebKit::CtapAuthenticator::makeCredential):
(WebKit::CtapAuthenticator::continueMakeCredentialAfterResponseReceived):
* Source/WebKit/UIProcess/WebAuthentication/fido/CtapAuthenticator.h:

Whenever security keys are unable to create a credential due to their internal key storage being full
they return the kCtap2ErrKeyStoreFull error code. In this case we should either retry the registeration
without a discoverable credential if the preference was set to preferred, otherwise surface an error to the
user notifying them they need to use a different key or clear space on the current key.

This patch fixes this by handling the kCtap2ErrKeyStoreFull error case. For security keys supporting the CTAP 2.1
standard, this patch also adds support for reading the "remainingDiscoverableCredentials" value from getInfo to detect
if the key store is full without having to first attempt a create.

Added layout tests for new functionality.

Canonical link: https://commits.webkit.org/257989@main
  • Loading branch information
pascoej committed Dec 16, 2022
1 parent 7301504 commit 4ad382e777def610a6a4c6b079722b85bb042176
Show file tree
Hide file tree
Showing 15 changed files with 125 additions and 1 deletion.
@@ -4,6 +4,7 @@ CONSOLE MESSAGE: User gesture is not detected. To use the WebAuthn API, call 'na
CONSOLE MESSAGE: User gesture is not detected. To use the WebAuthn API, call 'navigator.credentials.create' or 'navigator.credentials.get' within user activated events.
CONSOLE MESSAGE: User gesture is not detected. To use the WebAuthn API, call 'navigator.credentials.create' or 'navigator.credentials.get' within user activated events.
CONSOLE MESSAGE: User gesture is not detected. To use the WebAuthn API, call 'navigator.credentials.create' or 'navigator.credentials.get' within user activated events.
CONSOLE MESSAGE: User gesture is not detected. To use the WebAuthn API, call 'navigator.credentials.create' or 'navigator.credentials.get' within user activated events.

PASS PublicKeyCredential's [[create]] with timeout in a mock hid authenticator.
PASS PublicKeyCredential's [[create]] with malicious payload in a mock hid authenticator.
@@ -12,4 +13,5 @@ PASS PublicKeyCredential's [[create]] with unsupported options in a mock hid aut
PASS PublicKeyCredential's [[create]] with mixed options in a mock hid authenticator.
PASS PublicKeyCredential's [[create]] with mixed options in a mock hid authenticator. 2
PASS PublicKeyCredential's [[create]] with InvalidStateError in a mock hid authenticator.
PASS PublicKeyCredential's [[create]] with full key store.

@@ -159,4 +159,27 @@
internals.setMockWebAuthenticationConfiguration({ hid: { stage: "request", subStage: "msg", error: "malicious-payload", payloadBase64: [testCtapErrCredentialExcludedOnlyResponseBase64] } });
return promiseRejects(t, "InvalidStateError", navigator.credentials.create(options), "At least one credential matches an entry of the excludeCredentials list in the authenticator.");
}, "PublicKeyCredential's [[create]] with InvalidStateError in a mock hid authenticator.");

promise_test(function(t) {
const options = {
publicKey: {
rp: {
name: "example.com"
},
user: {
name: "John Appleseed",
id: asciiToUint8Array("123456"),
displayName: "John",
},
challenge: asciiToUint8Array("123456"),
pubKeyCredParams: [{ type: "public-key", alg: -7 }],
authenticatorSelection: { residentKey: "required" },
timeout: 10 // We wait for another authenticator upon full.
}
};

if (window.internals)
internals.setMockWebAuthenticationConfiguration({ hid: { stage: "request", subStage: "msg", error: "malicious-payload", payloadBase64: [testCreateMessageFullKeyStoreBase64] } });
return promiseRejects(t, "NotAllowedError", navigator.credentials.create(options), "Operation timed out.");
}, "PublicKeyCredential's [[create]] with full key store.");
</script>
@@ -18,4 +18,5 @@ PASS PublicKeyCredential's [[create]] with direct attestation in a mock hid auth
PASS PublicKeyCredential's [[create]] with indirect attestation in a mock hid authenticator.
PASS PublicKeyCredential's [[create]] with googleLegacyAppidSupport extension in a mock hid authenticator.
PASS PublicKeyCredential's [[create]] with googleLegacyAppidSupport and appid extensions in a mock hid authenticator.
PASS PublicKeyCredential's [[create]] with authenticatorSelection { 'cross-platform', 'preferred' } in a mock hid authenticator with a full key store.

@@ -424,4 +424,29 @@
checkCtapMakeCredentialResult(credential);
});
}, "PublicKeyCredential's [[create]] with googleLegacyAppidSupport and appid extensions in a mock hid authenticator.");

promise_test(t => {
const options = {
publicKey: {
rp: {
name: "localhost",
},
user: {
name: "John Appleseed",
id: Base64URL.parse(testUserhandleBase64),
displayName: "Appleseed",
},
challenge: Base64URL.parse("MTIzNDU2"),
pubKeyCredParams: [{ type: "public-key", alg: -7 }],
authenticatorSelection: { authenticatorAttachment: "cross-platform", residentKey: "preferred" },
extensions: { credProps: true }
}
};

if (window.internals)
internals.setMockWebAuthenticationConfiguration({ hid: { stage: "request", subStage: "msg", error: "success", payloadBase64: [testCreateMessageFullKeyStoreBase64, testCreationMessageBase64] } });
return navigator.credentials.create(options).then(credential => {
checkCtapMakeCredentialResult(credential);
});
}, "PublicKeyCredential's [[create]] with authenticatorSelection { 'cross-platform', 'preferred' } in a mock hid authenticator with a full key store.");
</script>
@@ -6,4 +6,5 @@ PASS PublicKeyCredential's [[create]] with U2F in a mock nfc authenticator.
PASS PublicKeyCredential's [[create]] with multiple physical tags in a mock nfc authenticator.
PASS PublicKeyCredential's [[create]] with service restart in a mock nfc authenticator.
PASS PublicKeyCredential's [[create]] with legacy U2F keys in a mock nfc authenticator.
PASS PublicKeyCredential's [[create]] with authenticatorSelection { 'cross-platform', 'preferred' } in a mock nfc authenticator with a full key store based on getInfo

@@ -166,4 +166,31 @@
checkCtapMakeCredentialResult(credential);
});
}, "PublicKeyCredential's [[create]] with legacy U2F keys in a mock nfc authenticator.");



promise_test(t => {
const options = {
publicKey: {
rp: {
name: "localhost",
},
user: {
name: "John Appleseed",
id: Base64URL.parse(testUserhandleBase64),
displayName: "Appleseed",
},
challenge: Base64URL.parse("MTIzNDU2"),
pubKeyCredParams: [{ type: "public-key", alg: -7 }],
authenticatorSelection: { authenticatorAttachment: "cross-platform", residentKey: "preferred" }
}
};

if (window.internals)
internals.setMockWebAuthenticationConfiguration({ nfc: { error: "success", payloadBase64: [testNfcCtapVersionBase64, testGetInfoResponseApduNoRemainingDiscoverableBase64, testCreationMessageApduBase64] } });

return navigator.credentials.create(options).then(credential => {
checkCtapMakeCredentialResult(credential);
});
}, "PublicKeyCredential's [[create]] with authenticatorSelection { 'cross-platform', 'preferred' } in a mock nfc authenticator with a full key store based on getInfo");
</script>
@@ -112,6 +112,8 @@ const testNfcCtapVersionBase64 = "RklET18yXzCQAA==";
const testGetInfoResponseApduBase64 =
"AKYBgmZVMkZfVjJoRklET18yXzACgWtobWFjLXNlY3JldANQbUS6m/bsLkm5MAyP" +
"6SDLcwSkYnJr9WJ1cPVkcGxhdPRpY2xpZW50UGlu9AUZBLAGgQGQAA==";
const testGetInfoResponseApduNoRemainingDiscoverableBase64 =
"AKcBgmZVMkZfVjJoRklET18yXzACgWtobWFjLXNlY3JldANQbUS6m/bsLkm5MAyP6SDLcwSkYnJr9WJ1cPVkcGxhdPRpY2xpZW50UGlu9AUZBLAGgQEUAJAA";
const testCreationMessageApduBase64 =
"AKMBZnBhY2tlZAJYxEbMf7lnnVWy25CS4cjZ5eHQK3WA8LSBLHcJYuHkj1rYQQAA" +
"AE74oBHzjApNFYAGFxEfntx9AEAoCK3O6P5OyXN6V/f+9nAga0NA2Cgp4V3mgSJ5" +
@@ -143,6 +145,7 @@ const testAssertionMessageApduBase64 =
"QoJ1L7Fe64G9uBeQAA==";
const testCcidNoUidBase64 = "aIE=";
const testCcidValidUidBase64 = "CH+d1ZAA";
const testCreateMessageFullKeyStoreBase64 = "KA==";

const RESOURCES_DIR = "/WebKit/webauthn/resources/";

@@ -83,6 +83,12 @@ AuthenticatorGetInfoResponse& AuthenticatorGetInfoResponse::setTransports(Vector
return *this;
}

AuthenticatorGetInfoResponse& AuthenticatorGetInfoResponse::setRemainingDiscoverableCredentials(uint32_t remainingDiscoverableCredentials)
{
m_remainingDiscoverableCredentials = remainingDiscoverableCredentials;
return *this;
}

static String toString(WebCore::AuthenticatorTransport transport)
{
switch (transport) {
@@ -139,6 +145,9 @@ Vector<uint8_t> encodeAsCBOR(const AuthenticatorGetInfoResponse& response)
deviceInfoMap.emplace(CBORValue(7), toArrayValue(transports.map(toString)));
}

if (response.remainingDiscoverableCredentials())
deviceInfoMap.emplace(CBORValue(8), CBORValue(static_cast<int64_t>(*response.maxMsgSize())));

auto encodedBytes = CBORWriter::write(CBORValue(WTFMove(deviceInfoMap)));
ASSERT(encodedBytes);
return *encodedBytes;
@@ -54,6 +54,7 @@ class WEBCORE_EXPORT AuthenticatorGetInfoResponse {
AuthenticatorGetInfoResponse& setExtensions(Vector<String>&&);
AuthenticatorGetInfoResponse& setOptions(AuthenticatorSupportedOptions&&);
AuthenticatorGetInfoResponse& setTransports(Vector<WebCore::AuthenticatorTransport>&&);
AuthenticatorGetInfoResponse& setRemainingDiscoverableCredentials(uint32_t);

const StdSet<ProtocolVersion>& versions() const { return m_versions; }
const Vector<uint8_t>& aaguid() const { return m_aaguid; }
@@ -62,6 +63,7 @@ class WEBCORE_EXPORT AuthenticatorGetInfoResponse {
const std::optional<Vector<String>>& extensions() const { return m_extensions; }
const AuthenticatorSupportedOptions& options() const { return m_options; }
const std::optional<Vector<WebCore::AuthenticatorTransport>>& transports() const { return m_transports; }
const std::optional<uint32_t>& remainingDiscoverableCredentials() const { return m_remainingDiscoverableCredentials; }

private:
StdSet<ProtocolVersion> m_versions;
@@ -71,6 +73,7 @@ class WEBCORE_EXPORT AuthenticatorGetInfoResponse {
std::optional<Vector<String>> m_extensions;
AuthenticatorSupportedOptions m_options;
std::optional<Vector<WebCore::AuthenticatorTransport>> m_transports;
std::optional<uint32_t> m_remainingDiscoverableCredentials;
};

WEBCORE_EXPORT Vector<uint8_t> encodeAsCBOR(const AuthenticatorGetInfoResponse&);
@@ -360,6 +360,14 @@ std::optional<AuthenticatorGetInfoResponse> readCTAPGetInfoResponse(const Vector
response.setTransports(WTFMove(transports));
}

it = responseMap.find(CBOR(20));
if (it != responseMap.end()) {
if (!it->second.isUnsigned())
return std::nullopt;

response.setRemainingDiscoverableCredentials(it->second.getUnsigned());
}

return WTFMove(response);
}

@@ -51,6 +51,7 @@ typedef NS_ENUM(NSInteger, _WKWebAuthenticationPanelUpdate) {
_WKWebAuthenticationPanelUpdateLAError,
_WKWebAuthenticationPanelUpdateLAExcludeCredentialsMatched,
_WKWebAuthenticationPanelUpdateLANoCredential,
_WKWebAuthenticationPanelUpdateKeyStoreFull,
} WK_API_AVAILABLE(macos(10.15.4), ios(13.4));

typedef NS_ENUM(NSInteger, _WKWebAuthenticationResult) {
@@ -76,6 +76,8 @@ static _WKWebAuthenticationPanelUpdate wkWebAuthenticationPanelUpdate(WebAuthent
return _WKWebAuthenticationPanelUpdateLAExcludeCredentialsMatched;
if (status == WebAuthenticationStatus::LANoCredential)
return _WKWebAuthenticationPanelUpdateLANoCredential;
if (status == WebAuthenticationStatus::KeyStoreFull)
return _WKWebAuthenticationPanelUpdateKeyStoreFull;
ASSERT_NOT_REACHED();
return _WKWebAuthenticationPanelUpdateMultipleNFCTagsPresent;
}
@@ -49,6 +49,7 @@ enum class WebAuthenticationStatus : uint8_t {
LAError,
LAExcludeCredentialsMatched,
LANoCredential,
KeyStoreFull,
};

enum class LocalAuthenticatorPolicy : bool {
@@ -98,6 +98,13 @@ void CtapAuthenticator::makeCredential()
auto& options = std::get<PublicKeyCredentialCreationOptions>(requestData().options);
auto internalUVAvailability = m_info.options().userVerificationAvailability();
auto residentKeyAvailability = m_info.options().residentKeyAvailability();
if (m_isKeyStoreFull || (m_info.remainingDiscoverableCredentials() && !m_info.remainingDiscoverableCredentials())) {
if (options.authenticatorSelection && (options.authenticatorSelection->requireResidentKey || options.authenticatorSelection->residentKey == ResidentKeyRequirement::Required)) {
observer()->authenticatorStatusUpdated(WebAuthenticationStatus::KeyStoreFull);
return;
}
residentKeyAvailability = AuthenticatorSupportedOptions::ResidentKeyAvailability::kNotSupported;
}
// If UV is required, then either built-in uv or a pin will work.
if (internalUVAvailability == UVAvailability::kSupportedAndConfigured && (!options.authenticatorSelection || options.authenticatorSelection->userVerification != UserVerificationRequirement::Discouraged) && m_pinAuth.isEmpty())
cborCmd = encodeMakeCredenitalRequestAsCBOR(requestData().hash, options, internalUVAvailability, residentKeyAvailability);
@@ -128,6 +135,16 @@ void CtapAuthenticator::continueMakeCredentialAfterResponseReceived(Vector<uint8
receiveRespond(ExceptionData { InvalidStateError, "At least one credential matches an entry of the excludeCredentials list in the authenticator."_s });
return;
}
if (error == CtapDeviceResponseCode::kCtap2ErrKeyStoreFull) {
auto& options = std::get<PublicKeyCredentialCreationOptions>(requestData().options);
if (options.authenticatorSelection->requireResidentKey || options.authenticatorSelection->residentKey == ResidentKeyRequirement::Required) {
observer()->authenticatorStatusUpdated(WebAuthenticationStatus::KeyStoreFull);
} else if (!m_isKeyStoreFull) {
m_isKeyStoreFull = true;
makeCredential();
}
return;
}

if (isPinError(error)) {
if (!m_pinAuth.isEmpty() && observer()) // Skip the very first command that acts like wink.
@@ -145,7 +162,7 @@ void CtapAuthenticator::continueMakeCredentialAfterResponseReceived(Vector<uint8

auto rkSupported = m_info.options().residentKeyAvailability() == AuthenticatorSupportedOptions::ResidentKeyAvailability::kSupported;
auto rkRequested = options.authenticatorSelection && ((options.authenticatorSelection->residentKey && options.authenticatorSelection->residentKey != ResidentKeyRequirement::Discouraged) || options.authenticatorSelection->requireResidentKey);
extensionOutputs.credProps = AuthenticationExtensionsClientOutputs::CredentialPropertiesOutput { rkSupported && rkRequested };
extensionOutputs.credProps = AuthenticationExtensionsClientOutputs::CredentialPropertiesOutput { rkSupported && rkRequested && !m_isKeyStoreFull };
response->setExtensions(WTFMove(extensionOutputs));
}
receiveRespond(response.releaseNonNull());
@@ -74,6 +74,7 @@ class CtapAuthenticator final : public FidoAuthenticator {

fido::AuthenticatorGetInfoResponse m_info;
bool m_isDowngraded { false };
bool m_isKeyStoreFull { false };
size_t m_remainingAssertionResponses { 0 };
Vector<Ref<WebCore::AuthenticatorAssertionResponse>> m_assertionResponses;
Vector<uint8_t> m_pinAuth;

0 comments on commit 4ad382e

Please sign in to comment.