Tls rootca option #337
Tls rootca option #337
Conversation
|
Hey @fortuna! Do you have an time frame for when this will be merged in? |
| @@ -94,6 +94,8 @@ type ClientConfig struct { | |||
| NextProtos []string | |||
| // The cache for sessin resumption. | |||
| SessionCache tls.ClientSessionCache | |||
| // The root certificate authorities for client to use for certificate verification. | |||
There was a problem hiding this comment.
We need to update the validation function, otherwise I think this will have no effect.
It would be helpful to have a test to exercise this. I wonder if LLM can help 😅
There was a problem hiding this comment.
I meant to generate the test that shows it working/not working, because it's a bit complicated to create the test. But we can probably piggy back on existing tests somewhere
There was a problem hiding this comment.
Here is an example test: https://pkg.go.dev/crypto/tls#example-Dial
We need to redesign the certificate validation. I don't want to repeat the same mistake many TLS libraries do that spread a bunch of options that tweak validation, making it hard to figure out what options are relevant for what and how they interact.
We should get rid of all validation options (CertificateName, RootCA), and replace with a VerifyConnection function instead. Unlike the standard function, I'd make it take a slice of PeerCertificates, not tls.ConnectionState, so we don't tie it to the standard TLS implementation.
Then we can provide a helper struct type with CertificateName and RootCAs that implements the verify function. It needs to set x509.VerifyOptions.Roots and DNSName. That function can be passed to our tls.ClientConfig.
If someone wants the "InsecureSkipVerify" behavior, they can simply set a function that returns nil.
| @@ -94,6 +94,8 @@ type ClientConfig struct { | |||
| NextProtos []string | |||
| // The cache for sessin resumption. | |||
| SessionCache tls.ClientSessionCache | |||
| // The root certificate authorities for client to use for certificate verification. | |||
There was a problem hiding this comment.
Here is an example test: https://pkg.go.dev/crypto/tls#example-Dial
We need to redesign the certificate validation. I don't want to repeat the same mistake many TLS libraries do that spread a bunch of options that tweak validation, making it hard to figure out what options are relevant for what and how they interact.
We should get rid of all validation options (CertificateName, RootCA), and replace with a VerifyConnection function instead. Unlike the standard function, I'd make it take a slice of PeerCertificates, not tls.ConnectionState, so we don't tie it to the standard TLS implementation.
Then we can provide a helper struct type with CertificateName and RootCAs that implements the verify function. It needs to set x509.VerifyOptions.Roots and DNSName. That function can be passed to our tls.ClientConfig.
If someone wants the "InsecureSkipVerify" behavior, they can simply set a function that returns nil.
This adds a way to include a custom Root CA to the TLS transport. (#315)