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

add an optional dependency to the cipher implementations in block-mode #3

Open
lovasoa opened this issue Jun 7, 2020 · 10 comments
Open

Comments

@lovasoa
Copy link

lovasoa commented Jun 7, 2020

The block-mode crate is generic and has to be used with concrete ciphers. Currently, the crate does not depend on any cipher, and it is a matter of trial and error to find out which version of block-mode is compatible with which version of a concrete cipher, causing issues like RustCrypto/block-ciphers#127. It would be very helpful if block-mode added optional dependencies to concrete ciphers. This would let the package manager (cargo) take care of the version management instead of having to do it manually.

Instead of having

# Cargo.toml
block-modes = "0.4"
aes-soft = "0.4"
use block_modes::{BlockMode, Cbc};
use block_modes::block_padding::Pkcs7;
use aes::Aes128;

type Aes128Cbc = Cbc<Aes128, Pkcs7>;

one would be able to write :

# Cargo.toml
block-modes = { version = "0.4", features = ["aes"] }
use block_modes::{BlockMode, Cbc, Aes128};
use block_modes::block_padding::Pkcs7;

type Aes128Cbc = Cbc<Aes128, Pkcs7>;

Of course, that wouldn't prevent users from using other ciphers, but the library would provide a set of ciphers that are guaranteed to work.

@tarcieri
Copy link
Member

tarcieri commented Jun 7, 2020

I think this might make sense the other way around: if each cipher were to include block-modes as an optional dependency.

The reason is the number of ciphers we support is open-ended.

@lovasoa
Copy link
Author

lovasoa commented Jun 7, 2020

Yes, it would also work. Can I go ahead and open a pull request ?

@tarcieri
Copy link
Member

tarcieri commented Jun 7, 2020

Sure. Curious what @newpavlov thinks though.

@lovasoa
Copy link
Author

lovasoa commented Jun 7, 2020

Ok, let's wait for @newpavlov to give a green light, and if we agree to do that, I'll implement it.

@newpavlov
Copy link
Member

I don't particularly have objections against adding block-modes as an optional dependency to block cipher crates, but I am not sure if we should keep it after 1.0 releases. After all, the problem with version synchronization has only surfaced during publication of the new crate versions. My guess is that block-modes will be used less compared to other modes (e.g. CTR and authenticated modes), so the question is: why give it a preferential treatment?

@lovasoa
Copy link
Author

lovasoa commented Jun 8, 2020

Why remove it after 1.0 ? Unless you are planning to yank all the 0.x versions and never publish a 2.x version, I don't see a reason to come back to manual dependency management after 1.0 . As long as various versions that are incompatible between themselves exist on crates.io, there should be a way for the package manager to tell which is compatible with which.
And there is no reason to give block-modes a preferential treatment. I don't use other modes, but you should probably fix this dependency management issue for all the crates for which it exists.

@newpavlov
Copy link
Member

Because if your dependency version policy is "only latest versions", then compatibility problem will not exist after 1.0. If you are using older pre-1.0 versions, then it's your responsibility to ensure compatibility between crates. Yes, ideally we should have rolled a big coordinated update of all crates and don't stretch it in time, but it's a harder thing to pull off compared to a gradual update.

Adding block-modes as an optional dependency is giving it a preferential treatment. Why should block cipher crates expose only block-modes? What about ctr, cfb-mode, cfb8, ofb, cmac, pmac crates and other constructs (e.g. SIV) which are built on top of block ciphers? Adding all those crates as optional dependencies will only increase coupling and churn for maintainers with really minor benefits.

there should be a way for the package manager to tell which is compatible with which

Having nicer error messages around trait version mismatches certainly would be nice (I think there should be an issue for that in rust-lang/rust, but I couldn't find it with a cursory search), but I don't think it's an area of responsibility for this project. We could alleviate some pain with workarounds like this one, but I don't think it's a good long-term solution.

@lovasoa
Copy link
Author

lovasoa commented Jun 8, 2020

Because if your dependency version policy is "only latest versions"

I don't have a dependency version policy; I rely on my package manager to make sure there are no incompatibilities between my various dependencies.

Adding block-modes as an optional dependency is giving it a preferential treatment.

Yes, you are right. What I was suggesting was doing the same for other dependencies; I was just saying that I wasn't ready to do that myself since I don't use them.

I do agree that it results in more work for the maintainers. Especially if you want to have dependencies from ciphers to modes and not from modes to ciphers. I think that getting to rely on the package manager for dependency version management is not a minor benefit, but of course it's up to you to decide what you are ready to maintain.

If you do not want to make this change, then could we at least document in the README, the documentation, and the changelog of the block crates which other crate versions they are compatible with ? This way users would still have to manage dependencies manually, but they wouldn't have to rely on trial and error to do so.

@tarcieri
Copy link
Member

tarcieri commented Jun 8, 2020

It seems less necessary after 1.0 because you'll know 1.0 crates are compatible with each other, but that said, it still seems convenient.

I'm not sure when we'll even be able to cut 1.0s, however. It seems like we'd either need generic-array 1.0 (fizyk20/generic-array#101) or const generics, both of which don't have clear roadmaps. So to me this seems like a reasonable stopgap for now.

@newpavlov
Copy link
Member

I don't think we should release 1.0s before const generics stabilization plus some "cool down" period, so I guess it will be 1.5-2 years at least. So as I wrote earlier I am not opposed to adding it to the current versions, but I doubt we should also add all other crates in addition to block-modes.

@newpavlov newpavlov transferred this issue from RustCrypto/block-ciphers Jan 11, 2022
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

No branches or pull requests

3 participants