From 328e33d92535ed181985151e6d9d9b64c65bf0da Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Mon, 30 Oct 2023 15:25:39 +0100 Subject: [PATCH 1/2] improve client certs for mtls config structure Signed-off-by: David Hontecillas --- config/config.go | 23 +++++++++++++-------- config/parser.go | 30 +++++++++++++++++++--------- transport/http/server/server.go | 18 ++++------------- transport/http/server/server_test.go | 7 +++++-- 4 files changed, 45 insertions(+), 33 deletions(-) diff --git a/config/config.go b/config/config.go index c9b631b20..80c0f1d5b 100644 --- a/config/config.go +++ b/config/config.go @@ -305,14 +305,21 @@ type TLS struct { // ClientTLS defines the configuration params for an HTTP Client type ClientTLS struct { - AllowInsecureConnections bool `mapstructure:"allow_insecure_connections"` - CaCerts []string `mapstructure:"ca_certs"` - DisableSystemCaPool bool `mapstructure:"disable_system_ca_pool"` - MinVersion string `mapstructure:"min_version"` - MaxVersion string `mapstructure:"max_version"` - CurvePreferences []uint16 `mapstructure:"curve_preferences"` - CipherSuites []uint16 `mapstructure:"cipher_suites"` - ClientCerts [][]string `mapstructure:"client_certs"` + AllowInsecureConnections bool `mapstructure:"allow_insecure_connections"` + CaCerts []string `mapstructure:"ca_certs"` + DisableSystemCaPool bool `mapstructure:"disable_system_ca_pool"` + MinVersion string `mapstructure:"min_version"` + MaxVersion string `mapstructure:"max_version"` + CurvePreferences []uint16 `mapstructure:"curve_preferences"` + CipherSuites []uint16 `mapstructure:"cipher_suites"` + ClientCerts []ClientTLSCert `mapstructure:"client_certs"` +} + +// ClientTLSCert holds a certificate with its private key to be +// used for mTLS against the backend services +type ClientTLSCert struct { + Certificate string `mapstructure:"certificate"` + PrivateKey string `mapstructure:"private_key"` } // ExtraConfig is a type to store extra configurations for customized behaviours diff --git a/config/parser.go b/config/parser.go index df1dcda14..cb53b9ffd 100644 --- a/config/parser.go +++ b/config/parser.go @@ -218,7 +218,14 @@ func (p *parseableServiceConfig) normalize() ServiceConfig { MaxVersion: p.ClientTLS.MaxVersion, CurvePreferences: p.ClientTLS.CurvePreferences, CipherSuites: p.ClientTLS.CipherSuites, - ClientCerts: p.ClientTLS.ClientCerts, + ClientCerts: make([]ClientTLSCert, 0, len(p.ClientTLS.ClientCerts)), + } + for _, cc := range p.ClientTLS.ClientCerts { + cfg.ClientTLS.ClientCerts = append(cfg.ClientTLS.ClientCerts, + ClientTLSCert{ + Certificate: cc.Certificate, + PrivateKey: cc.PrivateKey, + }) } } if p.ExtraConfig != nil { @@ -252,14 +259,19 @@ type parseableTLS struct { } type parseableClientTLS struct { - AllowInsecureConnections bool `json:"allow_insecure_connections"` - CaCerts []string `json:"ca_certs"` - DisableSystemCaPool bool `json:"disable_system_ca_pool"` - MinVersion string `json:"min_version"` - MaxVersion string `json:"max_version"` - CurvePreferences []uint16 `json:"curve_preferences"` - CipherSuites []uint16 `json:"cipher_suites"` - ClientCerts [][]string `json:"client_certs"` + AllowInsecureConnections bool `json:"allow_insecure_connections"` + CaCerts []string `json:"ca_certs"` + DisableSystemCaPool bool `json:"disable_system_ca_pool"` + MinVersion string `json:"min_version"` + MaxVersion string `json:"max_version"` + CurvePreferences []uint16 `json:"curve_preferences"` + CipherSuites []uint16 `json:"cipher_suites"` + ClientCerts []parseableClientTLSCert `json:"client_certs"` +} + +type parseableClientTLSCert struct { + Certificate string `json:"certificate"` + PrivateKey string `json:"private_key"` } type parseableEndpointConfig struct { diff --git a/transport/http/server/server.go b/transport/http/server/server.go index 989ec1a4a..5dc67de6d 100644 --- a/transport/http/server/server.go +++ b/transport/http/server/server.go @@ -234,23 +234,13 @@ func loadCertPool(disableSystemCaPool bool, caCerts []string, logger logging.Log return certPool } -func loadClientCerts(certFiles [][]string, logger logging.Logger) []tls.Certificate { +func loadClientCerts(certFiles []config.ClientTLSCert, logger logging.Logger) []tls.Certificate { certs := make([]tls.Certificate, 0, len(certFiles)) - for idx, certAndKey := range certFiles { - if len(certAndKey) < 2 { - logger.Error(fmt.Sprintf("%s Missing cert and key at idx %d: %v", - loggerPrefix, idx, certAndKey)) - continue - } - if len(certAndKey) > 2 { - logger.Warning(fmt.Sprintf("%s Extra fields at idx %d: %v", - loggerPrefix, idx, certAndKey)) - } - - cert, err := tls.LoadX509KeyPair(certAndKey[0], certAndKey[1]) + for _, certAndKey := range certFiles { + cert, err := tls.LoadX509KeyPair(certAndKey.Certificate, certAndKey.PrivateKey) if err != nil { logger.Error(fmt.Sprintf("%s Cannot load client certificate %s, %s: %s", - loggerPrefix, certAndKey[0], certAndKey[1], err.Error())) + loggerPrefix, certAndKey.Certificate, certAndKey.PrivateKey, err.Error())) continue } certs = append(certs, cert) diff --git a/transport/http/server/server_test.go b/transport/http/server/server_test.go index 42485c1ac..90401889b 100644 --- a/transport/http/server/server_test.go +++ b/transport/http/server/server_test.go @@ -114,8 +114,11 @@ func TestRunServer_MTLS(t *testing.T) { ClientTLS: &config.ClientTLS{ AllowInsecureConnections: false, // we do not check the server cert CaCerts: []string{"ca.pem"}, - ClientCerts: [][]string{ - []string{"cert.pem", "key.pem"}, + ClientCerts: []config.ClientTLSCert{ + config.ClientTLSCert{ + Certificate: "cert.pem", + PrivateKey: "key.pem", + }, }, }, } From 63204b0cda9b36e0df7fac017e5659a4209a6f7c Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Mon, 30 Oct 2023 18:55:51 +0100 Subject: [PATCH 2/2] apply deepsource suggestion Signed-off-by: David Hontecillas --- config/parser.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/config/parser.go b/config/parser.go index cb53b9ffd..c93705a47 100644 --- a/config/parser.go +++ b/config/parser.go @@ -221,11 +221,7 @@ func (p *parseableServiceConfig) normalize() ServiceConfig { ClientCerts: make([]ClientTLSCert, 0, len(p.ClientTLS.ClientCerts)), } for _, cc := range p.ClientTLS.ClientCerts { - cfg.ClientTLS.ClientCerts = append(cfg.ClientTLS.ClientCerts, - ClientTLSCert{ - Certificate: cc.Certificate, - PrivateKey: cc.PrivateKey, - }) + cfg.ClientTLS.ClientCerts = append(cfg.ClientTLS.ClientCerts, ClientTLSCert(cc)) } } if p.ExtraConfig != nil {