The branch master has been updated via f0f4a46c4f5c82d4d9d0fb8a51d546c3135668a2 (commit) via e70abb8b4cb3b6259812137f72efa100797bca22 (commit) via 56f0237938c7e99d04f004886d56cb76514c4d56 (commit) from 32b1da718d5d6f35fcef82f3794273807d6202e9 (commit)
- Log ----------------------------------------------------------------- commit f0f4a46c4f5c82d4d9d0fb8a51d546c3135668a2 Author: Nicola Tuveri <nic....@gmail.com> Date: Sun May 9 14:57:14 2021 +0300 FIPS checksums update Reviewed-by: Tomas Mraz <to...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/15108) commit e70abb8b4cb3b6259812137f72efa100797bca22 Author: Theo Buehler <t...@openbsd.org> Date: Sat May 1 13:09:10 2021 +0200 Test oct2point for hybrid point encoding of (0, y) Reviewed-by: Nicola Tuveri <nic....@gmail.com> Reviewed-by: Tomas Mraz <to...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/15108) commit 56f0237938c7e99d04f004886d56cb76514c4d56 Author: Theo Buehler <t...@openbsd.org> Date: Sat May 1 12:25:50 2021 +0200 Avoid division by zero in hybrid point encoding In hybrid and compressed point encodings, the form octet contains a bit of information allowing to calculate y from x. For a point on a binary curve, this bit is zero if x is zero, otherwise it must match the rightmost bit of of the field element y / x. The existing code only considers the second possibility. It could thus incorrecly fail with a division by zero error as found by Guido Vranken's cryptofuzz. This commit adds a few explanatory comments to oct2point. The only actual code change is in the last hunk which adds a BN_is_zero(x) check to avoid the division by zero. Fixes #15021 Reviewed-by: Nicola Tuveri <nic....@gmail.com> Reviewed-by: Tomas Mraz <to...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/15108) ----------------------------------------------------------------------- Summary of changes: crypto/ec/ec2_oct.c | 41 +++++++++++++++++++++++++------- providers/fips-sources.checksums | 2 +- providers/fips.checksum | 2 +- test/ectest.c | 50 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 10 deletions(-) diff --git a/crypto/ec/ec2_oct.c b/crypto/ec/ec2_oct.c index 9f6e5de6fd..1970efd65c 100644 --- a/crypto/ec/ec2_oct.c +++ b/crypto/ec/ec2_oct.c @@ -270,9 +270,21 @@ int ossl_ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, ERR_raise(ERR_LIB_EC, EC_R_BUFFER_TOO_SMALL); return 0; } - form = buf[0]; - y_bit = form & 1; - form = form & ~1U; + + /* + * The first octet is the point converison octet PC, see X9.62, page 4 + * and section 4.4.2. It must be: + * 0x00 for the point at infinity + * 0x02 or 0x03 for compressed form + * 0x04 for uncompressed form + * 0x06 or 0x07 for hybrid form. + * For compressed or hybrid forms, we store the last bit of buf[0] as + * y_bit and clear it from buf[0] so as to obtain a POINT_CONVERSION_*. + * We error if buf[0] contains any but the above values. + */ + y_bit = buf[0] & 1; + form = buf[0] & ~1U; + if ((form != 0) && (form != POINT_CONVERSION_COMPRESSED) && (form != POINT_CONVERSION_UNCOMPRESSED) && (form != POINT_CONVERSION_HYBRID)) { @@ -284,6 +296,7 @@ int ossl_ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, return 0; } + /* The point at infinity is represented by a single zero octet. */ if (form == 0) { if (len != 1) { ERR_raise(ERR_LIB_EC, EC_R_INVALID_ENCODING); @@ -337,11 +350,23 @@ int ossl_ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, goto err; } if (form == POINT_CONVERSION_HYBRID) { - if (!group->meth->field_div(group, yxi, y, x, ctx)) - goto err; - if (y_bit != BN_is_odd(yxi)) { - ERR_raise(ERR_LIB_EC, EC_R_INVALID_ENCODING); - goto err; + /* + * Check that the form in the encoding was set correctly + * according to X9.62 4.4.2.a, 4(c), see also first paragraph + * of X9.62, 4.4.1.b. + */ + if (BN_is_zero(x)) { + if (y_bit != 0) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_ENCODING); + goto err; + } + } else { + if (!group->meth->field_div(group, yxi, y, x, ctx)) + goto err; + if (y_bit != BN_is_odd(yxi)) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_ENCODING); + goto err; + } } } diff --git a/providers/fips-sources.checksums b/providers/fips-sources.checksums index 0ab5e40394..49535d99e5 100644 --- a/providers/fips-sources.checksums +++ b/providers/fips-sources.checksums @@ -140,7 +140,7 @@ eaa940893610f5ec1cc04f5b1842bfa0ba65bf048039e6cc2d2b83bbb575bb51 crypto/ec/curv a1211ed3991af967c728b9f6d0774b9ea098d43cef0631ff88984a2580d2ac4f crypto/ec/curve448/eddsa.c d4969259e4fa5b71d8abbf5e736e658bd1daad6e46d272a9b88e190e2de96b61 crypto/ec/curve448/f_generic.c 7aeddfe47959556f50856cb387d74b51d222c65f891acb83742313ddc49c0e93 crypto/ec/curve448/scalar.c -ed003170c5eaaaa4a33f4ef37b43465f2ba7a5fa5fec2d7d17c1e0897ea818d7 crypto/ec/ec2_oct.c +04f8d52acc6332bdf879bf1684e8c59d2f4d8ca303d16c74d87aab3dd4a94932 crypto/ec/ec2_oct.c 7579a156234dfa44e02d08e121f42035229364f9e40f38b11333edbae2282762 crypto/ec/ec2_smpl.c 69d64accd498583e65df2dc43730eee2922217a7bfefda2cd1a9da176e3d1dcd crypto/ec/ec_asn1.c 8cf8af8e9bfc29e0cdc41720ec4a6d6c74eb5c15a9fc8193f8ec8270c0df1d37 crypto/ec/ec_backend.c diff --git a/providers/fips.checksum b/providers/fips.checksum index cbb359f123..2f3dff8cfc 100644 --- a/providers/fips.checksum +++ b/providers/fips.checksum @@ -1 +1 @@ -db2202782291f6e77fbe9f6271517cb41d7c06790a606a61f69e564f002f76f5 providers/fips-sources.checksums +5a2795b0bfeec67d234e9cf05bbac1571f205ba2da7e378e81b6e105fec1c85b providers/fips-sources.checksums diff --git a/test/ectest.c b/test/ectest.c index 8b737149d8..f58cd4e4bc 100644 --- a/test/ectest.c +++ b/test/ectest.c @@ -1083,6 +1083,55 @@ err: BN_free(yplusone); return r; } + +static int hybrid_point_encoding_test(void) +{ + BIGNUM *x = NULL, *y = NULL; + EC_GROUP *group = NULL; + EC_POINT *point = NULL; + unsigned char *buf = NULL; + size_t len; + int r = 0; + + if (!TEST_true(BN_dec2bn(&x, "0")) + || !TEST_true(BN_dec2bn(&y, "1")) + || !TEST_ptr(group = EC_GROUP_new_by_curve_name(NID_sect571k1)) + || !TEST_ptr(point = EC_POINT_new(group)) + || !TEST_true(EC_POINT_set_affine_coordinates(group, point, x, y, NULL)) + || !TEST_size_t_ne(0, (len = EC_POINT_point2oct(group, + point, + POINT_CONVERSION_HYBRID, + NULL, + 0, + NULL))) + || !TEST_ptr(buf = OPENSSL_malloc(len)) + || !TEST_size_t_eq(len, EC_POINT_point2oct(group, + point, + POINT_CONVERSION_HYBRID, + buf, + len, + NULL))) + goto err; + + r = 1; + + /* buf contains a valid hybrid point, check that we can decode it. */ + if (!TEST_true(EC_POINT_oct2point(group, point, buf, len, NULL))) + r = 0; + + /* Flip the y_bit and verify that the invalid encoding is rejected. */ + buf[0] ^= 1; + if (!TEST_false(EC_POINT_oct2point(group, point, buf, len, NULL))) + r = 0; + +err: + BN_free(x); + BN_free(y); + EC_GROUP_free(group); + EC_POINT_free(point); + OPENSSL_free(buf); + return r; +} #endif static int internal_curve_test(int n) @@ -2929,6 +2978,7 @@ int setup_tests(void) ADD_ALL_TESTS(cardinality_test, crv_len); ADD_TEST(prime_field_tests); #ifndef OPENSSL_NO_EC2M + ADD_TEST(hybrid_point_encoding_test); ADD_TEST(char2_field_tests); ADD_ALL_TESTS(char2_curve_test, OSSL_NELEM(char2_curve_tests)); #endif