Skip to content

Commit

Permalink
fix: codereview suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@aarmam
aarmam committed Apr 19, 2022
1 parent 74396a8 commit f3182f8
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 79 deletions.
2 changes: 1 addition & 1 deletion consent/strategy_default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
Expand Down
49 changes: 39 additions & 10 deletions driver/config/tls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"crypto/tls"
"fmt"
"strings"

"github.com/hashicorp/go-secure-stdlib/tlsutil"
Expand All @@ -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 {
Expand Down Expand Up @@ -55,29 +80,33 @@ 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")
}
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 {
tlsClientConfig.MinVersion = tlsMinVer
}
}

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 {
Expand Down
38 changes: 26 additions & 12 deletions driver/config/tls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -26,47 +26,61 @@ 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)
}

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)
}

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)
}
134 changes: 78 additions & 56 deletions spec/config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -361,7 +422,7 @@
}
},
"tls": {
"$ref": "#/definitions/tls_config"
"$ref": "#/definitions/server_tls_config"
}
}
},
Expand Down Expand Up @@ -407,7 +468,7 @@
"tls": {
"allOf": [
{
"$ref": "#/definitions/tls_config"
"$ref": "#/definitions/server_tls_config"
},
{
"type": "object",
Expand All @@ -424,7 +485,7 @@
}
},
"tls": {
"$ref": "#/definitions/tls_config"
"$ref": "#/definitions/server_tls_config"
},
"cookies": {
"type": "object",
Expand Down Expand Up @@ -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"
}
]
}
}
},
Expand Down

0 comments on commit f3182f8

Please sign in to comment.