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 ChaCha20 encryption option (XOR) #15512

Merged
merged 2 commits into from May 10, 2019

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Mar 1, 2019

The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

This PR adds the actual XORing of the plaintext with the keystream in order to return the desired ciphertext.

Required for v2 message transport protocol.

@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Mar 5, 2019

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.

Loading

Copy link
Contributor

@ryanofsky ryanofsky left a comment

utACK 4325da9. I don't know very much about chacha20 or this method of encryption, but this seems to do what is described.

Loading

src/crypto/chacha20.cpp Outdated Show resolved Hide resolved
Loading
@@ -100,6 +102,10 @@ void ChaCha20::Output(unsigned char* c, size_t bytes)

for (;;) {
if (bytes < 64) {
if (m != nullptr) {
for (i = 0;i < bytes;++i) tmp[i] = m[i];
Copy link
Contributor

@ryanofsky ryanofsky Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just memcpy(tmp, m, bytes) might be simpler and easy to read than this for loop.

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli Mar 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not to familiar with compiler optimisations and stuff but I just checked the reference implementation by DJB and some other C based ChaCha implementations and all of them seems to use the byte per byte assignment. I prefer to leave it as it is.

Loading

src/crypto/chacha20.cpp Outdated Show resolved Hide resolved
Loading
src/crypto/chacha20.h Outdated Show resolved Hide resolved
Loading
src/test/crypto_tests.cpp Show resolved Hide resolved
Loading
src/test/crypto_tests.cpp Outdated Show resolved Hide resolved
Loading
laanwj added a commit that referenced this issue Mar 27, 2019
e9d5e97 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf30 Add Poly1305 bench (Jonas Schnelli)
03be7f4 Add Poly1305 implementation (Jonas Schnelli)

Pull request description:

  This adds a currently unused Poly1305 implementation including test vectors from RFC7539.

  Required for BIP151 (and related to #15512).

Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1
@@ -71,7 +71,7 @@ void ChaCha20::Seek(uint64_t pos)
input[13] = pos >> 32;
}

void ChaCha20::Output(unsigned char* c, size_t bytes)
void ChaCha20::Output(const unsigned char* m, unsigned char* c, size_t bytes)
Copy link
Member

@sipa sipa Mar 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to duplicate the logic here and turn the stream cipher version into a separate function.

As is, it introduces an unnecessary branch in the output version.

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli Mar 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Adapted the duplication approach.

Loading

Copy link
Member

@jnewbery jnewbery May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning here? Performance?

How does the version in the refrence implementation compare?

void ECRYPT_keystream_bytes(ECRYPT_ctx *x,u8 *stream,u32 bytes)
{
  u32 i;
  for (i = 0;i < bytes;++i) stream[i] = 0;
  ECRYPT_encrypt_bytes(x,stream,stream,bytes);
}

ie setting Output(c, bytes) to call Crypt(c, c, bytes)?

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it in the initial version like @jnewbery just proposed.
After @sipa recommended to separate it, I thought that a strict separation may be beneficial for verification and later optimizations. But maybe @sipa can clarify...

Loading

x14 += j14;
x15 += j15;

if (m != nullptr) {
Copy link
Member

@sipa sipa Mar 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this if.

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli Mar 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Fixed.

Loading

Copy link
Contributor

@ryanofsky ryanofsky left a comment

utACK 427b49d. Changes since last review: splitting Output/Crypt functions, dropping comment describing function arguments, adding comment describing m/tmp copy, dropping XOR macro, and making suggested test changes.

Loading

Copy link
Member

@jnewbery jnewbery left a comment

Implementation and tests look good and match the reference implementation here: https://cr.yp.to/streamciphers/timings/estreambench/submissions/salsa20/chacha8/merged/chacha.c and test vector here: https://tools.ietf.org/html/rfc7539#section-2.4.2

I have a few minor comments inline, and would be curious about the answer to:
#15512 (comment)

Loading

src/crypto/chacha20.h Show resolved Hide resolved
Loading
src/crypto/chacha20.h Show resolved Hide resolved
Loading
src/crypto/chacha20.h Outdated Show resolved Hide resolved
Loading
src/test/crypto_tests.cpp Show resolved Hide resolved
Loading
Copy link
Member

@jnewbery jnewbery left a comment

Looks great. Thanks for adding the comments to chacha20.h!

Just a couple of nits in the bench file. I'm still curious about the code duplication between Keystream() and Crypt().

Loading

src/bench/chacha20.cpp Show resolved Hide resolved
Loading
ctx.Crypt(in.data(), out.data(), in.size());
}

static void HASH(benchmark::State& state, size_t buffersize)
Copy link
Member

@jnewbery jnewbery May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a reason you've added these CHash256 benchmarks to chacha20.cpp?

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added if for comparison and the impact on the networking... though I think it belong to an extra commit in #15649
Will remove it from here.

Loading

@jonasschnelli jonasschnelli force-pushed the 2019/03/chacha branch 2 times, most recently from c8df2c0 to 2dfe275 May 3, 2019
@jnewbery
Copy link
Member

@jnewbery jnewbery commented May 3, 2019

Looks good. utACK 2dfe275.

In general, it's better to have less code duplication, so I'd like to hear from @sipa his reasoning for #15512 (comment)

Loading

@sipa
Copy link
Member

@sipa sipa commented May 3, 2019

@jnewbery I just thought that it'd be preferable not to burden the RNG code with branches that are only used for encryption. In cryptographic code like this, I don't care too much about duplication, as it isn't code that subject to many possible future changes.

Loading

@jnewbery
Copy link
Member

@jnewbery jnewbery commented May 6, 2019

ok, my aesthetic preference is for less duplication, either by having a single Keystream/Crypt function, or by calling Keystream() from Crypt() with a zero'ed message. As sipa points out though, this code is unlikely to be modified much in future, so it's not a big deal either way.

utACK 2dfe275

Loading

Copy link
Contributor

@ryanofsky ryanofsky left a comment

utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

I just thought that it'd be preferable not to burden the RNG code with branches that are only used for encryption.

I think the duplication is fine, but it's unclear if the concern with branches is about readability or performance. If it's performance, you could give the previous Output() function an additional template<bool use_input> template argument, and have Crypt() and Keystream() both call it inlined.

Loading

@sipa
Copy link
Member

@sipa sipa commented May 10, 2019

utACK 2dfe275

Loading

@jonasschnelli jonasschnelli merged commit 2dfe275 into bitcoin:master May 10, 2019
2 checks passed
Loading
jonasschnelli added a commit that referenced this issue May 10, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
@jonasschnelli jonasschnelli removed this from Blockers in High-priority for review May 10, 2019
sidhujag added a commit to syscoin/syscoin that referenced this issue May 10, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 19, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 19, 2019
e9d5e97 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf30 Add Poly1305 bench (Jonas Schnelli)
03be7f4 Add Poly1305 implementation (Jonas Schnelli)

Pull request description:

  This adds a currently unused Poly1305 implementation including test vectors from RFC7539.

  Required for BIP151 (and related to bitcoin#15512).

Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1

if (bytes <= 64) {
if (bytes < 64) {
for (i = 0;i < bytes;++i) ctarget[i] = c[i];
Copy link

@Kixunil Kixunil Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this no-op? When bytes < 64 line 215 get's executed which makes ctarget and c equal pointers, therefore it's copying a value to itself.

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so.
If we do less then 64 bytes, we execute the round on tmp (because we do 64bytes always, see line 216, c points to tmp if <64 bytes). So in the case of <64 bytes, c points to the temporary 64byte buffer and ctarget points to the function provided c argument.

Loading

Copy link

@Kixunil Kixunil Jul 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, missed that. Thanks and sorry for bothering.

Loading

void SetIV(uint64_t iv);
void Seek(uint64_t pos);
void Output(unsigned char* output, size_t bytes);
void SetKey(const unsigned char* key, size_t keylen); //!< set key with flexible keylength; 256bit recommended */
Copy link

@Kixunil Kixunil Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the renaming. Would be nice if the comments included information about pointers (e.g. "key must be non-null, pointer isn't stored").

Loading

Copy link
Contributor Author

@jonasschnelli jonasschnelli Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That would have been useful. Feel free to PR.

Loading

laanwj added a commit that referenced this issue Jul 11, 2019
bb326ad Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea04 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)

Pull request description:

  This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).

  Includes: #15519, #15512 (please review those first).

  The construct is specified here.
  https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite

  This aims for being used in v2 peer-to-peer messages.

ACKs for top commit:
  laanwj:
    code review ACK bb326ad

Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686
sidhujag added a commit to syscoin/syscoin that referenced this issue Jul 11, 2019
bb326ad Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea04 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)

Pull request description:

  This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).

  Includes: bitcoin#15519, bitcoin#15512 (please review those first).

  The construct is specified here.
  https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite

  This aims for being used in v2 peer-to-peer messages.

ACKs for top commit:
  laanwj:
    code review ACK bb326ad

Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 18, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 18, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 18, 2019
e9d5e97 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf30 Add Poly1305 bench (Jonas Schnelli)
03be7f4 Add Poly1305 implementation (Jonas Schnelli)

Pull request description:

  This adds a currently unused Poly1305 implementation including test vectors from RFC7539.

  Required for BIP151 (and related to bitcoin#15512).

Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 18, 2019
bb326ad Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea04 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)

Pull request description:

  This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).

  Includes: bitcoin#15519, bitcoin#15512 (please review those first).

  The construct is specified here.
  https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite

  This aims for being used in v2 peer-to-peer messages.

ACKs for top commit:
  laanwj:
    code review ACK bb326ad

Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 23, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 23, 2019
e9d5e97 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf30 Add Poly1305 bench (Jonas Schnelli)
03be7f4 Add Poly1305 implementation (Jonas Schnelli)

Pull request description:

  This adds a currently unused Poly1305 implementation including test vectors from RFC7539.

  Required for BIP151 (and related to bitcoin#15512).

Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 23, 2019
bb326ad Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea04 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)

Pull request description:

  This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).

  Includes: bitcoin#15519, bitcoin#15512 (please review those first).

  The construct is specified here.
  https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite

  This aims for being used in v2 peer-to-peer messages.

ACKs for top commit:
  laanwj:
    code review ACK bb326ad

Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686

Add new line
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Aug 6, 2019
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Aug 6, 2019
e9d5e97 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf30 Add Poly1305 bench (Jonas Schnelli)
03be7f4 Add Poly1305 implementation (Jonas Schnelli)

Pull request description:

  This adds a currently unused Poly1305 implementation including test vectors from RFC7539.

  Required for BIP151 (and related to bitcoin#15512).

Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Aug 6, 2019
bb326ad Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea04 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)

Pull request description:

  This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).

  Includes: bitcoin#15519, bitcoin#15512 (please review those first).

  The construct is specified here.
  https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite

  This aims for being used in v2 peer-to-peer messages.

ACKs for top commit:
  laanwj:
    code review ACK bb326ad

Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686

Add new line
barrystyle added a commit to PACGlobalOfficial/PAC that referenced this issue Jan 22, 2020
2dfe275 Add ChaCha20 bench (Jonas Schnelli)
2bc2b8b Add ChaCha20 encryption option (XOR) (Jonas Schnelli)

Pull request description:

  The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG).

  This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`.

  Required for v2 message transport protocol.

ACKs for commit 2dfe27:
  jnewbery:
    Looks good. utACK 2dfe275.
  jnewbery:
    utACK 2dfe275
  sipa:
    utACK 2dfe275
  ryanofsky:
    utACK 2dfe275. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark.

Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
barrystyle added a commit to PACGlobalOfficial/PAC that referenced this issue Jan 22, 2020
e9d5e97 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf30 Add Poly1305 bench (Jonas Schnelli)
03be7f4 Add Poly1305 implementation (Jonas Schnelli)

Pull request description:

  This adds a currently unused Poly1305 implementation including test vectors from RFC7539.

  Required for BIP151 (and related to bitcoin#15512).

Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1
barrystyle added a commit to PACGlobalOfficial/PAC that referenced this issue Jan 22, 2020
bb326ad Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea04 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)

Pull request description:

  This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).

  Includes: bitcoin#15519, bitcoin#15512 (please review those first).

  The construct is specified here.
  https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite

  This aims for being used in v2 peer-to-peer messages.

ACKs for top commit:
  laanwj:
    code review ACK bb326ad

Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686

Add new line
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Sep 21, 2020
Summary:
 * Add ChaCha20 bench

This is a backport of Core [[bitcoin/bitcoin#15512 | PR15512]]

Test Plan:
  ninja all check bench-bitcoin

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D7491
random-zebra added a commit to PIVX-Project/PIVX that referenced this issue Apr 14, 2021
cecbf6c Use secure.h header for secure allocators (Fuzzbawls)
d9f67da net: add ifaddrs.h include (fanquake)
e906436 build: check if -lsocket is required with *ifaddrs (fanquake)
414f405 rand: only try and use freeifaddrs if available (fanquake)
3a039d6 build: avoid getifaddrs when unavailable (Cory Fields)
77bddd7 Use GetStrongRandBytes in gmp bignum initialization (Fuzzbawls)
b70b26f Fix typo in comment in randomenv.cpp (Fuzzbawls)
fec460c Put bounds on the number of CPUID leaves explored (Pieter Wuille)
41ab1ff Fix CPUID subleaf iteration (Pieter Wuille)
8a9bbb1 Move events_hasher into RNGState() (Pieter Wuille)
88c2ae5 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake)
81d382f doc: correct random.h docs after bitcoin#17270 (fanquake)
f363ea9 Seed RNG with precision timestamps on receipt of net messages. (Matt Corallo)
7d6ddcb Run background seeding periodically instead of unpredictably (Pieter Wuille)
4679181 Add information gathered through getauxval() (Pieter Wuille)
88d97d0 Feed CPUID data into RNG (Pieter Wuille)
8f5b9c9 Use sysctl for seeding on MacOS/BSD (Pieter Wuille)
67de246 Gather additional entropy from the environment (Pieter Wuille)
6142e1f Seed randomness with process id / thread id / various clocks (Pieter Wuille)
7bde8b7 [MOVEONLY] Move cpuid code from random to compat/cpuid (Fuzzbawls)
52b5336 [MOVEONLY] Move perfmon data gathering to new randomenv module (Pieter Wuille)
27cf995 doc: minor corrections in random.cpp (fanquake)
fccd2b8 doc: correct function name in ReportHardwareRand() (fanquake)
909473e Fix FreeBSD build by including utilstrencodings.h (Fuzzbawls)
630931f break circular dependency: random/sync -> util -> random/sync (Fuzzbawls)
5eed08c random: remove call to RAND_screen() (Windows only) (fanquake)
ada9868 gui: remove OpenSSL PRNG seeding (Windows, Qt only) (fanquake)
22a7121 Fix non-deterministic coverage of test DoS_mapOrphans (Fuzzbawls)
79e7fd3 Add ChaCha20 bench (Jonas Schnelli)
6966aa9 Add ChaCha20 encryption option (XOR) (Jonas Schnelli)
28c9cdb tests: Add script checking for deterministic line coverage (practicalswift)
c82e359 test: Make bloom tests deterministic (MarcoFalke)
7b33223 Document strenghtening (Pieter Wuille)
0190dec Add hash strengthening to the RNG (Pieter Wuille)
67e336d Use RdSeed when available, and reduce RdRand load (Pieter Wuille)
4ffda1f Document RNG design in random.h (Pieter Wuille)
2b6381e Use secure allocator for RNG state (Pieter Wuille)
080deb3 Encapsulate RNGState better (Pieter Wuille)
787d72f DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille)
5bc2583 Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille)
774899f Remove hwrand_initialized. (Pieter Wuille)
698d133 Switch all RNG code to the built-in PRNG. (Pieter Wuille)
038a45a Integrate util/system's CInit into RNGState (Fuzzbawls)
5f20e62 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille)
298f97c Add thread safety annotations to RNG state (Pieter Wuille)
2326535 Rename some hardware RNG related functions (Pieter Wuille)
d76ee83 Automatically initialize RNG on first use. (Pieter Wuille)
1a5dbc5 Don't log RandAddSeedPerfmon details (Pieter Wuille)
32e6c42 Simplify testing RNG code (Fuzzbawls)
972effa Make unit tests use the insecure_rand_ctx exclusively (Fuzzbawls)
af52bf5 Use a FastRandomContext in LimitOrphanTxSize (Fuzzbawls)
746d466 Introduce a Shuffle for FastRandomContext and use it in wallet (Fuzzbawls)
1cdf124 Use a local FastRandomContext in a few more places in net (Fuzzbawls)
e862564 Make addrman use its local RNG exclusively (Fuzzbawls)
94b2ead Make FastRandomContext support standard C++11 RNG interface (Pieter Wuille)

Pull request description:

  This is a collection of upstream PRs that have been backported to bring our RNG (`src/random`) code more up-to-date. The following upstream PRs have been included here:

  - bitcoin#12742
  - bitcoin#14624
    - some of this had already been merged previously
  - bitcoin#14955
  - bitcoin#15250
  - bitcoin#15224
  - bitcoin#15324
  - bitcoin#15296
  - bitcoin#15512
  - bitcoin#16878
  - bitcoin#17151
  - bitcoin#17191
  - bitcoin#13236
  - bitcoin#13314
  - bitcoin#17169
  - bitcoin#17270
    -  omitted last commit as our testing framework doesn't support it currently
    - omitted bitcoin@64e1e02, to be pulled in after our time utility is updated in a separate PR
  - bitcoin#17573
  - bitcoin#17507
  - bitcoin#17670
  - bitcoin#17527
  - bitcoin#14127
  - bitcoin#21486

ACKs for top commit:
  furszy:
    ACK cecbf6c with a minor nit that can be easily tackled later.
  random-zebra:
    rebase utACK cecbf6c and merging...

Tree-SHA512: 3463b693cc9bddc1ec15228d264a794f5c2f159073fafa2ccf6e2563abfeb4369e49505f97ca84f2478ca792bd07b66d2cd83c58044d6a0cae6af42d22f5784b
@dhruv dhruv added this to Done in P2P encryption Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants