Bogo tests all the signature algorithms for client auth already, both
in client and server. Bogo tests TLS 1.3 and TLS 1.2 individually, which
is an important improvement over the redundant tests.
Regarding Ed25519 specifically, BoringSSL doesn't enable it by default,
but it does still test it, e.g.:
```
"Server-Verify-Ed25519-TLS12": {
"actual": "PASS",
"expected": "PASS",
"is_unexpected": false
},
"Server-Verify-Ed25519-TLS13": {
"actual": "PASS",
"expected": "PASS",
"is_unexpected": false
},
```
The implementation of `client_auth_mandatory()` is redundant with the
default implementation. By removing this redundant implementation, we
add test coverage of the default implementation, which is currently
uncovered.
ALPN is now tested in `alpn()` in rustls/tests/api.rs and by the Bogo tests
that have names staring with "ALPN".
Of the BoGo tests, only these are disabled in bogo.json:
```
"ALPN*SelectEmpty-*": "",
```
By removing these tests, we can remove `skipped()`.
These tests are redundant with the Bogo "CurveTest-" tests. Of those,
only the ones that are testing unsupported curves are disabled:
```
"CurveTest-Client-P-521-TLS12": "",
"CurveTest-Server-P-521-TLS12": "",
"CurveTest-Client-Compressed-P-521-TLS12": "",
"CurveTest-Server-Compressed-P-521-TLS12": "",
"CurveTest-Client-P-521-TLS13": "",
"CurveTest-Server-P-521-TLS13": "",
"CurveTest-Client-Compressed-P-521-TLS13": "",
"CurveTest-Server-Compressed-P-521-TLS13": "",
```
Reduce the chances that code for unwanted cipher suites will be linked
in, when not using the `Acceptor` API. This fixes a code size regression
from 0.20.0.
Doing the same for the case where `Acceptor` is used is tracked as
https://github.com/rustls/rustls/issues/973.
Make it easier to find the {Client,Server}CertVerifier tests. Split
them into separate files so that we can use `diff` to compare how we're
testing each.
This removes all of the `dangerous_configuration` tests from tests/api.rs.
Each of the mock Client/Server verifiers were eached move into the single test
suite that uses it.
Use these commands with a whitespace-smart diff tool to see that the tests
were not modified except for whitespace and formatting:
```
git difftool HEAD^1:rustls/tests/api.rs rustls/tests/server_cert_verifier.rs
git difftool HEAD^1:rustls/tests/api.rs rustls/tests/client_cert_verifier.rs
git difftool HEAD^1:rustls/tests/common/mod.rs rustls/tests/client_cert_verifier.rs
git difftool HEAD^1:rustls/tests/common/mod.rs rustls/tests/server_cert_verifier.rs
```
A common pattern in the code is:
```
check_message(...);
match m.payload {
}
```
The check_message call does its own pattern matching very much like what
is immediately done after. Avoid doing that redundant pattern matching.
Allow `inappropriate_handshake_message` to handle cases where
non-handshake messages are also accepted. This simplifies more callers.
I intentionally didn't try to simplify `check_message` because my next
set of commits would remove it.
Make the `read_buf` feature do nothing on non-Nightly Rust.
This makes it easier for people not depending on that feature to
build/test Rustls, as demonstrated by the CI/CD changes here.
Use `rustversion` instead of `rustc_version`; `rustc_version` has more
dependencies.
Serialize all tests that use `std::env::set_var` & isolate them. See the
comments in key_log_file_env.rs for details.
Also add notes about the fact that these tests aren't really testing the
functionality.
Use a whitespace-smart diff tool to compare the new file to what was in
api.rs:
```
git difftool HEAD^1:rustls/tests/api.rs rustls/tests/key_log_file_env.rs
```
`start_incoming_traffic()` doesn't need to exist as a `pub(crate)`
function, or at all, because it is only used by the function right
below it. Inline it into that function to make it clearer when
`may_receive_application_data` is set.
TLS 1.2 client and server `ExpectTraffic`, and TLS 1.3 server `ExpectTraffic` already
don't store `ClientConfig` any longer. The TLS 1.3 client `ExpectTraffic` seems to be
the last instance.
At the same time, support the -resumption-delay flag in bogo_shim.
This is achieved by editing the session data as it is persisted.
This also enables bogo tests that we respect TLS1.2 ticket lifetimes.
This previously existed, but only for QUIC.
There are some unfortunate shortcomings with the protocol design here:
Because the client must send 0-RTT data whether or not the server
accepts it or even the client hello, there must be several
disjoint methods for identifying and skipping these messages. One
of these is in the record_layer.rs, and works by trial decryption.
Another happens if the server rejects the client's hello altogether,
and skips encrypted messages between the two client hellos.
The amount of data to skip is limited but -- because the design
appears to be defective -- the quantity is expressed (in
`max_early_data_size` provided with a ticket) in units of plaintext
bytes, but skipping data requires it in units of padded, tagged
ciphertext bytes. The server cannot compute one from the other,
so we're interpretting `max_early_data_size` as both at the same time.
This means the server can send application data in its first
flight. We only do this, though, if no client auth is in play
(as otherwise we'd be sending data to an unauthenticated peer,
and that would be exceedingly bad.)
This is useful for server-speaks-first protocols, as well as
replying to a 0-RTT request in a client-speaks-first one.
In terms of code changes, this splits start_traffic() into
start_incoming_traffic() and start_outgoing_traffic().
- move unconditional extensions into vec construction
- make supported_versions unconditional: it was conditional because
one with zero elements is illegal, but in fact it is a library
invariant that at last one version is configured.
Be consistent with other modules.
```sh
git diff HEAD^1:rustls/src/keylog.rs rustls/src/key_log.rs
git diff HEAD^1:rustls/src/keylogfile.rs rustls/src/key_log_file.rs
```
Make it clear that `KeyLog` doesn't depend on `std::{env,fs,io,path,sync}`.
Use `git diff main:rustls/src/keylog.rs rustls/src/keylogfile.rs` to
verify that none of the `KeyLogFile` code has changed at all.
This reverts commit 5e3de58514 because it
isn't a hackward compatible change. The `KeyLogFile::new()` constructor
promises that the file is opened within `new()`.
Fixes https://github.com/rustls/rustls/issues/919.
When encoding a TLS vector we previously encoded the items into
a temporary `Vec`, then wrote the length of that `Vec` into the
output stream, then copied the temporary `Vec` into the output.
Instead, we can write the correct amount of zero bytes into the
output stream before writing the items, then overwrite the zero
bytes with the correct length. This avoids the allocation for
the temporary `Vec` and copying the item data from the temporary
`Vec` into the output stream.
Brian Smith
Dirkjan Ochtman
Joseph Birr-Pixton
Klim Tsoutsman
Alex Rawson