RFC: Spiffe Integration

[mauriciovasquezbernal]

What is this?

This is a request for comments PR regarding the Cilium-SPIFFE integration that has been proposed in this design document. This PR covers the "Identity Generation Hardening" part of such proposal. We already have some prototypes of the "Upgrading connections to mTLS" part but we prefer start discussing only the former one to keep it more scoped.

How to test?

If you want to test it by yourself you can follow these steps. Otherwise you can watch this recorded demo.

0. Download, compile and install cilium-agent based on this PR
1. Relax labels validations by updating the CNP CRD.

$ kubectl apply -f examples/crds/v2/ciliumnetworkpolicies.yaml

2. Add the --enable-spiffe flag to cilium-agent
3. Deploy Spire. I prepared a yaml manifest with all components needed to deploy it. It's based on the spire k8s quickstart tutorial but uses a custom spire-agent image that implements a POC of the spire privileged API.

$ kubectl apply -f https://gist.githubusercontent.com/mauriciovasquezbernal/820e8e11fa546c2b4e04a3a9105b47ed/raw/017ea2d66cdd1e7227ebde57f39d82ca9f1eb0f8/spire.yaml

4. Check that spire pods are running

$ kubectl get pods -n spire
NAME                READY   STATUS    RESTARTS   AGE
spire-agent-rqsp9   1/1     Running   0          14s
spire-server-0      1/1     Running   0          14s

5. Create some registration entries. This creates some registration entries for testing.

$ ./create_registration_entries.sh

6. Restart cilium-agent. (This is something that should be improved in the code to avoid this step)

$ sudo systemctl restart cilium

7. Create some pods

$ kubectl run podubuntu --image=ubuntu:latest -- sleep 1000000
$ kubectl run podpraqma --image=praqma/network-multitool:latest -- sleep 100000

8. Restrict all egress traffic

$ kubectl apply -f https://gist.githubusercontent.com/mauriciovasquezbernal/820e8e11fa546c2b4e04a3a9105b47ed/raw/017ea2d66cdd1e7227ebde57f39d82ca9f1eb0f8/deny-all-egress.yaml

9. Check that ping is failing

$ kubectl exec podpraqma -- ping <podubuntu IP>

10. Deploy image based policy that allows those two pods to talk to each other

$ kubectl apply -f https://gist.githubusercontent.com/mauriciovasquezbernal/820e8e11fa546c2b4e04a3a9105b47ed/raw/017ea2d66cdd1e7227ebde57f39d82ca9f1eb0f8/image-based.yaml

11. Ping now works

$ kubectl exec podpraqma -- ping <podubuntu IP>
PING 10.11.0.173 (10.11.0.173) 56(84) bytes of data.
64 bytes from 10.11.0.173: icmp_seq=1 ttl=63 time=0.124 ms

TODO

• Update code once Spire "privileged" PR is ready (Proposal: Add a Privileged Attestation API spiffe/spire#2246 & proto/agent: add privileged API spiffe/spire-api-sdk#7)
• Tests (both unit and integration)
• Documentation
• How to deploy spire. Create helm templates?, leave it up to the user?
• Improve label validation (undo validation relax done for this POC)
• How to initialize the spiffe watcher in a lazy way (avoid trying to connect to it when spire is not deployed yet)
• Rebase once vendor: bump etcd to v3.5.0 and grpc to v1.39.0 #15123 is merged
• Rebase once pkg/identity: Add missing labels to well-known identities #16585 is merged

ref #4016

cc @nyrahul @jrajahalme @jrfastab @navarrothiago @rscampos

[jrajahalme]

@mauriciovasquezbernal Go mod issues need fixing:

diff --git a/go.mod b/go.mod
index 258288a..d99b24e 100644
--- a/go.mod
+++ b/go.mod
@@ -118,10 +118,10 @@ replace (
 	github.com/miekg/dns => github.com/cilium/dns v1.1.4-0.20190417235132-8e25ec9a0ff3
 	github.com/optiopay/kafka => github.com/cilium/kafka v0.0.0-20180809090225-01ce283b732b
 
-	go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20210607221240-b4c60b959dd7
-
 	github.com/spiffe/spire-api-sdk => github.com/kinvolk/spire-api-sdk v1.0.0-pre.0.20210610130402-7108593e3cec
 
+	go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20210607221240-b4c60b959dd7
+
 	// Using private fork of controller-tools. See commit msg for more context
 	// as to why we are using a private fork.
 	sigs.k8s.io/controller-tools => github.com/christarazi/controller-tools v0.3.1-0.20200911184030-7e668c1fb4c2
diff --git a/go.sum b/go.sum
index 3daa888..3d5ba64 100644
--- a/go.sum
+++ b/go.sum
@@ -627,8 +627,6 @@ github.com/karrick/godirwalk v1.10.3/go.mod h1:RoGL9dQei4vP9ilrpETWE8CLOZ1kiN0Lh
 github.com/kevinburke/ssh_config v0.0.0-20201106050909-4977a11b4351 h1:DowS9hvgyYSX4TO5NpyC606/Z4SxnNYbT+WX27or6Ck=
 github.com/kevinburke/ssh_config v0.0.0-20201106050909-4977a11b4351/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF4nAY/ojJ6r6mM=
 github.com/keybase/go-ps v0.0.0-20190827175125-91aafc93ba19/go.mod h1:hY+WOq6m2FpbvyrI93sMaypsttvaIL5nhVR92dTMUcQ=
-github.com/kinvolk/spire-api-sdk v1.0.0-pre.0.20210531191218-f6af4bcf4c8e h1:TxTUdiVF31jdA2SrUdJOlH3lyIrWcaY1DlAjr6ui7BE=
-github.com/kinvolk/spire-api-sdk v1.0.0-pre.0.20210531191218-f6af4bcf4c8e/go.mod h1:2wSTZ6oEnKqI3uBST05Mmm751+yoHEvgxomYKYOQ6Ko=
 github.com/kinvolk/spire-api-sdk v1.0.0-pre.0.20210610130402-7108593e3cec h1:hBk3Ug1Vo+IZ0Yjkwtze8eyX0NS9l9SbtA4+Uhf67q4=
 github.com/kinvolk/spire-api-sdk v1.0.0-pre.0.20210610130402-7108593e3cec/go.mod h1:2wSTZ6oEnKqI3uBST05Mmm751+yoHEvgxomYKYOQ6Ko=
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 23b9e8c..096e596 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -650,7 +650,7 @@ go.etcd.io/etcd/client/v3/credentials
 go.etcd.io/etcd/client/v3/internal/endpoint
 go.etcd.io/etcd/client/v3/internal/resolver
 go.etcd.io/etcd/client/v3/yaml
-# go.mongodb.org/mongo-driver v1.4.4
+# go.mongodb.org/mongo-driver v1.4.6
 go.mongodb.org/mongo-driver/bson
 go.mongodb.org/mongo-driver/bson/bsoncodec
 go.mongodb.org/mongo-driver/bson/bsonoptions
@@ -673,8 +673,8 @@ go.uber.org/zap/internal/bufferpool
 go.uber.org/zap/internal/color
 go.uber.org/zap/internal/exit
 go.uber.org/zap/zapcore
-# go.universe.tf/metallb v0.9.6 => github.com/cilium/metallb v0.1.1-0.20210607221240-b4c60b959dd7
 go.uber.org/zap/zapgrpc
+# go.universe.tf/metallb v0.9.6 => github.com/cilium/metallb v0.1.1-0.20210607221240-b4c60b959dd7
 ## explicit
 go.universe.tf/metallb/pkg/allocator
 go.universe.tf/metallb/pkg/allocator/k8salloc
@@ -1295,6 +1295,6 @@ sigs.k8s.io/structured-merge-diff/v4/value
 sigs.k8s.io/yaml
 # github.com/miekg/dns => github.com/cilium/dns v1.1.4-0.20190417235132-8e25ec9a0ff3
 # github.com/optiopay/kafka => github.com/cilium/kafka v0.0.0-20180809090225-01ce283b732b
-# go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20210607221240-b4c60b959dd7
 # github.com/spiffe/spire-api-sdk => github.com/kinvolk/spire-api-sdk v1.0.0-pre.0.20210610130402-7108593e3cec
+# go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20210607221240-b4c60b959dd7
 # sigs.k8s.io/controller-tools => github.com/christarazi/controller-tools v0.3.1-0.20200911184030-7e668c1fb4c2
please run 'go mod tidy && go mod vendor', and submit your changes
Error: Process completed with exit code 1.

[jrajahalme]

Looks like generated deepcopy functions do not match the change to label regexes. However, we should expand the definition of the regexes to capture the allowed spiffe labels as well, maybe using the OR syntax so that non-spiffe labels would still be validated using the old rergex?

Ungenerated deepcopy source code:
diff --git a/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnetworkpolicies.yaml b/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnetworkpolicies.yaml
index bf0fc66..313ee45 100644
--- a/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnetworkpolicies.yaml
+++ b/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnetworkpolicies.yaml
@@ -181,7 +181,7 @@ spec:
                               description: MatchLabelsValue represents the value from
                                 the MatchLabels {key,value} pair.
                               maxLength: 63
-                              #pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
+                              pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
                               type: string
                             description: matchLabels is a map of {key,value} pairs.
                               A single {key,value} in the matchLabels map is equivalent
@@ -962,7 +962,7 @@ spec:
                               description: MatchLabelsValue represents the value from
                                 the MatchLabels {key,value} pair.
                               maxLength: 63
-                              #pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
+                              pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
                               type: string
                             description: matchLabels is a map of {key,value} pairs.
                               A single {key,value} in the matchLabels map is equivalent
@@ -2380,7 +2380,7 @@ spec:
                                 description: MatchLabelsValue represents the value
                                   from the MatchLabels {key,value} pair.
                                 maxLength: 63
-                                #pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
+                                pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
                                 type: string
                               description: matchLabels is a map of {key,value} pairs.
                                 A single {key,value} in the matchLabels map is equivalent
@@ -3176,7 +3176,7 @@ spec:
                                 description: MatchLabelsValue represents the value
                                   from the MatchLabels {key,value} pair.
                                 maxLength: 63
-                                #pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
+                                pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
                                 type: string
                               description: matchLabels is a map of {key,value} pairs.
                                 A single {key,value} in the matchLabels map is equivalent
Please run make generate-k8s-api and submit your changes
Error: Process completed with exit code 1.

[pchaigno]

I'm switching back to draft until the basic tests have been fixed. Feel free to mark as ready again once they are fixed or if you think it doesn't make sense to fix them first.

[pchaigno]

I added a new enable-spiffe flag in cilium-agent. I think it should be off by default, shouldn't it?

Yes 👍

[joestringer]

1. Does it make sense to use the SPIFFE ID as the label key in the CNP letting the value empty? For instance:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
 endpointSelector:
    matchLabels:
      spiffe://example.org/image/praqma/network-multitool: ""
  egress:
  - toEndpoints:
    - matchLabels:
        spiffe://example.org/image/ubuntu: ""

If we contrast this with the way that Cilium integrates objects from other external systems like Kubernetes, I would expect would be to have a dedicated key like io.cilium.spiffe.id and specify the SPIFFE id in the value, like this:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
  endpointSelector:
    matchLabels:
      io.cilium.spiffe.id: spiffe://example.org/image/praqma/network-multitool
  egress:
  - toEndpoints:
    - matchLabels:
        io.cilium.spiffe.id: spiffe://example.org/image/ubuntu

Here's a link to some kubernetes constructs being used in policy:

https://docs.cilium.io/en/latest/policy/kubernetes/#multi-cluster

2. I added a new enable-spiffe flag in cilium-agent. I think it should be off by default, shouldn't it?

Yes, at least initially I think this makes sense.

[aanm]

1. Does it make sense to use the SPIFFE ID as the label key in the CNP letting the value empty? For instance:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
 endpointSelector:
    matchLabels:
      spiffe://example.org/image/praqma/network-multitool: ""
  egress:
  - toEndpoints:
    - matchLabels:
        spiffe://example.org/image/ubuntu: ""

If we contrast this with the way that Cilium integrates objects from other external systems like Kubernetes, I would expect would be to have a dedicated key like io.cilium.spiffe.id and specify the SPIFFE id in the value, like this:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
  endpointSelector:
    matchLabels:
      io.cilium.spiffe.id: spiffe://example.org/image/praqma/network-multitool
  egress:
  - toEndpoints:
    - matchLabels:
        io.cilium.spiffe.id: spiffe://example.org/image/ubuntu

@joestringer in this case perhaps would be better as:

     - matchLabels:
         io.cilium.spiffe.id: example.org/image/ubuntu

Just to avoid duplicating spiffe

Here's a link to some kubernetes constructs being used in policy:

https://docs.cilium.io/en/latest/policy/kubernetes/#multi-cluster

1. I added a new enable-spiffe flag in cilium-agent. I think it should be off by default, shouldn't it?

Yes, at least initially I think this makes sense.

[mauriciovasquezbernal]

If we contrast this with the way that Cilium integrates objects from other external systems like Kubernetes, I would expect would be to have a dedicated key like io.cilium.spiffe.id and specify the SPIFFE id in the value, like this:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
  endpointSelector:
    matchLabels:
      io.cilium.spiffe.id: spiffe://example.org/image/praqma/network-multitool
  egress:
  - toEndpoints:
    - matchLabels:
        io.cilium.spiffe.id: spiffe://example.org/image/ubuntu

@joestringer I think it doesn't work because it's possible for an endpoint to have multiple spiffe-based labels. For instance, I don't think we are able to define the following policy using that approach.

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
  endpointSelector:
    matchLabels:
      spiffe://example.org/image/praqma/network-multitool: ""
  egress:
  - toEndpoints:
    - matchLabels:
        spiffe://example.org/image/ubuntu: ""
        spiffe://example.org/image/alpine: ""

[mauriciovasquezbernal]

Hi! I have been working with the AccuKnox folks (@nyrahul @navarrothiago @rscampos) on this PR and they are going to take over the task. I'll still be around to reply any questions and participate in the discussions.

[joestringer]

@mauriciovasquezbernal are you saying that a single workload may have multiple SPIFFE IDs associated and you want to match that the peer workload has both SPIFFE IDs to be able to allow the traffic? The example you provided is a bit confusing because it seems to specify SPIFFE IDs with OS images, but presumably a single workload can only have one OS image.

[nyrahul]

@mauriciovasquezbernal are you saying that a single workload may have multiple SPIFFE IDs associated and you want to match that the peer workload has both SPIFFE IDs to be able to allow the traffic? The example you provided is a bit confusing because it seems to specify SPIFFE IDs with OS images, but presumably a single workload can only have one OS image.

I think even if a workload/endpoint may have multiple SPIFFE IDs, I don't see how these multiple SPIFFE IDs could be used in the same rule since the connection/session will use/have just a single associated SPIFFE ID.

[joestringer]

👍 Then if you're trying to denote "allow SPIFFE ID X or SPIFFE ID Y", I think it could be done through two separate rules:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "image-based"
spec:
  endpointSelector:
    matchLabels:
      io.cilium.spiffe.id: example.org/image/praqma/network-multitool
  egress:
  - toEndpoints:
    - matchLabels:
        io.cilium.spiffe.id: example.org/image/ubuntu
  - toEndpoints:
    - matchLabels:
        io.cilium.spiffe.id: example.org/image/alpine

[joestringer]

We discussed this during the community meeting today. I had previously raised the question above whether it's possible to associate multiple SPIFFE IDs with a single endpoint and at least above in the PR I didn't get a clear answer. If this is the case, then the only way to encode this into labels would be to put the entire spiffe id into the key. The discussion during the meeting implied that multiple SPIFFE IDs may be associated with a single endpoint, but is there an upstream SPIFFE reference for this so we can better understand that model? Specifically something like a concepts document that describes the relationship between SPIFFE IDs and endpoints/connections?

[navarrothiago]

but is there an upstream SPIFFE reference for this so we can better understand that model? Specifically something like a concepts document that describes the relationship between SPIFFE IDs and endpoints/connections?

@joestringer here is https://spiffe.io/docs/latest/spiffe-about/spiffe-concepts/

[joestringer]

I couldn't see an explicit declaration of how many SPIFFE IDs may be associated with a single workload in the above concepts page. Maybe "A SPIFFE ID is a string that uniquely and specifically identifies a workload. " is supposed to imply the relationship but it's quite vague.

However, upon further digging into the SPIFFE Workload API, I did find the underlying gRPC protocol definition:

https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Workload_API.md#521-fetchx509svid

The X509SVIDResponse response consists of a mandatory svids field, which MUST contain one or more X509SVID messages (one for each identity granted to the client). The crl and federated_bundles fields are optional.

The "one for each identity granted to the client" seems to imply that a single client (by which I assume this is a SPIFFE workload, equivalent to pod in the model here) can have multiple identities.

The section on Default Identity seems to go further and describe the notion that it's up to the workload to understand the availability of multiple identities and how it should use those identities, or otherwise default to using the first one that is provided from the SPIFFE workload API:
https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Workload_API.md#53-default-identity

So yes, it seems that multiple SPIFFE IDs can be associated with a single workload.

[joestringer]

I have another question:

Let's say that we set up the spiffe identities using this system. We associate ids with workloads by encoding the spiffe ID as a key for labels associated with the endpoint. What happens if someone then manually creates a new pod with labels including a spiffe ID, something like the below?

apiVersion: v1
kind: Pod
metadata:
  name: example
  "spiffe://example.org/image/ubuntu": ""
spec:
  containers:
    - name: busybox
      image: busybox:1.25
      command:
        - sleep
        - 10000

I'm assuming that application developers in a cluster will likely have the ability to create such a pod, and at this time it would not have any interaction with SPIFFE (or if it does, SPIFFE is only adding another SPIFFE ID to the labels of the pod in addition to the one that the application developer manually added to the labels).

[rscampos]

Hello @joestringer , thank you for the observations. All good points. I did some validation on this. Please check the slides for more information. To summarize:
1- We change label source for spiffe ID labels from “k8s” to “spiffe” to avoid error during the k8s validation. Doing so, we are no more constrained with the size limits of label key.
2- Using a different source for spiffe labels, we also avoid the risk of someone specifying that labels from userspace. Details in the slides.
Note that I have validated these points using actual code changes.

[joestringer]

I've requested access to the doc. I think in terms of the label source, that part makes sense - then based on the source we can know that the SPIFFE ID comes from a legitimate SPIFFE server. What I'm still missing is how we ensure that the label match in the CiliumNetworkPolicy only matches on the SPIFFE-sourced labels, and not on k8s-sourced labels.

[joestringer]

Actually, on second thought for:

1- We change label source for spiffe ID labels from “k8s” to “spiffe” to avoid error during the k8s validation. Doing so, we are no more constrained with the size limits of label key.

This doesn't change the limits on the LabelSelector in the policy. If SPIFFE IDs are longer, will it still be possible to match on them using policy?

[rscampos]

@joestringer
I gave you permission to access the doc, could you try again please?
Let me split the comment in 3 parts:

Part 1. Use keyword spiffe as source in CNP:
After read your comment, I tried it and worked as well. I used spiffe as source in CiliumNetworkPolicy. I saw these two examples here and here. Is it a correct way of using it?

In this way, the policy is going to match just labels with spiffe source.

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "spiffe-based"
spec:
  endpointSelector:
    matchLabels:
      spiffe:io.spiffe.id.example.org/foo1: ""
  egress:
  - toEndpoints:
    - matchLabels:
        spiffe:io.spiffe.id.example.org/foo3: ""

Part 2. Label manually created by the user:
A user can manually create a label as follows, but is not possible to specify the source of the label, right? (Like this: "spiffe:io.spiffe.id.example.org/foo3": ""). Doing so, If a user try to create a label, the default source is going to be k8s. In this way the CNP used in Part 1 is not going to match.

apiVersion: v1
kind: Pod
metadata:
  name: podfoo3-personification
  labels:
    io.spiffe.id.example.org/foo3: ""
spec:
  containers:
  - name: podfoo3
    image: praqma/network-multitool
    command: ["bin/sh", "-c", "sleep 10000"]

Part 3. CNP label validation:
Using any source,k8s or spiffe in our example, a validation in going to happen in 2 places for our use case: 1) validateLabelKey() selector.go; and 2) ValidateLabelName() validation.go. Because Spiffe ID may use multiples "/", this is not a "qualified name" for Kubernetes defined here at IsQualifiedName() - this func is called by validateLabelKey() and ValidateLabelName(). This is why Mauricio changed a little the validation in these 2 places: selector.go and validation.go. Based on these changes, the len of name part of the label is not checked. I tried with a big Spiffe ID (pod creation and CNP) and worked.

Any feedback is welcome, thank you for highlighting these points :D

[joestringer]

👍 Part 1 / 2 makes sense.

I think for Part 3 you've addressed technically how the validation and internal storage of the SPIFFE IDs would work though I'd probably have to take a closer look to reason about how it's actually working against different inputs. I didn't yet quite understand how the policy can match on bigger SPIFFE IDs though, if you place a long (say ~100 character) SPIFFE match in the policy under matchLabels, can Cilium successfully create the LabelSelector based on that input so that it can actually match on the spiffe label which is internally associated with an identity?

[rscampos]

hey @joestringer, thank you for the answer
Yeah, it's possible to have a long Spiffe ID label. I recorded a PoC here. I think after the validation (that we bypassed), Cilium allows a long Spiffe ID to be used.

[rscampos]

Hey @joestringer and folks,

I'm going to send another PR from my Github account since the actual PR was sent by @mauriciovasquezbernal. Thank you @mauriciovasquezbernal for all the work.

I'm going to maintain the Mauricio's description and commit the new changes made on Spiffe ID label.

Could you folks please close this one? I'll refer this #16626 in the new PR.

cc @mauriciovasquezbernal @nyrahul @navarrothiago

Tks :)