hyperium / tonic

A native gRPC client & server implementation with async/await support.
https://docs.rs/tonic
MIT License
9.81k stars 998 forks source link

feat(tls): Use rustls_pki_types::CertificateDer to describe DER encoded certificate #1707

Closed tottoto closed 3 months ago

tottoto commented 4 months ago

Motivation

Although Certificate is for PEM encoded certificate, it is used by Request::peer_certs's return type, which is DER encoded certificate.

Solution

Adds CertificateDer type and changes Request::peer_certs to use it.

This is a breaking change.

djc commented 4 months ago

So I'm guessing the name is inspired by rustls_pki_types::CertificateDer<'_>. Given that rustls-pki-types is intended to have a long-term stable API, I would like to suggest that we adopt usage of types like CertificateDer<'_> in the tonic API directly instead of copying them here. This would ensure improved type safety and clarity across crates, and should cause no public API churn due to the stability promise.

@LucioFranco I'm biased here because I created rustls-pki-types -- what do you think? It's basically a collection of very lightweight wrappers for Vec<u8> and &[u8], with optional no_std support.

tottoto commented 4 months ago

So I'm guessing the name is inspired by rustls_pkitypes::CertificateDer<'>.

Yes. introduced CertificateDer is a type for not exposing rustls-pki-types crate. I think both exposing rustls-pki-types and hiding it are reasonable, so I am not sure which is more appropriate.

tottoto commented 4 months ago

Added a commit to show how the implementation would be when using rustls_pki_types::CertificateDer directly.

djc commented 4 months ago

LGTM. I think the same reasoning as in #1710 should apply here: rustls-pki-types is expected to have a more stable API than tonic so it should be okay to use rustls-pki-types API in the public tonic API.

tottoto commented 4 months ago

Updated this pull request title to reflect the latest status.