diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index 0ccbd6ed2e..13bb3e761d 100644 --- a/cmd/nginx/main.go +++ b/cmd/nginx/main.go @@ -110,14 +110,19 @@ func main() { } // create the default SSL certificate (dummy) + // TODO(elvinefendi) do this in a single function in ssl package defCert, defKey := ssl.GetFakeSSLCert() - c, err := ssl.AddOrUpdateCertAndKey(fakeCertificate, defCert, defKey, []byte{}, fs) + sslCert, err := ssl.CreateSSLCert(defCert, defKey) if err != nil { - klog.Fatalf("Error generating self-signed certificate: %v", err) + klog.Fatalf("unexpected error creating fake SSL Cert: %v", err) } - - conf.FakeCertificatePath = c.PemFileName - conf.FakeCertificateSHA = c.PemSHA + err = ssl.StoreSSLCertOnDisk(fs, fakeCertificate, sslCert) + if err != nil { + klog.Fatalf("unexpected error storing fake SSL Cert: %v", err) + } + conf.FakeCertificatePath = sslCert.PemFileName + conf.FakeCertificateSHA = sslCert.PemSHA + // end create default fake SSL certificates conf.Client = kubeClient diff --git a/internal/ingress/controller/store/backend_ssl.go b/internal/ingress/controller/store/backend_ssl.go index e593f564fc..93cdfdad99 100644 --- a/internal/ingress/controller/store/backend_ssl.go +++ b/internal/ingress/controller/store/backend_ssl.go @@ -98,17 +98,22 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error return nil, fmt.Errorf("key 'tls.key' missing from Secret %q", secretName) } - if s.isDynamicCertificatesEnabled { - sslCert, err = ssl.CreateSSLCert(nsSecName, cert, key, ca) + sslCert, err = ssl.CreateSSLCert(cert, key) + if err != nil { + return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err) + } + + if !s.isDynamicCertificatesEnabled || len(ca) > 0 { + err = ssl.StoreSSLCertOnDisk(s.filesystem, nsSecName, sslCert) if err != nil { - return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err) + return nil, fmt.Errorf("error while storing certificate and key: %v", err) } - } else { - // If 'ca.crt' is also present, it will allow this secret to be used in the - // 'nginx.ingress.kubernetes.io/auth-tls-secret' annotation - sslCert, err = ssl.AddOrUpdateCertAndKey(nsSecName, cert, key, ca, s.filesystem) + } + + if len(ca) > 0 { + err = ssl.ConfigureCACertWithCertAndKey(s.filesystem, nsSecName, ca, sslCert) if err != nil { - return nil, fmt.Errorf("unexpected error creating pem file: %v", err) + return nil, fmt.Errorf("error configuring CA certificate: %v", err) } } @@ -118,11 +123,15 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error } klog.V(3).Info(msg) - } else if ca != nil { - sslCert, err = ssl.AddCertAuth(nsSecName, ca, s.filesystem) + } else if ca != nil && len(ca) > 0 { + sslCert, err = ssl.CreateCACert(ca) + if err != nil { + return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err) + } + err = ssl.ConfigureCACert(s.filesystem, nsSecName, ca, sslCert) if err != nil { - return nil, err + return nil, fmt.Errorf("error configuring CA certificate: %v", err) } // makes this secret in 'syncSecret' to be used for Certificate Authentication diff --git a/internal/net/ssl/ssl.go b/internal/net/ssl/ssl.go index cdcf6f4f0e..7a7ce30b73 100644 --- a/internal/net/ssl/ssl.go +++ b/internal/net/ssl/ssl.go @@ -46,51 +46,42 @@ var ( oidExtensionSubjectAltName = asn1.ObjectIdentifier{2, 5, 29, 17} ) -// AddOrUpdateCertAndKey creates a .pem file with the cert and the key with the specified name -func AddOrUpdateCertAndKey(name string, cert, key, ca []byte, - fs file.Filesystem) (*ingress.SSLCert, error) { - - pemName := fmt.Sprintf("%v.pem", name) - pemFileName := fmt.Sprintf("%v/%v", file.DefaultSSLDirectory, pemName) - tempPemFile, err := fs.TempFile(file.DefaultSSLDirectory, pemName) +// getPemFileName returns absolute file path and file name of pem cert related to given fullSecretName +func getPemFileName(fullSecretName string) (string, string) { + pemName := fmt.Sprintf("%v.pem", fullSecretName) + return fmt.Sprintf("%v/%v", file.DefaultSSLDirectory, pemName), pemName +} - if err != nil { - return nil, fmt.Errorf("could not create temp pem file %v: %v", pemFileName, err) +func verifyPemCertAgainstRootCA(pemCert *x509.Certificate, ca []byte) error { + bundle := x509.NewCertPool() + bundle.AppendCertsFromPEM(ca) + opts := x509.VerifyOptions{ + Roots: bundle, } - klog.V(3).Infof("Creating temp file %v for Keypair: %v", tempPemFile.Name(), pemName) - _, err = tempPemFile.Write(cert) - if err != nil { - return nil, fmt.Errorf("could not write to pem file %v: %v", tempPemFile.Name(), err) - } - _, err = tempPemFile.Write([]byte("\n")) - if err != nil { - return nil, fmt.Errorf("could not write to pem file %v: %v", tempPemFile.Name(), err) - } - _, err = tempPemFile.Write(key) + _, err := pemCert.Verify(opts) if err != nil { - return nil, fmt.Errorf("could not write to pem file %v: %v", tempPemFile.Name(), err) + return err } - err = tempPemFile.Close() - if err != nil { - return nil, fmt.Errorf("could not close temp pem file %v: %v", tempPemFile.Name(), err) - } - defer fs.RemoveAll(tempPemFile.Name()) + return nil +} - pemCerts, err := fs.ReadFile(tempPemFile.Name()) - if err != nil { - return nil, err - } +// CreateSSLCert validates cert and key, extracts common names and returns corresponding SSLCert object +func CreateSSLCert(cert, key []byte) (*ingress.SSLCert, error) { + var pemCertBuffer bytes.Buffer - pemBlock, _ := pem.Decode(pemCerts) + pemCertBuffer.Write(cert) + pemCertBuffer.Write([]byte("\n")) + pemCertBuffer.Write(key) + + pemBlock, _ := pem.Decode(pemCertBuffer.Bytes()) if pemBlock == nil { return nil, fmt.Errorf("no valid PEM formatted block found") } - // If the file does not start with 'BEGIN CERTIFICATE' it's invalid and must not be used. if pemBlock.Type != "CERTIFICATE" { - return nil, fmt.Errorf("certificate %v contains invalid data, and must be created with 'kubectl create secret tls'", name) + return nil, fmt.Errorf("no certificate PEM data found, make sure certificate content starts with 'BEGIN CERTIFICATE'") } pemCert, err := x509.ParseCertificate(pemBlock.Bytes) @@ -98,9 +89,8 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte, return nil, err } - //Ensure that certificate and private key have a matching public key if _, err := tls.X509KeyPair(cert, key); err != nil { - return nil, err + return nil, fmt.Errorf("certificate and private key does not have a matching public key: %v", err) } cn := sets.NewString(pemCert.Subject.CommonName) @@ -127,146 +117,128 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte, } } - err = fs.Rename(tempPemFile.Name(), pemFileName) - if err != nil { - return nil, fmt.Errorf("could not move temp pem file %v to destination %v: %v", tempPemFile.Name(), pemFileName, err) - } - - if len(ca) > 0 { - bundle := x509.NewCertPool() - bundle.AppendCertsFromPEM(ca) - opts := x509.VerifyOptions{ - Roots: bundle, - } - - _, err := pemCert.Verify(opts) - if err != nil { - oe := fmt.Sprintf("failed to verify certificate chain: \n\t%s\n", err) - return nil, errors.New(oe) - } + return &ingress.SSLCert{ + Certificate: pemCert, + CN: cn.List(), + ExpireTime: pemCert.NotAfter, + PemCertKey: pemCertBuffer.String(), + }, nil +} - caData, err := fs.ReadFile(pemFileName) - if err != nil { - return nil, fmt.Errorf("could not open file %v for writing additional CA chains: %v", pemFileName, err) - } +// CreateCACert is similar to CreateSSLCert but it creates instance of SSLCert only based on given ca after +// parsing and validating it +func CreateCACert(ca []byte) (*ingress.SSLCert, error) { + pemCABlock, _ := pem.Decode(ca) + if pemCABlock == nil { + return nil, fmt.Errorf("no valid PEM formatted block found") + } + // If the first certificate does not start with 'BEGIN CERTIFICATE' it's invalid and must not be used. + if pemCABlock.Type != "CERTIFICATE" { + return nil, fmt.Errorf("no certificate PEM data found, make sure certificate content starts with 'BEGIN CERTIFICATE'") + } - caFile, err := fs.Create(pemFileName) - if err != nil { - return nil, fmt.Errorf("could not create CA cert file %v: %v", pemFileName, err) - } + pemCert, err := x509.ParseCertificate(pemCABlock.Bytes) + if err != nil { + return nil, err + } - _, err = caFile.Write(caData) - if err != nil { - return nil, fmt.Errorf("could not append CA to cert file %v: %v", pemFileName, err) - } + return &ingress.SSLCert{ + Certificate: pemCert, + }, nil +} - _, err = caFile.Write([]byte("\n")) - if err != nil { - return nil, fmt.Errorf("could not append CA to cert file %v: %v", pemFileName, err) - } - caFile.Write(ca) - caFile.Write([]byte("\n")) - defer caFile.Close() +// StoreSSLCertOnDisk creates a .pem file with content PemCertKey from the given sslCert +// and sets relevant remaining fields of sslCert object +func StoreSSLCertOnDisk(fs file.Filesystem, name string, sslCert *ingress.SSLCert) error { + pemFileName, _ := getPemFileName(name) - return &ingress.SSLCert{ - Certificate: pemCert, - CAFileName: pemFileName, - PemFileName: pemFileName, - PemSHA: file.SHA1(pemFileName), - CN: cn.List(), - ExpireTime: pemCert.NotAfter, - }, nil + pemFile, err := fs.Create(pemFileName) + if err != nil { + return fmt.Errorf("could not create PEM certificate file %v: %v", pemFileName, err) } + defer pemFile.Close() - s := &ingress.SSLCert{ - Certificate: pemCert, - PemFileName: pemFileName, - PemSHA: file.SHA1(pemFileName), - CN: cn.List(), - ExpireTime: pemCert.NotAfter, + _, err = pemFile.Write([]byte(sslCert.PemCertKey)) + if err != nil { + return fmt.Errorf("could not write data to PEM file %v: %v", pemFileName, err) } - return s, nil -} + sslCert.PemFileName = pemFileName + sslCert.PemSHA = file.SHA1(pemFileName) -// CreateSSLCert creates an SSLCert and avoids writing on disk -func CreateSSLCert(name string, cert, key, ca []byte) (*ingress.SSLCert, error) { - var pemCertBuffer bytes.Buffer + return nil +} - pemCertBuffer.Write(cert) - pemCertBuffer.Write([]byte("\n")) - pemCertBuffer.Write(key) +func isSSLCertStoredOnDisk(sslCert *ingress.SSLCert) bool { + return len(sslCert.PemFileName) > 0 +} - pemBlock, _ := pem.Decode(pemCertBuffer.Bytes()) - if pemBlock == nil { - return nil, fmt.Errorf("no valid PEM formatted block found") +// ConfigureCACertWithCertAndKey appends ca into existing PEM file consisting of cert and key +// and sets relevant fields in sslCert object +func ConfigureCACertWithCertAndKey(fs file.Filesystem, name string, ca []byte, sslCert *ingress.SSLCert) error { + err := verifyPemCertAgainstRootCA(sslCert.Certificate, ca) + if err != nil { + oe := fmt.Sprintf("failed to verify certificate chain: \n\t%s\n", err) + return errors.New(oe) } - // If the file does not start with 'BEGIN CERTIFICATE' it's invalid and must not be used. - if pemBlock.Type != "CERTIFICATE" { - return nil, fmt.Errorf("certificate %v contains invalid data, and must be created with 'kubectl create secret tls'", name) + certAndKey, err := fs.ReadFile(sslCert.PemFileName) + if err != nil { + return fmt.Errorf("could not read file %v for writing additional CA chains: %v", sslCert.PemFileName, err) } - pemCert, err := x509.ParseCertificate(pemBlock.Bytes) + f, err := fs.Create(sslCert.PemFileName) if err != nil { - return nil, err + return fmt.Errorf("could not create PEM file %v: %v", sslCert.PemFileName, err) } + defer f.Close() - //Ensure that certificate and private key have a matching public key - if _, err := tls.X509KeyPair(cert, key); err != nil { - return nil, err + _, err = f.Write(certAndKey) + if err != nil { + return fmt.Errorf("could not write cert and key bundle to cert file %v: %v", sslCert.PemFileName, err) } - cn := sets.NewString(pemCert.Subject.CommonName) - for _, dns := range pemCert.DNSNames { - if !cn.Has(dns) { - cn.Insert(dns) - } + _, err = f.Write([]byte("\n")) + if err != nil { + return fmt.Errorf("could not append newline to cert file %v: %v", sslCert.PemFileName, err) } - if len(pemCert.Extensions) > 0 { - klog.V(3).Info("parsing ssl certificate extensions") - for _, ext := range getExtension(pemCert, oidExtensionSubjectAltName) { - dns, _, _, err := parseSANExtension(ext.Value) - if err != nil { - klog.Warningf("unexpected error parsing certificate extensions: %v", err) - continue - } - - for _, dns := range dns { - if !cn.Has(dns) { - cn.Insert(dns) - } - } - } + _, err = f.Write(ca) + if err != nil { + return fmt.Errorf("could not write ca data to cert file %v: %v", sslCert.PemFileName, err) } - if len(ca) > 0 { - bundle := x509.NewCertPool() - bundle.AppendCertsFromPEM(ca) - opts := x509.VerifyOptions{ - Roots: bundle, - } + sslCert.CAFileName = sslCert.PemFileName + // since we updated sslCert.PemFileName we need to recalculate the checksum + sslCert.PemSHA = file.SHA1(sslCert.PemFileName) - _, err := pemCert.Verify(opts) - if err != nil { - oe := fmt.Sprintf("failed to verify certificate chain: \n\t%s\n", err) - return nil, errors.New(oe) - } + return nil +} - pemCertBuffer.Write([]byte("\n")) - pemCertBuffer.Write(ca) - pemCertBuffer.Write([]byte("\n")) +// ConfigureCACert is similar to ConfigureCACertWithCertAndKey but it creates a separate file +// for CA cert and writes only ca into it and then sets relevant fields in sslCert +func ConfigureCACert(fs file.Filesystem, name string, ca []byte, sslCert *ingress.SSLCert) error { + caName := fmt.Sprintf("ca-%v.pem", name) + fileName := fmt.Sprintf("%v/%v", file.DefaultSSLDirectory, caName) + + f, err := fs.Create(fileName) + if err != nil { + return fmt.Errorf("could not write CA file %v: %v", fileName, err) } + defer f.Close() - s := &ingress.SSLCert{ - Certificate: pemCert, - CN: cn.List(), - ExpireTime: pemCert.NotAfter, - PemCertKey: pemCertBuffer.String(), + _, err = f.Write(ca) + if err != nil { + return fmt.Errorf("could not write CA file %v: %v", fileName, err) } - return s, nil + sslCert.PemFileName = fileName + sslCert.CAFileName = fileName + sslCert.PemSHA = file.SHA1(fileName) + + klog.V(3).Infof("Created CA Certificate for Authentication: %v", fileName) + + return nil } func getExtension(c *x509.Certificate, id asn1.ObjectIdentifier) []pkix.Extension { @@ -335,51 +307,9 @@ func parseSANExtension(value []byte) (dnsNames, emailAddresses []string, ipAddre return } -// AddCertAuth creates a .pem file with the specified CAs to be used in Cert Authentication -// If it's already exists, it's clobbered. -func AddCertAuth(name string, ca []byte, fs file.Filesystem) (*ingress.SSLCert, error) { - - caName := fmt.Sprintf("ca-%v.pem", name) - caFileName := fmt.Sprintf("%v/%v", file.DefaultSSLDirectory, caName) - - pemCABlock, _ := pem.Decode(ca) - if pemCABlock == nil { - return nil, fmt.Errorf("no valid PEM formatted block found") - } - // If the first certificate does not start with 'BEGIN CERTIFICATE' it's invalid and must not be used. - if pemCABlock.Type != "CERTIFICATE" { - return nil, fmt.Errorf("CA file %v contains invalid data, and must be created only with PEM formatted certificates", name) - } - - pemCert, err := x509.ParseCertificate(pemCABlock.Bytes) - if err != nil { - return nil, err - } - - caFile, err := fs.Create(caFileName) - if err != nil { - return nil, fmt.Errorf("could not write CA file %v: %v", caFileName, err) - } - defer caFile.Close() - - _, err = caFile.Write(ca) - if err != nil { - return nil, fmt.Errorf("could not write CA file %v: %v", caFileName, err) - } - - klog.V(3).Infof("Created CA Certificate for Authentication: %v", caFileName) - return &ingress.SSLCert{ - Certificate: pemCert, - CAFileName: caFileName, - PemFileName: caFileName, - PemSHA: file.SHA1(caFileName), - }, nil -} - // AddOrUpdateDHParam creates a dh parameters file with the specified name func AddOrUpdateDHParam(name string, dh []byte, fs file.Filesystem) (string, error) { - pemName := fmt.Sprintf("%v.pem", name) - pemFileName := fmt.Sprintf("%v/%v", file.DefaultSSLDirectory, pemName) + pemFileName, pemName := getPemFileName(name) tempPemFile, err := fs.TempFile(file.DefaultSSLDirectory, pemName) diff --git a/internal/net/ssl/ssl_test.go b/internal/net/ssl/ssl_test.go index 04c7018fda..fa07b505c8 100644 --- a/internal/net/ssl/ssl_test.go +++ b/internal/net/ssl/ssl_test.go @@ -56,7 +56,7 @@ func generateRSACerts(host string) (*keyPair, *keyPair, error) { }, ca, nil } -func TestAddOrUpdateCertAndKey(t *testing.T) { +func TestStoreSSLCertOnDisk(t *testing.T) { fs := newFS(t) cert, _, err := generateRSACerts("echoheaders") @@ -69,21 +69,26 @@ func TestAddOrUpdateCertAndKey(t *testing.T) { c := certutil.EncodeCertPEM(cert.Cert) k := certutil.EncodePrivateKeyPEM(cert.Key) - ngxCert, err := AddOrUpdateCertAndKey(name, c, k, []byte{}, fs) + sslCert, err := CreateSSLCert(c, k) if err != nil { - t.Fatalf("unexpected error checking SSL certificate: %v", err) + t.Fatalf("unexpected error creating SSL certificate: %v", err) } - if ngxCert.PemFileName == "" { + err = StoreSSLCertOnDisk(fs, name, sslCert) + if err != nil { + t.Fatalf("unexpected error storing SSL certificate: %v", err) + } + + if sslCert.PemFileName == "" { t.Fatalf("expected path to pem file but returned empty") } - if len(ngxCert.CN) == 0 { + if len(sslCert.CN) == 0 { t.Fatalf("expected at least one cname but none returned") } - if ngxCert.CN[0] != "echoheaders" { - t.Fatalf("expected cname echoheaders but %v returned", ngxCert.CN[0]) + if sslCert.CN[0] != "echoheaders" { + t.Fatalf("expected cname echoheaders but %v returned", sslCert.CN[0]) } } @@ -101,11 +106,26 @@ func TestCACert(t *testing.T) { k := certutil.EncodePrivateKeyPEM(cert.Key) ca := certutil.EncodeCertPEM(CA.Cert) - ngxCert, err := AddOrUpdateCertAndKey(name, c, k, ca, fs) + sslCert, err := CreateSSLCert(c, k) if err != nil { - t.Fatalf("unexpected error checking SSL certificate: %v", err) + t.Fatalf("unexpected error creating SSL certificate: %v", err) + } + + err = StoreSSLCertOnDisk(fs, name, sslCert) + if err != nil { + t.Fatalf("unexpected error storing SSL certificate: %v", err) + } + + if sslCert.CAFileName != "" { + t.Fatalf("expected CA file name to be empty") + } + + err = ConfigureCACertWithCertAndKey(fs, name, ca, sslCert) + if err != nil { + t.Fatalf("unexpected error configuring CA certificate: %v", err) } - if ngxCert.CAFileName == "" { + + if sslCert.CAFileName == "" { t.Fatalf("expected a valid CA file name") } } @@ -120,7 +140,7 @@ func TestGetFakeSSLCert(t *testing.T) { } } -func TestAddCertAuth(t *testing.T) { +func TestConfigureCACert(t *testing.T) { fs, err := file.NewFakeFS() if err != nil { t.Fatalf("unexpected error creating filesystem: %v", err) @@ -132,11 +152,23 @@ func TestAddCertAuth(t *testing.T) { t.Fatalf("unexpected error creating SSL certificate: %v", err) } c := certutil.EncodeCertPEM(ca.Cert) - ic, err := AddCertAuth(cn, c, fs) + + sslCert, err := CreateCACert(c) if err != nil { t.Fatalf("unexpected error creating SSL certificate: %v", err) } - if ic.CAFileName == "" { + if sslCert.CAFileName != "" { + t.Fatalf("expected CAFileName to be empty") + } + if sslCert.Certificate == nil { + t.Fatalf("expected Certificate to be set") + } + + err = ConfigureCACert(fs, cn, c, sslCert) + if err != nil { + t.Fatalf("unexpected error creating SSL certificate: %v", err) + } + if sslCert.CAFileName == "" { t.Fatalf("expected a valid CA file name") } } @@ -155,12 +187,10 @@ func TestCreateSSLCert(t *testing.T) { t.Fatalf("unexpected error creating SSL certificate: %v", err) } - name := fmt.Sprintf("test-%v", time.Now().UnixNano()) - c := certutil.EncodeCertPEM(cert.Cert) k := certutil.EncodePrivateKeyPEM(cert.Key) - ngxCert, err := CreateSSLCert(name, c, k, []byte{}) + sslCert, err := CreateSSLCert(c, k) if err != nil { t.Fatalf("unexpected error checking SSL certificate: %v", err) } @@ -170,16 +200,16 @@ func TestCreateSSLCert(t *testing.T) { certKeyBuf.Write([]byte("\n")) certKeyBuf.Write(k) - if ngxCert.PemCertKey != certKeyBuf.String() { - t.Fatalf("expected concatenated PEM cert and key but returned %v", ngxCert.PemCertKey) + if sslCert.PemCertKey != certKeyBuf.String() { + t.Fatalf("expected concatenated PEM cert and key but returned %v", sslCert.PemCertKey) } - if len(ngxCert.CN) == 0 { - t.Fatalf("expected at least one cname but none returned") + if len(sslCert.CN) == 0 { + t.Fatalf("expected at least one CN but none returned") } - if ngxCert.CN[0] != "echoheaders" { - t.Fatalf("expected cname echoheaders but %v returned", ngxCert.CN[0]) + if sslCert.CN[0] != "echoheaders" { + t.Fatalf("expected cname echoheaders but %v returned", sslCert.CN[0]) } }