search

hyperium / tonic

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

feat(tls): Add ability to set der encoded certificate #1748

Open tottoto opened 1 week ago

tottoto commented 1 week ago

Adds ability to set DER encoded certificate.

djc commented 1 week ago

What's the motivating use case here? I feel like this adds quite a bit of API surface/complexity for stuff that's ultimately of little use when this API is mostly only needed as input stuff for building a transport. Also given that we're already using CertificateDer as output in the public API since #1707 I think it might be better to allow CertificateDer as input directly for Identity and Certificate (as opposed to all the AsRef<[u8]> generics.

tottoto commented 1 week ago

The use case is the case of when already having DER encoded certificates but not using tls-roots. Regarding using AsRef<[u8]> it is for API symmetry and the case users do not get CertificateDer directly.

However looking at the results after implementation, I thought that this is not worth the maintenance costs increasing, considering the main use case is solved by using tls-roots feature. And I am now inclined that it might be reasonable to have the implementation of these interfaces take CertificateDer directly. In this case, the users will have responsible for converting formats when they have PEM encoded certificates, but I am not convinced that whether it is a reasonable responsibility that users should have.