Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove insecure tls parameters or make them configurable #698

Closed
felixhuettner opened this issue Jan 26, 2022 · 8 comments
Closed

Remove insecure tls parameters or make them configurable #698

felixhuettner opened this issue Jan 26, 2022 · 8 comments

Comments

@felixhuettner
Copy link

Describe the solution you'd like
We are currently running trident in a kubernetes cluster as a csi provisioner (installed using tridentctl).
Each of the pods of the trident daemonset is listening on port 34572 for https requests. This https session does however support outdated versions/ciphers. In particular:

  • TLS 1.0
  • TLS 1.1
  • TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
  • TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
    It is however not possible to change the supported ciphers via configuration. Could trident be adjusted so that either the cipher/tls version list is configurable; or that only secure ciphers/tls versions are choosen?

Describe alternatives you've considered
None: supporting TLS 1.0 and 1.1 is not something that is needed

Additional context
Currently used version is netapp/trident:21.04.0

@adkerr
Copy link
Contributor

adkerr commented Jan 26, 2022

The HTTPS server should only be allowing a minimum of TLS 1.2, have you been able to verify that it's allowing 1.0 and 1.1?

MinTLSVersion = tls.VersionTLS12

TLSConfig: &tls.Config{ClientAuth: tls.RequireAndVerifyClientCert, MinVersion: config.MinTLSVersion},

@adkerr
Copy link
Contributor

adkerr commented Jan 26, 2022

Additionally, the node pods should only be opening a port on 17546 by default to listen for liveness probes from Kubelet, only the controller pod (running from the deployment not the daemonset) listens on port 34572.

@felixhuettner
Copy link
Author

it seems to be open on all of our nodes, so it seems to be attached to the daemonset.
The container does seem to start a /trident_orchestrator process that opens that port

xxx@k8s-worker97:~$ sudo netstat -tulpn  | grep 34572
tcp6       0      0 :::34572                :::*                    LISTEN      4929/trident_orches
xxx@k8s-worker97:~$ ps aux | grep trident_orches
root        4929  0.7  0.0 758920 53456 ?        Ssl   2021 877:48 /trident_orchestrator --no_persistence --rest=false --csi_node_name=k8s-worker97 --csi_endpoint=unix://plugin/csi.sock --csi_role=node --log_format=text --node_prep=false --https_rest --https_port=34572 -debug

And i can connect to it using TLS1.0 (the strange openssl path comes from my default openssl lib not supporting the required ciphers anymore)

xxx@k8s-worker97:~$ ./bin/openssl.Linux.x86_64.static s_client -connect localhost:34572 -tls1
---snip---
New, TLSv1/SSLv3, Cipher is ECDHE-ECDSA-AES128-SHA
Server public key is 521 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1
    Cipher    : ECDHE-ECDSA-AES128-SHA
    Session-ID: xxx
    Session-ID-ctx:
    Master-Key: xxx
    Key-Arg   : None
    PSK identity: None
    PSK identity hint: None
    SRP username: None
---snip---

I was quite confused as i saw the TLS1.2 configuration as well and i'm now not sure where this comes from

@adkerr
Copy link
Contributor

adkerr commented Jan 26, 2022

So the port number being different seems to be coming from your DaemonSet settings. It's giving the following option to the trident binary --https_port=34572 which is overwriting the default 17546. If that's not causing any port conflicts then there's no real reason to change that right now. However I'm not understanding why the other protocols are being allowed. We do run a port scan as part of our release testing and ever since we updated the minimum TLS version is has not shown those protocols being available on the probe port.

I'll change this to a bug and track it so we can try to reproduce. Are you by any chance able to try upgrading to the latest trident to see if it has the same issue?

@felixhuettner
Copy link
Author

Thanks for the suggesstion.
After the upgrade to 21.10.1 the port changed to 17546 and now only tls 1.2 is supported.

However the cbc ciphers are still supported:

  • TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
  • TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA

@adkerr
Copy link
Contributor

adkerr commented Jan 27, 2022

Thanks for verifying with the latest version. I'll have the team look into disabling the obsolete ciphers as well. Do keep in mind though that this port is only used for liveness and readiness probes so there's not really any sensitive info available on this port.

@ghost
Copy link

ghost commented Oct 11, 2022

TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA is flagged as non-secure from Tenable's Nessus, perhaps others. Any plans to drop it from code?

@gnarl
Copy link
Contributor

gnarl commented Oct 11, 2022

Hi @flngen,

There isn't anything in the Trident code that specifically uses that cipher. We bumped the minimum server TLS version to 1.3 as you can see in the above referenced commit.

You can find more information about what ciphers are included for specific TLS versions in Go in the crypto package documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants