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

Spiffe Integration #17335

Closed
wants to merge 6 commits into from
Closed

Conversation

rscampos
Copy link

@rscampos rscampos commented Sep 7, 2021

What is this?

This PR contains 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.

  1. Download, compile and install cilium-agent based on this PR
  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/rscampos/1e1e537c5183ccc32f2f64a949f37f41/raw/c5543cd286be7dda5d3940a09129378465de0dac/spire.yaml
  1. 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
  1. Create some registration entries. This creates some registration entries for testing.
$ ./create_registration_entries.sh
  1. Create some pods
$ kubectl run podubuntu --image=ubuntu:latest -- sleep 1000000
$ kubectl run podpraqma --image=praqma/network-multitool:latest -- sleep 100000
  1. Restrict all egress traffic
$ kubectl apply -f https://gist.githubusercontent.com/rscampos/1e1e537c5183ccc32f2f64a949f37f41/raw/c5543cd286be7dda5d3940a09129378465de0dac/deny-all-egress.yaml
  1. Check that ping is failing
$ kubectl exec podpraqma -- ping <podubuntu IP>
  1. Deploy image based policy that allows those two pods to talk to each other
$ kubectl apply -f https://gist.githubusercontent.com/rscampos/1e1e537c5183ccc32f2f64a949f37f41/raw/c5543cd286be7dda5d3940a09129378465de0dac/image-based.yaml
  1. 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

ref #4016
old PR #16626

cc @nyrahul @jrajahalme @jrfastab @navarrothiago @mauriciovasquezbernal

@rscampos rscampos requested a review from a team September 7, 2021 22:55
@rscampos rscampos requested review from a team as code owners September 7, 2021 22:55
@rscampos rscampos requested a review from a team September 7, 2021 22:55
@rscampos rscampos requested a review from a team as a code owner September 7, 2021 22:55
@rscampos rscampos requested a review from a team September 7, 2021 22:55
@rscampos rscampos requested a review from a team as a code owner September 7, 2021 22:55
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 7, 2021
@rscampos rscampos marked this pull request as draft September 7, 2021 22:58
@joestringer joestringer added the release-note/major This PR introduces major new functionality to Cilium. label Sep 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 13, 2021
@rscampos
Copy link
Author

rscampos commented Sep 20, 2021

Folks,

Just sent a commit updating to the last version of Delegated Identity API. Also, this commit introduces the single watch approach: for each endpoint, one stream between Cilium and Spire is created to get X509-SVID updates.

@aanm
Copy link
Member

aanm commented Oct 11, 2021

@rscampos can you rebase against master? I assume this PR can be marked for review? If it's in draft, people will tend to ignore it unless it is marked as ready :-)

@rscampos rscampos force-pushed the spiffe-integration-poc-part1 branch 4 times, most recently from f849715 to 12ec574 Compare October 24, 2021 23:32
@rscampos
Copy link
Author

Hello @aanm, I'm sorry for the delay... I did a rebase and organize all the commits in an easier way.
Besides that, do you folks think that would be good to send the second part (Upgrading connections to mTLS) later on or can we add together with the current PR?

I'll mark for review, feel free to ping me any time,

tks :)

@aanm aanm removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Dec 9, 2021
@tklauser tklauser removed their request for review January 11, 2022 09:02
@tklauser tklauser removed their assignment Jan 11, 2022
@HighWatersDev
Copy link

I've been looking forward to this integration. Just curious if you have any estimate on when this could get merged?

@thylong
Copy link

thylong commented May 11, 2022

Hey @aanm !
Since this is tagged as high priority, I apologise for duplicating @HighWatersDev's message but is it still a high priority?
Do you have any news to share that would go in this PR direction or another initiative toward Spiffe integration?

I believe we're numerous to look for this feature (as highlighted in the recent cilium blog post).

@navarrothiago
Copy link
Contributor

@thylong, regarding the initiative toward Spiffe integration, take a look at https://github.com/accuknox/cilium-spire-tutorials if you want to try it out. Maybe this could help to understand a little bit more about the integration.

@thylong
Copy link

thylong commented May 12, 2022

Thanks for your insights @navarrothiago , definitely appreciated !
I'll take a look at it 😊

@github-actions
Copy link

github-actions bot commented Jul 9, 2022

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Jul 9, 2022
@tklauser
Copy link
Member

It seems this PR hasn't seen activity in a while and picked up several merge conflicts in the meantime. I'll thus convert it to draft status.

@navarrothiago/@rscampos please feel free to move it out of draft again once the conflicts are resolved and the PR is ready for review again.

@tklauser tklauser marked this pull request as draft August 16, 2022 13:31
@github-actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Sep 16, 2022
@github-actions
Copy link

github-actions bot commented Oct 1, 2022

This pull request has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this Oct 1, 2022
@securitysamurai
Copy link

Is the Spiffe integration occurring anywhere else? Been looking forward to this going through.

@elinesterov
Copy link

Hey folks, what is missing for the integration here? Happy to figure out how to help and move this forward.

@nyrahul
Copy link
Contributor

nyrahul commented Oct 4, 2022

Let me put forward the status here based on what I know:

  1. The changes to SPIRE SDK and other spire accessory tooling that was necessary is in place and upstreamed. (the links are in the main comment).
  2. The changes in this PR were done during 1.9.x rev and the code base has substantially changed since then. However, the links to the document/design should still hold.
  3. Note that It would still be a substantial effort to handle the changes and upstream it.

If anyone wants to take this forward, here would be the steps:

  1. Reproduce the setup following this tutorial.
  2. Check with the cilium community if there is any other effort underway
  3. Rebase the code from this PR and fix the conflicts. This step is the most imp and non-trivial. If the cilium code base has changed significantly or any of its earlier design principles in the core have changed then this could be a major rework. This is the primary analysis that has to be done to get this integration done based on this PR.
  4. Reproduce the setup from step 1 with the code from step 3. If all goes well till this point, most of the work will be in place. You might need to add more tests and get the reviews from the community.

Even though some of the folks developing this PR have now moved on to do something else, I am hoping I should be able to check and pull some of them together in a discussion/call if anyone is interested to take this forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. release-note/major This PR introduces major new functionality to Cilium. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.