From ba99fbe39050a11f9e62d398321c7a32f7842324 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Doumenjou Date: Tue, 16 Oct 2018 11:00:04 +0200 Subject: [PATCH] Fix certificate insertion loop to keep valid certificate and ignore the bad one --- integration/fixtures/https/dynamic_https.toml | 15 +++++++++++++++ server/server_configuration.go | 11 ++++------- tls/certificate.go | 11 +++++++++++ tls/tls.go | 14 +++++--------- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/integration/fixtures/https/dynamic_https.toml b/integration/fixtures/https/dynamic_https.toml index a97bee293..264552b02 100644 --- a/integration/fixtures/https/dynamic_https.toml +++ b/integration/fixtures/https/dynamic_https.toml @@ -18,6 +18,21 @@ [frontends.frontend2.routes.test_2] rule = "Host:snitest.org" +[[tls]] +entryPoints = ["https"] + # bad certificates to validate the loop on the certificate appending + [tls.certificate] + # bad content + certFile = """-----BEGIN CERTIFICATE----- +MIIC/zCCAeegAwIBAgIJALAYHG/vGqWEMA0GCSqGSIb3DQEBBQUAMBYxFDASBgNV +-----END CERTIFICATE-----""" + # bad content + keyFile = """-----BEGIN RSA PRIVATE KEY----- +wihZ13e3i5UQEYuoRcH1RUd1wyYoBSKuQnsT2WwVZ1wlXSYaELAbQgaI9NtfBA0G +eRG3DaVpez4DQVupZDHMgxJUYqqKynUj6GD1YiaxGROj3TYCu6e7OxyhalhCllSu +w/X5M802XqzLjeec5zHoZDfknnAkgR9MsxZYmZPFaDyL6GOKUB8= +-----END RSA PRIVATE KEY-----""" + [[tls]] entryPoints = ["https"] [tls.certificate] diff --git a/server/server_configuration.go b/server/server_configuration.go index 164048bcd..3d224e1c4 100644 --- a/server/server_configuration.go +++ b/server/server_configuration.go @@ -118,8 +118,7 @@ func (s *Server) loadConfig(configurations types.Configurations, globalConfigura // Get new certificates list sorted per entrypoints // Update certificates - entryPointsCertificates, err := s.loadHTTPSConfiguration(configurations, globalConfiguration.DefaultEntryPoints) - // FIXME error management + entryPointsCertificates := s.loadHTTPSConfiguration(configurations, globalConfiguration.DefaultEntryPoints) // Sort routes and update certificates for serverEntryPointName, serverEntryPoint := range serverEntryPoints { @@ -558,17 +557,15 @@ func (s *Server) postLoadConfiguration() { } // loadHTTPSConfiguration add/delete HTTPS certificate managed dynamically -func (s *Server) loadHTTPSConfiguration(configurations types.Configurations, defaultEntryPoints configuration.DefaultEntryPoints) (map[string]map[string]*tls.Certificate, error) { +func (s *Server) loadHTTPSConfiguration(configurations types.Configurations, defaultEntryPoints configuration.DefaultEntryPoints) map[string]map[string]*tls.Certificate { newEPCertificates := make(map[string]map[string]*tls.Certificate) // Get all certificates for _, config := range configurations { if config.TLS != nil && len(config.TLS) > 0 { - if err := traefiktls.SortTLSPerEntryPoints(config.TLS, newEPCertificates, defaultEntryPoints); err != nil { - return nil, err - } + traefiktls.SortTLSPerEntryPoints(config.TLS, newEPCertificates, defaultEntryPoints) } } - return newEPCertificates, nil + return newEPCertificates } func (s *Server) buildServerEntryPoints() map[string]*serverEntryPoint { diff --git a/tls/certificate.go b/tls/certificate.go index f680c78f5..47b2a10eb 100644 --- a/tls/certificate.go +++ b/tls/certificate.go @@ -196,6 +196,17 @@ func (c *Certificate) AppendCertificates(certs map[string]map[string]*tls.Certif return err } +func (c *Certificate) getTruncatedCertificateName() string { + certName := c.CertFile.String() + + // Truncate certificate information only if it's a well formed certificate content with more than 50 characters + if !c.CertFile.IsPath() && strings.HasPrefix(certName, certificateHeader) && len(certName) > len(certificateHeader)+50 { + certName = strings.TrimPrefix(c.CertFile.String(), certificateHeader)[:50] + } + + return certName +} + // String is the method to format the flag's value, part of the flag.Value interface. // The String method's output will be used in diagnostics. func (c *Certificates) String() string { diff --git a/tls/tls.go b/tls/tls.go index 32a7583de..ea56dc8ab 100644 --- a/tls/tls.go +++ b/tls/tls.go @@ -80,27 +80,23 @@ func (r *FilesOrContents) Type() string { } // SortTLSPerEntryPoints converts TLS configuration sorted by Certificates into TLS configuration sorted by EntryPoints -func SortTLSPerEntryPoints(configurations []*Configuration, epConfiguration map[string]map[string]*tls.Certificate, defaultEntryPoints []string) error { +func SortTLSPerEntryPoints(configurations []*Configuration, epConfiguration map[string]map[string]*tls.Certificate, defaultEntryPoints []string) { if epConfiguration == nil { epConfiguration = make(map[string]map[string]*tls.Certificate) } for _, conf := range configurations { if conf.EntryPoints == nil || len(conf.EntryPoints) == 0 { if log.GetLevel() >= logrus.DebugLevel { - certName := conf.Certificate.CertFile.String() - // Truncate certificate information only if it's a well formed certificate content with more than 50 characters - if !conf.Certificate.CertFile.IsPath() && strings.HasPrefix(conf.Certificate.CertFile.String(), certificateHeader) && len(conf.Certificate.CertFile.String()) > len(certificateHeader)+50 { - certName = strings.TrimPrefix(conf.Certificate.CertFile.String(), certificateHeader)[:50] - } - log.Debugf("No entryPoint is defined to add the certificate %s, it will be added to the default entryPoints: %s", certName, strings.Join(defaultEntryPoints, ", ")) + log.Debugf("No entryPoint is defined to add the certificate %s, it will be added to the default entryPoints: %s", + conf.Certificate.getTruncatedCertificateName(), + strings.Join(defaultEntryPoints, ", ")) } conf.EntryPoints = append(conf.EntryPoints, defaultEntryPoints...) } for _, ep := range conf.EntryPoints { if err := conf.Certificate.AppendCertificates(epConfiguration, ep); err != nil { - return err + log.Errorf("Unable to append certificate %s to entrypoint %s: %v", conf.Certificate.getTruncatedCertificateName(), ep, err) } } } - return nil }