From 806e15539e03d3bcf28064a1619ff0c7c365ebb4 Mon Sep 17 00:00:00 2001 From: Zachary James Williamson Date: Mon, 8 Aug 2022 09:25:07 +0100 Subject: [PATCH 1/2] schnorr k parameter now derived from 512-bit int not 256-bit (#1018) * schnorr k parameter now derived from 512-bit int not 256-bit * replaced `get_field_from_byte_buffer` with `get_unbiased_field_from_hmac` Added HMAC to ecdsa Co-authored-by: Ariel * include fix Co-authored-by: Ariel Co-authored-by: arielgabizon --- src/aztec/crypto/ecdsa/ecdsa_impl.hpp | 8 +++++- src/aztec/crypto/hmac/hmac.hpp | 38 +++++++++++++++++++++++++++ src/aztec/crypto/schnorr/schnorr.tcc | 5 ++-- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/aztec/crypto/ecdsa/ecdsa_impl.hpp b/src/aztec/crypto/ecdsa/ecdsa_impl.hpp index d9ac8710d47..9be623aeef3 100644 --- a/src/aztec/crypto/ecdsa/ecdsa_impl.hpp +++ b/src/aztec/crypto/ecdsa/ecdsa_impl.hpp @@ -2,6 +2,7 @@ #include #include +#include "../hmac/hmac.hpp" namespace crypto { namespace ecdsa { @@ -10,7 +11,12 @@ template signature construct_signature(const std::string& message, const key_pair& account) { signature sig; - Fr k = Fr::random_element(); // TODO replace with HMAC + + // use HMAC in PRF mode to derive 32-byte secret `k` + std::vector pkey_buffer; + write(pkey_buffer, account.private_key); + Fr k = crypto::get_unbiased_field_from_hmac(message, pkey_buffer); + typename G1::affine_element R(G1::one * k); Fq::serialize_to_buffer(R.x, &sig.r[0]); diff --git a/src/aztec/crypto/hmac/hmac.hpp b/src/aztec/crypto/hmac/hmac.hpp index efe0fbd3427..30b3a679415 100644 --- a/src/aztec/crypto/hmac/hmac.hpp +++ b/src/aztec/crypto/hmac/hmac.hpp @@ -4,6 +4,7 @@ #include #include #include +#include namespace crypto { /** @@ -69,4 +70,41 @@ std::array hmac(const MessageContainer& message, con return result; } +/** + * @brief Takes a size-HASH_OUTPUT buffer from HMAC and converts into a field element + * + * @details We assume HASH_OUTPUT = 32, which is insufficient entropy. We hash input with `0` and `1` to produce 64 + * bytes of input data. This is then converted into a uin512_t, which is taken modulo Fr::modulus to produce our field + * element. + * + * @tparam Hash the hash function we're using + * @tparam Fr field type + * @param input the input buffer + * @return Fr output field element + */ +template +Fr get_unbiased_field_from_hmac(const MessageContainer& message, const KeyContainer& key) +{ + auto input = hmac(message, key); + + std::vector lo_buffer(input.begin(), input.end()); + lo_buffer.push_back(0); + std::vector hi_buffer(input.begin(), input.end()); + hi_buffer.push_back(1); + + auto klo = Hash::hash(lo_buffer); + auto khi = Hash::hash(hi_buffer); + + std::vector full_buffer(khi.begin(), khi.end()); + for (auto& v : klo) { + full_buffer.push_back(v); + } + + uint512_t field_as_u512; + const uint8_t* ptr = &full_buffer[0]; + numeric::read(ptr, field_as_u512); + + Fr result((field_as_u512 % Fr::modulus).lo); + return result; +} } // namespace crypto \ No newline at end of file diff --git a/src/aztec/crypto/schnorr/schnorr.tcc b/src/aztec/crypto/schnorr/schnorr.tcc index 1fec0334a3f..f5c69c5a527 100644 --- a/src/aztec/crypto/schnorr/schnorr.tcc +++ b/src/aztec/crypto/schnorr/schnorr.tcc @@ -6,6 +6,7 @@ #include #include + namespace crypto { namespace schnorr { @@ -33,9 +34,9 @@ signature construct_signature(const std::string& message, const key_pair // use HMAC in PRF mode to derive 32-byte secret `k` std::vector pkey_buffer; write(pkey_buffer, private_key); - std::array k_buffer = crypto::hmac(message, pkey_buffer); - Fr k = Fr::serialize_from_buffer(&k_buffer[0]); + Fr k = crypto::get_unbiased_field_from_hmac(message, pkey_buffer); + typename G1::affine_element R(G1::one * k); std::vector message_buffer; From 1d321feff41f9ebed7e84922c164ecd044cb2412 Mon Sep 17 00:00:00 2001 From: Innokentii Sennovskii Date: Mon, 8 Aug 2022 17:31:43 +0100 Subject: [PATCH 2/2] Fixed equality operator in affine_element (#1191) * Fixed equality operator in affine_element * Added regression test. * Fix typo in comment. Co-authored-by: codygunton --- src/aztec/ecc/groups/affine_element.test.cpp | 17 +++++++++++++++-- src/aztec/ecc/groups/affine_element_impl.hpp | 7 +++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/aztec/ecc/groups/affine_element.test.cpp b/src/aztec/ecc/groups/affine_element.test.cpp index 1b43f81f032..eecde30ac63 100644 --- a/src/aztec/ecc/groups/affine_element.test.cpp +++ b/src/aztec/ecc/groups/affine_element.test.cpp @@ -3,9 +3,11 @@ #include #include +namespace test_affine_element { + using namespace barretenberg; -TEST(AffineElement, ReadWriteBuffer) +TEST(affine_element, read_write_buffer) { g1::affine_element P = g1::affine_element(g1::element::random_element()); g1::affine_element Q; @@ -22,4 +24,15 @@ TEST(AffineElement, ReadWriteBuffer) ASSERT_FALSE(P == Q); ASSERT_TRUE(P == R); -} \ No newline at end of file +} + +// Regression test to ensure that the point at infinity is not equal to its coordinate-wise reduction, which may lie +// on the curve, depending on the y-coordinate. +TEST(affine_element, infinity_regression) +{ + g1::affine_element P; + P.self_set_infinity(); + g1::affine_element R(0, P.y); + ASSERT_FALSE(P == R); +} +} // namespace test_affine_element \ No newline at end of file diff --git a/src/aztec/ecc/groups/affine_element_impl.hpp b/src/aztec/ecc/groups/affine_element_impl.hpp index 3a064d7e9c4..b6b6f622495 100644 --- a/src/aztec/ecc/groups/affine_element_impl.hpp +++ b/src/aztec/ecc/groups/affine_element_impl.hpp @@ -123,8 +123,11 @@ template constexpr bool affine_element: template constexpr bool affine_element::operator==(const affine_element& other) const noexcept { - bool both_infinity = is_point_at_infinity() && other.is_point_at_infinity(); - return both_infinity || ((x == other.x) && (y == other.y)); + bool this_is_infinity = is_point_at_infinity(); + bool other_is_infinity = other.is_point_at_infinity(); + bool both_infinity = this_is_infinity && other_is_infinity; + bool only_one_is_infinity = this_is_infinity != other_is_infinity; + return !only_one_is_infinity && (both_infinity || ((x == other.x) && (y == other.y))); } /**