-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(tls): Add ability to add multiple ca certificates #1724
Conversation
bd66035
to
c3f9ae5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the next step here to get rid of the Certificate
abstraction around CertificateDer<'_>
? Would be nice!
I would think that |
Well, the caller would rely on rustls-pemfile so it's one less mandatory dependency to carry within tonic. |
Considering |
Added a method to add multiple ca certificates at once, which is useful when having some sets of certificates. |
8781f15
to
9e76a66
Compare
9e76a66
to
0837c0b
Compare
tonic/src/transport/channel/tls.rs
Outdated
@@ -51,6 +51,13 @@ impl ClientTlsConfig { | |||
ClientTlsConfig { certs, ..self } | |||
} | |||
|
|||
/// Sets the multiple CA Certificates against which to verify the server's TLS certificate. | |||
pub fn ca_certificates(self, ca_certificates: Vec<Certificate>) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take an impl IntoIterator<Item = Certificate>
here, instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Updated to use it.
0837c0b
to
2c3ebee
Compare
Resolves #1629.