Repository: traefik/traefik (language: go)

Issue:
cipher list is different when DefaultTLSOptions is used and when an unmarshaled TLSOption with nil CipherSuites
### Welcome!

- [X] Yes, I've searched similar issues on [GitHub](https://github.com/traefik/traefik/issues) and didn't find any.
- [X] Yes, I've searched similar issues on the [Traefik community forum](https://community.traefik.io) and didn't find any.

### What did you do?

traefik 2.11.2
Configured using kubernetes.

Reference TLSOption:
```yaml
apiVersion: traefik.io/v1alpha1
kind: cipher
metadata:
  name: default
  namespace: default
spec:
  sniStrict: true
```

The cipher list presented to `nmap -p 443 --script ssl-enum-ciphers <domain>` is different when TLSOption is absent  vs when present, even though the above TLSOption configuration has no ciphers present. Specifically, the cipher "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA" is present when TLSOption has been applied, even if no ciphers are listed.

Ciphers without TLSOption applied:
```
PORT    STATE SERVICE
443/tcp open  https
| ssl-enum-ciphers: 
|   TLSv1.2: 
|     ciphers: 
|       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (secp256r1) - A
|     compressors: 
|       NULL
|     cipher preference: client
|   TLSv1.3: 
|     ciphers: 
|       TLS_AKE_WITH_AES_128_GCM_SHA256 (ecdh_x25519) - A
|       TLS_AKE_WITH_AES_256_GCM_SHA384 (ecdh_x25519) - A
|       TLS_AKE_WITH_CHACHA20_POLY1305_SHA256 (ecdh_x25519) - A
|     cipher preference: server
|_  least strength: A
```

Ciphers with TLSOption applied:
```
PORT    STATE SERVICE
443/tcp open  https
| ssl-enum-ciphers: 
|   TLSv1.2: 
|     ciphers: 
|       TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (secp256r1) - C
|       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
|       TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (secp256r1) - A
|     compressors: 
|       NULL
|     cipher preference: client
|     warnings: 
|       64-bit block cipher 3DES vulnerable to SWEET32 attack
|   TLSv1.3: 
|     ciphers: 
|       TLS_AKE_WITH_AES_128_GCM_SHA256 (ecdh_x25519) - A
|       TLS_AKE_WITH_AES_256_GCM_SHA384 (ecdh_x25519) - A
|       TLS_AKE_WITH_CHACHA20_POLY1305_SHA256 (ecdh_x25519) - A
|     cipher preference: server
|_  least strength: C
```

The difference is the addition of `TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA` when TLSOption is applied.

Code Analysis
---
As of 2024-04-12 commit 1ffbffb26a3676b368c4fc52c4fb737de56cd65d on master.

`pkg/tls/tlsmanager.go:37` function `getCipherSuites()` builds the list from `crypto/tls.CipherSuites()`.  This results in the 3DES not being applied because the CipherSuites() call excludes that cipher.

`pkg/tls/tlsmanager.go:384` in function `buildTLSConfig`, checks if the `TLSOptions.CipherSuites` is not nil. Because the default TLSOption builds the cipher suite (above) is called, when no user defined TLSOption is present, this will be called an a list of ciphers will be constructed here. However, If a TLSOption is provided, but leaves the `CipherSuites` off, then the `*tls.Config.CipherSuites` will be unset, which will result in a different list of ciphers getting applied.

This results in a different list of applied ciphers if no user supplied TLSOption is supplied vs a TLSOption is supplied but no ciphers are listed.

One solution would be to make the default ciphers explicit in all situations by adjusting `pkg/tls/tlsmanager.go:384`
```diff
diff --git a/pkg/tls/tlsmanager.go b/pkg/tls/tlsmanager.go
index ecf9692cf..776fc5854 100644
--- a/pkg/tls/tlsmanager.go
+++ b/pkg/tls/tlsmanager.go
@@ -381,16 +381,19 @@ func buildTLSConfig(tlsOption Options) (*tls.Config, error) {
 	}
 
 	// Set the list of CipherSuites if set in the config
-	if tlsOption.CipherSuites != nil {
-		// if our list of CipherSuites is defined in the entryPoint config, we can re-initialize the suites list as empty
-		conf.CipherSuites = make([]uint16, 0)
-		for _, cipher := range tlsOption.CipherSuites {
-			if cipherConst, exists := CipherSuites[cipher]; exists {
-				conf.CipherSuites = append(conf.CipherSuites, cipherConst)
-			} else {
-				// CipherSuite listed in the toml does not exist in our listed
-				return nil, fmt.Errorf("invalid CipherSuite: %s", cipher)
-			}
+	// Set default TLS Ciphers consistently.
+	ciphers := tlsOption.CipherSuites
+	if len(ciphers) == 0 {
+		ciphers = getCipherSuites()
+	}
+	// if our list of CipherSuites is defined in the entryPoint config, we can re-initialize the suites list as empty
+	conf.CipherSuites = make([]uint16, 0)
+	for _, cipher := range ciphers {
+		if cipherConst, exists := CipherSuites[cipher]; exists {
+			conf.CipherSuites = append(conf.CipherSuites, cipherConst)
+		} else {
+			// CipherSuite listed in the toml does not exist in our listed
+			return nil, fmt.Errorf("invalid CipherSuite: %s", cipher)
 		}
 	}
 ```



### What did you see instead?

Inconsistent list of default ciphers, depending on if I built the TLSOption or if the default TLSOption was provided by traefik.

### What version of Traefik are you using?

v2.11.2

### What is your environment & configuration?

Linux
Kubernetes

### If applicable, please paste the log output in DEBUG level

_No response_


Discussion / hints:
Hello @dtscd,

Thanks a lot for reporting this, and I'm sorry for the late answer.
We confirmed the problem and attributed the p1 priority to this bug.

Fix patch:
diff --git a/pkg/provider/kubernetes/crd/kubernetes.go b/pkg/provider/kubernetes/crd/kubernetes.go
index 0ef4d78ad2..83f0b48460 100644
--- a/pkg/provider/kubernetes/crd/kubernetes.go
+++ b/pkg/provider/kubernetes/crd/kubernetes.go
@@ -876,64 +876,69 @@ func createChainMiddleware(ctx context.Context, namespace string, chain *traefik
 }
 
 func buildTLSOptions(ctx context.Context, client Client) map[string]tls.Options {
-	tlsOptionsCRD := client.GetTLSOptions()
+	tlsOptionsCRDs := client.GetTLSOptions()
 	var tlsOptions map[string]tls.Options
 
-	if len(tlsOptionsCRD) == 0 {
+	if len(tlsOptionsCRDs) == 0 {
 		return tlsOptions
 	}
 	tlsOptions = make(map[string]tls.Options)
 	var nsDefault []string
 
-	for _, tlsOption := range tlsOptionsCRD {
-		logger := log.FromContext(log.With(ctx, log.Str("tlsOption", tlsOption.Name), log.Str("namespace", tlsOption.Namespace)))
+	for _, tlsOptionsCRD := range tlsOptionsCRDs {
+		logger := log.FromContext(log.With(ctx, log.Str("tlsOption", tlsOptionsCRD.Name), log.Str("namespace", tlsOptionsCRD.Namespace)))
 		var clientCAs []tls.FileOrContent
 
-		for _, secretName := range tlsOption.Spec.ClientAuth.SecretNames {
-			secret, exists, err := client.GetSecret(tlsOption.Namespace, secretName)
+		for _, secretName := range tlsOptionsCRD.Spec.ClientAuth.SecretNames {
+			secret, exists, err := client.GetSecret(tlsOptionsCRD.Namespace, secretName)
 			if err != nil {
-				logger.Errorf("Failed to fetch secret %s/%s: %v", tlsOption.Namespace, secretName, err)
+				logger.Errorf("Failed to fetch secret %s/%s: %v", tlsOptionsCRD.Namespace, secretName, err)
 				continue
 			}
 
 			if !exists {
-				logger.Warnf("Secret %s/%s does not exist", tlsOption.Namespace, secretName)
+				logger.Warnf("Secret %s/%s does not exist", tlsOptionsCRD.Namespace, secretName)
 				continue
 			}
 
-			cert, err := getCABlocks(secret, tlsOption.Namespace, secretName)
+			cert, err := getCABlocks(secret, tlsOptionsCRD.Namespace, secretName)
 			if err != nil {
-				logger.Errorf("Failed to extract CA from secret %s/%s: %v", tlsOption.Namespace, secretName, err)
+				logger.Errorf("Failed to extract CA from secret %s/%s: %v", tlsOptionsCRD.Namespace, secretName, err)
 				continue
 			}
 
 			clientCAs = append(clientCAs, tls.FileOrContent(cert))
 		}
 
-		id := makeID(tlsOption.Namespace, tlsOption.Name)
+		id := makeID(tlsOptionsCRD.Namespace, tlsOptionsCRD.Name)
 		// If the name is default, we override the default config.
-		if tlsOption.Name == tls.DefaultTLSConfigName {
-			id = tlsOption.Name
-			nsDefault = append(nsDefault, tlsOption.Namespace)
+		if tlsOptionsCRD.Name == tls.DefaultTLSConfigName {
+			id = tlsOptionsCRD.Name
+			nsDefault = append(nsDefault, tlsOptionsCRD.Namespace)
 		}
 
-		alpnProtocols := tls.DefaultTLSOptions.ALPNProtocols
-		if len(tlsOption.Spec.ALPNProtocols) > 0 {
-			alpnProtocols = tlsOption.Spec.ALPNProtocols
+		tlsOption := tls.Options{}
+		tlsOption.SetDefaults()
+
+		tlsOption.MinVersion = tlsOptionsCRD.Spec.MinVersion
+		tlsOption.MaxVersion = tlsOptionsCRD.Spec.MaxVersion
+
+		if tlsOptionsCRD.Spec.CipherSuites != nil {
+			tlsOption.CipherSuites = tlsOptionsCRD.Spec.CipherSuites
 		}
 
-		tlsOptions[id] = tls.Options{
-			MinVersion:       tlsOption.Spec.MinVersion,
-			MaxVersion:       tlsOption.Spec.MaxVersion,
-			CipherSuites:     tlsOption.Spec.CipherSuites,
-			CurvePreferences: tlsOption.Spec.CurvePreferences,
-			ClientAuth: tls.ClientAuth{
-				CAFiles:        clientCAs,
-				ClientAuthType: tlsOption.Spec.ClientAuth.ClientAuthType,
-			},
-			SniStrict:     tlsOption.Spec.SniStrict,
-			ALPNProtocols: alpnProtocols,
+		tlsOption.CurvePreferences = tlsOptionsCRD.Spec.CurvePreferences
+		tlsOption.ClientAuth = tls.ClientAuth{
+			CAFiles:        clientCAs,
+			ClientAuthType: tlsOptionsCRD.Spec.ClientAuth.ClientAuthType,
 		}
+		tlsOption.SniStrict = tlsOptionsCRD.Spec.SniStrict
+
+		if tlsOptionsCRD.Spec.ALPNProtocols != nil {
+			tlsOption.ALPNProtocols = tlsOptionsCRD.Spec.ALPNProtocols
+		}
+
+		tlsOptions[id] = tlsOption
 	}
 
 	if len(nsDefault) > 1 {
diff --git a/pkg/tls/tls.go b/pkg/tls/tls.go
index bacc3e3376..3bf3668051 100644
--- a/pkg/tls/tls.go
+++ b/pkg/tls/tls.go
@@ -32,6 +32,7 @@ type Options struct {
 func (o *Options) SetDefaults() {
 	// ensure http2 enabled
 	o.ALPNProtocols = DefaultTLSOptions.ALPNProtocols
+	o.CipherSuites = DefaultTLSOptions.CipherSuites
 }
 
 // +k8s:deepcopy-gen=true


Test patch:
diff --git a/pkg/provider/kubernetes/crd/kubernetes_test.go b/pkg/provider/kubernetes/crd/kubernetes_test.go
index 6724e5c631..1d9869d617 100644
--- a/pkg/provider/kubernetes/crd/kubernetes_test.go
+++ b/pkg/provider/kubernetes/crd/kubernetes_test.go
@@ -860,6 +860,21 @@ func TestLoadIngressRouteTCPs(t *testing.T) {
 								"http/1.1",
 								"acme-tls/1",
 							},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -915,6 +930,21 @@ func TestLoadIngressRouteTCPs(t *testing.T) {
 								"http/1.1",
 								"acme-tls/1",
 							},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -3063,6 +3093,21 @@ func TestLoadIngressRoutes(t *testing.T) {
 								"http/1.1",
 								"acme-tls/1",
 							},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -3121,6 +3166,21 @@ func TestLoadIngressRoutes(t *testing.T) {
 								"http/1.1",
 								"acme-tls/1",
 							},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -5511,6 +5571,21 @@ func TestCrossNamespace(t *testing.T) {
 						"cross-ns-tls-options-cn": {
 							MinVersion:    "VersionTLS12",
 							ALPNProtocols: []string{"h2", "http/1.1", "acme-tls/1"},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -5555,6 +5630,21 @@ func TestCrossNamespace(t *testing.T) {
 						"cross-ns-tls-options-cn": {
 							MinVersion:    "VersionTLS12",
 							ALPNProtocols: []string{"h2", "http/1.1", "acme-tls/1"},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -5803,6 +5893,21 @@ func TestCrossNamespace(t *testing.T) {
 						"cross-ns-tls-options-cn": {
 							MinVersion:    "VersionTLS12",
 							ALPNProtocols: []string{"h2", "http/1.1", "acme-tls/1"},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},
@@ -5846,6 +5951,21 @@ func TestCrossNamespace(t *testing.T) {
 						"cross-ns-tls-options-cn": {
 							MinVersion:    "VersionTLS12",
 							ALPNProtocols: []string{"h2", "http/1.1", "acme-tls/1"},
+							CipherSuites: []string{
+								"TLS_AES_128_GCM_SHA256",
+								"TLS_AES_256_GCM_SHA384",
+								"TLS_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+								"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+								"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+								"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+								"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
+								"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
+							},
 						},
 					},
 				},