Skip to content

Conversation

theStack
Copy link
Contributor

This PR is a tiny improvement for the bip324_cipher_roundtrip fuzz target: currently the damaging of input data for decryption (either ciphertext or aad) only ever happens in the lower nibble within the byte at the damage position, as the bit position for the damage_val byte was calculated with damage_bit & 3 (corresponding to % 4) rather than damage_bit & 7 (corresponding to the expected % 8).

Noticed while reviewing #28263 which uses similar constructs.

Currently the damaging of input data for decryption (either ciphertext
or aad) only ever happens in the lower nibble within the byte at the
damage position, as the bit position for the `damage_val` byte was
calculated with `damage_bit & 3` (corresponding to `% 4`) rather than
`damage_bit & 7` (corresponding to the expected `% 8`).
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 28, 2023

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK stratospher, dergoegge

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Copy link
Contributor

@stratospher stratospher left a comment

Choose a reason for hiding this comment

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

ACK e67634e.

@fanquake fanquake requested a review from dergoegge November 28, 2023 08:33
Copy link
Member

@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

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

utACK e67634e

@fanquake fanquake merged commit 05d3f8e into bitcoin:master Nov 30, 2023
@theStack theStack deleted the 202311-fuzz-bip324-damage_in_full_byte_range branch November 30, 2023 15:07
kwvg added a commit to kwvg/dash that referenced this pull request Sep 17, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Sep 17, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Sep 17, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Sep 18, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Oct 2, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Oct 9, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Oct 14, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Oct 15, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Oct 15, 2024
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 16, 2024
, bitcoin#28849, bitcoin#28805, bitcoin#28951, bitcoin#29058, bitcoin#29239, partial bitcoin#28331, bitcoin#28452 (BIP324 backports: part 2)

5dd60c4 merge bitcoin#29239: Make v2transport default for addnode RPC when enabled (Kittywhiskers Van Gogh)
b2ac426 merge bitcoin#29058: use v2transport for manual/addrfetch connections, add to -netinfo (Kittywhiskers Van Gogh)
92e862a merge bitcoin#28951: damage ciphertext/aad in full byte range (Kittywhiskers Van Gogh)
4e96e26 merge bitcoin#28805: Make existing functional tests compatible with --v2transport (Kittywhiskers Van Gogh)
9371e2e merge bitcoin#28849: fix node index bug when comparing peerinfo (Kittywhiskers Van Gogh)
400c9dd merge bitcoin#28634: add check for missing garbage terminator detection (Kittywhiskers Van Gogh)
65eb194 merge bitcoin#28588: add checks for v1 prefix matching / wrong network magic detection (Kittywhiskers Van Gogh)
6074974 merge bitcoin#28577: raise V1_PREFIX_LEN from 12 to 16 (Kittywhiskers Van Gogh)
ff92d1a partial bitcoin#28331: BIP324 integration (Kittywhiskers Van Gogh)
f9f8805 fix: drop `CConnman::mapNodesWithDataToSend`, use transport data (UdjinM6)
d39d8a4 partial bitcoin#28452: Do not use std::vector = {} to release memory (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on #6280

  * Dependent on #6276

  * Dependency for #6329

  * [bitcoin#28452](bitcoin#28452) was backported as the behavior it introduces is required for backports like [bitcoin#28331](bitcoin#28331). As it pre-dates earlier BIP324 backports, the `ClearShrink` usage from those backports have also been incorporated into this backport.

  * When backporting [bitcoin#28331](bitcoin#28331), in TestFramework's `add_nodes()`, `extra_args[i]` is not converted to a `list` like it is done upstream and avoiding duplication of `-v2transport=1` is instead done by an additional check. This is done to prevent test failures in `feature_mnehf.py`.

  * The local variable `args` is removed in [bitcoin#28805](bitcoin#28805) as it does nothing (the argument appending logic is removed as part of the backport) and upstream removes it anyways.

  Special thanks to UdjinM6 for significant contributions to this and dependent PRs! 🎉

  ## Breaking Changes

  None expected.

  ## Checklist

  - [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
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    light ACK 5dd60c4
  PastaPastaPasta:
    utACK 5dd60c4

Tree-SHA512: 7f3d0e54e1c96fc99b2d6b1e64485110aa73aeec0e4e4245ed4e6dc0529a7608e9c39103af636d5945d289775c40d3d15d7d4a75bea82462194dbf355fca48dc
@bitcoin bitcoin locked and limited conversation to collaborators Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants