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

0.22 release planning #1435

Closed
12 of 15 tasks
djc opened this issue Aug 31, 2023 · 16 comments · Fixed by #1651
Closed
12 of 15 tasks

0.22 release planning #1435

djc opened this issue Aug 31, 2023 · 16 comments · Fixed by #1651

Comments

@djc
Copy link
Member

djc commented Aug 31, 2023

Let's gather some context on what we should fix before pushing out the next semver-incompatible release.

Some potential validation we could do:

@djc
Copy link
Member Author

djc commented Aug 31, 2023

@jsha anything else on your mind?

@jsha
Copy link
Contributor

jsha commented Sep 1, 2023

Filed #1437; that's all I can think of at the moment. Thanks for organizing this list.

@djc
Copy link
Member Author

djc commented Sep 14, 2023

Some updates:

@cpu
Copy link
Member

cpu commented Sep 14, 2023

#1386
#1437

I've started working on both of these.

@ctz
Copy link
Member

ctz commented Sep 14, 2023

I'd like to get

in too.

@djc djc pinned this issue Oct 2, 2023
@ctz
Copy link
Member

ctz commented Nov 17, 2023

As mentioned on that ticket, I think we should drop #1372 for 0.22 and do it 0.23.

@djc
Copy link
Member Author

djc commented Nov 17, 2023

What's your rationale for wanting to push it out to the next release?

@ctz
Copy link
Member

ctz commented Nov 17, 2023

I think there's some imperative to release 0.22 pretty soon, as PRs we don't really need or want in there are getting closer and/or landing already. I don't want to be keeping back PRs that are otherwise ready, or even worse backing them out, or having multiple trunks, etc.

@cpu
Copy link
Member

cpu commented Nov 17, 2023

Selfishly I'd also like rustls-ffi to be ready for the 0.22 release as close to the time we cut the release as possible. Refactoring the builders will require another substantial-ish set of changes to rustls/rustls-ffi#341 and getting those ready + reviewed will take more time.

@djc
Copy link
Member Author

djc commented Nov 20, 2023

Removed #1372 from the list.

@jsha
Copy link
Contributor

jsha commented Nov 21, 2023

I wanted to look through changes to the public API to make sure we got everything in the draft changelog at https://github.com/rustls/rustls/releases/tag/untagged-725839514beefeccaf57.

I visited the rustdoc for 0.21.9 and latest main, went to "All Items", copy-pasted, filtered with %s/\(.*\)::\([^:]*\)/\2 \1 to put the type name first and the path last, sorted, and diffed, then manually looked to see whether the differences were mentioned already. Some things I found:

SignError is a weird type. It says "Errors while signing", but is only returned by these six functions. In other words, it's used as a parse error, not a signing error.

rustls::crypto::aws_lc_rs::sign::any_ecdsa_type
rustls::crypto::ring::sign::any_ecdsa_type
rustls::crypto::aws_lc_rs::sign::any_eddsa_type
rustls::crypto::ring::sign::any_eddsa_type
rustls::crypto::aws_lc_rs::sign::any_supported_type
rustls::crypto::ring::sign::any_supported_type

Since we consider the top level namespace to be important, "well paved path" stuff, I thought it would be useful to list all the top level items in particular in this issue to see if they are as expected, and if they require special callouts in the changelog.

Removed from top level

  • ALL_CIPHER_SUITES
  • ALL_KX_GROUPS
  • BulkAlgorithm
  • Certificate
  • Constants
  • DEFAULT_CIPHER_SUITES
  • OwnedTrustAnchor
  • PrivateKey
  • SupportedKxGroup
  • Ticketer

Added to top level

  • CipherSuiteCommon
  • OtherError
  • TicketSwitcher
  • WebPkiSupportedAlgorithms

I've attached old.txt and new.txt in case you'd like to look at the difference yourself. Note this doesn't get into method signatures, just structs / traits / enums / etc.

@jsha
Copy link
Contributor

jsha commented Nov 22, 2023

Here's another view, Moved / Renamed / Added / Removed. As a user of a library I appreciate changelogs that have each modified item listed out explicitly, so when I get a compile error upon migrating, I can quickly check "was this removed outright or do I just need to import it under a different name?"

Moved

  • ALL_CIPHER_SUITES (crypto providers)
  • ALL_KX_GROUPS (crypto providers)
  • DEFAULT_CIPHER_SUITES (crypto providers)
  • SECP256R1 (crypto providers)
  • SECP384R1 (crypto providers)
  • any_ecdsa_type (crypto providers)
  • any_eddsa_type (crypto providers)
  • any_supported_type (crypto providers)
  • CipherSuiteCommon (crypto)
  • ClientCertVerified (server::danger)
  • ClientCertVerifier (server::danger)
  • DangerousClientConfig (client::danger)
  • DnsName (server::danger)
  • HandshakeSignatureValid (client::danger)
  • ServerCertVerified (client::danger)
  • ServerCertVerifier (client::danger)
  • SupportedKxGroup (crypto)
  • Ticketer (crypto providers)
  • TLS13_AES_128_GCM_SHA256 (crypto providers)
  • TLS13_AES_256_GCM_SHA384 (crypto providers)
  • TLS13_CHACHA20_POLY1305_SHA256 (crypto providers)
  • TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (crypto providers)
  • TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 (crypto providers)
  • TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (crypto providers)
  • TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (crypto providers)
  • TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (crypto providers)
  • TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (crypto providers)
  • X25519 (crypto providers)

Renamed

  • WebPkiVerifier (client::WebPkiServerVerifier)
  • Certificate (rustls_pki_types::CertificateDer)
  • PrivateKey (rustls_pki_types::PrivateKeyDer)
  • SignError (crypto::ring::sign::InvalidKeyError)

Added

  • ActiveKeyExchange (crypto)
  • AeadKey (crypto::cipher)
  • Algorithm (quic)
  • ClientCertVerifierBuilder (server)
  • DangerousClientConfigBuilder (client::danger)
  • expand (crypto::tls13)
  • OkmBlock (crypto::tls13)
  • OutputLengthError (crypto::tls13)
  • ServerCertVerifierBuilder (client)
  • TicketSwitcher (ticketer)
  • WebPkiClientVerifier (server)

Added (crypto provider extensibility)

  • RING (crypto::ring)
  • AWS_LC_RS (crypto::aws_lc_rs)
  • HashAlgorithm (crypto::hash)
  • Hash (crypto::hash)
  • Hkdf (crypto::tls13)
  • HkdfExpander (crypto::tls13)
  • HkdfExpanderUsingHmac (crypto::tls13)
  • HkdfUsingHmac (crypto::tls13)
  • Hmac (crypto::hmac)
  • Iv (crypto::cipher)
  • KeyBlockShape (crypto::cipher)
  • Key (crypto::hmac)
  • KeyExchangeAlgorithm (crypto)
  • make_tls12_aad (crypto::cipher)
  • make_tls13_aad (crypto::cipher)
  • MessageDecrypter (crypto::cipher)
  • MessageEncrypter (crypto::cipher)
  • Nonce (crypto::cipher)
  • Nonce (crypto::cipher)
  • OpaqueMessage (crypto::cipher)
  • Output (crypto::hash)
  • PlainMessage (crypto::cipher)
  • Prf (crypto::tls12)
  • PrfUsingHmac (crypto::tls12)
  • SharedSecret (crypto)
  • Tag (crypto::hmac)
  • Tls12AeadAlgorithm (crypto::cipher)
  • UnsupportedOperationError (crypto::cipher)
  • WebPkiSupportedAlgorithms (crypto)

Added (error types)

  • GetRandomFailed (crypto)
  • OtherError (top level)
  • UnsupportedOperationError (crypto::cipher)
  • VerifierBuilderError (client)
  • VerifierBuilderError (server)

Removed

  • AllowAnyAnonymousOrAuthenticatedClient
  • AllowAnyAuthenticatedClient
  • BulkAlgorithm
  • CertificateTransparencyPolicy
  • supported_sign_tls13
  • WantsTransparencyPolicyOrClientCert
  • OwnedTrustAnchor

@jsha
Copy link
Contributor

jsha commented Nov 30, 2023

One theme in the upcoming release is that there are several new error types:

GetRandomFailed (crypto)
OtherError (top level)
UnsupportedOperationError (crypto::cipher)
VerifierBuilderError (client)
VerifierBuilderError (server)

OtherError was added so that crypto providers can return arbitrary errors and bundle them into a variant of rustls::Error.

GetRandomFailed and UnsupportedOperationError seem to suggest an API direction where crypto providers return more specific error types, and rustls itself is the only source of rustls::Error. But OtherError seems to run counter to that.

VerifierBuilderError in client and server are specific and are generated from within rustls.

Do we have a philosophy of when we want to use the big rustls::Error enum vs when we want to have a specialty error type?

As another example: in each provider, any_supported_type, any_ecdsa_type, etc return the specific type InvalidKeyError. But when those get called via the CryptoProvider::load_private_key method, that error type is mapped directly into Error::General(String::from("invalid private key"). Perhaps we don't need a specific InvalidKeyError and can return Error::General directly from those functions?

@djc
Copy link
Member Author

djc commented Nov 30, 2023

GetRandomFailed existed before the random API became public. For well-defined concrete APIs it can be helpful to have smaller error types. For APIs that are generic (that is, methods defined in public traits) we've found it helpful to have the wider Error type because downstream implementers might need more latitude -- see #1575.

@ctz ctz mentioned this issue Nov 30, 2023
@djc
Copy link
Member Author

djc commented Dec 1, 2023

FYI, release preparation for the final 0.22.0 release is happening in #1651.

@ctz ctz closed this as completed in #1651 Dec 1, 2023
@djc djc unpinned this issue Dec 2, 2023
@djc
Copy link
Member Author

djc commented Dec 2, 2023

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 a pull request may close this issue.

4 participants