Add HKDF_HMAC256_L32 and method to negate a private key #14047

pull jonasschnelli wants to merge 4 commits into bitcoin:master from jonasschnelli:2018/08/bip151_key_hkdf changing 7 files +128 −2
  1. jonasschnelli commented at 9:20 am on August 24, 2018: contributor

    This adds a limited implementation of HKDF (defined by rfc5869) that supports only HMAC-SHA256 and length output of 32 bytes (will be required for v2 transport protocol).

    This PR also includes a method to negate a private key which is useful to enforce public keys starting with 0x02 (or 0x03) (a requirement for the v2 transport protocol). The new CKey::Negate() method is pretty much a wrapper around secp256k1_ec_privkey_negate().

    Including tests.

    This is a subset of #14032 and a pre-requirement for the v2 transport protocol.

  2. DrahtBot commented at 12:15 pm on August 24, 2018: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15649 (Add ChaCha20Poly1305@Bitcoin AEAD by jonasschnelli)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. in src/crypto/hkdf_sha256_32.cpp:22 in 83e435f002 outdated 
    17+    // expand a 32byte key (single round)
18+    assert(info.size() <= 128);
19+    unsigned char data[129];
20+    memcpy(&data[0], &info[0], info.size());
21+    data[info.size()] = 0x01;
22+    CHMAC_SHA256(m_prk, 32).Write(&data[0], info.size() + 1).Finalize(out);
    sipa commented at 6:09 pm on August 26, 2018:
    You can avoid the copy to data if you use static const char one[1] = {1}; CHMAC_SHA256(m_prk, 32).Write(info.data(), info.size()).Write(one, 1).Finalize(out); instead.
    jonasschnelli commented at 11:42 am on August 27, 2018:
    Oh yes. That’s more efficient. Fixed.
  4. jonasschnelli force-pushed on Aug 27, 2018
  5. jonasschnelli referenced this in commit 955ac1d876 on Aug 29, 2018
  6. in src/test/crypto_tests.cpp:532 in dc02f90bd4 outdated 
    524@@ -524,6 +525,16 @@ BOOST_AUTO_TEST_CASE(chacha20_testvector)
525                  "fab78c9");
526 }
527 
528+BOOST_AUTO_TEST_CASE(hkdf_hmac_sha256_l32_tests)
529+{
530+    // rfc5869 has no test vectors for HMAC_SHA256 length 32, the one below was created with python HKDF and compared against openssl
531+    std::vector<unsigned char> key = ParseHex("7768617420646f2079612077616e7420666f72206e6f7468696e673fab76cfa9");
532+    CHKDF_HMAC_SHA256_L32 hkdf32(key.data(), key.size(), "bitcoinecdh");
    Sjors commented at 10:21 am on September 2, 2018:
    Why not use BitcoinSharedSecretas the salt, like in the BIP?
  7. in src/crypto/hkdf_sha256_32.h:22 in dc02f90bd4 outdated 
    17+    unsigned char m_prk[32];
18+    static const size_t OUTPUT_SIZE = 32;
19+
20+public:
21+    CHKDF_HMAC_SHA256_L32(const unsigned char* ikm, size_t ikmlen, const std::string& salt);
22+    void Expand32(const std::string& info, unsigned char hash[OUTPUT_SIZE]);
    practicalswift commented at 5:23 pm on September 10, 2018: 
    0src/./crypto/hkdf_sha256_32.h:22:10: warning: function 'CHKDF_HMAC_SHA256_L32::Expand32' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
    jonasschnelli commented at 7:34 pm on September 11, 2018:
    Thanks. Fixed.
  8. jonasschnelli force-pushed on Sep 11, 2018
  9. jonasschnelli force-pushed on Sep 18, 2018
  10. DrahtBot added the label Needs rebase on Nov 5, 2018
  11. jonasschnelli force-pushed on Mar 5, 2019
  12. jonasschnelli commented at 2:48 pm on March 5, 2019: contributor
    Rebased
  13. DrahtBot removed the label Needs rebase on Mar 5, 2019
  14. DrahtBot added the label Build system on Mar 19, 2019
  15. DrahtBot added the label Tests on Mar 19, 2019
  16. DrahtBot added the label Utils/log/libs on Mar 19, 2019
  17. in src/test/key_tests.cpp:201 in dd6c1b61be outdated 
    187@@ -188,4 +188,14 @@ BOOST_AUTO_TEST_CASE(key_signature_tests)
188     BOOST_CHECK(found_small);
189 }
190 
191+BOOST_AUTO_TEST_CASE(key_key_negation)
192+{
193+    CKey key = DecodeSecret(strSecret1C);
    sipa commented at 11:25 pm on March 25, 2019:
    The test here could be a bit more extensive (for example, signing/verifying a fixed message with the corresponding private key, and checking that the private key roundtrips after 2 negations).
    jonasschnelli commented at 5:10 pm on March 26, 2019:
    Good point. Extended the test to cover the double negation signature comparison.
  18. in src/test/crypto_tests.cpp:530 in 32633ff159 outdated 
    524@@ -524,6 +525,16 @@ BOOST_AUTO_TEST_CASE(chacha20_testvector)
525                  "fab78c9");
526 }
527 
528+BOOST_AUTO_TEST_CASE(hkdf_hmac_sha256_l32_tests)
529+{
530+    // rfc5869 has no test vectors for HMAC_SHA256 length 32, the one below was created with python HKDF and compared against openssl
    sipa commented at 11:33 pm on March 25, 2019:
    I think shorter-length HMAC_HKDF can be obtained by truncating the output of a longer-length one. So you could take all of the RFC’s test vectors and truncate them to 32 bytes?
    jonasschnelli commented at 5:09 pm on March 26, 2019:
    Good idea. Followed @sipa’s advice and changed to the 3 rfc5869 SHA256 test vectors (truncated OKM to 32 bytes).
  19. jonasschnelli force-pushed on Mar 26, 2019
  20. sipa commented at 10:50 pm on March 26, 2019: member
    utACK acf3debc6a2f8b80a5de1e71bc658086c6de77b4
  21. DrahtBot added the label Needs rebase on Mar 27, 2019
  22. CKey: add method to negate the key 463921bb64
  23. QA: add test for CKey::Negate() 3b64f852e4
  24. jonasschnelli force-pushed on Mar 27, 2019
  25. jonasschnelli commented at 4:40 pm on March 27, 2019: contributor
    rebased
  26. DrahtBot removed the label Needs rebase on Mar 27, 2019
  27. jonasschnelli added this to the "Blockers" column in a project
  28. in src/crypto/hkdf_sha256_32.cpp:12 in 2f75b3dcd2 outdated 
     7+#include <assert.h>
 8+#include <string.h>
 9+
10+CHKDF_HMAC_SHA256_L32::CHKDF_HMAC_SHA256_L32(const unsigned char* ikm, size_t ikmlen, const std::string& salt)
11+{
12+    CHMAC_SHA256(reinterpret_cast<const unsigned char*>(salt.c_str()), salt.size()).Write(ikm, ikmlen).Finalize(m_prk);
    sipa commented at 9:53 pm on May 10, 2019:
    Style nit: we don’t usually use the C++ cast operators for primitive types (just (const unsigned char*)salt.c_str() works).
  29. in src/crypto/hkdf_sha256_32.cpp:20 in 2f75b3dcd2 outdated 
    15+void CHKDF_HMAC_SHA256_L32::Expand32(const std::string& info, unsigned char hash[OUTPUT_SIZE])
16+{
17+    // expand a 32byte key (single round)
18+    assert(info.size() <= 128);
19+    static const unsigned char one[1] = {1};
20+    CHMAC_SHA256(m_prk, 32).Write(reinterpret_cast<const unsigned char*>(info.data()), info.size()).Write(one, 1).Finalize(hash);
    sipa commented at 9:54 pm on May 10, 2019:
    Same here.
  30. sipa commented at 9:56 pm on May 10, 2019: member
    utACK 65948ee3d41c33cd442a6572ab9e9d071499022d, just some nits.
  31. Add HKDF HMAC_SHA256 L=32 implementations 551d489416
  32. QA: add test for HKDF HMAC_SHA256 L32 8794a4b3ae
  33. jonasschnelli force-pushed on May 11, 2019
  34. jonasschnelli commented at 7:14 am on May 11, 2019: contributor
    Thanks for the review. Fixed the C++ cast nit.
  35. promag commented at 6:13 pm on May 13, 2019: member
    Restarted failed job.
  36. in src/crypto/hkdf_sha256_32.cpp:1 in 8794a4b3ae 
    0@@ -0,0 +1,21 @@
1+// Copyright (c) 2018 The Bitcoin Core developers
    promag commented at 6:15 pm on May 13, 2019:
    nit, 2019?
  37. in src/test/crypto_tests.cpp:221 in 8794a4b3ae 
    212@@ -212,6 +213,22 @@ static void TestPoly1305(const std::string &hexmessage, const std::string &hexke
213     BOOST_CHECK(tag == tagres);
214 }
215 
216+static void TestHKDF_SHA256_32(const std::string &ikm_hex, const std::string &salt_hex, const std::string &info_hex, const std::string &okm_check_hex) {
217+    std::vector<unsigned char> initial_key_material = ParseHex(ikm_hex);
218+    std::vector<unsigned char> salt = ParseHex(salt_hex);
219+    std::vector<unsigned char> info = ParseHex(info_hex);
220+
221+
    promag commented at 6:16 pm on May 13, 2019:
    nit, remove 2nd empty line.
  38. in src/test/crypto_tests.cpp:216 in 8794a4b3ae 
    212@@ -212,6 +213,22 @@ static void TestPoly1305(const std::string &hexmessage, const std::string &hexke
213     BOOST_CHECK(tag == tagres);
214 }
215 
216+static void TestHKDF_SHA256_32(const std::string &ikm_hex, const std::string &salt_hex, const std::string &info_hex, const std::string &okm_check_hex) {
    promag commented at 6:17 pm on May 13, 2019:
    nit, space after &, not before.
  39. promag commented at 6:19 pm on May 13, 2019: member
    some nits 👼
  40. laanwj commented at 5:24 pm on May 16, 2019: member
    utACK 8794a4b3ae4d34a4cd21a7dee9f694eef7726a4f Don’t want to hold this up on a few last-minute style nits and empty lines.
  41. laanwj merged this on May 16, 2019
  42. laanwj closed this on May 16, 2019
  43. laanwj referenced this in commit 376638afcf on May 16, 2019
  44. jnewbery removed this from the "Blockers" column in a project
  45. sidhujag referenced this in commit 0a01b31f24 on May 18, 2019
  46. PastaPastaPasta referenced this in commit e01676c82b on Jan 25, 2020
  47. PastaPastaPasta referenced this in commit be3eaed62b on Jan 25, 2020
  48. jasonbcox referenced this in commit 983266ac99 on Sep 9, 2020
  49. jasonbcox referenced this in commit 8912da45f1 on Sep 9, 2020
  50. jasonbcox referenced this in commit 5b38c54d2b on Sep 9, 2020
  51. jasonbcox referenced this in commit ddeda021da on Sep 9, 2020
  52. UdjinM6 referenced this in commit c4f7bb5d72 on Aug 10, 2021
  53. 5tefan referenced this in commit f7d8279f3d on Aug 12, 2021
  54. DrahtBot locked this on Dec 16, 2021
  55. dhruv added this to the "Done" column in a project


jonasschnelli DrahtBot sipa Sjors practicalswift promag laanwj

Labels
Build system Tests Utils/log/libs

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-18 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me