Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/config/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ type TLSConfig struct {
// CipherSuites is a list of TLS cipher suite IDs.
CipherSuites []uint16

// MinVersion is the minimum TLS version (e.g. tls.VersionTLS12).
// Zero means no minimum version is configured.
MinVersion uint16

// OpenShift holds OpenShift-specific TLS configuration.
// It is nil when not running on OpenShift.
OpenShift *OpenShiftTLS
Expand Down Expand Up @@ -78,6 +82,7 @@ func NewTLSConfigForOpenShift(ctx context.Context, log logr.Logger, cl client.Cl
goTLSConfig := &tls.Config{MinVersion: tls.VersionTLS12}
tlsConfigFunc(goTLSConfig)
tlsConfig.CipherSuites = goTLSConfig.CipherSuites
tlsConfig.MinVersion = goTLSConfig.MinVersion
}

return tlsConfig, nil
Expand Down
51 changes: 50 additions & 1 deletion pkg/istiovalues/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@ import (
"crypto/tls"
"strings"

"github.com/Masterminds/semver/v3"
v1 "github.com/istio-ecosystem/sail-operator/api/v1"
"github.com/istio-ecosystem/sail-operator/pkg/config"

"istio.io/istio/pkg/log"
)

var istio1_29 = semver.MustParse("1.29.0")

// ApplyTLSConfig applies TLS configuration to the Istio values.
// If TLS settings are already set, they are not overridden.
func ApplyTLSConfig(tlsConfig *config.TLSConfig, values *v1.Values) {
func ApplyTLSConfig(tlsConfig *config.TLSConfig, istioVersion string, values *v1.Values) {
if tlsConfig == nil || values == nil || len(tlsConfig.CipherSuites) == 0 {
return
}
Expand All @@ -45,17 +50,61 @@ func ApplyTLSConfig(tlsConfig *config.TLSConfig, values *v1.Values) {
values.MeshConfig.TlsDefaults.CipherSuites = cipherNames
}

if values.MeshConfig.TlsDefaults.MinProtocolVersion == "" {
values.MeshConfig.TlsDefaults.MinProtocolVersion = tlsProtocolVersion(tlsConfig.MinVersion)
}

if values.MeshConfig.MeshMTLS == nil {
values.MeshConfig.MeshMTLS = &v1.MeshConfigTLSConfig{}
}
if len(values.MeshConfig.MeshMTLS.CipherSuites) == 0 {
values.MeshConfig.MeshMTLS.CipherSuites = cipherNames
}

if values.MeshConfig.MeshMTLS.MinProtocolVersion == "" {
values.MeshConfig.MeshMTLS.MinProtocolVersion = tlsProtocolVersion(tlsConfig.MinVersion)
}

if values.Pilot == nil {
values.Pilot = &v1.PilotConfig{}
}
addExtraContainerArg(values.Pilot, "--tls-cipher-suites", strings.Join(cipherNames, ","))

if minVersionName := tlsVersionName(tlsConfig.MinVersion); minVersionName != "" {
v, err := semver.NewVersion(istioVersion)
if err != nil {
log.Warnf("failed to parse Istio version %q: %v", istioVersion, err)
}

// This flag is only supported on Istio 1.29+. TODO: Remove this check when we drop support for Istio 1.28
if v.GreaterThanEqual(istio1_29) {
addExtraContainerArg(values.Pilot, "--tls-min-version", minVersionName)
} else {
log.Infof("Istio version %q is less than 1.29 and --tls-min-version flag for istiod is only supported on Istio 1.29+. Skipping sync of flag.", istioVersion)
}
}
}

func tlsProtocolVersion(v uint16) v1.MeshConfigTLSConfigTLSProtocol {
switch v {
case tls.VersionTLS12:
return v1.MeshConfigTLSConfigTLSProtocolTlsv12
case tls.VersionTLS13:
return v1.MeshConfigTLSConfigTLSProtocolTlsv13
default:
return ""
}
}

func tlsVersionName(v uint16) string {
switch v {
case tls.VersionTLS12:
return "1.2"
case tls.VersionTLS13:
return "1.3"
default:
return ""
}
}

// addExtraContainerArg adds an argument to ExtraContainerArgs if not already present.
Expand Down
140 changes: 124 additions & 16 deletions pkg/istiovalues/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,42 @@ import (

func TestApplyTLSConfig(t *testing.T) {
tests := []struct {
name string
tlsConfig *config.TLSConfig
inputValues *v1.Values
wantValues *v1.Values
name string
tlsConfig *config.TLSConfig
istioVersion string
inputValues *v1.Values
wantValues *v1.Values
}{
{
name: "nil TLS config does not change values",
tlsConfig: nil,
inputValues: &v1.Values{},
wantValues: &v1.Values{},
name: "nil TLS config does not change values",
tlsConfig: nil,
istioVersion: "1.29.0",
inputValues: &v1.Values{},
wantValues: &v1.Values{},
},
{
name: "nil values is safe",
tlsConfig: &config.TLSConfig{CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}},
inputValues: nil,
wantValues: nil,
name: "nil values is safe",
tlsConfig: &config.TLSConfig{CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}},
istioVersion: "1.29.0",
inputValues: nil,
wantValues: nil,
},
{
name: "empty cipher suites does not change values",
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{},
},
inputValues: &v1.Values{},
wantValues: &v1.Values{},
istioVersion: "1.29.0",
inputValues: &v1.Values{},
wantValues: &v1.Values{},
},
{
name: "applies multiple cipher suites",
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384},
},
inputValues: &v1.Values{},
istioVersion: "1.28.0",
inputValues: &v1.Values{},
wantValues: &v1.Values{
MeshConfig: &v1.MeshConfig{
MeshMTLS: &v1.MeshConfigTLSConfig{
Expand All @@ -75,6 +80,7 @@ func TestApplyTLSConfig(t *testing.T) {
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
},
istioVersion: "1.28.0",
inputValues: &v1.Values{
MeshConfig: &v1.MeshConfig{
MeshMTLS: &v1.MeshConfigTLSConfig{
Expand Down Expand Up @@ -107,6 +113,7 @@ func TestApplyTLSConfig(t *testing.T) {
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
},
istioVersion: "1.28.0",
inputValues: &v1.Values{
Pilot: &v1.PilotConfig{
ExtraContainerArgs: []string{"--some-arg=value"},
Expand All @@ -131,6 +138,7 @@ func TestApplyTLSConfig(t *testing.T) {
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
},
istioVersion: "1.28.0",
inputValues: &v1.Values{
Pilot: &v1.PilotConfig{
ExtraContainerArgs: []string{"--tls-cipher-suites-foo=bar"},
Expand All @@ -150,11 +158,111 @@ func TestApplyTLSConfig(t *testing.T) {
},
},
},
{
name: "adds tls-min-version for istio 1.29+",
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
MinVersion: tls.VersionTLS12,
},
istioVersion: "1.29.0",
inputValues: &v1.Values{},
wantValues: &v1.Values{
MeshConfig: &v1.MeshConfig{
MeshMTLS: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
MinProtocolVersion: v1.MeshConfigTLSConfigTLSProtocolTlsv12,
},
TlsDefaults: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
MinProtocolVersion: v1.MeshConfigTLSConfigTLSProtocolTlsv12,
},
},
Pilot: &v1.PilotConfig{
ExtraContainerArgs: []string{
"--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
"--tls-min-version=1.2",
},
},
},
},
{
name: "adds tls-min-version TLS 1.3 for istio 1.29+",
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
MinVersion: tls.VersionTLS13,
},
istioVersion: "1.30.0",
inputValues: &v1.Values{},
wantValues: &v1.Values{
MeshConfig: &v1.MeshConfig{
MeshMTLS: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
MinProtocolVersion: v1.MeshConfigTLSConfigTLSProtocolTlsv13,
},
TlsDefaults: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
MinProtocolVersion: v1.MeshConfigTLSConfigTLSProtocolTlsv13,
},
},
Pilot: &v1.PilotConfig{
ExtraContainerArgs: []string{
"--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
"--tls-min-version=1.3",
},
},
},
},
{
name: "does not add tls-min-version for istio < 1.29",
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
MinVersion: tls.VersionTLS12,
},
istioVersion: "1.28.3",
inputValues: &v1.Values{},
wantValues: &v1.Values{
MeshConfig: &v1.MeshConfig{
MeshMTLS: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
MinProtocolVersion: v1.MeshConfigTLSConfigTLSProtocolTlsv12,
},
TlsDefaults: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
MinProtocolVersion: v1.MeshConfigTLSConfigTLSProtocolTlsv12,
},
},
Pilot: &v1.PilotConfig{
ExtraContainerArgs: []string{"--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
},
},
},
{
name: "does not add tls-min-version when MinVersion is zero",
tlsConfig: &config.TLSConfig{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
MinVersion: 0,
},
istioVersion: "1.29.0",
inputValues: &v1.Values{},
wantValues: &v1.Values{
MeshConfig: &v1.MeshConfig{
MeshMTLS: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
},
TlsDefaults: &v1.MeshConfigTLSConfig{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
},
},
Pilot: &v1.PilotConfig{
ExtraContainerArgs: []string{"--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ApplyTLSConfig(tt.tlsConfig, tt.inputValues)
ApplyTLSConfig(tt.tlsConfig, tt.istioVersion, tt.inputValues)
if diff := cmp.Diff(tt.wantValues, tt.inputValues); diff != "" {
t.Errorf("ApplyTLSConfig() mismatch (-want +got):\n%s", diff)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/revision/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func ComputeValues(
}

// apply OpenShift TLS config from APIServer before FIPS values
istiovalues.ApplyTLSConfig(tlsConfig, values)
istiovalues.ApplyTLSConfig(tlsConfig, version, values)

// apply FipsValues on top of merged values from profile
istiovalues.ApplyFipsValues(values)
Expand Down
Loading