From 7a1e8accacd3c7fdfcc2687e22a58a6f1f30c6cb Mon Sep 17 00:00:00 2001 From: Mart Aarma Date: Wed, 23 Mar 2022 16:19:07 +0200 Subject: [PATCH] fix: codereview suggestions --- consent/strategy_default.go | 2 +- driver/config/tls.go | 49 ++++++++++--- driver/config/tls_test.go | 38 ++++++---- spec/config.json | 134 +++++++++++++++++++++--------------- 4 files changed, 144 insertions(+), 79 deletions(-) diff --git a/consent/strategy_default.go b/consent/strategy_default.go index 997423dd2b0..a8f865bc058 100644 --- a/consent/strategy_default.go +++ b/consent/strategy_default.go @@ -74,7 +74,7 @@ type DefaultStrategy struct { } func NewStrategy(r InternalRegistry, c *config.Provider) *DefaultStrategy { - tlsClientConfig, err := c.TLSClientConfig() + tlsClientConfig, err := c.TLSClientConfigWithDefaultFallback(config.KeyPrefixClientBackChannelLogout) if err != nil { r.Logger().WithError(err).Fatalf("Unable to setup backchannel logout request client TLS configuration.") } diff --git a/driver/config/tls.go b/driver/config/tls.go index cb96b602ce4..647bff1f56b 100644 --- a/driver/config/tls.go +++ b/driver/config/tls.go @@ -2,6 +2,7 @@ package config import ( "crypto/tls" + "fmt" "strings" "github.com/hashicorp/go-secure-stdlib/tlsutil" @@ -24,9 +25,33 @@ const ( KeyTLSCertPath = "serve." + KeySuffixTLSCertPath KeyTLSKeyPath = "serve." + KeySuffixTLSKeyPath - KeyClientTLSCipherSuites = "client.tls.cipher_suites" - KeyClientTLSMinVer = "client.tls.min_version" - KeyClientTLSMaxVer = "client.tls.max_version" + KeySuffixClientTLSCipherSuites = "tls.cipher_suites" + KeySuffixClientTLSMinVer = "tls.min_version" + KeySuffixClientTLSMaxVer = "tls.max_version" +) + +type ClientInterface interface { + Key(suffix string) string +} + +func (iface *clientPrefix) Key(suffix string) string { + return fmt.Sprintf("%s.%s", iface.prefix, suffix) +} + +type clientPrefix struct { + prefix string +} + +var ( + KeyPrefixClientDefault ClientInterface = &clientPrefix{ + prefix: "client.default", + } + KeyPrefixClientBackChannelLogout ClientInterface = &clientPrefix{ + prefix: "client.back_channel_logout", + } + KeyPrefixClientRefreshTokenHook ClientInterface = &clientPrefix{ + prefix: "client.refresh_token_hook", + } ) type TLSConfig interface { @@ -55,11 +80,15 @@ func (p *Provider) TLS(iface ServeInterface) TLSConfig { } } -func (p *Provider) TLSClientConfig() (*tls.Config, error) { +func (p *Provider) TLSClientConfigDefault() (*tls.Config, error) { + return p.TLSClientConfigWithDefaultFallback(KeyPrefixClientDefault) +} + +func (p *Provider) TLSClientConfigWithDefaultFallback(iface ClientInterface) (*tls.Config, error) { tlsClientConfig := new(tls.Config) - if p.p.Exists(KeyClientTLSCipherSuites) { - keyCipherSuites := p.p.Strings(KeyClientTLSCipherSuites) + if p.p.Exists(KeyPrefixClientDefault.Key(KeySuffixClientTLSCipherSuites)) || p.p.Exists(iface.Key(KeySuffixClientTLSCipherSuites)) { + keyCipherSuites := p.p.StringsF(iface.Key(KeySuffixClientTLSCipherSuites), p.p.Strings(KeyPrefixClientDefault.Key(KeySuffixClientTLSCipherSuites))) cipherSuites, err := tlsutil.ParseCiphers(strings.Join(keyCipherSuites[:], ",")) if err != nil { return nil, errors.WithMessage(err, "Unable to setup client TLS configuration") @@ -67,8 +96,8 @@ func (p *Provider) TLSClientConfig() (*tls.Config, error) { tlsClientConfig.CipherSuites = cipherSuites } - if p.p.Exists(KeyClientTLSMinVer) { - keyMinVer := p.p.String(KeyClientTLSMinVer) + if p.p.Exists(KeyPrefixClientDefault.Key(KeySuffixClientTLSMinVer)) || p.p.Exists(iface.Key(KeySuffixClientTLSMinVer)) { + keyMinVer := p.p.StringF(iface.Key(KeySuffixClientTLSMinVer), p.p.String(KeyPrefixClientDefault.Key(KeySuffixClientTLSMinVer))) if tlsMinVer, found := tlsutil.TLSLookup[keyMinVer]; !found { return nil, errors.Errorf("Unable to setup client TLS configuration. Invalid minimum TLS version: %s", keyMinVer) } else { @@ -76,8 +105,8 @@ func (p *Provider) TLSClientConfig() (*tls.Config, error) { } } - if p.p.Exists(KeyClientTLSMaxVer) { - keyMaxVer := p.p.String(KeyClientTLSMaxVer) + if p.p.Exists(KeyPrefixClientDefault.Key(KeySuffixClientTLSMaxVer)) || p.p.Exists(iface.Key(KeySuffixClientTLSMaxVer)) { + keyMaxVer := p.p.StringF(iface.Key(KeySuffixClientTLSMaxVer), p.p.String(KeyPrefixClientDefault.Key(KeySuffixClientTLSMaxVer))) if tlsMaxVer, found := tlsutil.TLSLookup[keyMaxVer]; !found { return nil, errors.Errorf("Unable to setup client TLS configuration. Invalid maximum TLS version: %s", keyMaxVer) } else { diff --git a/driver/config/tls_test.go b/driver/config/tls_test.go index 406fa0c4ed6..e3ccf178d87 100644 --- a/driver/config/tls_test.go +++ b/driver/config/tls_test.go @@ -13,9 +13,9 @@ import ( func TestTLSClientConfig_CipherSuite(t *testing.T) { l := logrusx.New("", "") - c := MustNew(context.TODO(), l, configx.WithValue("client.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"})) + c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"})) - tlsClientConfig, err := c.TLSClientConfig() + tlsClientConfig, err := c.TLSClientConfigDefault() assert.NoError(t, err) cipherSuites := tlsClientConfig.CipherSuites @@ -26,18 +26,18 @@ func TestTLSClientConfig_CipherSuite(t *testing.T) { func TestTLSClientConfig_InvalidCipherSuite(t *testing.T) { l := logrusx.New("", "") - c := MustNew(context.TODO(), l, configx.WithValue("client.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_INVALID_CIPHER_SUITE"})) + c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_INVALID_CIPHER_SUITE"})) - _, err := c.TLSClientConfig() + _, err := c.TLSClientConfigDefault() assert.EqualError(t, err, "Unable to setup client TLS configuration: unsupported cipher \"TLS_INVALID_CIPHER_SUITE\"") } func TestTLSClientConfig_MinVersion(t *testing.T) { l := logrusx.New("", "") - c := MustNew(context.TODO(), l, configx.WithValue("client.tls.min_version", "tls13")) + c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.min_version", "tls13")) - tlsClientConfig, err := c.TLSClientConfig() + tlsClientConfig, err := c.TLSClientConfigDefault() assert.NoError(t, err) assert.Equal(t, uint16(tls.VersionTLS13), tlsClientConfig.MinVersion) @@ -45,18 +45,18 @@ func TestTLSClientConfig_MinVersion(t *testing.T) { func TestTLSClientConfig_InvalidMinVersion(t *testing.T) { l := logrusx.New("", "") - c := MustNew(context.TODO(), l, configx.WithValue("client.tls.min_version", "tlsx")) + c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.min_version", "tlsx")) - _, err := c.TLSClientConfig() + _, err := c.TLSClientConfigDefault() assert.EqualError(t, err, "Unable to setup client TLS configuration. Invalid minimum TLS version: tlsx") } func TestTLSClientConfig_MaxVersion(t *testing.T) { l := logrusx.New("", "") - c := MustNew(context.TODO(), l, configx.WithValue("client.tls.max_version", "tls10")) + c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.max_version", "tls10")) - tlsClientConfig, err := c.TLSClientConfig() + tlsClientConfig, err := c.TLSClientConfigDefault() assert.NoError(t, err) assert.Equal(t, uint16(tls.VersionTLS10), tlsClientConfig.MaxVersion) @@ -64,9 +64,23 @@ func TestTLSClientConfig_MaxVersion(t *testing.T) { func TestTLSClientConfig_InvalidMaxTlsVersion(t *testing.T) { l := logrusx.New("", "") - c := MustNew(context.TODO(), l, configx.WithValue("client.tls.max_version", "tlsx")) + c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.max_version", "tlsx")) - _, err := c.TLSClientConfig() + _, err := c.TLSClientConfigDefault() assert.EqualError(t, err, "Unable to setup client TLS configuration. Invalid maximum TLS version: tlsx") } + +func TestTLSClientConfig_WithDefaultFallback(t *testing.T) { + l := logrusx.New("", "") + c := MustNew(context.TODO(), l) + c.MustSet("client.default.tls.min_version", "tls11") + c.MustSet("client.default.tls.max_version", "tls12") + c.MustSet("client.back_channel_logout.tls.max_version", "tls13") + + tlsClientConfig, err := c.TLSClientConfigWithDefaultFallback(KeyPrefixClientBackChannelLogout) + + assert.NoError(t, err) + assert.Equal(t, uint16(tls.VersionTLS11), tlsClientConfig.MinVersion) + assert.Equal(t, uint16(tls.VersionTLS13), tlsClientConfig.MaxVersion) +} diff --git a/spec/config.json b/spec/config.json index 7b50a14b6ed..7ea4d5d9b67 100644 --- a/spec/config.json +++ b/spec/config.json @@ -217,7 +217,7 @@ "1h" ] }, - "tls_config": { + "server_tls_config": { "type": "object", "description": "Configures HTTPS (HTTP over TLS). If configured, the server automatically supports HTTP/2.", "properties": { @@ -246,6 +246,67 @@ } } }, + "client_tls_config": { + "type": "object", + "additionalProperties": false, + "description": "Configures http client TLS settings.", + "properties": { + "min_version": { + "type": "string", + "description": "Minimum supported TLS version.", + "examples": [ + "tls10", + "tls11", + "tls12", + "tls13" + ] + }, + "max_version": { + "type": "string", + "description": "Maximum supported TLS version.", + "examples": [ + "tls10", + "tls11", + "tls12", + "tls13" + ] + }, + "cipher_suites": { + "type": "array", + "description": "A list of supported cipher suites.", + "items": { + "type": "string" + }, + "examples": [ + "TLS_RSA_WITH_RC4_128_SHA", + "TLS_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_RSA_WITH_AES_128_CBC_SHA", + "TLS_RSA_WITH_AES_256_CBC_SHA", + "TLS_RSA_WITH_AES_128_CBC_SHA256", + "TLS_RSA_WITH_AES_128_GCM_SHA256", + "TLS_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_RC4_128_SHA", + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", + "TLS_AES_128_GCM_SHA256", + "TLS_AES_256_GCM_SHA384", + "TLS_CHACHA20_POLY1305_SHA256" + ] + } + } + }, "grantJwt": { "type": "object", "additionalProperties": false, @@ -361,7 +422,7 @@ } }, "tls": { - "$ref": "#/definitions/tls_config" + "$ref": "#/definitions/server_tls_config" } } }, @@ -407,7 +468,7 @@ "tls": { "allOf": [ { - "$ref": "#/definitions/tls_config" + "$ref": "#/definitions/server_tls_config" }, { "type": "object", @@ -424,7 +485,7 @@ } }, "tls": { - "$ref": "#/definitions/tls_config" + "$ref": "#/definitions/server_tls_config" }, "cookies": { "type": "object", @@ -452,58 +513,19 @@ } } }, - "client.tls": { - "type": "object", - "additionalProperties": false, - "description": "Configures http client TLS settings.", - "properties": { - "min_version": { - "type": "string", - "description": "Minimum supported TLS version.", - "examples": [ - "tls10","tls11","tls12","tls13" - ] - }, - "max_version": { - "type": "string", - "description": "Maximum supported TLS version.", - "examples": [ - "tls10","tls11","tls12","tls13" - ] - }, - "cipher_suites": { - "type": "array", - "description": "A list of supported cipher suites.", - "items": { - "type": "string" - }, - "examples": [ - "TLS_RSA_WITH_RC4_128_SHA", - "TLS_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_RSA_WITH_AES_128_CBC_SHA", - "TLS_RSA_WITH_AES_256_CBC_SHA", - "TLS_RSA_WITH_AES_128_CBC_SHA256", - "TLS_RSA_WITH_AES_128_GCM_SHA256", - "TLS_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_RC4_128_SHA", - "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", - "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", - "TLS_AES_128_GCM_SHA256", - "TLS_AES_256_GCM_SHA384", - "TLS_CHACHA20_POLY1305_SHA256" - ] + "client": { + "default": { + "tls": { + "$ref": "#/definitions/client_tls_config" + } + }, + "back_channel_logout": { + "tls": { + "allOf": [ + { + "$ref": "#/definitions/client_tls_config" + } + ] } } },