Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KEYCLOAK-7560 Refactor Token Sign and Verify by Token Signature SPI #5260

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions core/src/main/java/org/keycloak/TokenVerifier.java
Expand Up @@ -24,12 +24,15 @@
import org.keycloak.jose.jws.JWSHeader;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.jose.jws.JWSInputException;
import org.keycloak.jose.jws.JWSSignatureProvider;
import org.keycloak.jose.jws.crypto.HMACProvider;
import org.keycloak.jose.jws.crypto.RSAProvider;
import org.keycloak.representations.JsonWebToken;
import org.keycloak.util.TokenUtil;

import javax.crypto.SecretKey;

import java.security.Key;
import java.security.PublicKey;
import java.util.*;
import java.util.logging.Level;
Expand Down Expand Up @@ -144,6 +147,18 @@ public boolean test(JsonWebToken t) throws VerificationException {
private JWSInput jws;
private T token;

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
private Key verifyKey = null;
private JWSSignatureProvider signatureProvider = null;
public TokenVerifier<T> verifyKey(Key verifyKey) {
this.verifyKey = verifyKey;
return this;
}
public TokenVerifier<T> signatureProvider(JWSSignatureProvider signatureProvider) {
this.signatureProvider = signatureProvider;
return this;
}

protected TokenVerifier(String tokenString, Class<T> clazz) {
this.tokenString = tokenString;
this.clazz = clazz;
Expand Down Expand Up @@ -334,6 +349,12 @@ public JWSHeader getHeader() throws VerificationException {
}

public void verifySignature() throws VerificationException {
// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
if (this.signatureProvider != null && this.verify() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verify() calls verifySignature(), and then now verifySignature() calls verify() => stack overflow
Should be this.verifyKey != null instead?
Even in that case, I'd say it should suffice that this.signatureProvider != null since e.g. in case of hardware token, one might have no access to the key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I also think that only this.signatureProvider != null is sufficient to check.
I've commented above that this TokenVerifier's refactoring had not yet completely been implemented and I had not yet written arquillian integration test cases for it so I had not recognized this stack overflow potential. Thank you very much.

verifySignatureByProvider();
return;
}

AlgorithmType algorithmType = getHeader().getAlgorithm().getType();

if (null == algorithmType) {
Expand All @@ -358,6 +379,13 @@ public void verifySignature() throws VerificationException {
}
}

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
private void verifySignatureByProvider() throws VerificationException {
if (!signatureProvider.verify(jws, verifyKey)) {
throw new TokenSignatureInvalidException(token, "Invalid token signature");
}
}

public TokenVerifier<T> verify() throws VerificationException {
if (getToken() == null) {
parse();
Expand Down
62 changes: 41 additions & 21 deletions core/src/main/java/org/keycloak/jose/jws/JWSBuilder.java
Expand Up @@ -25,6 +25,7 @@
import javax.crypto.SecretKey;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.security.Key;
import java.security.PrivateKey;

/**
Expand All @@ -36,7 +37,7 @@ public class JWSBuilder {
String kid;
String contentType;
byte[] contentBytes;

public JWSBuilder type(String type) {
this.type = type;
return this;
Expand Down Expand Up @@ -66,22 +67,6 @@ public EncodingBuilder jsonContent(Object object) {
return new EncodingBuilder();
}


protected String encodeHeader(Algorithm alg) {
StringBuilder builder = new StringBuilder("{");
builder.append("\"alg\":\"").append(alg.toString()).append("\"");

if (type != null) builder.append(",\"typ\" : \"").append(type).append("\"");
if (kid != null) builder.append(",\"kid\" : \"").append(kid).append("\"");
if (contentType != null) builder.append(",\"cty\":\"").append(contentType).append("\"");
builder.append("}");
try {
return Base64Url.encode(builder.toString().getBytes("UTF-8"));
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
}

protected String encodeAll(StringBuffer encoding, byte[] signature) {
encoding.append('.');
if (signature != null) {
Expand All @@ -91,15 +76,37 @@ protected String encodeAll(StringBuffer encoding, byte[] signature) {
}

protected void encode(Algorithm alg, byte[] data, StringBuffer encoding) {
encoding.append(encodeHeader(alg));
encoding.append('.');
encoding.append(Base64Url.encode(data));
// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
encode(alg.name(), data, encoding);
}

protected byte[] marshalContent() {
return contentBytes;
}

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
protected void encode(String sigAlgName, byte[] data, StringBuffer encoding) {
encoding.append(encodeHeader(sigAlgName));
encoding.append('.');
encoding.append(Base64Url.encode(data));
}

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
protected String encodeHeader(String sigAlgName) {
StringBuilder builder = new StringBuilder("{");
builder.append("\"alg\":\"").append(sigAlgName).append("\"");

if (type != null) builder.append(",\"typ\" : \"").append(type).append("\"");
if (kid != null) builder.append(",\"kid\" : \"").append(kid).append("\"");
if (contentType != null) builder.append(",\"cty\":\"").append(contentType).append("\"");
builder.append("}");
try {
return Base64Url.encode(builder.toString().getBytes("UTF-8"));
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
}

public class EncodingBuilder {
public String none() {
StringBuffer buffer = new StringBuffer();
Expand All @@ -108,6 +115,20 @@ public String none() {
return encodeAll(buffer, null);
}

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
public String sign(JWSSignatureProvider signatureProvider, String sigAlgName, Key key) {
StringBuffer buffer = new StringBuffer();
byte[] data = marshalContent();
encode(sigAlgName, data, buffer);
byte[] signature = null;
try {
signature = signatureProvider.sign(buffer.toString().getBytes("UTF-8"), sigAlgName, key);
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
return encodeAll(buffer, signature);
}

public String sign(Algorithm algorithm, PrivateKey privateKey) {
StringBuffer buffer = new StringBuffer();
byte[] data = marshalContent();
Expand All @@ -133,7 +154,6 @@ public String rsa512(PrivateKey privateKey) {
return sign(Algorithm.RS512, privateKey);
}


public String hmac256(byte[] sharedSecret) {
StringBuffer buffer = new StringBuffer();
byte[] data = marshalContent();
Expand Down
@@ -0,0 +1,9 @@
package org.keycloak.jose.jws;

import java.security.Key;

public interface JWSSignatureProvider {
// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
byte[] sign(byte[] data, String sigAlgName, Key key);
boolean verify(JWSInput input, Key key);
}
Expand Up @@ -25,6 +25,7 @@
import javax.crypto.Mac;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

Expand Down
24 changes: 13 additions & 11 deletions core/src/main/java/org/keycloak/jose/jws/crypto/HashProvider.java
Expand Up @@ -27,18 +27,16 @@
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class HashProvider {

// See "at_hash" and "c_hash" in OIDC specification
public static String oidcHash(Algorithm jwtAlgorithm, String input) {
byte[] digest = digest(jwtAlgorithm, input);
// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
public static String oidcHash(String jwtAlgorithmName, String input) {
byte[] digest = digest(jwtAlgorithmName, input);

int hashLength = digest.length / 2;
byte[] hashInput = Arrays.copyOf(digest, hashLength);

return Base64Url.encode(hashInput);
}

private static byte[] digest(Algorithm algorithm, String input) {
private static byte[] digest(String algorithm, String input) {
String digestAlg = getJavaDigestAlgorithm(algorithm);

try {
Expand All @@ -49,18 +47,22 @@ private static byte[] digest(Algorithm algorithm, String input) {
throw new RuntimeException(e);
}
}

private static String getJavaDigestAlgorithm(Algorithm alg) {
private static String getJavaDigestAlgorithm(String alg) {
switch (alg) {
case RS256:
case "RS256":
return "SHA-256";
case RS384:
case "RS384":
return "SHA-384";
case RS512:
case "RS512":
return "SHA-512";
default:
throw new IllegalArgumentException("Not an RSA Algorithm");
}
}

// See "at_hash" and "c_hash" in OIDC specification
public static String oidcHash(Algorithm jwtAlgorithm, String input) {
return oidcHash(jwtAlgorithm.name(), input);
}

}
@@ -0,0 +1,12 @@
package org.keycloak.jose.jws;

import java.security.Key;

import org.keycloak.provider.Provider;

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI

public interface TokenSignatureProvider extends Provider {
byte[] sign(byte[] data, String sigAlgName, Key key);
boolean verify(JWSInput jws, Key verifyKey);
}
@@ -0,0 +1,11 @@
package org.keycloak.jose.jws;

import org.keycloak.component.ComponentFactory;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.KeycloakSession;

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI

public interface TokenSignatureProviderFactory<T extends TokenSignatureProvider> extends ComponentFactory<T, TokenSignatureProvider> {
T create(KeycloakSession session, ComponentModel model);
}
@@ -0,0 +1,29 @@
package org.keycloak.jose.jws;

import org.keycloak.provider.Provider;
import org.keycloak.provider.ProviderFactory;
import org.keycloak.provider.Spi;

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI

public class TokenSignatureSpi implements Spi {
@Override
public boolean isInternal() {
return true;
}

@Override
public String getName() {
return "tokenSignature";
}

@Override
public Class<? extends Provider> getProviderClass() {
return TokenSignatureProvider.class;
}

@Override
public Class<? extends ProviderFactory> getProviderFactoryClass() {
return TokenSignatureProviderFactory.class;
}
}
Expand Up @@ -18,12 +18,8 @@
package org.keycloak.keys;

import org.keycloak.crypto.KeyWrapper;
import org.keycloak.jose.jws.AlgorithmType;
import org.keycloak.provider.Provider;

import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.cert.X509Certificate;
import java.util.List;

/**
Expand Down
@@ -0,0 +1,10 @@
package org.keycloak.keys;

import java.security.Key;

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI

public interface SignatureKeyProvider {
Key getSignKey();
Key getVerifyKey(String kid);
}
@@ -0,0 +1,35 @@
package org.keycloak.models.utils;

import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.component.ComponentModel;
import org.keycloak.jose.jws.TokenSignatureProvider;
import org.keycloak.models.RealmModel;

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI

public class DefaultTokenSignatureProviders {
private static final String COMPONENT_SIGNATURE_ALGORITHM_KEY = "org.keycloak.jose.jws.TokenSignatureProvider.algorithm";
private static final String RSASSA_PROVIDER_ID = "rsassa-signature";
private static final String HMAC_PROVIDER_ID = "hmac-signature";

public static void createProviders(RealmModel realm) {
createAndAddProvider(realm, RSASSA_PROVIDER_ID, "RS256");
createAndAddProvider(realm, RSASSA_PROVIDER_ID, "RS384");
createAndAddProvider(realm, RSASSA_PROVIDER_ID, "RS512");
createAndAddProvider(realm, HMAC_PROVIDER_ID, "HS256");
createAndAddProvider(realm, HMAC_PROVIDER_ID, "HS384");
createAndAddProvider(realm, HMAC_PROVIDER_ID, "HS512");
}

private static void createAndAddProvider(RealmModel realm, String providerId, String sigAlgName) {
ComponentModel generated = new ComponentModel();
generated.setName(providerId);
generated.setParentId(realm.getId());
generated.setProviderId(providerId);
generated.setProviderType(TokenSignatureProvider.class.getName());
MultivaluedHashMap<String, String> config = new MultivaluedHashMap<>();
config.putSingle(COMPONENT_SIGNATURE_ALGORITHM_KEY, sigAlgName);
generated.setConfig(config);
realm.addComponentModel(generated);
}
}
Expand Up @@ -53,6 +53,7 @@
import org.keycloak.common.util.UriUtils;
import org.keycloak.component.ComponentModel;
import org.keycloak.credential.CredentialModel;
import org.keycloak.jose.jws.TokenSignatureProvider;
import org.keycloak.keys.KeyProvider;
import org.keycloak.migration.MigrationProvider;
import org.keycloak.migration.migrators.MigrationUtils;
Expand Down Expand Up @@ -420,6 +421,12 @@ public static void importRealm(KeycloakSession session, RealmRepresentation rep,
DefaultKeyProviders.createProviders(newRealm);
}
}

// KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
if (newRealm.getComponents(newRealm.getId(), TokenSignatureProvider.class.getName()).isEmpty()) {
DefaultTokenSignatureProviders.createProviders(newRealm);
}

}

public static void importUserFederationProvidersAndMappers(KeycloakSession session, RealmRepresentation rep, RealmModel newRealm) {
Expand Down
Expand Up @@ -70,4 +70,7 @@ org.keycloak.credential.hash.PasswordHashSpi
org.keycloak.credential.CredentialSpi
org.keycloak.keys.PublicKeyStorageSpi
org.keycloak.keys.KeySpi
org.keycloak.storage.client.ClientStorageProviderSpi
org.keycloak.storage.client.ClientStorageProviderSpi
# KEYCLOAK-7560 Refactoring Token Signing and Verifying by Token Signature SPI
org.keycloak.jose.jws.TokenSignatureSpi