crypto: BIP324 ciphersuite follow-up #28267
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
follow-up to bitcoin#28008. * move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every time * use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()` * comment for initiator in `BIP324Cipher::Initialize()` * systematically damage ciphertext with bit positions in bip324_tests * use 4095 max bytes for aad in bip324 fuzz test
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
This was referenced Aug 14, 2023
Merged
|
utACK d22d5d9 |
|
cc @theStack |
maflcko
reviewed
Aug 14, 2023
fanquake
reviewed
Aug 14, 2023
stratospher
force-pushed
the
followup_28008
branch
from
8eb8af1 to
93cb8f0
Compare
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Aug 17, 2023
93cb8f0 refactor: add missing headers for BIP324 ciphersuite (stratospher) d22d5d9 crypto: BIP324 ciphersuite follow-up (stratospher) Pull request description: follow-up to bitcoin#28008. * move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every time * use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()` * comment for initiator in `BIP324Cipher::Initialize()` * systematically damage ciphertext with bit positions in bip324_tests * use 4095 max bytes for `aad` in bip324 fuzz test ACKs for top commit: fanquake: ACK 93cb8f0 - thanks for following up here. Tree-SHA512: 361f3e226d3168fdef69a2eebe6092cfc04ba14ce009420222e762698001eaf8be69a1138dab0be237964509c2b96a41a0b4db5c1df43ef75062f143c5aa741a
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 19, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 20, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 23, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 24, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 27, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 27, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 29, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Mar 5, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Mar 5, 2024
PastaPastaPasta
added a commit
to dashpay/dash
that referenced
this pull request
Mar 6, 2024
, bitcoin#28267, bitcoin#28374, bitcoin#28100 (p2p primitives) b60c493 merge bitcoin#28100: more Span<std::byte> modernization & follow-ups (Kittywhiskers Van Gogh) c2aa01c merge bitcoin#28374: python cryptography required for BIP 324 functional tests (Kittywhiskers Van Gogh) 7c5edf7 merge bitcoin#28267: BIP324 ciphersuite follow-up (Kittywhiskers Van Gogh) 1b1924e merge bitcoin#28008: BIP324 ciphersuite (Kittywhiskers Van Gogh) ff54219 merge bitcoin#27993: Make poly1305 support incremental computation + modernize (Kittywhiskers Van Gogh) d7482eb merge bitcoin#27985: Add support for RFC8439 variant of ChaCha20 (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #5900 * Dependent on #5901 * Without modifications, tests introduced in [bitcoin#28008](bitcoin#28008) will fail due to salt comprising of a fixed string (`bitcoin_v2_shared_secret`) and network bytes ([source](https://github.com/sipa/bitcoin/blob/1c7582ead6e1119899922041c1af2b4169b0bc74/src/bip324.cpp#L39-L40)). Bitcoin uses [`{0xf9, 0xbe, 0xb4, 0xd9}`](https://github.com/sipa/bitcoin/blob/1c7582ead6e1119899922041c1af2b4169b0bc74/src/kernel/chainparams.cpp#L114-L117) for mainnet while Dash uses [`{0xbf, 0x0c, 0x6b, 0xbd}`](https://github.com/dashpay/dash/blob/37f43e4e56de0d510110a7f790df34ea77748dc9/src/chainparams.cpp#L238-L241). * The replacement parameters are generated by: * Cloning https://github.com/bitcoin/bips (as of this writing, at bitcoin/bips@b3701fa) * Editing `bip-0324/reference.py` * Changing `NETWORK_MAGIC` to Dash's network magic * Running `gen_test_vectors.py` to get a new `packet_encoding_test_vectors.csv` * Using [this python script](https://github.com/bitcoin/bitcoin/blob/1c7582ead6e1119899922041c1af2b4169b0bc74/src/test/bip324_tests.cpp#L174-L196) mentioned in a comment in `src/test/bip324_tests.cpp`, generate the values that will be used to replace the ones in `bip324_tests.cpp` (it will print to `stdout` so it's recommended to pipe it to a file) * Paste the new values over the old ones ## Breaking Changes None. Changes are restricted to BIP324 cryptosuite, tests and associated logic. ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ Top commit has no ACKs. Tree-SHA512: bb056de8588026adae63e78d56878274ff3934a439e2eae81606fa9c0a37dab432a129315bb9c1b754400b5c883bf460eea3a0c857a3be0816c8fbf55c479843
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
follow-up to #28008.
dummy_tagvariable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every timecipher.last()inAEADChaCha20Poly1305::Decrypt()BIP324Cipher::Initialize()aadin bip324 fuzz test