Address #3331 and #3080 (#3335)

* Address !3331 and !3080

* Modify webhook provider TLS config to restrict tls renegotiation to once per client
** Addresses !3331
* Modify webhook certs validation to include intermediates held within tls.crt
** Addresses !3080
** [Cert-Manager recommendation](https://cert-manager.io/docs/configuration/ca) for CA issuer

Signed-off-by: Rick Mulder <rickymulder@gmail.com>

* Add tls chain tests related to #3080

Signed-off-by: Rick Mulder <rickymulder@gmail.com>

* Clean up tls chain test based on sonarcloud recommendation

Signed-off-by: Rick Mulder <rickymulder@gmail.com>

* Fix checkEndpoints and checkCRDs to use proper pointer reference

Signed-off-by: Rick Mulder <rickymulder@gmail.com>

---------

Signed-off-by: Rick Mulder <rickymulder@gmail.com>

pkg/common/webhook/webhook.go

@@ -203,8 +203,9 @@ func (w *Webhook) GetHTTPClient(provider *Spec) (*http.Client, error) {
 	}
 
 	tlsConf := &tls.Config{
-		RootCAs:    caCertPool,
-		MinVersion: tls.VersionTLS12,
+		RootCAs:       caCertPool,
+		MinVersion:    tls.VersionTLS12,
+		Renegotiation: tls.RenegotiateOnceAsClient,
 	}
 	client.Transport = &http.Transport{TLSClientConfig: tlsConf}
 	return client, nil

pkg/controllers/crds/crds_controller.go

@@ -68,6 +68,7 @@ type Reconciler struct {
 	CrdResources    []string
 	dnsName         string
 	CAName          string
+	CAChainName     string
 	CAOrganization  string
 	RequeueInterval time.Duration
 
@@ -152,7 +153,7 @@ func (r *Reconciler) ReadyCheck(_ *http.Request) error {
 	return r.checkEndpoints()
 }
 
-func (r Reconciler) checkCRDs() error {
+func (r *Reconciler) checkCRDs() error {
 	for _, res := range r.CrdResources {
 		r.readyStatusMapMu.Lock()
 		rdy := r.readyStatusMap[res]
@@ -164,7 +165,7 @@ func (r Reconciler) checkCRDs() error {
 	return nil
 }
 
-func (r Reconciler) checkEndpoints() error {
+func (r *Reconciler) checkEndpoints() error {
 	var eps corev1.Endpoints
 	err := r.Get(context.TODO(), types.NamespacedName{
 		Name:      r.SvcName,
@@ -288,10 +289,18 @@ func ValidCert(caCert, cert, key []byte, dnsName string, at time.Time) (bool, er
 		return false, err
 	}
 
-	b, _ := pem.Decode(cert)
+	b, rest := pem.Decode(cert)
 	if b == nil {
 		return false, err
 	}
+	if len(rest) > 0 {
+		intermediate, _ := pem.Decode(rest)
+		inter, err := x509.ParseCertificate(intermediate.Bytes)
+		if err != nil {
+			return false, err
+		}
+		pool.AddCert(inter)
+	}
 
 	crt, err := x509.ParseCertificate(b.Bytes)
 	if err != nil {
@@ -438,6 +447,42 @@ func (r *Reconciler) CreateCACert(begin, end time.Time) (*KeyPairArtifacts, erro
 	return &KeyPairArtifacts{Cert: cert, Key: key, CertPEM: certPEM, KeyPEM: keyPEM}, nil
 }
 
+func (r *Reconciler) CreateCAChain(ca *KeyPairArtifacts, begin, end time.Time) (*KeyPairArtifacts, error) {
+	templ := &x509.Certificate{
+		SerialNumber: big.NewInt(2),
+		Subject: pkix.Name{
+			CommonName:   r.CAChainName,
+			Organization: []string{r.CAOrganization},
+		},
+		DNSNames: []string{
+			r.CAChainName,
+		},
+		NotBefore:             begin,
+		NotAfter:              end,
+		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
+		IsCA:                  true,
+	}
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		return nil, err
+	}
+	der, err := x509.CreateCertificate(rand.Reader, templ, ca.Cert, key.Public(), ca.Key)
+	if err != nil {
+		return nil, err
+	}
+	certPEM, keyPEM, err := pemEncode(der, key)
+	if err != nil {
+		return nil, err
+	}
+	cert, err := x509.ParseCertificate(der)
+	if err != nil {
+		return nil, err
+	}
+
+	return &KeyPairArtifacts{Cert: cert, Key: key, CertPEM: certPEM, KeyPEM: keyPEM}, nil
+}
+
 func (r *Reconciler) CreateCertPEM(ca *KeyPairArtifacts, begin, end time.Time) ([]byte, []byte, error) {
 	templ := &x509.Certificate{
 		SerialNumber: big.NewInt(1),

pkg/controllers/crds/crds_controller_test.go

@@ -33,9 +33,13 @@ import (
 
 const (
 	failedCreateCaCerts     = "could not create ca certificates:%v"
+	failedCreateCaChain     = "could not create the intermediate certificates:%v"
 	failedCreateServerCerts = "could not create server certificates:%v"
 	invalidCerts            = "generated certificates are invalid:%v,%v"
 	dnsName                 = "foobar"
+	tlscrt                  = "/tmp/tls"
+	tlskey                  = "/tmp/key"
+	cacrt                   = "/tmp/ca"
 )
 
 func newReconciler() Reconciler {
@@ -253,9 +257,57 @@ func TestCheckCerts(t *testing.T) {
 	if err != nil {
 		t.Errorf(failedCreateServerCerts, err)
 	}
-	os.WriteFile("/tmp/ca", caArtifacts.CertPEM, 0644)
-	os.WriteFile("/tmp/tls", certPEM, 0644)
-	os.WriteFile("/tmp/key", keyPEM, 0644)
+	os.WriteFile(cacrt, caArtifacts.CertPEM, 0644)
+	os.WriteFile(tlscrt, certPEM, 0644)
+	os.WriteFile(tlskey, keyPEM, 0644)
+	cert := CertInfo{
+		CertDir:  "/tmp",
+		CertName: "tls",
+		CAName:   "ca",
+		KeyName:  "key",
+	}
+	err = CheckCerts(cert, rec.dnsName, time.Now())
+	if err != nil {
+		t.Errorf("error checking valid cert: %v", err)
+	}
+	err = CheckCerts(cert, rec.dnsName, time.Now().AddDate(-1, 0, 0))
+	if err == nil {
+		t.Error("expected failure due to expired certificate, got success")
+	}
+	err = CheckCerts(cert, "wrong", time.Now())
+	if err == nil {
+		t.Error("expected failure due to dns name got, success")
+	}
+	cert.CAName = "wrong"
+	err = CheckCerts(cert, rec.dnsName, time.Now())
+	if err == nil {
+		t.Error("expected failure due to wrong certificate name, got success")
+	}
+}
+
+func TestCheckCertChain(t *testing.T) {
+	rec := newReconciler()
+	rec.dnsName = dnsName
+	caArtifacts, err := rec.CreateCACert(time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2))
+	if err != nil {
+		t.Fatalf(failedCreateCaCerts, err)
+	}
+	chainArtifacts, err := rec.CreateCAChain(caArtifacts, time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2))
+	if err != nil {
+		t.Fatalf(failedCreateCaChain, err)
+	}
+	certPEM, keyPEM, err := rec.CreateCertPEM(chainArtifacts, time.Now(), time.Now().AddDate(0, 0, 1))
+	if err != nil {
+		t.Errorf(failedCreateServerCerts, err)
+	}
+	os.WriteFile(cacrt, caArtifacts.CertPEM, 0644)
+	os.WriteFile(tlscrt, certPEM, 0644)
+	f, _ := os.OpenFile(tlscrt, os.O_APPEND|os.O_WRONLY, 0644)
+	defer f.Close()
+	if _, err = f.Write(chainArtifacts.CertPEM); err != nil {
+		t.Errorf(failedCreateCaChain, err)
+	}
+	os.WriteFile(tlskey, keyPEM, 0644)
 	cert := CertInfo{
 		CertDir:  "/tmp",
 		CertName: "tls",