[java-idp-plugin-webauthn] branch main updated: Change Challenge and UserID generation to use a secure IdentifierGenerationStrategy
Phil Smart
philip.smart at jisc.ac.uk
Thu Aug 15 14:20:57 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=873066c5dd7c739e6a1a78bb455e64de6935ecce
The following commit(s) were added to refs/heads/main by this push:
new 873066c Change Challenge and UserID generation to use a secure IdentifierGenerationStrategy
873066c is described below
commit 873066c5dd7c739e6a1a78bb455e64de6935ecce
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Aug 15 15:20:54 2024 +0100
Change Challenge and UserID generation to use a secure
IdentifierGenerationStrategy
- we previously used getInstanceStrong which returns a blocking PRNG.
this can hang in certain server environments.
---
.../authn/webauthn/admin/impl/AddUserId.java | 10 +++--
.../webauthn/admin/impl/RandomUserIdGenerator.java | 48 +++++++++++++-------
.../webauthn/impl/GenerateServerChallenge.java | 52 +++++++++++++++-------
3 files changed, 72 insertions(+), 38 deletions(-)
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
index 5befab7..e4493bf 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
@@ -61,7 +61,7 @@ public class AddUserId extends AbstractWebAuthnAction<WebAuthnRegistrationContex
@Nonnull private final Logger log = LoggerFactory.getLogger(AddUserId.class);
/** Strategy used to generate the user.id. */
- @Nonnull private Function<ProfileRequestContext,byte[]> userIdGeneratorStrategy;
+ @NonnullAfterInit private Function<ProfileRequestContext,byte[]> userIdGeneratorStrategy;
/** The stashed username.*/
@NonnullBeforeExec private String username;
@@ -71,8 +71,7 @@ public class AddUserId extends AbstractWebAuthnAction<WebAuthnRegistrationContex
/** Constructor. */
public AddUserId() {
- super(new ChildContextLookup<>(WebAuthnRegistrationContext.class));
- userIdGeneratorStrategy = new RandomUserIdGenerator();
+ super(new ChildContextLookup<>(WebAuthnRegistrationContext.class));
}
/** {@inheritDoc} */
@@ -83,6 +82,9 @@ public class AddUserId extends AbstractWebAuthnAction<WebAuthnRegistrationContex
if (repository == null) {
throw new ComponentInitializationException("Credential repository can not be null");
}
+ if (userIdGeneratorStrategy == null) {
+ userIdGeneratorStrategy = new RandomUserIdGenerator();
+ }
}
/**
@@ -151,7 +153,7 @@ public class AddUserId extends AbstractWebAuthnAction<WebAuthnRegistrationContex
// Do nothing, just 'null' userId;
userIdBase64 = null;
}
- log.trace("{} Generated user.id '{}'",getLogPrefix(),userIdBase64);
+ log.trace("{} Generated user.id '{}' of size '{}'",getLogPrefix(),userIdBase64, userId.length);
}
context.setUserId(userId);
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java
index b14ae4f..100ef4a 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java
@@ -14,21 +14,27 @@
package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
+import java.security.InvalidAlgorithmParameterException;
import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
import java.util.function.Function;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Hex;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
+import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.security.IdentifierGenerationStrategy;
+import net.shibboleth.shared.security.IdentifierGenerationStrategy.ProviderType;
+import net.shibboleth.shared.security.RandomIdentifierParameterSpec;
/**
- * A user.id generator that generates a random 64 byte user.id.
+ * A user.id generator that generates a random 64 byte user.id. Will return {@code null} if one can not be generated.
*/
@ThreadSafe
public final class RandomUserIdGenerator implements Function<ProfileRequestContext, byte[]>{
@@ -36,25 +42,33 @@ public final class RandomUserIdGenerator implements Function<ProfileRequestConte
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(RandomUserIdGenerator.class);
- /** Secure random generator. */
- @Nullable private final SecureRandom randomGenerator;
+ /** The user.id generation strategy.*/
+ private final IdentifierGenerationStrategy idGeneratorStrategy;
- /** Constructor.*/
- public RandomUserIdGenerator() {
- try {
- randomGenerator = SecureRandom.getInstance("SHA1PRNG");
- } catch (final NoSuchAlgorithmException e) {
- throw new RuntimeException("SHA1PRNG is required to be supported by the JVM but is not", e);
+ /**
+ * Constructor.
+ *
+ * @throws ComponentInitializationException if the secure random implementation can not be created
+ */
+ public RandomUserIdGenerator() throws ComponentInitializationException {
+ try {
+ idGeneratorStrategy = IdentifierGenerationStrategy.getInstance(ProviderType.SECURE,
+ new RandomIdentifierParameterSpec(null, 64, new Hex()));
+ } catch (final InvalidAlgorithmParameterException | NoSuchAlgorithmException e) {
+ throw new ComponentInitializationException(e);
}
}
-
+
/** {@inheritDoc} */
@Override
- @Nullable public byte[] apply(final ProfileRequestContext input) {
- final SecureRandom randomGeneratorLocal = randomGenerator;
- assert randomGeneratorLocal != null;
- final byte[] bytes = new byte[64];
- randomGeneratorLocal.nextBytes(bytes);
- return bytes;
+ @Nullable public byte[] apply(final ProfileRequestContext input) {
+ // It is hex encoded, so decode it
+ final String userIdStringHex = idGeneratorStrategy.generateIdentifier(false);
+ try {
+ return Hex.decodeHex(userIdStringHex);
+ } catch (final DecoderException e) {
+ log.warn("Unable to generate user.id", e);
+ return null;
+ }
}
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
index f585172..9290502 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
@@ -14,13 +14,15 @@
package net.shibboleth.idp.plugin.authn.webauthn.impl;
+import java.security.InvalidAlgorithmParameterException;
import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
import java.util.function.Function;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Hex;
import org.opensaml.messaging.context.navigate.ChildContextLookup;
import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.action.EventIds;
@@ -29,7 +31,12 @@ import org.slf4j.Logger;
import net.shibboleth.idp.authn.context.AuthenticationContext;
import net.shibboleth.idp.plugin.authn.webauthn.context.BaseWebAuthnContext;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.security.IdentifierGenerationStrategy;
+import net.shibboleth.shared.security.IdentifierGenerationStrategy.ProviderType;
+import net.shibboleth.shared.security.RandomIdentifierParameterSpec;
/**
@@ -47,12 +54,18 @@ public class GenerateServerChallenge extends AbstractWebAuthnAction<BaseWebAuthn
@Nonnull private final Logger log = LoggerFactory.getLogger(GenerateServerChallenge.class);
/** Strategy used to generate the challenge to use. */
- @Nonnull private Function<ProfileRequestContext,byte[]> challengeGeneratorStrategy;
+ @NonnullAfterInit private Function<ProfileRequestContext,byte[]> challengeGeneratorStrategy;
/** Constructor. */
public GenerateServerChallenge() {
super(new ChildContextLookup<>(BaseWebAuthnContext.class).
- compose(new ChildContextLookup<>(AuthenticationContext.class)));
+ compose(new ChildContextLookup<>(AuthenticationContext.class)));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void doInitialize() throws ComponentInitializationException {
+ super.doInitialize();
challengeGeneratorStrategy = new DefaultChallengeGenerator();
}
@@ -80,7 +93,7 @@ public class GenerateServerChallenge extends AbstractWebAuthnAction<BaseWebAuthn
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
return;
}
- log.trace("{} Generated server challenge {}",getLogPrefix() ,challenge);
+ log.trace("{} Generated server challenge {} of size '{} bytes'",getLogPrefix() ,challenge, challenge.length);
context.setServerChallenge(challenge);
}
@@ -93,30 +106,35 @@ public class GenerateServerChallenge extends AbstractWebAuthnAction<BaseWebAuthn
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DefaultChallengeGenerator.class);
- /** Random number generator. */
- @Nullable private final SecureRandom randomGenerator;
+ /** The secure random challenge generation strategy.*/
+ private final IdentifierGenerationStrategy challengeGenerator;
- /** Constructor.*/
- public DefaultChallengeGenerator() {
+ /** Constructor.
+ * @throws ComponentInitializationException */
+ public DefaultChallengeGenerator() throws ComponentInitializationException {
try {
- randomGenerator = SecureRandom.getInstance("SHA1PRNG");
- } catch (final NoSuchAlgorithmException e) {
- throw new RuntimeException("SHA1PRNG is required to be supported by the JVM but is not", e);
+ challengeGenerator = IdentifierGenerationStrategy.getInstance(ProviderType.SECURE,
+ new RandomIdentifierParameterSpec(null, 32, new Hex()));
+ } catch (final InvalidAlgorithmParameterException | NoSuchAlgorithmException e) {
+ throw new ComponentInitializationException(e);
}
}
/** {@inheritDoc} */
@Override
@Nullable public byte[] apply(@Nullable final ProfileRequestContext input) {
- if (randomGenerator == null) {
+ if (challengeGenerator == null) {
log.error("Unable to generate challenge, random number generator is null");
return null;
}
- final SecureRandom randomGeneratorLocal = randomGenerator;
- assert randomGeneratorLocal != null;
- final byte[] bytes = new byte[32];
- randomGeneratorLocal.nextBytes(bytes);
- return bytes;
+ final String challengeHexString = challengeGenerator.generateIdentifier(false);
+ try {
+ // We know it was hex encoded, so hex decode to retrieve the bytes
+ return Hex.decodeHex(challengeHexString);
+ } catch (final DecoderException e) {
+ log.warn("Unable to generate challenge", e);
+ return null;
+ }
}
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list