Merge pull request #632 from vmware-tanzu/ldap_additional_unit_test

Add another unit test for the LDAP client code

internal/testutil/tlsserver.go

@@ -1,13 +1,18 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testutil
 
 import (
+	"crypto/tls"
 	"encoding/pem"
+	"errors"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 // TLSTestServer starts a test server listening on a local port using a test CA. It returns the PEM CA bundle and the
@@ -23,3 +28,35 @@ func TLSTestServer(t *testing.T, handler http.HandlerFunc) (caBundlePEM string,
 	}))
 	return caBundle, server.URL
 }
+
+func TLSTestServerWithCert(t *testing.T, handler http.HandlerFunc, certificate *tls.Certificate) (url string) {
+	t.Helper()
+
+	server := http.Server{
+		TLSConfig: &tls.Config{
+			Certificates: []tls.Certificate{*certificate},
+			MinVersion:   tls.VersionTLS12,
+		},
+		Handler: handler,
+	}
+
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	serverShutdownChan := make(chan error)
+	go func() {
+		// Empty certFile and keyFile will use certs from Server.TLSConfig.
+		serverShutdownChan <- server.ServeTLS(l, "", "")
+	}()
+
+	t.Cleanup(func() {
+		_ = server.Close()
+		serveErr := <-serverShutdownChan
+		if !errors.Is(serveErr, http.ErrServerClosed) {
+			t.Log("Got an unexpected error while starting the fake http server!")
+			require.NoError(t, serveErr)
+		}
+	})
+
+	return l.Addr().String()
+}

internal/upstreamldap/upstreamldap_test.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/url"
 	"testing"
+	"time"
 
 	"github.com/go-ldap/ldap/v3"
 	"github.com/golang/mock/gomock"
@@ -19,6 +20,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
 
+	"go.pinniped.dev/internal/certauthority"
 	"go.pinniped.dev/internal/mocks/mockldapconn"
 	"go.pinniped.dev/internal/testutil"
 )
@@ -1123,6 +1125,13 @@ func TestRealTLSDialing(t *testing.T) {
 	require.NoError(t, err)
 	testServerHostAndPort := parsedURL.Host
 
+	caForTestServerWithBadCertName, err := certauthority.New("Test CA", time.Hour)
+	require.NoError(t, err)
+	wrongIP := net.ParseIP("10.2.3.4")
+	cert, err := caForTestServerWithBadCertName.IssueServerCert([]string{"wrong-dns-name"}, []net.IP{wrongIP}, time.Hour)
+	require.NoError(t, err)
+	testServerWithBadCertNameAddr := testutil.TLSTestServerWithCert(t, func(w http.ResponseWriter, r *http.Request) {}, cert)
+
 	unusedPortGrabbingListener, err := net.Listen("tcp", "127.0.0.1:0")
 	require.NoError(t, err)
 	recentlyClaimedHostAndPort := unusedPortGrabbingListener.Addr().String()
@@ -1146,6 +1155,14 @@ func TestRealTLSDialing(t *testing.T) {
 			connProto: TLS,
 			context:   context.Background(),
 		},
+		{
+			name:      "server cert name does not match the address to which the client connected",
+			host:      testServerWithBadCertNameAddr,
+			caBundle:  caForTestServerWithBadCertName.Bundle(),
+			connProto: TLS,
+			context:   context.Background(),
+			wantError: `LDAP Result Code 200 "Network Error": x509: certificate is valid for 10.2.3.4, not 127.0.0.1`,
+		},
 		{
 			name:      "invalid CA bundle with TLS",
 			host:      testServerHostAndPort,