Confusing name for CNP authentication type

[lizrice]

CiliumNetworkPolicy can now define an authentication type thanks to #24263. However, the name of the supported type mtls-spiffe is confusing: the "mtls" part implies that the resulting connection will be encrypted (as in TLS), but at the moment this field is only used to specify that the handshake that performs authentication must take place. The intention is that the resulting identities will be passed to the kernel where transparent encryption will take place, but it's possible that the encryption part will be controlled separately. With the name as it is, we risk disappointing users who will assume that they have a CNP that defines a requirement for encryption as well as authentication.

Also, if possible, it would be nice to decouple the authentication type specified in the CNP from the authentication method so that a CNP could specify that it requires mutual authentication, but doesn't care whether the identities are provided by SPIFFE, X.509 certs, or whatever.

My suggestion is that instead of auth: mtls-spiffe we use auth: mutual if it can be independent of the authentication method, or auth: mutual-spiffe if it has to be specifically SPIFFE.

It would be great if the encryption requirement could also be specified in the CNP too, like this:

auth: mutual
encrypt: true

[youngnick]

I've spent some time looking into this and thinking today, here's my summary:

I agree that we should use a non-mtls name for the SPIFFE auth method.

From what I can see, neither Wireguard nor IPSec transparent encryption modes in Cilium have a way to exclude specific flows, so I don't know if adding an encrypt field would be useful - the encryption properties are currently set at the cluster or Cilium install level (as is the mutual auth method currently), so setting encrypt: false for only a single NetworkPolicy isn't possible yet.

The one thing I can think of if we're going to be changing things is that since auth will always imply mutual, maybe we should move the word mutual into the type field?

So,

  auth:
    mutual-type: spiffe

or similar?

[squeed]

One quick suggestion: "auth" is ambiguous between authentication and authorization. I'd spell it out more clearly (or at least use authn / authz as relevant).

[youngnick]

oh, that is a good point, I think we can still change that since these fields are not useful until 1.14 is released.

[youngnick]

Okay, coming back to this to try and close this out, I have two proposals:

• rename the value in NetworkPolicy to spiffe. Just removing the tls part removes any possibility of confusion here, and we'll address that spiffe uses TLS in other parts of the docs. Something like this is basically a must-do.
• rename the current mTLS stanza in Helm to mutual instead. Something like this is another must-do.
• rename the auth stanza in NetworkPolicy and Helm to authentication before release. tbh I'm not sure here, and would appreciate further input.

I think that there are further followups about language in #25216 that will also be necessary so that we make clear in as many places as possible that having mTLS in Cilium Service Mesh requires both mutual auth and encryption of some form.

Edit: Updated the change in the third option to be authentication spelled out in full, since it's clearer and worth the 9 extra characters.

[lizrice]

rename the value in NetworkPolicy to spiffe
rename the current mTLS stanza in Helm to mutual instead
rename the auth stanza in NetworkPolicy and Helm to authn before release

I agree on all three of these. On the third one, we could even spell out authentication for clarity, but I would be happy with that or authn

[youngnick]

Okay, I'm proposing a lazy consensus period here of 48 hours, if there are no objections in that time, we'll proceed.

[joestringer]

Broadly I'm fine with the proposal above. Some thoughts/discussion points:

• If the authn configuration is in the network policy, then presumably this applies to only the endpoints selected by the policy. As a semantic I think that then says "all traffic for endpoints selected by the policy must be communicated with an authenticated peer". Does that imply mutual auth or is it up to the peer to have its own network policy to ensure that the other peer performs the authentication to this peer?

• Related, naming like mutual-type implies that there would be a non-mutual type. Do we foresee the need for non-mutual authentication as a feature? An alternative might be to just define that authentication is mutual always, then just use type. My gut feeling is that "mutual" is a different dimension from the type, so it should either be another field if we ever want to support non-mutual, or just implicit. (We can always add a comment in the Helm values docs/example YAML to make it clear that it's mutual).

[youngnick]

Thanks @joestringer!

My thoughts here:

• Right now, the auth field (to be changed to authentication) is an aliased string type present in each of ingress and egress inside CiliumNetworkPolicy. The net effect is to say "for all traffic this Policy applies to, perform mutual authentication between the source and destination". So you don't need to specify two policies, one for each direction, but the tradeoff is that more things might have identities than you thought. However, this isn't much of a tradeoff as we create crypto identities for every Cilium Identity in the cluster when mutual authentication is enabled.
  The change here will just change the field name from auth to authentication, and the value from mtls-spiffe to spiffe. So the type field is actually me confusing the Helm chart with the NetworkPolicy setup.

• Reading your second comment over, I think I agree that we should document that authentication in CiliumNetworkPolicy means "mutual authentication", and update the docstrings and other docs accordingly. authentication in the Helm config means "setting related to mutual authentication", and will be updated accordingly.

• We'll keep the type field inside the Helm config as spiffe as well, which implies that we should consider the values of authentication.type in the Helm chart and possible values of spec.rule.ingress.authentication in CiliumNetworkPolicy should match a valid option for authentication.type in the Helm chart (with the exception of the always-fail authentication.type value in NetworkPolicy, which will always fail, which is used in cilum-cli connectivity testing).

[youngnick]

With that in mind, the final proposed changes here are:

• rename the field in NetworkPolicy to authentication (from auth)
• rename the value in NetworkPolicy to spiffe from mtls-spiffe. Just removing the tls part removes any possibility of confusion here, and we'll address that spiffe uses TLS in other parts of the docs. Something like this is basically a must-do.
• rename the existing auth stanza in Helm to authentication instead.
• rename the current mTLS stanza in Helm to mutual instead. This means that the complete path will be authentication.mutual. To me, this leaves the authentication level with the possibility to add other types of authentication for Cilium to configure. I don't know if this is viable, or if we should collapse what's currently mTLS into authentication instead.
• Change all command-line flags and config file options for cilium to use mutual, mutual-auth, or MutualAuth instead of mtls, mtls-auth, or MTLS respectively.
• Refactor code to remove mTLS references (lowest priority).

So, final config for Helm values will look like this:

# Configuration for types of authentication for Cilium
authentication:
  # Configuration for Cilium's service-to-service mutual authentication using TLS handhakes.
  # Note that this is not full mTLS support without also enabling encryption of some form.
  # Current encryption options are Wireguard or IPSec, configured elsewhere in this file.
  mutual:
    # -- Port on the agent where mutual TLS handshakes between agents will be performed
    port: 4250
    # Settings for SPIRE
    spire:
      # snip, no changes to SPIRE settings.

and NetworkPolicy:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "rule1"
spec:
  description: "mTLS-auth enabled L7 policy to restrict access to specific HTTP call"
  endpointSelector:
    matchLabels:
      org: empire
      class: deathstar
  ingress:
  - fromEndpoints:
    - matchLabels:
        org: empire
-    auth:
-      type: "mtls-spiffe"
+    authentication:
+      type: "spiffe"
    toPorts:
    - ports:
      - port: "80"
        protocol: TCP
      rules:
        http:
        - method: "POST"
          path: "/v1/request-landing"

[tgraf]

I agree that the current name is confusing and we should not call anything mTLS if we are not ensuring encryption of the traffic end to end with a session-specific key. However, I would prefer an even more generic approach and separate policy intent from implementation and environment configuration.

If we look at NetworkPolicy, the policy itself does not make any implications on how the policy is being enforced. Different strategies may be used to enforce depending on what environment Cilium is running in (overlay with encapsulated identities, direct routing with ipcache, ...).

As far as CiliumNetworkPolicy is concerned, I would apply the same logic and require the user to declare intent whether mutual authentication is required or not and leave the implementation up to the configuration of Cilium. This may be SPIFFE-based with the mTLS datapath hook and the auth agent, it may be cert-manager / Vault based, or it may be zTunnel based in the future.

This will also avoid a massive potential conflict in the future. What if a user defines multiple policies in a cluster with different authentication types? Would we be running SPIFFE, vault, and cert-manager all at once? How would we perform authentication if ingress and egress authentication types differ?

I would therefore favor:

+    authentication:
+      mode: "required"

This keeps the policy simple and conflict-free.

[sayboras]

As #25761 is merged, I think we can close this issue out :)