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

stream-cipher: remove NewBlockCipher bound on FromBlockCipher #333

Merged

Conversation

tarcieri
Copy link
Member

Note: breaking change.

As far as I can tell this was an oversight: FromBlockCipher never needs to instantiate a new block cipher (isn't that the point?), making this bound needlessly limiting.

The rationale for it in the first place appears to be for the blanket impl of NewStreamCipher for C: FromBlockCipher which needs it, but we can add the bound to the blanket impl, rather than (needlessly) sticking it on the trait.

@tarcieri
Copy link
Member Author

@newpavlov I'd like to merge this for now as it's badly blocking RustCrypto/stream-ciphers#170 and more specifically migrating aes-gcm and aes-gcm-siv to use it (since it has a "viral" NewBlockCipher bound, which prevents using the impl<B: BlockCipher> BlockCipher for &B blanket impl).

Will wait for your input before cutting another release, though. Worst case we can revert it.

Note: breaking change.

As far as I can tell this was an oversight: `FromBlockCipher` never
needs to instantiate a new block cipher (isn't that the point?),
making this bound needlessly limiting.

The rationale for it in the first place appears to be for the blanket
impl of `NewStreamCipher` for `C: FromBlockCipher` which needs it, but
we can add the bound to the blanket impl, rather than (needlessly)
sticking it on the trait.
@tarcieri tarcieri force-pushed the stream-cipher/remove-new-bound-on-from-block-cipher branch from 7c7a0db to b24065e Compare October 14, 2020 15:08
@tarcieri tarcieri merged commit 6c23a07 into master Oct 14, 2020
@tarcieri tarcieri deleted the stream-cipher/remove-new-bound-on-from-block-cipher branch October 14, 2020 15:17
@newpavlov
Copy link
Member

Looks good! I think it was indeed an oversight on my part.

dns2utf8 pushed a commit to dns2utf8/traits that referenced this pull request Jan 24, 2023
- Bump version to `0.5.0-pre` (RustCrypto#247 contained breaking changes)
- Use pointer casts to convert `Block` integer array to byte array
- Rename `permutate!` to `permute!` (former isn't in OED, latter is)
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