Skip to content

Commit

Permalink
RFC: Try passing explicit key usages
Browse files Browse the repository at this point in the history
  • Loading branch information
caseydavenport committed Jul 26, 2023
1 parent 09624d9 commit 6f9ccc0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 15 deletions.
5 changes: 4 additions & 1 deletion pkg/controller/apiserver/apiserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package apiserver

import (
"context"
"crypto/x509"
"fmt"
"time"

Expand Down Expand Up @@ -305,7 +306,9 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re
}

if managementCluster != nil {
tunnelCASecret, err = certificateManager.GetKeyPair(r.client, render.VoltronTunnelSecretName, common.OperatorNamespace())
// We don't require any ExtKeyusage on this cert, so pass an empty slice.
usage := []x509.ExtKeyUsage{}
tunnelCASecret, err = certificateManager.GetKeyPair(r.client, render.VoltronTunnelSecretName, common.OperatorNamespace(), usage...)
if tunnelCASecret == nil {
tunnelSecret, err := certificatemanagement.CreateSelfSignedSecret(render.VoltronTunnelSecretName, common.OperatorNamespace(), "tigera-voltron", []string{"voltron"})
if err == nil {
Expand Down
38 changes: 24 additions & 14 deletions pkg/controller/certificatemanager/certificatemanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type certificateManager struct {
// brings their own secrets, CertificateManager will preserve and return them.
type CertificateManager interface {
// GetKeyPair returns an existing KeyPair. If the KeyPair is not found, nil is returned.
GetKeyPair(cli client.Client, secretName, secretNamespace string) (certificatemanagement.KeyPairInterface, error)
GetKeyPair(cli client.Client, secretName, secretNamespace string, requiredKeyUsages ...x509.ExtKeyUsage) (certificatemanagement.KeyPairInterface, error)
// GetOrCreateKeyPair returns a KeyPair. If one exists, some checks are performed. Otherwise, a new KeyPair is created.
GetOrCreateKeyPair(cli client.Client, secretName, secretNamespace string, dnsNames []string) (certificatemanagement.KeyPairInterface, error)
// GetCertificate returns a Certificate. If the certificate is not found, nil is returned.
Expand Down Expand Up @@ -207,7 +207,7 @@ func (cm *certificateManager) GetOrCreateKeyPair(cli client.Client, secretName,
}

// getKeyPair is an internal convenience method to retrieve a keypair or a certificate.
func (cm *certificateManager) getKeyPair(cli client.Client, secretName, secretNamespace string, readCertOnly bool) (certificatemanagement.KeyPairInterface, *x509.Certificate, error) {
func (cm *certificateManager) getKeyPair(cli client.Client, secretName, secretNamespace string, readCertOnly bool, requiredKeyUsages ...x509.ExtKeyUsage) (certificatemanagement.KeyPairInterface, *x509.Certificate, error) {
secret := &corev1.Secret{}
err := cli.Get(context.Background(), types.NamespacedName{
Name: secretName,
Expand Down Expand Up @@ -238,7 +238,7 @@ func (cm *certificateManager) getKeyPair(cli client.Client, secretName, secretNa
}

timeInvalid := x509Cert.NotAfter.Before(time.Now()) || x509Cert.NotBefore.After(time.Now())
if timeInvalid || !ValidForClientAndServer(x509Cert) {
if timeInvalid || !HasRequiredKeyUsage(x509Cert, requiredKeyUsages) {
if !readCertOnly && strings.HasPrefix(x509Cert.Issuer.CommonName, rmeta.TigeraOperatorCAIssuerPrefix) {
if cm.keyPair.CertificateManagement != nil {
// When certificate management is enabled, we can simply return a certificate management key pair;
Expand Down Expand Up @@ -282,18 +282,23 @@ func (cm *certificateManager) getKeyPair(cli client.Client, secretName, secretNa
}, x509Cert, nil
}

// ValidForClientAndServer returns true if the given certificate is valid
// HasRequiredKeyUsage returns true if the given certificate is valid
// for use as both a server certificate, as well as a client certificate for mTLS connections.
func ValidForClientAndServer(cert *x509.Certificate) bool {
var server, client bool
for _, ku := range cert.ExtKeyUsage {
if ku == x509.ExtKeyUsageClientAuth {
client = true
} else if ku == x509.ExtKeyUsageServerAuth {
server = true
func HasRequiredKeyUsage(cert *x509.Certificate, required []x509.ExtKeyUsage) bool {
for _, ku := range required {
found := false
for _, certKU := range cert.ExtKeyUsage {
if certKU == ku {
found = true
break
}
}

if !found {
return false
}
}
return client && server
return true
}

// GetCertificate returns a Certificate. If the certificate is not found or outdated, a k8s.io NotFound error is returned.
Expand All @@ -303,8 +308,13 @@ func (cm *certificateManager) GetCertificate(cli client.Client, secretName, secr
}

// GetKeyPair returns an existing KeyPair. If the KeyPair is not found, nil is returned.
func (cm *certificateManager) GetKeyPair(cli client.Client, secretName, secretNamespace string) (certificatemanagement.KeyPairInterface, error) {
keyPair, _, err := cm.getKeyPair(cli, secretName, secretNamespace, false)
func (cm *certificateManager) GetKeyPair(cli client.Client, secretName, secretNamespace string, requiredKeyUsages ...x509.ExtKeyUsage) (certificatemanagement.KeyPairInterface, error) {
if requiredKeyUsages == nil {
// Default the required ExtKeyUages if nil. Note the distinction between an empty slice and a nil slice.
// An empty slice will not hit this branch, and will thus result in no ExtKeyUsage requirement.
requiredKeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}
}
keyPair, _, err := cm.getKeyPair(cli, secretName, secretNamespace, false, requiredKeyUsages...)
return keyPair, err
}

Expand Down

0 comments on commit 6f9ccc0

Please sign in to comment.