From 59099922f53a478903da304cc591c4baae549dc5 Mon Sep 17 00:00:00 2001 From: Kelly McLaughlin Date: Wed, 29 Mar 2017 07:24:46 -0600 Subject: Demonstrate the bug with AES CFB 128 encryption Demonstrate a bug with AES CFB 128 for certain key sizes introduced with the Erlang 19.0 release. The code in the block_crypt_nif function in the crypto.c source file incorrectly calls aes_cfb_8_crypt when the specified cipher is aes_cfb8 or aes_cfb128 and the key size is 24 or 32. The aes_cfb_8_crypt function calls the AES_cfb8_encrypt function from the openssl interface, but this is incorrect when the cipher is aes_cfb128. Unfortunately the test cases in the crypto test suite are insufficient to detect an issue like this because it exercises the encryption and decryption roundtrip using the same incorrect underlying function. The problem was observed when trying to update an application to Erlang 19 that attempted to decrypt data that was encrypted using aes_cfb128 by another source. In this commit I altered the crypto test suite to provide a demonstration of this problem. --- lib/crypto/c_src/crypto.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'lib/crypto/c_src') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index 2c8fb445dd..cd375e6d50 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -231,6 +231,7 @@ static ERL_NIF_TERM hmac_update_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM static ERL_NIF_TERM hmac_final_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); static ERL_NIF_TERM block_crypt_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); static ERL_NIF_TERM aes_cfb_8_crypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); +static ERL_NIF_TERM aes_cfb_128_crypt_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); static ERL_NIF_TERM aes_ige_crypt_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); static ERL_NIF_TERM aes_ctr_encrypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); static ERL_NIF_TERM aes_ctr_stream_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); @@ -301,6 +302,7 @@ static ErlNifFunc nif_funcs[] = { {"hmac_final_nif", 2, hmac_final_nif}, {"block_crypt_nif", 5, block_crypt_nif}, {"block_crypt_nif", 4, block_crypt_nif}, + {"aes_cfb_128_crypt_nif", 4, aes_cfb_128_crypt_nif}, {"aes_ige_crypt_nif", 4, aes_ige_crypt_nif}, {"aes_ctr_encrypt", 3, aes_ctr_encrypt}, @@ -1483,6 +1485,31 @@ static ERL_NIF_TERM aes_cfb_8_crypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM return ret; } +static ERL_NIF_TERM aes_cfb_128_crypt_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) +{/* (Key, IVec, Data, IsEncrypt) */ + ErlNifBinary key, ivec, text; + AES_KEY aes_key; + unsigned char ivec_clone[16]; /* writable copy */ + int new_ivlen = 0; + ERL_NIF_TERM ret; + + if (!enif_inspect_iolist_as_binary(env, argv[0], &key) + || !(key.size == 16 || key.size == 24 || key.size == 32) + || !enif_inspect_binary(env, argv[1], &ivec) || ivec.size != 16 + || !enif_inspect_iolist_as_binary(env, argv[2], &text)) { + return enif_make_badarg(env); + } + + memcpy(ivec_clone, ivec.data, 16); + AES_set_encrypt_key(key.data, key.size * 8, &aes_key); + AES_cfb128_encrypt((unsigned char *) text.data, + enif_make_new_binary(env, text.size, &ret), + text.size, &aes_key, ivec_clone, &new_ivlen, + (argv[3] != atom_true)); + CONSUME_REDS(env,text); + return ret; +} + static ERL_NIF_TERM aes_ige_crypt_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) {/* (Key, IVec, Data, IsEncrypt) */ #ifdef HAVE_AES_IGE -- cgit v1.2.3 From 25b8f8119f5b64b5c07cb5ed4978f7df64d4799f Mon Sep 17 00:00:00 2001 From: Kelly McLaughlin Date: Wed, 29 Mar 2017 08:49:17 -0600 Subject: Fix bug with AES CFB 128 Fix a bug with the use of the aes_cfb128 cipher by calling the correct underlying openssl interface function when the cipher is specified. --- lib/crypto/c_src/crypto.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'lib/crypto/c_src') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index cd375e6d50..d4264335b6 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -1405,13 +1405,20 @@ static ERL_NIF_TERM block_crypt_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM return enif_raise_exception(env, atom_notsup); } - if ((argv[0] == atom_aes_cfb8 || argv[0] == atom_aes_cfb128) + if (argv[0] == atom_aes_cfb8 && (key.size == 24 || key.size == 32)) { /* Why do EVP_CIPHER_CTX_set_key_length() fail on these key sizes? * Fall back on low level API */ return aes_cfb_8_crypt(env, argc-1, argv+1); } + else if (argv[0] == atom_aes_cfb128 + && (key.size == 24 || key.size == 32)) { + /* Why do EVP_CIPHER_CTX_set_key_length() fail on these key sizes? + * Fall back on low level API + */ + return aes_cfb_128_crypt_nif(env, argc-1, argv+1); + } ivec_size = EVP_CIPHER_iv_length(cipher); -- cgit v1.2.3