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

cipher: v0.3 changes (without block-modes integration) #435

Merged
merged 11 commits into from Dec 30, 2020

Conversation

tarcieri
Copy link
Member

This PR contains the proposed API changes from #394, but removes the WIP work to also integrate the block-modes crate.

The reason is this work is a blocker on getting downstream crates updated to the new API, and the block-modes integration work is otherwise purely additive so it can easily be done separately.


/// Create new block cipher instance from key with variable size.
///
/// Default implementation will accept only keys with length equal to
/// `KeySize`, but some ciphers can accept range of key lengths.
fn new_varkey(key: &[u8]) -> Result<Self, InvalidLength> {
fn new_var(key: &[u8]) -> Result<Self, InvalidLength> {
Copy link
Member Author

Choose a reason for hiding this comment

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

As long as we're bikeshedding this method name, I've thought it would be clearer if it had slice in the name, e.g. new_from_slice or from_slice.

Copy link
Member

Choose a reason for hiding this comment

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

new_from_slice sounds good! from_slice is shorter, but I think will be more confusing, since it could be mistaken for a specialized version of the From trait. Also usually from* methods imply simple conversion, which is not the case here.

i wonder if for stream cipher and block modes we should use plural new_from_slices, since they accept two slices.

Copy link
Member Author

Choose a reason for hiding this comment

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

@newpavlov sounds good

@tarcieri
Copy link
Member Author

Going to go ahead and land this. Hopefully that's ok!

@tarcieri tarcieri merged commit d5d137d into master Dec 30, 2020
@tarcieri tarcieri deleted the cipher/v03_changes_simplified branch December 30, 2020 01:38
tarcieri added a commit to RustCrypto/stream-ciphers that referenced this pull request Dec 30, 2020
Implements the changes from:

RustCrypto/traits#435

Unfortunately there's a circular dependency with `aes`, which now pulls
in `ctr`, so the tests which depend on AES are failing.

The plan is to land these changes first, then update the `block-ciphers`
repository, then circle back and update the `aes` crate dependencies
used in this repo.
tarcieri added a commit that referenced this pull request Dec 30, 2020
This reverts the implementation of `SeekNum::from_block_byte` which was
merged as part of #435.

I'm not exactly sure what the issue is and it's somewhat difficult to
debug in that it's code that involves both generics and macros causing
an error as part of any failure in a long chain of checked arithmetic,
which I'm trying to debug from the context of a concrete stream cipher
impl (both `chacha20` and `salsa20`) where it's operating over a generic
type.

The error manifests as `OverflowError`:

RustCrypto/stream-ciphers#205 (comment)

This commit reverts to the previous implementation, which is at least
much simpler.
tarcieri added a commit that referenced this pull request Dec 30, 2020
This reverts the implementation of `SeekNum::from_block_byte` which was
merged as part of #435.

I'm not exactly sure what the issue is and it's somewhat difficult to
debug in that it's code that involves both generics and macros causing
an error as part of any failure in a long chain of checked arithmetic,
which I'm trying to debug from the context of a concrete stream cipher
impl (both `chacha20` and `salsa20`) where it's operating over a generic
type.

The error manifests as `OverflowError`:

RustCrypto/stream-ciphers#205 (comment)

This commit reverts to the previous implementation, which is at least
much simpler.
tarcieri added a commit to RustCrypto/stream-ciphers that referenced this pull request Dec 30, 2020
Implements the changes from:

RustCrypto/traits#435

Unfortunately there's a circular dependency with `aes`, which now pulls
in `ctr`, so the tests which depend on AES are failing.

The plan is to land these changes first, then update the `block-ciphers`
repository, then circle back and update the `aes` crate dependencies
used in this repo.
tarcieri added a commit to RustCrypto/stream-ciphers that referenced this pull request Dec 30, 2020
Implements the changes from:

RustCrypto/traits#435

Unfortunately there's a circular dependency with `aes`, which now pulls
in `ctr`, so the tests which depend on AES are failing.

The plan is to land these changes first, then update the `block-ciphers`
repository, then circle back and update the `aes` crate dependencies
used in this repo.
tarcieri added a commit to RustCrypto/stream-ciphers that referenced this pull request Dec 30, 2020
Implements the changes from:

RustCrypto/traits#435

Unfortunately there's a circular dependency with `aes`, which now pulls
in `ctr`, so the tests which depend on AES are failing.

The plan is to land these changes first, then update the `block-ciphers`
repository, then circle back and update the `aes` crate dependencies
used in this repo.
tarcieri added a commit to RustCrypto/block-ciphers that referenced this pull request Dec 30, 2020
Implements the API changes introduced in:

RustCrypto/traits#435
tarcieri added a commit to RustCrypto/block-ciphers that referenced this pull request Dec 30, 2020
Implements the API changes introduced in:

RustCrypto/traits#435
tarcieri added a commit to RustCrypto/block-ciphers that referenced this pull request Dec 30, 2020
Implements the API changes introduced in:

RustCrypto/traits#435
tarcieri added a commit to RustCrypto/block-ciphers that referenced this pull request Dec 30, 2020
Implements the API changes introduced in:

RustCrypto/traits#435
tarcieri added a commit to RustCrypto/block-ciphers that referenced this pull request Dec 30, 2020
Implements the API changes introduced in:

RustCrypto/traits#435
@newpavlov
Copy link
Member

Hopefully that's ok!

No problem! Sorry, I couldn't finish the PR myself.

tarcieri added a commit that referenced this pull request Dec 30, 2020
Renames `new_var`(key) methods to use `new_from_slice(s)` as discussed
earlier here:

#435 (comment)
tarcieri added a commit that referenced this pull request Dec 30, 2020
Renames `new_var`(key) methods to use `new_from_slice(s)` as discussed
earlier here:

#435 (comment)
@tarcieri tarcieri mentioned this pull request Apr 28, 2021
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.

None yet

2 participants