Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HKDF_HMAC256_L32 and method to negate a private key #14047

Merged
merged 4 commits into from May 16, 2019

Conversation

Projects
None yet
7 participants
@jonasschnelli
Copy link
Member

commented Aug 24, 2018

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.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

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.

Show resolved Hide resolved src/crypto/hkdf_sha256_32.cpp Outdated

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2018/08/bip151_key_hkdf branch Aug 27, 2018

jonasschnelli added a commit to jonasschnelli/bitcoin that referenced this pull request Aug 29, 2018

Show resolved Hide resolved src/test/crypto_tests.cpp Outdated

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2018/08/bip151_key_hkdf branch 2 times, most recently to a034ec2 Sep 11, 2018

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2018/08/bip151_key_hkdf branch from a034ec2 to 32633ff Mar 5, 2019

@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2019

Rebased

Show resolved Hide resolved src/test/key_tests.cpp
Show resolved Hide resolved src/test/crypto_tests.cpp Outdated

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2018/08/bip151_key_hkdf branch from 32633ff to acf3deb Mar 26, 2019

@sipa

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

utACK acf3deb

jonasschnelli added some commits Aug 13, 2018

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2018/08/bip151_key_hkdf branch from acf3deb to 65948ee Mar 27, 2019

@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

rebased

@sipa
Copy link
Member

left a comment

utACK 65948ee, just some nits.


CHKDF_HMAC_SHA256_L32::CHKDF_HMAC_SHA256_L32(const unsigned char* ikm, size_t ikmlen, const std::string& salt)
{
CHMAC_SHA256(reinterpret_cast<const unsigned char*>(salt.c_str()), salt.size()).Write(ikm, ikmlen).Finalize(m_prk);

This comment has been minimized.

Copy link
@sipa

sipa May 10, 2019

Member

Style nit: we don't usually use the C++ cast operators for primitive types (just (const unsigned char*)salt.c_str() works).

// expand a 32byte key (single round)
assert(info.size() <= 128);
static const unsigned char one[1] = {1};
CHMAC_SHA256(m_prk, 32).Write(reinterpret_cast<const unsigned char*>(info.data()), info.size()).Write(one, 1).Finalize(hash);

This comment has been minimized.

Copy link
@sipa

sipa May 10, 2019

Member

Same here.

jonasschnelli added some commits Aug 24, 2018

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2018/08/bip151_key_hkdf branch from 65948ee to 8794a4b May 11, 2019

@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented May 11, 2019

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

@promag

This comment has been minimized.

Copy link
Member

commented May 13, 2019

Restarted failed job.

@promag
Copy link
Member

left a comment

some nits 👼

@@ -0,0 +1,21 @@
// Copyright (c) 2018 The Bitcoin Core developers

This comment has been minimized.

Copy link
@promag

promag May 13, 2019

Member

nit, 2019?

std::vector<unsigned char> salt = ParseHex(salt_hex);
std::vector<unsigned char> info = ParseHex(info_hex);


This comment has been minimized.

Copy link
@promag

promag May 13, 2019

Member

nit, remove 2nd empty line.

@@ -212,6 +213,22 @@ static void TestPoly1305(const std::string &hexmessage, const std::string &hexke
BOOST_CHECK(tag == tagres);
}

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) {

This comment has been minimized.

Copy link
@promag

promag May 13, 2019

Member

nit, space after &, not before.

@laanwj

This comment has been minimized.

Copy link
Member

commented May 16, 2019

utACK 8794a4b
Don't want to hold this up on a few last-minute style nits and empty lines.

@laanwj laanwj merged commit 8794a4b into bitcoin:master May 16, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request May 16, 2019

Merge #14047: Add HKDF_HMAC256_L32 and method to negate a private key
8794a4b QA: add test for HKDF HMAC_SHA256 L32 (Jonas Schnelli)
551d489 Add HKDF HMAC_SHA256 L=32 implementations (Jonas Schnelli)
3b64f85 QA: add test for CKey::Negate() (Jonas Schnelli)
463921b CKey: add method to negate the key (Jonas Schnelli)

Pull request description:

  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.

ACKs for commit 8794a4:

Tree-SHA512: 5341929dfa29f5da766ec3612784baec6a3ad69972f08b5a985a8aafdae4dae36f104a2b888d1f5d1f33561456bd111f960d7e32c2cc4fd18e48358468f26c1a

@jnewbery jnewbery removed this from Blockers in High-priority for review May 16, 2019

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 18, 2019

Merge bitcoin#14047: Add HKDF_HMAC256_L32 and method to negate a priv…
…ate key

8794a4b QA: add test for HKDF HMAC_SHA256 L32 (Jonas Schnelli)
551d489 Add HKDF HMAC_SHA256 L=32 implementations (Jonas Schnelli)
3b64f85 QA: add test for CKey::Negate() (Jonas Schnelli)
463921b CKey: add method to negate the key (Jonas Schnelli)

Pull request description:

  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 bitcoin#14032 and a pre-requirement for the v2 transport protocol.

ACKs for commit 8794a4:

Tree-SHA512: 5341929dfa29f5da766ec3612784baec6a3ad69972f08b5a985a8aafdae4dae36f104a2b888d1f5d1f33561456bd111f960d7e32c2cc4fd18e48358468f26c1a

@LitecoinZ LitecoinZ referenced this pull request May 31, 2019

Open

Backports from upstream #1

44 of 244 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.