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 +++++++++++++++++++++++++++ lib/crypto/src/crypto.erl | 6 +++++- lib/crypto/test/crypto_SUITE.erl | 10 ++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) (limited to 'lib/crypto') 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 diff --git a/lib/crypto/src/crypto.erl b/lib/crypto/src/crypto.erl index 696929ba4e..60e0affda0 100644 --- a/lib/crypto/src/crypto.erl +++ b/lib/crypto/src/crypto.erl @@ -822,6 +822,8 @@ sha_mac_96(Key, Data) -> hmac(sha, Key, Data, 12). block_crypt_nif(_Type, _Key, _Ivec, _Text, _IsEncrypt) -> ?nif_stub. block_crypt_nif(_Type, _Key, _Text, _IsEncrypt) -> ?nif_stub. +aes_cfb_128_crypt_nif(_Key, _Ivec, _Text, _IsEncrypt) -> ?nif_stub. + check_des3_key(Key) -> case lists:map(fun erlang:iolist_to_binary/1, Key) of ValidKey = [B1, B2, B3] when byte_size(B1) =:= 8, @@ -915,7 +917,9 @@ blowfish_ofb64_encrypt(Key, IVec, Data) -> -spec aes_cfb_128_decrypt(iodata(), binary(), iodata()) -> binary(). aes_cfb_128_encrypt(Key, IVec, Data) -> - block_encrypt(aes_cfb128, Key, IVec, Data). + %% block_encrypt(aes_cfb128, Key, IVec, Data). + aes_cfb_128_crypt_nif(Key, IVec, Data, true). + aes_cfb_128_decrypt(Key, IVec, Data) -> block_decrypt(aes_cfb128, Key, IVec, Data). diff --git a/lib/crypto/test/crypto_SUITE.erl b/lib/crypto/test/crypto_SUITE.erl index 7b07cef33f..dbd335c693 100644 --- a/lib/crypto/test/crypto_SUITE.erl +++ b/lib/crypto/test/crypto_SUITE.erl @@ -358,6 +358,16 @@ block_cipher({Type, Key, PlainText}) -> ct:fail({{crypto, block_decrypt, [Type, Key, CipherText]}, {expected, Plain}, {got, Other}}) end; +block_cipher({aes_cfb128, Key, IV, PlainText}) -> + Plain = iolist_to_binary(PlainText), + CipherText = crypto:aes_cfb_128_encrypt(Key, IV, PlainText), + case crypto:block_decrypt(aes_cfb128, Key, IV, CipherText) of + Plain -> + ok; + Other -> + ct:fail({{crypto, block_decrypt, [aes_cfb128, Key, IV, CipherText]}, {expected, Plain}, {got, Other}}) + end; + block_cipher({Type, Key, IV, PlainText}) -> Plain = iolist_to_binary(PlainText), CipherText = crypto:block_encrypt(Type, Key, IV, PlainText), -- 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') 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 From 71f7e9155c4867f4e8036704337c21127f508dfb Mon Sep 17 00:00:00 2001 From: Erlang/OTP Date: Fri, 31 Mar 2017 12:58:36 +0200 Subject: Update version numbers --- lib/crypto/vsn.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/crypto') diff --git a/lib/crypto/vsn.mk b/lib/crypto/vsn.mk index 81cb2f8130..f3e0623ac9 100644 --- a/lib/crypto/vsn.mk +++ b/lib/crypto/vsn.mk @@ -1 +1 @@ -CRYPTO_VSN = 3.7.3 +CRYPTO_VSN = 3.7.4 -- cgit v1.2.3 From 19427107ca9305a931dcaea8c2134017aa385fbd Mon Sep 17 00:00:00 2001 From: Erlang/OTP Date: Fri, 31 Mar 2017 12:59:07 +0200 Subject: Update release notes --- lib/crypto/doc/src/notes.xml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'lib/crypto') diff --git a/lib/crypto/doc/src/notes.xml b/lib/crypto/doc/src/notes.xml index 37997b649b..887aeca680 100644 --- a/lib/crypto/doc/src/notes.xml +++ b/lib/crypto/doc/src/notes.xml @@ -31,6 +31,22 @@

This document describes the changes made to the Crypto application.

+
Crypto 3.7.4 + +
Fixed Bugs and Malfunctions + + +

+ Fix a bug with AES CFB 128 for 192 and 256 bit keys. + Thanks to kellymclaughlin !

+

+ Own Id: OTP-14313 Aux Id: PR-1393

+ + +
+ +
+
Crypto 3.7.3
Improvements and New Features -- cgit v1.2.3