diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainSigner.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainSigner.java index bd47401ae..c889ffb81 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainSigner.java +++ b/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainSigner.java @@ -30,6 +30,9 @@ import com.nimbusds.jose.util.Base64URL; /** * Signer to support "alg:none" JWS signing option (no signature). * + * FIXME: The JWSSigner interface was never intended to be used with plain JWTs. + * Use of the signer/verifier pattern alongside the other JWSSigner/Verifiers will require refactoring. + * * @author wkim * */ @@ -44,15 +47,25 @@ public final class PlainSigner implements JWSSigner { @Override public Base64URL sign(ReadOnlyJWSHeader header, byte[] signingInput) throws JOSEException { - if (header instanceof PlainHeader) { + if (header instanceof PlainHeader) { // XXX impossible due to interface method signature return new Base64URL(""); - } else { + } else { throw new JOSEException("Invalid header. This signer is for use with Plain JWTs only."); } } + + /** + * Utility method to return an empty signature. + * + * @return + */ + public static Base64URL sign() { + + return new Base64URL(""); + } } diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainVerifier.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainVerifier.java index 56486d14c..34deebe4f 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainVerifier.java +++ b/openid-connect-common/src/main/java/org/mitre/jwt/signer/PlainVerifier.java @@ -33,6 +33,9 @@ import com.nimbusds.jwt.PlainJWT; /** * Verifier to support "alg:none" JWS signing option (no signature). * + * FIXME: The JWSVerifier interface was never intended to be used with plain JWTs. + * Use of the signer/verifier pattern alongside the other JWSSigner/Verifiers will require refactoring. + * * @author wkim * */ diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DefaultJwtSigningAndValidationService.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DefaultJwtSigningAndValidationService.java index 468396003..89386b51a 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DefaultJwtSigningAndValidationService.java +++ b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DefaultJwtSigningAndValidationService.java @@ -53,7 +53,7 @@ import com.nimbusds.jwt.SignedJWT; public class DefaultJwtSigningAndValidationService implements JwtSigningAndValidationService { - public static final String ALG_NONE = "none"; // TODO storing a default "alg:none" id smells a bit.. + public static final String ALG_NONE = "none"; // map of identifier to signer private Map signers = new HashMap(); @@ -131,7 +131,7 @@ public class DefaultJwtSigningAndValidationService implements JwtSigningAndValid } /** - * @param defaultSignerKeyId the defaultSignerKeyId to set + * @param defaultSignerKeyId the defaultSignerKeyId to set. The "none" key id is reserved for alg:none in this implementation. */ public void setDefaultSignerKeyId(String defaultSignerId) { this.defaultSignerKeyId = defaultSignerId; @@ -165,8 +165,6 @@ public class DefaultJwtSigningAndValidationService implements JwtSigningAndValid private void buildSignersAndVerifiers() throws NoSuchAlgorithmException, InvalidKeySpecException { signers.put(ALG_NONE, new PlainSigner()); - verifiers.put(ALG_NONE, new PlainVerifier()); - for (Map.Entry jwkEntry : keys.entrySet()) { @@ -230,10 +228,12 @@ public class DefaultJwtSigningAndValidationService implements JwtSigningAndValid // At this point, this is a plain JWT and is already good-to-go. - } else { // we have a signable JWS at this point. + } else if (jwt instanceof SignedJWT) { // we have a signable JWS at this point. ((SignedJWT) jwt).sign(signer); + } else { + logger.warn("Attempted to sign an unsupported JWT type."); } } catch (JOSEException e) { @@ -270,10 +270,13 @@ public class DefaultJwtSigningAndValidationService implements JwtSigningAndValid // do nothing because PlainJWT is good already. - } else { // we have a signable JWS at this point. + } else if (jwt instanceof SignedJWT){ // we have a signable JWS at this point. ((SignedJWT) jwt).sign(signer); + } else { + + logger.warn("Attempted to sign an unsupported JWT type."); } } catch (JOSEException e) { @@ -285,21 +288,21 @@ public class DefaultJwtSigningAndValidationService implements JwtSigningAndValid @Override public boolean validateSignature(JWT jwt) { - if (getDefaultSigningAlgorithm().equals(JWSAlgorithm.NONE) { + if (getDefaultSignerKeyId().equals(ALG_NONE) && (jwt instanceof PlainJWT)) { - if (jwt instanceof PlainJWT) { - return - } - } - - for (JWSVerifier verifier : verifiers.values()) { - try { - if (jwt.verify(verifier)) { - return true; + return PlainVerifier.verify((PlainJWT) jwt); + + } else { + + for (JWSVerifier verifier : verifiers.values()) { + try { + if (((SignedJWT) jwt).verify(verifier)) { + return true; + } + } catch (JOSEException e) { + + logger.error("Failed to validate signature, error was: ", e); } - } catch (JOSEException e) { - - logger.error("Failed to validate signature, error was: ", e); } } return false; @@ -328,8 +331,6 @@ public class DefaultJwtSigningAndValidationService implements JwtSigningAndValid public Collection getAllSigningAlgsSupported() { Set algs = new HashSet(); - - //TODO add 'none' for (JWSSigner signer : signers.values()) { algs.addAll(signer.supportedAlgorithms());