Follow-up comments to hash_to_scalar. See https://boringssl-review.googlesource.com/c/boringssl/+/40646/3#message-ee607e82b0c62dd73a1b8a81f03acd9329cbbf02 Additionally, to be consistent with hash_to_field, we ought to use a big-endian value. It's also probably time to have some common functions for dealing with converting BN_ULONG[]s to and from big-endian bytes. Coding all those free-handed is a little tedious and error-prone. Change-Id: I6bdcd9362cee60e160e5a8eca25206b052206e1f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40647 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ec_extra/hash_to_curve.c b/crypto/ec_extra/hash_to_curve.c index 5c0e2e5..5c454de 100644 --- a/crypto/ec_extra/hash_to_curve.c +++ b/crypto/ec_extra/hash_to_curve.c
@@ -113,26 +113,6 @@ return ret; } -// reduce_to_felem implements step 7 of hash_to_field, described in section 5.2 -// of draft-irtf-cfrg-hash-to-curve-06. -static void reduce_to_felem(const EC_GROUP *group, EC_FELEM *out, - const uint8_t *in, size_t len) { - union { - BN_ULONG words[2 * EC_MAX_WORDS]; - uint8_t bytes[2 * EC_MAX_BYTES]; - } buf; - OPENSSL_memset(&buf, 0, sizeof(buf)); - - // |in| is encoded in big-endian. - assert(len <= sizeof(buf.bytes)); - for (size_t i = 0; i < len; i++) { - buf.bytes[len - i - 1] = in[i]; - } - - size_t num_words = group->field.width; - group->meth->felem_reduce(group, out, buf.words, num_words * 2); -} - // num_bytes_to_derive determines the number of bytes to derive when hashing to // a number modulo |modulus|. See the hash_to_field operation defined in // section 5.2 of draft-irtf-cfrg-hash-to-curve-06. @@ -153,8 +133,22 @@ return 1; } +// big_endian_to_words decodes |in| as a big-endian integer and writes the +// result to |out|. |num_words| must be large enough to contain the output. +static void big_endian_to_words(BN_ULONG *out, size_t num_words, + const uint8_t *in, size_t len) { + assert(len <= num_words * sizeof(BN_ULONG)); + // Ensure any excess bytes are zeroed. + OPENSSL_memset(out, 0, num_words * sizeof(BN_ULONG)); + uint8_t *out_u8 = (uint8_t *)out; + for (size_t i = 0; i < len; i++) { + out_u8[len - 1 - i] = in[i]; + } +} + // hash_to_field implements the operation described in section 5.2 -// of draft-irtf-cfrg-hash-to-curve-06, with count = 2. +// of draft-irtf-cfrg-hash-to-curve-06, with count = 2. |k| is the security +// factor. static int hash_to_field2(const EC_GROUP *group, const EVP_MD *md, EC_FELEM *out1, EC_FELEM *out2, const uint8_t *dst, size_t dst_len, unsigned k, const uint8_t *msg, @@ -165,24 +159,31 @@ !expand_message_xmd(md, buf, 2 * L, msg, msg_len, dst, dst_len)) { return 0; } - reduce_to_felem(group, out1, buf, L); - reduce_to_felem(group, out2, buf + L, L); + BN_ULONG words[2 * EC_MAX_WORDS]; + size_t num_words = 2 * group->field.width; + big_endian_to_words(words, num_words, buf, L); + group->meth->felem_reduce(group, out1, words, num_words); + big_endian_to_words(words, num_words, buf + L, L); + group->meth->felem_reduce(group, out2, words, num_words); return 1; } // hash_to_scalar behaves like |hash_to_field2| but returns a value modulo the -// group order rather than a field element. +// group order rather than a field element. |k| is the security factor. static int hash_to_scalar(const EC_GROUP *group, const EVP_MD *md, EC_SCALAR *out, const uint8_t *dst, size_t dst_len, unsigned k, const uint8_t *msg, size_t msg_len) { size_t L; - BN_ULONG words[EC_MAX_WORDS * 2] = {0}; + uint8_t buf[EC_MAX_BYTES * 2]; if (!num_bytes_to_derive(&L, &group->order, k) || - !expand_message_xmd(md, (uint8_t *)words, L, msg, msg_len, dst, - dst_len)) { + !expand_message_xmd(md, buf, L, msg, msg_len, dst, dst_len)) { return 0; } - ec_scalar_reduce(group, out, words, 2 * group->order.width); + + BN_ULONG words[2 * EC_MAX_WORDS]; + size_t num_words = 2 * group->order.width; + big_endian_to_words(words, num_words, buf, L); + ec_scalar_reduce(group, out, words, num_words); return 1; }
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index eb53b2f..95b3db0 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -1215,13 +1215,13 @@ }; static const HashToScalarTest kTests[] = { {"P521_XMD:SHA-512_SCALAR_TEST", "", - "01407998b20d948d6ef4e68c981d24f44ed3e65a49849a16296770" - "14b48d4664e150074ccf9afcdf791c6afc648e69b94989881f1f0b" - "4e2b86ce40b1dc2ce4bb20f0"}, + "01a6206c2fc677c11d51807bf46d64a17f92396673074c5cee9299" + "4d28eec5445d5ed89799b30b39c964ecf62f39d59e7d43de15d910" + "c2c1d69f3ebc01eab241e5dc"}, {"P521_XMD:SHA-512_SCALAR_TEST", "abcdef0123456789", - "019fab7021eeae5476d7ae7352793025a9aed0193831a42cbcd183" - "e377a83d33ee178e11f34f9b6cffeffdee40c9260e5aff50ebf276" - "c992b78d086dd4475d7b098e"}, + "00af484a5d9389a9912f555234c578d4b1b7c4a6f5009018d133a4" + "069172c9f5ce2d853b8643fe7bb50a83427ed3520a7a793c41a455" + "a02aa99431434fb6b5b0b26e"}, {"P521_XMD:SHA-512_SCALAR_TEST", "a512_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" @@ -1233,9 +1233,9 @@ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "00ab2c0feabe9bbd93d4570fe627fa88667bb8f2f117e76b77d41a" - "15bb5dd995f61c64cd70a96dc9cda1f70b426dfd7a1c11a2865272" - "f4698f501e57f8c4c2ed0008"}, + "00b2db2ceb64ad055cafc5a0fc92560525d6dcc4975b86bbb79013" + "a1c3ab5d412320cb55df8088a658039a70c5657d5aefaaaa81cc5d" + "eecdd40c03eb0517fe2e158c"}, }; for (const auto &test : kTests) {