From 218cdb7e0fbfbcb843da34d5a52d83b29ad16ab3 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2022 16:06:12 +0000 Subject: [PATCH] Use RenegotiateOnceAsClient and explain why Signed-off-by: Richard Wall --- pkg/issuer/venafi/client/venaficlient.go | 103 ++++++++++++++++++++--- 1 file changed, 92 insertions(+), 11 deletions(-) diff --git a/pkg/issuer/venafi/client/venaficlient.go b/pkg/issuer/venafi/client/venaficlient.go index cf684890f..4648b7ded 100644 --- a/pkg/issuer/venafi/client/venaficlient.go +++ b/pkg/issuer/venafi/client/venaficlient.go @@ -17,7 +17,11 @@ limitations under the License. package client import ( + "crypto/tls" + "crypto/x509" "fmt" + "net" + "net/http" "time" vcert "github.com/Venafi/vcert/v4" @@ -135,28 +139,27 @@ func configForIssuer(iss cmapi.GenericIssuer, secretsLister corelisters.SecretLi username := string(tppSecret.Data[tppUsernameKey]) password := string(tppSecret.Data[tppPasswordKey]) accessToken := string(tppSecret.Data[tppAccessTokenKey]) - caBundle := string(tpp.CABundle) return &vcert.Config{ ConnectorType: endpoint.ConnectorTypeTPP, BaseUrl: tpp.URL, Zone: venCfg.Zone, // always enable verbose logging for now - LogVerbose: true, - ConnectionTrust: caBundle, + LogVerbose: true, + // We supply the CA bundle here, to trigger the vcert's builtin + // validation of the supplied PEM content. + // This is somewhat redundant because the value (if valid) will be + // ignored by vcert since we also supply a custom HTTP client, + // below. But we want to retain the CA bundle validation errors that + // were returned in previous versions of this code. + // https://github.com/Venafi/vcert/blob/89645a7710a7b529765274cb60dc5e28066217a1/client.go#L55-L61 + ConnectionTrust: string(tpp.CABundle), Credentials: &endpoint.Authentication{ User: username, Password: password, AccessToken: accessToken, }, - // this is needed for local development when tunneling to the TPP server - //Client: &http.Client{ - // Transport: &http.Transport{ - // TLSClientConfig: &tls.Config{ - // Renegotiation: tls.RenegotiateOnceAsClient, - // }, - // }, - //}, + Client: httpClientForVcertTPP(tpp.CABundle), }, nil case venCfg.Cloud != nil: cloud := venCfg.Cloud @@ -187,6 +190,84 @@ func configForIssuer(iss cmapi.GenericIssuer, secretsLister corelisters.SecretLi return nil, fmt.Errorf("neither Venafi Cloud or TPP configuration found") } +// httpClientForVcertTPP creates an HTTP client and customises it to allow client TLS renegotiation. +// +// Here's why: +// +// 1. The TPP API server served by Microsoft Windows Server + IIS. +// 2. IIS uses TLS 1.2 by default[1] and it uses a +// TLS-1.2 feature called "renegotiation" to allow client certificate +// settings to be configured at the folder level. e.g. +// https://tpp.example.com/vedauth may Require or Accept client +// certificates while https://tpp.example.com/vedsdk may Ignore +// client certificates. +// 3. When IIS is configured this way it behaves as follows[2]: +// "Server receives a connection request on port 443; it begins a +// handshake. The server does not ask for a client certificate. Once +// the handshake is completed, the client sends the actual target URL +// as a HTTP request in the SSL tunnel. Up to that point, the server +// did not know which page was targeted; it only knew, at best, the +// intended server name (through the Server Name Indication). Now +// that the server knows which page is targeted, he knows which +// "site" (i.e. part of the server, in IIS terminology) is to be +// used." +// 4. In this scenario, the Go HTTP client MUST be configured to +// renegotiate. By default it will refuse to renegotiate. We use +// RenegotiateOnceAsClient rather than RenegotiateFreelyAsClient +// because cert-manager establishes a new HTTPS connection for each +// API request and therefore should only ever need to renegotiate +// once in this scenario. +// 5. But overriding the HTTP client like this causes vcert to ignore +// the `vcert.Config.ConnectionTrust` field, so we also have to set +// up the root CA trust pool ourselves. +// 6. And the value of RootCAs MUST be nil unless the user has supplied +// custom CA, because a nil value causes the Go HTTP client to load +// the system default root CAs. +// +// [1] TLS protocol version support in Microsoft Windows: https://learn.microsoft.com/en-us/windows/win32/secauthn/protocols-in-tls-ssl--schannel-ssp-#tls-protocol-version-support +// [2] Should I use SSL/TLS renegotiation?: https://security.stackexchange.com/a/24569 +func httpClientForVcertTPP(caBundle []byte) *http.Client { + // Copy vcert's default HTTP transport, which is mostly identical to the + // http.DefaultTransport settings in Go's stdlib. + // https://github.com/Venafi/vcert/blob/89645a7710a7b529765274cb60dc5e28066217a1/pkg/venafi/tpp/tpp.go#L481-L513 + transport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + // Note: This DualStack setting is copied from vcert but + // deviates from the http.DefaultTransport in Go's stdlib. + DualStack: true, + }).DialContext, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + + // Copy vcert's initialization of the TLS client config + tlsClientConfig := http.DefaultTransport.(*http.Transport).TLSClientConfig.Clone() + if len(caBundle) > 0 { + if tlsClientConfig == nil { + tlsClientConfig = &tls.Config{} + } + rootCAs := x509.NewCertPool() + rootCAs.AppendCertsFromPEM(caBundle) + tlsClientConfig.RootCAs = rootCAs + } + transport.TLSClientConfig = tlsClientConfig + + // Enable TLS 1.2 renegotiation (see earlier comment for justification). + transport.TLSClientConfig.Renegotiation = tls.RenegotiateOnceAsClient + + // Copy vcert's initialization of the HTTP client, which overrides the default timeout. + // https://github.com/Venafi/vcert/blob/89645a7710a7b529765274cb60dc5e28066217a1/pkg/venafi/tpp/tpp.go#L481-L513 + return &http.Client{ + Transport: transport, + Timeout: time.Second * 30, + } +} + func (v *Venafi) Ping() error { return v.vcertClient.Ping() }