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

Configurable control plane mTLS version and cipher settings #13138

Open
rdkr opened this issue Apr 8, 2019 · 22 comments
Open

Configurable control plane mTLS version and cipher settings #13138

rdkr opened this issue Apr 8, 2019 · 22 comments
Labels
area/security kind/enhancement lifecycle/staleproof Indicates a PR or issue has been deemed to be immune from becoming stale and/or automatically closed

Comments

@rdkr
Copy link

rdkr commented Apr 8, 2019

Describe the feature request
Currently it looks like there are no settings applied or configurable for Envoy's TLS min version / max version / cipher suites for control plane mTLS. For example, Pilot discovery on port 15011 with mTLS enabled:

Starting Nmap 7.70 ( https://nmap.org ) at 2019-04-08 09:23 BST
Nmap scan report for localhost (127.0.0.1)
Host is up (0.00048s latency).
Other addresses for localhost (not scanned): ::1

PORT      STATE SERVICE
15011/tcp open  unknown
| ssl-enum-ciphers:
|   TLSv1.0:
|     ciphers:
|       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (ecdh_x25519) - A
|       TLS_RSA_WITH_AES_128_CBC_SHA (rsa 2048) - A
|       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (ecdh_x25519) - A
|       TLS_RSA_WITH_AES_256_CBC_SHA (rsa 2048) - A
|     compressors:
|       NULL
|     cipher preference: server
|   TLSv1.1:
|     ciphers:
|       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (ecdh_x25519) - A
|       TLS_RSA_WITH_AES_128_CBC_SHA (rsa 2048) - A
|       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (ecdh_x25519) - A
|       TLS_RSA_WITH_AES_256_CBC_SHA (rsa 2048) - A
|     compressors:
|       NULL
|     cipher preference: server
|   TLSv1.2:
|     ciphers:
|       TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (ecdh_x25519) - A
|       TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (ecdh_x25519) - A
|       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (ecdh_x25519) - A
|       TLS_RSA_WITH_AES_128_GCM_SHA256 (rsa 2048) - A
|       TLS_RSA_WITH_AES_128_CBC_SHA (rsa 2048) - A
|       TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (ecdh_x25519) - A
|       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (ecdh_x25519) - A
|       TLS_RSA_WITH_AES_256_GCM_SHA384 (rsa 2048) - A
|       TLS_RSA_WITH_AES_256_CBC_SHA (rsa 2048) - A
|     compressors:
|       NULL
|     cipher preference: server
|_  least strength: A

Nmap done: 1 IP address (1 host up) scanned in 2.37 seconds

For security and compliance I would like to restrict what versions of TLS are used throughout Istio.

Describe alternatives you've considered
I believe overwriting the config files in my own image to include the TLS parameters, such as in https://github.com/istio/istio/blob/1.1.2/pilot/docker/envoy_pilot.yaml.tmpl#L204, would allow me to set these, however it would be preferable to set them through config.

Additional context
This is currently possible for some parts of Istio, such as gateway configuration. There are a few closed issues I have found which are relevant to these components, but none seem to directly address the control plane components:

@yangminzhu
Copy link
Contributor

@costinm @rshriram @diemtvu
Any suggestion for the place to add such options? Maybe 2 helm flags global.controlPlaneSecurityTLSVersionMin and global.controlPlaneSecurityTLSVersionMax below the global.controlPlaneSecurityEnabled?

@yangminzhu yangminzhu self-assigned this Apr 12, 2019
@diemtvu
Copy link
Contributor

diemtvu commented Apr 15, 2019

Do we need this configurable? Can we simply disable all well known weak cipher / protocols?

@yangminzhu
Copy link
Contributor

@diemtvu We could change the sidecar to TLS 1.2 but that might break some legacy application (no sidecar) that only supports TLS 1.0 or 1.1. Also I think from the nmap output, none of the ciphers is weak in a well known way.

The other option, instead of introducing the helm config, is to introduce a global security setting CRD, which could be used to specify the TLS version and cipher list for the sidecar in the mesh. (The same CRD also has some use cases for RBAC)

@rdkr
Copy link
Author

rdkr commented Apr 16, 2019

Any suggestion for the place to add such options? Maybe 2 helm flags global.controlPlaneSecurityTLSVersionMin and global.controlPlaneSecurityTLSVersionMax below the global.controlPlaneSecurityEnabled?

This, plus global.controlPlaneSecurityTLSCipherSuites would work well for our use case, and be similar to the configuration for gateways.

Do we need this configurable? Can we simply disable all well known weak cipher / protocols?

As mentioned above, I don't believe any of the Envoy defaults are known to be weak. The requirement to configure this is mainly governance / regulatory related rather than technical. As an example, TLS 1.0 is deprecated by PCI DSS.

@hercynium
Copy link

I believe overwriting the config files in my own image to include the TLS parameters, such as in https://github.com/istio/istio/blob/1.1.2/pilot/docker/envoy_pilot.yaml.tmpl#L204, would allow me to set these, however it would be preferable to set them through config.

I've attempted this and it didn't seem to work. However, I may be doing something wrong. Does the version of Envoy Istio is built on even support setting the min TLS version?

@tiurikov
Copy link

Are there any estimates to make this feature implemented?
Any suggested workaround?

@stale
Copy link

stale bot commented Aug 15, 2019

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 15, 2019
@dave-pollock
Copy link

The suggestion from @rdkr to also include an option for configuring ciphers would be perfect. I'm currently seeing this flagged by our internal security scans.

@lukesiler
Copy link

This is supported on networking/v1alpha3/gateway.proto with three parameters per #8804

I would suggest the same three parameters for peer mTLS config as well. Min and max TLS version support would make sense for version change scenarios where sidecars may be heterogeneous during upgrade window.

@martinbaillie
Copy link
Contributor

Noting that the policy bot has kicked in, FWIW, I would also be keen to see something happen in this area 👍

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Nov 6, 2019
@martinbaillie
Copy link
Contributor

Is it possible to re-open this issue?

@tiurikov
Copy link

I think possibility to disable weak cipher / protocols is essential. Is it really hard to implement this feature?

@tmshort
Copy link
Contributor

tmshort commented Mar 6, 2020

It is desirable to control control-plane (envoy or otherwise) and mTLS. It also appears that the gateway configuration does not apply when PASSTHROUGH mode is used (which supports mTLS, so I guess that's the crux of the problem).

@incfly incfly reopened this Apr 1, 2020
@incfly incfly added lifecycle/staleproof Indicates a PR or issue has been deemed to be immune from becoming stale and/or automatically closed and removed lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. labels Apr 1, 2020
@linsun
Copy link
Member

linsun commented Apr 1, 2020

subscribe - our users also indicate the desire to configure TLS version from 1.2 to 1.3.

@howardjohn howardjohn added this to P1 in Prioritization Sep 4, 2020
@howardjohn
Copy link
Member

We now set default to

            "tls_params": {
             "tls_minimum_protocol_version": "TLSv1_2",
             "cipher_suites": [
              "ECDHE-ECDSA-AES256-GCM-SHA384",
              "ECDHE-RSA-AES256-GCM-SHA384",
              "ECDHE-ECDSA-AES128-GCM-SHA256",
              "ECDHE-RSA-AES128-GCM-SHA256",
              "AES256-GCM-SHA384",
              "AES128-GCM-SHA256"
             ]

@tmshort
Copy link
Contributor

tmshort commented Nov 3, 2020

We now set default to...

Where is this specified and/or what was the PR?

@howardjohn
Copy link
Member

PR #27500

@howardjohn
Copy link
Member

Ignore my above message - that is for workload mTLS while this is for control plane TLS.

@huornlmj
Copy link

huornlmj commented Jun 3, 2022

@diemtvu We could change the sidecar to TLS 1.2 but that might break some legacy application (no sidecar) that only supports TLS 1.0 or 1.1. Also I think from the nmap output, none of the ciphers is weak in a well known way.

FYI:

TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA WEAK
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 SECURE
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA WEAK
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 SECURE
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 SECURE
TLS_RSA_WITH_AES_128_CBC_SHA WEAK
TLS_RSA_WITH_AES_128_GCM_SHA256 WEAK
TLS_RSA_WITH_AES_256_CBC_SHA WEAK
TLS_RSA_WITH_AES_256_GCM_SHA384 WEAK

In fact I enumerated the endpoint myself and found two additional ciphers that cause the SWEET32 vulnerability:

  • TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA VULNERABLE
  • TLS_RSA_WITH_3DES_EDE_CBC_SHA VULNERABLE

@justmenyou
Copy link

any update on how can i get rid of weak cipher?

@incfly
Copy link

incfly commented Jul 26, 2022

I think this is actually supported, @hzxuzhonghu fixed it in #31705. though not intentional for this. tls-cipher-suites flags on the istiod allows you to customize. Manually verified against 1.14.2

# after setting flag with value "TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_CBC_SHA"
nmap --script  ssl-enum-ciphers  -p 15012 localhost
Starting Nmap 7.80 ( https://nmap.org ) at 2022-07-26 22:16 UTC
Nmap scan report for localhost (127.0.0.1)
Host is up (0.000073s latency).

PORT      STATE SERVICE
15012/tcp open  unknown
| ssl-enum-ciphers:
|   TLSv1.2:
|     ciphers:
|       TLS_RSA_WITH_AES_256_GCM_SHA384 (rsa 2048) - A
|       TLS_RSA_WITH_AES_256_CBC_SHA (rsa 2048) - A
|     compressors:
|       NULL
|     cipher preference: server
|     warnings:
|       Forward Secrecy not supported by any cipher
|_  least strength: A

Nmap done: 1 IP address (1 host up) scanned in 0.22 seconds

@incfly
Copy link

incfly commented Jul 26, 2022

maybe the action item is just to add some FAQ in https://istio.io/latest/docs/ops/common-problems/security-issues/ section for discoverability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/enhancement lifecycle/staleproof Indicates a PR or issue has been deemed to be immune from becoming stale and/or automatically closed
Projects
Development

No branches or pull requests