rustls / webpki Goto Github PK

https://github.com/rustls/webpki/blob/6a8ae20089d78c4d39d50ff65ca48fa83794386b/src/ring_algs.rs#L21C8-L21C21 (RingAlgorithm) does not implement the Debug trait.

This is an issue as https://github.com/rustls/pki-types/blob/d99520cbdca7c1b55f6008a7a6a03f6af4e8b1cd/src/lib.rs#L323 requires this trait, making it in the newest alpha versions impossible to use RingAlgorithm where this trait is expected.

As a result I have currently the current error:

error[E0277]: `RingAlgorithm` doesn't implement `Debug`
   --> /Users/glendc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-webpki-0.102.0-alpha.7/src/ring_algs.rs:27:41
    |
27  | impl SignatureVerificationAlgorithm for RingAlgorithm {
    |                                         ^^^^^^^^^^^^^ `RingAlgorithm` cannot be formatted using `{:?}`
    |
    = help: the trait `Debug` is not implemented for `RingAlgorithm`
    = note: add `#[derive(Debug)]` to `RingAlgorithm` or manually `impl Debug for RingAlgorithm`
note: required by a bound in `SignatureVerificationAlgorithm`
   --> /Users/glendc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-pki-types-0.2.3/src/lib.rs:323:57
    |
323 | pub trait SignatureVerificationAlgorithm: Send + Sync + fmt::Debug {
    |                                                         ^^^^^^^^^^ required by this bound in `SignatureVerificationAlgorithm`
help: consider annotating `RingAlgorithm` with `#[derive(Debug)]`
    |
21  + #[derive(Debug)]
22  | struct RingAlgorithm {

To support checking certificate status for revocation using Certificate Revocation Lists (CRLs) we need webpki to be able to parse CRLs in DER format. Like trust anchors I think it makes sense to consider PEM encoding out of scope. Focusing on ASN.1 DER we can reuse the untrusted crate that webpki/ring use today for X509 to parse CRLs and won't require any extra dependencies.

I made a PR adding a test on briansmith/webpki#266 repo. Seeing this repo has more recent activity, I could make the same PR here if you are interested.

in quite a number of places we use Error::BadDer as a generic error code. i think we should be reserving it for genuine bad DER encoding cases and introducing more specific errors where the DER encoding is OK but the contents/semantics of the encoding are wrong.

The ultimate goal here would be to allow use of the webpki crate with other cryptography libraries.

This would come in two parts, I think:

• replacing remaining references to ring::io -- much of this was already done, but a handful of instances remain.
• rework SignatureAlgorithm to be a trait that can be implemented outside the crate, with implementations backed by ring being provided by default (eg, ECDSA_P256_SHA256). Alongside, we should export AlgorithmIdentifier and the common values thereof.

This repository is reported as a fork of https://github.com/briansmith/webpki. Will this repository be publishing to https://crates.io/crates/webpki (which currently reports https://github.com/briansmith/webpk as its git repository)? Seems like it since it is under the rustls org.

It might be worth mentioning in the readme since it reports as a fork.

The Cargo.toml file does not include a license entry, merely points to a file.

I am only opening this issue because it came up in a cargo-deny license check.

While the top-level license seems to be standard ISC, I am not sure how to include an optional sub-component in an SPDX expression, I'm even unsure if those files are distributed with the package.

Hi,

This is related to briansmith/webpki#85, currently it is not possible to access a certificate's public key and other details. A use case for this is the validation of end entity's certificates with DNS-Based Authentication of Named Entities (DANE).

It would also be quite useful to have access to the parsed X.509 certificate such as https://github.com/rusticata/x509-parser does.

Thanks

A meta-issue to collect up some of the pieces I think will be needed to complete CRL support in Webpki.

For now I'm trying to limit the scope to verifying end-entity certificate revocation state (based on discussion in rustls/rustls#1164 and similar functionality in other TLS libraries), and not considering CRLs during path-building, however where possible we should leave the door open for supporting that in the future.

• #54
• #58
• #52
• #53
• #55
• #56
• #79

Auxiliary work (nice to have, not required for initial support):

• #80
• #81

Pulling out a discussion issue from #66 for more targeted discussion:

Error messages for revocation related events are incorrect. All error states are reported as "UnknownIssuer". I will pull out a separate issue for discussion on this but the short explanation is that the existing path building code offers no opportunity to return a unique error, every path building error case is swallowed by the loop_while_non_fatal_error helper and turned into a Error::UnknownIssuer result. There are some comments that indicate "fatal" errors should be treated separately, but in practice all errors are considered non-fatal. For now I've left this as-is and the unit tests have TODOs to check for more granular errors.

Spelunking around for historic context, I've found this was an explicit design decision made by Brian: the goal was for webpki to be as minimal as possible and yeild only a binary result: "yes - valid chain to a trust root was constructed" or "no - invalid issuer" (e.g. see comments in briansmith/webpki#183). Later, it seems his perspective changed and he expressed interest in renaming InvalidIssuer and some strategies for surfacing more granular errors were put on the table for evaluation.

I agree with the new directions Brian suggested. I think we should separately consider:

1. Renaming UnknownIssuer to something that more generically suggests path building failure (briansmith/webpki#221)
2. Trying to surface more specific errors where possible (briansmith/webpki#206).

For point 1, I think we mostly need to bikeshed a new name, so this issue is more for discussing approaches to point 2. There have been several suggestions made for this:

1. Following the approach Firefox/libpkix use: if all attempts to build a chain yield the same error, return that error.
2. Updating verify_cert.rs's loop_while_non_fatal_error fn to return the last error observed (implemented in briansmith/webpki#140)
3. Updating verify_cert.rs's loop_while_non_fatal_error fn to heuristically score errors based on their relative importance, returning the most important error observed during path building (implemented in briansmith/webpki#188)

I've found it hard to map the described Firefox approach to the implementation (both of webpki and in the Firefox codebase). It sounds promising, but the path to implementation is less clear. This approach feels trickier to nail down without allocation - would it require building a chain of error refs in Cert ala the EndEntityOrCa links?

The "return last error" approach from 140 is simple, but not sufficient for surfacing errors in all cases we care about. For instance if an intermediate is revoked there will be further path building operations that ultimately squash the error with UnknownIssuer even with this suggested error handling strategy in place.

The approach in 188 is better suited to our needs, but also requires that we heuristically score errors based on their relative importance. That's a little bit cumbersome, but with care it does yield what I consider to be the most important errors when run across our existing testcases (the majority of which are currently asserting UnknownIssuer).

I'm leaning towards adopting the "heuristic" approach, but I'm eager to here what others think. If it would be helpful, I can stage a branch with the diff updated for the fork and the affected unit tests updated. I think this is likely a blocker for landing proper CRL support, it will be a very poor user experience to yield "UnknownIssuer" when the problem is a revoked end-entity certificate (as one example).

My Problem

I generated a self-signed certificate and tried using that to connect via TLSv1.3 to a server written with libtls from libressl (written by me; just a test project).

When connecting/reading to/from the server I get the following error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidCertificate(Other(CaUsedAsEndEntity))', src/lib.rs:84:36

where the line src/lib.rs:84 contains client.process_new_packets().unwrap();. client is a ClientConnection.

The reason for this error seems to be that I use a self-signed certificate.

My research

In my research I found the issue briansmith/webpki#114 from the original webpki repo. From the discussion in that issue I learn that connecting via a self-signed certificate is known to be not supported and that there is more important work to do before implementing it.

My question

Is there a will and time to implement connecting via a self-signed certificate in this repo?

Additional infos

I generated my certificate like this:

openssl req -subj "/CN=192.168.2.30" \
  -x509 -nodes -newkey rsa:4096 -sha256 \
  -config /etc/ssl/openssl.cnf \
  -keyout signed.key -out signed.crt

Running openssl x509 -in signed.crt -noout -text gives the following output:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            cb:ca:a7:2d:e0:d2:90:44
    Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN=192.168.2.30
        Validity
            Not Before: Nov  4 13:08:18 2023 GMT
            Not After : Dec  4 13:08:18 2023 GMT
        Subject: CN=192.168.2.30
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (4096 bit)
                Modulus:
                    00:e2:af:9b:ee:b7:7d:f2:b8:be:36:be:f7:11:7a:
                    5d:f5:a2:fc:b1:c2:6e:c7:78:23:29:f4:fc:40:7a:
                    3e:da:73:52:ec:66:93:2f:fc:b9:04:a5:d7:9e:af:
                    79:49:8c:2d:45:15:1b:38:da:8b:a5:96:3a:b0:e0:
                    04:c8:e5:6e:1c:fc:f1:97:d1:b7:88:e7:50:3b:d8:
                    e3:bd:bb:34:5e:5e:6f:f6:23:cd:78:ca:3b:07:ba:
                    24:1a:97:23:4b:e7:ea:de:71:e9:54:24:9f:d5:4e:
                    c0:9c:4a:d7:40:46:1a:48:57:c2:df:54:95:73:b3:
                    0d:20:a5:4f:dc:3f:ef:e2:7e:a4:1e:dd:1d:2c:c1:
                    ab:67:49:cc:47:22:aa:de:f7:e5:2b:8d:a6:b8:97:
                    66:72:dc:d6:b2:d8:63:33:f0:26:b3:62:86:e9:e0:
                    7d:63:04:5c:1d:cb:a9:e6:a2:d6:14:26:e0:e4:41:
                    c7:bd:bd:a4:45:d4:a8:84:9f:d8:17:2a:e6:ac:ca:
                    99:b9:4c:a0:2f:90:77:1a:73:24:82:cd:e4:90:79:
                    9e:37:ae:b9:39:ec:f0:09:35:ae:b3:14:25:a9:6f:
                    8b:e9:c7:67:59:89:f6:0d:40:40:ce:fa:62:8a:99:
                    70:85:3e:c7:82:4f:30:b3:79:69:d0:a5:7b:9f:ad:
                    0a:bd:0f:59:41:9e:fc:5d:fd:c2:08:62:c3:1b:58:
                    18:28:c4:2a:ba:8a:6d:1e:d2:27:f9:31:98:89:d6:
                    1b:88:e2:84:f2:bb:33:66:d3:aa:8c:55:0f:7b:9e:
                    0c:9f:cf:29:f4:f0:c2:63:71:29:1d:7a:a1:62:ce:
                    fc:f0:cf:64:a2:55:36:dd:b9:42:c9:18:e2:f8:27:
                    b8:8c:bc:a8:7c:be:fe:3a:a5:dc:fb:e1:88:1b:fe:
                    86:69:90:bd:2f:eb:c4:be:ab:76:1e:94:9a:50:6b:
                    7b:7b:9d:c3:3c:ef:d4:39:3d:6c:bf:6f:c3:a5:27:
                    23:26:5b:b7:65:ee:44:09:eb:5a:22:68:70:72:08:
                    5c:a3:1c:dd:4f:0a:89:48:a0:72:fc:5f:a6:38:41:
                    21:a6:ea:b2:a9:8d:88:1c:e9:68:cc:b2:c4:0c:6f:
                    6c:0e:a4:38:96:08:d9:03:fc:01:f3:3c:9e:5c:21:
                    19:80:62:12:22:ca:f1:c1:38:c5:58:ad:7f:9c:57:
                    45:3e:07:ed:47:d5:63:7f:d0:fb:30:18:6b:06:31:
                    13:3e:7e:11:6c:93:9c:b8:a4:17:00:0e:4e:b9:81:
                    41:0c:03:a3:e8:3f:1c:b7:a5:96:93:1b:8d:65:97:
                    e6:90:f5:05:9c:47:83:c8:b2:7a:e0:9f:27:0e:04:
                    e6:6d:77
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Key Identifier:
                5B:DC:4F:76:6A:17:19:CD:23:C2:D1:AE:00:C1:F6:AC:3F:4B:FB:6B
            X509v3 Authority Key Identifier:
                keyid:5B:DC:4F:76:6A:17:19:CD:23:C2:D1:AE:00:C1:F6:AC:3F:4B:FB:6B

            X509v3 Basic Constraints: critical
                CA:TRUE
    Signature Algorithm: sha256WithRSAEncryption
         44:1d:60:72:9c:74:16:c4:51:82:e8:9f:2b:0b:25:23:97:10:
         a7:ca:e8:07:8a:62:af:04:2b:97:78:40:5c:de:cb:88:86:8e:
         79:63:35:1b:88:5e:b4:77:39:01:cb:15:ab:b2:a5:ed:52:91:
         93:1c:02:5e:9a:db:31:cf:4e:78:1c:69:e1:51:36:95:26:49:
         e2:2f:bf:8b:86:db:99:57:4b:55:72:15:1a:5d:77:af:0e:fa:
         23:3a:a3:b7:5d:cd:f9:9b:13:65:f0:f5:5c:4a:cf:ec:92:6f:
         22:6b:88:ae:55:f7:05:40:39:74:21:1c:1e:eb:3b:c1:e5:ba:
         af:4d:88:61:88:5c:d2:4a:40:48:45:26:65:1d:74:7d:9a:bc:
         fb:00:89:c9:83:99:68:2f:df:23:6c:5d:e6:86:5a:9d:98:a2:
         94:ef:99:a0:03:be:51:2a:06:a3:84:9b:9d:4a:ee:55:e3:9b:
         2b:0f:72:1c:e0:f0:62:f7:0c:bc:3d:75:c3:88:41:59:ae:41:
         46:12:54:73:7b:9e:0c:c2:55:6a:fb:9a:2b:69:44:2e:81:ad:
         1b:b5:ce:fe:d1:30:fd:3f:e8:79:24:79:b6:91:f7:73:0d:f3:
         06:d1:b9:fc:6c:42:40:11:99:39:a8:df:bb:8d:02:1a:df:90:
         a1:84:6c:7d:64:46:0e:71:73:fd:cd:9b:44:ef:b3:1d:a3:59:
         32:ff:68:70:ae:6d:aa:eb:99:c5:b6:78:6c:29:80:74:be:19:
         96:6c:8c:11:88:06:d0:75:7f:9e:e6:59:25:a8:26:05:b2:6e:
         c2:f8:eb:a9:58:98:4a:8b:e0:7f:9a:a9:ce:e4:20:91:24:39:
         e6:e3:80:70:ce:2f:17:2c:1d:7f:0a:e9:e8:13:0f:ca:f4:2b:
         b8:a4:0c:5a:c9:34:bf:46:48:b1:14:36:d9:fc:bf:7f:9d:a4:
         43:55:c5:6e:35:74:5d:83:d1:3b:9d:70:88:68:72:c8:60:39:
         aa:f2:ae:19:fa:14:e5:7d:f2:a5:bd:b1:a2:04:47:45:08:4d:
         77:ef:f4:16:a4:4b:aa:6b:e0:ff:6c:61:06:9a:2f:a4:65:eb:
         ed:ab:06:61:ee:a7:55:cc:c7:af:43:3a:46:6f:65:2e:ce:4e:
         07:13:2f:8f:51:47:0c:f4:56:7a:ff:51:4a:24:5d:46:e7:6b:
         8f:84:f4:5a:67:6f:4f:29:0f:fe:20:62:aa:9e:0b:19:6e:64:
         db:64:1e:b4:5f:2c:fc:a0:99:bb:48:1a:f8:f0:9e:18:ff:9d:
         4d:a4:fb:99:2a:f8:df:ce:83:84:3a:b6:9a:b8:08:dc:3f:60:
         fe:be:a2:9b:7c:a8:9f:3f

If there is something I can further do or test, please let me know.

We (Linkerd) currently depend on the changes in briansmith/webpki#91 to read a client certificate's DNS SANs from a server-terminated TLS connection. This PR was superseded by @Geal's briansmith/webpki#103, but this also was never merged. I'd love to find a path forward for these changes.

Promoting my comment from #44 into an issue:

Ring's read_tag_and_get_value function from io/der.rs only supported values with lengths that could be encoded in at most two bytes. I couldn't find a definitive reason in the development history but suspect this was a cautious choice that worked for the domain of smaller certificate data Ring was operating with. Unfortunately, CRLs can be quite large (up to 50mb in my corpus) and thus require processing DER values with lengths that may span three or four bytes. To unblock my testing I made a crude patch to ring and found no further roadblocks with this branch. I don't think this patch is an appropriate change as-is, we likely want to restrict these large values to the domain of CRL processing and not allow large values in other domains.

Possible options:

• Patch Ring's read_tag_and_get_value to allow larger reads as I did in ae80a1.
• Implement a second, more targeted version of this function, or introduce a parameter, so that we can allow large reads but only for the CRL context, not certificates
• Something else I'm not thinking of

Open questions:

• Do we want to impose a size-limit on CRLs? I haven't done an analysis of other implementations to see what they do but CRLs in the 50 ... 100mb range do seem to exist in the wild. I've seen references to DoD CRLs that are particularly chonky.

Once CRLs are provided (#55) we need to identify the correct CRL to consult for a given end entity certificate.

I'm strongly in favour of not supporting delta CRLs, since their implementation is significantly more complex than for full CRLs and ecosystem support seems very minimal. Similarly, I would advocate for not supporting indirect CRLs, since their support in the broader ecosystem is also minimal and it would complicate implementation.

With those two considerations off the table I believe we can identify the correct CRL based on matching the end entity certificate's issuer to the CRL issuer.

Once the correct CRL is identified (and validated, see #52 and #53), we also need to search the revoked certificate entries within the CRL for the end entity certificate's serial number. In some implementations (e.g. BoringSSL), the CRL's revoked certificate entries can be maintained in sorted order to allow a binary search for the serial. This would prevent linear scan of the CRL content (which can often be quite large), but I believe would require allocation to implement in webpki.

Just to record: this and the parent repo do not correctly process directoryName name constraints. The easier reproducer for this is (in rustls):

$ ./target/debug/tlsclient-mio --http www.indicepa.gov.it
TLS error: InvalidCertificateData("invalid peer certificate: UnknownIssuer")

The issuer here is https://crt.sh/?id=5715019745&opt=cablint,x509lint,zlint and we're incorrectly processing the constraint against the end-certificate subject. There seems to be a lack of code that dissects the subject into name attributes, and no code at all for comparing sets of DN attributes for equality?

golang has the same issue golang/go#55872

In #66 we've staged support for using Certificate Revocation Lists (CRLs) to make revocation decisions during path building.

The code in that branch performs CRL signature verification as part of verifying the signatures on the end-entity to trust anchor path that was found from path building: https://github.com/rustls/webpki/blob/8425fa3f54ab7a8790a572023910e5bc850f55cc/src/verify_cert.rs#L219C10-L222

This was done for two a few reasons:

1. It was easier to integrate with the existing path building code used for certificate validation. That code relies on building a linked list from trust anchor -> intermediate certificate(s) -> end entity certificate. It's not super trivial to generalize to building a path from trust anchor to CRL and we'd need that ability to verify a CRL signature outside the context of cert validation (e.g. when loading the CRL).
2. From an end-user perspective, it's appealing to be able to load CRLs without needing to first parse each of the issuers associated with the CRL (and the issuers of the issuers up to a root). E.g. with the current implementation its easy to load CRLs for intermediate certificates without needing to source those intermediates up-front, if the TLS handshake provides intermediates associated with loaded CRLs they can be used during path building without extra effort up-front.
3. On the topic of CRL processing, RFC 5280 §6.3.3 says:

(f) Obtain and validate the certification path for the issuer of
the complete CRL. The trust anchor for the certification
path MUST be the same as the trust anchor used to validate
the target certificate.

It will take some care to be able to meet this requirement outside the context of building and verifying the path used to validate an end entity certificate.

However, since signature validation is expensive and CRLs are loaded infrequently but consulted for revoked certificates frequently it would be a nice optimization if we could perform signature validation once-up front instead of per-access.

ac2faa7 was backported to the v0.101.x release in v0.101.5. This work originated in #164, but shouldn't have been backported to the 0.101.x release stream, because it changes the CertificateRevocationList trait in a semver breaking way, adding a new argument to the verify_signature fn. This trait and its fns are part of the public API, e.g. in rcgen's webpki tests (reported by est31).

Let's fix this in a point release and leave this note in place until then so the issue is discoverable.

The folks at Trail of Bits have been working on a Rust based certificate path building and validation backend for use in PyCa Cryptography. As part of that work they've built x509-limbo, "A suite of testvectors for X.509 certificate path validation".

There's a harness in that repo for testing against briansmith/webpki, and I inquired about adding a harness for this repo. In that issue there was a suggestion that x509-limbo is designed to be integrated into other repository test suites. We should consider doing that in this repo that like we did with the BetterTLS suite.

To support checking the revocation status of an end entity certificate we need to be able to provide an optional set of one or more parsed CRLs (see #54 for parsing).

At a minimum we will need to augment the end entity verify_is_valid_tls_client_cert function to provide a list of parsed CRLs to consider when verifying the certificate. It may be prudent to be forward looking here and also allow specifying a "scope" for whether the CRLs are considered during complete path building, or just for the end entity certificate. Initially we will only support consulting the CRLs for the end entity certificate but in the future could augment the implementation for use during path building.

In some frameworks (e.g. boring SSL, openssl, s2n) users can instead provide a callback function that can be invoked by the validation logic at the time a revocation check is required. The callback can be invoked with subject information and consumers can implement their own logic for providing the CRL for that subject that may include fetching it from a distribution point, or loading it from disk. We should consider whether this is functionality webpki should provide.

I think these have more meaningful semantics and are potentially useful when used in, for example, a HashMap.

In rustls, I was looking at the API for verifying client certificates against a CRL or set of CRLs. It seems like there's a problem: say the server provides a CRL from http://example.com/1.crl, but the client certificate is not covered by that CRL. For instance, the server could be using sharded CRLs and the client certificate could be on a different shard. Or, more likely, the server is using an out-of-date config and the client certificate is now reported on an entirely different CRL, or is issued by a different issuer.

One helpful thing here is the CRL Distribution Points extension. We could modify the API such that a CertRevocationList has an accessor that provides the Issuing Distribution Point encoded in that CRL. Then, when validating a client certificate that contains a CRLDP, we could require that there is a CertRevocationList matching a CRLDP from the client certificate. This also provides resistance against substitution attacks.

That would not solve the case where the client certificate does not contain a CRLDP. I think probably we should "fail closed" in such a case, provided the the crls list is non-empty. If the server was expecting to process revocation information from CRLs, and the client certificates do not provide CRLDP, that seems too risky in terms of misconfiguration.

Another check we could perform: if the CRLs list is non-empty, then there must be at least one CRL whose issuer matches the issuer of the certificate we are verifying. Right now it looks like we fail open: https://docs.rs/rustls-webpki/0.101.1/src/webpki/verify_cert.rs.html#203-209. I think that should be inverted. If the user provided a list of CRLs, they probably expected that list of CRLs to be complete for all the issuers of certificates they might encounter during the handshake. Or another way of putting it: if the user provided a list of CRLs, they want to treat certificates of "unknown" status as invalid.

Summary

rustls-webpki's EndEntityCert::dns_names iterator returns an error when called on a certificate generated using cloudflare/cfssl and converted from PEM to DER using the openssl command-line tool. Meanwhile, Go's crypto/x509 package successfully parses the DNS names.

Both crypto/x509 and rustls-webpki do successfully validate these certificates.

Details

• rustls-webpki versions: This issue occurs with both v0.101.4 and v0.102.0-alpha-1 of rustls-webpki.
• Presence or lack of a common name: This issue occurs when a certificate is generated with or without a common name field, although v0.102.0-alpha.1 emits a different error when there is no common name (TrailingData(CommonNameOuter) rather than BadDer).

Reproduction

I've written a reproduction for this issue in https://github.com/hawkw/rustls-webpki-repro. The reproduction includes:

• a Rust program that attempts to read the cert's DNS names using rustls-webpki v0.101.4 and v0.102.0-alpha.1
• a Go program that does the same thing using the Go crypto/x509 package
• the certificates themselves
• a Bash script for generating the certificates.

I'm not really a TLS or ASN.1 expert, so please let me know if there's any additional information I can provide. Thanks!

Go program output

$ go run repro
=== Path: testdata/no-cn-test-ca1/crt.der===

--- go crypto/x509 ---

Verified valid for no-cn.test.com
Subject:
NotBefore: 2023-09-06 17:21:00 +0000 UTC
NotAfter: 2033-09-03 17:21:00 +0000 UTC

printing DNS names...
DNSNames[0]: no-cn.test.com


=== Path: testdata/cn-test-ca1/crt.der===

--- go crypto/x509 ---

Verified valid for cn.test.com
Subject: CN=cn.test.com
NotBefore: 2023-09-06 17:21:00 +0000 UTC
NotAfter: 2033-09-03 17:21:00 +0000 UTC

printing DNS names...
DNSNames[0]: cn.test.com

Rust repro output

$ cargo run
   Compiling rustls-webpki-repro v0.1.0 (/home/eliza/Code/rustls-webpki-repro)
    Finished dev [unoptimized + debuginfo] target(s) in 0.39s
     Running `target/debug/rustls-webpki-repro`
=== Path: testdata/no-cn-test-ca1/crt.der ===

--- rustls-webpki v0.101.4 ---

Verified valid for no-cn.test.com
printing DNS names...
Error: BadDer

--- rustls-webpki v0.102.0-alpha.1 ---

Verified valid for no-cn.test.com
printing DNS names...
Error: TrailingData(CommonNameOuter)

=== Path: testdata/cn-test-ca1/crt.der ===

--- rustls-webpki v0.101.4 ---

Verified valid for cn.test.com
printing DNS names...
Error: BadDer

--- rustls-webpki v0.102.0-alpha.1 ---

Verified valid for cn.test.com
printing DNS names...
Error: BadDer

Expected output

$ cargo run
   Compiling rustls-webpki-repro v0.1.0 (/home/eliza/Code/rustls-webpki-repro)
    Finished dev [unoptimized + debuginfo] target(s) in 0.39s
     Running `target/debug/rustls-webpki-repro`
=== Path: testdata/no-cn-test-ca1/crt.der ===

--- rustls-webpki v0.101.4 ---

Verified valid for no-cn.test.com
printing DNS names...
dns_names[0]: no-cn.test.com

--- rustls-webpki v0.102.0-alpha.1 ---

Verified valid for no-cn.test.com
printing DNS names...
dns_names[0]: no-cn.test.com

=== Path: testdata/cn-test-ca1/crt.der ===

--- rustls-webpki v0.101.4 ---

Verified valid for cn.test.com
printing DNS names...
dns_names[0]: cn.test.com

--- rustls-webpki v0.102.0-alpha.1 ---

Verified valid for cn.test.com
printing DNS names...
dns_names[0]: cn.test.com

The inherent Time::try_from() method has been deprecated since briansmith/webpki#220 (and briansmith/webpki#139 before it). In addition, the TryFrom::Error type exposes ring as a public dependency (I believe this is the only item doing that). We should fix these issues in the next semver-breaking release (unfortunately we didn't do so for 0.100).

We (Linkerd) have recently noticed a bug (linkerd/linkerd2#9299) that prevents webpki from validating certificates that include name constraints. We can probably produce a smaller reproduction outside of Linkerd, but our testing indicates that this applies to any certificate issued by a CA that uses name constraints. briansmith/webpki#20 suggests that this issue has existed for quite a while.

Last year, the folks at Deno ran into this issue (denoland/deno#10312) and @bnoordhuis kindly put together a PR (briansmith/webpki#226). We have not yet confirmed that this PR fixes the bugs that we encountered, but it would be great to find a path forward for name constraint support.

Today the webpki expression of TrustAnchors contains just the bare essentials required for path building and verification: the raw subject, the raw SPKI, and optionally raw name constraints

The Cert type used for intermediate certificates contains more metadata (e.g. serial, validity, extended key usage, etc).

There's no support for adding a TrustAnchor with a specific KeyUsage extension, and the Cert type ignores the KeyUsage extension:

For the purpose of verifying CRLs we will need Certs to hold their KeyUsage when specified so that we can check for the presence of the cRLSign bit:

  (f)  Obtain and validate the certification path for the issuer of
       the complete CRL.  The trust anchor for the certification
       path MUST be the same as the trust anchor used to validate
       the target certificate.  If a key usage extension is present
       in the CRL issuer's certificate, verify that the cRLSign bit
       is set.

(Note that this is a key usage bit, and not an extended key usage attribute for which we have some existing cert support).

It may also be prudent to allow TrustAnchors to express a KeyUsage since it's conceivable that a CRL could be issued directly from an anchor.

Related issue for end-entity certificates: briansmith/webpki#95

I noticed at https://docs.rs/rustls-webpki/latest/webpki/struct.TlsClientTrustAnchors.html that the type and its field are marked with #[deprecated(since = "0.101.2")]. There's an additional parameter to the deprecated attribute, called note:

https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute

note — Specifies a string that should be included in the deprecation message. This is typically used to provide an explanation about the deprecation and preferred alternatives.

It would be useful to attach a note to this deprecation (and others that don't have notes) explaining why it was deprecated and what to use instead.

In #42 we adopted some upstream work that included a new GeneralDnsNameRef enum that had a DnsName variant for a standard DnsNameRef (no wildcards, can be used for subject matching) and a Wildcard variant for a new WildcardDnsNameRef type (wildcards supported, only used in the context of a presented DNS name). The original idea that prompted development of this enum is that it would support changing the GeneralName::DnsName enum variant from holding untrusted::Input to holding GeneralDnsNameRef. This allows more specificity while still maintaining a strong separation between the appropriate uses of the two types of name refs (see briansmith/webpki#66).

We never fully implemented this idea after landing #42 and using it only in the context of iterating certificate SAN dNSNames for informational purposes. We considered exporting the types in #178 but arrived at a simpler solution whereby we iterate &str and don't need to expose the underlying types. As a result, we removed the GeneralDnsNameRef type entirely as it was unused.

This issue is a reminder that we should consider restoring the GeneralDnsNameRef type and using it for its intended purpose: as part of the GeneralName enum.

The About sidebar links to briansmith.org/rustdoc/webpki/, which is outdated and for the original pre-fork repo.

webpki does not currently implement IP address SAN validation, as described in briansmith/webpki#54. This makes webpki/rustls unusable in a variety of situations.

@ereslibre opened briansmith/webpki#260 to fix this issue, but it has not yet been reviewed.

I have a server certificate that contains the following DNS values in the SAN extension:
{service-instance}.example.com
host1.example.com

When connecting to host1.example.com the validation fails because the { is not a valid DNS name. However, there is a valid DNS that matches and the server cert validation should succeed. The validation works fine for other languages (tried Java and go) as well as curl.

Maybe the iteration should continue when there's an error:

I am currently trying to build rustls for the riscv64 architecture. Unfortunately riscv support is only possible with ring 0.17 and not with ring 0.16.20, and I tried to find my way through the dependencies and ended up here with getting build errors. The webpki build fails when having a mismatch in the untrusted versions (0.7.1 for webpki and 0.9 for ring), and also fails when switching to 0.9 for both (I assume the API changed). Are there plans yet to update the ring/unstrusted dependencies?

If not, I would try myself to fix this somehow.

Presently the validate_cert.rs implementation of build_path used for path-building only returns an indication of whether the operation failed or not:

CRL validation also requires building a path from the issuer of the CRL to a trust anchor. Further, RFC 5280 6.3.3 says the path must be built to the same trust anchor used to validate the target end-entity certificate for which we're considering revocation status:

(f)  Obtain and validate the certification path for the issuer of
     the complete CRL.  The trust anchor for the certification
     path MUST be the same as the trust anchor used to validate
     the target certificate.  ....

At a minimum, the build_path implementation should yield the final TrustAnchor that was used to build the validated path so that we can ensure this requirement is met.

To avoid having to build a path from scratch from the issuer to that TrustAnchor it would be best if build_path could yield the full validated path including the intermediate certificates used. That would allow the CRL validation to proceed using the same validation path to the same root. This is the approach used by the gRPC implementation of CRL validation based on providing the implementation the verifiedChains [][]*x509.Certificate produced by path building.

Changing the implementation to yield just a TrustAnchor is fairly straight forward. Producing the full chain that was validated without making it require alloc is likely to be more difficult.

Related issue: briansmith/webpki#264

Originally filed by @ctz as briansmith/webpki#256, making a clone here since it seems useful.

This is a big list of unsorted, unprioritised issues found from x509test cases. I'm not making any particular claim that these are important issues, or even issues we want to fix. For example, some of the RFC assertions are requirements on issuers, not verifiers.

• duplicate extensions are not rejected for extensions webpki does not support

  • RFC5280 s4.2 'A certificate MUST NOT include more than one instance of a particular extension'
    • xf_duplicate_extension
    • xf_duplicate_extension2

• illegal subjectAltName extensions not rejected

  • RFC5280 s4.2.1.6 'If the subjectAltName extension is present, the sequence MUST contain at least one entry'
    • xf_ext_altname_empty
  • RFC5280 s4.2.1.6 'conforming CAs MUST NOT issue certificates with subjectAltNames containing empty GeneralName fields'
    • xf_ext_altname_empty2
  • RFC5280 s4.2.1.6 'subjectAltName extensions with a dNSName of ' ' MUST NOT be used'
    • xf_ext_altname_blank_domain
  • RFC5280 s4.2.1.6 'if the only subject identity included in the certificate is an alternative name form (e.g., an electronic mail address), then the subject distinguished name MUST be empty'
    • xf_ext_altname_email_only
  • RFC5280 s4.2.1.6 'the address MUST be stored in the rfc822Name. The format of an rfc822Name is a Mailbox as defined in Section 4.1.2 of [RFC2821]. A Mailbox has t he form Local-part@Domain'
    • xf_ext_altname_invalid_email
  • RFC5280 s4.2.1.6 'For IP version 4, as specified in [RFC791], the octet string MUST contain exactly four octets. For IP version 6, as specified in [RFC2460], the octet string MUST contain exactly sixteen octets'
    • xf_ext_altname_ip_wrong
  • RFC5280 s4.2.1.6 'If the subject field contains an empty sequence, then the issuing CA MUST include a subjectAltName extension that is marked as critical.'
    • xf_ext_altname_noncrit_nosubj

• empty OID is not rejected inside extKeyUsage extension

  • RFC5280 s4.2.1.12 'ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId'
    • xf_ext_extended_key_usage_empty_oid

• certain CA-only extensions not rejected if basic constraints cA=false

  • RFC5280 s4.2.1.3 'If the keyCertSign bit is asserted, then the cA bit in the basic constraints extension (Section 4.2.1.9) MUST also be asserted'
    • xf_ext_keysign_nonca
  • RFC5280 s4.2.1.10 'The name constraints extension ... MUST be used only in a CA certificate'
    • xf_ext_name_constraints_nonca

• end entity subject public key validation seems not to happen during parsing/chain/name validation -- (maybe that is deferred to verify_signature? in which case ignore these)

  • xf_der_pubkey_rsa_nonminimal_int
  • xf_pubkey_ecdsa_not_on_curve
  • xf_pubkey_ecdsa_secp192r1
  • xf_pubkey_ecdsa_unknown_curve
  • xf_pubkey_rsa_exponent_negative
  • xf_pubkey_rsa_modulus_negative
  • xf_pubkey_rsa_param_nonnull

• missing validations during trust anchor parsing(?)

  • RFC5280 s4.2.1.10 'For IPv4 addresses, the iPAddress field of GeneralName MUST contain eight (8) octets'
    • xf_ext_name_constraints_badip
  • RFC5280 s4.2.1.10 'Conforming CAs MUST NOT issue certificates where name constraints is an empty sequence.'
    • xf_ext_name_constraints_empty
  • RFC5280 s4.2.1.10 'Conforming CAs MUST mark this extension as critical'
    • xf_ext_name_constraints_noncrit
  • RFC5280 s4.2.1.10 'the minimum MUST be zero, and maximum MUST be absent'
    • xf_ext_name_constraints_minmax
  • RFC5280 s4.2.1.2 'this extension MUST appear in all conforming CA certificates'
    • xf_ext_subject_keyid_ca_absent

• comparison of string encodings in subject/issuer

  • RFC5280 s7.1 'Conforming implementations MUST use ...RFC4518 as the basis for comparison of distinguished name attributes encoded in either PrintableString or UTF8String.'
    • ok_issuer_asn1_differ

• optional subjectUniqueID causes parse failures

  • parsing of extensions (context[3]) fails rather than jumping over subjectUniqueID (context[2])
    • ok_uniqueid_incomplete_byte

• policy constraint extension (probably no advantage to implementing this, but listed here for completeness)

  • RFC5280 s4.2 'applications conforming to this profile MUST recognize the following extensions: ...policy constraints (Section 4.2.1.11)'
  • ok_ext_policy_constraints

We have a variety of places where something is gated by #[cfg(feature = "alloc")] and then the documentation text says "Only available when the "alloc" feature is enabled." For example:

There is a rustdoc feature that displays feature-gated information in a more standardized way, for example:

In rustls/rustls#1155 we adopted this style for the rustls repo; we should do the same for webpki.

The TrustAnchor::try_from_cert_der fn does not perform any validation of the provided certificate - it simply extracts the minimum necessary metadata (subject, subject public key information, and optionally, name constraints) to represent a trust anchor internally in webpki. The associated rustdoc comment explains:

In particular, there is no check that the certificate is self-signed or even that the certificate has the cA basic constraint.

The original commit that introduced the helper in 2015 did not offer any explicit rationale for why this validation isn't performed.

In particular, not enforcing the presence of a true cA basic constraint when building a TrustAnchor this way means that while we advertise an intentional limitation for webpki to not support self-signed certificates this isn't strictly true.

If a self-signed end-entity certificate includes a basic constraints extension that asserts a true cA bool then we will reject the certificate in check_issuer_independent_properties when path building, returning Error::CaUsedAsEndEntity. This happens even if the same self-signed certificate is configured as a trust anchor. The advertised limitation holds.

However, if a self-signed end-entity certificate omits the basic constraints extension (or includes one w/ a false cA bool) then we will not reject it as a CA used as an end-entity in check_issuer_independent_properties. Since TrustAnchor::try_from_cert_der doesn't enforce the presence of a true cA bool in its input it will also not error if given the self-signed certificate DER. The end result is that it is possible for path building to succeed if such a self-signed end entity certificate is also configured as a TrustAnchor created with TrustAnchor::try_from_cert_der. This is in contrast to the advertised limitation of not supporting self-signed certificates.

On the topic of basic constraints extensions RFC 5280 4.2.1.9 says:

The cA boolean indicates whether the certified public key may be used to verify certificate signatures. If the cA boolean is not asserted, then the keyCertSign bit in the key usage extension MUST NOT be asserted. If the basic constraints extension is not present in a version 3 certificate, or the extension is present but the cA boolean is not asserted, then the certified public key MUST NOT be used to verify certificate signatures.

My reading here is that we're acting in contradiction to a normative MUST NOT in 5280: We're using a trust anchor certificate without an asserted cA boolean's public key to verify the end-entity certificate signature.

If folks agree with my interpretation, I think we should update TrustAnchor::try_from_cert_der to perform more validation. Trust anchors participating in the web pki should all have the correct basic constraint extension present.

If folks disagree with my interpretation, I think we should update the limitation description to clarify this situation since there's more nuance than advertised in the README.

Steps to reproduce

$ cargo b --no-default-features --features alloc

yields errors when building src/crl.rs due to two reasons

1. Vec is used without importing it from alloc first
2. HashMap is imported but that's defined in the std crate

Fixing (1) is easy. The easiest fix for (2) is using BTreeMap but that has different performance costs

If we supported SCTs here we could enable people wanting to do CT verification of the certificates they see.

We could either do this directly by taking a dependency on sct.rs and requiring certificate validation to take a set of sct::Logs. It's more integrated but less flexible.

Alternatively we could let callers access the SCTs given a EndEntityCert and do the verification themselves outside this crate, after they've done other checks.

I wonder if it's in the interest of this fork to support these two types of certificates. The original repository already has two open pull requests to add these features:

• briansmith/webpki#234
• briansmith/webpki#241

It feels possible to refactor the verify.rs's list_cert_dns_names function added in #42 to lean heavier on iterators and to avoid allocation to a Vec. This would let us expose this functionality without depending on the alloc feature.

In two places (Time::try_from and From::from for DnsName) there are "soft" deprecation comments:

We should consider replacing these with "hard" deprecated attributes so that the the deprecations are warned about. Probably best done as part of the next breaking changeset.

In #44 we landed initial support for processing Certificate Revocation List (CRL) objects.

The code that landed in that branch avoids performing any allocation, but at the cost of having to do a linear O(N) scan of the CRL contents when looking for a RevokedCert by serial number:

For large CRLs with many revoked cert entries this will not perform as well as it could given some optimizations. In particular we could consider:

• Sorting the revoked certificate entries by serial number at CRL-parse time, accepting an up-front sort cost. This would let us find a serial with a binary search in O(log N) instead of O(N).
• Building a map from serial to revoked cert entry. This would let us find a revoked entry in something closer to O(1) in the common case.

We likely want to gate these optimizations by whether the alloc flag is enabled.

I think this is pretty much ready to go?

• #10 -- might be nice to get in, but it doesn't have any API changes
• #6 -- no API changes either, I think

Now that we added CertificateError::Revoked in rustls, should add an Error::Revoked here?

SPIFFE uses URI instead of DNS in the SAN. The webpki library currently does not support URI.

I propose we add support for it.

https://github.com/spiffe/spiffe/blob/main/standards/X509-SVID.md#2-spiffe-id

Let's gather some context for release blockers

• Update pki-types to 1.0

Done:

• #147
• #165
• #164
• #163
• #168
• #167
• #50
• #158
• #174
• Fix Budget in CertRevocationList trait.
• Push RevocationOptions into pki-types to avoid public webpki type in rustls API?
• #207

Hi, I just noticed #77, since I'm packaging rustls-webpki for Fedora Linux.

I wonder why you have included both ISC and BSD-3-Clause in the crate metadata? As far as I can tell, BSD-3-Clause license only applies to test data, and not to any actual source code that is linked into applications. As such, I think the current ISC AND BSD-3-Clause is potentially misleading, especially for tools like cargo-deny or cargo-license, since it doesn't reflect the fact that no BSD-3-Clause licensed material actually ends up in compiled binaries.

If you think that crate metadata should still include BSD-3-Clause since you are redistributing BSD-licensed test data (even if they end up unused when downloaded from crates.io), it might be an option to exclude these data files and the tests that read them from published crates? That would also make download size for this crate noticably smaller.

I'm dealing with a certificate that has critical Certificate Policy (2.5.29.32) with anyPolicy policy. It would be appreciated if rustls-webpki supported this scenario. Also created an issue in briansmith/webpki#268.

NSS supports Certificate Policies with anyPolicy.

Targets affected in our CI:

• i686-unknown-linux-gnu
• i686-pc-windows-msvc

   Compiling hmac v0.12.1
error[E0080]: evaluation of constant value failed
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-webpki-0.101.0/src/der.rs:246:45
    |
246 | const LONG_FORM_LEN_FOUR_BYTES_MAX: usize = (1 << (8 * 4)) - 1;
    |                                             ^^^^^^^^^^^^^^ attempt to shift left by `32_i32`, which would overflow

note: erroneous constant used
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-webpki-0.101.0/src/der.rs:211:40
    |
211 | pub(crate) const MAX_DER_SIZE: usize = LONG_FORM_LEN_FOUR_BYTES_MAX;
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0080`.
error: could not compile `rustls-webpki` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...

Compilation of failed (code 101)

but shouldn't

Steps to reproduce

$ cargo b --target x86_64-unknown-none --no-default-features --features alloc
   Compiling ring v0.16.20
error[E0425]: cannot find function `fill_impl` in this scope

this line in the Cargo.toml

is bringing in the ring dependency when the alloc feature is enabled. probably the intention what to enable ring's alloc feature iff the ring feature is also enabled. in that case, the Cargo.toml should use ring?/alloc

Presently when webpki encounters an invalidly encoded DNS identifier (for e.g. a subject alternate name dNSName) the verify_is_valid_for_subject_name fn of an end entity cert returns a generic BadDer error. It would be more helpful if we could indicate that it's an encoding error with a subject name in particular.

Related to #38, rustls/rustls#1292

In #42 we discussed the AsRef trait implementations for the existing DnsNameRef and DnsName types, as well as the new WildcardDnsNameRef type.

We reached the conclusion that WildcardDnsNameRef should implement AsRef<str>, since it best captures the guarantee that the contents of WildcardDnsNameRef are valid UTF-8, and its trivial to go from &str to &[u8]. See this discussion thread for more information.

The same reasoning can be applied to DnsName/DnsNameRef. Both should offer AsRef<str> and we should remove the AsRef<[u8]> impl from DnsNameRef. This will be a breaking change, so we need to hold this for the next API incompatible change set.