From ecc6db3e0ab0ca861ffe2fa857d4a9f2b8657b70 Mon Sep 17 00:00:00 2001 From: Juan Hernandez Date: Mon, 22 Mar 2021 12:49:57 +0100 Subject: [PATCH] Fix wrong TLS server name When support for Unix sockets was introduced it was necessary to also explicitly set the `ServerName` in the TLS configuration to the host name of the target host, otherwise the Go library would send the Unix socket name as the host, something like `Host: /tmp/my.socket`. But the TCP client is shared for all hosts, for example for _api.openshift.com_ and _sso.redhat.com_. So if the first request happens to be a request to _sso.redhat.com_ (it will usually be) the HTTP client will use _sso.redhat.com_ as the TLS server name also for API requests, not only for SSO requests. In this case the API server happens to be behind an OpenShift router that uses SNI to select the target service and certificates. As there is no _sso.redhat.com_ target behind that OpenShift router it returns the default, which fails validation against the _sso.redhat.com_ name. To address that this patch changes the SDK so that it uses a different client for each host. Related: https://github.com/openshift-online/ocm-sdk-go/issues/356 Signed-off-by: Juan Hernandez --- send.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/send.go b/send.go index 0b4fbba3..b642d399 100644 --- a/send.go +++ b/send.go @@ -211,11 +211,26 @@ func (c *Connection) selectBaseURL(ctx context.Context, request *http.Request) ( // selectClient selects an HTTP client to use to connect to the given base URL. func (c *Connection) selectClient(ctx context.Context, base *urlInfo) (client *http.Client, err error) { - // We need a client for TCP and another client for each combination of Unix and socket name, - // so we need to calculate the key for the clients table accordingly: + // We need to use a different client for each TCP host name and each Unix socket because we + // explicitly set the TLS server name to the host name. For example, if the first request is + // for the SSO service (it will usually be) then we would set the TLS server name to + // `sso.redhat.com`. The next API request would then use the same client and therefore it + // will use `sso.redhat.com` as the TLS server name. If the server uses SNI to select the + // certificates it will then fail because the API server doesn't have any certificate for + // `sso.redhat.com`, it will return the default certificates, and then the validation would + // fail with an error message like this: + // + // x509: certificate is valid for *.apps.app-sre-prod-04.i5h0.p1.openshiftapps.com, + // api.app-sre-prod-04.i5h0.p1.openshiftapps.com, + // rh-api.app-sre-prod-04.i5h0.p1.openshiftapps.com, not sso.redhat.com + // + // To avoid this we add the host name or socket path as a suffix to the key. key := base.network - if base.network == unixNetwork { + switch base.network { + case unixNetwork: key = fmt.Sprintf("%s:%s", key, base.socket) + case tcpNetwork: + key = fmt.Sprintf("%s:%s", key, base.URL.Hostname()) } // We will be modifiying the table of clients so we need to acquire the lock before