traefik / traefik

The Cloud Native Application Proxy
https://traefik.io
MIT License
51.34k stars 5.1k forks source link

cipher list is different when DefaultTLSOptions is used and when an unmarshaled TLSOption with nil CipherSuites #10609

Closed dtscd closed 3 months ago

dtscd commented 7 months ago

Welcome!

What did you do?

traefik 2.11.2 Configured using kubernetes.

Reference TLSOption:

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

rtribotte commented 6 months ago

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.

traefiker commented 3 months ago

Closed by #10907.