[java-idp-plugin-duo] branch main updated: JDUO-29 - Move JWT Signature Validation to OIDC commons
Phil Smart
philip.smart at jisc.ac.uk
Fri Feb 5 14:15:48 UTC 2021
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-idp-plugin-duo.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=790ae69e8f21ff7e3b31f3f7a2f5f6701c3349b7
The following commit(s) were added to refs/heads/main by this push:
new 790ae69 JDUO-29 - Move JWT Signature Validation to OIDC commons
790ae69 is described below
commit 790ae69e8f21ff7e3b31f3f7a2f5f6701c3349b7
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Feb 5 14:15:45 2021 +0000
JDUO-29 - Move JWT Signature Validation to OIDC commons
Switch signature validation to oidc-commons.
https://issues.shibboleth.net/jira/browse/JDUO-29
---
.../plugin/authn/duo/impl/ValidateTokenClaims.java | 4 +-
.../authn/duo/impl/ValidateTokenSignature.java | 91 ++++++++++------
.../flows/authn/DuoOIDC/duo-oidc-authn-beans.xml | 4 +-
.../flows/authn/DuoOIDC/duo-oidc-authn-flow.xml | 2 +-
.../authn/duo/impl/ValidateTokenClaimsTest.java | 1 -
.../authn/duo/impl/ValidateTokenSignatureTest.java | 16 +++
.../plugin/authn/mock/MockDuoOIDCClient_FAIL.java | 13 +--
.../plugin/authn/mock/MockDuoOIDCClient_OK.java | 13 +--
.../mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java | 13 +--
.../authn/mock/MockDuoOIDCClient_UNKNOWN.java | 13 +--
idp-duo-nimbus-client-impl/pom.xml | 5 +
.../authn/duo/nimbus/impl/NimbusClientSupport.java | 121 ++-------------------
idp-duo-sdk-client-impl/pom.xml | 5 +
.../authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 22 ++--
pom.xml | 2 +-
15 files changed, 134 insertions(+), 191 deletions(-)
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java
index 9ffbaa5..623234f 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java
@@ -46,7 +46,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
/**
* Action that validates the claims of the Duo id_token using the supplied
* {@link JWTClaimsValidation claims validator}. The verifier <b>must</b> be thread-safe and validate the
- * claims set against the OpenID Connect core 1.0 section 3.1.3.7 specification, and those required by Duo.
+ * claims set against the OpenID Connect core 1.0 section 3.1.3.7 specification and those required by Duo.
*
* @pre
*
@@ -94,7 +94,7 @@ public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
super.doInitialize();
if (claimsValidator == null) {
- throw new ComponentInitializationException("Duo ClaimSet Verifier cannot be null");
+ throw new ComponentInitializationException("Duo ClaimSet Validator cannot be null");
}
}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java
index b88fbd8..5fd054b 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java
@@ -18,15 +18,18 @@
package net.shibboleth.idp.plugin.authn.duo.impl;
import java.text.ParseException;
+import java.util.List;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.crypto.spec.SecretKeySpec;
import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.nimbusds.jose.Algorithm;
import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jwt.JWT;
@@ -41,6 +44,12 @@ import net.shibboleth.idp.authn.context.AuthenticationContext;
import net.shibboleth.idp.plugin.authn.duo.AbstractDuoAuthenticationAction;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
+import net.shibboleth.oidc.security.credential.BasicJWKCredential;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
+import net.shibboleth.oidc.security.impl.JWTSignatureValidationUtil;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
/**
@@ -69,6 +78,13 @@ public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(ValidateTokenSignature.class);
+
+ /**
+ * The signature algorithm used. This is fixed and not taken from the JWS.
+ * There is not reason in the Duo case to determine the algorithm from the JWS,
+ * as HMAC is the only required algorithm - hence this is safer.
+ */
+ @Nonnull private Algorithm signatureAlgorithm;
/** The Duo authentication token. */
@Nullable private JWT token;
@@ -78,6 +94,27 @@ public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
/** The Duo integration appropriate for this request.*/
@Nullable private DuoOIDCIntegration integration;
+
+ /** Constructor.*/
+ public ValidateTokenSignature() {
+ //this is the default HMAC algorithm Duo support, and no other
+ signatureAlgorithm = JWSAlgorithm.HS512;
+ }
+
+ /**
+ * Set the signature algorithm to use. Only supports one of the HMAC_SHA family.
+ *
+ * @param algo the JWS signature algorithm.
+ */
+ public void setSignatureAlgorithm(@Nonnull final JWSAlgorithm algo) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ Constraint.isNotNull(algo, "Signature algorithm can not be null");
+
+ if (!SUPPORTED_SIGNATURE_FAMILY.contains(algo)) {
+ throw new ConstraintViolationException("Signature algorithm must be one of "+SUPPORTED_SIGNATURE_FAMILY);
+ }
+ signatureAlgorithm = algo;
+ }
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@@ -116,48 +153,40 @@ public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
log.debug("{} Validating token signature for subject '{}'",getLogPrefix(),claimSet.getSubject());
- //only supports HMAC signatures. Asymmetric or 'none' are not allowed.
-
+ //only supports HMAC signatures. Plain JWT's or those with a 'none' algorithm are not allowed.
+ //So fail-fast here, before we attempt validation.
if (token instanceof PlainJWT || JWSAlgorithm.NONE == token.getHeader().getAlgorithm()) {
- log.error("{} Invalid token for subject '{}'. Token must be signed using one of the supported "
+ log.error("{} Invalid token signature for subject '{}'. Token must be signed using one of the supported "
+ "algorithms '{}'",getLogPrefix(),claimSet.getSubject(),SUPPORTED_SIGNATURE_FAMILY);
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
return;
} else if (token instanceof SignedJWT) {
- try {
- if (SUPPORTED_SIGNATURE_FAMILY.contains(((SignedJWT)token).getHeader().getAlgorithm())) {
- final JWSVerifier verifier = new MACVerifier(integration.getSecretKey());
- if (!((SignedJWT)token).verify(verifier)) {
- log.error("{} Token signature is invalid for subject '{}' and client '{}'",getLogPrefix(),
- claimSet.getSubject(),integration.getClientId());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
- return;
- } else {
- log.debug("{} Token signature is valid for subject '{}'; using algorithm family "
- + "'{}' for client '{}'", getLogPrefix(),claimSet.getSubject(),
- SUPPORTED_SIGNATURE_FAMILY, integration.getClientId());
- //Valid token
- return;
- }
- } else {
- log.error("{} Invalid token. Token signature algorithm not supported, token algorithm '{}',"
- + " supported algorithms '{}', for client '{}'",getLogPrefix(),
- ((SignedJWT)token).getHeader().getAlgorithm().getName(),
- SUPPORTED_SIGNATURE_FAMILY,integration.getClientId());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
- return;
- }
- } catch (final IllegalStateException | JOSEException e) {
- log.error("{} Unable to validate token using algorithm family '{}' for client '{}'",getLogPrefix(),
- SUPPORTED_SIGNATURE_FAMILY,integration.getClientId(),e);
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+
+ final BasicJWKCredential jwkCredential = new BasicJWKCredential();
+ jwkCredential.setSecretKey(new SecretKeySpec(
+ JWSAssemblyUtils.getSecretBytes(integration.getSecretKey()), "NONE"));
+ jwkCredential.setAlgorithm(signatureAlgorithm);
+ final String errorEventId =
+ JWTSignatureValidationUtil.validateSignature(List.of(jwkCredential),
+ (SignedJWT)token, AuthnEventIds.NO_CREDENTIALS);
+
+ if (errorEventId != null) {
+ log.error("{} Token signature is invalid for subject '{}' and client '{}'",getLogPrefix(),
+ claimSet.getSubject(),integration.getClientId());
+ ActionSupport.buildEvent(profileRequestContext, errorEventId);
+ return;
+ } else {
+ log.debug("{} Token signature is valid for subject '{}'; using algorithm "
+ + "'{}' for client '{}'", getLogPrefix(),claimSet.getSubject(),
+ signatureAlgorithm, integration.getClientId());
+ //Valid token
return;
}
}
- log.error("{} Unable to validate token for subject '{}' and client '{}', "
+ log.error("{} Unable to validate token signature for subject '{}' and client '{}', "
+ "unkown token type",getLogPrefix(),claimSet.getSubject(),integration.getClientId());
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
return;
diff --git a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
index 7db4f8b..e26a3fe 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
@@ -57,6 +57,7 @@
p:duoIntegrationLookupStrategy-ref="shibboleth.authn.DuoOIDC.DuoIntegrationStrategy"
p:usernameLookupStrategy-ref="shibboleth.authn.DuoOIDC.UsernameLookupStrategy"
p:clientRegistry-ref="shibboleth.authn.DuoOIDC.clientRegistry" />
+
<bean id="HealthCheckDuoOIDCAuthAPI" scope="prototype"
class="net.shibboleth.idp.plugin.authn.duo.impl.HealthCheckDuoOIDCAuthAPI" />
@@ -65,7 +66,8 @@
class="net.shibboleth.idp.plugin.authn.duo.impl.ValidateDuoResponseState" />
<bean id="ValidateTokenSignature" scope="prototype"
- class="net.shibboleth.idp.plugin.authn.duo.impl.ValidateTokenSignature" />
+ class="net.shibboleth.idp.plugin.authn.duo.impl.ValidateTokenSignature"
+ p:signatureAlgorithm="HS512"/>
<bean id="shibboleth.authn.DuoOIDC.jwt.claims.DefaultCleanupHook"
class="net.shibboleth.idp.plugin.authn.duo.impl.ValidateTokenClaims.DuoOIDAuthenticationContextCleanupHook" />
diff --git a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
index 9fdb8f1..5d4805f 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
@@ -7,7 +7,7 @@
<!-- TODO: throws an AuthnException if the endpoint is not healthy, no backoff etc. -->
<action-state id="CheckDuoOIDCAuthAPI">
<evaluate expression="PopulateDuoAuthenticationContext" />
- <evaluate expression="HealthCheckDuoOIDCAuthAPI" />
+ <evaluate expression="HealthCheckDuoOIDCAuthAPI" />
<evaluate expression="'proceed'" />
<transition on="proceed" to="Duo2FAAuthorizationRequest" />
</action-state>
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java
index e6c9d31..2f39349 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java
@@ -45,7 +45,6 @@ import org.testng.annotations.Test;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.proc.BadJWTException;
-import com.nimbusds.jwt.proc.JWTClaimsSetVerifier;
import net.shibboleth.idp.authn.AuthnEventIds;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
index 5eb6ca5..45e3004 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
@@ -43,10 +43,13 @@ import org.springframework.webflow.execution.Event;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import com.nimbusds.jose.JWSAlgorithm;
+
import net.shibboleth.idp.authn.AuthnEventIds;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
import net.shibboleth.utilities.java.support.codec.EncodingException;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
/**
* Tests for the {@link ValidateTokenSignature} class.
@@ -59,7 +62,9 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
@BeforeMethod
public void setUp() throws Exception {
super.setup();
+ //use a HS256 sig, Duo is HS512
action = new ValidateTokenSignature();
+ action.setSignatureAlgorithm(JWSAlgorithm.HS256);
}
@@ -179,6 +184,17 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
assertEventId(event, AuthnEventIds.NO_CREDENTIALS);
}
+ /**
+ * Test setting an unsupported Signature Algorithm.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws EncodingException on error.
+ */
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public final void testSetUnsupportedSignatureAlgorithm() {
+ action.setSignatureAlgorithm(JWSAlgorithm.RS256);
+ }
+
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java
index 701d1e4..893191a 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java
@@ -39,7 +39,9 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientCapabilities;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
/**
* Mock a Duo client which is not available (unhealthy).
@@ -128,14 +130,11 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
"}";
try {
- //sign the token using the client secret
- final JWSSigner signer = new MACSigner(integration.getSecretKey());
- final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.HS256).build();
+ //sign the token using the client secret. Secret key size (length) is not checked.
final JWTClaimsSet claims = JWTClaimsSet.parse(jwtJson);
- final SignedJWT signedJWT = new SignedJWT(header,claims);
- signedJWT.sign(signer);
- return signedJWT;
- } catch (final ParseException | JOSEException e) {
+ return JWSAssemblyUtils.assembleMacJws(JWSAlgorithm.HS512,claims,
+ JWSAssemblyUtils.getSecretBytes(integration.getSecretKey()));
+ } catch (final ParseException | JOSEException | EncodingException e) {
throw new DuoClientException(e);
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java
index f479252..c14b64f 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java
@@ -39,7 +39,9 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientCapabilities;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
/**
* Mock a Duo client which is available and returns a valid response.
@@ -125,15 +127,12 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
"}";
try {
- //sign the token using the client secret
- final JWSSigner signer = new MACSigner(integration.getSecretKey());
- final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.HS256).build();
+ //sign the token using the client secret. Secret key size (length) is not checked.
final JWTClaimsSet claims = JWTClaimsSet.parse(jwtJson);
- final SignedJWT signedJWT = new SignedJWT(header,claims);
- signedJWT.sign(signer);
+ return JWSAssemblyUtils.assembleMacJws(JWSAlgorithm.HS512,claims,
+ JWSAssemblyUtils.getSecretBytes(integration.getSecretKey()));
- return signedJWT;
- } catch (final ParseException | JOSEException e) {
+ } catch (final ParseException | JOSEException | EncodingException e) {
throw new DuoClientException(e);
}
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java
index 6e47901..c7571e6 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java
@@ -39,7 +39,9 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientCapabilities;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
/**
* Mock a Duo client which is available and returns a valid response.
@@ -127,14 +129,11 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
"}";
try {
- //sign the token using the client secret
- final JWSSigner signer = new MACSigner(integration.getSecretKey());
- final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.HS256).build();
+ //sign the token using the client secret. Secret key size (length) is not checked.
final JWTClaimsSet claims = JWTClaimsSet.parse(jwtJson);
- final SignedJWT signedJWT = new SignedJWT(header,claims);
- signedJWT.sign(signer);
- return signedJWT;
- } catch (final ParseException | JOSEException e) {
+ return JWSAssemblyUtils.assembleMacJws(JWSAlgorithm.HS512,claims,
+ JWSAssemblyUtils.getSecretBytes(integration.getSecretKey()));
+ } catch (final ParseException | JOSEException | EncodingException e) {
throw new DuoClientException(e);
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java
index 07215dc..22c188b 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java
@@ -39,7 +39,9 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientCapabilities;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
/**
* Mock a Duo client which returns an unknown response for the health check and a login failure in the
@@ -124,14 +126,11 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
"}";
try {
- //sign the token using the client secret
- final JWSSigner signer = new MACSigner(integration.getSecretKey());
- final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.HS256).build();
+ //sign the token using the client secret. Secret key size (length) is not checked.
final JWTClaimsSet claims = JWTClaimsSet.parse(jwtJson);
- final SignedJWT signedJWT = new SignedJWT(header,claims);
- signedJWT.sign(signer);
- return signedJWT;
- } catch (final ParseException | JOSEException e) {
+ return JWSAssemblyUtils.assembleMacJws(JWSAlgorithm.HS512,claims,
+ JWSAssemblyUtils.getSecretBytes(integration.getSecretKey()));
+ } catch (final ParseException | JOSEException | EncodingException e) {
throw new DuoClientException(e);
}
}
diff --git a/idp-duo-nimbus-client-impl/pom.xml b/idp-duo-nimbus-client-impl/pom.xml
index e648999..686e90d 100644
--- a/idp-duo-nimbus-client-impl/pom.xml
+++ b/idp-duo-nimbus-client-impl/pom.xml
@@ -45,6 +45,11 @@
<artifactId>nimbus-jose-jwt</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>net.shibboleth.oidc</groupId>
+ <artifactId>oidc-common-crypto-impl</artifactId>
+ <scope>provided</scope>
+ </dependency>
<!-- Needed for module/plugin impls. -->
<dependency>
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
index 61ec9d1..6bafefc 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
@@ -18,7 +18,6 @@
package net.shibboleth.idp.plugin.authn.duo.nimbus.impl;
-import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.text.ParseException;
import java.time.Duration;
@@ -27,19 +26,12 @@ import java.util.Date;
import javax.annotation.Nonnull;
import com.nimbusds.jose.JOSEException;
-import com.nimbusds.jose.JOSEObjectType;
import com.nimbusds.jose.JWSAlgorithm;
-import com.nimbusds.jose.JWSHeader;
-import com.nimbusds.jose.Payload;
-import com.nimbusds.jose.crypto.impl.AlgorithmSupportMessage;
-import com.nimbusds.jose.crypto.impl.HMAC;
-import com.nimbusds.jose.crypto.impl.MACProvider;
-import com.nimbusds.jose.util.Base64URL;
import com.nimbusds.jwt.JWTClaimsSet;
-import com.nimbusds.jwt.SignedJWT;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.codec.EncodingException;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -85,7 +77,7 @@ public final class NimbusClientSupport {
*
* @return a signed JWT
*/
- //TODO this method and the below should be nimbus, inside oidc-commons, and merged into a single API
+ //TODO this method and the below should be inside oidc-commons, and merged into a single API
@Nonnull static String createJWSRequestObject(@Nonnull final DuoOIDCIntegration duoIntegration,
@Nonnull @NotEmpty final String state, @Nonnull final String username) throws DuoClientException{
@@ -107,7 +99,8 @@ public final class NimbusClientSupport {
.claim("response_type", "code")
.build();
- return assembleMacJws(JWSAlgorithm.HS512,claimsSet,getSecretBytes(duoIntegration.getSecretKey()));
+ return JWSAssemblyUtils.assembleMacJwsAsString(
+ JWSAlgorithm.HS512,claimsSet,JWSAssemblyUtils.getSecretBytes(duoIntegration.getSecretKey()));
} catch (final JOSEException | EncodingException | ParseException e) {
throw new DuoClientException(e);
@@ -129,12 +122,11 @@ public final class NimbusClientSupport {
* @return a signed JWT.
*
*/
- //TODO: replace with nimbus method inside of commons
@Nonnull static String createJWS(@Nonnull final String aud,
@Nonnull final DuoOIDCIntegration duoIntegration) throws DuoClientException{
Constraint.isNotNull(duoIntegration, "Duo Integration can not be null");
- Constraint.isNotNull(aud, "Audience can not be null");
+ Constraint.isNotNull(aud, "Audience can not be null");//'
final Date expiration = new Date();
expiration.setTime(expiration.getTime() + Duration.ofHours(1).toMillis());
@@ -148,7 +140,8 @@ public final class NimbusClientSupport {
.jwtID(NimbusClientSupport.generateJWTId(32))
.build();
- return assembleMacJws(JWSAlgorithm.HS512,claimsSet,getSecretBytes(duoIntegration.getSecretKey()));
+ return JWSAssemblyUtils.assembleMacJwsAsString(
+ JWSAlgorithm.HS512,claimsSet,JWSAssemblyUtils.getSecretBytes(duoIntegration.getSecretKey()));
} catch (final EncodingException | JOSEException | ParseException e) {
throw new DuoClientException(e);
@@ -156,109 +149,9 @@ public final class NimbusClientSupport {
}
- /**
- * Assemble a HMAC based JSON Web Signature token using the given algorithm, claims, and secret.
- *
- * @param algorithm the JWA algorithm, **must** be one from the HMAC family.
- * @param claimsSet the claims that form the payload.
- * @param secret the pre-shared secret used to construct the HMAC.
- *
- * @return a fully assembled JWS using the JSON compact serialisation.
- *
- * @throws EncodingException On error during encoding.
- * @throws JOSEException If the algorithm is not supported.
- * @throws ParseException If an error occurs during serialisation.
- */
- @Nonnull private static String assembleMacJws(@Nonnull final JWSAlgorithm algorithm,
- @Nonnull final JWTClaimsSet claimsSet, @Nonnull final byte[] secret) throws
- EncodingException, JOSEException, ParseException {
-
- Constraint.isNotNull(algorithm, "Algorithm can not be null");
- Constraint.isNotNull(claimsSet, "JWT claims can not be null");
- Constraint.isNotNull(secret, "Secret can not be null");
-
- final JWSHeader header = new JWSHeader.Builder(algorithm)
- .type(JOSEObjectType.JWT)
- .build();
- final Payload payload = new Payload(claimsSet.toJSONObject());
-
- final String signingInput = composeSigningInput(header,payload);
- /*
- * A null JCA provider is supplied, as a result the preferred provider that supports the specified
- * algorithm will be used.
- */
- final byte[] hmac = HMAC.compute(getJCAAlgorithmName(algorithm), secret,
- signingInput.getBytes(StandardCharsets.UTF_8), null);
-
- final SignedJWT signedJwt = new SignedJWT(header.toBase64URL(), payload.toBase64URL(),
- Base64URL.encode(hmac));
-
- return signedJwt.serialize();
-
- }
-
- /**
- * Gets the matching Java Cryptography Architecture (JCA) algorithm
- * name for the specified HMAC-based JSON Web Algorithm (JWA).
- * <p>
- * This is taken from the Nimbus {@link MACProvider} class.
- * </p>
- *
- * @param alg The JSON Web Algorithm (JWA). Must be supported and not
- * {@code null}.
- *
- * @return The matching JCA algorithm name.
- *
- * @throws JOSEException If the algorithm is not supported.
- */
- @Nonnull private static String getJCAAlgorithmName(@Nonnull final JWSAlgorithm alg)
- throws JOSEException {
- Constraint.isNotNull(alg, "Algorithm can not be null");
-
- if (alg.equals(JWSAlgorithm.HS256)) {
- return "HMACSHA256";
- } else if (alg.equals(JWSAlgorithm.HS384)) {
- return "HMACSHA384";
- } else if (alg.equals(JWSAlgorithm.HS512)) {
- return "HMACSHA512";
- } else {
- throw new JOSEException(AlgorithmSupportMessage.unsupportedJWSAlgorithm(
- alg,
- MACProvider.SUPPORTED_ALGORITHMS));
- }
- }
- /**
- * Compose the message that is to be signed.
- *
- * @param header the header component of the message to be signed.
- * @param payload the payload component of the message to be signed.
- *
- * @return the message in its compact/serialised state ready to be signed.
- *
- * @throws EncodingException if there is an error base64 encoding the components.
- */
- @Nonnull private static String composeSigningInput(@Nonnull final JWSHeader header,
- @Nonnull final Payload payload) throws EncodingException {
- Constraint.isNotNull(header, "JWS Header can not be null");
- Constraint.isNotNull(payload, "JWT payload can not be null");
-
- return header.toBase64URL().toString() + "."+ payload.toBase64URL().toString();
- }
-
- /**
- * Convert the String secret into its byte representation assuming a UTF-8 encoding.
- *
- * @param secret the secret as a UTF-8 encoding string, must not be {@code null}.
- *
- * @return the UTF-8 byte representation of the secret.
- */
- @Nonnull private static byte[] getSecretBytes(@Nonnull final String secret) {
- return secret.getBytes(StandardCharsets.UTF_8);
- }
-
}
diff --git a/idp-duo-sdk-client-impl/pom.xml b/idp-duo-sdk-client-impl/pom.xml
index 6829a68..e4a6505 100644
--- a/idp-duo-sdk-client-impl/pom.xml
+++ b/idp-duo-sdk-client-impl/pom.xml
@@ -99,6 +99,11 @@
</dependency>
<!-- provided dependencies -->
+ <dependency>
+ <groupId>net.shibboleth.oidc</groupId>
+ <artifactId>oidc-common-crypto-impl</artifactId>
+ <scope>provided</scope>
+ </dependency>
<dependency>
<groupId>net.shibboleth.utilities</groupId>
<artifactId>java-support</artifactId>
diff --git a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
index 9d2d5c6..8d6c1c9 100644
--- a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
+++ b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
@@ -18,6 +18,7 @@
package net.shibboleth.idp.plugin.authn.duo.sdk.impl;
+
import java.text.ParseException;
import java.util.List;
import java.util.function.BiFunction;
@@ -38,12 +39,8 @@ import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.JWSAlgorithm;
-import com.nimbusds.jose.JWSHeader;
-import com.nimbusds.jose.JWSSigner;
-import com.nimbusds.jose.crypto.MACSigner;
import com.nimbusds.jwt.JWT;
import com.nimbusds.jwt.JWTClaimsSet;
-import com.nimbusds.jwt.SignedJWT;
import net.shibboleth.idp.plugin.authn.duo.AbstractDuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
@@ -51,7 +48,9 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheckResponse;
+import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
@@ -219,20 +218,19 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
@Nullable public JWT apply(@Nonnull final Token t, @Nonnull final DuoOIDCIntegration integ) {
try {
final String duoTokenAsJson = objectMapper.writeValueAsString(t);
- //re-sign the token using the client secret
- final JWSSigner signer = new MACSigner(integ.getSecretKey());
- //needs to be HS512 (or not?) this is only passed internally, so should be fine
- final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.HS256).build();
final JWTClaimsSet claims = JWTClaimsSet.parse(duoTokenAsJson);
- final SignedJWT signedJWT = new SignedJWT(header,claims);
- signedJWT.sign(signer);
- return signedJWT;
- } catch (final JsonProcessingException | ParseException | JOSEException e) {
+ //re-sign the JWT using an incompatible key for the given algorithm!
+ //FIXME please change this Duo!
+ return JWSAssemblyUtils.assembleMacJws(JWSAlgorithm.HS512,claims,
+ JWSAssemblyUtils.getSecretBytes(integ.getSecretKey()));
+
+ } catch (final JsonProcessingException | ParseException | JOSEException | EncodingException e) {
log.error("Could not convert Duo Token to a Nimbus JWT Token",e);
return null;
}
}
+
}
diff --git a/pom.xml b/pom.xml
index 4a448a4..d74973b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -96,7 +96,7 @@
<!-- OIDC Common BOM when importing OIDC dependencies -->
<dependency>
<groupId>net.shibboleth.oidc</groupId>
- <artifactId>oidc-common-parent</artifactId>
+ <artifactId>oidc-common-bom</artifactId>
<version>${oidc.common.version}</version>
<type>pom</type>
<scope>import</scope>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list