Previously, calls to `complete_io()` may return as if handshaking has
completed, but leave pending TLS writes queued that won't be sent until
a subsequent call to `complete_io()` is made.
This happens because `is_handshaking()` can begin to return false after
calls to `process_new_packets()` while there are final handshake packets
put in the connection's buffers, but not yet extracted to be sent to the
peer.
The end result is that calling `complete_io()` once is not
sufficient to fully complete a handshake with a peer. A second call
was required to flush the pending packets.
In this commit the `complete_io()` logic is updated to continue
processing IO when calling `process_new_packets()` has queued TLS
writes, only returning to the caller when all pending IO has been dealt
with and the handshake truly completed.
We can test this behaviour by updating the
`client_complete_io_for_handshake` and
`server_complete_io_for_handshake` unit tests to assert there are no
pending TLS writes after calling `complete_io()`. Prior to this commit
these assertions would fail, and with the updated logic they pass as
expected.
Previously we linked to the *ring* README to describe Ring's supported
architectures in more detail. Unfortunately that section of the upstream
README was removed without a replacement.
This commit emphasizes that while Rustls is platform independent, *ring*
is not. To replace the detailed platform support information we now link
directly to the relevant *ring* CI configuration for the version in use
by Rustls.
Previously the `ClientCertVerifier` rust docs had a broken link to the
`Error::InvalidCertificate(CertificateError::BadEncoding)` type.
This commit breaks up the link into two parts, one for the
`Error::InvalidCertificate` variant and one for the
`CertificateError::BadEncoding` variant.
Previously when an illegal SNI hostname was received rustls would
`warn!` the received value as a raw `Vec<u8>`, making it hard for
a human to read the value received.
This commit changes to `warn!` the `from_utf8_lossy` string version of
the hostname. This will make it easier for end users to diagnose the
root cause.
This commit removes two "Future release" items from the release history
section of the README.
It seems clearer to have this section dedicated to the release history,
not upcoming work. I also think the two described pieces of work might
not be what the project is currently prioritizing.
Originally developed in #1259.
Co-authored-by: Daniel McCarney <daniel@binaryparadox.net>
Co-authored-by: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
In #862, we added CIFuzz integration. This is great, but it runs until a
specific timeout (currently 600s), in addition to needing about 6 minutes
of setup time. This is more than 3 times as long as the next longest CI job
(coverage took about 4m in #1242 just now). Given that we already have
OSS-Fuzz spending quite a bit of time running fuzzing for us and that (as
far as I remember) we have yet to find any issues from the CIFuzz integration,
I feel safe reducing the runtime for CIFuzz to 150s. This should still cover
quite a bit of ground given that we're executing pretty fast Rust code.
This commit adds two `allow` directives for clippy warnings present when
building with `--no-default-features`:
1. Ignore `clippy::unnecessary_lazy_evaluations` for `find_session`. The
suggestion to use `or` instead of `or_else` to avoid unnecessary lazy
evaluation breaks a unit test
(`test_client_tls12_no_resume_after_server_downgrade`).
2. Ignore `clippy::bind_instead_of_map` for `handle`. The suggestion to
use `map` doesn't play well with the inner `match` that has a `None`
arm for TLS 1.2 feature builds.
Previously we ran clippy (both stable and nightly) only for
`--all-features` builds. This allows warnings specific to
`--no-default-features` to slip by.
This commit adds clippy invocations that build w/
`--no-default-features` so we can catch warnings specific to this
configuration during CI.
The `common` field of the `Tls12ClientSessionValue` type is only used
when the "tls12" feature is enabled.
To avoid an unused clippy err and to reduce the size of the struct when
not using TLS1.2 this commit feature-gates the field on "tls12".
This removes a requirement that an implementation of ClientCertVerifier
produce a fresh Vec of acceptable root Subjects on each call. Instead,
the ClientCertVerifier can store a list of acceptable root subjects and
return references to it, which seems like the most common use case by far.
For client_auth_mandatory and client_auth_root_subjects, it was possible
to return None to abort the connection. With the removal of the `sni`
input parameter, this no longer makes sense, so remove the
Option-wrapping of these return values.
The previous comment added to `RootCertStore.add` pointing out the
existence of `add_parsable_certificates` didn't offer enough clarity
around when to prefer `add` vs `add_parsable_certificates`. This commit
tweaks the language further based on review feedback.
Previously the example directories weren't being tested with
`--no-default-features`, letting bitrot affect those configurations.
This commit includes those directories in the `--no-default-features`
task that run `cargo test`.
This commit simplifies the examples sub project to make logging
mandatory instead of an optional feature flag.
In general this is easier to reason about for small example code, and it
resolves a build error that was present when building w/
`--no-default-features` due to the unconditional use of the `log` crate.
Previously this was removed when adding `Arc<dyn StdError>` as an enum
field for CertificateError, which made it impossible to automatically
derive PartialEq for the whole enum. Trait objects (dyn Trait) don't
implement PartialEq.
However, we can get back PartialEq on the whole Error struct by manually
implementing it for CertificateError, considering `Other` values to
never be equal.
Dirkjan Ochtman
Daniel McCarney
Steven Fackler
dependabot[bot]
Dennis Collinson
Jacob Hoffman-Andrews