[java-idp-plugin-webauthn] branch main updated: JWEBAUTHN-29 - username that does not canonicalize leads to uncaught exception
Phil Smart
philip.smart at jisc.ac.uk
Thu Oct 3 16:12:34 UTC 2024
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=90d05209861a9f6c87f1523ee85d236da6b320c2
The following commit(s) were added to refs/heads/main by this push:
new 90d0520 JWEBAUTHN-29 - username that does not canonicalize leads to uncaught exception
90d0520 is described below
commit 90d05209861a9f6c87f1523ee85d236da6b320c2
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Oct 3 17:12:32 2024 +0100
JWEBAUTHN-29 - username that does not canonicalize leads to uncaught
exception
- Usernames that do not canonicalize are left blank and any existing
credentials are left empty. The flow then continues as normal to the
authentication subsystem.
- Showing an error here would allow pretty easy username enumeration
https://shibboleth.atlassian.net/browse/JWEBAUTHN-29
---
.../webauthn/impl/LookupRegisteredCredentials.java | 37 +++++++++++++++++++++-
.../webauthn-registration-beans.xml | 5 +++
.../webauthn-registration-flow.xml | 11 ++++---
3 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java
index c638624..745c3ca 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java
@@ -49,7 +49,12 @@ import net.shibboleth.shared.primitive.LoggerFactory;
/**
* An action that lookups existing registered credentials based on the username contained in the WebAuthn context
* and sets them back onto the context. Also sets the user.id from the userHandle associated with the username.
+ *
+ * <p>
+ * If the username is not required and is not found in the context (e.g. failed c14n) the set of existing credentials
+ * will remain empty. If the username is required and is not found in the context an error event will be returned.
* If no credentials exist and the trigger event condition is set, an error event will be produced.
+ * </p>
*
* @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
* @post BaseWebAuthnContext.setExistingCredentials() is either null if no existing credentials are found, or contains
@@ -73,6 +78,9 @@ public class LookupRegisteredCredentials extends AbstractWebAuthnAction<BaseWebA
/** The credential repository to use.*/
@NonnullAfterInit private WebAuthnCredentialRepository repository;
+
+ /** Is the username required? If not, just leave an empty set of existing credentials. */
+ @Nonnull private Predicate<ProfileRequestContext> usernameRequiredPredicate;
/** Constructor. */
public LookupRegisteredCredentials() {
@@ -80,6 +88,27 @@ public class LookupRegisteredCredentials extends AbstractWebAuthnAction<BaseWebA
compose(new ChildContextLookup<>(AuthenticationContext.class)));
noCredentialsEventId = WebAuthnAuthenticationEventIds.NO_REGISTERED_WEBAUTHN_CREDENTIALS;
triggerEventOnNoCredentialsPredicate = PredicateSupport.alwaysFalse();
+ usernameRequiredPredicate = PredicateSupport.alwaysTrue();
+ }
+
+ /**
+ * Set a flag to determine if the username is required or not.
+ *
+ * @param flag is the username required?
+ */
+ public void setUsernameRequired(final boolean flag) {
+ checkSetterPreconditions();
+ usernameRequiredPredicate = flag ? PredicateSupport.alwaysTrue() : PredicateSupport.alwaysFalse();
+ }
+
+ /**
+ * Set a strategy to determine if the username is required or not.
+ *
+ * @param predicate the predicate to set.
+ */
+ public void setUsernameRequiredPredicate(@Nonnull final Predicate<ProfileRequestContext> predicate){
+ checkSetterPreconditions();
+ usernameRequiredPredicate = Constraint.isNotNull(predicate, "Username required predicate can not be null");
}
/** {@inheritDoc} */
@@ -129,7 +158,13 @@ public class LookupRegisteredCredentials extends AbstractWebAuthnAction<BaseWebA
@Nonnull final BaseWebAuthnContext context) {
final String username = context.getUsername();
- if (username == null) {
+ // If we do not find a username and we do not require a username, just leave the set of existing credentials
+ // blank
+ if (username == null && !usernameRequiredPredicate.test(profileRequestContext)) {
+ log.error("{} Unable to find username in WebAuthn context, no credentials found", getLogPrefix());
+ return;
+ }
+ if (username == null && usernameRequiredPredicate.test(profileRequestContext)) {
log.error("{} Unable to find username in WebAuthn context", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
return;
diff --git a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
index 6fcf0d9..1872f15 100644
--- a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
@@ -72,6 +72,11 @@
<bean id="UpdateRegistrationContextUsernameWithC14nPrincipal" parent="AbstractWebAuthnBaseAction" scope="prototype"
class="net.shibboleth.idp.plugin.authn.webauthn.impl.UpdateWebAuthnContextWithC14nPrincipal"
p:webAuthnContextLookupStrategy-ref="shibboleth.ChildLookup.WebAuthnRegistrationContext"/>
+
+ <bean id="LookupRegisteredCredentialsPostC14n" parent="AbstractWebAuthnBaseAction" scope="prototype"
+ class="net.shibboleth.idp.plugin.authn.webauthn.impl.LookupRegisteredCredentials"
+ p:webAuthnContextLookupStrategy-ref="shibboleth.ChildLookup.WebAuthnRegistrationContext"
+ p:usernameRequired="false"/>
<!-- Start of registration flow post authentication -->
diff --git a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml
index 2b69ddf..5b1ec99 100644
--- a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml
@@ -64,15 +64,16 @@
and align with the authentication result -->
<subflow-state id="CallSubjectCanonicalization" subflow="c14n">
<input name="calledAsSubflow" value="true" />
- <transition on="proceed" to="LookupRegisteredCredentials" />
-
- <transition on="SubjectCanonicalizationError" to="InvalidSubjectCanonicalizationContext" />
+ <transition on="proceed" to="LookupRegisteredCredentialsPostC14n" />
+
+ <!-- continue on error, we will just have no existing credentials -->
+ <transition on="SubjectCanonicalizationError" to="LookupRegisteredCredentialsPostC14n"/>
</subflow-state>
<!-- After we canonicalize the username input, use that to lookup registered credentials -->
- <action-state id="LookupRegisteredCredentials">
+ <action-state id="LookupRegisteredCredentialsPostC14n">
<evaluate expression="UpdateRegistrationContextUsernameWithC14nPrincipal"/>
- <evaluate expression="LookupRegisteredCredentials"/>
+ <evaluate expression="LookupRegisteredCredentialsPostC14n"/>
<evaluate expression="'proceed'" />
<!-- Branch to determine if authentication is required. -->
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list