From ed4d44535204955188decebeed88c6c54272c94c Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Fri, 8 Jun 2018 11:57:15 +0100 Subject: [PATCH] Check certificate thumbprint when configured --- .../providers/vsphere/vclib/connection.go | 31 ++++ .../vsphere/vclib/connection_test.go | 138 ++++++++++++++---- 2 files changed, 142 insertions(+), 27 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/connection.go b/pkg/cloudprovider/providers/vsphere/vclib/connection.go index fd82370e4ba..8d2e7b27d08 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/connection.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/connection.go @@ -17,6 +17,7 @@ limitations under the License. package vclib import ( + "bytes" "context" "crypto/tls" "encoding/pem" @@ -24,6 +25,7 @@ import ( "net" neturl "net/url" "sync" + "unicode" "github.com/golang/glog" "github.com/vmware/govmomi/session" @@ -169,6 +171,13 @@ func (connection *VSphereConnection) NewClient(ctx context.Context) (*vim25.Clie } } + tpHost := connection.Hostname + ":" + connection.Port + tp, err := normalizeThumbprint(connection.Thumbprint) + if err != nil { + return nil, err + } + sc.SetThumbprint(tpHost, tp) + client, err := vim25.NewClient(ctx, sc) if err != nil { glog.Errorf("Failed to create new client. err: %+v", err) @@ -201,3 +210,25 @@ func (connection *VSphereConnection) UpdateCredentials(username string, password connection.Username = username connection.Password = password } + +func normalizeThumbprint(original string) (string, error) { + buffer := &bytes.Buffer{} + outIdx := 0 + + for _, r := range original { + if outIdx%2 == 0 && outIdx > 0 { + if _, err := buffer.WriteRune(':'); err != nil { + return "", err + } + } + if r == ':' { + continue + } + if _, err := buffer.WriteRune(unicode.ToUpper(r)); err != nil { + return "", err + } + outIdx++ + } + + return buffer.String(), nil +} diff --git a/pkg/cloudprovider/providers/vsphere/vclib/connection_test.go b/pkg/cloudprovider/providers/vsphere/vclib/connection_test.go index 9dc31e235ca..dcf9f9adaa1 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/connection_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/connection_test.go @@ -18,20 +18,29 @@ package vclib_test import ( "context" + "crypto/sha1" "crypto/tls" "crypto/x509" + "fmt" "io/ioutil" "net/http" "net/http/httptest" "net/url" "os" + "strings" "testing" "k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere/vclib" "k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere/vclib/fixtures" ) -func createTestServer(t *testing.T, caCertPath, serverCertPath, serverKeyPath string, handler http.HandlerFunc) *httptest.Server { +func createTestServer( + t *testing.T, + caCertPath string, + serverCertPath string, + serverKeyPath string, + handler http.HandlerFunc, +) (*httptest.Server, string) { caCertPEM, err := ioutil.ReadFile(caCertPath) if err != nil { t.Fatalf("Could not read ca cert from file") @@ -55,18 +64,18 @@ func createTestServer(t *testing.T, caCertPath, serverCertPath, serverKeyPath st RootCAs: certPool, } - // // calculate the leaf certificate's fingerprint - // x509LeafCert := server.TLS.Certificates[0].Certificate[0] - // tpBytes := sha1.Sum(x509LeafCert) - // tpString := fmt.Sprintf("%x", tpBytes) + // calculate the leaf certificate's fingerprint + x509LeafCert := server.TLS.Certificates[0].Certificate[0] + tpBytes := sha1.Sum(x509LeafCert) + tpString := fmt.Sprintf("%x", tpBytes) - return server + return server, tpString } func TestWithValidCaCert(t *testing.T) { - handler, verify := getRequestVerifier(t) + handler, verifyConnectionWasMade := getRequestVerifier(t) - server := createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) + server, _ := createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) server.StartTLS() u := mustParseUrl(t, server.URL) @@ -79,27 +88,88 @@ func TestWithValidCaCert(t *testing.T) { // Ignoring error here, because we only care about the TLS connection connection.NewClient(context.Background()) - verify() + verifyConnectionWasMade() } -// func TestWithValidThumbprint(t *testing.T) { -// handler, verify := getRequestVerifier(t) -// -// server, serverThumbprint := createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) -// server.StartTLS() -// u := mustParseUrl(t, server.URL) -// -// connection := &vclib.VSphereConnection{ -// Hostname: u.Hostname(), -// Port: u.Port(), -// Thumbprint: serverThumbprint, -// } -// -// // Ignoring error here, because we only care about the TLS connection -// connection.NewClient(context.Background()) -// -// verify() -// } +func TestWithVerificationWithWrongThumbprint(t *testing.T) { + handler, _ := getRequestVerifier(t) + + server, _ := createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) + server.StartTLS() + u := mustParseUrl(t, server.URL) + + connection := &vclib.VSphereConnection{ + Hostname: u.Hostname(), + Port: u.Port(), + Thumbprint: "obviously wrong", + } + + _, err := connection.NewClient(context.Background()) + + if msg := err.Error(); !strings.Contains(msg, "thumbprint does not match") { + t.Fatalf("Expected wrong thumbprint error, got '%s'", msg) + } +} + +func TestWithVerificationWithoutCaCertOrThumbprint(t *testing.T) { + handler, _ := getRequestVerifier(t) + + server, _ := createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) + server.StartTLS() + u := mustParseUrl(t, server.URL) + + connection := &vclib.VSphereConnection{ + Hostname: u.Hostname(), + Port: u.Port(), + } + + _, err := connection.NewClient(context.Background()) + + verifyWrappedX509UnkownAuthorityErr(t, err) +} + +func TestWithValidThumbprint(t *testing.T) { + handler, verifyConnectionWasMade := getRequestVerifier(t) + + server, thumbprint := + createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) + server.StartTLS() + u := mustParseUrl(t, server.URL) + + connection := &vclib.VSphereConnection{ + Hostname: u.Hostname(), + Port: u.Port(), + Thumbprint: thumbprint, + } + + // Ignoring error here, because we only care about the TLS connection + connection.NewClient(context.Background()) + + verifyConnectionWasMade() +} + +func TestWithValidThumbprintAlternativeFormat(t *testing.T) { + handler, verifyConnectionWasMade := getRequestVerifier(t) + + server, thumbprint := + createTestServer(t, fixtures.CaCertPath, fixtures.ServerCertPath, fixtures.ServerKeyPath, handler) + server.StartTLS() + u := mustParseUrl(t, server.URL) + + // lowercase, remove the ':' + tpDifferentFormat := strings.Replace(strings.ToLower(thumbprint), ":", "", -1) + + connection := &vclib.VSphereConnection{ + Hostname: u.Hostname(), + Port: u.Port(), + Thumbprint: tpDifferentFormat, + } + + // Ignoring error here, because we only care about the TLS connection + connection.NewClient(context.Background()) + + verifyConnectionWasMade() +} func TestWithInvalidCaCertPath(t *testing.T) { connection := &vclib.VSphereConnection{ @@ -130,6 +200,20 @@ func TestInvalidCaCert(t *testing.T) { } } +func verifyWrappedX509UnkownAuthorityErr(t *testing.T, err error) { + urlErr, ok := err.(*url.Error) + if !ok { + t.Fatalf("Expected to receive an url.Error, got '%s' (%#v)", err.Error(), err) + } + x509Err, ok := urlErr.Err.(x509.UnknownAuthorityError) + if !ok { + t.Fatalf("Expected to receive a wrapped x509.UnknownAuthorityError, got: '%s' (%#v)", urlErr.Error(), urlErr) + } + if msg := x509Err.Error(); msg != "x509: certificate signed by unknown authority" { + t.Fatalf("Expected 'signed by unknown authority' error, got: '%s'", msg) + } +} + func getRequestVerifier(t *testing.T) (http.HandlerFunc, func()) { gotRequest := false