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

Let singleuser.cloudMetadata.blockWithIpTables only block TCP port 80 #3185

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Aug 2, 2023

Notes thinking about this

  • I've learned we can do --destination-ports, in plural instead of singular, allowing us to specify 80 and 443 (EDIT: didn't get this to work)
  • Is the metadata API by GKE/EKS/AKS exposed only on TCP port 80? I'm not 100%, but it seems so.
    • EDIT: GKE, EKS, AKS doesn't have anything on port 443 in clusters I inspected
  • Should we block UDP port 80 as well since there are things like QUIC protocol to send HTTP traffic also etc, or is it safe to just assume there is no other ports open etc?
  • I've learned we can use an exclamation mark before a flag to say the opposite, like ! --destination-port 53 to block traffic except to port 53.
    • EDIT: But this makes us need to specify a --protocol, either tcp or udp, one at at the time, not all or similar.

I currently think that blocking only traffic to 169.254.169.254 TCP port 80 this is a clean solution thats easy to document, but I'd like quite good faith in that. Going to check this against GKE EKS AKS

@consideRatio consideRatio force-pushed the pr/metadata-api-block-only-port-80 branch 2 times, most recently from bebf07c to 7c7523c Compare August 2, 2023 10:25
@consideRatio consideRatio force-pushed the pr/metadata-api-block-only-port-80 branch from f16a1b2 to 231f12f Compare August 2, 2023 10:40
@consideRatio consideRatio marked this pull request as ready for review August 2, 2023 10:43
@manics
Copy link
Member

manics commented Aug 2, 2023

I've learned we can use an exclamation mark before a flag to say the opposite, like ! --destination-port 53 to block traffic except to port 53

This sounds safer and less likely to have unexpected consequences, but I'm happy to go with the consensus.

@consideRatio
Copy link
Member Author

Its safer, but its harder to get done. To use ! --destination-port 53, I have to specify the protocol, and I can't use all because then the flag to specify destination-port isn't available. So, what I'll end up specifying is tcp or udp for which the flag is available.

I've created three rules below inside a dummy container started like docker run -it --rm --privileged alpine:3. Maybe there are some variations to consider here, I'm not confident enough about iptables.

# what we currently do
iptables --append OUTPUT --jump DROP --destination 169.254.169.254

# options to what we currently do
# option 1 - drop just TCP port 80, this is currently in PR #3184
iptables --append OUTPUT --protocol tcp --jump DROP --destination 169.254.169.254 --destination-port 80

# option 2 - drop either only TCP or TCP and UDP for all ports except 53
iptables --append OUTPUT --protocol tcp --jump DROP --destination 169.254.169.254 ! --destination-port 53
iptables --append OUTPUT --protocol udp --jump DROP --destination 169.254.169.254 ! --destination-port 53
/ # iptables --list
Chain INPUT (policy ACCEPT)
target     prot opt source               destination         

Chain FORWARD (policy ACCEPT)
target     prot opt source               destination         

Chain OUTPUT (policy ACCEPT)
target     prot opt source               destination         
DROP       tcp  --  anywhere             169.254.169.254      tcp dpt:http
DROP       all  --  anywhere             169.254.169.254     
DROP       tcp  --  anywhere             169.254.169.254      tcp dpt:!domain
DROP       udp  --  anywhere             169.254.169.254      udp dpt:!domain

@consideRatio
Copy link
Member Author

Looking through notes in https://book.hacktricks.xyz/pentesting-web/ssrf-server-side-request-forgery/cloud-ssrf, and testing that only port 80 seems to be relevant among 80 and 443 in GKE/EKS/AKS by practical tests, I'm starting to feel confident on just blocking TCP 80.

I'll ask Yuvi later today about this.

@manics
Copy link
Member

manics commented Aug 2, 2023

This should work

iptables --append OUTPUT --destination 169.254.169.254 --protocol tcp --destination-port 53 --jump ACCEPT
iptables --append OUTPUT --destination 169.254.169.254 --protocol udp --destination-port 53 --jump ACCEPT
iptables --append OUTPUT --destination 169.254.169.254 --jump DROP

@consideRatio
Copy link
Member Author

I think that strategy is quite clean, so i appreciate it more than others in a way. But, would doing this possibly make port 53 access to that ip allowed when it perhaps isnt to be
allowed though, and is that a problem?

@manics
Copy link
Member

manics commented Aug 2, 2023

Yes, but this is still more restrictive than option 1 which allows all ports except 80.

The alternative, most secure option, is to keep the default iptables --append OUTPUT --jump DROP --destination 169.254.169.254, and replace them with these new rules if hub.networkPolicy.egressAllowRules.dnsPortsCloudMetadataServer is set.

@consideRatio
Copy link
Member Author

consideRatio commented Aug 2, 2023

Options overview

  1. Let iptables remain having a "drop all" rule (current behavior)
  2. Let iptables add two accept rules for TCP/UDP port 53 before the the "drop all" rule (see Let singleuser.cloudMetadata.blockWithIpTables only block TCP port 80 #3185 (comment))
  3. Let iptables not add a "drop all" rule, but instead a drop TCP port 80 rule (currently in this PR)
  4. Let iptables add a "drop all" rule excluding port 53 for TCP or for TCP/UDP (see Let singleuser.cloudMetadata.blockWithIpTables only block TCP port 80 #3185 (comment))
  5. Whats described in Let singleuser.cloudMetadata.blockWithIpTables only block TCP port 80 #3185 (comment)

@consideRatio
Copy link
Member Author

consideRatio commented Aug 3, 2023

I chatted with Yuvi who were inclined towards the safest approach, and a varying behavior based on configuration. I was agreeing when we chatted yesterday. Now I'm having doubts again thinking about the growing complexity of things taking this path. The growing complexity stems from use of iptables, and it makes me re-think committing further to use of iptables.

By working with iptables in the first place, we can't predict or sustainably research and document the interactions with different NetworkPolicy enforcements. Due to that, I think we shouldn't couple further to it, but instead retain current behavior and just declare singleuser.cloudMetadata.blockWithIpTables as a crude failsafe and highlight NetworkPolicy enforcement as the way to go. That blockWithIpTables should be disabled in favor of having a NetworkPolicy controller, and rely on the charts NetworkPolicy rules that doesn't allow user servers to communicate with the cloud metadata server besides using port 53 by default.

What's clear to me is that I'm not comfortable with locking us into further maintenance effort involving iptables low level details and interactions with NetworkPolicy rules and its varying enforcement implementations.

@manics
Copy link
Member

manics commented Aug 3, 2023

I'm happy with deprecating (though not removing) iptables, and saying that

  • everyone should use network policies
  • anyone with non-default requirements must use network policies

#3167 is related to Dataplane V2. According to
https://cloud.google.com/kubernetes-engine/docs/how-to/network-policy#overview

Network policy enforcement is built into GKE Dataplane V2. You do not need to enable network policy enforcement in clusters that use GKE Dataplane V2.

So we know that issue can be safely resolved without iptables, and without requiring k8s add-ons.

@manics
Copy link
Member

manics commented Aug 3, 2023

A belated thought regarding dnsPortsCloudMetadataServer- how likely is it that other ports will be required in the future? Is it worth making this not specific to DNS, and taking a list of ports, so that we don't need to add another config option in future?

@consideRatio
Copy link
Member Author

A belated thought regarding dnsPortsCloudMetadataServer- how likely is it that other ports will be required in the future? Is it worth making this not specific to DNS, and taking a list of ports, so that we don't need to add another config option in future?

I'm happy with the choice to provide a pre-defined rule specifically for 169.254.169.254:53 and not going more general in this case, thinking the combination of simple pre-defined boolean togglable rules + full freedom egress config does the trick quite well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants