Address PR comments

corretto · May 7, 2024 · 3bff6ab · 3bff6ab
1 parent eef8b64
commit 3bff6ab
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 21 deletions.
diff --git a/csrc/aes_cbc.cpp b/csrc/aes_cbc.cpp
@@ -2,15 +2,12 @@
 // SPDX-License-Identifier: Apache-2.0
 #include "buffer.h"
 #include "env.h"
+#include "util.h"
 #include <openssl/err.h>
 #include <openssl/evp.h>
 #include <cstring>
 #include <jni.h>
 
-#define AWS_LC_BAD_PADDING_ERROR_CODE 0x1e000065
-
-#define EX_BAD_PADDING "javax/crypto/BadPaddingException"
-
 #define AES_CBC_BLOCK_SIZE_IN_BYTES 16
 #define KEY_LEN_AES128              16
 #define KEY_LEN_AES192              24
@@ -143,8 +140,8 @@ class AesCbcCipher {
     {
         int result = 0;
         if (EVP_CipherFinal_ex(ctx_, output, &result) != 1) {
-            if (ERR_get_error() == AWS_LC_BAD_PADDING_ERROR_CODE) {
-                throw java_ex(EX_BAD_PADDING, "Bad padding");
+            if (ERR_GET_REASON(ERR_get_error()) == CIPHER_R_BAD_DECRYPT) {
+                throw java_ex(EX_BADPADDING, "Bad padding");
             } else {
                 throw_openssl(EX_RUNTIME_CRYPTO, "EVP_CipherFinal_ex failed.");
             }
@@ -159,7 +156,7 @@ using namespace AmazonCorrettoCryptoProvider;
 
 extern "C" JNIEXPORT jint JNICALL Java_com_amazon_corretto_crypto_provider_AesCbcSpi_nInitUpdateFinal(JNIEnv* env,
     jclass,
-    jint opMod,
+    jint opMode,
     jint padding,
     jbyteArray key,
     jint keyLen,
@@ -181,7 +178,7 @@ extern "C" JNIEXPORT jint JNICALL Java_com_amazon_corretto_crypto_provider_AesCb
         {
             JBinaryBlob j_key(env, nullptr, key);
             JBinaryBlob j_iv(env, nullptr, iv);
-            aes_cbc_cipher.init(opMod, padding, j_key.get(), keyLen, j_iv.get());
+            aes_cbc_cipher.init(opMode, padding, j_key.get(), keyLen, j_iv.get());
         }
 
         int result = 0;
@@ -206,7 +203,7 @@ extern "C" JNIEXPORT jint JNICALL Java_com_amazon_corretto_crypto_provider_AesCb
 
 extern "C" JNIEXPORT jint JNICALL Java_com_amazon_corretto_crypto_provider_AesCbcSpi_nInitUpdate(JNIEnv* env,
     jclass,
-    jint opMod,
+    jint opMode,
     jint padding,
     jbyteArray key,
     jint keyLen,
@@ -227,7 +224,7 @@ extern "C" JNIEXPORT jint JNICALL Java_com_amazon_corretto_crypto_provider_AesCb
         {
             JBinaryBlob j_key(env, nullptr, key);
             JBinaryBlob j_iv(env, nullptr, iv);
-            aes_cbc_cipher.init(opMod, padding, j_key.get(), keyLen, j_iv.get());
+            aes_cbc_cipher.init(opMode, padding, j_key.get(), keyLen, j_iv.get());
         }
 
         // update

diff --git a/csrc/buffer.h b/csrc/buffer.h
@@ -623,8 +623,11 @@ class JBinaryBlob {
     uint8_t* get();
 
 private:
+    // The native pointer that is either backed by a direct ByteBuffer or a byte array.
     uint8_t* ptr_;
     JNIEnv* env_;
+    // In case the blob is backed by a byte array, we need to keep a reference that is used when the destructor is
+    // invoked.
     jbyteArray array_;
 };
 

diff --git a/csrc/util.cpp b/csrc/util.cpp
@@ -42,7 +42,8 @@ std::string formatOpensslError(unsigned long errCode, const char* fallback)
     }
 }
 
-JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_Utils_releaseEvpCipherCtx(JNIEnv*, jclass, jlong ctxPtr)
+extern "C" JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_Utils_releaseEvpCipherCtx(
+    JNIEnv*, jclass, jlong ctxPtr)
 {
     EVP_CIPHER_CTX_free(reinterpret_cast<EVP_CIPHER_CTX*>(ctxPtr));
 }

diff --git a/src/com/amazon/corretto/crypto/provider/AesCbcSpi.java b/src/com/amazon/corretto/crypto/provider/AesCbcSpi.java
@@ -27,10 +27,15 @@
 import javax.crypto.spec.IvParameterSpec;
 
 class AesCbcSpi extends CipherSpi {
+  // The value of padding is passed to AWS-LC and it respects EVP_CIPHER_CTX_set_padding API:
+  // https://github.com/aws/aws-lc/blob/main/include/openssl/cipher.h#L294-L297
+  public static final int NO_PADDING = 0;
+  public static final int PKCS7_PADDING = 1;
   public static final Set<String> AES_CBC_NO_PADDING_NAMES;
   public static final Set<String> AES_CBC_PKCS7_PADDING_NAMES;
 
   static {
+    Loader.load();
     AES_CBC_NO_PADDING_NAMES = new HashSet<>();
     AES_CBC_NO_PADDING_NAMES.add("AES/CBC/NoPadding".toLowerCase());
     AES_CBC_NO_PADDING_NAMES.add("AES_128/CBC/NoPadding".toLowerCase());
@@ -52,10 +57,10 @@ class AesCbcSpi extends CipherSpi {
   }
 
   private static final byte[] EMPTY_ARRAY = new byte[0];
-  private static final int NO_PADDING = 0;
-  private static final int PKCS7_PADDING = 1;
   private static final int BLOCK_SIZE_IN_BYTES = 128 / 8;
   private static final int MODE_NOT_SET = -1;
+  // ENC_MODE and DEC_MODE are passed to AWS-LC and respect EVP_CipherInit_ex API:
+  // https://github.com/aws/aws-lc/blob/main/include/openssl/cipher.h#L168
   private static final int ENC_MODE = 1;
   private static final int DEC_MODE = 0;
 
@@ -84,8 +89,8 @@ private enum CipherState {
   // controlled by a system property.
   private final boolean saveContext;
 
-  AesCbcSpi(final boolean paddingEnabled, final boolean saveContext) {
-    this.padding = paddingEnabled ? PKCS7_PADDING : NO_PADDING;
+  AesCbcSpi(final int padding, final boolean saveContext) {
+    this.padding = padding;
     this.cipherState = CipherState.NEEDS_INITIALIZATION;
     this.unprocessedInput = 0;
     this.opMode = MODE_NOT_SET;
@@ -383,7 +388,7 @@ private int update(
       return result;
     } catch (final Exception e) {
       cipherState = CipherState.NEEDS_INITIALIZATION;
-      saveNativeContextIfNeeded(ctxContainer[0]);
+      cleanUpNativeContextIfNeeded(ctxContainer[0]);
       throw e;
     }
   }
@@ -597,14 +602,14 @@ private int doFinal(
 
     } catch (final Exception e) {
       cipherState = CipherState.NEEDS_INITIALIZATION;
-      saveNativeContextIfNeeded(ctxContainer[0]);
+      cleanUpNativeContextIfNeeded(ctxContainer[0]);
       throw e;
     }
   }
 
-  private void saveNativeContextIfNeeded(final long ctxPtr) {
+  private void cleanUpNativeContextIfNeeded(final long ctxPtr) {
     if (nativeCtx == null && ctxPtr != 0) {
-      nativeCtx = new NativeEvpCipherCtx(ctxPtr);
+      Utils.releaseEvpCipherCtx(ctxPtr);
     }
   }
 

diff --git a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java
@@ -331,15 +331,15 @@ public Object newInstance(final Object constructorParameter) throws NoSuchAlgori
           final boolean saveContext =
               AmazonCorrettoCryptoProvider.this.nativeContextReleaseStrategy
                   == Utils.NativeContextReleaseStrategy.LAZY;
-          return new AesCbcSpi(true, saveContext);
+          return new AesCbcSpi(AesCbcSpi.PKCS7_PADDING, saveContext);
         }
 
         if ("Cipher".equalsIgnoreCase(type)
             && AES_CBC_NO_PADDING_NAMES.contains(algo.toLowerCase())) {
           final boolean saveContext =
               AmazonCorrettoCryptoProvider.this.nativeContextReleaseStrategy
                   == Utils.NativeContextReleaseStrategy.LAZY;
-          return new AesCbcSpi(false, saveContext);
+          return new AesCbcSpi(AesCbcSpi.NO_PADDING, saveContext);
         }
 
         throw new NoSuchAlgorithmException(String.format("No service class for %s/%s", type, algo));