Skip to content

Add deny_networks to silk-cni job - #23

Merged
ameowlia merged 2 commits into
cloudfoundry:developfrom
tlwr:deny-networks
Feb 11, 2020
Merged

Add deny_networks to silk-cni job#23
ameowlia merged 2 commits into
cloudfoundry:developfrom
tlwr:deny-networks

Conversation

@tlwr

@tlwr tlwr commented Feb 10, 2020

Copy link
Copy Markdown
Contributor

See #20

Context

As a platform operator, restrict certain isolation segments from accessing certain networks.

It is infeasible to use dynamic egress policies, or application security groups (native CF features), if the platform has a default security group which allows access to these networks: ASGs and egress policies do not allow you to deny destinations, only allow.

Allowing ASGs and/or egress policies to do both allow/deny is quite an involved change. Exposing this inside silk is probably fine (IMO)

Changes

This pull request allows a platform operator to specify a list of CIDR ranges which an app container should not be able to talk to.

This is similar to the garden property (from the garden-runc release) called deny_networks, but is controlled via silk instead of garden.

This pull request

  • Exposes a new property deny_networks in the silk-cni job
  • Any destinations in the deny_networks property is added to a container's netout chain, before any of the ASG/dynamic egress policy rules
  • Reformats some IPTables expectations to make them easier to read
  • Updates the BOSH job spec conflist spec test

How to use the feature

Consider the following silk-cni job running on a diego-cell:

consumes:
  vpa:
    from: vpa-dev-1
name: silk-cni
properties:
  deny_networks:
    always:
    - 172.16.0.0/12
    staging: []
    running:
    - 192.168.0.0/16
release: silk

Will cause prevent applications from accessing 172.16.0.0/12 all the time, and 192.168.0.0/16 after the application has been staged.

These rules are added before ASGs and dynamic egress policies, but after DNS requests.

How to test this against a live environment

  1. Deploy against a live environment

  2. Play around with /var/vcap/jobs/silk-cni/config/cni/cni-wrapper-plugin.conflist

  3. Deny 0.0.0.0/0 when staging and try to deploy an app

  4. Deny 93.184.216.34/32 when running and try to curl example.com from an app

  5. Deny the subnet for dig buildpacks.cloudfoundry.org and try pushing an app

Checklist

  • Updated the relevant BOSH job spec
  • Run all the tests locally /scripts/docker-test
  • Deployed to my development environment
  • Signed the CLA

Contact

@tlwr in Cloud Foundry slack

Appendix

I've deployed this, after rebasing and changing the job interface

Let me know if you want me to fix up the commits to make things more atomic

Signed-off-by: toby lorne <toby@toby.codes>
@cf-gitbot

Copy link
Copy Markdown

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/171187745

The labels on this github issue will be updated when the story is started.

Comment thread src/lib/rules/rules.go
Comment on lines +256 to +263
func NewInputRejectRule(destinationIP string) IPTablesRule {
return IPTablesRule{
"-d", destinationIP,
"--jump", "REJECT",
"--reject-with", "icmp-port-unreachable",
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for caring about formatting. Your code is very pretty.

Comment thread src/cni-wrapper-plugin/integration/cni_wrapper_plugin_test.go Outdated
Comment thread jobs/silk-cni/templates/cni-wrapper-plugin.conflist.erb
@adobley

adobley commented Feb 11, 2020

Copy link
Copy Markdown
Contributor

Aside from the two changes we asked for, we think this is good to go.

  1. Template validation of the deny_networks properties to prevent a bad CIDR at template rendering time - we should still check in the code to be safe
  2. Possibly redundant test By

Thanks for your contribution!

Amelia and Aidan

@tlwr

tlwr commented Feb 11, 2020

Copy link
Copy Markdown
Contributor Author

I think I've resolved your comments but I'm unsure if the template validation is adequate.

Now the git history is a bit messy, let me know if you want me to squash

Thanks again :)

@KauzClay

Copy link
Copy Markdown
Contributor

Hey @tlwr ,

This isn't necessary, but if you squash them, could you please put this into the bottom of your commit message?

[#171187745](https://www.pivotaltracker.com/story/show/171187745)

It's a little tag that will make this commit show up in our pipeline job that checks what changes will be included in the next release.

Thanks!

@ameowlia

ameowlia commented Feb 11, 2020

Copy link
Copy Markdown
Member

Issues look resolved to us. :bowtie: 🐱

If you could squash with the message @KauzClay added above that would be great and then we can merge.

--- Amelia + @adobley

[#171187745](https://www.pivotaltracker.com/story/show/171187745)

This allows a platform operator to specify a list of CIDR ranges which
an container should not be able to talk to. The CIDR ranges can be
applied for different lifecycles (always, staging, running)

This is similar to the garden property (from the garden-runc release)
called "deny_networks", but is applied via silk.

This can be used by a platform operator to limit outbound connectivity
for all apps within an isolation segment, by specifying deny_networks

Any ip address or cidr range in the deny_networks property is added
before the container rules

So the following deny rules:

- 0.0.0.0/5
- 8.0.0.0/7
- 11.0.0.0/8
- 12.0.0.0/6
- 16.0.0.0/4
- 32.0.0.0/3
- 64.0.0.0/2
- 128.0.0.0/1

Generate the following netout rules (added formatting for clarity)

> Chain netout--13e80c9e-5fd1-485f-7 (1 references)
> target     prot opt source               destination
> ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0            state RELATED,ESTABLISHED
> DROP       tcp  --  0.0.0.0/0            0.0.0.0/0            state INVALID
> REJECT     all  --  0.0.0.0/0            128.0.0.0/1          reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            64.0.0.0/2           reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            32.0.0.0/3           reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            16.0.0.0/4           reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            12.0.0.0/6           reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            11.0.0.0/8           reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            8.0.0.0/7            reject-with icmp-port-unreachable                          < ADDED RULE
> REJECT     all  --  0.0.0.0/0            0.0.0.0/5            reject-with icmp-port-unreachable                          < ADDED RULE
> ACCEPT     tcp  --  0.0.0.0/0            0.0.0.0/0            destination IP range 10.0.52.0-10.0.55.255 tcp dpt:6379
> ACCEPT     tcp  --  0.0.0.0/0            0.0.0.0/0            destination IP range 10.0.52.0-10.0.55.255 tcp dpt:3306
> ACCEPT     tcp  --  0.0.0.0/0            0.0.0.0/0            destination IP range 10.0.52.0-10.0.55.255 tcp dpt:5432
> ACCEPT     udp  --  0.0.0.0/0            0.0.0.0/0            destination IP range 0.0.0.0-255.255.255.255 udp dpt:53
> ACCEPT     tcp  --  0.0.0.0/0            0.0.0.0/0            destination IP range 0.0.0.0-255.255.255.255 tcp dpt:53
> ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0            destination IP range 192.169.0.0-255.255.255.255
> ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0            destination IP range 172.32.0.0-192.167.255.255
> ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0            destination IP range 169.255.0.0-172.15.255.255
> ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0            destination IP range 11.0.0.0-169.253.255.255
> ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0            destination IP range 0.0.0.0-9.255.255.255
> REJECT     all  --  0.0.0.0/0            0.0.0.0/0            reject-with icmp-port-unreachable

The structs for deny_networks are duplicated to not create a new
dependency between legacynet and lib, within the cni-wrapper-plugin

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Chris <chris.farmiloe@digital.cabinet-office.gov.uk>
@tlwr

tlwr commented Feb 11, 2020

Copy link
Copy Markdown
Contributor Author

I've squashed into two commits, each is atomic, and the second commit (3ff16b4) has the message with @KauzClay posted above :bowtie: 😄

Thanks folks for your guidance, help, and encouragement throughout this pull request! ❤️

@ameowlia
ameowlia merged commit 2a41cb6 into cloudfoundry:develop Feb 11, 2020
@ameowlia

Copy link
Copy Markdown
Member

I clicked the button! 🎉

(ps @adobley is visiting London in may and he's too scared to ask if you want to get a beer @tlwr so I'm posting this for him 🍺 🇬🇧 )

@tlwr

tlwr commented Feb 11, 2020

Copy link
Copy Markdown
Contributor Author

You’re the best, thanks both!

I’ll message you in CF slack @adobley

@tlwr
tlwr deleted the deny-networks branch February 11, 2020 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants