Skip to content

Commit f050b70

Browse files
committed
[SYNCOPE-1996] Security production mode
1 parent a910f08 commit f050b70

28 files changed

Lines changed: 343 additions & 118 deletions

File tree

‎core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/PlainAttrValue.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public class PlainAttrValue implements Serializable {
4545

4646
protected static final Logger LOG = LoggerFactory.getLogger(PlainAttrValue.class);
4747

48-
private static final Pattern SPRING_ENV_PROPERTY = Pattern.compile("^\\$\\{.*\\}$");
48+
public static final Pattern SPRING_ENV_PROPERTY = Pattern.compile("^\\$\\{.*\\}$");
4949

5050
@JsonIgnore
5151
@NotNull

‎core/persistence-common/src/main/java/org/apache/syncope/core/persistence/common/validation/PlainSchemaValidator.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@
1818
*/
1919
package org.apache.syncope.core.persistence.common.validation;
2020

21+
import static org.apache.syncope.core.persistence.api.entity.PlainAttrValue.SPRING_ENV_PROPERTY;
22+
2123
import jakarta.validation.ConstraintValidatorContext;
24+
import java.util.Optional;
25+
import org.apache.syncope.common.lib.SyncopeConstants;
26+
import org.apache.syncope.common.lib.types.CipherAlgorithm;
2227
import org.apache.syncope.common.lib.types.EntityViolationType;
2328
import org.apache.syncope.core.persistence.api.entity.PlainSchema;
2429

@@ -55,6 +60,20 @@ public boolean isValid(final PlainSchema schema, final ConstraintValidatorContex
5560
addPropertyNode("secretKey").addPropertyNode("cipherAlgorithm").addConstraintViolation();
5661
return false;
5762
}
63+
if (schema.getCipherAlgorithm() == CipherAlgorithm.AES
64+
&& !SyncopeConstants.ENCRYPTED_DECODE_CONVERSION_PATTERN.equals(schema.getConversionPattern())
65+
&& !SPRING_ENV_PROPERTY.matcher(schema.getSecretKey()).matches()
66+
&& Optional.ofNullable(schema.getSecretKey()).
67+
map(key -> key.length() != 16 && key.length() != 24 && key.length() != 32).
68+
orElse(true)) {
69+
70+
context.disableDefaultConstraintViolation();
71+
context.buildConstraintViolationWithTemplate(
72+
getTemplate(EntityViolationType.InvalidSchema,
73+
"SecretKey length requirements not met for AES: must be 16 24 or 32")).
74+
addPropertyNode("secretKey").addConstraintViolation();
75+
return false;
76+
}
5877
}
5978

6079
default -> {

‎core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/PersistenceTestContext.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.syncope.core.persistence.jpa.spring.DomainRoutingEntityManagerFactory;
3434
import org.apache.syncope.core.provisioning.api.ConnectorManager;
3535
import org.apache.syncope.core.provisioning.api.ImplementationLookup;
36+
import org.apache.syncope.core.spring.security.DefaultCredentialChecker;
3637
import org.apache.syncope.core.spring.security.DefaultEncryptorManager;
3738
import org.apache.syncope.core.spring.security.DefaultPasswordGenerator;
3839
import org.apache.syncope.core.spring.security.PasswordGenerator;
@@ -118,7 +119,8 @@ public ConnectorManager connectorManager() {
118119
public EncryptorManager encryptorManager() {
119120
SecurityProperties securityProperties = new SecurityProperties();
120121
securityProperties.setAesSecretKey(StringUtils.EMPTY);
121-
return new DefaultEncryptorManager(securityProperties);
122+
securityProperties.setProductionMode(false);
123+
return new DefaultEncryptorManager(new DefaultCredentialChecker("", "", "", "", false), securityProperties);
122124
}
123125

124126
@Bean

‎core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/PlainAttrTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ public void encryptedWithDecodeConversionPattern() throws Exception {
167167
obscureWithDecodeConversionPattern.setAnyTypeClass(anyTypeClassDAO.findById("other").orElseThrow());
168168
obscureWithDecodeConversionPattern.setType(AttrSchemaType.Encrypted);
169169
obscureWithDecodeConversionPattern.setCipherAlgorithm(CipherAlgorithm.AES);
170-
obscureWithDecodeConversionPattern.setSecretKey(SecureRandomUtils.generateRandomUUID().toString());
170+
obscureWithDecodeConversionPattern.setSecretKey(SecureRandomUtils.generateRandomPassword(16));
171171

172172
obscureWithDecodeConversionPattern = plainSchemaDAO.save(obscureWithDecodeConversionPattern);
173173

‎core/persistence-neo4j/src/test/java/org/apache/syncope/core/persistence/neo4j/PersistenceTestContext.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.syncope.core.persistence.neo4j.spring.DomainRoutingDriver;
3333
import org.apache.syncope.core.provisioning.api.ConnectorManager;
3434
import org.apache.syncope.core.provisioning.api.ImplementationLookup;
35+
import org.apache.syncope.core.spring.security.DefaultCredentialChecker;
3536
import org.apache.syncope.core.spring.security.DefaultEncryptorManager;
3637
import org.apache.syncope.core.spring.security.DefaultPasswordGenerator;
3738
import org.apache.syncope.core.spring.security.PasswordGenerator;
@@ -110,7 +111,8 @@ public ConnectorManager connectorManager() {
110111
public EncryptorManager encryptorManager() {
111112
SecurityProperties securityProperties = new SecurityProperties();
112113
securityProperties.setAesSecretKey(StringUtils.EMPTY);
113-
return new DefaultEncryptorManager(securityProperties);
114+
securityProperties.setProductionMode(false);
115+
return new DefaultEncryptorManager(new DefaultCredentialChecker("", "", "", "", false), securityProperties);
114116
}
115117

116118
@Bean

‎core/persistence-neo4j/src/test/java/org/apache/syncope/core/persistence/neo4j/inner/PlainAttrTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public void encryptedWithDecodeConversionPattern() throws Exception {
180180
obscureWithDecodeConversionPattern.setAnyTypeClass(anyTypeClassDAO.findById("other").orElseThrow());
181181
obscureWithDecodeConversionPattern.setType(AttrSchemaType.Encrypted);
182182
obscureWithDecodeConversionPattern.setCipherAlgorithm(CipherAlgorithm.AES);
183-
obscureWithDecodeConversionPattern.setSecretKey(SecureRandomUtils.generateRandomUUID().toString());
183+
obscureWithDecodeConversionPattern.setSecretKey(SecureRandomUtils.generateRandomPassword(16));
184184

185185
obscureWithDecodeConversionPattern = plainSchemaDAO.save(obscureWithDecodeConversionPattern);
186186

‎core/provisioning-java/src/test/resources/core-test.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
security.productionMode=false
1819
security.adminUser=${adminUser}
1920
security.anonymousUser=${anonymousUser}
2021
security.jwsKey=${jwsKey}

‎core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultCredentialChecker.java‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,43 +28,88 @@ public class DefaultCredentialChecker {
2828

2929
private static final Logger LOG = LoggerFactory.getLogger(DefaultCredentialChecker.class);
3030

31+
private static final String DEFAULT_AES_KEY_ERROR_MESSAGE =
32+
"The default AES key property is being used. "
33+
+ "This must be changed to avoid a security breach!";
34+
35+
private static final String DEFAULT_AES_KEY = "1abcdefghilmnopqrstuvz2!";
36+
37+
private static final String DEFAULT_JWS_KEY_ERROR_MESSAGE =
38+
"The default JWKS key property is being used. "
39+
+ "This must be changed to avoid a security breach!";
40+
3141
private static final String DEFAULT_JWS_KEY = "ZW7pRixehFuNUtnY5Se47IemgMryTzazPPJ9CGX5LTCmsOJpOgHAQEuPQeV9A28f";
3242

43+
private static final String DEFAULT_ADMIN_PASSWORD_ERROR_MESSAGE =
44+
"The default adminPassword property is being used. "
45+
+ "This must be changed to avoid a security breach!";
46+
3347
private static final String DEFAULT_ADMIN_PASSWORD =
34-
"DE088591C00CC98B36F5ADAAF7DA2B004CF7F2FE7BBB45B766B6409876E2F3DB13C7905C6AA59464";
48+
"DE088591C00CC98B36F5ADAAF7DA2B004CF7F2FE7BBB45B766B6409876E2F3DB13C7905C6AA59464";
49+
50+
private static final String DEFAULT_ANON_KEY_ERROR_MESSAGE =
51+
"The default anonymousKey property is being used. "
52+
+ "This must be changed to avoid a security breach!";
3553

3654
private static final String DEFAULT_ANON_KEY = "anonymousKey";
3755

38-
private final boolean defaultAdminPasswordInUse;
56+
private final boolean defaultAesKeyInUse;
3957

4058
private final boolean defaultJwsKeyInUse;
4159

60+
private final boolean defaultAdminPasswordInUse;
61+
4262
private final boolean defaultAnonymousKeyInUse;
4363

44-
public DefaultCredentialChecker(final String jwsKey, final String adminPassword, final String anonymousKey) {
64+
private final boolean productionMode;
65+
66+
public DefaultCredentialChecker(
67+
final String aesKey,
68+
final String jwsKey,
69+
final String adminPassword,
70+
final String anonymousKey,
71+
final boolean productionMode) {
72+
73+
defaultAesKeyInUse = DEFAULT_AES_KEY.equals(aesKey);
4574
defaultJwsKeyInUse = DEFAULT_JWS_KEY.equals(jwsKey);
4675
defaultAdminPasswordInUse = DEFAULT_ADMIN_PASSWORD.equals(adminPassword);
4776
defaultAnonymousKeyInUse = DEFAULT_ANON_KEY.equals(anonymousKey);
77+
this.productionMode = productionMode;
78+
}
79+
80+
public void checkIsDefaultAESKeyInUse() {
81+
if (defaultAesKeyInUse) {
82+
if (productionMode) {
83+
throw new IllegalStateException(DEFAULT_AES_KEY_ERROR_MESSAGE);
84+
}
85+
LOG.warn(DEFAULT_AES_KEY_ERROR_MESSAGE);
86+
}
4887
}
4988

5089
public void checkIsDefaultJWSKeyInUse() {
5190
if (defaultJwsKeyInUse) {
52-
LOG.warn("The default jwsKey property is being used. "
53-
+ "This must be changed to avoid a security breach!");
91+
if (productionMode) {
92+
throw new IllegalStateException(DEFAULT_JWS_KEY_ERROR_MESSAGE);
93+
}
94+
LOG.warn(DEFAULT_JWS_KEY_ERROR_MESSAGE);
5495
}
5596
}
5697

5798
public void checkIsDefaultAdminPasswordInUse() {
5899
if (defaultAdminPasswordInUse) {
59-
LOG.warn("The default adminPassword property is being used. "
60-
+ "This must be changed to avoid a security breach!");
100+
if (productionMode) {
101+
throw new IllegalStateException(DEFAULT_ADMIN_PASSWORD_ERROR_MESSAGE);
102+
}
103+
LOG.warn(DEFAULT_ADMIN_PASSWORD_ERROR_MESSAGE);
61104
}
62105
}
63106

64107
public void checkIsDefaultAnonymousKeyInUse() {
65108
if (defaultAnonymousKeyInUse) {
66-
LOG.warn("The default anonymousKey property is being used. "
67-
+ "This must be changed to avoid a security breach!");
109+
if (productionMode) {
110+
throw new IllegalStateException(DEFAULT_ANON_KEY_ERROR_MESSAGE);
111+
}
112+
LOG.warn(DEFAULT_ANON_KEY_ERROR_MESSAGE);
68113
}
69114
}
70115
}

‎core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultEncryptor.java‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public class DefaultEncryptor implements Encryptor {
5151

5252
protected DefaultEncryptor(
5353
final String aesSecretKey,
54+
final boolean productionMode,
5455
final SecurityProperties.DigesterProperties digesterProperties) {
5556

5657
this.digesterProperties = digesterProperties;
@@ -60,30 +61,34 @@ protected DefaultEncryptor(
6061
if (StringUtils.isNotBlank(aesSecretKey)) {
6162
String actualKey = aesSecretKey;
6263

63-
Integer pad = null;
64-
boolean truncate = false;
65-
if (actualKey.length() < 16) {
66-
pad = 16 - actualKey.length();
67-
} else if (actualKey.length() > 16 && actualKey.length() < 24) {
68-
pad = 24 - actualKey.length();
69-
} else if (actualKey.length() > 24 && actualKey.length() < 32) {
70-
pad = 32 - actualKey.length();
71-
} else if (actualKey.length() > 32) {
72-
truncate = true;
73-
}
64+
if (!productionMode) {
65+
Integer pad = null;
66+
boolean truncate = false;
67+
if (actualKey.length() < 16) {
68+
pad = 16 - actualKey.length();
69+
} else if (actualKey.length() > 16 && actualKey.length() < 24) {
70+
pad = 24 - actualKey.length();
71+
} else if (actualKey.length() > 24 && actualKey.length() < 32) {
72+
pad = 32 - actualKey.length();
73+
} else if (actualKey.length() > 32) {
74+
truncate = true;
75+
}
7476

75-
if (pad != null) {
76-
StringBuilder actualKeyPadding = new StringBuilder(actualKey);
77-
String randomChars = SecureRandomUtils.generateRandomPassword(pad);
77+
if (pad != null) {
78+
StringBuilder actualKeyPadding = new StringBuilder(actualKey);
79+
String randomChars = SecureRandomUtils.generateRandomPassword(pad);
7880

79-
actualKeyPadding.append(randomChars);
80-
actualKey = actualKeyPadding.toString();
81-
LOG.warn("The configured AES secret key is too short (< {}), padding with random chars: {}",
82-
actualKey.length(), actualKey);
83-
}
84-
if (truncate) {
85-
actualKey = actualKey.substring(0, 32);
86-
LOG.warn("The configured AES secret key is too long (> 32), truncating: {}", actualKey);
81+
actualKeyPadding.append(randomChars);
82+
actualKey = actualKeyPadding.toString();
83+
LOG.warn("The configured AES secret key is too short (< {}), padding with random chars",
84+
actualKey.length());
85+
LOG.debug("Using\nsecurity.aesSecretKey={}", actualKey);
86+
}
87+
if (truncate) {
88+
actualKey = actualKey.substring(0, 32);
89+
LOG.warn("The configured AES secret key is too long (> 32), truncating");
90+
LOG.debug("Using\nsecurity.aesSecretKey={}", actualKey);
91+
}
8792
}
8893

8994
try {

‎core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultEncryptorManager.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,17 @@
2626

2727
public class DefaultEncryptorManager implements EncryptorManager {
2828

29+
protected final DefaultCredentialChecker credentialChecker;
30+
2931
protected final SecurityProperties securityProperties;
3032

3133
protected final Map<String, DefaultEncryptor> instances = new ConcurrentHashMap<>();
3234

33-
public DefaultEncryptorManager(final SecurityProperties securityProperties) {
35+
public DefaultEncryptorManager(
36+
final DefaultCredentialChecker credentialChecker,
37+
final SecurityProperties securityProperties) {
38+
39+
this.credentialChecker = credentialChecker;
3440
this.securityProperties = securityProperties;
3541
}
3642

@@ -41,7 +47,11 @@ public Encryptor getInstance() {
4147

4248
@Override
4349
public Encryptor getInstance(final String aesSecretKey) {
50+
credentialChecker.checkIsDefaultAESKeyInUse();
51+
4452
String actualKey = StringUtils.isBlank(aesSecretKey) ? securityProperties.getAesSecretKey() : aesSecretKey;
45-
return instances.computeIfAbsent(actualKey, k -> new DefaultEncryptor(k, securityProperties.getDigester()));
53+
return instances.computeIfAbsent(
54+
actualKey,
55+
k -> new DefaultEncryptor(k, securityProperties.isProductionMode(), securityProperties.getDigester()));
4656
}
4757
}

0 commit comments

Comments
 (0)