From 6af5b297e19ac1f9d6de354e19c9d72018d1fff3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francesco=20Chicchiricc=C3=B2?= Date: Wed, 19 Nov 2025 15:48:30 +0100 Subject: [PATCH] [SYNCOPE-1932] Improve AES management --- .../syncope/core/logic/AccessTokenLogic.java | 4 +- .../resources/core-majson-test.properties | 2 +- .../resources/core-myjson-test.properties | 2 +- .../test/resources/core-ojson-test.properties | 2 +- .../resources/core-pgjsonb-test.properties | 2 +- .../jpa/entity/user/JPALinkedAccount.java | 22 +-- .../persistence/jpa/entity/user/JPAUser.java | 26 ++-- .../src/test/resources/core-test.properties | 2 +- .../java/DefaultMappingManager.java | 4 +- .../java/data/AnyTypeDataBinderImpl.java | 16 +-- .../java/utils/ConnObjectUtils.java | 4 +- .../src/test/resources/core-debug.properties | 2 +- .../spring/policy/DefaultPasswordRule.java | 4 +- .../policy/HaveIBeenPwnedPasswordRule.java | 17 +-- .../spring/security/AuthDataAccessor.java | 4 +- .../core/spring/security/Encryptor.java | 133 +++++++++++------- .../core/spring/security/SecurityContext.java | 7 +- .../spring/security/SecurityProperties.java | 10 +- .../security/SyncopeJWTSSOProvider.java | 4 +- ...sernamePasswordAuthenticationProvider.java | 6 +- .../core/spring/security/EncryptorTest.java | 4 +- .../src/main/resources/core.properties | 9 +- .../main/resources/core-embedded.properties | 2 +- .../syncope/fit/core/KeymasterITCase.java | 4 +- pom.xml | 2 +- .../configurationparameters.adoc | 15 +- 26 files changed, 163 insertions(+), 146 deletions(-) diff --git a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java index 56d2063e2e2..78b1d69b2d9 100644 --- a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java +++ b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java @@ -43,12 +43,10 @@ public class AccessTokenLogic extends AbstractTransactionalLogic { - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected static byte[] getAuthorities() { byte[] authorities = null; try { - authorities = ENCRYPTOR.encode(POJOHelper.serialize( + authorities = Encryptor.getInstance().encode(POJOHelper.serialize( AuthContextUtils.getAuthorities()), CipherAlgorithm.AES). getBytes(); } catch (Exception e) { diff --git a/core/persistence-jpa-json/src/test/resources/core-majson-test.properties b/core/persistence-jpa-json/src/test/resources/core-majson-test.properties index 9f235bece14..f89cebdcb82 100644 --- a/core/persistence-jpa-json/src/test/resources/core-majson-test.properties +++ b/core/persistence-jpa-json/src/test/resources/core-majson-test.properties @@ -18,7 +18,7 @@ security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=${secretKey} persistence.domain[0].jdbcURL=jdbc:mariadb://${DB_CONTAINER_IP}:3306/syncope?characterEncoding=UTF-8 persistence.domain[0].poolMaxActive=10 diff --git a/core/persistence-jpa-json/src/test/resources/core-myjson-test.properties b/core/persistence-jpa-json/src/test/resources/core-myjson-test.properties index 4bd61242286..53a03729ad5 100644 --- a/core/persistence-jpa-json/src/test/resources/core-myjson-test.properties +++ b/core/persistence-jpa-json/src/test/resources/core-myjson-test.properties @@ -18,7 +18,7 @@ security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=${secretKey} persistence.domain[0].jdbcURL=jdbc:mysql://${DB_CONTAINER_IP}:3306/syncope?useSSL=false&allowPublicKeyRetrieval=true&characterEncoding=UTF-8 persistence.domain[0].poolMaxActive=10 diff --git a/core/persistence-jpa-json/src/test/resources/core-ojson-test.properties b/core/persistence-jpa-json/src/test/resources/core-ojson-test.properties index bc5d7a97a1e..0565a4cdea9 100644 --- a/core/persistence-jpa-json/src/test/resources/core-ojson-test.properties +++ b/core/persistence-jpa-json/src/test/resources/core-ojson-test.properties @@ -18,7 +18,7 @@ security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=${secretKey} persistence.domain[0].jdbcURL=jdbc:oracle:thin:@${DB_CONTAINER_IP}:1521/XEPDB1 #persistence.domain[0].jdbcURL=jdbc:oracle:thin:@192.168.0.176:1521/orcl diff --git a/core/persistence-jpa-json/src/test/resources/core-pgjsonb-test.properties b/core/persistence-jpa-json/src/test/resources/core-pgjsonb-test.properties index a7a7b9594a3..4007496a9b7 100644 --- a/core/persistence-jpa-json/src/test/resources/core-pgjsonb-test.properties +++ b/core/persistence-jpa-json/src/test/resources/core-pgjsonb-test.properties @@ -18,7 +18,7 @@ security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=${secretKey} persistence.domain[0].jdbcURL=jdbc:postgresql://${DB_CONTAINER_IP}:5432/syncope?stringtype=unspecified persistence.domain[0].poolMaxActive=10 diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java index cccd14bc737..663e95c8a64 100644 --- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java +++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java @@ -61,8 +61,6 @@ public class JPALinkedAccount extends AbstractGeneratedKeyEntity implements Link public static final String TABLE = "LinkedAccount"; - private static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - @NotNull private String connObjectKeyValue; @@ -151,7 +149,7 @@ public void setCipherAlgorithm(final CipherAlgorithm cipherAlgorithm) { throw new IllegalArgumentException("Cannot override existing cipher algorithm"); } } - + @Override public boolean canDecodeSecrets() { return this.cipherAlgorithm != null && this.cipherAlgorithm.isInvertible(); @@ -168,14 +166,22 @@ public void setEncodedPassword(final String password, final CipherAlgorithm ciph this.cipherAlgorithm = cipherAlgoritm; } + protected String encode(final String value) throws Exception { + return Encryptor.getInstance().encode( + value, + Optional.ofNullable(cipherAlgorithm). + orElseGet(() -> CipherAlgorithm.valueOf( + ApplicationContextProvider.getBeanFactory().getBean(ConfParamOps.class).get( + AuthContextUtils.getDomain(), + "password.cipher.algorithm", + CipherAlgorithm.AES.name(), + String.class)))); + } + @Override public void setPassword(final String password) { try { - this.password = ENCRYPTOR.encode(password, cipherAlgorithm == null - ? CipherAlgorithm.valueOf(ApplicationContextProvider.getBeanFactory().getBean(ConfParamOps.class). - get(AuthContextUtils.getDomain(), "password.cipher.algorithm", CipherAlgorithm.AES.name(), - String.class)) - : cipherAlgorithm); + this.password = encode(password); } catch (Exception e) { LOG.error("Could not encode password", e); this.password = null; diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java index e4d2f7f8d0b..1efba4d64b7 100644 --- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java +++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java @@ -77,8 +77,6 @@ public class JPAUser public static final String TABLE = "SyncopeUser"; - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected static final TypeReference> TYPEREF = new TypeReference>() { }; @@ -223,14 +221,22 @@ public void setEncodedPassword(final String password, final CipherAlgorithm ciph setMustChangePassword(false); } + protected String encode(final String value) throws Exception { + return Encryptor.getInstance().encode( + value, + Optional.ofNullable(cipherAlgorithm). + orElseGet(() -> CipherAlgorithm.valueOf( + ApplicationContextProvider.getBeanFactory().getBean(ConfParamOps.class).get( + AuthContextUtils.getDomain(), + "password.cipher.algorithm", + CipherAlgorithm.AES.name(), + String.class)))); + } + @Override public void setPassword(final String password) { try { - this.password = ENCRYPTOR.encode(password, cipherAlgorithm == null - ? CipherAlgorithm.valueOf(ApplicationContextProvider.getBeanFactory().getBean(ConfParamOps.class). - get(AuthContextUtils.getDomain(), "password.cipher.algorithm", CipherAlgorithm.AES.name(), - String.class)) - : cipherAlgorithm); + this.password = encode(password); setMustChangePassword(false); } catch (Exception e) { LOG.error("Could not encode password", e); @@ -414,11 +420,7 @@ public String getSecurityAnswer() { @Override public void setSecurityAnswer(final String securityAnswer) { try { - this.securityAnswer = ENCRYPTOR.encode(securityAnswer, cipherAlgorithm == null - ? CipherAlgorithm.valueOf(ApplicationContextProvider.getBeanFactory().getBean(ConfParamOps.class). - get(AuthContextUtils.getDomain(), "password.cipher.algorithm", CipherAlgorithm.AES.name(), - String.class)) - : cipherAlgorithm); + this.securityAnswer = encode(securityAnswer); } catch (Exception e) { LOG.error("Could not encode security answer", e); this.securityAnswer = null; diff --git a/core/persistence-jpa/src/test/resources/core-test.properties b/core/persistence-jpa/src/test/resources/core-test.properties index bcd721e5842..b15b2440efb 100644 --- a/core/persistence-jpa/src/test/resources/core-test.properties +++ b/core/persistence-jpa/src/test/resources/core-test.properties @@ -18,7 +18,7 @@ security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=${secretKey} persistence.domain[0].key=Master persistence.domain[0].jdbcDriver=org.h2.Driver diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java index 87e4f5ffda9..a0c1a19a11e 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java @@ -110,8 +110,6 @@ public class DefaultMappingManager implements MappingManager { protected static final Logger LOG = LoggerFactory.getLogger(MappingManager.class); - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected final AnyTypeDAO anyTypeDAO; protected final UserDAO userDAO; @@ -500,7 +498,7 @@ public Pair> prepareAttrsFromRealm(final Realm realm, fin protected String decodePassword(final Account account) { try { - return ENCRYPTOR.decode(account.getPassword(), account.getCipherAlgorithm()); + return Encryptor.getInstance().decode(account.getPassword(), account.getCipherAlgorithm()); } catch (Exception e) { LOG.error("Could not decode password for {}", account, e); return null; diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyTypeDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyTypeDataBinderImpl.java index 54db91e11e0..72045e94fab 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyTypeDataBinderImpl.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyTypeDataBinderImpl.java @@ -49,8 +49,6 @@ public class AnyTypeDataBinderImpl implements AnyTypeDataBinder { protected static final Logger LOG = LoggerFactory.getLogger(AnyTypeDataBinder.class); - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected final SecurityProperties securityProperties; protected final AnyTypeDAO anyTypeDAO; @@ -86,15 +84,14 @@ public AnyType create(final AnyTypeTO anyTypeTO) { AccessToken accessToken = accessTokenDAO.findByOwner(AuthContextUtils.getUsername()); try { Set authorities = new HashSet<>(POJOHelper.deserialize( - ENCRYPTOR.decode(new String(accessToken.getAuthorities()), CipherAlgorithm.AES), + Encryptor.getInstance().decode(new String(accessToken.getAuthorities()), CipherAlgorithm.AES), new TypeReference>() { })); added.forEach(e -> authorities.add(new SyncopeGrantedAuthority(e, SyncopeConstants.ROOT_REALM))); - accessToken.setAuthorities(ENCRYPTOR.encode( - POJOHelper.serialize(authorities), CipherAlgorithm.AES). - getBytes()); + accessToken.setAuthorities(Encryptor.getInstance().encode( + POJOHelper.serialize(authorities), CipherAlgorithm.AES).getBytes()); accessTokenDAO.save(accessToken); } catch (Exception e) { @@ -142,16 +139,15 @@ public AnyTypeTO delete(final AnyType anyType) { AccessToken accessToken = accessTokenDAO.findByOwner(AuthContextUtils.getUsername()); try { Set authorities = new HashSet<>(POJOHelper.deserialize( - ENCRYPTOR.decode(new String(accessToken.getAuthorities()), CipherAlgorithm.AES), + Encryptor.getInstance().decode(new String(accessToken.getAuthorities()), CipherAlgorithm.AES), new TypeReference>() { })); authorities.removeAll(authorities.stream(). filter(authority -> removed.contains(authority.getAuthority())).collect(Collectors.toList())); - accessToken.setAuthorities(ENCRYPTOR.encode( - POJOHelper.serialize(authorities), CipherAlgorithm.AES). - getBytes()); + accessToken.setAuthorities(Encryptor.getInstance().encode( + POJOHelper.serialize(authorities), CipherAlgorithm.AES).getBytes()); accessTokenDAO.save(accessToken); } catch (Exception e) { diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java index 9390daafa37..f537ebc78ef 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java @@ -68,8 +68,6 @@ public class ConnObjectUtils { protected static final Logger LOG = LoggerFactory.getLogger(ConnObjectUtils.class); - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - public static SyncToken toSyncToken(final String syncToken) { return Optional.ofNullable(syncToken).map(st -> POJOHelper.deserialize(st, SyncToken.class)).orElse(null); } @@ -266,7 +264,7 @@ public U getAnyUR( // update password if and only if password is really changed User user = userDAO.authFind(key); if (StringUtils.isBlank(updatedUser.getPassword()) - || ENCRYPTOR.verify(updatedUser.getPassword(), + || Encryptor.getInstance().verify(updatedUser.getPassword(), user.getCipherAlgorithm(), user.getPassword())) { updatedUser.setPassword(null); diff --git a/core/self-keymaster-starter/src/test/resources/core-debug.properties b/core/self-keymaster-starter/src/test/resources/core-debug.properties index 6876fce5301..7425b3bb200 100644 --- a/core/self-keymaster-starter/src/test/resources/core-debug.properties +++ b/core/self-keymaster-starter/src/test/resources/core-debug.properties @@ -33,7 +33,7 @@ spring.h2.console.path=/h2 security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=${secretKey} persistence.domain[0].key=Master persistence.domain[0].jdbcDriver=org.h2.Driver diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java index e9a63c2af73..224b2f46b4e 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java @@ -59,8 +59,6 @@ public class DefaultPasswordRule implements PasswordRule { protected static final Logger LOG = LoggerFactory.getLogger(DefaultPasswordRule.class); - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - public static List conf2Rules(final DefaultPasswordRuleConf conf) { List rules = new ArrayList<>(); @@ -205,7 +203,7 @@ public void enforce(final LinkedAccount account) { String clear = null; if (account.canDecodeSecrets()) { try { - clear = ENCRYPTOR.decode(account.getPassword(), account.getCipherAlgorithm()); + clear = Encryptor.getInstance().decode(account.getPassword(), account.getCipherAlgorithm()); } catch (Exception e) { LOG.error("Could not decode password for {}", account, e); } diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java index 11b71dec4bd..b5c62d1c9c8 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java @@ -18,15 +18,10 @@ */ package org.apache.syncope.core.spring.policy; -import java.io.UnsupportedEncodingException; import java.net.URI; -import java.security.InvalidKeyException; -import java.security.NoSuchAlgorithmException; +import java.security.GeneralSecurityException; import java.util.Optional; import java.util.stream.Stream; -import javax.crypto.BadPaddingException; -import javax.crypto.IllegalBlockSizeException; -import javax.crypto.NoSuchPaddingException; import org.apache.commons.lang3.StringUtils; import org.apache.syncope.common.lib.policy.HaveIBeenPwnedPasswordRuleConf; import org.apache.syncope.common.lib.policy.PasswordRuleConf; @@ -51,8 +46,6 @@ public class HaveIBeenPwnedPasswordRule implements PasswordRule { protected static final Logger LOG = LoggerFactory.getLogger(HaveIBeenPwnedPasswordRule.class); - private static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - private HaveIBeenPwnedPasswordRuleConf conf; @Override @@ -72,7 +65,7 @@ public void setConf(final PasswordRuleConf conf) { protected void enforce(final String clearPassword) { try { - String sha1 = ENCRYPTOR.encode(clearPassword, CipherAlgorithm.SHA1); + String sha1 = Encryptor.getInstance().encode(clearPassword, CipherAlgorithm.SHA1); HttpHeaders headers = new HttpHeaders(); headers.set(HttpHeaders.USER_AGENT, "Apache Syncope"); @@ -88,9 +81,7 @@ protected void enforce(final String clearPassword) { throw new PasswordPolicyException("Password pwned"); } } - } catch (UnsupportedEncodingException | InvalidKeyException | NoSuchAlgorithmException - | BadPaddingException | IllegalBlockSizeException | NoSuchPaddingException e) { - + } catch (GeneralSecurityException e) { LOG.error("Could not encode the password value as SHA1", e); } catch (HttpStatusCodeException e) { LOG.error("Error while contacting the PwnedPasswords service", e); @@ -115,7 +106,7 @@ public void enforce(final LinkedAccount account) { String clearPassword = null; if (account.canDecodeSecrets()) { try { - clearPassword = ENCRYPTOR.decode(account.getPassword(), account.getCipherAlgorithm()); + clearPassword = Encryptor.getInstance().decode(account.getPassword(), account.getCipherAlgorithm()); } catch (Exception e) { LOG.error("Could not decode password for {}", account, e); } diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java index 21b61493231..d53ffa1f71a 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java @@ -83,8 +83,6 @@ public class AuthDataAccessor { public static final String GROUP_OWNER_ROLE = "GROUP_OWNER"; - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected static final Set ANONYMOUS_AUTHORITIES = Set.of(new SyncopeGrantedAuthority(IdRepoEntitlement.ANONYMOUS)); @@ -255,7 +253,7 @@ public Triple authenticate(final String domain, final Aut } protected boolean authenticate(final User user, final String password) { - boolean authenticated = ENCRYPTOR.verify(password, user.getCipherAlgorithm(), user.getPassword()); + boolean authenticated = Encryptor.getInstance().verify(password, user.getCipherAlgorithm(), user.getPassword()); LOG.debug("{} authenticated on internal storage: {}", user.getUsername(), authenticated); for (Iterator itor = getPassthroughResources(user).iterator(); diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/Encryptor.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/Encryptor.java index 52db5fbf8f1..8c411c265ba 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/Encryptor.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/Encryptor.java @@ -18,19 +18,18 @@ */ package org.apache.syncope.core.spring.security; -import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.util.Base64; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.NoSuchPaddingException; import javax.crypto.spec.SecretKeySpec; -import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; import org.apache.syncope.common.lib.types.CipherAlgorithm; import org.apache.syncope.core.spring.ApplicationContextProvider; @@ -46,53 +45,85 @@ public final class Encryptor { private static final Map INSTANCES = new ConcurrentHashMap<>(); - private static final String DEFAULT_SECRET_KEY = "1abcdefghilmnopqrstuvz2!"; - public static Encryptor getInstance() { return getInstance(null); } - public static Encryptor getInstance(final String secretKey) { - String actualKey = StringUtils.isBlank(secretKey) ? DEFAULT_SECRET_KEY : secretKey; - - Encryptor instance = INSTANCES.get(actualKey); - if (instance == null) { - instance = new Encryptor(actualKey); - INSTANCES.put(actualKey, instance); - } - - return instance; + public static Encryptor getInstance(final String aesSecretKey) { + SecurityProperties securityProperties = Optional.ofNullable(ApplicationContextProvider.getApplicationContext()). + flatMap(ctx -> { + try { + return Optional.ofNullable(ctx.getBean(SecurityProperties.class)); + } catch (Exception e) { + return Optional.empty(); + } + }). + orElseGet(() -> { + SecurityProperties props = new SecurityProperties(); + props.setAesSecretKey(StringUtils.EMPTY); + return props; + }); + + String actualKey = StringUtils.isBlank(aesSecretKey) ? securityProperties.getAesSecretKey() : aesSecretKey; + return INSTANCES.computeIfAbsent(actualKey, k -> new Encryptor(k, securityProperties.getDigester())); } + private final SecurityProperties.DigesterProperties digesterProperties; + private final Map digesters = new ConcurrentHashMap<>(); - private SecretKeySpec keySpec; + private final Optional aesKeySpec; - private Encryptor(final String secretKey) { - String actualKey = secretKey; - if (actualKey.length() < 16) { - StringBuilder actualKeyPadding = new StringBuilder(actualKey); - int length = 16 - actualKey.length(); - String randomChars = SecureRandomUtils.generateRandomPassword(length); + private Encryptor( + final String aesSecretKey, + final SecurityProperties.DigesterProperties digesterProperties) { - actualKeyPadding.append(randomChars); - actualKey = actualKeyPadding.toString(); - LOG.warn("The secret key is too short (< 16), adding some random characters. " - + "Passwords encrypted with AES and this key will not be recoverable " - + "as a result if the container is restarted."); - } + this.digesterProperties = digesterProperties; - try { - keySpec = new SecretKeySpec(ArrayUtils.subarray( - actualKey.getBytes(StandardCharsets.UTF_8), 0, 16), - CipherAlgorithm.AES.getAlgorithm()); - } catch (Exception e) { - LOG.error("Error during key specification", e); + SecretKeySpec sks = null; + + if (StringUtils.isNotBlank(aesSecretKey)) { + String actualKey = aesSecretKey; + + Integer pad = null; + boolean truncate = false; + if (actualKey.length() < 16) { + pad = 16 - actualKey.length(); + } else if (actualKey.length() > 16 && actualKey.length() < 24) { + pad = 24 - actualKey.length(); + } else if (actualKey.length() > 24 && actualKey.length() < 32) { + pad = 32 - actualKey.length(); + } else if (actualKey.length() > 32) { + truncate = true; + } + + if (pad != null) { + StringBuilder actualKeyPadding = new StringBuilder(actualKey); + String randomChars = SecureRandomUtils.generateRandomPassword(pad); + + actualKeyPadding.append(randomChars); + actualKey = actualKeyPadding.toString(); + LOG.warn("The configured AES secret key is too short (< {}), padding with random chars: {}", + actualKey.length(), actualKey); + } + if (truncate) { + actualKey = actualKey.substring(0, 32); + LOG.warn("The configured AES secret key is too long (> 32), truncating: {}", actualKey); + } + + try { + sks = new SecretKeySpec(actualKey.getBytes(StandardCharsets.UTF_8), CipherAlgorithm.AES.getAlgorithm()); + LOG.debug("AES-{} successfully configured", actualKey.length() * 8); + } catch (Exception e) { + LOG.error("Error during key specification", e); + } } + + aesKeySpec = Optional.ofNullable(sks); } public String encode(final String value, final CipherAlgorithm cipherAlgorithm) - throws UnsupportedEncodingException, NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, + throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, IllegalBlockSizeException, BadPaddingException { String encoded = null; @@ -100,7 +131,8 @@ public String encode(final String value, final CipherAlgorithm cipherAlgorithm) if (value != null) { if (cipherAlgorithm == null || cipherAlgorithm == CipherAlgorithm.AES) { Cipher cipher = Cipher.getInstance(CipherAlgorithm.AES.getAlgorithm()); - cipher.init(Cipher.ENCRYPT_MODE, keySpec); + cipher.init(Cipher.ENCRYPT_MODE, aesKeySpec. + orElseThrow(() -> new IllegalArgumentException("AES not configured"))); encoded = Base64.getEncoder().encodeToString(cipher.doFinal(value.getBytes(StandardCharsets.UTF_8))); } else if (cipherAlgorithm == CipherAlgorithm.BCRYPT) { @@ -134,14 +166,15 @@ public boolean verify(final String value, final CipherAlgorithm cipherAlgorithm, } public String decode(final String encoded, final CipherAlgorithm cipherAlgorithm) - throws UnsupportedEncodingException, NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, + throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, IllegalBlockSizeException, BadPaddingException { String decoded = null; if (encoded != null && cipherAlgorithm == CipherAlgorithm.AES) { Cipher cipher = Cipher.getInstance(CipherAlgorithm.AES.getAlgorithm()); - cipher.init(Cipher.DECRYPT_MODE, keySpec); + cipher.init(Cipher.DECRYPT_MODE, aesKeySpec. + orElseThrow(() -> new IllegalArgumentException("AES not configured"))); decoded = new String(cipher.doFinal(Base64.getDecoder().decode(encoded)), StandardCharsets.UTF_8); } @@ -150,24 +183,19 @@ public String decode(final String encoded, final CipherAlgorithm cipherAlgorithm } private StandardStringDigester getDigester(final CipherAlgorithm cipherAlgorithm) { - StandardStringDigester digester = digesters.get(cipherAlgorithm); - if (digester == null) { - digester = new StandardStringDigester(); + return digesters.computeIfAbsent(cipherAlgorithm, k -> { + StandardStringDigester digester = new StandardStringDigester(); if (cipherAlgorithm.getAlgorithm().startsWith("S-")) { - SecurityProperties securityProperties = - ApplicationContextProvider.getApplicationContext().getBean(SecurityProperties.class); - // Salted ... digester.setAlgorithm(cipherAlgorithm.getAlgorithm().replaceFirst("S\\-", "")); - digester.setIterations(securityProperties.getDigester().getSaltIterations()); - digester.setSaltSizeBytes(securityProperties.getDigester().getSaltSizeBytes()); + digester.setIterations(digesterProperties.getSaltIterations()); + digester.setSaltSizeBytes(digesterProperties.getSaltSizeBytes()); digester.setInvertPositionOfPlainSaltInEncryptionResults( - securityProperties.getDigester().isInvertPositionOfPlainSaltInEncryptionResults()); + digesterProperties.isInvertPositionOfPlainSaltInEncryptionResults()); digester.setInvertPositionOfSaltInMessageBeforeDigesting( - securityProperties.getDigester().isInvertPositionOfSaltInMessageBeforeDigesting()); - digester.setUseLenientSaltSizeCheck( - securityProperties.getDigester().isUseLenientSaltSizeCheck()); + digesterProperties.isInvertPositionOfSaltInMessageBeforeDigesting()); + digester.setUseLenientSaltSizeCheck(digesterProperties.isUseLenientSaltSizeCheck()); } else { // Not salted ... digester.setAlgorithm(cipherAlgorithm.getAlgorithm()); @@ -176,10 +204,7 @@ private StandardStringDigester getDigester(final CipherAlgorithm cipherAlgorithm } digester.setStringOutputType(CommonUtils.STRING_OUTPUT_TYPE_HEXADECIMAL); - - digesters.put(cipherAlgorithm, digester); - } - - return digester; + return digester; + }); } } diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityContext.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityContext.java index ece1679733f..a1ccb95e637 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityContext.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityContext.java @@ -27,6 +27,7 @@ import java.io.Reader; import java.security.NoSuchAlgorithmException; import java.security.spec.InvalidKeySpecException; +import java.util.Optional; import org.apache.syncope.common.lib.types.CipherAlgorithm; import org.apache.syncope.core.persistence.api.dao.AccessTokenDAO; import org.apache.syncope.core.persistence.api.dao.RealmDAO; @@ -63,10 +64,8 @@ public static GrantedAuthorityDefaults grantedAuthorityDefaults() { } protected static String jwsKey(final JWSAlgorithm jwsAlgorithm, final SecurityProperties props) { - String jwsKey = props.getJwsKey(); - if (jwsKey == null) { - throw new IllegalArgumentException("No JWS key provided"); - } + String jwsKey = Optional.ofNullable(props.getJwsKey()). + orElseThrow(() -> new IllegalArgumentException("No JWS key provided")); if (JWSAlgorithm.Family.HMAC_SHA.contains(jwsAlgorithm)) { int minLength = jwsAlgorithm.equals(JWSAlgorithm.HS256) diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityProperties.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityProperties.java index c5d9690117d..002c2fa3b54 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityProperties.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/SecurityProperties.java @@ -98,7 +98,7 @@ public void setUseLenientSaltSizeCheck(final boolean useLenientSaltSizeCheck) { private String jwsAlgorithm = JWSAlgorithm.HS512.getName(); - private String secretKey; + private String aesSecretKey; private String groovyBlacklist = "classpath:META-INF/groovy.blacklist"; @@ -168,12 +168,12 @@ public void setJwsAlgorithm(final String jwsAlgorithm) { this.jwsAlgorithm = jwsAlgorithm; } - public String getSecretKey() { - return secretKey; + public String getAesSecretKey() { + return aesSecretKey; } - public void setSecretKey(final String secretKey) { - this.secretKey = secretKey; + public void setAesSecretKey(final String aesSecretKey) { + this.aesSecretKey = aesSecretKey; } public String getGroovyBlacklist() { diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/SyncopeJWTSSOProvider.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/SyncopeJWTSSOProvider.java index c77123bccf6..65b3e064a9a 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/SyncopeJWTSSOProvider.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/SyncopeJWTSSOProvider.java @@ -47,8 +47,6 @@ public class SyncopeJWTSSOProvider implements JWTSSOProvider { protected static final Logger LOG = LoggerFactory.getLogger(SyncopeJWTSSOProvider.class); - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected final SecurityProperties securityProperties; protected final AccessTokenJWSVerifier delegate; @@ -104,7 +102,7 @@ public Pair> resolve(final JWTClaimsSet jwtCl if (accessToken.getAuthorities() != null) { try { authorities = POJOHelper.deserialize( - ENCRYPTOR.decode(new String(accessToken.getAuthorities()), CipherAlgorithm.AES), + Encryptor.getInstance().decode(new String(accessToken.getAuthorities()), CipherAlgorithm.AES), new TypeReference<>() { }); } catch (Throwable t) { diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/UsernamePasswordAuthenticationProvider.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/UsernamePasswordAuthenticationProvider.java index d6e0acb80fe..53f51a659bf 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/UsernamePasswordAuthenticationProvider.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/UsernamePasswordAuthenticationProvider.java @@ -42,8 +42,6 @@ public class UsernamePasswordAuthenticationProvider implements AuthenticationPro protected static final Logger LOG = LoggerFactory.getLogger(UsernamePasswordAuthenticationProvider.class); - protected static final Encryptor ENCRYPTOR = Encryptor.getInstance(); - protected final DomainOps domainOps; protected final AuthDataAccessor dataAccessor; @@ -97,12 +95,12 @@ public Authentication authenticate(final Authentication authentication) { username.set(securityProperties.getAdminUser()); if (SyncopeConstants.MASTER_DOMAIN.equals(domain.getKey())) { credentialChecker.checkIsDefaultAdminPasswordInUse(); - authenticated = ENCRYPTOR.verify( + authenticated = Encryptor.getInstance().verify( authentication.getCredentials().toString(), securityProperties.getAdminPasswordAlgorithm(), securityProperties.getAdminPassword()); } else { - authenticated = ENCRYPTOR.verify( + authenticated = Encryptor.getInstance().verify( authentication.getCredentials().toString(), domain.getAdminCipherAlgorithm(), domain.getAdminPassword()); diff --git a/core/spring/src/test/java/org/apache/syncope/core/spring/security/EncryptorTest.java b/core/spring/src/test/java/org/apache/syncope/core/spring/security/EncryptorTest.java index 8f4cf5b75a0..eed813e89fb 100644 --- a/core/spring/src/test/java/org/apache/syncope/core/spring/security/EncryptorTest.java +++ b/core/spring/src/test/java/org/apache/syncope/core/spring/security/EncryptorTest.java @@ -36,7 +36,9 @@ public class EncryptorTest { @BeforeAll public static void setUp() { - ApplicationContextProvider.getBeanFactory().registerSingleton("securityProperties", new SecurityProperties()); + SecurityProperties props = new SecurityProperties(); + props.setAesSecretKey("1abcdefghilmnopq"); + ApplicationContextProvider.getBeanFactory().registerSingleton("securityProperties", props); ENCRYPTOR = Encryptor.getInstance(); } diff --git a/core/starter/src/main/resources/core.properties b/core/starter/src/main/resources/core.properties index 933aed41812..cb962828a59 100644 --- a/core/starter/src/main/resources/core.properties +++ b/core/starter/src/main/resources/core.properties @@ -96,7 +96,14 @@ security.jwtIssuer=ApacheSyncope security.jwsAlgorithm=HS512 security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +# Key length drives AES algorithm variant selection: +# +# * 16 chars => AES-128 +# * 24 chars => AES-192 +# * 32 chars => AES-256 +# +# Shorter keys will be padded to the nearest longer option available; keys > 32 will be trucated +security.aesSecretKey=${secretKey} # default for LDAP / RFC2307 SSHA security.digester.saltIterations=1 diff --git a/fit/core-reference/src/main/resources/core-embedded.properties b/fit/core-reference/src/main/resources/core-embedded.properties index 778a6d34787..5e57aa93e62 100644 --- a/fit/core-reference/src/main/resources/core-embedded.properties +++ b/fit/core-reference/src/main/resources/core-embedded.properties @@ -40,7 +40,7 @@ spring.datasource.driver-class-name=org.h2.Driver security.adminUser=${adminUser} security.anonymousUser=${anonymousUser} security.jwsKey=${jwsKey} -security.secretKey=${secretKey} +security.aesSecretKey=NyefOIpekEJVBASbbETMbcns11HouPNn persistence.domain[0].key=Master persistence.domain[0].jdbcDriver=org.h2.Driver diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/KeymasterITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/KeymasterITCase.java index 697fba24a96..cc288b62e68 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/KeymasterITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/KeymasterITCase.java @@ -330,8 +330,8 @@ public void domainUpdateAdminPassword() throws Exception { // 1. change admin pwd for domain Two domainOps.changeAdminPassword( two.getKey(), - Encryptor.getInstance().encode("password3", CipherAlgorithm.AES), - CipherAlgorithm.AES); + Encryptor.getInstance().encode("password3", CipherAlgorithm.BCRYPT), + CipherAlgorithm.BCRYPT); // 2. attempt to access with old pwd -> fail try { diff --git a/pom.xml b/pom.xml index c889503d10f..e1c61d30610 100644 --- a/pom.xml +++ b/pom.xml @@ -1710,7 +1710,7 @@ under the License. org.apache.maven.plugins maven-jar-plugin - 3.4.2 + 3.5.0 diff --git a/src/main/asciidoc/reference-guide/configuration/configurationparameters.adoc b/src/main/asciidoc/reference-guide/configuration/configurationparameters.adoc index 474a17655ad..1f7d15b623b 100644 --- a/src/main/asciidoc/reference-guide/configuration/configurationparameters.adoc +++ b/src/main/asciidoc/reference-guide/configuration/configurationparameters.adoc @@ -24,13 +24,16 @@ Most run-time configuration options are available as parameters and can be tuned * `password.cipher.algorithm` - which cipher algorithm shall be used for encrypting password values; supported algorithms include `SHA-1`, `SHA-256`, `SHA-512`, `AES`, `S-MD5`, `S-SHA-1`, `S-SHA-256`, `S-SHA-512` and `BCRYPT`; salting options are available in the `core.properties` file; +* `security.aesSecretKey` - used for AES-based encryption / decryption: besides password values, this is also used +whenever reversible encryption is needed, throughout the whole system; [WARNING] -The value of the `security.secretKey` property in the `core.properties` file is used for AES-based encryption / decryption. -Besides password values, this is also used whenever reversible encryption is needed, throughout the whole system. + -When the `secretKey` value has length less than 16, it is right-padded by random characters during startup, to reach -such mininum value. + -It is *strongly* recommended to provide a value long at least 16 characters, in order to avoid unexpected behaviors -at runtime, expecially with high-availability. +The actual length of the `security.aesSecretKey` value is used to drive the AES algorithm variant selection: +16 characters implies `AES-128`, 24 selects `AES-192` and 32 configures `AES-256`. + +When the `security.aesSecretKey` value has length less than 16, between 17 and 23 or between 25 and 31, it is +right-padded by random characters during startup, to reach the nearest option. If the specified value is instead longer +than 32 characters, it is truncated to 32. + +It is *strongly* recommended to provide a value long exactly 16, 24 or 32 characters, in order to avoid unexpected +behaviors at runtime, expecially with high-availability. * `jwt.lifetime.minutes` - validity of https://en.wikipedia.org/wiki/JSON_Web_Token[JSON Web Token^] values used for <> (in minutes); * `notificationjob.cronExpression` -