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.

Sign up for GitHub

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

Jump to bottom

core/config: remove support for base64 encoded certificates #4725

Merged
merged 1 commit into from Nov 8, 2023
Merged

core/config: remove support for base64 encoded certificates #4725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 35 additions & 17 deletions config/options.go
View file
Open in desktop
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
}

Expand Down
12 changes: 5 additions & 7 deletions config/options_test.go
View file
Open in desktop
Expand Up @@ -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},
}
Expand Down Expand Up @@ -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)
Expand All @@ -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")
})
}

Expand Down