Make acceptable AAGUID ckeck in WebAuthn stricter (26.4) (#48517)

* Make acceptable AAGUID ckeck in WebAuthn stricter Closes #48388 Signed-off-by: rmartinc <rmartinc@redhat.com> * Check Acceptable AAGUIDs sets a attestation preference different to None in admin console Closes #48388 Signed-off-by: rmartinc <rmartinc@redhat.com> * Changes for rebase and review. Closes #48388 Signed-off-by: rmartinc <rmartinc@redhat.com> --------- Signed-off-by: rmartinc <rmartinc@redhat.com>
2026-05-07 15:41:29 -05:00 · 2026-04-28 09:27:28 +02:00
parent cff1d42383
commit afd8c93959
8 changed files with 103 additions and 33 deletions
@@ -122,7 +122,7 @@ The configurable items and their description are as follows:
 |If enabled, {project_name} cannot re-register an already registered WebAuthn authenticator.

 |Acceptable AAGUIDs
-|The white list of AAGUIDs which a WebAuthn authenticator must register against.
+|The list of allowed AAGUIDs which a WebAuthn authenticator must register against. An AAGUID (Authenticator Attestation Global Unique Identifier) is a 128-bit identifier indicating the authenticator's type (e.g., make and model). This option needs the **Attestation conveyance preference** to be configured (normally `Direct`) to ensure a trusted AAGUID is passed. Default attestation `None` is not reliable, and can anonymize the AAGUID to zero value. If you setup **Acceptable AAGUIDs** only those authenticators are valid to register new WebAuthn credentials.

 |===

@@ -15,3 +15,16 @@ bin/kc.[sh|bat] start --spi-connections-http-client--default--allow-redirects tr
 ----

 See link:{server_guide_base_link}/outgoinghttp[Outgoing HTTP requests documentation] for more information.
+
+== Notable changes
+
+Notable changes may include internal behavior changes that prevent common misconfigurations, bugs that are fixed, or changes to simplify running {project_name}.
+It also lists significant changes to internal APIs.
+
+=== WebAuthn acceptable AAGUIDs option restricts authenticators strictly
+
+The WebAuthn policy presents the option **Acceptable AAGUIDs** to restrict the authenticators that are allowed to register new credentials. The AAGUID (Authenticator Attestation Global Unique Identifier) is an identifier for the authenticator's type (e.g., make and model). This option requires the **Attestation conveyance preference** to be configured too (normally `Direct`), in order to force the authenticator to include the attestation inside the registration data.
+
+Since this release, when this option is set up, the attestation is required to be present and signed with a valid certificate for the {project_name} trust-store. The `None` attestation format is explicitly not permitted. Previously, there were some corner cases in which a self attestation was accepted. The change is expected to be harmless, but maybe there are combinations of authenticators and WebAuthn policies that can present issues.
+
+See chapter link:{adminguide_link}#_webauthn-policy[Managing policy] in the {adminguide_name} for more information.
@@ -604,7 +604,8 @@ regenerate=Regenerate
 ignoreMissingGroups=Ignore missing groups
 sslType.external=External requests
 showMetaData=Show metadata
-webAuthnPolicyAttestationConveyancePreferenceHelp=Communicates to an authenticator the preference of how to generate an attestation statement.
+webAuthnPolicyAttestationConveyancePreferenceHelp=Communicates to an authenticator the preference of how to generate an attestation statement. None is used by specification if not specified.
+acceptableAAGUIDsRequiresAttestation=Acceptable AAGUIDs require an attestation conveyance preference to be set and not None
 top-level-flow-type.basic-flow=Basic flow
 groupRemoveError=Error removing group {error}
 temporaryPasswordHelpText=If enabled, the user must change the password on the next login
@@ -1309,7 +1310,7 @@ revoke=Revoke
 admin=Admin
 syncUsersError=Could not sync users\: '{{error}}'
 generatedAccessTokenHelp=See the example access token, which will be generated and sent to the client when the selected user is authenticated. You can see claims and roles that the token will contain based on the effective protocol mappers and role scope mappings and also based on the claims and roles assigned to the actual user.
-webAuthnPolicyAcceptableAaguidsHelp=The list of allowed AAGUIDs of which an authenticator can be registered. An AAGUID is a 128-bit identifier indicating the authenticator's type (e.g., make and model).
+webAuthnPolicyAcceptableAaguidsHelp=The list of allowed AAGUIDs of which an authenticator can be registered. An AAGUID is a 128-bit identifier indicating the authenticator's type (e.g., make and model). This option needs the Attestation conveyance preference to be configured (normally `Direct`) to ensure a trusted AAGUID is passed. Default attestation `None` is not reliable, and can anonymize the AAGUID to zero value.
 keyPasswordHelp=Password for the private key
 frontchannelLogout=Front channel logout
 clientUpdaterTrustedHostsTooltip=List of Hosts, which are trusted. If that client registration or update request comes from the host/domain specified in this configuration, the condition evaluates to true. You can use hostnames or IP addresses. If you use star at the beginning (for example '*.example.com'), the whole domain example.com is trusted.
@@ -1,4 +1,5 @@
 import type RealmRepresentation from "@keycloak/keycloak-admin-client/lib/defs/realmRepresentation";
+import type { FieldValues } from "react-hook-form";
 import {
  ActionGroup,
  AlertVariant,
@@ -12,7 +13,7 @@ import {
 } from "@patternfly/react-core";
 import { QuestionCircleIcon } from "@patternfly/react-icons";
 import { useEffect } from "react";
-import { FormProvider, useForm } from "react-hook-form";
+import { FormProvider, useForm, Validate } from "react-hook-form";
 import { useTranslation } from "react-i18next";
 import {
  HelpItem,
@@ -71,6 +72,7 @@ type WeauthnSelectProps = {
  options: readonly string[];
  labelPrefix?: string;
  isMultiSelect?: boolean;
+  validate?: Validate<any, FieldValues>;
 };

 const WebauthnSelect = ({
@@ -80,6 +82,7 @@ const WebauthnSelect = ({
  options,
  labelPrefix,
  isMultiSelect = false,
+  validate,
 }: WeauthnSelectProps) => {
  const { t } = useTranslation();
  return (
@@ -88,7 +91,7 @@ const WebauthnSelect = ({
      label={label}
      labelIcon={labelIcon}
      variant={isMultiSelect ? "typeaheadMulti" : "single"}
-      controller={{ defaultValue: options[0] }}
+      controller={{ defaultValue: options[0], rules: { validate: validate } }}
      options={options.map((option) => ({
        key: option,
        value: labelPrefix ? t(`${labelPrefix}.${option}`) : option,
@@ -118,6 +121,7 @@ export const WebauthnPolicy = ({
  const {
    setValue,
    handleSubmit,
+    watch,
    formState: { isDirty },
  } = form;

@@ -143,6 +147,7 @@ export const WebauthnPolicy = ({
  };

  const isFeatureEnabled = useIsFeatureEnabled();
+  const acceptableAAGUIDs = watch(`${namePrefix}AcceptableAaguids`, []);

  return (
    <PageSection variant="light">
@@ -187,6 +192,18 @@ export const WebauthnPolicy = ({
            labelIcon={t("webAuthnPolicyAttestationConveyancePreferenceHelp")}
            options={ATTESTATION_PREFERENCE}
            labelPrefix="attestationPreference"
+            validate={(value) => {
+              const hasValidAAGUIDs = acceptableAAGUIDs.some(
+                (guid: string) => guid?.trim().length > 0,
+              );
+
+              if (
+                (value === "none" || value === "not specified") &&
+                hasValidAAGUIDs
+              ) {
+                return t("acceptableAAGUIDsRequiresAttestation");
+              }
+            }}
          />
          <WebauthnSelect
            name={`${namePrefix}AuthenticatorAttachment`}
@@ -65,6 +65,7 @@ import com.webauthn4j.data.AttestationConveyancePreference;
 import com.webauthn4j.data.attestation.authenticator.AttestedCredentialData;
 import com.webauthn4j.data.attestation.statement.AttestationStatement;
 import com.webauthn4j.data.attestation.statement.COSEAlgorithmIdentifier;
+import com.webauthn4j.data.attestation.statement.NoneAttestationStatement;
 import com.webauthn4j.data.client.Origin;
 import com.webauthn4j.data.client.challenge.Challenge;
 import com.webauthn4j.data.client.challenge.DefaultChallenge;
@@ -266,7 +267,7 @@ public class WebAuthnRegister implements RequiredActionProvider, CredentialRegis
            AuthenticatorUtil.logoutOtherSessions(context);
        }

-        WebAuthnRegistrationManager webAuthnRegistrationManager = createWebAuthnRegistrationManager(policy.getAttestationConveyancePreference());
+        WebAuthnRegistrationManager webAuthnRegistrationManager = createWebAuthnRegistrationManager(policy);
        try {
            // parse
            RegistrationData registrationData = webAuthnRegistrationManager.parse(registrationRequest);
@@ -316,11 +317,12 @@ public class WebAuthnRegister implements RequiredActionProvider, CredentialRegis
     * Create WebAuthnRegistrationManager instance
     * Can be overridden in subclasses to customize the used attestation validators
     *
-     * @param attestationPreference The attestation selected in the policy
+     * @param policy The webauthn policy defined
     * @return webauthn4j WebAuthnRegistrationManager instance
     */
-    protected WebAuthnRegistrationManager createWebAuthnRegistrationManager(String attestationPreference) {
+    protected WebAuthnRegistrationManager createWebAuthnRegistrationManager(WebAuthnPolicy policy) {
        List<AttestationStatementVerifier> verifiers = new ArrayList<>(6);
+        final String attestationPreference = policy.getAttestationConveyancePreference();
        if (attestationPreference == null
                || Constants.DEFAULT_WEBAUTHN_POLICY_NOT_SPECIFIED.equals(attestationPreference)
                || AttestationConveyancePreference.NONE.getValue().equals(attestationPreference)) {
@@ -332,10 +334,15 @@ public class WebAuthnRegister implements RequiredActionProvider, CredentialRegis
        verifiers.add(new AndroidSafetyNetAttestationStatementVerifier());
        verifiers.add(new FIDOU2FAttestationStatementVerifier());

+        DefaultSelfAttestationTrustworthinessVerifier selfAttestationVerifier = new DefaultSelfAttestationTrustworthinessVerifier();
+        final List<String> acceptableAaguids = policy.getAcceptableAaguids();
+        // self attestation should be disabled to be sure the AAGUID can be trusted
+        selfAttestationVerifier.setSelfAttestationAllowed(acceptableAaguids == null || acceptableAaguids.isEmpty());
+
        return new WebAuthnRegistrationManager(
                verifiers,
                this.certPathtrustVerifier,
-                new DefaultSelfAttestationTrustworthinessVerifier(),
+                selfAttestationVerifier,
                Collections.emptyList(), // Custom Registration Verifier is not supported
                new ObjectConverter()
        );
@@ -404,20 +411,12 @@ public class WebAuthnRegister implements RequiredActionProvider, CredentialRegis
    private void checkAcceptedAuthenticator(RegistrationData response, WebAuthnPolicy policy) throws Exception {
        String aaguid = response.getAttestationObject().getAuthenticatorData().getAttestedCredentialData().getAaguid().toString();
        List<String> acceptableAaguids = policy.getAcceptableAaguids();
-        boolean isAcceptedAuthenticator = false;
        if (acceptableAaguids != null && !acceptableAaguids.isEmpty()) {
-            for(String acceptableAaguid : acceptableAaguids) {
-                if (aaguid.equals(acceptableAaguid)) {
-                    isAcceptedAuthenticator = true;
-                    break;
-                }
+            if (NoneAttestationStatement.FORMAT.equals(response.getAttestationObject().getFormat())) {
+                throw new WebAuthnException("Acceptable AAGUIDs require an attestation format other than 'none'.");
+            } else if (acceptableAaguids.stream().noneMatch(acceptableAaguid -> aaguid.equals(acceptableAaguid))) {
+                throw new WebAuthnException("not acceptable aaguid = " + aaguid);
            }
-        } else {
-            // no accepted authenticators means accepting any kind of authenticator
-            isAcceptedAuthenticator = true;
-        }
-        if (!isAcceptedAuthenticator) {
-            throw new WebAuthnException("not acceptable aaguid = " + aaguid);
        }
    }

@@ -118,6 +118,7 @@ public abstract class AbstractWebAuthnVirtualTest extends AbstractChangeImported

    protected static final String ALL_ZERO_AAGUID = "00000000-0000-0000-0000-000000000000";
    protected static final String ALL_ONE_AAGUID = "11111111-1111-1111-1111-111111111111";
+    protected static final String CHROME_AAGUID = "01020304-0506-0708-0102-030405060708";
    protected static final String USERNAME = "UserWebAuthn";
    protected static final String EMAIL = "UserWebAuthn@email";

@@ -55,7 +55,6 @@ import org.keycloak.util.JsonSerialization;
 import org.openqa.selenium.firefox.FirefoxDriver;

 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
@@ -86,12 +85,6 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest {
    public void addTestRealms(List<RealmRepresentation> testRealms) {
        RealmRepresentation realmRepresentation = AbstractAdminTest.loadJson(getClass().getResourceAsStream("/webauthn/testrealm-webauthn.json"), RealmRepresentation.class);

-        List<String> acceptableAaguids = new ArrayList<>();
-        acceptableAaguids.add("00000000-0000-0000-0000-000000000000");
-        acceptableAaguids.add("6d44ba9b-f6ec-2e49-b930-0c8fe920cb73");
-
-        realmRepresentation.setWebAuthnPolicyAcceptableAaguids(acceptableAaguids);
-
        testRealms.add(realmRepresentation);
        configureTestRealm(realmRepresentation);
    }
@@ -506,8 +499,7 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest {
                .setWebAuthnPolicyAuthenticatorAttachment("cross-platform")
                .setWebAuthnPolicyRequireResidentKey("No")
                .setWebAuthnPolicyRpId(null)
-                .setWebAuthnPolicyUserVerificationRequirement("preferred")
-                .setWebAuthnPolicyAcceptableAaguids(Collections.singletonList(ALL_ZERO_AAGUID));
+                .setWebAuthnPolicyUserVerificationRequirement("preferred");
    }

    /**
@@ -167,19 +167,66 @@ public class WebAuthnOtherSettingsTest extends AbstractWebAuthnVirtualTest {
    @Test
    @IgnoreBrowserDriver(FirefoxDriver.class) // See https://github.com/keycloak/keycloak/issues/10368
    public void excludeCredentials() throws IOException {
-        List<String> acceptableAaguids = Collections.singletonList(ALL_ONE_AAGUID);
+        List<String> acceptableAaguids = Collections.singletonList(ALL_ZERO_AAGUID);
+
+        try (Closeable u = getWebAuthnRealmUpdater()
+                .setWebAuthnPolicyAcceptableAaguids(acceptableAaguids)
+                .setWebAuthnPolicyAttestationConveyancePreference(AttestationConveyancePreference.DIRECT.getValue())
+                .update()) {
+            // webauthn virtual emulator in chrome sets a self signed certificate every time, truststore needs to be disabled
+            testingClient.testing().disableTruststoreSpi();
+
+            WebAuthnRealmData realmData = new WebAuthnRealmData(testRealm().toRepresentation(), isPasswordless());
+            assertThat(realmData.getAcceptableAaguids(), Matchers.contains(ALL_ZERO_AAGUID));
+
+            registerDefaultUser();
+
+            webAuthnErrorPage.assertCurrent();
+            assertThat(webAuthnErrorPage.getError(), allOf(containsString("not acceptable aaguid"), containsString(CHROME_AAGUID)));
+        } finally {
+            testingClient.testing().reenableTruststoreSpi();
+        }
+    }
+
+    @Test
+    @IgnoreBrowserDriver(FirefoxDriver.class) // See https://github.com/keycloak/keycloak/issues/10368
+    public void excludeCredentialsSuccess() throws IOException {
+        List<String> acceptableAaguids = Collections.singletonList(CHROME_AAGUID);
+
+        try (Closeable u = getWebAuthnRealmUpdater()
+                .setWebAuthnPolicyAcceptableAaguids(acceptableAaguids)
+                .setWebAuthnPolicyAttestationConveyancePreference(AttestationConveyancePreference.DIRECT.getValue())
+                .update()) {
+            // webauthn virtual emulator in chrome sets a self signed certificate every time, truststore needs to be disabled
+            testingClient.testing().disableTruststoreSpi();
+
+            WebAuthnRealmData realmData = new WebAuthnRealmData(testRealm().toRepresentation(), isPasswordless());
+            assertThat(realmData.getAcceptableAaguids(), Matchers.contains(CHROME_AAGUID));
+
+            registerDefaultUser();
+
+            appPage.assertCurrent();
+        } finally {
+            testingClient.testing().reenableTruststoreSpi();
+        }
+    }
+
+    @Test
+    @IgnoreBrowserDriver(FirefoxDriver.class) // See https://github.com/keycloak/keycloak/issues/10368
+    public void excludeCredentialsUsingNone() throws IOException {
+        List<String> acceptableAaguids = Collections.singletonList(ALL_ZERO_AAGUID);

        try (Closeable u = getWebAuthnRealmUpdater()
                .setWebAuthnPolicyAcceptableAaguids(acceptableAaguids)
                .update()) {

            WebAuthnRealmData realmData = new WebAuthnRealmData(testRealm().toRepresentation(), isPasswordless());
-            assertThat(realmData.getAcceptableAaguids(), Matchers.contains(ALL_ONE_AAGUID));
+            assertThat(realmData.getAcceptableAaguids(), Matchers.contains(ALL_ZERO_AAGUID));

            registerDefaultUser();

            webAuthnErrorPage.assertCurrent();
-            assertThat(webAuthnErrorPage.getError(), allOf(containsString("not acceptable aaguid"), containsString(ALL_ZERO_AAGUID)));
+            assertThat(webAuthnErrorPage.getError(), containsString("Acceptable AAGUIDs require an attestation format other than 'none'."));
        }
    }
 }