You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "coheigea (via GitHub)" <gi...@apache.org> on 2026/08/28 09:42:44 UTC
[PR] Default Oauth2 encryption providers to use AES-GCM [cxf]
coheigea opened a new pull request, #3408:
URL: https://github.com/apache/cxf/pull/3408
Note: Only for application on main.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Default Oauth2 encryption providers to use AES-GCM [cxf]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #3408:
URL: https://github.com/apache/cxf/pull/3408#discussion_r3879930916
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java:
##########
@@ -229,10 +242,55 @@
String encodedData,
Key key,
KeyProperties props) throws SecurityException {
- String decryptedSequence = CryptoUtils.decryptSequence(encodedData, key, props);
+ String decryptedSequence = decryptSequence(encodedData, key, props);
return recreateCodeGrant(provider, decryptedSequence);
}
+ /**
+ * Encrypts the sequence. When no explicit KeyProperties are supplied the
+ * authenticated AES/GCM transformation is used, with a random IV prepended
+ * to the ciphertext. The previous default (the JCE "AES" shorthand, i.e.
+ * AES/ECB/PKCS5Padding) produced deterministic, malleable ciphertexts which
+ * a client holding several tokens could cut and splice block-by-block to
+ * forge token state; pass explicit KeyProperties only if a legacy
+ * transformation must be retained.
+ */
+ private static String encryptSequence(String sequence, Key secretKey,
+ KeyProperties props) throws SecurityException {
+ if (props != null) {
+ return CryptoUtils.encryptSequence(sequence, secretKey, props);
+ }
+ byte[] iv = new byte[DEFAULT_IV_SIZE];
+ RANDOM.nextBytes(iv);
+ byte[] cipherText = CryptoUtils.encryptBytes(sequence.getBytes(StandardCharsets.UTF_8),
+ secretKey, gcmProperties(iv));
+ byte[] out = new byte[iv.length + cipherText.length];
Review Comment:
## CodeQL / Uncontrolled data in arithmetic expression
This arithmetic expression depends on an [uncontrolled value](1), potentially causing an overflow.
[Show more details](https://github.com/apache/cxf/security/code-scanning/2252)
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Default Oauth2 encryption providers to use AES-GCM [cxf]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea merged PR #3408:
URL: https://github.com/apache/cxf/pull/3408
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Default Oauth2 encryption providers to use AES-GCM [cxf]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea commented on code in PR #3408:
URL: https://github.com/apache/cxf/pull/3408#discussion_r3881037666
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java:
##########
@@ -46,6 +49,12 @@
*/
public final class ModelEncryptionSupport {
public static final String SEP = "|";
+
+ private static final String DEFAULT_TRANSFORMATION = "AES/GCM/NoPadding";
+ private static final int DEFAULT_IV_SIZE = 12;
+ private static final int DEFAULT_AUTH_TAG_LENGTH = 128;
+ private static final SecureRandom RANDOM = new SecureRandom();
Review Comment:
Thanks @reta , I applied that change and did a little cleanup
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Default Oauth2 encryption providers to use AES-GCM [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #3408:
URL: https://github.com/apache/cxf/pull/3408#discussion_r3879909411
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java:
##########
@@ -229,10 +242,55 @@ public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider pr
String encodedData,
Key key,
KeyProperties props) throws SecurityException {
- String decryptedSequence = CryptoUtils.decryptSequence(encodedData, key, props);
+ String decryptedSequence = decryptSequence(encodedData, key, props);
return recreateCodeGrant(provider, decryptedSequence);
}
+ /**
+ * Encrypts the sequence. When no explicit KeyProperties are supplied the
+ * authenticated AES/GCM transformation is used, with a random IV prepended
+ * to the ciphertext. The previous default (the JCE "AES" shorthand, i.e.
+ * AES/ECB/PKCS5Padding) produced deterministic, malleable ciphertexts which
+ * a client holding several tokens could cut and splice block-by-block to
+ * forge token state; pass explicit KeyProperties only if a legacy
+ * transformation must be retained.
+ */
+ private static String encryptSequence(String sequence, Key secretKey,
+ KeyProperties props) throws SecurityException {
+ if (props != null) {
+ return CryptoUtils.encryptSequence(sequence, secretKey, props);
+ }
+ byte[] iv = new byte[DEFAULT_IV_SIZE];
Review Comment:
```suggestion
byte[] iv =CryptoUtils.generateSecureRandomBytes(DEFAULT_IV_SIZE);
```
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java:
##########
@@ -229,10 +242,55 @@ public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider pr
String encodedData,
Key key,
KeyProperties props) throws SecurityException {
- String decryptedSequence = CryptoUtils.decryptSequence(encodedData, key, props);
+ String decryptedSequence = decryptSequence(encodedData, key, props);
return recreateCodeGrant(provider, decryptedSequence);
}
+ /**
+ * Encrypts the sequence. When no explicit KeyProperties are supplied the
+ * authenticated AES/GCM transformation is used, with a random IV prepended
+ * to the ciphertext. The previous default (the JCE "AES" shorthand, i.e.
+ * AES/ECB/PKCS5Padding) produced deterministic, malleable ciphertexts which
+ * a client holding several tokens could cut and splice block-by-block to
+ * forge token state; pass explicit KeyProperties only if a legacy
+ * transformation must be retained.
+ */
+ private static String encryptSequence(String sequence, Key secretKey,
+ KeyProperties props) throws SecurityException {
+ if (props != null) {
+ return CryptoUtils.encryptSequence(sequence, secretKey, props);
+ }
+ byte[] iv = new byte[DEFAULT_IV_SIZE];
Review Comment:
```suggestion
byte[] iv = CryptoUtils.generateSecureRandomBytes(DEFAULT_IV_SIZE);
```
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Default Oauth2 encryption providers to use AES-GCM [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #3408:
URL: https://github.com/apache/cxf/pull/3408#discussion_r3879914696
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java:
##########
@@ -46,6 +49,12 @@
*/
public final class ModelEncryptionSupport {
public static final String SEP = "|";
+
+ private static final String DEFAULT_TRANSFORMATION = "AES/GCM/NoPadding";
+ private static final int DEFAULT_IV_SIZE = 12;
+ private static final int DEFAULT_AUTH_TAG_LENGTH = 128;
+ private static final SecureRandom RANDOM = new SecureRandom();
Review Comment:
We probably don't need it and could use CryptoUtils?
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org