Skip to content

bash-prg-hash: Initial implementation#751

Merged
newpavlov merged 58 commits into
RustCrypto:masterfrom
makavity:master
May 12, 2026
Merged

bash-prg-hash: Initial implementation#751
newpavlov merged 58 commits into
RustCrypto:masterfrom
makavity:master

Conversation

@makavity

@makavity makavity commented Nov 4, 2025

Copy link
Copy Markdown
Contributor
  1. I am not sure to assert in block_api
  2. I am not sure in new and new_with_empty_header functions.

@makavity

makavity commented Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

I am also not sure, it should be implemented as prg-hash.
Because AEAD algorithm uses start, squeeze, absorb and encrypt functions. But it is correct to implement encrypt here and make block_api methods - pub?

@newpavlov newpavlov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the late review!

Some preliminary comments without looking deep at the implementation and the spec.

Comment thread bash-prg-hash/Cargo.toml Outdated
Comment thread bash-prg-hash/tests/mod.rs Outdated
Comment thread bash-prg-hash/tests/mod.rs Outdated
Comment thread bash-prg-hash/src/oids.rs Outdated
Comment thread bash-prg-hash/README.md Outdated
Comment thread bash-prg-hash/src/lib.rs Outdated
Comment thread bash-prg-hash/src/block_api.rs Outdated
Comment thread bash-prg-hash/src/block_api.rs Outdated
Comment thread bash-prg-hash/src/block_api.rs Outdated
Comment thread bash-prg-hash/src/block_api.rs Outdated
@makavity

Copy link
Copy Markdown
Contributor Author

Thanks for review, @newpavlov.
As I see, all fixed

@makavity

Copy link
Copy Markdown
Contributor Author

Looks like the sha1 is failed because timeout, not because PR changes.

@makavity makavity requested a review from newpavlov April 28, 2026 10:20
Comment thread bash-prg-hash/src/block_api.rs Outdated
Comment thread bash-prg-hash/src/block_api.rs Outdated
Comment thread bash-prg-hash/src/lib.rs Outdated
Comment thread bash-prg-hash/src/lib.rs

// Step 2: S[r] <- S[r] ⊕ 1, where r = 1536 - 2 d ℓ (bit index).
const { assert!(RATE % 8 == 0) }
self.state[RATE / 8] ^= 1u64 << 7;

@newpavlov newpavlov May 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks a bit weird, but I guess it's a consequence of mixing little-endian byte order and big-endian bit order in the spec and commit acting on the state outside of the rate part. For comparison, in cshake and turboshake we use self.state[RATE / 8 - 1] ^= 1 << 63;.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, I suppose. I've noticed earlier, that spec is using first byte after rate.

@makavity

makavity commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

I refactored the code a fair bit. Feel free to comment on the changes if something is not clear or if you have suggestions on how we could improve it.

Looks like the commit 1dc21053 broke the customization flow.
I'll commit the tests with announce, which worked before commit and continue to investigate.

@makavity

makavity commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Just misspell in the header length calculation.

@makavity

makavity commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

@newpavlov in the methodic, there are test with 1000000 of 0x61 for input.
Should we add that tests for the completeness, or we can just skip it?

@makavity

makavity commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

I refactored the code a fair bit. Feel free to comment on the changes if something is not clear or if you have suggestions on how we could improve it.

Really a fair bit. Now it's just two little functions :D
Awesome work, thank you!

@newpavlov

Copy link
Copy Markdown
Member

there are test with 1000000 of 0x61 for input.

I think it's fine to add it (e.g. by feeding 1k byte chunks 1k times). It should be relatively fast.

@newpavlov

newpavlov commented May 8, 2026

Copy link
Copy Markdown
Member

Please merge master (or rebase) and fix the readme issue.

Comment thread bash-prg-hash/Cargo.toml Outdated
@newpavlov newpavlov merged commit cefd70f into RustCrypto:master May 12, 2026
277 checks passed
newpavlov added a commit that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants