[java-idp-plugin-webauthn] branch main updated: JWEBAUTHN-42 - credential policies are not evaluated for all keys
Phil Smart
philip.smart at jisc.ac.uk
Mon Jan 27 17:51:50 UTC 2025
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-idp-plugin-webauthn.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-webauthn.git;a=commit;h=1fcf503246c34664ae75854480b4c4b674700316
The following commit(s) were added to refs/heads/main by this push:
new 1fcf503 JWEBAUTHN-42 - credential policies are not evaluated for all keys
1fcf503 is described below
commit 1fcf503246c34664ae75854480b4c4b674700316
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Jan 27 17:51:46 2025 +0000
JWEBAUTHN-42 - credential policies are not evaluated for all keys
- Add flag to reject an assertion if the credential used can not be
found from the repository. Defaults to false, to maintain existing
compatibility.
- Improve logging
- Inject strategies and flags
https://shibboleth.atlassian.net/browse/JWEBAUTHN-42
---
.../authn/webauthn/impl/CheckCredentialPolicy.java | 80 +++++++++++++++++---
.../impl/DefaultUserHandleLookupStrategy.java | 1 +
.../idp/flows/authn/WebAuthn/webauthn-beans.xml | 7 +-
.../authn/webauthn/conf/authn/webauthn.properties | 2 +
.../webauthn/impl/CheckCredentialPolicyTest.java | 88 ++++++++++++++++++++--
5 files changed, 161 insertions(+), 17 deletions(-)
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicy.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicy.java
index 663c936..1d5bcd8 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicy.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicy.java
@@ -17,6 +17,7 @@ package net.shibboleth.idp.plugin.authn.webauthn.impl;
import java.util.Collection;
import java.util.List;
import java.util.function.Function;
+import java.util.function.Predicate;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -45,10 +46,14 @@ import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.logic.PredicateSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * A policy engine action that checks with the configured policy if the credential can be used to authenticate.
+ * A policy engine action that checks with the configured policy if the credential, used to sign the assertion, can
+ * be used to authenticate. If #rejectIfNoCredentialsFound is true, and the credential used to sign the assertion
+ * can not be found from the credential repository, a {@link WebAuthnAuthenticationEventIds#CREDENTIAL_POLICY_REJECTION}
+ * will be returned, else the policy is ignored.
*
* @event {EventIds#INVALID_PROFILE_CTX}\
* @event {WebAuthnAuthenticationEventIds#CREDENTIAL_POLICY_REJECTION}
@@ -65,7 +70,13 @@ public class CheckCredentialPolicy extends AbstractWebAuthnAction<WebAuthnAuthen
private PublicKeyCredential<AuthenticatorAssertionResponse, ClientAssertionExtensionOutputs> assertion;
/** Get the userHandle to help find the credential used to sign the assertion.*/
- @NonnullAfterInit private Function<ProfileRequestContext, byte[]> userHandleLookupStrategy;
+ @Nonnull private Function<ProfileRequestContext, byte[]> userHandleLookupStrategy;
+
+ /**
+ * Reject the assertion if no credential can be found in the repository, otherwise the policy is ignored.
+ * Defaults to false.
+ */
+ @Nonnull private Predicate<ProfileRequestContext> rejectIfNoCredentialFound;
/** The credential policy to check.*/
@Nullable private CredentialPolicy credentialPolicy;
@@ -83,6 +94,7 @@ public class CheckCredentialPolicy extends AbstractWebAuthnAction<WebAuthnAuthen
super(new ChildContextLookup<>(WebAuthnAuthenticationContext.class).
compose(new ChildContextLookup<>(AuthenticationContext.class)));
userHandleLookupStrategy = new DefaultUserHandleLookupStrategy();
+ rejectIfNoCredentialFound = PredicateSupport.alwaysFalse();
}
/**
@@ -100,6 +112,8 @@ public class CheckCredentialPolicy extends AbstractWebAuthnAction<WebAuthnAuthen
* assertion.
*
* @param strategy The user handle lookup strategy to set.
+ *
+ * @since 1.1.0
*/
public void setUserHandleLookupStrategy(final Function<ProfileRequestContext, byte[]> strategy) {
checkSetterPreconditions();
@@ -107,6 +121,31 @@ public class CheckCredentialPolicy extends AbstractWebAuthnAction<WebAuthnAuthen
"UserHandle lookup strategy can not be null");
}
+ /**
+ * Set the predicate to determine if finding no credential to test should result in a policy rejection.
+ *
+ * @param predicate The predicate to set.
+ *
+ * @since 1.1.0
+ */
+ public void setRejectIfNoCredentialFoundPredicate(final Predicate<ProfileRequestContext> predicate) {
+ checkSetterPreconditions();
+ rejectIfNoCredentialFound = Constraint.isNotNull(predicate,
+ "rejectIfNoCredentialsFound can not be null");
+ }
+
+ /**
+ * Set the flag to determine if finding no credential to test should result in a policy rejection.
+ *
+ * @param flag The flag to set.
+ *
+ * @since 1.1.0
+ */
+ public void setRejectIfNoCredentialFound(final boolean flag) {
+ checkSetterPreconditions();
+ rejectIfNoCredentialFound = flag ? PredicateSupport.alwaysTrue() : PredicateSupport.alwaysFalse();
+ }
+
/** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
@@ -156,7 +195,15 @@ public class CheckCredentialPolicy extends AbstractWebAuthnAction<WebAuthnAuthen
final byte[] userHandleBytes = userHandleLookupStrategy.apply(profileRequestContext);
if (userHandleBytes == null) {
log.debug("{} UserHandle could not be found, policy can not be applied",getLogPrefix());
- return;
+ if (!rejectIfNoCredentialFound.test(profileRequestContext)) {
+ return;
+ } else {
+ authnContext.ensureSubcontext(AuthenticationErrorContext.class).getClassifiedErrors().add(
+ WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
+ ActionSupport.buildEvent(profileRequestContext,
+ WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
+ return;
+ }
}
final Collection<EnhancedCredentialRecord> registeredCredentials = context.getExistingCredentials();
@@ -170,19 +217,32 @@ public class CheckCredentialPolicy extends AbstractWebAuthnAction<WebAuthnAuthen
if (credentials.isEmpty()) {
log.trace("{} UserHandle '{}' has no registered credential, policy can not be applied",getLogPrefix(),
- userHandle.getHex());
- return;
+ userHandle.getBase64());
+ if (!rejectIfNoCredentialFound.test(profileRequestContext)) {
+ return;
+ } else {
+ authnContext.ensureSubcontext(AuthenticationErrorContext.class).getClassifiedErrors().add(
+ WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
+ ActionSupport.buildEvent(profileRequestContext,
+ WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
+ return;
+ }
}
// Reject if more than one credential that matches the credentialID and userHandle in the assertion.
if (credentials.size() != 1) {
log.debug("{} Credential ID '{}' for userHandle '{}' has more than one registered credential, "
+ "policy can not be applied, rejecting",getLogPrefix(), assertion.getId(),
- userHandle.getHex());
- authnContext.ensureSubcontext(AuthenticationErrorContext.class).getClassifiedErrors().add(
- WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
- ActionSupport.buildEvent(profileRequestContext, WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
- return;
+ userHandle.getHex());
+ if (!rejectIfNoCredentialFound.test(profileRequestContext)) {
+ return;
+ } else {
+ authnContext.ensureSubcontext(AuthenticationErrorContext.class).getClassifiedErrors().add(
+ WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
+ ActionSupport.buildEvent(profileRequestContext,
+ WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
+ return;
+ }
}
final EnhancedCredentialRecord credentialToEvaluate = credentials.get(0);
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/DefaultUserHandleLookupStrategy.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/DefaultUserHandleLookupStrategy.java
index 827ffaa..c9a10be 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/DefaultUserHandleLookupStrategy.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/DefaultUserHandleLookupStrategy.java
@@ -69,6 +69,7 @@ public class DefaultUserHandleLookupStrategy implements Function<ProfileRequestC
log.trace("Found userHandle from userId in authentication context");
return webAuthnContext.getUserId();
} else {
+ log.trace("Unable to find userHandle from either the assertion or the authentication context");
return null;
}
diff --git a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/WebAuthn/webauthn-beans.xml b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/WebAuthn/webauthn-beans.xml
index 2b4a82f..1aee6c5 100644
--- a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/WebAuthn/webauthn-beans.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/WebAuthn/webauthn-beans.xml
@@ -148,8 +148,13 @@
class="net.shibboleth.idp.plugin.authn.webauthn.impl.CheckCredentialPolicy"
p:credentialRepository="#{getObject('shibboleth.authn.WebAuthn.CredentialRepository') ?: getObject('shibboleth.authn.WebAuthn.DefaultCredentialRepository')}"
p:credentialPolicy="#{getObject('%{idp.authn.webauthn.credential.policy:shibboleth.authn.WebAuthn.ChainedCredentialPolicy}')}"
- p:activationCondition="%{idp.authn.webauthn.credential.policy.enabled:false}"/>
+ p:activationCondition="%{idp.authn.webauthn.credential.policy.enabled:false}"
+ p:rejectIfNoCredentialFound="%{idp.authn.webauthn.credential.policy.rejectIfNoCredentials:false}"
+ p:userHandleLookupStrategy="#{getObject('shibboleth.authn.WebAuthn.CredentialPolicyUserHandleLookupStrategy') ?: getObject('DefaultCredentialPolicyUserHandleLookupStrategy')}"/>
+ <bean id="DefaultCredentialPolicyUserHandleLookupStrategy" scope="prototype"
+ class="net.shibboleth.idp.plugin.authn.webauthn.impl.DefaultUserHandleLookupStrategy"/>
+
<bean id="shibboleth.authn.WebAuthn.ChainedCredentialPolicy" scope="prototype"
class="net.shibboleth.idp.plugin.authn.webauthn.policy.impl.ChainingCredentialPolicyRule"
p:credentialPolicyChain="#{getObject('%{idp.authn.webauthn.credential.policy.chainedlist:shibboleth.authn.WebAuthn.ChainedCredentialPolicyList}')}"/>
diff --git a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties
index 52303f9..162bea3 100644
--- a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties
+++ b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties
@@ -66,6 +66,8 @@ idp.authn.webauthn.supportedPrincipals = \
# Enable the credential policy engine
#idp.authn.webauthn.credential.policy.enabled = false
+# Should the policy reject the credential used to sign the assertion if no credentials can be found in the repository for the user
+#idp.authn.webauthn.credential.policy.rejectIfNoCredentials = false
# Set the credential policies to use, defaults to a chained set of policies
#idp.authn.webauthn.credential.policy = shibboleth.authn.WebAuthn.ChainedCredentialPolicy
# When using the default chained policy, which policy list should we use?
diff --git a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicyTest.java b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicyTest.java
index b229ade..66a4c39 100644
--- a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicyTest.java
+++ b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/CheckCredentialPolicyTest.java
@@ -18,6 +18,7 @@ import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
+import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.util.Arrays;
import java.util.Map;
@@ -62,6 +63,9 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
/** The stashed attestation.*/
private PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs> attestation;
+ /** The stashed userHandle.*/
+ private ByteArray uHandleArray;
+
@Override
@BeforeMethod
@@ -76,7 +80,7 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
mockAuthenticator = new MockAuthenticator(RPID);
final var user = UserIdentity.builder()
- .name("jdoe")
+ .name(USERNAME)
.displayName("John Doe")
.id(ByteArray.fromBase64(USER_HANDLE_B64))
.build();
@@ -84,7 +88,7 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
attestation = createAttestationReponse();
- final var uHandleArray = ByteArray.fromBase64(USER_HANDLE_B64);
+ uHandleArray = ByteArray.fromBase64(USER_HANDLE_B64);
assert uHandleArray.getBase64().equals(USER_HANDLE_B64);
assert Arrays.equals(uHandleArray.getBytes(), Base64Support.decode(USER_HANDLE_B64));
@@ -113,14 +117,14 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
credentialRepo.addRegistrationByUsername(USERNAME, reg);
context.setExistingCredentials(CollectionSupport.setOf(
new EnhancedCredentialRecord(reg)));
- context.setUserId(uHandleArray.getBytes());
+
context.setUsername(USERNAME);
}
@Test
public void testCredentialPolicy_Accept() throws Exception {
-
+ context.setUserId(uHandleArray.getBytes());
action.setCredentialPolicy(new CredentialPolicy() {
@Override
@@ -149,9 +153,47 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
assertNull(event);
}
+ /*
+ * Test to ensure if a different userId got into the authentication context, which does not match to the
+ * credential ID used to sign the assertion, the policy will not run.
+ */
@Test
- public void testCredentialPolicy_Rejected() throws Exception {
+ public void testCredentialPolicy_Accept_DifferentUserIDInContext() throws Exception {
+ context.setUserId(uHandleArray.getBytes());
+ action.setCredentialPolicy(new CredentialPolicy() {
+
+ @Override
+ public String getId() {
+ return "Dummy Accept Policy";
+ }
+
+ @Override
+ public CredentialPolicyOutcome evaluate(final EnhancedCredentialRecord credential, final ProfileRequestContext prc) {
+ return CredentialPolicyOutcome.ACCEPT;
+ }
+
+ });
+ action.initialize();
+
+ final Map<String, String> clientDataGet = createClientData("webauthn.get", ORIGIN, CHALLENGE_B64);
+
+ final PublicKeyCredential<AuthenticatorAssertionResponse, ClientAssertionExtensionOutputs>
+ assertion = mockAuthenticator.createAuthenticatorAssertionResponse(attestation.getId().getBytes(),
+ clientDataGet, null, true);
+ //Set the assertion (authentication) response based on the credential we've already registered
+ context.setPublicKeyCredentialAssertionResponse(assertion);
+
+ //Now set the userId in the context to something different
+ context.setUserId("thisisdifferent".getBytes(StandardCharsets.UTF_8));
+
+ final Event event = action.execute(src);
+ assertNull(event);
+ }
+
+ @Test
+ public void testCredentialPolicy_Rejected() throws Exception {
+ context.setUserId(uHandleArray.getBytes());
action.setCredentialPolicy(new CredentialPolicy() {
@Override
@@ -186,7 +228,7 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
* to contain a userHandle, the IdP already knows it.*/
@Test
public void testCredentialPolicy_NoUserHandle() throws Exception {
-
+ context.setUserId(uHandleArray.getBytes());
action.setCredentialPolicy(new CredentialPolicy() {
@Override
@@ -245,6 +287,40 @@ public class CheckCredentialPolicyTest extends AbstractWebAuthnTest {
context.setPublicKeyCredentialAssertionResponse(assertion);
+ final Event event = action.execute(src);
+ assertNull(event);
+ }
+
+ /* Policy can not run because userHandle is not present in the assertion and userId is not present in the context.*/
+ @Test
+ public void testCredentialPolicy_NoUserHandle_NoUserId_RejectionEnabled() throws Exception {
+
+ action.setRejectIfNoCredentialFound(true);
+ action.setCredentialPolicy(new CredentialPolicy() {
+
+ @Override
+ public String getId() {
+ return "Dummy Reject Policy";
+ }
+
+ @Override
+ public CredentialPolicyOutcome evaluate(final EnhancedCredentialRecord credential, final ProfileRequestContext prc) {
+ return CredentialPolicyOutcome.REJECT;
+ }
+
+ });
+ action.initialize();
+
+ final Map<String, String> clientDataGet = createClientData("webauthn.get", ORIGIN, CHALLENGE_B64);
+
+ final PublicKeyCredential<AuthenticatorAssertionResponse, ClientAssertionExtensionOutputs>
+ assertion = mockAuthenticator.createAuthenticatorAssertionResponse(attestation.getId().getBytes(),
+ clientDataGet, null, true);
+
+ //Set the assertion (authentication) response based on the credential we've already registered
+ context.setPublicKeyCredentialAssertionResponse(assertion);
+
+
final Event event = action.execute(src);
assert event != null;
assertEquals(event.getId(), WebAuthnAuthenticationEventIds.CREDENTIAL_POLICY_REJECTION);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list