Conversation
| @@ -0,0 +1,292 @@ | |||
| { | |||
There was a problem hiding this comment.
I used Claude to help populate the "reason" fields, and it would be a good idea to do a second pass to make sure it hasn't missed the mark.
Probably also worth filing issues for some of the cases where it makes sense to change the verifier to align with the expected result.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #467 +/- ##
==========================================
+ Coverage 96.71% 96.94% +0.22%
==========================================
Files 20 20
Lines 3927 3927
==========================================
+ Hits 3798 3807 +9
+ Misses 129 120 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| #[test] | ||
| fn x509_limbo() { | ||
| let mut data_file = | ||
| File::open("third-party/x509-limbo/limbo.json.bz2").expect("failed to open data file"); |
There was a problem hiding this comment.
Ideally we could get this content from the dev-dep on the harness so that they A) stay in sync B) we only have one thing to update over time.
I'll give this more thought but wanted to get a first pass up for review before making it perfect.
cpu
force-pushed
the
ci/cpu-x509-limbo-rebase
branch
from
4408e41 to
c52384b
Compare
| .expect("invalid validation time!"), | ||
| ); | ||
|
|
||
| let sig_algs = rustls_aws_lc_rs::ALL_VERIFICATION_ALGS; |
There was a problem hiding this comment.
nb: didn't feel like it made much sense to test both aws-lc-rs and ring 🤷
cpu
force-pushed
the
ci/cpu-x509-limbo-rebase
branch
from
c52384b to
5c206eb
Compare
This adds x509-limbo coverage using a vendored copy of the limbo.json test data (bzipped to reduce the size from 39mb to 7.3mb). We take a git dev dep on the upstream x509-limbo repo to reuse its harness helpers. By doing this we can catch regressions proactively as part of the development cycle instead of reactively when the upstream x509-limbo project updates published webpki releases. The test rigging is similar to the upstream "rustls-webpki" harness, except that it tests against the expected outcomes per-testcase mod an exceptions JSON file. That exceptions file is pre-populated based on the current divergences listed on the x509-limbo.com website[0]. Some of these divergences may motivate changes in the verifier that will remove the exception, while others (e.g. not honoring EE cert CNs) will always remain due to explicit design choices of this crate. The test is ignored by default, because the runtime is longer than the other tests. With this in place we can also remove the ignored by default bettertls coverage, since that project is included as a subset[1] of the x509-limbo test cases. [0]: https://x509-limbo.com/anomalous-results/rustls-webpki/ [1]: https://x509-limbo.com/testcases/bettertls/
cpu
force-pushed
the
ci/cpu-x509-limbo-rebase
branch
from
5c206eb to
cea4e45
Compare
| [bans] | ||
| wildcards = "deny" | ||
| # Allow git/path dev-dependencies (like limbo-harness-support) without version specs | ||
| allow-wildcard-paths = true |
There was a problem hiding this comment.
I think based on the docs this is what we want and (emphasis mine):
path or git dependencies and build-dependencies in public crates will continue to produce warnings and errors.
1 participant
This adds x509-limbo coverage using a vendored copy of the limbo.json test data (bzipped to reduce the size from 39mb to 7.3mb). We take a git dev dep on the upstream x509-limbo repo to reuse its harness helpers. By doing this we can catch regressions proactively as part of the development cycle instead of reactively when the upstream x509-limbo project updates published webpki releases.
The test rigging is similar to the upstream "rustls-webpki" harness, except that it tests against the expected outcomes per-testcase mod an exceptions JSON file. That exceptions file is pre-populated based on the current divergences listed on the x509-limbo.com website. Some of these divergences may motivate changes in the verifier that will remove the exception, while others (e.g. not honoring EE cert CNs) will always remain due to explicit design choices of this crate.
The test is ignored by default, because the runtime is longer than the other tests.
With this in place we can also remove the ignored by default bettertls coverage, since that project is included as a subset of the x509-limbo test cases.