github-advanced-security[bot] commented on code in PR #1243:
URL: https://github.com/apache/syncope/pull/1243#discussion_r2542406585
##########
core/spring/src/main/java/org/apache/syncope/core/spring/security/Encryptor.java:
##########
@@ -46,61 +45,94 @@
private static final Map<String, Encryptor> 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<CipherAlgorithm, StandardStringDigester> digesters = new
ConcurrentHashMap<>();
- private SecretKeySpec keySpec;
+ private final Optional<SecretKeySpec> 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);
Review Comment:
## Insertion of sensitive information into log files
This [potentially sensitive information](1) is written to a log file.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2359)
##########
core/spring/src/main/java/org/apache/syncope/core/spring/security/Encryptor.java:
##########
@@ -46,61 +45,94 @@
private static final Map<String, Encryptor> 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<CipherAlgorithm, StandardStringDigester> digesters = new
ConcurrentHashMap<>();
- private SecretKeySpec keySpec;
+ private final Optional<SecretKeySpec> 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);
Review Comment:
## Insertion of sensitive information into log files
This [potentially sensitive information](1) is written to a log file.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2360)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
