Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepend certs instead of append #48

Merged
merged 1 commit into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/certificate/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,15 @@ func (m *Manager) getCACertsFromCABundle() ([]*x509.Certificate, error) {
return cas, nil
}

func (m *Manager) getLastAppendedCACertFromCABundle() (*x509.Certificate, error) {
func (m *Manager) getLastPrependedCACertFromCABundle() (*x509.Certificate, error) {
cas, err := m.getCACertsFromCABundle()
if err != nil {
return nil, errors.Wrap(err, "failed getting CA certificates from CA bundle")
}
if len(cas) == 0 {
return nil, nil
}
return cas[len(cas)-1], nil
return cas[0], nil
}

func (m *Manager) rotateAll() error {
Expand Down Expand Up @@ -262,7 +262,7 @@ func (m *Manager) nextRotationDeadlineForCA() time.Time {

// Last rotated CA cert at CABundle is the last at the slice so this
// calculate deadline from it.
caCert, err := m.getLastAppendedCACertFromCABundle()
caCert, err := m.getLastPrependedCACertFromCABundle()
if err != nil {
m.log.Info("Failed reading last CA cert from CABundle, forcing rotation", "err", err)
return m.now()
Expand Down
2 changes: 1 addition & 1 deletion pkg/certificate/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ var _ = Describe("certificate manager", func() {

obtainedMutatingWebhookConfiguration := loadMutatingWebhook(m)
caBundle := obtainedMutatingWebhookConfiguration.Webhooks[0].ClientConfig.CABundle
hackedCABundle := append(caBundle, triple.EncodeCertPEM(hackedCA.Cert)...)
hackedCABundle := append(triple.EncodeCertPEM(hackedCA.Cert), caBundle...)
obtainedMutatingWebhookConfiguration.Webhooks[0].ClientConfig.CABundle = hackedCABundle
updateMutatingWebhook(m, &obtainedMutatingWebhookConfiguration)
},
Expand Down
12 changes: 6 additions & 6 deletions pkg/certificate/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (m *Manager) verifyTLSSecret(secretKey types.NamespacedName, caKeyPair *tri
return errors.New("CA bundle has no certificates")
}

lastCertFromCABundle := getLastCert(certsFromCABundle)
lastCertFromCABundle := getFirstCert(certsFromCABundle)

if !reflect.DeepEqual(*lastCertFromCABundle, *caKeyPair.Cert) {
return errors.New("CA bundle and CA secret certificate are different")
Expand Down Expand Up @@ -236,9 +236,9 @@ func (m *Manager) getTLSKeyPair(secretKey types.NamespacedName) (*triple.KeyPair
return nil, errors.Wrapf(err, "failed parsing TLS private key PEM at secret %s", secretKey)
}

lastAppendedCert := getLastCert(certs)
lastPrependedCert := getFirstCert(certs)

return &triple.KeyPair{Key: privateKey.(*rsa.PrivateKey), Cert: lastAppendedCert}, nil
return &triple.KeyPair{Key: privateKey.(*rsa.PrivateKey), Cert: lastPrependedCert}, nil
}

func (m *Manager) getTLSCerts(secretKey types.NamespacedName) ([]*x509.Certificate, error) {
Expand All @@ -265,11 +265,11 @@ func (m *Manager) caSecretKey() types.NamespacedName {
return types.NamespacedName{Namespace: m.namespace, Name: m.webhookName + "-ca"}
}

// Certs are appended to implement overlap so we take the last one
// Certs are prepended to implement overlap so we take the first one
// it will match with the key
func getLastCert(certs []*x509.Certificate) *x509.Certificate {
func getFirstCert(certs []*x509.Certificate) *x509.Certificate {
if len(certs) == 0 {
return nil
}
return certs[len(certs)-1]
return certs[0]
}
6 changes: 6 additions & 0 deletions pkg/certificate/triple/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"crypto/rand"
cryptorand "crypto/rand"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
Expand Down Expand Up @@ -177,6 +178,11 @@ func VerifyTLS(certsPEM, keyPEM, caBundle []byte) error {
return errors.Wrap(err, "failed to verify certificate")
}

_, err = tls.X509KeyPair(certsPEM, keyPEM)
if err != nil {
return errors.Wrap(err, "failed parsing TLS public/private key")
}

logger.Info("TLS certificates chain verified")
return nil
}
4 changes: 3 additions & 1 deletion pkg/certificate/triple/pem.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ func AddCertToPEM(cert *x509.Certificate, pemCerts []byte) ([]byte, error) {
return nil, fmt.Errorf("failed parsing current certs PEM: %w", err)
}
}
certs = append(certs, cert)
// Prepend cert since it's what TLS expects [1]
// [1] https://github.com/golang/go/blob/master/src/crypto/tls/tls.go#L292-L294
certs = append([]*x509.Certificate{cert}, certs...)
return EncodeCertsPEM(certs), nil
}

Expand Down