feat(tonic-xds): gRFC A29 cert validation building blocks#2616
Conversation
UpstreamTlsContext parsing, SAN matching, and an A29 ServerCertVerifier impl, plus a StringMatcher extraction reused by SAN matching. Connector wiring deferred to a follow-up PR.
| GeneralName::URI(s) => Some(SanEntry::Uri(s.to_string())), | ||
| GeneralName::RFC822Name(s) => Some(SanEntry::Email(s.to_string())), | ||
| GeneralName::IPAddress(bytes) => parse_ip_san(bytes).map(SanEntry::IpAddress), | ||
| GeneralName::OtherName(oid, value) => Some(SanEntry::OtherName { |
There was a problem hiding this comment.
should be something like:
GeneralName::OtherName(oid, raw) => {
// raw = `[0] EXPLICIT ANY` DER. Pull out the inner Any, then take its
// content bytes — typically a UTF8String / IA5String / PrintableString.
let (_, outer) = der_parser::ber::parse_ber(raw).ok()?;
let inner = outer.as_slice().ok()?; // contents of [0] EXPLICIT
let (_, any) = der_parser::ber::parse_ber(inner).ok()?;
let value = any.as_slice().ok()?.to_vec(); // inner string bytes
Some(SanEntry::OtherName { oid: oid.to_id_string(), value })
}
There was a problem hiding this comment.
yeah if we want to properly parse OtherName in SAN this will be needed. To avoid this complexity, since A29 only defined support for the above 4 SAN types (DNS, URI, Email, IP), I've removed all logic related to OtherName and rejects it at config time instead.
| ocsp_response: &[u8], | ||
| now: UnixTime, | ||
| ) -> Result<ServerCertVerified, RustlsError> { | ||
| self.inner.verify_server_cert( |
There was a problem hiding this comment.
will this support SPIFFE-only cert ?
There was a problem hiding this comment.
Thanks for pointing this out, actually the hostname-checking nature of the WebPkiServerVerifier would cause SPIFFE cert to be rejected always. I refactored the verifier to only use rustls's chain verification impl and now all SAN checking is handled by our xDS matcher logic, allowing URI-only SPIFFE certs use case.
| oid: oid.to_id_string(), | ||
| value: value.to_vec(), | ||
| }), | ||
| _ => None, |
There was a problem hiding this comment.
Should we pass it along to SanEntry as a "Unspecified" variant? I'm not familiar with the other variants, yet I feel maybe there is a chance that the custom verifier may need to validate against them?
There was a problem hiding this comment.
Related to the above comment, no SAN matching behavior of other variants is defined in A29 and other gRPC impls (grpc-go, grpc-java) or skip those types. So I'll keep the None case here and also dropping the OtherName variant as well. Updated comments to reflect this.
| fn parse_ip_san(bytes: &[u8]) -> Option<IpAddr> { | ||
| match bytes.len() { | ||
| 4 => <[u8; 4]>::try_from(bytes).ok().map(IpAddr::from), | ||
| 16 => <[u8; 16]>::try_from(bytes).ok().map(IpAddr::from), | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
According to https://github.com/rusticata/x509-parser/blob/b7dcc9397b596cf9fa3df65115c3f405f1748b2a/src/extensions/mod.rs#L824 , ip addr can also be 8 bytes or 32 bytes because of network masks.
There was a problem hiding this comment.
according to RFC 5280 4.2.1.6, IP SAN is 4/16 bytes. The 8/32 bytes is for CA nameConstraints which happens to be represented with the same extensions type here in x509-parser. I've updated the comments here to reflect this.
gu0keno0
left a comment
gu0keno0
left a comment
There was a problem hiding this comment.
Looks good overall, put a few comments for details.
support SPIFFE certs, drop OtherName support, better UTF8 string handling and better comments
4 participants
Motivation
Ref: #2444
Continues the gRFC A29 (xDS-Based TLS Security) work in
tonic-xds. The cert provider foundation merged in #2593 gives us a pluggable source of trust roots and identity. The next step toward end-to-end mTLS in the data plane is to parse the cluster's TLS config from xDS, validate server certs against the configured trust roots, and apply SAN matching on top of WebPKI chain validation.This PR adds the building blocks for that validation. Integration with the
connector factory is deferred to a follow-up PR — see below.
Solution
xds/resource/string_matcher.rs): pulled out ofrouting.rsso it can be reused. The SAN matcher uses it for the string-comparison primitives.xds/resource/security.rs): parsesenvoy.extensions.transport_sockets.tls.v3.UpstreamTlsContextinto a typed config.xds/resource/san_matcher.rs): RFC 6125 wildcard DNS matching, RFC 5952 IP canonicalization, plus URI, EMAIL, and OTHER_NAME paths matching using the above extractedStringMatcher.ServerCertVerifier(xds/cert_provider/verifier.rs): wrapsWebPkiServerVerifierand applies SAN matching after standard chain validation. Sources trust roots from the existingCertProviderRegistry.All new types are tested in isolation against synthetic certs.
Why integration is deferred
The tonic-side hook for installing a custom rustls
ServerCertVerifieron aChannelis still in flight as #2612. Without that API, the verifier can't be wired into the per-cluster connector from a downstream crate.