KEYCLOAK-9651 Wrong ECDSA signature R and S encoding

This commit is contained in:
Martin Kanis 2019-11-01 14:44:56 +01:00 committed by Hynek Mlnařík
parent df5cdea1e8
commit 25511d4dbf
17 changed files with 341 additions and 18 deletions

View file

@ -0,0 +1,36 @@
package org.keycloak.crypto;
import org.keycloak.common.VerificationException;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.keys.loader.PublicKeyStorageManager;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
public class ClientECDSASignatureVerifierContext extends AsymmetricSignatureVerifierContext {
public ClientECDSASignatureVerifierContext(KeycloakSession session, ClientModel client, JWSInput input) throws VerificationException {
super(getKey(session, client, input));
}
private static KeyWrapper getKey(KeycloakSession session, ClientModel client, JWSInput input) throws VerificationException {
KeyWrapper key = PublicKeyStorageManager.getClientPublicKeyWrapper(session, client, input);
if (key == null) {
throw new VerificationException("Key not found");
}
return key;
}
@Override
public boolean verify(byte[] data, byte[] signature) throws VerificationException {
try {
/*
Fallback for backwards compatibility of ECDSA signed tokens which were issued in previous versions.
TODO remove by https://issues.jboss.org/browse/KEYCLOAK-11911
*/
int expectedSize = ECDSASignatureProvider.ECDSA.valueOf(getAlgorithm()).getSignatureLength();
byte[] derSignature = expectedSize != signature.length && signature[0] == 0x30 ? signature : ECDSASignatureProvider.concatenatedRSToASN1DER(signature, expectedSize);
return super.verify(data, derSignature);
} catch (Exception e) {
throw new VerificationException("Signing failed", e);
}
}
}

View file

@ -0,0 +1,21 @@
package org.keycloak.crypto;
import org.keycloak.common.VerificationException;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
public class ECDSAClientSignatureVerifierProvider implements ClientSignatureVerifierProvider {
private final KeycloakSession session;
private final String algorithm;
public ECDSAClientSignatureVerifierProvider(KeycloakSession session, String algorithm) {
this.session = session;
this.algorithm = algorithm;
}
@Override
public SignatureVerifierContext verifier(ClientModel client, JWSInput input) throws VerificationException {
return new ClientECDSASignatureVerifierContext(session, client, input);
}
}

View file

@ -0,0 +1,94 @@
package org.keycloak.crypto;
import org.bouncycastle.asn1.ASN1InputStream;
import org.bouncycastle.asn1.ASN1Integer;
import org.bouncycastle.asn1.ASN1Primitive;
import org.bouncycastle.asn1.ASN1Sequence;
import org.bouncycastle.asn1.DERSequenceGenerator;
import org.bouncycastle.asn1.x9.X9IntegerConverter;
import org.keycloak.common.VerificationException;
import org.keycloak.models.KeycloakSession;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.math.BigInteger;
public class ECDSASignatureProvider implements SignatureProvider {
private final KeycloakSession session;
private final String algorithm;
public ECDSASignatureProvider(KeycloakSession session, String algorithm) {
this.session = session;
this.algorithm = algorithm;
}
@Override
public SignatureSignerContext signer() throws SignatureException {
return new ServerECDSASignatureSignerContext(session, algorithm);
}
@Override
public SignatureVerifierContext verifier(String kid) throws VerificationException {
return new ServerECDSASignatureVerifierContext(session, kid, algorithm);
}
public static byte[] concatenatedRSToASN1DER(final byte[] signature, int signLength) throws IOException {
int len = signLength / 2;
int arraySize = len + 1;
byte[] r = new byte[arraySize];
byte[] s = new byte[arraySize];
System.arraycopy(signature, 0, r, 1, len);
System.arraycopy(signature, len, s, 1, len);
BigInteger rBigInteger = new BigInteger(r);
BigInteger sBigInteger = new BigInteger(s);
ByteArrayOutputStream bos = new ByteArrayOutputStream();
DERSequenceGenerator seqGen = new DERSequenceGenerator(bos);
seqGen.addObject(new ASN1Integer(rBigInteger.toByteArray()));
seqGen.addObject(new ASN1Integer(sBigInteger.toByteArray()));
seqGen.close();
bos.close();
return bos.toByteArray();
}
public static byte[] asn1derToConcatenatedRS(final byte[] derEncodedSignatureValue, int signLength) throws IOException {
int len = signLength / 2;
ASN1InputStream asn1InputStream = new ASN1InputStream(derEncodedSignatureValue);
ASN1Primitive asn1Primitive = asn1InputStream.readObject();
asn1InputStream.close();
ASN1Sequence asn1Sequence = (ASN1Sequence.getInstance(asn1Primitive));
ASN1Integer rASN1 = (ASN1Integer) asn1Sequence.getObjectAt(0);
ASN1Integer sASN1 = (ASN1Integer) asn1Sequence.getObjectAt(1);
X9IntegerConverter x9IntegerConverter = new X9IntegerConverter();
byte[] r = x9IntegerConverter.integerToBytes(rASN1.getValue(), len);
byte[] s = x9IntegerConverter.integerToBytes(sASN1.getValue(), len);
byte[] concatenatedSignatureValue = new byte[signLength];
System.arraycopy(r, 0, concatenatedSignatureValue, 0, len);
System.arraycopy(s, 0, concatenatedSignatureValue, len, len);
return concatenatedSignatureValue;
}
public enum ECDSA {
ES256(64),
ES384(96),
ES512(132);
private final int signatureLength;
ECDSA(int signatureLength) {
this.signatureLength = signatureLength;
}
public int getSignatureLength() {
return this.signatureLength;
}
}
}

View file

@ -29,7 +29,7 @@ public class ES256ClientSignatureVerifierProviderFactory implements ClientSignat
@Override
public ClientSignatureVerifierProvider create(KeycloakSession session) {
return new AsymmetricClientSignatureVerifierProvider(session, Algorithm.ES256);
return new ECDSAClientSignatureVerifierProvider(session, Algorithm.ES256);
}
}

View file

@ -29,7 +29,7 @@ public class ES256SignatureProviderFactory implements SignatureProviderFactory {
@Override
public SignatureProvider create(KeycloakSession session) {
return new AsymmetricSignatureProvider(session, Algorithm.ES256);
return new ECDSASignatureProvider(session, Algorithm.ES256);
}
}

View file

@ -29,7 +29,7 @@ public class ES384ClientSignatureVerifierProviderFactory implements ClientSignat
@Override
public ClientSignatureVerifierProvider create(KeycloakSession session) {
return new AsymmetricClientSignatureVerifierProvider(session, Algorithm.ES384);
return new ECDSAClientSignatureVerifierProvider(session, Algorithm.ES384);
}
}

View file

@ -29,7 +29,7 @@ public class ES384SignatureProviderFactory implements SignatureProviderFactory {
@Override
public SignatureProvider create(KeycloakSession session) {
return new AsymmetricSignatureProvider(session, Algorithm.ES384);
return new ECDSASignatureProvider(session, Algorithm.ES384);
}
}

View file

@ -29,7 +29,7 @@ public class ES512ClientSignatureVerifierProviderFactory implements ClientSigna
@Override
public ClientSignatureVerifierProvider create(KeycloakSession session) {
return new AsymmetricClientSignatureVerifierProvider(session, Algorithm.ES512);
return new ECDSAClientSignatureVerifierProvider(session, Algorithm.ES512);
}
}

View file

@ -29,7 +29,7 @@ public class ES512SignatureProviderFactory implements SignatureProviderFactory {
@Override
public SignatureProvider create(KeycloakSession session) {
return new AsymmetricSignatureProvider(session, Algorithm.ES512);
return new ECDSASignatureProvider(session, Algorithm.ES512);
}
}

View file

@ -24,7 +24,7 @@ public class ServerAsymmetricSignatureSignerContext extends AsymmetricSignatureS
super(getKey(session, algorithm));
}
private static KeyWrapper getKey(KeycloakSession session, String algorithm) {
static KeyWrapper getKey(KeycloakSession session, String algorithm) {
KeyWrapper key = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.SIG, algorithm);
if (key == null) {
throw new SignatureException("Active key for " + algorithm + " not found");

View file

@ -25,7 +25,7 @@ public class ServerAsymmetricSignatureVerifierContext extends AsymmetricSignatur
super(getKey(session, kid, algorithm));
}
private static KeyWrapper getKey(KeycloakSession session, String kid, String algorithm) throws VerificationException {
static KeyWrapper getKey(KeycloakSession session, String kid, String algorithm) throws VerificationException {
KeyWrapper key = session.keys().getKey(session.getContext().getRealm(), kid, KeyUse.SIG, algorithm);
if (key == null) {
throw new VerificationException("Key not found");

View file

@ -0,0 +1,24 @@
package org.keycloak.crypto;
import org.keycloak.models.KeycloakSession;
public class ServerECDSASignatureSignerContext extends AsymmetricSignatureSignerContext {
public ServerECDSASignatureSignerContext(KeycloakSession session, String algorithm) throws SignatureException {
super(ServerAsymmetricSignatureSignerContext.getKey(session, algorithm));
}
public ServerECDSASignatureSignerContext(KeyWrapper key) {
super(key);
}
@Override
public byte[] sign(byte[] data) throws SignatureException {
try {
int size = ECDSASignatureProvider.ECDSA.valueOf(getAlgorithm()).getSignatureLength();
return ECDSASignatureProvider.asn1derToConcatenatedRS(super.sign(data), size);
} catch (Exception e) {
throw new SignatureException("Signing failed", e);
}
}
}

View file

@ -0,0 +1,29 @@
package org.keycloak.crypto;
import org.keycloak.common.VerificationException;
import org.keycloak.models.KeycloakSession;
public class ServerECDSASignatureVerifierContext extends AsymmetricSignatureVerifierContext {
public ServerECDSASignatureVerifierContext(KeycloakSession session, String kid, String algorithm) throws VerificationException {
super(ServerAsymmetricSignatureVerifierContext.getKey(session, kid, algorithm));
}
public ServerECDSASignatureVerifierContext(KeyWrapper key) {
super(key);
}
@Override
public boolean verify(byte[] data, byte[] signature) throws VerificationException {
try {
/*
Fallback for backwards compatibility of ECDSA signed tokens which were issued in previous versions.
TODO remove by https://issues.jboss.org/browse/KEYCLOAK-11911
*/
int expectedSize = ECDSASignatureProvider.ECDSA.valueOf(getAlgorithm()).getSignatureLength();
byte[] derSignature = expectedSize != signature.length && signature[0] == 0x30 ? signature : ECDSASignatureProvider.concatenatedRSToASN1DER(signature, expectedSize);
return super.verify(data, derSignature);
} catch (Exception e) {
throw new VerificationException("Signing failed", e);
}
}
}

View file

@ -28,6 +28,7 @@ import org.keycloak.crypto.AsymmetricSignatureSignerContext;
import org.keycloak.crypto.KeyType;
import org.keycloak.crypto.KeyUse;
import org.keycloak.crypto.KeyWrapper;
import org.keycloak.crypto.ServerECDSASignatureSignerContext;
import org.keycloak.crypto.SignatureSignerContext;
import org.keycloak.jose.jwe.JWEConstants;
import org.keycloak.jose.jwk.JSONWebKeySet;
@ -214,7 +215,16 @@ public class TestingOIDCEndpointsApplicationResource {
keyWrapper.setAlgorithm(clientData.getSigningKeyAlgorithm());
keyWrapper.setKid(kid);
keyWrapper.setPrivateKey(privateKey);
SignatureSignerContext signer = new AsymmetricSignatureSignerContext(keyWrapper);
SignatureSignerContext signer;
switch (clientData.getSigningKeyAlgorithm()) {
case Algorithm.ES256:
case Algorithm.ES384:
case Algorithm.ES512:
signer = new ServerECDSASignatureSignerContext(keyWrapper);
break;
default:
signer = new AsymmetricSignatureSignerContext(keyWrapper);
}
clientData.setOidcRequest(new JWSBuilder().kid(kid).jsonContent(oidcRequest).sign(signer));
}
}

View file

@ -37,14 +37,21 @@ import org.keycloak.OAuth2Constants;
import org.keycloak.TokenVerifier;
import org.keycloak.broker.provider.util.SimpleHttp;
import org.keycloak.common.VerificationException;
import org.keycloak.common.util.KeyUtils;
import org.keycloak.common.util.KeystoreUtil;
import org.keycloak.constants.AdapterConstants;
import org.keycloak.crypto.Algorithm;
import org.keycloak.crypto.AsymmetricSignatureSignerContext;
import org.keycloak.crypto.AsymmetricSignatureVerifierContext;
import org.keycloak.crypto.KeyUse;
import org.keycloak.crypto.KeyWrapper;
import org.keycloak.crypto.ServerECDSASignatureSignerContext;
import org.keycloak.crypto.ServerECDSASignatureVerifierContext;
import org.keycloak.crypto.SignatureSignerContext;
import org.keycloak.jose.jwk.JSONWebKeySet;
import org.keycloak.jose.jwk.JWK;
import org.keycloak.jose.jwk.JWKParser;
import org.keycloak.jose.jws.JWSBuilder;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.models.Constants;
import org.keycloak.models.utils.KeycloakModelUtils;
@ -75,6 +82,7 @@ import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.security.KeyStore;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.util.Collections;
import java.util.HashMap;
@ -710,7 +718,16 @@ public class OAuthClient {
String kid = verifier.getHeader().getKeyId();
String algorithm = verifier.getHeader().getAlgorithm().name();
KeyWrapper key = getRealmPublicKey(realm, algorithm, kid);
AsymmetricSignatureVerifierContext verifierContext = new AsymmetricSignatureVerifierContext(key);
AsymmetricSignatureVerifierContext verifierContext;
switch (algorithm) {
case Algorithm.ES256:
case Algorithm.ES384:
case Algorithm.ES512:
verifierContext = new ServerECDSASignatureVerifierContext(key);
break;
default:
verifierContext = new AsymmetricSignatureVerifierContext(key);
}
verifier.verifierContext(verifierContext);
verifier.verify();
return verifier.getToken();
@ -719,6 +736,24 @@ public class OAuthClient {
}
}
public SignatureSignerContext createSigner(PrivateKey privateKey, String kid, String algorithm) {
KeyWrapper keyWrapper = new KeyWrapper();
keyWrapper.setAlgorithm(algorithm);
keyWrapper.setKid(kid);
keyWrapper.setPrivateKey(privateKey);
SignatureSignerContext signer;
switch (algorithm) {
case Algorithm.ES256:
case Algorithm.ES384:
case Algorithm.ES512:
signer = new ServerECDSASignatureSignerContext(keyWrapper);
break;
default:
signer = new AsymmetricSignatureSignerContext(keyWrapper);
}
return signer;
}
public String getClientId() {
return clientId;
}

View file

@ -24,6 +24,7 @@ import org.apache.http.client.methods.HttpPost;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.message.BasicNameValuePair;
import org.hamcrest.collection.IsArrayContaining;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
@ -34,7 +35,9 @@ import org.keycloak.admin.client.resource.ClientScopeResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.common.enums.SslRequired;
import org.keycloak.common.util.Base64Url;
import org.keycloak.crypto.Algorithm;
import org.keycloak.crypto.ECDSASignatureProvider;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.jose.jws.JWSHeader;
@ -94,6 +97,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import static org.keycloak.testsuite.admin.ApiUtil.findClientByClientId;
@ -1141,6 +1145,40 @@ public class AccessTokenTest extends AbstractKeycloakTest {
conductAccessTokenRequest(Algorithm.HS256, Algorithm.ES512, Algorithm.RS256);
}
@Test
public void validateECDSASignatures() {
validateTokenECDSASignature(Algorithm.ES256);
validateTokenECDSASignature(Algorithm.ES384);
validateTokenECDSASignature(Algorithm.ES512);
}
private void validateTokenECDSASignature(String expectedAlg) {
assertThat(ECDSASignatureProvider.ECDSA.values(), IsArrayContaining.hasItemInArray(ECDSASignatureProvider.ECDSA.valueOf(expectedAlg)));
try {
TokenSignatureUtil.changeRealmTokenSignatureProvider(adminClient, expectedAlg);
TokenSignatureUtil.changeClientAccessTokenSignatureProvider(ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"), expectedAlg);
validateTokenSignatureLength(ECDSASignatureProvider.ECDSA.valueOf(expectedAlg).getSignatureLength());
} finally {
TokenSignatureUtil.changeRealmTokenSignatureProvider(adminClient, Algorithm.RS256);
TokenSignatureUtil.changeClientAccessTokenSignatureProvider(ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"), Algorithm.RS256);
}
}
private void validateTokenSignatureLength(int expectedLength) {
oauth.doLogin("test-user@localhost", "password");
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, "password");
String token = response.getAccessToken();
oauth.verifyToken(token);
String encodedSignature = token.split("\\.",3)[2];
byte[] signature = Base64Url.decode(encodedSignature);
Assert.assertEquals(expectedLength, signature.length);
oauth.openLogout();
}
private void conductAccessTokenRequest(String expectedRefreshAlg, String expectedAccessAlg, String expectedIdTokenAlg) throws Exception {
try {
/// Realm Setting is used for ID Token Signature Algorithm

View file

@ -46,9 +46,8 @@ import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.common.util.*;
import org.keycloak.constants.ServiceUrlConstants;
import org.keycloak.crypto.Algorithm;
import org.keycloak.crypto.AsymmetricSignatureSignerContext;
import org.keycloak.crypto.ECDSASignatureProvider;
import org.keycloak.crypto.KeyType;
import org.keycloak.crypto.KeyWrapper;
import org.keycloak.crypto.SignatureSignerContext;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
@ -295,6 +294,48 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
testCodeToTokenRequestSuccess(Algorithm.PS256);
}
@Test
public void testECDSASignature() throws Exception {
testECDSASignatureLength(getClientSignedToken(Algorithm.ES256), Algorithm.ES256);
testECDSASignatureLength(getClientSignedToken(Algorithm.ES384), Algorithm.ES384);
testECDSASignatureLength(getClientSignedToken(Algorithm.ES512), Algorithm.ES512);
}
private void testECDSASignatureLength(String clientSignedToken, String alg) {
String encodedSignature = clientSignedToken.split("\\.",3)[2];
byte[] signature = Base64Url.decode(encodedSignature);
assertEquals(ECDSASignatureProvider.ECDSA.valueOf(alg).getSignatureLength(), signature.length);
}
private String getClientSignedToken(String alg) throws Exception {
ClientRepresentation clientRepresentation = app2;
ClientResource clientResource = getClient(testRealm.getRealm(), clientRepresentation.getId());
clientRepresentation = clientResource.toRepresentation();
String clientSignedToken;
try {
// setup Jwks
KeyPair keyPair = setupJwks(alg, clientRepresentation, clientResource);
PublicKey publicKey = keyPair.getPublic();
PrivateKey privateKey = keyPair.getPrivate();
// test
oauth.clientId("client2");
oauth.doLogin("test-user@localhost", "password");
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
clientSignedToken = createSignedRequestToken("client2", getRealmInfoUrl(), privateKey, publicKey, alg);
OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, clientSignedToken);
assertEquals(200, response.getStatusCode());
oauth.verifyToken(response.getAccessToken());
oauth.openLogout();
return clientSignedToken;
} finally {
// Revert jwks_url settings
revertJwksSettings(clientRepresentation, clientResource);
}
}
private void testCodeToTokenRequestSuccess(String algorithm) throws Exception {
ClientRepresentation clientRepresentation = app2;
ClientResource clientResource = getClient(testRealm.getRealm(), clientRepresentation.getId());
@ -1154,14 +1195,9 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
private String createSignedRequestToken(String clientId, String realmInfoUrl, PrivateKey privateKey, PublicKey publicKey, String algorithm) {
JsonWebToken jwt = createRequestToken(clientId, realmInfoUrl);
String kid = KeyUtils.createKeyId(publicKey);
KeyWrapper keyWrapper = new KeyWrapper();
keyWrapper.setAlgorithm(algorithm);
keyWrapper.setKid(kid);
keyWrapper.setPrivateKey(privateKey);
SignatureSignerContext signer = new AsymmetricSignatureSignerContext(keyWrapper);
SignatureSignerContext signer = oauth.createSigner(privateKey, kid, algorithm);
String ret = new JWSBuilder().kid(kid).jsonContent(jwt).sign(signer);
return ret;
}
private JsonWebToken createRequestToken(String clientId, String realmInfoUrl) {