From 7c58f014e86f6014e8be53623b7dccef2124df78 Mon Sep 17 00:00:00 2001 From: Billy Brumley Date: Mon, 8 Jun 2020 17:36:10 +0300 Subject: [PATCH] GOST key agreement cofactor fix (#265) * GOST key agreement cofactor fix (cherry picked from commit dbc8f4780fa78d66a68174f78f9ae9aa9cdad53c) --- gost_ec_keyx.c | 66 ++++++++++++++++++++++++++------------------------ test/04-pkey.t | 24 +++++++++++++++--- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/gost_ec_keyx.c b/gost_ec_keyx.c index de52dec..85a6998 100644 --- a/gost_ec_keyx.c +++ b/gost_ec_keyx.c @@ -24,16 +24,16 @@ int VKO_compute_key(unsigned char *shared_key, const int vko_dgst_nid) { unsigned char *databuf = NULL; - BIGNUM *UKM = NULL, *p = NULL, *order = NULL, *X = NULL, *Y = NULL, *cofactor = NULL; - const BIGNUM *key = EC_KEY_get0_private_key(priv_key); - EC_POINT *pnt = EC_POINT_new(EC_KEY_get0_group(priv_key)); - BN_CTX *ctx = BN_CTX_secure_new(); + BIGNUM *scalar = NULL, *order = NULL, *X = NULL, *Y = NULL; + const EC_GROUP *grp = NULL; + EC_POINT *pnt = NULL; + BN_CTX *ctx = NULL; EVP_MD_CTX *mdctx = NULL; const EVP_MD *md = NULL; int buf_len, half_len; int ret = 0; - if (!ctx) { + if ((ctx = BN_CTX_secure_new()) == NULL) { GOSTerr(GOST_F_VKO_COMPUTE_KEY, ERR_R_MALLOC_FAILURE); return 0; } @@ -45,30 +45,40 @@ int VKO_compute_key(unsigned char *shared_key, goto err; } - UKM = BN_lebin2bn(ukm, ukm_size, NULL); - p = BN_CTX_get(ctx); - order = BN_CTX_get(ctx); - cofactor = BN_CTX_get(ctx); + grp = EC_KEY_get0_group(priv_key); + scalar = BN_CTX_get(ctx); + order = BN_CTX_get(ctx); X = BN_CTX_get(ctx); - Y = BN_CTX_get(ctx); - EC_GROUP_get_order(EC_KEY_get0_group(priv_key), order, ctx); - EC_GROUP_get_cofactor(EC_KEY_get0_group(priv_key), cofactor, ctx); - BN_mod_mul(UKM, UKM, cofactor, order, ctx); - BN_mod_mul(p, key, UKM, order, ctx); - if (!EC_POINT_mul(EC_KEY_get0_group(priv_key), pnt, NULL, pub_key, p, ctx)) { + + if ((Y = BN_CTX_get(ctx)) == NULL + || (pnt = EC_POINT_new(grp)) == NULL + || BN_lebin2bn(ukm, ukm_size, scalar) == NULL + || !BN_mod_mul(scalar, scalar, EC_KEY_get0_private_key(priv_key), + EC_GROUP_get0_order(grp), ctx)) + goto err; + + /* these two curves have cofactor 4; the rest have cofactor 1 */ + switch (EC_GROUP_get_curve_name(grp)) { + case NID_id_tc26_gost_3410_2012_256_paramSetA: + case NID_id_tc26_gost_3410_2012_512_paramSetC: + if (!BN_lshift(scalar, scalar, 2)) + goto err; + break; + } + + if (!EC_POINT_mul(grp, pnt, NULL, pub_key, scalar, ctx)) { GOSTerr(GOST_F_VKO_COMPUTE_KEY, GOST_R_ERROR_POINT_MUL); goto err; } - if (!EC_POINT_get_affine_coordinates(EC_KEY_get0_group(priv_key), - pnt, X, Y, ctx)) { - GOSTerr(GOST_F_VKO_COMPUTE_KEY, ERR_R_EC_LIB); - goto err; + if (!EC_POINT_get_affine_coordinates(grp, pnt, X, Y, ctx) || + !EC_GROUP_get_order(grp, order, ctx)) { + GOSTerr(GOST_F_VKO_COMPUTE_KEY, ERR_R_EC_LIB); + goto err; } half_len = BN_num_bytes(order); buf_len = 2 * half_len; - databuf = OPENSSL_zalloc(buf_len); - if (!databuf) { + if ((databuf = OPENSSL_malloc(buf_len)) == NULL) { GOSTerr(GOST_F_VKO_COMPUTE_KEY, ERR_R_MALLOC_FAILURE); goto err; } @@ -76,13 +86,11 @@ int VKO_compute_key(unsigned char *shared_key, /* * Serialize elliptic curve point same way as we do it when saving key */ - store_bignum(Y, databuf, half_len); - store_bignum(X, databuf + half_len, half_len); - /* And reverse byte order of whole buffer */ - BUF_reverse(databuf, NULL, buf_len); + if (BN_bn2lebinpad(X, databuf, half_len) != half_len + || BN_bn2lebinpad(Y, databuf + half_len, half_len) != half_len) + goto err; - mdctx = EVP_MD_CTX_new(); - if (!mdctx) { + if ((mdctx = EVP_MD_CTX_new()) == NULL) { GOSTerr(GOST_F_VKO_COMPUTE_KEY, ERR_R_MALLOC_FAILURE); goto err; } @@ -93,14 +101,10 @@ int VKO_compute_key(unsigned char *shared_key, ret = (EVP_MD_size(md) > 0) ? EVP_MD_size(md) : 0; err: - BN_free(UKM); BN_CTX_end(ctx); BN_CTX_free(ctx); - EC_POINT_free(pnt); - EVP_MD_CTX_free(mdctx); - OPENSSL_free(databuf); return ret; diff --git a/test/04-pkey.t b/test/04-pkey.t index 5065745..f5e654d 100644 --- a/test/04-pkey.t +++ b/test/04-pkey.t @@ -226,7 +226,10 @@ MD4CAQAwFwYIKoUDBwEBAQEwCwYJKoUDBwECAQEBBCD5+u2ebYwQ9iDYWHmif4XeGgj2OijJuq4YsbTN MD4CAQAwFwYIKoUDBwEBAQEwCwYJKoUDBwECAQEBBCDVwXdvq1zdBBmzVjG1WOBQR/dkwCzF6KSIiVkfQVCsKg== -----END PRIVATE KEY-----', 'c019d8939e12740a328625cea86efa3b39170412772b3c110536410bdd58a854', -'e9f7c57547fa0cd3c9942c62f9c74a553626d5f9810975a476825cd6f22a4e86'], +'e9f7c57547fa0cd3c9942c62f9c74a553626d5f9810975a476825cd6f22a4e86', +'-----BEGIN PUBLIC KEY----- +MF4wFwYIKoUDBwEBAQEwCwYJKoUDBwECAQEBA0MABEB3WS+MEcXnrMCdavPRgF28U5PDlV1atDh1ADUFxoB/f80OjqQ0T7cGQtk/2nWCGDX7uUrBGA8dql8Bnw9Sgn5+ +-----END PUBLIC KEY-----'], 'id-tc26-gost-3410-2012-256-paramSetB'=> ['-----BEGIN PRIVATE KEY----- MD4CAQAwFwYIKoUDBwEBAQEwCwYJKoUDBwECAQECBCDQ6G51VK2+96rvFyG/dRqWOFNJA33jQajAnzra585aIA== @@ -286,11 +289,14 @@ MF4CAQAwFwYIKoUDBwEBAQIwCwYJKoUDBwECAQIDBEA79FKW7MqF4pQJJvpAhKd9YkwsFXBzcaUhYt3N MF4CAQAwFwYIKoUDBwEBAQIwCwYJKoUDBwECAQIDBEAiCNNQAMnur4EG8eSDpr5WjJaoHquSsK3wydCrGM3Cdbaa0kiuj5m0Mx16Vow7AwvG2DvlKJL8HgwuBqWlDaYa -----END PRIVATE KEY-----', '6e1db0da8832660fbf761119e41d356a1599686a157c9a598b8e18b56cb09791', -'2df0dfa8d437689d41fad965f13ea28ce27c29dd84514b376ea6ad9f0c7e3ece'] +'2df0dfa8d437689d41fad965f13ea28ce27c29dd84514b376ea6ad9f0c7e3ece', +'-----BEGIN PUBLIC KEY----- +MIGgMBcGCCqFAwcBAQECMAsGCSqFAwcBAgECAwOBhAAEgYCPdAER26Ym73DSUXBamTLJcntdV3oZ7RRx/+Ijf13GnF36o36i8tEC13uJqOOmujEkAGPtui6yE4iJNVU0uM6yHmIEM5H0c81Sd/VQD8yXW1hyGAZvTMc+U/6oa30YU9YY7+t759d1CIVznPmq9C+VbAApyDCMFjuYnKD/nChsGA== +-----END PUBLIC KEY-----'] ); - plan(52); + plan(54); while(my($id, $v) = each %derives) { - my ($alice,$alicehash,$bob,$bobhash,$secrethash) = @$v; + my ($alice,$alicehash,$bob,$bobhash,$secrethash,$malice) = @$v; # Alice: keygen open $F,">",'alice.prv'; print $F $alice; @@ -309,6 +315,14 @@ MF4CAQAwFwYIKoUDBwEBAQIwCwYJKoUDBwECAQIDBEAiCNNQAMnur4EG8eSDpr5WjJaoHquSsK3wydCr # Bob: derive system("openssl pkeyutl -derive -inkey bob.prv -keyform PEM -peerkey alice.pub.der -peerform DER -pkeyopt ukmhex:0100000000000000 -out secret_b.bin"); like(`openssl dgst -sha256 -r secret_b.bin`, qr/^$secrethash/, "Compute shared key:$id:Bob"); + if ($malice ne "") { + # Malice: negative test -- this PEM is in the small subgroup + open $F,">",'malice.pub'; + print $F $malice; + close $F; + # NB system should return true on failure, so this is a negative test + ok(system("openssl pkeyutl -derive -inkey bob.prv -keyform PEM -peerkey malice.pub -peerform PEM -pkeyopt ukmhex:0100000000000000 -out secret_m.bin"), "Compute shared key:$id:Malice"); + } } unlink "alice.prv"; unlink "alice.pub.der"; @@ -316,6 +330,8 @@ MF4CAQAwFwYIKoUDBwEBAQIwCwYJKoUDBwECAQIDBEAiCNNQAMnur4EG8eSDpr5WjJaoHquSsK3wydCr unlink "bob.pub.der"; unlink "secret_a.bin"; unlink "secret_b.bin"; + unlink "malice.pub"; + unlink "secret_m.bin"; }; # 10. Разобрать стандартый encrypted key -- 2.39.2