Skip to content

Conversation

@dcbw
Copy link
Contributor

@dcbw dcbw commented Mar 18, 2019

Update CI and other infrastructure to use the CNI plugins v0.7.5 to pick up fixes to portmap and other plugins. @thockin @bboreham @squeed @philips

/kind bug

NONE

Update from Brandon @philips of the Kubernetes Security Commitee:

A security issue was discovered with interactions between the CNI (Container Networking Interface) portmap plugin versions prior to 0.7.5 and Kubernetes. The CNI portmap plugin is embedded into Kubernetes releases so new releases of Kubernetes are required to fix this issue. The issue is Medium and upgrading to Kubernetes 1.11.9, 1.12.7, 1.13.5, and 1.14.0 is encouraged to fix this issue if this plugin is used in your environment.

Am I vulnerable?

As this affects a Kubernetes plugin interface it is difficult to say with certainty without a complete understanding of your Kubernetes configuration. The issue was identified in a configuration of kube-proxy in IPVS mode along with a pod using a HostPort. However, other network configurations may use the CNI portmap plugin as well.

Run kubectl version --short | grep Server and if it does not say 1.11.9, 1.12.7, 1.13.5, and 1.14.0 or newer you are running a vulnerable version if paired with a CNI configuration that uses the portmap plugin.

How do I upgrade?

Follow your management tool or vendor instructions to upgrade to the latest release of Kubernetes.

Vulnerability Details

Before this fix the 'portmap' plugin, used to setup HostPorts for CNI, would insert rules at the front of the iptables nat chains; which would take precedence over the KUBE- SERVICES chain. Because of this, the HostPort/portmap rule could match incoming traffic even if there were better fitting, more specific service definition rules like NodePorts later in the chain.

Switching the portmap plugin to append its rules, rather than prepend, allows traffic to be processed by KUBE-SERVICES rules first. Only if traffic does not match a service will it be considered for HostPorts. This is compatible with the behavior of the legacy ‘kubenet’ network driver.

See the GitHub issue for details. #75455 and containernetworking/plugins#269

Thank you

Thank you to Etienne Champetier of Anevia for identifying the issue, Tim Hockin, Dan Williams, Casey Callendrello, Dujun, Tim Pepper, and the patch release managers for the coordination is making this release.

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/release Categorizes an issue or PR as relevant to SIG Release. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 18, 2019
@k8s-ci-robot k8s-ci-robot requested review from gmarek and jbeda March 18, 2019 21:34
@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 18, 2019
@spiffxp
Copy link
Contributor

spiffxp commented Mar 18, 2019

/milestone v1.14
/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Mar 18, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.14 milestone Mar 18, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Mar 18, 2019
@spiffxp
Copy link
Contributor

spiffxp commented Mar 18, 2019

/approve
for cluster and test

@thockin
Copy link
Member

thockin commented Mar 18, 2019

I'm OK with updating this. I am a little dismayed at the state of automation around this. Maybe a 2019 effort to clean this up is in order :)

I'll approve now, but let's keep a close eye on tests, maybe force them to re-run a few times just to be sure.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, spiffxp, thockin

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

The pull request process is described here

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2019
@spiffxp
Copy link
Contributor

spiffxp commented Mar 18, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 18, 2019
@dcbw
Copy link
Contributor Author

dcbw commented Mar 18, 2019

@thockin should we do-no-merge/hold until we soak it a couple times? I don't recall the exact / incantation, and I can't manually add/remove labels through github for some reason, so I'll leave that determination to you...

@k8s-ci-robot k8s-ci-robot merged commit 39277cd into kubernetes:master Mar 18, 2019
@dcbw
Copy link
Contributor Author

dcbw commented Mar 18, 2019

OK, I guess we missed the do-no-merge/hold window to soak this a couple times... yay for CNI API stability? :)

@k8s-ci-robot
Copy link
Contributor

@dcbw: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e bfd8ad3 link /test pull-kubernetes-local-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.

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.

@tpepper
Copy link

tpepper commented Mar 19, 2019

This seems like a release note of "bump CNI version to 0.7.5" would have been in order.

@neolit123
Copy link
Member

@dcbw why are we backporting this bump?
if it's not a critical bug fix (e.g. panics, security fixes etc), we shouldn't backport such bumps.

cc @timothysc

@ncdc
Copy link
Member

ncdc commented Mar 20, 2019

@neolit123 this is against the master branch so it's not a backport?

@bboreham
Copy link
Contributor

Also it includes a security fix.

@neolit123
Copy link
Member

@ncdc there are some xref-ed cherry pick PRs above.

@bboreham

Also it includes a security fix.

it was not outlined in the PR.
where can we read more about it?

@bboreham
Copy link
Contributor

(a) I can't find the CVE and (b) I'm fairly sure it's embargoed. Sorry.

@timothysc
Copy link
Contributor

If it is a CVE then there would also be a cherry-pick.

@neolit123
Copy link
Member

ok, in that case we should bump this in the remaining locations - e.g. k8s debs and rpms.

k8s-ci-robot added a commit that referenced this pull request Mar 20, 2019
…5-upstream-release-1.11

Automated cherry pick of #75455: build/gci: bump CNI version to 0.7.5
k8s-ci-robot added a commit that referenced this pull request Mar 21, 2019
…5-upstream-release-1.13

Automated cherry pick of #75455: build/gci: bump CNI version to 0.7.5
k8s-ci-robot added a commit that referenced this pull request Mar 21, 2019
…5-upstream-release-1.12

Automated cherry pick of #75455: build/gci: bump CNI version to 0.7.5
justinsb added a commit to justinsb/kops that referenced this pull request Mar 26, 2019
justinsb added a commit to justinsb/kops that referenced this pull request Mar 26, 2019
justinsb added a commit to justinsb/kops that referenced this pull request Mar 28, 2019
@philips philips changed the title build/gci: bump CNI version to 0.7.5 build/gci: bump CNI version to 0.7.5 - CVE-2019-9946 Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/security area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants