Skip to content

Commit

Permalink
bogo: implement most client-side ECH tests
Browse files Browse the repository at this point in the history
Getting bogo test coverage working for ECH requires taking a (temporary)
dev-dep. on the provider-example crate so we can use the Rust Crypto
HPKE provider. This in turn means that we now have two Rustls versions
in tree, the src crate and the older version being used by the
provider example by way of hickory-dns. This will fall away when proper
HPKE support is implemented with one/both built-in crypto providers but
in the meantime requires some small adjustments in CI and the runme
script. In general the scope of a proper HPKE impl will be much less
invasive than implementing ECH so we can use this hack for now and
revisit shortly.

The bogo shim also requires some updates to support new command line
flags. Additionally in order to be able to assert some details in errors
(e.g. that an ECH required err contained expected retry configs) we have
to pipe the `Options` struct deeper into the client/server processing
logic.

Beyond these changes, it's worth discussing the ignored tests:

First and foremost, we're ignoring all of the ECH bogo tests when
running w/ aws-lc-rs as the crypto provider. I'm not sure why yet, but
there's a panic from the HKDF code with that provider. This needs to be
investigated.

There are also 5 tests ignored when testing w/ ring that need
further investigation. I think they're bugs in our impl:

"TLS-ECH-Client-EarlyData":
  This is a TODO; we generate an unexpected bad MAC error.

"TLS-ECH-Client-Reject-HelloRetryRequest":
  This is a TODO; we end up with no retry configs in our error when
  there should be some.

"TLS-ECH-Client-Reject-ResumeInnerSession-TLS13":
  This is a TODO; we reject ECH when the test expects to get an
  unexpected extension error.

"TLS-ECH-GREASE-Client-TLS13-HelloRetryRequest"
"TLS-ECH-Client-GREASE-IgnoreHRRExtension":
  These are a TODO; these both give an unexpected extension change
  error (the ECH ext). I think it's a bug with GREASE + HRR?

We also ignore a handful of other tests, but not because I think we have
any bugs. They're either not applicable, or need upstream bogo fixes.

"TLS-ECH-Server*":
  We ignore all the TLS-ECH-Server tests. We haven't implemented server
  side ECH yet

"TLS-ECH-Client-ExpectECHOuterExtensions"
"TLS-ECH-Client-CompressSupportedVersions":
  These rely on extension compression between inner/outer hellos. NYI.

"TLS-ECH-Client-SelectECHConfig"
"TLS-ECH-Client-NoSupportedConfigs"
  These are meant to test unsupported configs are handled correctly: we
  happen to support the HPKE ciphersuites that make them "unsupported".
  There's a fix for this upstream we can take later.

"TLS-ECH-Client-SkipInvalidPublicName*":
  Our name validation allows underscores in names. We also don't
  fallback to GREASE when there are no valid ECH configs.

"TLS-ECH-Client-NoSupportedConfigs-GREASE":
  We don't fallback to GREASE for no ECH configs.

"TLS-ECH-Client-TLS12-RejectRetryConfigs"
"TLS-ECH-Client-Reject-NoClientCertificate-TLS12"
"TLS-ECH-Client-Reject-TLS12"
"TLS-ECH-Client-Reject-ResumeInnerSession-TLS12"
"TLS-ECH-GREASE-Client-TLS12-RejectRetryConfigs"
"TLS-ECH-Client-Reject-EarlyDataRejected-TLS12"
"TLS-ECH-Client-Reject-NoClientCertificate-TLS12-Async"
  We never offer/support TLS 1.2 when using ECH. There's no fallback to
  plaintext or GREASE for a server that won't support TLS 1.3
  • Loading branch information
cpu committed Apr 25, 2024
1 parent ed60945 commit 2572e58
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 29 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,13 @@ jobs:
uses: dtolnay/rust-toolchain@nightly

- name: Smoke-test benchmark program (ring)
run: cargo run -p rustls --profile=bench --locked --example bench
run: cargo run -p rustls@0.23.5 --profile=bench --locked --example bench

- name: Smoke-test benchmark program (aws-lc-rs)
run: cargo run -p rustls --profile=bench --locked --example bench --no-default-features --features aws_lc_rs,tls12,std
run: cargo run -p rustls@0.23.5 --profile=bench --locked --example bench --no-default-features --features aws_lc_rs,tls12,std

- name: Smoke-test benchmark program (fips)
run: cargo run -p rustls --profile=bench --locked --example bench --no-default-features --features fips,tls12,std
run: cargo run -p rustls@0.23.5 --profile=bench --locked --example bench --no-default-features --features fips,tls12,std

- name: Run micro-benchmarks
run: cargo bench --locked --all-features
Expand All @@ -258,7 +258,7 @@ jobs:
uses: dtolnay/rust-toolchain@nightly

- name: cargo doc (rustls; all features)
run: cargo doc --locked --all-features --no-deps --document-private-items --package rustls
run: cargo doc --locked --all-features --no-deps --document-private-items --package rustls@0.23.5
env:
RUSTDOCFLAGS: -Dwarnings

Expand Down Expand Up @@ -403,7 +403,7 @@ jobs:
components: clippy
# because examples enable rustls' features, `--workspace --no-default-features` is not
# the same as `--package rustls --no-default-features` so run it separately
- run: cargo clippy --locked --package rustls --no-default-features --all-targets -- --deny warnings
- run: cargo clippy --locked --package rustls@0.23.5 --no-default-features --all-targets -- --deny warnings
- run: cargo clippy --locked --workspace --all-features --all-targets -- --deny warnings
# not part of the workspace
- run: cargo clippy --locked --manifest-path=fuzz/Cargo.toml --all-features --all-targets -- --deny warnings
Expand All @@ -420,7 +420,7 @@ jobs:
uses: dtolnay/rust-toolchain@nightly
with:
components: clippy
- run: cargo clippy --locked --package rustls --no-default-features --all-targets
- run: cargo clippy --locked --package rustls@0.23.5 --no-default-features --all-targets
- run: cargo clippy --locked --workspace --all-features --all-targets
- run: cargo clippy --locked --manifest-path=fuzz/Cargo.toml --all-features --all-targets

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 25 additions & 1 deletion bogo/config.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,31 @@
"ServerOCSPCallback*": "",
"CertCompression*": "not implemented",
"DuplicateCertCompressionExt*": "",
"TLS-ECH-*": "",
"TLS-ECH-Server*": "ECH server support NYI",
#ifdef RING
"TLS-ECH-Client-ExpectECHOuterExtensions": "ECH extension compression NYI",
"TLS-ECH-Client-CompressSupportedVersions": "ECH extension compression NYI",
"TLS-ECH-Client-SelectECHConfig": "TODO(XXX): re-enable after upstream bogo fix",
"TLS-ECH-Client-NoSupportedConfigs": "TODO(XXX): re-enable after upstream bogo fix",
"TLS-ECH-Client-SkipInvalidPublicName*": "we allow underscore names, don't fallback on no ECH configs",
"TLS-ECH-Client-NoSupportedConfigs-GREASE": "we don't fallback to GREASE for no ECH configs",
"TLS-ECH-Client-TLS12-RejectRetryConfigs": "we disable TLS1.2 w/ ECH",
"TLS-ECH-Client-Reject-NoClientCertificate-TLS12": "we disable TLS1.2 w/ ECH",
"TLS-ECH-Client-Reject-TLS12": "we disable TLS1.2 w/ ECH",
"TLS-ECH-Client-Reject-ResumeInnerSession-TLS12": "we disable TLS1.2 w/ ECH",
"TLS-ECH-GREASE-Client-TLS12-RejectRetryConfigs": "we disable TLS1.2 w/ ECH",
"TLS-ECH-Client-Reject-EarlyDataRejected-TLS12": "we disable TLS1.2 w/ ECH",
"TLS-ECH-Client-Reject-NoClientCertificate-TLS12-Async": "we disable TLS1.2 w/ ECH",
#if 1
"TLS-ECH-Client-EarlyData": "TODO(XXX): fix this test",
"TLS-ECH-Client-Reject-HelloRetryRequest": "TODO(XXX): fix this test",
"TLS-ECH-Client-Reject-ResumeInnerSession-TLS13": "TODO(XXX): fix this test",
"TLS-ECH-GREASE-Client-TLS13-HelloRetryRequest": "TODO(XXX): fix this test",
"TLS-ECH-Client-GREASE-IgnoreHRRExtension": "TODO(XXX): fix this test",
#endif /* 1 */
#else
"TLS-ECH-*": "TODO(XXXX): figure this out...",
#endif /* ring */
"ALPS-*": "",
"*Kyber*": "",
"ExtraClientEncryptedExtension-*": "we don't implement ALPS",
Expand Down
4 changes: 2 additions & 2 deletions bogo/runme
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ set -xe

case ${BOGO_SHIM_PROVIDER:-ring} in
ring)
cargo build -p rustls --example bogo_shim --no-default-features --features ring,tls12,logging,std
cargo build -p rustls@0.23.5 --example bogo_shim --no-default-features --features ring,tls12,logging,std
cpp -P -DRING config.json.in -oconfig.json
;;
aws-lc-rs)
cargo build -p rustls --example bogo_shim --no-default-features --features aws_lc_rs,tls12,logging,std
cargo build -p rustls@0.23.5 --example bogo_shim --no-default-features --features aws_lc_rs,tls12,logging,std
cpp -P -DAWS_LC_RS config.json.in -oconfig.json
;;
existing)
Expand Down
4 changes: 2 additions & 2 deletions fuzz/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions rustls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ rcgen = { version = "0.13", default-features = false, features = ["aws_lc_rs", "
rustls-pemfile = "2"
time = { version = "0.3.6", default-features = false }
webpki-roots = "0.26"
# TODO(@cpu): Remove once HPKE provider backed by aws-lc-rs and/or ring is available.
rustls-provider-example = { path = "../provider-example" }

[target.'cfg(not(target_env = "msvc"))'.dev-dependencies]
tikv-jemallocator = "0.5"
Expand Down

0 comments on commit 2572e58

Please sign in to comment.