Skip to content

Commit

Permalink
fix(jans-auth-server): key_ops in jwks must be array #3777 (#3778)
Browse files Browse the repository at this point in the history
  • Loading branch information
yuriyz committed Feb 6, 2023
1 parent 592c58e commit 2be2a03
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static io.jans.as.model.jwk.JWKParameter.*;
Expand Down Expand Up @@ -235,7 +236,7 @@ private void generateEncryptionKeys(KeyGeneratorContext context, List<Algorithm>
key.setE(result.optString(EXPONENT));
key.setX(result.optString(X));
key.setY(result.optString(Y));
key.setKeyOps(keyOps);
key.setKeyOps(Collections.singletonList(keyOps));

JSONArray x5c = result.optJSONArray(CERTIFICATE_CHAIN);
key.setX5c(StringUtils.toList(x5c));
Expand Down Expand Up @@ -265,7 +266,7 @@ private void generateSignatureKeys(KeyGeneratorContext context, List<Algorithm>
key.setE(result.optString(EXPONENT));
key.setX(result.optString(X));
key.setY(result.optString(Y));
key.setKeyOps(keyOps);
key.setKeyOps(Collections.singletonList(keyOps));

JSONArray x5c = result.optJSONArray(CERTIFICATE_CHAIN);
key.setX5c(StringUtils.toList(x5c));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public List<String> getKeys() {
public abstract PublicKey getPublicKey(String alias) throws CryptoProviderException;

@SuppressWarnings("java:S1130")
public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use) throws CryptoProviderException {
public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use, KeyOps keyOps) throws CryptoProviderException {
if (algorithm == null || AlgorithmFamily.HMAC.equals(algorithm.getFamily())) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ public PublicKey getPublicKey(String alias) throws CryptoProviderException {
}

@Override
public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use) throws CryptoProviderException {
public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use, KeyOps keyOps) throws CryptoProviderException {
if (algorithm == null || AlgorithmFamily.HMAC.equals(algorithm.getFamily())) {
return null;
}
Expand All @@ -315,7 +315,8 @@ public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use
List<JSONWebKey> keysByAlgAndUse = new ArrayList<>();

for (JSONWebKey key : keys) {
if (algorithm == key.getAlg() && (use == null || use == key.getUse())) {
boolean keyOpsCondition = keyOps == null || (key.getKeyOps() == null || key.getKeyOps().contains(keyOps));
if (algorithm == key.getAlg() && (use == null || use == key.getUse()) && keyOpsCondition) {
kid = key.getKid();
Key keyFromStore;
keyFromStore = keyStore.getKey(kid, keyStoreSecret.toCharArray());
Expand All @@ -326,7 +327,7 @@ public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use
}

if (keysByAlgAndUse.isEmpty()) {
LOG.trace("kid is not in keystore, algorithm: " + algorithm + ", kid: " + kid + ", keyStorePath:" + keyStoreFile);
LOG.trace("kid is not in keystore, algorithm: {}" + algorithm + ", kid: " + kid + ", keyStorePath:" + keyStoreFile + ", keyOps: " + keyOps + ", use: " + use);
return kid;
}

Expand All @@ -339,7 +340,6 @@ public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use
} catch (UnrecoverableKeyException | KeyStoreException | NoSuchAlgorithmException e) {
throw new CryptoProviderException(e);
}

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.jans.as.model.util.JwtUtil;
import io.jans.as.model.util.StringUtils;
import io.jans.as.model.util.Util;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;

Expand Down Expand Up @@ -44,7 +45,7 @@ public class JSONWebKey {
private EllipticEdvardsCurve crv;
private List<String> x5c;
@JsonProperty("key_ops")
private KeyOps keyOps;
private List<KeyOps> keyOps;

/**
* Modulus
Expand All @@ -64,7 +65,7 @@ public class JSONWebKey {
*
* @return key ops
*/
public KeyOps getKeyOps() {
public List<KeyOps> getKeyOps() {
return keyOps;
}

Expand All @@ -73,7 +74,7 @@ public KeyOps getKeyOps() {
*
* @param keyOps key ops
*/
public void setKeyOps(KeyOps keyOps) {
public void setKeyOps(List<KeyOps> keyOps) {
this.keyOps = keyOps;
}

Expand Down Expand Up @@ -371,9 +372,15 @@ public JSONObject toJSONObject() throws JSONException {
if (use != null) {
jsonObj.put(JWKParameter.KEY_USE, use.getParamName());
}

JSONArray keyOpsArray = new JSONArray();
if (keyOps != null) {
jsonObj.put(JWKParameter.KEY_OPS, keyOps.getValue());
for (KeyOps keyOp : keyOps) {
keyOpsArray.put(keyOp.getValue());
}
}
jsonObj.put(JWKParameter.KEY_OPS, keyOpsArray);

jsonObj.put(JWKParameter.ALGORITHM, alg);
jsonObj.put(JWKParameter.EXPIRATION_TIME, exp);
if (crv != null) {
Expand Down Expand Up @@ -407,7 +414,7 @@ public static JSONWebKey fromJSONObject(JSONObject jwkJSONObject) throws JSONExc
jwk.setKty(KeyType.fromString(jwkJSONObject.optString(JWKParameter.KEY_TYPE)));
jwk.setUse(Use.fromString(jwkJSONObject.optString(JWKParameter.KEY_USE)));
jwk.setAlg(Algorithm.fromString(jwkJSONObject.optString(JWKParameter.ALGORITHM)));
jwk.setKeyOps(KeyOps.fromString(jwkJSONObject.optString(JWKParameter.KEY_OPS)));
jwk.setKeyOps(KeyOps.fromJSONArray(jwkJSONObject.optJSONArray(JWKParameter.KEY_OPS)));
if (jwkJSONObject.has(JWKParameter.EXPIRATION_TIME)) {
jwk.setExp(jwkJSONObject.optLong(JWKParameter.EXPIRATION_TIME));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package io.jans.as.model.jwk;

import com.fasterxml.jackson.annotation.JsonCreator;
import org.json.JSONArray;

import java.util.ArrayList;
import java.util.List;

/**
* @author Yuriy Z
Expand Down Expand Up @@ -33,6 +37,22 @@ public static KeyOps fromString(String valueString) {
return null;
}

public static List<KeyOps> fromJSONArray(JSONArray jsonArray) {
List<KeyOps> result = new ArrayList<>();
if (jsonArray == null) {
return result;
}

for (int i = 0; i < jsonArray.length(); i++) {
final KeyOps v = fromString(jsonArray.optString(i));
if (v != null) {
result.add(v);
}
}

return result;
}

@Override
public String toString() {
return "KeyOps{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.jans.as.model.jwe.Jwe;
import io.jans.as.model.jwk.Algorithm;
import io.jans.as.model.jwk.JSONWebKeySet;
import io.jans.as.model.jwk.KeyOps;
import io.jans.as.model.jwk.Use;
import io.jans.as.model.jwt.Jwt;
import io.jans.as.model.jwt.JwtClaimName;
Expand Down Expand Up @@ -434,7 +435,7 @@ public void fillRedirectUriResponseforJARM(RedirectUriResponse redirectUriRespon
.fromString(client.getAttributes().getAuthorizationSignedResponseAlg());

String nestedKeyId = new ServerCryptoProvider(cryptoProvider).getKeyId(webKeysConfiguration,
Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE);
Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE, KeyOps.CONNECT);

JSONObject jsonWebKeys = CommonUtils.getJwks(client);
redirectUriResponse.getRedirectUri().setNestedJsonWebKeys(jsonWebKeys);
Expand All @@ -449,7 +450,7 @@ public void fillRedirectUriResponseforJARM(RedirectUriResponse redirectUriRespon
if (jsonWebKeys != null) {
keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(JSONWebKeySet.fromJSONObject(jsonWebKeys),
Algorithm.fromString(client.getAttributes().getAuthorizationEncryptedResponseAlg()),
Use.ENCRYPTION);
Use.ENCRYPTION, KeyOps.CONNECT);
}
String sharedSecret = clientService.decryptSecret(client.getClientSecret());
byte[] sharedSymmetricKey = sharedSecret.getBytes(StandardCharsets.UTF_8);
Expand All @@ -464,7 +465,7 @@ public void fillRedirectUriResponseforJARM(RedirectUriResponse redirectUriRespon
}

keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(webKeysConfiguration,
Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE);
Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE, KeyOps.CONNECT);

JSONObject jsonWebKeys = CommonUtils.getJwks(client);
redirectUriResponse.getRedirectUri().setJsonWebKeys(jsonWebKeys);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.jans.as.model.jwe.JweEncrypterImpl;
import io.jans.as.model.jwk.Algorithm;
import io.jans.as.model.jwk.JSONWebKeySet;
import io.jans.as.model.jwk.KeyOps;
import io.jans.as.model.jwk.Use;
import io.jans.as.model.jwt.Jwt;
import io.jans.as.model.jwt.JwtType;
Expand Down Expand Up @@ -106,7 +107,7 @@ private Jwe encryptJwe(Jwe jwe, Client client) throws Exception {
JSONObject jsonWebKeys = CommonUtils.getJwks(client);
String keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(JSONWebKeySet.fromJSONObject(jsonWebKeys),
Algorithm.fromString(keyEncryptionAlgorithm.getName()),
Use.ENCRYPTION);
Use.ENCRYPTION, KeyOps.CONNECT);
PublicKey publicKey = cryptoProvider.getPublicKey(keyId, jsonWebKeys, null);
jwe.getHeader().setKeyId(keyId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
import io.jans.as.model.configuration.AppConfiguration;
import io.jans.as.model.crypto.AbstractCryptoProvider;
import io.jans.as.model.crypto.signature.SignatureAlgorithm;
import io.jans.as.model.exception.CryptoProviderException;
import io.jans.as.model.jwk.Algorithm;
import io.jans.as.model.jwk.JSONWebKeySet;
import io.jans.as.model.jwk.KeyOps;
import io.jans.as.model.jwk.Use;
import io.jans.as.model.jwt.Jwt;
import io.jans.as.model.jwt.JwtType;
Expand Down Expand Up @@ -90,14 +92,14 @@ public Jwt newJwt() throws Exception {
return jwt;
}

private String getKid() throws Exception {
private String getKid() throws CryptoProviderException {
final String staticKid = appConfiguration.getStaticKid();
if (StringUtils.isNotBlank(staticKid)) {
log.trace("Use staticKid: " + staticKid);
log.trace("Use staticKid: {}", staticKid);
return staticKid;
}

return cryptoProvider.getKeyId(webKeys, Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE);
return cryptoProvider.getKeyId(webKeys, Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE, KeyOps.CONNECT);
}

public Jwt sign() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.jans.as.model.error.ErrorResponseFactory;
import io.jans.as.model.exception.CryptoProviderException;
import io.jans.as.model.jwk.Algorithm;
import io.jans.as.model.jwk.KeyOps;
import io.jans.as.model.jwk.Use;
import io.jans.as.persistence.model.Scope;
import io.jans.as.server.auth.Authenticator;
Expand Down Expand Up @@ -287,7 +288,7 @@ private String createJarmRedirectUri(RedirectUri redirectUri, Client client, fin
String clientSecret = clientService.decryptSecret(client.getClientSecret());
redirectUri.setSharedSecret(clientSecret);
keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(webKeysConfiguration,
Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE);
Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE, KeyOps.CONNECT);
} catch (CryptoProviderException e) {
log.error(e.getMessage(), e);
} catch (StringEncrypter.EncryptionException e) {
Expand All @@ -312,7 +313,6 @@ public List<Scope> getScopes() {
String scope = session.getSessionAttributes().get("scope");

return getScopes(scope);

}

public List<Scope> getScopes(String scopes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.jans.as.model.jwk.*;
import io.jans.as.server.model.config.ConfigurationFactory;
import io.jans.service.cdi.util.CdiUtil;
import org.apache.commons.lang.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.json.JSONObject;
Expand All @@ -40,14 +41,14 @@ public ServerCryptoProvider(AbstractCryptoProvider cryptoProvider) {
}

@Override
public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use) throws CryptoProviderException {
public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use, KeyOps keyOps) throws CryptoProviderException {
try {
if (algorithm == null || AlgorithmFamily.HMAC.equals(algorithm.getFamily())) {
return null;
}

final AppConfiguration appConfiguration = configurationFactory.getAppConfiguration();
if (appConfiguration.getKeySignWithSameKeyButDiffAlg()) { // open banking: same key with different algorithms
if (BooleanUtils.isTrue(appConfiguration.getKeySignWithSameKeyButDiffAlg())) { // open banking: same key with different algorithms
LOG.trace("Getting key by use: " + use);
for (JSONWebKey key : jsonWebKeySet.getKeys()) {
if (use != null && use == key.getUse()) {
Expand All @@ -63,16 +64,16 @@ public String getKeyId(JSONWebKeySet jsonWebKeySet, Algorithm algorithm, Use use
return staticKid;
}

final String kid = cryptoProvider.getKeyId(jsonWebKeySet, algorithm, use);
final String kid = cryptoProvider.getKeyId(jsonWebKeySet, algorithm, use, keyOps);
if (!cryptoProvider.getKeys().contains(kid) && configurationFactory.reloadConfFromLdap()) {
return cryptoProvider.getKeyId(jsonWebKeySet, algorithm, use);
return cryptoProvider.getKeyId(jsonWebKeySet, algorithm, use, keyOps);
}
return kid;

} catch (CryptoProviderException e) {
LOG.trace("Try to re-load configuration due to keystore exception (it can be rotated).");
if (configurationFactory.reloadConfFromLdap()) {
return cryptoProvider.getKeyId(jsonWebKeySet, algorithm, use);
return cryptoProvider.getKeyId(jsonWebKeySet, algorithm, use, keyOps);
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.jans.as.model.jwe.JweEncrypterImpl;
import io.jans.as.model.jwk.Algorithm;
import io.jans.as.model.jwk.JSONWebKeySet;
import io.jans.as.model.jwk.KeyOps;
import io.jans.as.model.jwk.Use;
import io.jans.as.model.jwt.Jwt;
import io.jans.as.model.jwt.JwtClaims;
Expand Down Expand Up @@ -242,7 +243,7 @@ private String getJwtResponse(SignatureAlgorithm signatureAlgorithm, User user,
jwt.getHeader().setType(JwtType.JWT);
jwt.getHeader().setAlgorithm(signatureAlgorithm);

String keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(webKeysConfiguration, Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE);
String keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(webKeysConfiguration, Algorithm.fromString(signatureAlgorithm.getName()), Use.SIGNATURE, KeyOps.CONNECT);
if (keyId != null) {
jwt.getHeader().setKeyId(keyId);
}
Expand Down Expand Up @@ -288,7 +289,7 @@ public String getJweResponse(
JSONObject jsonWebKeys = CommonUtils.getJwks(authorizationGrant.getClient());
String keyId = new ServerCryptoProvider(cryptoProvider).getKeyId(JSONWebKeySet.fromJSONObject(jsonWebKeys),
Algorithm.fromString(keyEncryptionAlgorithm.getName()),
Use.ENCRYPTION);
Use.ENCRYPTION, KeyOps.CONNECT);
PublicKey publicKey = cryptoProvider.getPublicKey(keyId, jsonWebKeys, null);

if (publicKey != null) {
Expand Down

0 comments on commit 2be2a03

Please sign in to comment.