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. jonasschnelli cross-referenced this on Aug 24, 2018 from issue Add p2p layer encryption with ECDH/ChaCha20Poly1305 by jonasschnelli
  3. DrahtBot commented at 12:15 PM on August 24, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    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.

  4. DrahtBot cross-referenced this on Aug 24, 2018 from issue Add chacha20/poly1305 and chacha20poly1305_AEAD from openssh by jonasschnelli
  5. 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.

  6. jonasschnelli force-pushed on Aug 27, 2018
  7. jonasschnelli referenced this in commit 955ac1d876 on Aug 29, 2018
  8. DrahtBot cross-referenced this on Aug 30, 2018 from issue export der always compressed by fingera
  9. 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?

  10. 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: 
    src/./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.

  11. jonasschnelli force-pushed on Sep 11, 2018
  12. jonasschnelli force-pushed on Sep 18, 2018
  13. DrahtBot cross-referenced this on Oct 23, 2018 from issue Move util files to directory by jimpo
  14. DrahtBot added the label Needs rebase on Nov 5, 2018
  15. jonasschnelli force-pushed on Mar 5, 2019
  16. jonasschnelli commented at 2:48 PM on March 5, 2019: contributor

    Rebased

  17. DrahtBot removed the label Needs rebase on Mar 5, 2019
  18. DrahtBot added the label Build system on Mar 19, 2019
  19. DrahtBot added the label Tests on Mar 19, 2019
  20. DrahtBot added the label Utils/log/libs on Mar 19, 2019
  21. 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.

  22. 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).

  23. jonasschnelli force-pushed on Mar 26, 2019
  24. sipa commented at 10:50 PM on March 26, 2019: member

    utACK acf3debc6a2f8b80a5de1e71bc658086c6de77b4

  25. DrahtBot added the label Needs rebase on Mar 27, 2019
  26. CKey: add method to negate the key 463921bb64
  27. QA: add test for CKey::Negate() 3b64f852e4
  28. jonasschnelli force-pushed on Mar 27, 2019
  29. jonasschnelli commented at 4:40 PM on March 27, 2019: contributor

    rebased

  30. DrahtBot removed the label Needs rebase on Mar 27, 2019
  31. jonasschnelli added this to the "Blockers" column in a project
  32. 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).

  33. 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.

  34. sipa commented at 9:56 PM on May 10, 2019: member

    utACK 65948ee3d41c33cd442a6572ab9e9d071499022d, just some nits.

  35. Add HKDF HMAC_SHA256 L=32 implementations 551d489416
  36. QA: add test for HKDF HMAC_SHA256 L32 8794a4b3ae
  37. jonasschnelli force-pushed on May 11, 2019
  38. jonasschnelli commented at 7:14 AM on May 11, 2019: contributor

    Thanks for the review. Fixed the C++ cast nit.

  39. promag commented at 6:13 PM on May 13, 2019: member

    Restarted failed job.

  40. promag cross-referenced this on May 13, 2019 from issue Failure in wallet_balance.py by promag
  41. 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?

  42. 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.

  43. 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.

  44. promag commented at 6:19 PM on May 13, 2019: member

    some nits 👼

  45. 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.

  46. laanwj merged this on May 16, 2019
  47. laanwj closed this on May 16, 2019
  48. laanwj referenced this in commit 376638afcf on May 16, 2019
  49. jnewbery removed this from the "Blockers" column in a project
  50. sidhujag referenced this in commit 0a01b31f24 on May 18, 2019
  51. PastaPastaPasta referenced this in commit e01676c82b on Jan 25, 2020
  52. PastaPastaPasta referenced this in commit be3eaed62b on Jan 25, 2020
  53. jasonbcox referenced this in commit 983266ac99 on Sep 9, 2020
  54. jasonbcox referenced this in commit 8912da45f1 on Sep 9, 2020
  55. jasonbcox referenced this in commit 5b38c54d2b on Sep 9, 2020
  56. jasonbcox referenced this in commit ddeda021da on Sep 9, 2020
  57. UdjinM6 referenced this in commit c4f7bb5d72 on Aug 10, 2021
  58. 5tefan referenced this in commit f7d8279f3d on Aug 12, 2021
  59. bitcoin locked this on Dec 16, 2021
  60. dhruv added this to the "Done" column in a project
Contributors
jonasschnelliDrahtBotsipaSjorspracticalswiftpromaglaanwj
Labels
Build systemTestsUtils/log/libs
Linked (view graph)
#14032 Add p2p layer encryption with ECDH/ChaCha20Poly1305#16020 Failure in wallet_balance.py#27634 BIP324 tracking issue#30218 refactor: remove unused `CKey::Negate` method
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: 2026-05-20 06:54 UTC