You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/08/25 19:05:39 UTC
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2895: Improvements to AESCryptoService
dlmarion commented on code in PR #2895:
URL: https://github.com/apache/accumulo/pull/2895#discussion_r955320506
##########
core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java:
##########
@@ -516,48 +559,45 @@ public static Key generateKey(SecureRandom random, int size) {
return new SecretKeySpec(bytes, "AES");
}
- @SuppressFBWarnings(value = "CIPHER_INTEGRITY",
- justification = "integrity not needed for key wrap")
- public static Key unwrapKey(byte[] fek, Key kek) {
- Key result = null;
+ public static synchronized Key unwrapKey(byte[] fek, Key kek) {
try {
- Cipher c = Cipher.getInstance(KEY_WRAP_TRANSFORM);
+ final Cipher c = KEY_UNWRAP_CIPHER.get();
c.init(Cipher.UNWRAP_MODE, kek);
- result = c.unwrap(fek, "AES", Cipher.SECRET_KEY);
- } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException e) {
+ return c.unwrap(fek, "AES", Cipher.SECRET_KEY);
+ } catch (InvalidKeyException | NoSuchAlgorithmException e) {
throw new CryptoException("Unable to unwrap file encryption key", e);
}
- return result;
}
- @SuppressFBWarnings(value = "CIPHER_INTEGRITY",
- justification = "integrity not needed for key wrap")
- public static byte[] wrapKey(Key fek, Key kek) {
- byte[] result = null;
+ public static synchronized byte[] wrapKey(Key fek, Key kek) {
Review Comment:
Same comment here.
##########
core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java:
##########
@@ -516,48 +559,45 @@ public static Key generateKey(SecureRandom random, int size) {
return new SecretKeySpec(bytes, "AES");
}
- @SuppressFBWarnings(value = "CIPHER_INTEGRITY",
- justification = "integrity not needed for key wrap")
- public static Key unwrapKey(byte[] fek, Key kek) {
- Key result = null;
+ public static synchronized Key unwrapKey(byte[] fek, Key kek) {
Review Comment:
I don't think this needs the `synchronized` modifier. This may have been included with an earlier change of mine and then not removed when I changed the code to use a ThreadLocal.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org