aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHans Nilsson <[email protected]>2018-04-26 14:58:45 +0200
committerHans Nilsson <[email protected]>2018-04-26 14:58:45 +0200
commit9779a7fe2dd318f9b1e5a2d4b3b4e413200d0f9c (patch)
tree24aee1efa92c60af6ce55002e07846c9bb39b64e
parent0183ddffb112d898d9b8c0396afa7f2210b9e169 (diff)
parent93ac4697e3289f0fd1623a4b44b312f5196d91c5 (diff)
downloadotp-9779a7fe2dd318f9b1e5a2d4b3b4e413200d0f9c.tar.gz
otp-9779a7fe2dd318f9b1e5a2d4b3b4e413200d0f9c.tar.bz2
otp-9779a7fe2dd318f9b1e5a2d4b3b4e413200d0f9c.zip
Merge branch 'hans/crypto/EVP_DH_key/OTP-14864'
* hans/crypto/EVP_DH_key/OTP-14864: crypto: Test case with a failing Pub/Priv/P/G combination This quadruple is from a failing test when trying to EVP-ify the dh functions. crypto: Use EVP-api in dh_compute_key_nif and dh_generate_key_nif
-rw-r--r--lib/crypto/c_src/crypto.c175
-rw-r--r--lib/crypto/test/crypto_SUITE.erl21
2 files changed, 149 insertions, 47 deletions
diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c
index 149387bcee..dbb6bf8135 100644
--- a/lib/crypto/c_src/crypto.c
+++ b/lib/crypto/c_src/crypto.c
@@ -3004,16 +3004,21 @@ static ERL_NIF_TERM rsa_generate_key_nif(ErlNifEnv* env, int argc, const ERL_NIF
static ERL_NIF_TERM dh_generate_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{/* (PrivKey|undefined, DHParams=[P,G], Mpint, Len|0) */
- DH* dh_params;
+ DH* dh_params = NULL;
int pub_len, prv_len;
unsigned char *pub_ptr, *prv_ptr;
ERL_NIF_TERM ret, ret_pub, ret_prv, head, tail;
int mpint; /* 0 or 4 */
- BIGNUM *priv_key = NULL;
+ BIGNUM *priv_key_in = NULL;
BIGNUM *dh_p = NULL, *dh_g = NULL;
unsigned long len = 0;
+#ifdef HAS_EVP_PKEY_CTX
+ EVP_PKEY_CTX *ctx = NULL;
+ EVP_PKEY *dhkey = NULL,
+ *params = NULL;
+#endif
- if (!(get_bn_from_bin(env, argv[0], &priv_key)
+ if (!(get_bn_from_bin(env, argv[0], &priv_key_in)
|| argv[0] == atom_undefined)
|| !enif_get_list_cell(env, argv[1], &head, &tail)
|| !get_bn_from_bin(env, head, &dh_p)
@@ -3021,40 +3026,63 @@ static ERL_NIF_TERM dh_generate_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_
|| !get_bn_from_bin(env, head, &dh_g)
|| !enif_is_empty_list(env, tail)
|| !enif_get_int(env, argv[2], &mpint) || (mpint & ~4)
- || !enif_get_ulong(env, argv[3], &len) ) {
-
- if (priv_key) BN_free(priv_key);
+ || !enif_get_ulong(env, argv[3], &len)
+
+ /* Load dh_params with values to use by the generator.
+ Mem mgmnt transfered from dh_p etc to dh_params */
+ || !(dh_params = DH_new())
+ || (priv_key_in && !DH_set0_key(dh_params, NULL, priv_key_in))
+ || !DH_set0_pqg(dh_params, dh_p, NULL, dh_g)
+ ) {
+ if (priv_key_in) BN_free(priv_key_in);
if (dh_p) BN_free(dh_p);
if (dh_g) BN_free(dh_g);
+ if (dh_params) DH_free(dh_params);
return enif_make_badarg(env);
}
- dh_params = DH_new();
- DH_set0_key(dh_params, NULL, priv_key);
- DH_set0_pqg(dh_params, dh_p, NULL, dh_g);
-
if (len) {
if (len < BN_num_bits(dh_p))
DH_set_length(dh_params, len);
else {
- DH_free(dh_params);
+ if (priv_key_in) BN_free(priv_key_in);
+ if (dh_p) BN_free(dh_p);
+ if (dh_g) BN_free(dh_g);
+ if (dh_params) DH_free(dh_params);
return enif_make_badarg(env);
}
}
+#ifdef HAS_EVP_PKEY_CTX
+ if ((dhkey = EVP_PKEY_new())
+ && (params = EVP_PKEY_new())
+ && EVP_PKEY_set1_DH(params, dh_params) /* set the key referenced by params to dh_params.
+ dh_params (and params) must be freed by us*/
+ && (ctx = EVP_PKEY_CTX_new(params, NULL))
+ && EVP_PKEY_keygen_init(ctx)
+ && EVP_PKEY_keygen(ctx, &dhkey)
+ && (dh_params = EVP_PKEY_get1_DH(dhkey)) /* return the referenced key. dh_params and dhkey must be freed */
+ ) {
+#else
if (DH_generate_key(dh_params)) {
- const BIGNUM *pub_key, *priv_key;
- DH_get0_key(dh_params, &pub_key, &priv_key);
- pub_len = BN_num_bytes(pub_key);
- prv_len = BN_num_bytes(priv_key);
+#endif
+ const BIGNUM *pub_key_gen, *priv_key_gen;
+
+ DH_get0_key(dh_params,
+ &pub_key_gen, &priv_key_gen); /* Get pub_key_gen and priv_key_gen.
+ "The values point to the internal representation of
+ the public key and private key values. This memory
+ should not be freed directly." says man */
+ pub_len = BN_num_bytes(pub_key_gen);
+ prv_len = BN_num_bytes(priv_key_gen);
pub_ptr = enif_make_new_binary(env, pub_len+mpint, &ret_pub);
prv_ptr = enif_make_new_binary(env, prv_len+mpint, &ret_prv);
if (mpint) {
put_int32(pub_ptr, pub_len); pub_ptr += 4;
put_int32(prv_ptr, prv_len); prv_ptr += 4;
}
- BN_bn2bin(pub_key, pub_ptr);
- BN_bn2bin(priv_key, prv_ptr);
+ BN_bn2bin(pub_key_gen, pub_ptr);
+ BN_bn2bin(priv_key_gen, prv_ptr);
ERL_VALGRIND_MAKE_MEM_DEFINED(pub_ptr, pub_len);
ERL_VALGRIND_MAKE_MEM_DEFINED(prv_ptr, prv_len);
ret = enif_make_tuple2(env, ret_pub, ret_prv);
@@ -3062,21 +3090,33 @@ static ERL_NIF_TERM dh_generate_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_
else {
ret = atom_error;
}
+
DH_free(dh_params);
+#ifdef HAS_EVP_PKEY_CTX
+ if (ctx) EVP_PKEY_CTX_free(ctx);
+ /* if (dhkey) EVP_PKEY_free(dhkey); */
+ /* if (params) EVP_PKEY_free(params); */
+#endif
return ret;
}
static ERL_NIF_TERM dh_compute_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{/* (OthersPublicKey, MyPrivateKey, DHParams=[P,G]) */
- DH* dh_params;
- BIGNUM *dummy_pub_key = NULL, *priv_key = NULL;
- BIGNUM *other_pub_key;
- BIGNUM *dh_p = NULL, *dh_g = NULL;
- int i;
+ BIGNUM *dummy_pub_key = NULL,
+ *priv_key = NULL,
+ *other_pub_key = NULL,
+ *dh_p = NULL,
+ *dh_g = NULL;
ErlNifBinary ret_bin;
ERL_NIF_TERM ret, head, tail;
-
- dh_params = DH_new();
+ DH *dh_priv = DH_new(), *dh_pub = DH_new();
+#ifdef HAS_EVP_PKEY_CTX
+ EVP_PKEY_CTX *ctx = NULL;
+ EVP_PKEY *my_priv_key = NULL, *peer_pub_key = NULL;
+ size_t skeylen;
+#else
+ int i;
+#endif
if (!get_bn_from_bin(env, argv[0], &other_pub_key)
|| !get_bn_from_bin(env, argv[1], &priv_key)
@@ -3084,35 +3124,78 @@ static ERL_NIF_TERM dh_compute_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_T
|| !get_bn_from_bin(env, head, &dh_p)
|| !enif_get_list_cell(env, tail, &head, &tail)
|| !get_bn_from_bin(env, head, &dh_g)
- || !enif_is_empty_list(env, tail)) {
+ || !enif_is_empty_list(env, tail)
+
+ /* Note: DH_set0_key() does not allow setting only the
+ * private key, although DH_compute_key() does not use the
+ * public key. Work around this limitation by setting
+ * the public key to a copy of the private key.
+ */
+ || !(dummy_pub_key = BN_dup(priv_key))
+ || !DH_set0_key(dh_priv, dummy_pub_key, priv_key)
+ || !DH_set0_pqg(dh_priv, dh_p, NULL, dh_g)
+ ) {
if (dh_p) BN_free(dh_p);
if (dh_g) BN_free(dh_g);
- ret = enif_make_badarg(env);
+ if (other_pub_key) BN_free(other_pub_key);
+ if (dummy_pub_key) BN_free(dummy_pub_key);
+ if (priv_key) BN_free(priv_key);
+ return enif_make_badarg(env);
+ }
+
+#ifdef HAS_EVP_PKEY_CTX
+ if (!(my_priv_key = EVP_PKEY_new())
+ || !EVP_PKEY_set1_DH(my_priv_key, dh_priv) /* set the key referenced by my_priv_key to dh_priv.
+ dh_priv (and my_priv_key) must be freed by us*/
+
+ || !(peer_pub_key = EVP_PKEY_new())
+ || !DH_set0_key(dh_pub, other_pub_key, NULL)
+ || !DH_set0_pqg(dh_pub, dh_p, NULL, dh_g)
+ || !EVP_PKEY_set1_DH(peer_pub_key, dh_pub)
+
+ || !(ctx = EVP_PKEY_CTX_new(my_priv_key, NULL))
+ || (EVP_PKEY_derive_init(ctx) <= 0)
+ || (EVP_PKEY_derive_set_peer(ctx, peer_pub_key) <= 0)
+ || (EVP_PKEY_derive(ctx, NULL, &skeylen) <= 0)) {
+
+ ret = atom_error;
}
else {
- /* Note: DH_set0_key() does not allow setting only the
- * private key, although DH_compute_key() does not use the
- * public key. Work around this limitation by setting
- * the public key to a copy of the private key.
- */
- dummy_pub_key = BN_dup(priv_key);
- DH_set0_key(dh_params, dummy_pub_key, priv_key);
- DH_set0_pqg(dh_params, dh_p, NULL, dh_g);
- enif_alloc_binary(DH_size(dh_params), &ret_bin);
- i = DH_compute_key(ret_bin.data, other_pub_key, dh_params);
- if (i > 0) {
- if (i != ret_bin.size) {
- enif_realloc_binary(&ret_bin, i);
- }
- ret = enif_make_binary(env, &ret_bin);
- }
- else {
+ enif_alloc_binary(skeylen, &ret_bin);
+
+ if ((EVP_PKEY_derive(ctx, ret_bin.data, &skeylen) > 0)
+ && (ret_bin.size >= skeylen)) {
+ /* Derivation succeded */
+ if (ret_bin.size > skeylen) enif_realloc_binary(&ret_bin, skeylen);
+ ret = enif_make_binary(env, &ret_bin);
+ }
+ else {
enif_release_binary(&ret_bin);
- ret = atom_error;
- }
+ ret = atom_error;
+ }
}
+
+#else
+ enif_alloc_binary(DH_size(dh_priv), &ret_bin);
+ i = DH_compute_key(ret_bin.data, other_pub_key, dh_priv);
+ if (i > 0) {
+ if (i != ret_bin.size) enif_realloc_binary(&ret_bin, i);
+ ret = enif_make_binary(env, &ret_bin);
+ }
+ else {
+ enif_release_binary(&ret_bin);
+ ret = atom_error;
+ }
+#endif
+
if (other_pub_key) BN_free(other_pub_key);
- DH_free(dh_params);
+ if (dh_priv) DH_free(dh_priv);
+ if (dh_pub) DH_free(dh_pub);
+#ifdef HAS_EVP_PKEY_CTX
+ if (ctx) EVP_PKEY_CTX_free(ctx);
+ /* if (my_priv_key) EVP_PKEY_free(my_priv_key); */
+ /* if (peer_pub_key) EVP_PKEY_free(peer_pub_key); */
+#endif
return ret;
}
diff --git a/lib/crypto/test/crypto_SUITE.erl b/lib/crypto/test/crypto_SUITE.erl
index 6dab459df6..d148fa3856 100644
--- a/lib/crypto/test/crypto_SUITE.erl
+++ b/lib/crypto/test/crypto_SUITE.erl
@@ -131,7 +131,8 @@ groups() ->
{ecdsa, [], [sign_verify
%% Does not work yet: ,public_encrypt, private_encrypt
]},
- {dh, [], [generate_compute]},
+ {dh, [], [generate_compute,
+ compute_bug]},
{ecdh, [], [compute, generate]},
{srp, [], [generate_compute]},
{des_cbc, [], [block]},
@@ -463,6 +464,24 @@ generate_compute(Config) when is_list(Config) ->
GenCom = proplists:get_value(generate_compute, Config),
lists:foreach(fun do_generate_compute/1, GenCom).
%%--------------------------------------------------------------------
+compute_bug() ->
+ [{doc, "Test that it works even if the Secret is smaller than expected"}].
+compute_bug(Config) ->
+ ExpectedSecret = <<118,89,171,16,156,18,156,103,189,134,130,49,28,144,111,241,247,82,79,32,228,11,209,141,119,176,251,80,105,143,235,251,203,121,223,211,129,3,233,133,45,2,31,157,24,111,5,75,153,66,135,185,128,115,229,178,216,39,73,52,80,151,8,241,34,52,226,71,137,167,53,48,59,224,175,154,89,110,76,83,24,117,149,21,72,6,186,78,149,74,188,56,98,244,30,77,108,248,88,194,195,237,23,51,20,242,254,123,21,12,209,74,217,168,230,65,7,60,211,139,128,239,234,153,22,229,180,59,159,121,41,156,121,200,177,130,163,162,54,224,93,1,94,11,177,254,118,28,156,26,116,10,207,145,219,166,214,189,214,230,221,170,228,15,69,88,31,68,94,255,113,58,49,82,86,192,248,176,131,133,39,186,194,172,206,84,184,16,66,68,153,128,178,227,27,118,52,130,122,92,24,222,102,195,221,207,255,13,152,175,65,32,167,84,54,244,243,109,244,18,234,16,159,224,188,2,106,123,27,17,131,171,226,34,111,251,62,119,155,124,221,124,254,62,97,167,1,105,116,98,98,19,197,30,72,180,79,221,100,134,120,117,124,85,73,132,224,223,222,41,155,137,218,130,238,237,157,161,134,150,69,206,91,141,17,89,120,218,235,229,37,150,76,197,7,157,56,144,42,203,137,100,200,72,141,194,239,1,67,236,238,183,48,214,75,76,108,235,3,237,67,40,137,45,182,236,246,37,116,103,144,237,142,211,88,233,11,24,21,218,41,245,250,51,130,250,104,74,189,17,69,145,70,50,50,215,253,155,10,128,41,114,185,211,82,164,72,92,17,145,104,66,6,140,226,80,43,62,1,166,216,153,118,96,15,147,126,137,118,191,192,75,149,241,206,18,92,17,154,215,219,18,6,139,190,103,210,156,184,29,224,213,157,60,112,189,104,220,125,40,186,50,119,17,143,136,149,38,74,107,21,192,59,61,59,42,231,144,59,175,3,176,87,23,16,122,54,31,82,34,230,211,44,81,41,47,86,37,228,175,130,148,88,136,131,254,241,202,99,199,175,1,141,215,124,155,120,43,141,89,11,140,120,141,29,35,82,219,155,204,75,12,66,241,253,33,250,84,24,85,68,13,80,85,142,227,34,139,26,146,24>>,
+ OthersPublicKey = 635619632099733175381667940709387641100492974601603060984753028943194386334921787463327680809776598322996634648015962954045728174069768874873236397421720142610982770302060309928552098274817978606093380781524199673890631795310930242601197479471368910519338301177304682162189801040921618559902948819107531088646753320486728060005223263561551402855338732899079439899705951063999951507319258050864346087428042978411873495523439615429804957374639092580169417598963105885529553632847023899713490485619763926900318508906706745060947269748612049634207985438016935262521715769812475329234748426647554362991758104620357149045960316987533503707855364806010494793980069245562784050236811004893018183726397041999426883788660276453352521120006817370050691205529335316794439089316232980047277245051173281601960196573681285904611182521967067911862467395705665888521948321299521549941618586026714676885890192323289343756440666276226084448279082483536164085883288884231665240707495770544705648564889889198060417915693315346959170105413290799314390963124178046425737828369059171472978294050322371452255088799865552038756937873388385970088906560408959959429398326288750834357514847891423941047433478384621074116184703014798814515161475596555032391555842,
+ MyPrivateKey = 387759582879975726965038486537011291913744975764132199838375902680222019267527675651273586836110220500657652661706223760165097275862806031329642160439090779625708664007910974206651834216043397115514725827856461492311499129200688538220719685637154290305617686974719521885238198226075381217068175824097878445476010193039590876624464274744156624589136789060427283492343902761765833713520850870233407503430180028104167029073459918756981323130062648615262139444306321256382009848217866984408901761817655567071716275177768316006340055589170095799943481591033461616307776069027985761229636731465482676467627154100912586936231051371168178564599296638350391246393336702334311781595616786107810962134407697848002331639021101685320844880636050048769216986088652236979636019052557155807310341483407890060105599892252118584570558049301477535792498672552850760356632076013402382600669875697284264329434950712239302528367835155163504374877787288116104285944993818319105835423479332617802010952731990182088670508346704423006877514817882782443833997288652405892920173712497948376815825396272381214976859009518623799156300136570204539240675245115597412280078940442452936425561984312708387584800789375684525365060589104566195610526570099527133097201479,
+ P = 818034524162384276004384029858643530286875094391273833506734966261806257117433972760379103507630310628953496150318170372254219924175532996281953750642804369831900894594960807970232131410638888573275563720690293481410915588408505771183615664441221559618326229227448328879290185035795866796496147000467456347856187771645103733376761936369144682074588463621004219054111244232031965820058057143484947957179035662640791007685559024477920075136419228662974090561346329074935312181886940693299380892129818458511403741106419480550004799657220331973244248753744516935739033770420884365608406478656143540532371463443324228730693491647103274058971797182813283112583029849186056551355376851686616057869624968077484471229044125401535456699914745876082047459812392122562460031611344154642406382436701361983114768023990405077450124649862159757605118611426368650203370143674925598905779061402007525955196464201496773278952462368223659263492419274489020447849336502432222101793313731259141617677580646998184158969477474527427664187763741360356528830301163614618231141541403007931347398186427059736520580903587497382362610721261644208653717495736748724114113311672504064943864203789205551568648546606356374830209356446449765364678719909024329058480379,
+ G = 2,
+ DHParameters = [P, G],
+ case crypto:compute_key(dh, OthersPublicKey, MyPrivateKey, DHParameters) of
+ ExpectedSecret ->
+ ok;
+ Others ->
+ ct:log("Got ~p",[Others]),
+ {fail, "crypto:compute_key(dh,...) failed for the bug test"}
+ end.
+
+%%--------------------------------------------------------------------
no_generate_compute() ->
[{doc, "Test crypto:genarate_key and crypto:compute_key "
"for disabled algorithms"}].