aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKelly McLaughlin <[email protected]>2017-03-29 07:24:46 -0600
committerKelly McLaughlin <[email protected]>2017-03-29 07:24:46 -0600
commit59099922f53a478903da304cc591c4baae549dc5 (patch)
treefcff54ac6d7ab54ff20de24fde3b97697e5b7e94
parenta748cafdc7063d9f181ba12088db6458793ced2f (diff)
downloadotp-59099922f53a478903da304cc591c4baae549dc5.tar.gz
otp-59099922f53a478903da304cc591c4baae549dc5.tar.bz2
otp-59099922f53a478903da304cc591c4baae549dc5.zip
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.
-rw-r--r--lib/crypto/c_src/crypto.c27
-rw-r--r--lib/crypto/src/crypto.erl6
-rw-r--r--lib/crypto/test/crypto_SUITE.erl10
3 files changed, 42 insertions, 1 deletions
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),