daemon: Add --disable-sip-verification flag #18274

michaelraskansky · 2021-12-16T09:29:37Z

Add flag to disable sip verification.
This will allow to configure the datapath so it doesn't drop packets due to invalid source IP in the datapath.
This is helpful when routing IP payload from external ip networks through kubernets via ip tunnels. See #16134 for more information.

Signed-off-by: Michael Raskansky michaelraskansky@gmail.com

daemon/cmd/endpoint.go

maintainer-s-little-helper · 2021-12-29T08:34:36Z

Commit 48354afbba80ebaa341ac77f6d6c3951bc28bbb9 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

maintainer-s-little-helper · 2021-12-29T08:43:09Z

Commits 48354afbba80ebaa341ac77f6d6c3951bc28bbb9, 73c37466ab070b4affc2db87fb5c3f452c31545b do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

maintainer-s-little-helper · 2021-12-29T08:49:27Z

Commits 48354afbba80ebaa341ac77f6d6c3951bc28bbb9, 73c37466ab070b4affc2db87fb5c3f452c31545b, 40797cd72e4e5439315f5c03c3eb4a8ebc6d0f2d do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

maintainer-s-little-helper · 2021-12-29T09:41:52Z

Commits 48354afbba80ebaa341ac77f6d6c3951bc28bbb9, 73c37466ab070b4affc2db87fb5c3f452c31545b, 40797cd72e4e5439315f5c03c3eb4a8ebc6d0f2d, 751010b24fb96338d9d7a83622704e8c91b01876 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

Add flag to disable sip veification. This will allow to configure the datapath so it dose'nt drop packets due to invalid source ip in the datapath. This is helpful when routing IP payload from external ip networks through kubernets via ip tunnels. See cilium#16134 for more infomations. Signed-off-by: Michael Raskansky <michaelraskansky@gmail.com>

aanm · 2022-01-19T02:10:08Z

/test

joestringer

I suspect that this will break chaining mode as-is, would be good to confirm whether or not that is the case and ensure that we mitigate such a problem before merging.

I've posted another couple of other minor nits, then one bigger security question as well.

daemon/cmd/daemon_main.go

joestringer · 2022-01-20T19:33:35Z

daemon/cmd/endpoint.go

+	if epTemplate.DatapathConfiguration == nil {
+		epTemplate.DatapathConfiguration = &models.EndpointDatapathConfiguration{}
+	}
 	if option.Config.EnableEndpointRoutes {
-		if epTemplate.DatapathConfiguration == nil {
-			epTemplate.DatapathConfiguration = &models.EndpointDatapathConfiguration{}
-		}
-


I'm concerned that this will break chaining mode, for reasons described in commit 8da8b88.

joestringer · 2022-01-20T19:34:06Z

daemon/cmd/endpoint.go

@@ -1045,7 +1048,6 @@ func (h *putEndpointIDLabels) Handle(params PatchEndpointIDLabelsParams) middlew
 func (d *Daemon) QueueEndpointBuild(ctx context.Context, epID uint64) (func(), error) {
 	// Acquire build permit. This may block.
 	err := d.buildEndpointSem.Acquire(ctx, 1)
-


Unrelated change, no need to drop this line.

joestringer · 2022-01-20T19:38:46Z

pkg/option/config.go

@@ -2892,6 +2899,7 @@ func (c *DaemonConfig) Populate() {
 	c.DisableCNPStatusUpdates = viper.GetBool(DisableCNPStatusUpdates)
 	c.EnableICMPRules = viper.GetBool(EnableICMPRules)
 	c.BypassIPAvailabilityUponRestore = viper.GetBool(BypassIPAvailabilityUponRestore)
+	c.DisableSipVerification = viper.GetBool(DisableSipVerification)


I wonder about the security implications of this and how to appropriately warn users.

This can allow IP spoofing across the network via pods created with this option. This can mean bypassing security policy if someone is able to send traffic to this pod and convince it to route that traffic with an in-cluster source IP towards an in-cluster destination.

At minimum, I would be inclined to hide this option and warn very loudly and angrily to anyone deploying the option that they are potentially compromising the security of their cluster by using this option. Arguably we could perhaps discuss a more comprehensive solution to this sort of problem and come up with better guardrails. I'd welcome proposals that outline potential security concerns and mitigations with this approach.

Obviously if the user just knows best and how to properly secure their network, then they presumably could just configure this option and whatever else they need in order to have this do what they want. I don't necessarily see a reason to get in their way for this (although I could also see an argument that maybe they should just maintain these changes out-of-tree). Mostly what I'd like to avoid is giving the impression that this is a standard supported option in Cilium that anyone can safely turn on.

Hi, Joe.

I understand your point, of course security should be a primary concern, I think as you suggested, the option should be hidden and if it is used, we can log an explicit warning regarding the potential security risk of running the cluster with this option set in the startup log and evaluate adding it in the cilium status command output.

The issue I am having with source IP verification, is that it blocks all traffic exiting the pod which has a different source IP than the internal cluster IP assigned to the pod by the cluster, whether it is traffic forwarded via the pod or traffic which originated from the pod (I.E using a virtual IP). This can be problematic when running NFV applications within pods, for instance a pod can act as a VPN gateway which handles external user traffic and then forwards the traffic either internally (within the cluster) or to an external service.

A more comprehensive solution which can be evaluated is to associate external networks (not cluster-internal) with specific endpoints, for instance, endpoint X can be configured to be associated with external networks X.X.X.X/24 and Y.Y.Y.Y/32, any traffic exiting the endpoint from these networks will be allowed, (not dropped by source IP validation mechanism).

Co-authored-by: Joe Stringer <joestringernz@gmail.com>

github-actions · 2022-02-24T01:54:29Z

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 · 2022-03-10T02:00:59Z

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

doup123 · 2024-01-22T11:22:09Z

Like in similar requests, we have applications that require to be able to send packets from IP addresses different than the ones assigned by Cilium, (e.g. NetFlow data replicated by samplicator-> https://github.com/sleinen/samplicator).
It would be extremely beneficial for simplifying the CI/CD of applications like that to be able:

to disable totally the source IP verification (without each restart of cilium pods to enable it again) or
via annotations the deployment manifest to be able to select the IP ranges from which a pod is able to send spoofed traffic (note that a similar feature has been already implemented in Calico: Add option to selectively disable rpf check for workloads projectcalico/calico#5742)
The latter approach enables to define the required behavior on the deployment manifest file allowing to apply configuration on a pod without directly configuring Cilium.

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 Dec 16, 2021

michaelraskansky marked this pull request as ready for review December 16, 2021 09:38

michaelraskansky requested a review from a team December 16, 2021 09:38

michaelraskansky requested review from a team as code owners December 16, 2021 09:38

michaelraskansky requested a review from joamaki December 16, 2021 09:38

maintainer-s-little-helper bot assigned joamaki Dec 16, 2021

michaelraskansky requested review from twpayne and jrajahalme December 16, 2021 09:38

maintainer-s-little-helper bot assigned twpayne and jrajahalme Dec 16, 2021

oblazek reviewed Dec 16, 2021

View reviewed changes

daemon/cmd/endpoint.go Outdated Show resolved Hide resolved

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 29, 2021

michaelraskansky force-pushed the pr/disable-sip-verification-option branch from 751010b to 861013c Compare December 29, 2021 09:53

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 29, 2021

michaelraskansky force-pushed the pr/disable-sip-verification-option branch from 861013c to 9c2ead1 Compare December 29, 2021 10:03

jrajahalme approved these changes Jan 4, 2022

View reviewed changes

maintainer-s-little-helper bot unassigned jrajahalme Jan 4, 2022

twpayne added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jan 4, 2022

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 Jan 4, 2022

twpayne approved these changes Jan 4, 2022

View reviewed changes

maintainer-s-little-helper bot unassigned twpayne Jan 4, 2022

joamaki approved these changes Jan 18, 2022

View reviewed changes

maintainer-s-little-helper bot unassigned joamaki Jan 18, 2022

joestringer requested changes Jan 20, 2022

View reviewed changes

Update daemon/cmd/daemon_main.go

30a0443

Co-authored-by: Joe Stringer <joestringernz@gmail.com>

github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Feb 24, 2022

github-actions bot closed this Mar 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

daemon: Add --disable-sip-verification flag #18274

daemon: Add --disable-sip-verification flag #18274

michaelraskansky commented Dec 16, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

aanm commented Jan 19, 2022

joestringer left a comment

joestringer Jan 20, 2022

joestringer Jan 20, 2022

joestringer Jan 20, 2022

michaelraskansky Jan 24, 2022 •

edited

github-actions bot commented Feb 24, 2022

github-actions bot commented Mar 10, 2022

doup123 commented Jan 22, 2024

daemon: Add --disable-sip-verification flag #18274

daemon: Add --disable-sip-verification flag #18274

Conversation

michaelraskansky commented Dec 16, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

maintainer-s-little-helper bot commented Dec 29, 2021

aanm commented Jan 19, 2022

joestringer left a comment

Choose a reason for hiding this comment

joestringer Jan 20, 2022

Choose a reason for hiding this comment

joestringer Jan 20, 2022

Choose a reason for hiding this comment

joestringer Jan 20, 2022

Choose a reason for hiding this comment

michaelraskansky Jan 24, 2022 • edited

Choose a reason for hiding this comment

github-actions bot commented Feb 24, 2022

github-actions bot commented Mar 10, 2022

doup123 commented Jan 22, 2024

michaelraskansky Jan 24, 2022 •

edited