Repository: commons-crypto Updated Branches: refs/heads/master dc28a2b7b -> 9456415cd
CRYPTO-141: Avoid calling operations on an invalid context. There are two main changes here. The first one wraps the OpenSSL cipher context with some extra state needed by the JNI wrapper. This allows checking whether it's safe to call operations such as EVP_CipherUpdate, which can cause problems if the context is not properly initialized. This more closely follows the semantics of the Java interface. The second change avoids cleaning up the OpenSSL context when an error happens. Doing that would invalidate the memory used by the context, but the Java code would still have the address of the now invalid context, and calling another JNI method would crash the JVM. This ties the lifecycle of the OpenSSL context to the Java cipher instance. As part of those changes, all uses of EVP_CIPHER_CTX_cleanup were removed; the remaining cleanup paths use EVP_CIPHER_CTX_free, which is its replacement. There are a few more changes that are not required for the above but helped with testing and debugging: - some very minor changes in Java code, especially using final fields where they make sense. - enable -Wall and -Werror when compiling on linux64. - fixed some minor issues with error handling in the native code. Closes #80 Project: http://git-wip-us.apache.org/repos/asf/commons-crypto/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-crypto/commit/9456415c Tree: http://git-wip-us.apache.org/repos/asf/commons-crypto/tree/9456415c Diff: http://git-wip-us.apache.org/repos/asf/commons-crypto/diff/9456415c Branch: refs/heads/master Commit: 9456415cd185b17782507006013556c306441a23 Parents: dc28a2b Author: Marcelo Vanzin <van...@cloudera.com> Authored: Fri Oct 5 09:31:34 2018 -0700 Committer: Marcelo Vanzin <van...@cloudera.com> Committed: Fri Oct 5 09:31:34 2018 -0700 ---------------------------------------------------------------------- Makefile.common | 6 +- .../apache/commons/crypto/cipher/OpenSsl.java | 17 +- .../crypto/cipher/OpenSslFeedbackCipher.java | 6 +- .../org/apache/commons/crypto/utils/Utils.java | 18 +- .../commons/crypto/cipher/OpenSslNative.c | 186 ++++++++++++------- .../crypto/cipher/OpenSslCipherTest.java | 47 +++++ 6 files changed, 194 insertions(+), 86 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/9456415c/Makefile.common ---------------------------------------------------------------------- diff --git a/Makefile.common b/Makefile.common index f6bd9ff..96a53b8 100644 --- a/Makefile.common +++ b/Makefile.common @@ -86,8 +86,8 @@ Linux-x86_COMMONS_CRYPTO_FLAGS:= Linux-x86_64_CC := $(CROSS_PREFIX)gcc Linux-x86_64_CXX := $(CROSS_PREFIX)g++ Linux-x86_64_STRIP := $(CROSS_PREFIX)strip -Linux-x86_64_CXXFLAGS := -Ilib/inc_linux -I$(JAVA_HOME)/include -Ilib/inc_mac -O2 -fPIC -fvisibility=hidden -m64 -Linux-x86_64_CFLAGS := -Ilib/inc_linux -I$(JAVA_HOME)/include -Ilib/inc_mac -O2 -fPIC -fvisibility=hidden -m64 +Linux-x86_64_CXXFLAGS := -Ilib/inc_linux -I$(JAVA_HOME)/include -Ilib/inc_mac -O2 -fPIC -fvisibility=hidden -m64 -Wall -Werror +Linux-x86_64_CFLAGS := -Ilib/inc_linux -I$(JAVA_HOME)/include -Ilib/inc_mac -O2 -fPIC -fvisibility=hidden -m64 -Wall -Werror Linux-x86_64_LINKFLAGS := -shared -static-libgcc Linux-x86_64_LIBNAME := libcommons-crypto.so Linux-x86_64_COMMONS_CRYPTO_FLAGS := @@ -219,7 +219,7 @@ STRIP := $($(os_arch)_STRIP) CC := $($(os_arch)_CC) CXX := $($(os_arch)_CXX) STRIP := $($(os_arch)_STRIP) -CFLAGS := $($(os_arch)_CXXFLAGS) +CFLAGS := $($(os_arch)_CFLAGS) CXXFLAGS := $($(os_arch)_CXXFLAGS) LINKFLAGS := $($(os_arch)_LINKFLAGS) LIBNAME := $($(os_arch)_LIBNAME) http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/9456415c/src/main/java/org/apache/commons/crypto/cipher/OpenSsl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/crypto/cipher/OpenSsl.java b/src/main/java/org/apache/commons/crypto/cipher/OpenSsl.java index e7c061a..12c0d9d 100644 --- a/src/main/java/org/apache/commons/crypto/cipher/OpenSsl.java +++ b/src/main/java/org/apache/commons/crypto/cipher/OpenSsl.java @@ -36,12 +36,12 @@ import org.apache.commons.crypto.utils.Utils; */ final class OpenSsl { - OpenSslFeedbackCipher opensslBlockCipher; - // Mode constant defined by OpenSsl JNI public static final int ENCRYPT_MODE = 1; public static final int DECRYPT_MODE = 0; + private final OpenSslFeedbackCipher opensslBlockCipher; + /** Currently only support AES/CTR/NoPadding. */ private static enum AlgorithmMode { AES_CTR, AES_CBC, AES_GCM; @@ -99,7 +99,7 @@ final class OpenSsl { } catch (Exception t) { loadingFailure = t; } catch (UnsatisfiedLinkError t) { - loadingFailure = t; + loadingFailure = t; } finally { loadingFailureReason = loadingFailure; } @@ -217,7 +217,6 @@ final class OpenSsl { */ public void init(int mode, byte[] key, AlgorithmParameterSpec params) throws InvalidAlgorithmParameterException { - checkState(); opensslBlockCipher.init(mode, key, params); } @@ -251,7 +250,6 @@ final class OpenSsl { */ public int update(ByteBuffer input, ByteBuffer output) throws ShortBufferException { - checkState(); Utils.checkArgument(input.isDirect() && output.isDirect(), "Direct buffers are required."); return opensslBlockCipher.update(input, output); @@ -272,7 +270,6 @@ final class OpenSsl { */ public int update(byte[] input, int inputOffset, int inputLen, byte[] output, int outputOffset) throws ShortBufferException { - checkState(); return opensslBlockCipher.update(input, inputOffset, inputLen, output, outputOffset); } @@ -301,8 +298,6 @@ final class OpenSsl { byte[] output, int outputOffset) throws ShortBufferException, IllegalBlockSizeException, BadPaddingException{ - - checkState(); return opensslBlockCipher.doFinal(input, inputOffset, inputLen, output, outputOffset); } @@ -348,7 +343,6 @@ final class OpenSsl { */ public int doFinal(ByteBuffer input, ByteBuffer output) throws ShortBufferException, IllegalBlockSizeException, BadPaddingException { - checkState(); Utils.checkArgument(output.isDirect(), "Direct buffer is required."); return opensslBlockCipher.doFinal(input, output); @@ -380,11 +374,6 @@ final class OpenSsl { } } - /** Checks whether context is initialized. */ - private void checkState() { - Utils.checkState(opensslBlockCipher != null); - } - @Override protected void finalize() throws Throwable { clean(); http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/9456415c/src/main/java/org/apache/commons/crypto/cipher/OpenSslFeedbackCipher.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/crypto/cipher/OpenSslFeedbackCipher.java b/src/main/java/org/apache/commons/crypto/cipher/OpenSslFeedbackCipher.java index 4fa2743..67f8058 100644 --- a/src/main/java/org/apache/commons/crypto/cipher/OpenSslFeedbackCipher.java +++ b/src/main/java/org/apache/commons/crypto/cipher/OpenSslFeedbackCipher.java @@ -32,8 +32,8 @@ import java.security.spec.AlgorithmParameterSpec; abstract class OpenSslFeedbackCipher { protected long context = 0; - protected int algorithmMode; - protected int padding; + protected final int algorithmMode; + protected final int padding; protected int cipherMode = OpenSsl.DECRYPT_MODE; @@ -68,6 +68,6 @@ abstract class OpenSslFeedbackCipher { } public void checkState() { - Utils.checkState(context != 0); + Utils.checkState(context != 0, "Cipher context is invalid."); } } http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/9456415c/src/main/java/org/apache/commons/crypto/utils/Utils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/crypto/utils/Utils.java b/src/main/java/org/apache/commons/crypto/utils/Utils.java index 1c96b44..0bef9ba 100644 --- a/src/main/java/org/apache/commons/crypto/utils/Utils.java +++ b/src/main/java/org/apache/commons/crypto/utils/Utils.java @@ -89,13 +89,13 @@ public final class Utils { /** * Gets a properties instance that defaults to the System Properties - * plus any other properties found in the file + * plus any other properties found in the file * {@link #SYSTEM_PROPERTIES_FILE} * @return a Properties instance with defaults */ public static Properties getDefaultProperties() { return new Properties(DefaultPropertiesHolder.DEFAULT_PROPERTIES); - } + } /** * Gets the properties merged with default properties. @@ -183,8 +183,20 @@ public final class Utils { * @throws IllegalStateException if expression is false. */ public static void checkState(boolean expression) { + checkState(expression, null); + } + + /** + * Ensures the truth of an expression involving the state of the calling + * instance, but not involving any parameters to the calling method. + * + * @param expression a boolean expression. + * @param message Error message for the exception when the expression is false. + * @throws IllegalStateException if expression is false. + */ + public static void checkState(boolean expression, String message) { if (!expression) { - throw new IllegalStateException(); + throw new IllegalStateException(message); } } http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/9456415c/src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c ---------------------------------------------------------------------- diff --git a/src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c b/src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c index 078e2f2..ec9c945 100644 --- a/src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c +++ b/src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c @@ -32,7 +32,6 @@ #ifdef UNIX static EVP_CIPHER_CTX * (*dlsym_EVP_CIPHER_CTX_new)(void); static void (*dlsym_EVP_CIPHER_CTX_free)(EVP_CIPHER_CTX *); -static int (*dlsym_EVP_CIPHER_CTX_cleanup)(EVP_CIPHER_CTX *); static void (*dlsym_EVP_CIPHER_CTX_init)(EVP_CIPHER_CTX *); static int (*dlsym_EVP_CIPHER_CTX_set_padding)(EVP_CIPHER_CTX *, int); static int (*dlsym_EVP_CIPHER_CTX_ctrl)(EVP_CIPHER_CTX *, int, int, void *); @@ -55,7 +54,6 @@ static EVP_CIPHER * (*dlsym_EVP_aes_128_gcm)(void); #ifdef WINDOWS typedef EVP_CIPHER_CTX * (__cdecl *__dlsym_EVP_CIPHER_CTX_new)(void); typedef void (__cdecl *__dlsym_EVP_CIPHER_CTX_free)(EVP_CIPHER_CTX *); -typedef int (__cdecl *__dlsym_EVP_CIPHER_CTX_cleanup)(EVP_CIPHER_CTX *); typedef void (__cdecl *__dlsym_EVP_CIPHER_CTX_init)(EVP_CIPHER_CTX *); typedef int (__cdecl *__dlsym_EVP_CIPHER_CTX_set_padding)(EVP_CIPHER_CTX *, int); typedef int (__cdecl *__dlsym_EVP_CIPHER_CTX_ctrl)(EVP_CIPHER_CTX *, int, int, void *); @@ -77,7 +75,6 @@ typedef EVP_CIPHER * (__cdecl *__dlsym_EVP_aes_192_gcm)(void); typedef EVP_CIPHER * (__cdecl *__dlsym_EVP_aes_128_gcm)(void); static __dlsym_EVP_CIPHER_CTX_new dlsym_EVP_CIPHER_CTX_new; static __dlsym_EVP_CIPHER_CTX_free dlsym_EVP_CIPHER_CTX_free; -static __dlsym_EVP_CIPHER_CTX_cleanup dlsym_EVP_CIPHER_CTX_cleanup; static __dlsym_EVP_CIPHER_CTX_init dlsym_EVP_CIPHER_CTX_init; static __dlsym_EVP_CIPHER_CTX_set_padding dlsym_EVP_CIPHER_CTX_set_padding; static __dlsym_EVP_CIPHER_CTX_ctrl dlsym_EVP_CIPHER_CTX_ctrl; @@ -168,8 +165,6 @@ JNIEXPORT void JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_initI "EVP_CIPHER_CTX_new"); LOAD_DYNAMIC_SYMBOL(dlsym_EVP_CIPHER_CTX_free, env, openssl, \ "EVP_CIPHER_CTX_free"); - LOAD_DYNAMIC_SYMBOL(dlsym_EVP_CIPHER_CTX_cleanup, env, openssl, \ - "EVP_CIPHER_CTX_cleanup"); LOAD_DYNAMIC_SYMBOL(dlsym_EVP_CIPHER_CTX_init, env, openssl, \ "EVP_CIPHER_CTX_init"); LOAD_DYNAMIC_SYMBOL(dlsym_EVP_CIPHER_CTX_set_padding, env, openssl, \ @@ -189,9 +184,6 @@ JNIEXPORT void JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_initI env, openssl, "EVP_CIPHER_CTX_new"); LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_free, dlsym_EVP_CIPHER_CTX_free, \ env, openssl, "EVP_CIPHER_CTX_free"); - LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_cleanup, \ - dlsym_EVP_CIPHER_CTX_cleanup, env, - openssl, "EVP_CIPHER_CTX_cleanup"); LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_init, dlsym_EVP_CIPHER_CTX_init, \ env, openssl, "EVP_CIPHER_CTX_init"); LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_set_padding, \ @@ -215,6 +207,56 @@ JNIEXPORT void JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_initI } } +typedef struct EVP_CTX_Wrapper { + int initialized; + EVP_CIPHER_CTX *ctx; +} EVP_CTX_Wrapper; + +#define CTX_WRAPPER(addr) ((EVP_CTX_Wrapper *)(ptrdiff_t) addr) + +static EVP_CTX_Wrapper * new_context_wrapper(JNIEnv *env) { + EVP_CTX_Wrapper *wrapper = calloc(1, sizeof(EVP_CTX_Wrapper)); + if (wrapper == NULL) { + THROW(env, "java/lang/OutOfMemoryError", NULL); + return NULL; + } + + wrapper->ctx = dlsym_EVP_CIPHER_CTX_new(); + if (wrapper->ctx == NULL) { + free(wrapper); + wrapper = NULL; + + THROW(env, "java/lang/OutOfMemoryError", NULL); + return NULL; + } + + return wrapper; +} + +static void free_context_wrapper(EVP_CTX_Wrapper *wrapper) { + if (wrapper != NULL) { + if (wrapper->ctx != NULL) { + dlsym_EVP_CIPHER_CTX_free(wrapper->ctx); + } + free(wrapper); + } +} + +static EVP_CIPHER_CTX * get_context(JNIEnv *env, jlong addr) { + EVP_CTX_Wrapper *wrapper = CTX_WRAPPER(addr); + if (wrapper == NULL || wrapper->ctx == NULL) { + THROW(env, "java/lang/NullPointerException", "Context address is null."); + return NULL; + } + + if (!wrapper->initialized) { + THROW(env, "java/lang/IllegalStateException", "Context is not initialized."); + return NULL; + } + + return wrapper->ctx; +} + JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_initContext (JNIEnv *env, jclass clazz, jint alg, jint padding) { @@ -250,14 +292,8 @@ JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_init return (jlong)0; } - // Create and initialize a EVP_CIPHER_CTX - EVP_CIPHER_CTX *context = dlsym_EVP_CIPHER_CTX_new(); - if (!context) { - THROW(env, "java/lang/OutOfMemoryError", NULL); - return (jlong)0; - } - - return JLONG(context); + EVP_CTX_Wrapper *wrapper = new_context_wrapper(env); + return JLONG(wrapper); } // Only supports AES-CTR and AES-CBC currently @@ -296,9 +332,17 @@ JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_init (JNIEnv *env, jclass clazz, jlong ctx, jint mode, jint alg, jint padding, jbyteArray key, jbyteArray iv) { - jlong result = 0L; - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CTX_Wrapper *wrapper = CTX_WRAPPER(ctx); + int is_new_context = 0; + if (wrapper == NULL) { + wrapper = new_context_wrapper(env); + if (wrapper == NULL) { + return JLONG(NULL); + } + is_new_context = 1; + } + EVP_CIPHER_CTX *context = wrapper->ctx; jbyte *jKey = NULL; jbyte *jIv = NULL; int jKeyLen = (*env)->GetArrayLength(env, key); @@ -315,15 +359,6 @@ JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_init goto cleanup; } - if (context == 0) { - // Create and initialize a EVP_CIPHER_CTX - context = dlsym_EVP_CIPHER_CTX_new(); - if (!context) { - THROW(env, "java/lang/OutOfMemoryError", NULL); - return (jlong)0; - } - } - jKey = (*env)->GetByteArrayElements(env, key, NULL); if (jKey == NULL) { THROW(env, "java/lang/InternalError", "Cannot get bytes array for key."); @@ -341,8 +376,13 @@ JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_init } // initialize cipher & mode - int rc = dlsym_EVP_CipherInit_ex(context, getEvpCipher(alg, jKeyLen), \ - NULL, NULL, NULL, mode == ENCRYPT_MODE); + EVP_CIPHER *cipher = getEvpCipher(alg, jKeyLen); + if (cipher == NULL) { + THROW(env, "java/security/InvalidKeyException", "Invalid key length."); + goto cleanup; + } + + int rc = dlsym_EVP_CipherInit_ex(context, cipher, NULL, NULL, NULL, mode == ENCRYPT_MODE); if (rc == 0) { THROW(env, "java/lang/InternalError", "Error in EVP_CipherInit_ex."); goto cleanup; @@ -352,7 +392,12 @@ JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_init // Note: set IV length after cipher is initialized, before iv is initialized. if (alg == AES_GCM) { rc = dlsym_EVP_CIPHER_CTX_ctrl(context, EVP_CTRL_GCM_SET_IVLEN, jIvLen, NULL); + if (rc == 0) { + THROW(env, "java/lang/InternalError", "Error setting GCM initial vector length."); + goto cleanup; + } } + rc = dlsym_EVP_CipherInit_ex(context, NULL, NULL, \ (unsigned char *)jKey, (unsigned char *)jIv, mode == ENCRYPT_MODE); if (rc == 0) { @@ -367,24 +412,20 @@ JNIEXPORT jlong JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_init } // everything is OK, - result = JLONG(context); + wrapper->initialized = 1; cleanup: - if (result == 0 && context != NULL) { - if (CONTEXT(ctx) != NULL) { - dlsym_EVP_CIPHER_CTX_cleanup(context); - } else { - dlsym_EVP_CIPHER_CTX_free(context); - } - } if (jKey != NULL) { (*env)->ReleaseByteArrayElements(env, key, jKey, 0); } if (jIv != NULL) { (*env)->ReleaseByteArrayElements(env, iv, jIv, 0); } - - return result; + if (is_new_context && !wrapper->initialized) { + free_context_wrapper(wrapper); + wrapper = NULL; + } + return JLONG(wrapper); } // https://www.openssl.org/docs/crypto/EVP_EncryptInit.html @@ -416,7 +457,11 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_updat (JNIEnv *env, jclass clazz, jlong ctx, jobject input, jint input_offset, jint input_len, jobject output, jint output_offset, jint max_output_len) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CIPHER_CTX *context = get_context(env, ctx); + if (context == NULL) { + return 0; + } + if (!check_update_max_output_len(context, input_len, max_output_len)) { THROW(env, "javax/crypto/ShortBufferException", \ "Output buffer is not sufficient."); @@ -434,7 +479,6 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_updat int output_len = 0; if (!dlsym_EVP_CipherUpdate(context, output_bytes, &output_len, \ input_bytes, input_len)) { - dlsym_EVP_CIPHER_CTX_cleanup(context); THROW(env, "java/lang/InternalError", "Error in EVP_CipherUpdate."); return 0; } @@ -445,7 +489,10 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_updat (JNIEnv *env, jclass clazz, jlong ctx, jbyteArray input, jint input_offset, jint input_len, jbyteArray output, jint output_offset, jint max_output_len) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CIPHER_CTX *context = get_context(env, ctx); + if (context == NULL) { + return 0; + } // when provide AAD to EVP cipher, output is NULL. if (output != NULL @@ -467,13 +514,12 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_updat } if (input_bytes == NULL || (output != NULL && output_bytes == NULL)) { THROW(env, "java/lang/InternalError", "Cannot get buffer address."); - return 0; + goto cleanup; } int rc = dlsym_EVP_CipherUpdate(context, output_bytes + output_offset, &output_len, \ input_bytes + input_offset, input_len); if (rc == 0) { - dlsym_EVP_CIPHER_CTX_cleanup(context); THROW(env, "java/lang/InternalError", "Error in EVP_CipherUpdate."); output_len = 0; } @@ -493,28 +539,35 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_updat (JNIEnv *env, jclass clazz, jlong ctx, jbyteArray input, jint input_offset, jint input_len, jobject output, jint output_offset, jint max_output_len) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CIPHER_CTX *context = get_context(env, ctx); + if (context == NULL) { + return 0; + } + if (!check_update_max_output_len(context, input_len, max_output_len)) { THROW(env, "javax/crypto/ShortBufferException", \ "Output buffer is not sufficient."); return 0; } + unsigned char *input_bytes = (unsigned char *) (*env)->GetByteArrayElements(env, input, 0); unsigned char *output_bytes = (*env)->GetDirectBufferAddress(env, output); + int output_len = 0; + if (input_bytes == NULL || output_bytes == NULL) { THROW(env, "java/lang/InternalError", "Cannot get buffer address."); - return 0; + goto cleanup; } - input_bytes = input_bytes + input_offset; - output_bytes = output_bytes + output_offset; - int output_len = 0; - if (!dlsym_EVP_CipherUpdate(context, output_bytes, &output_len, \ - input_bytes, input_len)) { - (*env)->ReleaseByteArrayElements(env, input, (jbyte *) input_bytes, 0); - dlsym_EVP_CIPHER_CTX_cleanup(context); + int rc = dlsym_EVP_CipherUpdate(context, output_bytes + output_offset, &output_len, + input_bytes + input_offset, input_len); + if (rc == 0) { THROW(env, "java/lang/InternalError", "Error in EVP_CipherUpdate."); - return 0; + } + +cleanup: + if (input_bytes != NULL) { + (*env)->ReleaseByteArrayElements(env, input, (jbyte *) input_bytes, 0); } return output_len; } @@ -540,7 +593,11 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_doFin (JNIEnv *env, jclass clazz, jlong ctx, jobject output, jint offset, jint max_output_len) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CIPHER_CTX *context = get_context(env, ctx); + if (context == NULL) { + return 0; + } + if (!check_doFinal_max_output_len(context, max_output_len)) { THROW(env, "javax/crypto/ShortBufferException", \ "Output buffer is not sufficient."); @@ -562,7 +619,6 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_doFin } else { THROW(env, "java/lang/InternalError", "Error in EVP_CipherFinal_ex."); } - dlsym_EVP_CIPHER_CTX_cleanup(context); return 0; } return output_len; @@ -572,7 +628,11 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_doFin (JNIEnv *env, jclass clazz, jlong ctx, jbyteArray output, jint offset, jint max_output_len) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CIPHER_CTX *context = get_context(env, ctx); + if (context == NULL) { + return 0; + } + if (!check_doFinal_max_output_len(context, max_output_len)) { THROW(env, "javax/crypto/ShortBufferException", \ "Output buffer is not sufficient."); @@ -597,7 +657,6 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_doFin } else { THROW(env, "java/lang/InternalError", "Error in EVP_CipherFinal_ex."); } - dlsym_EVP_CIPHER_CTX_cleanup(context); return 0; } return output_len; @@ -606,7 +665,10 @@ JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_doFin JNIEXPORT jint JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_ctrl (JNIEnv *env, jclass clazz, jlong ctx, jint type, jint arg, jbyteArray data) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); + EVP_CIPHER_CTX *context = get_context(env, ctx); + if (context == NULL) { + return 0; + } int rc = 0; void *data_ptr = NULL; @@ -651,8 +713,6 @@ exit_: JNIEXPORT void JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_clean (JNIEnv *env, jclass clazz, jlong ctx) { - EVP_CIPHER_CTX *context = CONTEXT(ctx); - if (context) { - dlsym_EVP_CIPHER_CTX_free(context); - } + EVP_CTX_Wrapper *wrapper = CTX_WRAPPER(ctx); + free_context_wrapper(wrapper); } http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/9456415c/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java b/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java index de6d67c..98fc325 100644 --- a/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java +++ b/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java @@ -22,9 +22,11 @@ import java.nio.ByteBuffer; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; +import java.util.Properties; import javax.crypto.NoSuchPaddingException; import javax.crypto.ShortBufferException; import javax.crypto.spec.IvParameterSpec; +import javax.crypto.spec.SecretKeySpec; import org.junit.Assert; import org.junit.Assume; @@ -133,4 +135,49 @@ public class OpenSslCipherTest extends AbstractCipherTest { cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new IvParameterSpec(invalidIV)); } + @Test + public void testCipherLifecycle() throws Exception { + try (OpenSslCipher cipher = new OpenSslCipher(new Properties(), "AES/CTR/NoPadding")) { + try { + cipher.update(dummyBuffer(), dummyBuffer()); + Assert.fail("Should have thrown exception."); + } catch (IllegalStateException ise) { + // expected; + } + + cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(KEY, "AES"), + new IvParameterSpec(IV)); + cipher.update(dummyBuffer(), dummyBuffer()); + + try { + cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(new byte[1], "AES"), + new IvParameterSpec(IV)); + Assert.fail("Should have thrown exception."); + } catch (InvalidKeyException ike) { + // expected; + } + + // Should keep working with previous init parameters. + cipher.update(dummyBuffer(), dummyBuffer()); + cipher.doFinal(dummyBuffer(), dummyBuffer()); + cipher.close(); + + try { + cipher.update(dummyBuffer(), dummyBuffer()); + Assert.fail("Should have thrown exception."); + } catch (IllegalStateException ise) { + // expected; + } + + cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(KEY, "AES"), + new IvParameterSpec(IV)); + cipher.update(dummyBuffer(), dummyBuffer()); + cipher.close(); + } + } + + private ByteBuffer dummyBuffer() { + return ByteBuffer.allocateDirect(8); + } + }
