From 9598c99943011bdae4d5c3c3f805647452d90e71 Mon Sep 17 00:00:00 2001 From: Caleb Doxsey Date: Wed, 8 Nov 2023 13:08:24 -0700 Subject: [PATCH] core/config: remove support for base64 encoded certificates (#4718) * core/config: update file watcher source to handle base64 encoded certificates * fix data race * core/config: only allow files in certificates * remove test * re-add test --- config/options.go | 52 ++++++++++++++++++++++++++++-------------- config/options_test.go | 12 ++++------ 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/config/options.go b/config/options.go index 8221f14b607..d7fa77c23ac 100644 --- a/config/options.go +++ b/config/options.go @@ -96,6 +96,7 @@ type Options struct { // If this setting is not specified, the value defaults to V4_PREFERRED. DNSLookupFamily string `mapstructure:"dns_lookup_family" yaml:"dns_lookup_family,omitempty"` + CertificateData []*config.Settings_Certificate CertificateFiles []certificateFilePair `mapstructure:"certificates" yaml:"certificates,omitempty"` // Cert and Key is the x509 certificate used to create the HTTPS server. @@ -666,13 +667,18 @@ func (o *Options) Validate() error { hasCert = true } - for _, c := range o.CertificateFiles { - _, err := cryptutil.CertificateFromBase64(c.CertFile, c.KeyFile) + for _, c := range o.CertificateData { + _, err := tls.X509KeyPair(c.GetCertBytes(), c.GetKeyBytes()) if err != nil { - _, err = cryptutil.CertificateFromFile(c.CertFile, c.KeyFile) + return fmt.Errorf("config: bad cert entry, cert is invalid: %w", err) } + hasCert = true + } + + for _, c := range o.CertificateFiles { + _, err := cryptutil.CertificateFromFile(c.CertFile, c.KeyFile) if err != nil { - return fmt.Errorf("config: bad cert entry, base64 or file reference invalid. %w", err) + return fmt.Errorf("config: bad cert entry, file reference invalid. %w", err) } hasCert = true } @@ -1020,14 +1026,18 @@ func (o *Options) GetCertificates() ([]tls.Certificate, error) { certs = append(certs, *cert) } for _, c := range o.CertificateFiles { - cert, err := cryptutil.CertificateFromBase64(c.CertFile, c.KeyFile) + cert, err := cryptutil.CertificateFromFile(c.CertFile, c.KeyFile) if err != nil { - cert, err = cryptutil.CertificateFromFile(c.CertFile, c.KeyFile) + return nil, fmt.Errorf("config: invalid certificate entry: %w", err) } + certs = append(certs, *cert) + } + for _, c := range o.CertificateData { + cert, err := tls.X509KeyPair(c.GetCertBytes(), c.GetKeyBytes()) if err != nil { return nil, fmt.Errorf("config: invalid certificate entry: %w", err) } - certs = append(certs, *cert) + certs = append(certs, cert) } if o.CertFile != "" && o.KeyFile != "" { cert, err := cryptutil.CertificateFromFile(o.CertFile, o.KeyFile) @@ -1041,7 +1051,12 @@ func (o *Options) GetCertificates() ([]tls.Certificate, error) { // HasCertificates returns true if options has any certificates. func (o *Options) HasCertificates() bool { - return o.Cert != "" || o.Key != "" || len(o.CertificateFiles) > 0 || o.CertFile != "" || o.KeyFile != "" + return o.Cert != "" || + o.Key != "" || + len(o.CertificateFiles) > 0 || + o.CertFile != "" || + o.KeyFile != "" || + len(o.CertificateData) > 0 } // GetX509Certificates gets all the x509 certificates from the options. Invalid certificates are ignored. @@ -1063,11 +1078,17 @@ func (o *Options) GetX509Certificates() []*x509.Certificate { } } - for _, c := range o.CertificateFiles { - cert, err := cryptutil.ParsePEMCertificateFromBase64(c.CertFile) + for _, c := range o.CertificateData { + cert, err := cryptutil.ParsePEMCertificate(c.GetCertBytes()) if err != nil { - cert, err = cryptutil.ParsePEMCertificateFromFile(c.CertFile) + log.Error(context.Background()).Err(err).Msg("invalid certificate") + } else { + certs = append(certs, cert) } + } + + for _, c := range o.CertificateFiles { + cert, err := cryptutil.ParsePEMCertificateFromFile(c.CertFile) if err != nil { log.Error(context.Background()).Err(err).Msg("invalid certificate_file") } else { @@ -1373,21 +1394,18 @@ func (o *Options) Checksum() uint64 { func (o *Options) applyExternalCerts(ctx context.Context, certsIndex *cryptutil.CertificatesIndex, certs []*config.Settings_Certificate) { for _, c := range certs { - cfp := certificateFilePair{} - cfp.CertFile = base64.StdEncoding.EncodeToString(c.CertBytes) - cfp.KeyFile = base64.StdEncoding.EncodeToString(c.KeyBytes) - - cert, err := cryptutil.ParsePEMCertificateFromBase64(cfp.CertFile) + cert, err := cryptutil.ParsePEMCertificate(c.GetCertBytes()) if err != nil { log.Error(ctx).Err(err).Msg("parsing cert from databroker: skipped") continue } + if overlaps, name := certsIndex.OverlapsWithExistingCertificate(cert); overlaps { log.Error(ctx).Err(err).Str("domain", name).Msg("overlaps with local certs: skipped") continue } - o.CertificateFiles = append(o.CertificateFiles, cfp) + o.CertificateData = append(o.CertificateData, c) } } diff --git a/config/options_test.go b/config/options_test.go index bcd7021f3ba..b6a31dbc9cf 100644 --- a/config/options_test.go +++ b/config/options_test.go @@ -628,17 +628,12 @@ func TestCertificatesArrayParsing(t *testing.T) { testCertFileRef := "./testdata/example-cert.pem" testKeyFileRef := "./testdata/example-key.pem" - testCertFile, _ := os.ReadFile(testCertFileRef) - testKeyFile, _ := os.ReadFile(testKeyFileRef) - testCertAsBase64 := base64.StdEncoding.EncodeToString(testCertFile) - testKeyAsBase64 := base64.StdEncoding.EncodeToString(testKeyFile) tests := []struct { name string certificateFiles []certificateFilePair wantErr bool }{ - {"Handles base64 string as params", []certificateFilePair{{KeyFile: testKeyAsBase64, CertFile: testCertAsBase64}}, false}, {"Handles file reference as params", []certificateFilePair{{KeyFile: testKeyFileRef, CertFile: testCertFileRef}}, false}, {"Returns an error otherwise", []certificateFilePair{{KeyFile: "abc", CertFile: "abc"}}, true}, } @@ -936,8 +931,11 @@ func TestOptions_ApplySettings(t *testing.T) { options := NewDefaultOptions() cert1, err := cryptutil.GenerateCertificate(nil, "example.com") require.NoError(t, err) + cert1path := filepath.Join(t.TempDir(), "example.com.pem") + err = os.WriteFile(cert1path, cert1.Certificate[0], 0o600) + require.NoError(t, err) options.CertificateFiles = append(options.CertificateFiles, certificateFilePair{ - CertFile: base64.StdEncoding.EncodeToString(encodeCert(cert1)), + CertFile: cert1path, }) cert2, err := cryptutil.GenerateCertificate(nil, "example.com") require.NoError(t, err) @@ -955,7 +953,7 @@ func TestOptions_ApplySettings(t *testing.T) { }, } options.ApplySettings(ctx, certsIndex, settings) - assert.Len(t, options.CertificateFiles, 2, "should prevent adding duplicate certificates") + assert.Len(t, options.CertificateData, 1, "should prevent adding duplicate certificates") }) }