Skip to content

Add calico GlobalNetworkPolicy to block traffic to 168.63.129.16#1682

Closed
CecileRobertMichon wants to merge 2 commits into
kubernetes-sigs:mainfrom
CecileRobertMichon:calico-block-ws
Closed

Add calico GlobalNetworkPolicy to block traffic to 168.63.129.16#1682
CecileRobertMichon wants to merge 2 commits into
kubernetes-sigs:mainfrom
CecileRobertMichon:calico-block-ws

Conversation

@CecileRobertMichon
Copy link
Copy Markdown
Contributor

@CecileRobertMichon CecileRobertMichon commented Sep 11, 2021

What type of PR is this?

What this PR does / why we need it: This PR explicitly ensures that TCP traffic bound for the reserved Azure IP 168.63.129.16 is blocked from containers to remediate https://msrc.microsoft.com/update-guide/en-US/vulnerability/CVE-2021-27075.

This is done in addition to kubernetes-sigs/image-builder#690.

Future work: determine if and how to do this for other CNI specs in the repo, IPv6 Calico, and Windows Flannel.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

Add calico GlobalNetworkPolicy to block traffic to 168.63.129.16 to remediate https://msrc.microsoft.com/update-guide/en-US/vulnerability/CVE-2021-27075

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 11, 2021
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from cecilerobertmichon after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the area/provider/azure Issues or PRs related to azure provider label Sep 11, 2021
@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Sep 11, 2021
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

/cc @devigned

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

/retest

@devigned
Copy link
Copy Markdown
Contributor

What do you think about calling out the CVE in release notes?

@devigned
Copy link
Copy Markdown
Contributor

/retest

1 similar comment
@devigned
Copy link
Copy Markdown
Contributor

/retest

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

@devigned the network policy e2e test is failing, it seems the problem is that we're applying a k8s network policy in one namespace to block egress and that is being overridden by our global allow calico policy. I'm trying to find documentation about the official order of precedence when both a calico net pol and a k8s net pol apply but couldn't find anything explicit. Any ideas on how we can get around this?

@nader-ziada
Copy link
Copy Markdown
Contributor

@devigned the network policy e2e test is failing, it seems the problem is that we're applying a k8s network policy in one namespace to block egress and that is being overridden by our global allow calico policy. I'm trying to find documentation about the official order of precedence when both a calico net pol and a k8s net pol apply but couldn't find anything explicit. Any ideas on how we can get around this?

The order field controls the order of precedence. Calico applies the policy with the lowest value first.

https://github.com/projectcalico/api/blob/586917f350df582610dfe8c0ef44a3758506f001/pkg/apis/projectcalico/v3/globalnetworkpolicy.go#L54

https://docs.projectcalico.org/reference/resources/globalnetworkpolicy

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

The order field controls the order of precedence. Calico applies the policy with the lowest value first.

@nader-ziada this is only for Calico network policies. The k8s Network Policies (not the CRD, the native type) don't have an order and it's unclear how they are applied when there are also Calico Network Policies https://docs.projectcalico.org/security/calico-network-policy (Works seamlessly with Kubernetes network policies )

@nader-ziada
Copy link
Copy Markdown
Contributor

w they are applied when there are also Calico Network Policies

found this here: https://stackoverflow.com/questions/67006051/project-calico-priority-between-global-policy-and-network-policy


Global vs non-global is not a factor in deciding the order that policies are applied in. Ordering is determined by the "order" field on Calico NetworkPolicy and GlobalNetworkPolicy resources, with smaller "order" policies being applied first.

If not specified, "order" defaults to infinity, so policies with an unspecified "order" will be applied last.

Calico also implements the Kubernetes NetworkPolicy resource, which doesn't have an explicit "order" field. To order those against the Calico resources, we treat Kubernetes NetworkPolicy resource as though they had an implicit "order" of 1000.

There is a tie-breaker in the code for policies with the same order value, but you shouldn't need to know what that is, or rely on it, because it's better to use an explicit "order" value, whenever ordering matters.

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

@nader-ziada great find 🤯

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

trying with order 9999 so k8s native policies always take precedence

@nader-ziada
Copy link
Copy Markdown
Contributor

looks like it didn't work :(

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

that one looks like a different error though... progress?

I'll dig in shortly

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

/retest

just in case

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

/retest

provisioning error

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

so the issue is that NetworkPolicies (the k8s native ones) don't have a concept of "explicit deny" (this blog has a nice diagram to visualize if anyone is interested). It's either default allow (no policies on the pod), default-deny (at least one policy on the pod), or explicit allow. So by doing an explicit allow on all the pods with Calico, there is no way to do an explicit deny on top of it for some pods, via k8s network policy.. With Calico you can do:

apiVersion: crd.projectcalico.org/v1
kind: NetworkPolicy
metadata:
  name: backend-deny-ingress-calico
  namespace: development
spec:
  selector: role == 'backend'
  types:
  - Ingress
  order: 1000
  ingress: 
  - action: Deny

which does what we want.

Options to move forward:

  1. close this PR, rely on block traffic to 168.63.129.16 port 80 for cve-2021-27075 image-builder#690 to block traffic
  2. modify the test to use calico network policies
  3. modify the delete the calico default-allow policy before testing network policies, and document users do the same to apply network policies

@nader-ziada @devigned what do you think?

@nader-ziada
Copy link
Copy Markdown
Contributor

I think option 1; rely on image-builder#690 to block traffic is valid as long as we announce to users to pick up/generate new images

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@CecileRobertMichon: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Rerun command
pull-cluster-api-provider-azure-e2e 98cb290 link /test pull-cluster-api-provider-azure-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

/close

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@CecileRobertMichon: Closed this PR.

Details

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@CecileRobertMichon CecileRobertMichon deleted the calico-block-ws branch February 17, 2023 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants