This is an intermediate step towards moving them into a separate crate.
Leave the `tests` submodule for now, to make the comparison with the
old (identical) code easier. The next commit will remove it and
re-indent the code.
Prior to this commit parsing and processing certificate name constraints
was done before validating a chain of signatures to a known trust
anchor. This increases the attack surface of these features, allowing an
adversary to force webpki to process name constraints on a crafted
certificate without needing to have that certificate issued by a trusted
entity.
This commit moves the parsing and processing of name constraints to
after building and verifying the chain of signatures to reduce the
potential for mischief.
Cherry-picked from e473ee1ecb335d8efa3d4ceb2feb369f46b125f2 and modified
by Brian Smith. The main modifications were:
1. Maintain API compatibility with webpki 0.22.0.
2. (In `build_chain_inner`), stop immediately on fatal error, without
considering any more paths. The point of having such fatal errors
is to fail ASAP and avoid unneeded work in the failure case.
3. The test uses rcgen which requires Rust 1.67.0 or later. (I don't
think the non-test MSRV of webpki changes though.)
The original commit message is below:
Pathbuilding complexity can be quadratic, particularly when the set of
intermediates all have subjects matching a trust anchor. In these cases
we need to bound the number of expensive signature validation operations
that are performed to avoid a DoS on CPU usage.
This commit implements a simple maximum signature check limit inspired
by the approach taken in the Golang x509 package. No more than 100
signatures will be evaluated while pathbuilding. This limit works in
practice for Go when processing real world certificate chains and so
should be appropriate for our use case as well.
[`git cherry-pick f0259b9588bab116c7dfbc62524b98794c90aaef`, merged by Brian Smith.]
Crate-internal consumers of `build_chain` always pass `0` as the sub CA
count, only the `verify_cert.rs` internal recursion changes this
parameter.
This commit separates the external interface from the internal
recursion to remove one extra parameter from an already complicated
interface.
Reset the crate contents (sources, tests, etc.)
to what they were at that commit, while retaining the newer CI
configuration.
The changes since the 0.22.0 release were primarily intended to
accomplish two goals:
* Fix and improve the GitHub Actions configuration.
* Prepare a 0.21.5 release that was backward compatible with 0.21.4
but which also contained the improvements that were in 0.22.0.
0.21.5 was never released and will not be released. Therefore all
of the noise to facilitate the 0.21.5 release can just be deleted,
as long as we leave the CI changes that are necessary for GitHub
Actions to work correctly now.
The exact commands I used were:
```
git checkout \
6c334a2cf5 \
-- \
Cargo.toml \
LICENSE \
README.md \
src \
tests \
third-party
git rm src/trust_anchor_util.rs
```
Commit 6c334a2cf5 was the commit from
which 0.22.0 was released. It is confusing because the commit
immediately prior, 0b7cbf2d32, has
commit message "0.22.0". It appears that I merged the "0.22.0"
commit, expecting to `cargo publish` from that commit, but then
`cargo publish` failed. Then I added
6c334a2cf5 to fix `cargo publish`
and did the `cargo publish` from that commit. That's why I added
the `package` CI step at that time, to prevent this confusing
situation from happening again.
`trust_anchor_utils.rs` was not in 0.22.0; the `git checkout` didn't
delete it, so I had to do it separately.
I left the tests added subsequent to 0.22.0 in `tests/` (e.g.
`name_tests.rs`) since those tests pass with the 0.22.0 sources too.
Unfortunately, this requires disabling a bunch of Clippy lints, to
avoid modifying the contents from 0.22.0.
(I know it is confusing. It took me a while to figure it out myself
today.)
This was added as a prerequisite for a feature that hasn't landed yet, but it
is currently unused. Further, it is using deprecated features of `untrusted`.
Remove it. We'll add it back if/when we need it, probably with a different
implementation.
Better error than `BadDER` when certificate is generated incorrectly.
I agree to license my contributions to each file under the terms given at the top of each file I changed.
Brian Smith
Tim Düsterhus
Daniel McCarney
Joseph Birr-Pixton
Stiopa Koltsov