Simplify TLS configuration on sidecar proxy

[shriramsharma]

The current implementation of a recent feature of enabling TLS on sidecar proxy seems to be a little complicated since we are required to disable mTLS on the app's port. See the example below.

If PeerAuthentication is not configured and with the default mode PERMISSIVE, istiod would create listeners allowing plaintext and istio in-mesh mTLS with spiffe certs.
When enabling TLS on the sidecar proxy with user-provided certs, the requirement of the feature was to not allow any plaintext traffic.
One way to fix this would be to remove the plaintext listener but doing so would give an incorrect impression to the mesh client as the mode itself is set to PERMISSIVE.
Plus this gets further convoluted with the mode set to STRICT.

I wanted to create this issue so that we can start a discussion on this and come to a conclusion on how we can simplify this configuration.

apiVersion: v1
kind: Service
metadata:
  name: httpbin
  labels:
    app: httpbin
    service: httpbin
spec:
  ports:
  - name: https
    port: 8443
    targetPort: 80
  selector:
    app: httpbin
---

apiVersion: networking.istio.io/v1alpha3
kind: Sidecar
metadata:
  name: httpbin
  namespace: app1
spec:
  workloadSelector:
    labels:
      app: httpbin
  ingress:
    - port:
        number: 80
        name: http
        protocol: HTTPS
      defaultEndpoint: 127.0.0.1:80
      tls:
        mode: SIMPLE
        privateKey: "/etc/certs/tls.key"
        serverCertificate: "/etc/certs/tls.crt"
---

apiVersion: security.istio.io/v1beta1
kind: PeerAuthentication
metadata:
  name: httpbin-pa
  namespace: app1
spec:
  selector:
    matchLabels:
      app: httpbin
  mtls:
    mode: STRICT
  portLevelMtls:
    80:
      mode: DISABLE

[hzxuzhonghu]

Ingress tls mode and pa tls mode seems confusing, and have conflict.

PA is to control istio mtls, should not be set at all if ingress tls has been set

[kfaseela]

@howardjohn any comments? Though the current feature is extremely useful and helps us get rid of the envoy filter we have been using so far, the need to explicitly disable the PeerAuth at port level looks a bit of an overhead.

[howardjohn]

We need to decide if we want mTLS or TLS, or to support mTLS and TLS

[kfaseela]

mTLS "or" TLS on the same port, "without" the need of explicit peerAuth disable would be good enough(if that is easier to achieve). Like if we specify hybrid sidecar and use a port for ingress it should automatically be excluded and not an explicit PA should be necessary

[hzxuzhonghu]

Looks we can support mtls and tls simultaneously, one for outsidecar access and one for inner mesh access.

The later filter chain match with

             "applicationProtocols": [
                        "istio",
                        "istio-peer-exchange",
                        "istio-http/1.0",
                        "istio-http/1.1",
                        "istio-h2"
                    ]

[hzxuzhonghu]

Doing this will bring some change to permissive mode.

[shriramsharma]

@kfaseela @howardjohn @hzxuzhonghu ... Here are some proposals we came up with.

cc: @aattuluri

Solution 1:

Honor PeerAuthentication only for mTLS auth.
If the sidecar config has TLS mode set to SIMPLE , then all we do is we make sure we remove the plaintext listener for that port only even if the PeerAuthentication is set to PERMISSIVE or DISABLE.

If the sidecar config has TLS mode set to MUTUAL

a. and if PA is set to STRICT, then we allow both user mTLS and istio mTLS. We would use ALPN to find the listener match.

b. and if PA is set to PERMISSIVE, then we allow plaintext, user mTLS and istio mTLS

c. and if PA is set to DISABLE, then we only allow plaintext and the mTLS setting on the sidecar is ignored.

Since PeerAuthentication is supposed to control mTLS communication within the mesh, we should not even consider it when it comes to one-way TLS. With this proposal, we can probably extend it control mTLS communication ingressing from outside the mesh.

Pros:

1. We remove the need for PeerAuthentication configuration for one-way TLS. Makes the configuration reletaively simple.

Cons:

1. We still need to create/update PeerAuth configurations in order to enable this feature for mTLS.

Solution 2:

Add another mode called EXTERNAL_TLS. We use this setting to primarily drive the user provided TLS or mTLS configuration certificate overrides on the Sidecar object.

Example:

apiVersion: networking.istio.io/v1alpha3
kind: Sidecar
metadata:
  name: httpbin
  namespace: app1
spec:
  workloadSelector:
    labels:
      app: httpbin
  ingress:
    - port:
        number: 80
        name: http
        protocol: HTTPS
      defaultEndpoint: 127.0.0.1:80
      tls:
        mode: SIMPLE
        privateKey: "/etc/certs/tls.key"
        serverCertificate: "/etc/certs/tls.crt"
---

apiVersion: security.istio.io/v1beta1
kind: PeerAuthentication
metadata:
  name: httpbin-pa
  namespace: app1
spec:
  selector:
    matchLabels:
      app: httpbin
  mtls:
    mode: STRICT
  portLevelMtls:
    80:
      mode: EXTERNAL_TLS

Pros:

1. This explicit mode (EXTERNAL_TLS) is more intuitive than setting to DISABLED.

Cons:

1. We still need to create/update two separate configurations in order to enable this feature.
2. Will need to add additional validations to make sure both the configurations are set correctly.

Solution 3:

Move the TLS configuration from the Sidecar IstioIngressListener API to PeerAuthentication. Even though this approach is not backwards compatible, this would eliminate the need for two separate config.

Example:

apiVersion: security.istio.io/v1beta1
kind: PeerAuthentication
metadata:
  name: httpbin-pa
  namespace: app1
spec:
  selector:
    matchLabels:
      app: httpbin
  mtls:
    mode: STRICT
  portLevelMtls:
    80:
      tls:
        mode: SIMPLE
        privateKey: "/etc/certs/tls.key"
        serverCertificate: "/etc/certs/tls.crt"

[howardjohn]

Solution 1:

Note: auto mtls relies on PeerAuthentication so we would needto update that code as well. Actually the client side code would also need to change, most likely, to detect if the client sent TLS (in which case we passthrough?) or plaintext (in which case we wrap in mTLS).

[hzxuzhonghu]

c. and if PA is set to DISABLE, then we only allow plaintext and the mTLS setting on the sidecar is ignored.

This seems conflict with sidecar tls config

[hzxuzhonghu]

b. and if PA is set to PERMISSIVE, then we allow plaintext, user mTLS and istio mTLS

I guess this iss too loose, not user's intention

[hzxuzhonghu]

For simplicity why cannot we make user-defined tls override istio tls?

Or can we move sidecar tls setting to PA?

[shriramsharma]

Or can we move sidecar tls setting to PA?

@hzxuzhonghu , yeah that's the Solution 3 mentioned above but that brings in the issue of backward compatibility.

[shriramsharma]

For simplicity why cannot we make user-defined tls override istio tls?

@hzxuzhonghu , I agree this would be the simplest approach. Basically TLS config on the sidecar takes precedence over PA .
@howardjohn @kfaseela @aattuluri ... any concerns with this?

[kfaseela]

For simplicity why cannot we make user-defined tls override istio tls?

@hzxuzhonghu , I agree this would be the simplest approach. Basically TLS config on the sidecar takes precedence over PA . @howardjohn @kfaseela @aattuluri ... any concerns with this?

Assuming when you say "TLS config on sidecar" it covers both SIMPLE and MUTUAL TLS, that works for the usecases I am looking for!

[howardjohn]

I agree this would be the simplest approach. Basically TLS config on the sidecar takes precedence over PA .

I do have some concerns here. The presence of PA is used in a number of places. inside istio, it is used to determine auto mTLS and multinetwork connectivity; that would all need to be updated to take Sidecar into account as well. The bigger problem is externally. There is an ecosystem of tooling around this. For example, users have Gatekeeper policies that say "All namespaces must have STRICT mTLS". If we have this new config suddenly override that, they may have policy violations.

If we have a security policy that is implicitly overriden, that is very scary.

[hzxuzhonghu]

Or can we move sidecar tls setting to PA?

@hzxuzhonghu , yeah that's the Solution 3 mentioned above but that brings in the issue of backward compatibility.

Yes, it does break. I just think it is just introduced, and istio 1.13 is just released, maybe no user yet.

Actually this looks the best way, no implicit overriden.

[kfaseela]

@shriramsharma If Solution 1 has its own complexities, and Solution 2 is anyways not simplifying any configuration, so Solution 3 is only the way forward? :D But that means re-doing everything you already did??

[shriramsharma]

Thanks @howardjohn @hzxuzhonghu @kfaseela for your feedback. I'll bring this up in the next Networking Group meeting and see if we can get a good consensus on the solutions discussed here.

[shriramsharma]

It seems like moving the TLS config to PeerAuthN was not an acceptable solution as discussed here istio/api#2314.
As @howardjohn and @costinm mentioned, in the future we could either utilize the new K8s Gateway API to configure this ( might have to figure how to apply them to sidecar) and/or with HTTP CONNECT, separating Istio mTLS and user mTLS will not be a concern.

I think, for now, we can continue using the feature in its current form.

cc: @kfaseela @aattuluri

[kfaseela]

It seems like moving the TLS config to PeerAuthN was not an acceptable solution as discussed here istio/api#2314. As @howardjohn and @costinm mentioned, in the future we could either utilize the new K8s Gateway API to configure this ( might have to figure how to apply them to sidecar) and/or with HTTP CONNECT, separating Istio mTLS and user mTLS will not be a concern.

I think, for now, we can continue using the feature in its current form.

cc: @kfaseela @aattuluri

Thanks for following up on this @shriramsharma !

[istio-policy-bot]

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2022-05-05. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.