diff --git a/pkg/cmd/cli/cmd/login/helpers.go b/pkg/cmd/cli/cmd/login/helpers.go index 9cbf4dfe7222..e4a421cebc0f 100644 --- a/pkg/cmd/cli/cmd/login/helpers.go +++ b/pkg/cmd/cli/cmd/login/helpers.go @@ -2,6 +2,7 @@ package login import ( "bytes" + "crypto/x509" "fmt" "io" "net" @@ -80,10 +81,23 @@ func dialToServer(clientConfig restclient.Config) error { return nil } -func promptForInsecureTLS(reader io.Reader, out io.Writer) bool { +func promptForInsecureTLS(reader io.Reader, out io.Writer, reason error) bool { + var insecureTLSRequestReason string + if reason != nil { + switch reason.(type) { + case x509.UnknownAuthorityError: + insecureTLSRequestReason = "The server uses a certificate signed by an unknown authority." + case x509.HostnameError: + insecureTLSRequestReason = fmt.Sprintf("The server is using a certificate that does not match its hostname: %s", reason.Error()) + case x509.CertificateInvalidError: + insecureTLSRequestReason = fmt.Sprintf("The server is using an invalid certificate: %s", reason.Error()) + } + } var input bool if term.IsTerminal(reader) { - fmt.Fprintln(out, "The server uses a certificate signed by an unknown authority.") + if len(insecureTLSRequestReason) > 0 { + fmt.Fprintln(out, insecureTLSRequestReason) + } fmt.Fprintln(out, "You can bypass the certificate check, but any data you send to the server could be intercepted by others.") input = cmdutil.PromptForBool(os.Stdin, out, "Use insecure connections? (y/n): ") fmt.Fprintln(out) diff --git a/pkg/cmd/cli/cmd/login/loginoptions.go b/pkg/cmd/cli/cmd/login/loginoptions.go index 8315a865b2e0..d568d995a738 100644 --- a/pkg/cmd/cli/cmd/login/loginoptions.go +++ b/pkg/cmd/cli/cmd/login/loginoptions.go @@ -121,10 +121,10 @@ func (o *LoginOptions) getClientConfig() (*restclient.Config, error) { // certificate authority unknown, check or prompt if we want an insecure // connection or if we already have a cluster stanza that tells us to // connect to this particular server insecurely - case x509.UnknownAuthorityError: + case x509.UnknownAuthorityError, x509.HostnameError, x509.CertificateInvalidError: if o.InsecureTLS || hasExistingInsecureCluster(*clientConfig, *o.StartingKubeConfig) || - promptForInsecureTLS(o.Reader, o.Out) { + promptForInsecureTLS(o.Reader, o.Out, err) { clientConfig.Insecure = true clientConfig.CAFile = "" clientConfig.CAData = nil diff --git a/pkg/cmd/cli/cmd/login/loginoptions_test.go b/pkg/cmd/cli/cmd/login/loginoptions_test.go index 8db03f978c1e..000f7ebf392a 100644 --- a/pkg/cmd/cli/cmd/login/loginoptions_test.go +++ b/pkg/cmd/cli/cmd/login/loginoptions_test.go @@ -1,15 +1,21 @@ package login import ( + "crypto/tls" + "fmt" "net/http" "net/http/httptest" + "regexp" "strings" "testing" + "github.com/MakeNowJust/heredoc" + "github.com/openshift/origin/pkg/cmd/cli/config" "github.com/openshift/origin/pkg/cmd/util/clientcmd" "k8s.io/kubernetes/pkg/client/restclient" + kclientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api" ) func TestNormalizeServerURL(t *testing.T) { @@ -55,6 +61,163 @@ func TestNormalizeServerURL(t *testing.T) { } } +func TestTLSWithCertificateNotMatchingHostname(t *testing.T) { + // generated by 'go run src/crypto/tls/generate_cert.go --rsa-bits 1024 --host invalidhost.com,8.8.8.8 --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h' + invalidHostCert := heredoc.Doc(` + -----BEGIN CERTIFICATE----- + MIICBjCCAW+gAwIBAgIRALOIWXyeLzunaiVkP2itHAEwDQYJKoZIhvcNAQELBQAw + EjEQMA4GA1UEChMHQWNtZSBDbzAgFw03MDAxMDEwMDAwMDBaGA8yMDg0MDEyOTE2 + MDAwMFowEjEQMA4GA1UEChMHQWNtZSBDbzCBnzANBgkqhkiG9w0BAQEFAAOBjQAw + gYkCgYEAuKDlC4aMBbHaXgS+MFud5h3zeE4boSqKgFI6HceySF/a+qg0v+ID6EwQ + DpJ2W5AdJGEBfixo+tym6q3oKWHJUX0hInkJ6dXIdUbVOeO5dIsGG0fZmRD7DDDx + snkXrDB/E0JglHNckRbIh/jvznbDfbddIcdgZ7JVIfnNpigtHZECAwEAAaNaMFgw + DgYDVR0PAQH/BAQDAgKkMBMGA1UdJQQMMAoGCCsGAQUFBwMBMA8GA1UdEwEB/wQF + MAMBAf8wIAYDVR0RBBkwF4IPaW52YWxpZGhvc3QuY29thwQICAgIMA0GCSqGSIb3 + DQEBCwUAA4GBAAkPU044aFkBl4f/muwSh/oPGinnA4fp8ei0KMnLk+0/CjNb3Waa + GtuRVIudRTK2M/RzdpUrwfWlVmkezV4BR1K/aOH9a29zqDTkEjnkIbWwe+piAs+w + VxIxrTqM8rqq8qxeWS54AyF/OaLJgXzDpCFnCb7kY3iyHv6lcmCjluLW + -----END CERTIFICATE-----`) + invalidHostKey := heredoc.Doc(` + -----BEGIN RSA PRIVATE KEY----- + MIICXQIBAAKBgQC4oOULhowFsdpeBL4wW53mHfN4ThuhKoqAUjodx7JIX9r6qDS/ + 4gPoTBAOknZbkB0kYQF+LGj63KbqregpYclRfSEieQnp1ch1RtU547l0iwYbR9mZ + EPsMMPGyeResMH8TQmCUc1yRFsiH+O/OdsN9t10hx2BnslUh+c2mKC0dkQIDAQAB + AoGAZ0ZAuNC7NFhHEL5QcJZe3aC1Vv9B/0XfkWXtckkJFejggcNjNk5D50Xc2Xnd + 0NvtITNN9Xj8BA83IyDCM5uqUwDbOLIc6qYgAGWzxZZSDAQg1iOAAZoXmMTNS6Zf + hQhNUIwB68ELGvbcq7cxQL7L9n4GfISz7PKOOUKTZp0Q8G0CQQD07K7NES340c3I + QVkCW5/ygNK0GuQ8nTcG5yC8R5SDS47N8YzPp17Pajah8+wawYiemY1fUmD7P/bq + Cjl2RtIHAkEAwPo1GzJubN7PSYgPir3TxUGtMJoyc3jfdjblXyGJHwTu2YxeRjd2 + YUPVRpu9JvNjZc+GONvTbTZeNWCvy0JNpwJBAKEsi49JCd6eefOZBTDnCKd1nLKG + q8Ezl/2D5WfhFtsbwrrFhOs1cc++Tnte3/VvfC8aTwz2UfmkyyCSX+P0kMsCQCIL + glb7/LNEU7mbQXKurq+8OHu8mG36wyGt6aVw2yoXyrOiqfclTcM3HmdIjoRSqBSM + Ghfp4FECKHiuSBVJ6z0CQQDF37CRpdQRDPnAedhyApLcIxSbYo1oUm7FxBLyVb7V + HQjFvsOylsSCABXz0FyC7zXQxkEo6CiSahVI/PHz6Zta + -----END RSA PRIVATE KEY-----`) + + server, err := newTLSServer(invalidHostCert, invalidHostKey) + if err != nil { + t.Errorf(err.Error()) + } + server.StartTLS() + defer server.Close() + + testCases := map[string]struct { + serverURL string + skipTLSVerify bool + expectedErrMsg *regexp.Regexp + }{ + "succeed skipping tls": { + serverURL: server.URL, + skipTLSVerify: true, + }, + "certificate hostname doesn't match": { + serverURL: server.URL, + expectedErrMsg: regexp.MustCompile(`The server is using a certificate that does not match its hostname(.*)is valid for 8\.8\.8\.8`), + }, + } + + for name, test := range testCases { + t.Logf("evaluating test: %s", name) + options := &LoginOptions{ + Server: test.serverURL, + InsecureTLS: test.skipTLSVerify, + StartingKubeConfig: &kclientcmdapi.Config{}, + } + + if _, err = options.getClientConfig(); err != nil { + if !test.expectedErrMsg.MatchString(err.Error()) { + t.Errorf("%s: expected error %q but got %q", name, test.expectedErrMsg, err) + } + if test.expectedErrMsg == nil { + t.Errorf("%s: unexpected error: %v", name, err) + } + } else { + if test.expectedErrMsg != nil { + t.Errorf("%s: expected error but got nothing", name) + } + } + } +} + +func TestTLSWithExpiredCertificate(t *testing.T) { + // generated by 'go run src/crypto/tls/generate_cert.go --rsa-bits 1024 --host 127.0.0.1,::1,example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1h' + expiredCert := heredoc.Doc(` + -----BEGIN CERTIFICATE----- + MIICEjCCAXugAwIBAgIRALf82bYpro/jQS8fP74dG5EwDQYJKoZIhvcNAQELBQAw + EjEQMA4GA1UEChMHQWNtZSBDbzAeFw03MDAxMDEwMDAwMDBaFw03MDAxMDEwMTAw + MDBaMBIxEDAOBgNVBAoTB0FjbWUgQ28wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ + AoGBAONNgDXBk2Q1i/aJjTwt03KpQ3nQblMS3IX/H9JWw6ta6UublKBOaD/2o5Xt + FM+Q7XDEnzYw88CK5KHdyejkJo5IBpUjQYJZFzUJ1BC8Lw7yy6dXWYBJboRR1S+1 + JhkMJOtpPecv+4cTaynplYj0WMBjcQthg2RM7tdpyUYpsp2rAgMBAAGjaDBmMA4G + A1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggrBgEFBQcDATAPBgNVHRMBAf8EBTAD + AQH/MC4GA1UdEQQnMCWCC2V4YW1wbGUuY29thwR/AAABhxAAAAAAAAAAAAAAAAAA + AAABMA0GCSqGSIb3DQEBCwUAA4GBAFpdiiM5YAQQN0H5ZMNuHWGlprjp7qVilO8/ + WFePZRWY2vQF8g7/c1cX4bPqG+qFJd+9j2UZNjhadNfMCxvu6BY7NCupOHVHmnRQ + ocvkPoSqobE7qDPfiUuU1J+61Libu6b2IjV3/K9pvZkLiBrqn0YhoXXa0PG+rG1L + 9X7+mb5z + -----END CERTIFICATE-----`) + expiredKey := heredoc.Doc(` + -----BEGIN RSA PRIVATE KEY----- + MIICXQIBAAKBgQDjTYA1wZNkNYv2iY08LdNyqUN50G5TEtyF/x/SVsOrWulLm5Sg + Tmg/9qOV7RTPkO1wxJ82MPPAiuSh3cno5CaOSAaVI0GCWRc1CdQQvC8O8sunV1mA + SW6EUdUvtSYZDCTraT3nL/uHE2sp6ZWI9FjAY3ELYYNkTO7XaclGKbKdqwIDAQAB + AoGBAJPFWKqZ9CZboWhfuE/9Qs/yNonE9VRQmMkMOTXXblHCQpUCyjcFgkTDJUpc + 3QCsKZD8Yr0qSe1M3qJUu+UKHf18LqwiL/ynnalYggxIFS5/SidWCngKvIuEfkLK + VsnCK3jt5qx21iljGHU6bQZHnHB9IGEiBYcnQlvvw/WdvRDBAkEA8/KMpJVwnI1W + 7fzcZ1+mbMeSJoAVIa9u7MgI+LIRZMokDRYeAMvEjm3GYpZDqA5l1dp7KochMep/ + 0vSSTHt7ewJBAO6IbcUIDhXuh2qdxR/Xk5DdDCoxaD1o4ivyj9JsSlGa9JWD7kKN + 6ZFFrn8i7uQuniC1Rwc/4yHhs6OqbiF695ECQQCBwVKzvFUwwDEr1yK4zXStSZ3g + YqJaz4CV63RyK+z6ilaQq2H8FGaRR6yNBdYozre1/0ciAMxUS6H/6Fzk141/AkBe + SguqIP8AaGObH3Z2mc65KsfOPe2IqNcOrDlx4mCWVXxtRdN+933mcPcDRpnMFSlo + oH/NO9Ha6M8L2SjjjyohAkBJHU61+OWz/TAy1nxsMbFsISLn/JrdEZIf2uFORlDN + Z3/XIQ+yeg4Jk1VbTMZ0/fHf9xMFR8acC/7n7jxnzQau + -----END RSA PRIVATE KEY-----`) + + server, err := newTLSServer(expiredCert, expiredKey) + if err != nil { + t.Errorf(err.Error()) + } + server.StartTLS() + defer server.Close() + + testCases := map[string]struct { + serverURL string + skipTLSVerify bool + expectedErrMsg *regexp.Regexp + }{ + "succeed skipping tls": { + serverURL: server.URL, + skipTLSVerify: true, + }, + "certificate expired": { + serverURL: server.URL, + expectedErrMsg: regexp.MustCompile(`The server is using an invalid certificate(.*)has expired`), + }, + } + + for name, test := range testCases { + t.Logf("evaluating test: %s", name) + options := &LoginOptions{ + Server: test.serverURL, + InsecureTLS: test.skipTLSVerify, + StartingKubeConfig: &kclientcmdapi.Config{}, + } + + if _, err = options.getClientConfig(); err != nil { + if !test.expectedErrMsg.MatchString(err.Error()) { + t.Errorf("%s: expected error %q but got %q", name, test.expectedErrMsg, err) + } + if test.expectedErrMsg == nil { + t.Errorf("%s: unexpected error: %v", name, err) + } + } else { + if test.expectedErrMsg != nil { + t.Errorf("%s: expected error but got nothing", name) + } + } + } +} + func TestDialToHTTPServer(t *testing.T) { invoked := make(chan struct{}, 1) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -92,3 +255,60 @@ func TestDialToHTTPServer(t *testing.T) { } } } + +func TestDialToHTTPSServer(t *testing.T) { + invoked := make(chan struct{}, 1) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + invoked <- struct{}{} + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + testCases := map[string]struct { + serverURL string + skipTLSVerify bool + evalExpectedErr func(error) bool + }{ + "succeed dialing": { + serverURL: server.URL, + skipTLSVerify: true, + }, + "certificate unknown": { + serverURL: server.URL, + evalExpectedErr: clientcmd.IsCertificateAuthorityUnknown, + }, + } + + for name, test := range testCases { + t.Logf("evaluating test: %s", name) + clientConfig := &restclient.Config{ + Host: test.serverURL, + Insecure: test.skipTLSVerify, + } + if err := dialToServer(*clientConfig); err != nil { + if test.evalExpectedErr == nil || !test.evalExpectedErr(err) { + t.Errorf("%s: unexpected error: %v", name, err) + } + } else { + if test.evalExpectedErr != nil { + t.Errorf("%s: expected error but got nothing", name) + } + } + } +} + +func newTLSServer(certString, keyString string) (*httptest.Server, error) { + invoked := make(chan struct{}, 1) + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + invoked <- struct{}{} + w.WriteHeader(http.StatusOK) + })) + cert, err := tls.X509KeyPair([]byte(certString), []byte(keyString)) + if err != nil { + return nil, fmt.Errorf("error configuring server cert: %s", err) + } + server.TLS = &tls.Config{ + Certificates: []tls.Certificate{cert}, + } + return server, nil +} diff --git a/pkg/cmd/util/clientcmd/errors.go b/pkg/cmd/util/clientcmd/errors.go index f10006fd76d2..6d75e75ea7d7 100644 --- a/pkg/cmd/util/clientcmd/errors.go +++ b/pkg/cmd/util/clientcmd/errors.go @@ -1,6 +1,7 @@ package clientcmd import ( + "crypto/x509" "errors" "fmt" "strings" @@ -13,6 +14,8 @@ const ( unknownReason = iota noServerFoundReason certificateAuthorityUnknownReason + certificateHostnameErrorReason + certificateInvalidReason configurationInvalidReason tlsOversizedRecordReason @@ -50,6 +53,12 @@ func GetPrettyMessageForServer(err error, serverName string) string { serverName = "server" } return fmt.Sprintf(tlsOversizedRecordMsg, err, serverName) + + case certificateHostnameErrorReason: + return fmt.Sprintf("The server is using a certificate that does not match its hostname: %s", err) + + case certificateInvalidReason: + return fmt.Sprintf("The server is using an invalid certificate: %s", err) } return err.Error() @@ -91,6 +100,17 @@ func IsTLSOversizedRecord(err error) bool { return detectReason(err) == tlsOversizedRecordReason } +// IsCertificateHostnameError checks whether the set of authorized names doesn't match the requested name +func IsCertificateHostnameError(err error) bool { + return detectReason(err) == certificateHostnameErrorReason +} + +// IsCertificateInvalid checks whether the certificate is invalid for reasons like expired, CA not authorized +// to sign, there are too many cert intermediates, or the cert usage is not valid for the wanted purpose. +func IsCertificateInvalid(err error) bool { + return detectReason(err) == certificateInvalidReason +} + func detectReason(err error) int { if err != nil { switch { @@ -103,6 +123,14 @@ func detectReason(err error) int { case strings.Contains(err.Error(), "tls: oversized record received"): return tlsOversizedRecordReason } + switch err.(type) { + case x509.UnknownAuthorityError: + return certificateAuthorityUnknownReason + case x509.HostnameError: + return certificateHostnameErrorReason + case x509.CertificateInvalidError: + return certificateInvalidReason + } } return unknownReason }