Structural refactor: Code Part #103
Conversation
|
There's something wrong with the dependencies? |
|
@stevefan1999-personal Unfortunately, even if you cargo update all the crates at this point, the dependencies are still broken and they cannot be built correctly. As you know, RustCrypto is broken down into separate features, so you need to carefully find a cut that will allow you to update your dependencies correctly. The reason I can continue to pull off this trick in #102 is because I periodically run the following command on my laptop and commit & push when it passes: Here's a suggestion: First, let's accept #102 into main. Then, by rebasing this PR, you'll at least be able to focus on what you want to do without having to worry about the consistency of existing crates. If there are any crates you want to add, just run We will continue to work on improving Dependabot's ability to consistently update the rustcrypto crate. @tarcieri How do you like it? |
|
This is just my opinion, but I think this PR would be easier for everyone to understand if it were further broken down into smaller sections with different purposes. For example (I'm not sure if this is correct), I think there is a way to do it like this.
|
|
@stevefan1999-personal perhaps we could get something like #102 merged first, and then you could merge/rebase from there? I agree with @nabetti1720's comment |
| default = ["std", "tls12", "zeroize", "full", "fast", "quic", "ticketer"] | ||
| full = [ | ||
| "aead-full", | ||
| "sign-full", | ||
| "verify-full", | ||
| "kx-full", | ||
| "hash-full", | ||
| "format", | ||
| ] |
There was a problem hiding this comment.
Users of this crate will want to use it in an appropriate scope to avoid increasing its footprint.
In that sense, we thought the cipher suite support shown in README.md was optimal for now. (The modest description states that "it should be well enough to cover 70% of the usage," but we believe it will cover most use cases.)
https://github.com/RustCrypto/rustls-rustcrypto?tab=readme-ov-file#supported-cipher-suites
If you want to use only these cipher suites, what features should you choose?
I think it would be better if the feature names that users were allowed to select were more direct, like the cipher suite names.
There was a problem hiding this comment.
Oh, what I wanted is that what you pick is what you only have, so that if I don't want AES-256GCM or if I don't want RSA, I could isolate the possible set of cipher suite for that exactly. The purpose of it is quite hard to explain, but I will try...
That means you need to disable the default features and pick enough features so that at least one cipher suite exist. For example, if you have SHA256, ChaChaPoly1305, P-521 and Ed25519 enabled, that means any features that could use those features will get implemented. I used to think about letting user specify a cipher suite directly, but I guess that would be even more convoluted.
There was a problem hiding this comment.
Your idea is to select the components you want to use and automatically select the cipher suites that satisfy them.
While keeping that idea, having a convenient feature like this would be very helpful for users who want a simpler setup.
// Cargo.toml
# Other cipher suites can be defined in this way.
tls13_chacha20_poly1305_sha256 = ["aead-chacha20poly1305", ...] # Cherry-picked components you need
And ideally, it would be better to keep with the current thinking and enable TLS1.3 cipher suites by default, with the rest being added via tls12. If you set default-features = false, we can specify convenient feature individually, or those with a deep cryptographic understanding may be able to take advantage of more granular feature gates.
3 participants
Seems like #87 is getting a little too big, so I think we need to break it down first.