[jjo] add docker-ce 18.06.2 for CVE-2019-5736 #6460

jjo · 2019-02-11T19:29:56Z

Update CentOS, Debian Stretch and Ubuntu Bionic
docker-ce packages to 18.06.2

Fixes kubernetes#6459. * Update CoreOS, Debian Stretch and Ubuntu Bionic docker-ce packages to 18.06.2

k8s-ci-robot · 2019-02-11T19:30:04Z

Hi @jjo. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

jjo · 2019-02-11T19:33:27Z

FYI

$ git show 49615d5|sed -rn '/^[+].*Source:/s/.*(https:[^"]+).*/\1/p' > kops6459.list

# Checking checksums
$ < kops6459.list xargs -I@ sh -c 'echo "@ $(curl -Ls @ |shasum)"'
https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/amd64/docker-ce_18.06.2~ce~3-0~ubuntu_amd64.deb 9607c67644e3e1ad9661267c99499004f2e84e05  -
https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce_18.06.2~ce~3-0~debian_amd64.deb aad1efd2c90725034e996c6a368ccc2bf41ca5b8  -
https://download.docker.com/linux/centos/7/x86_64/stable/Packages/docker-ce-18.06.2.ce-3.el7.x86_64.rpm 456eb7c5bfb37fac342e9ade21b602c076c5b367  -

# Double checking pkg timestamps
$ < kops6459.list xargs -I@ sh -c 'echo "@ $(curl -ILs @ |egrep -i last-mod)"'
https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/amd64/docker-ce_18.06.2~ce~3-0~ubuntu_amd64.deb Last-Modified: Mon, 11 Feb 2019 18:11:26 GMT
https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce_18.06.2~ce~3-0~debian_amd64.deb Last-Modified: Mon, 11 Feb 2019 18:12:26 GMT
https://download.docker.com/linux/centos/7/x86_64/stable/Packages/docker-ce-18.06.2.ce-3.el7.x86_64.rpm Last-Modified: Mon, 11 Feb 2019 18:07:47 GMT

mikesplain

Thanks for the PR @jjo. I was opening up a similar one as well. I think for the moment, since this will likely be cherry-picked into a release branch, can we make this additive only and not remove any older versions?

Something like this should do it: https://github.com/kubernetes/kops/compare/master...mikesplain:add_docker?expand=1

mikesplain · 2019-02-11T19:42:36Z

nodeup/pkg/model/docker.go

@@ -525,50 +525,50 @@ var dockerVersions = []dockerVersion{

 	// 18.03.1 - Bionic
 	{
-		DockerVersion: "18.03.1",


Can we leave all the existing versions and add these new versions instead?

Thanks @mikesplain for the prompt feedback, added 18.06.2 instead
of replacing existing entries, PTAL.

mikesplain · 2019-02-11T19:47:06Z

Our docker version is actually defined here:

kops/pkg/model/components/docker.go

Lines 56 to 57 in 1cc14e2

    
           if sv.Major == 1 && sv.Minor >= 9 { 
        
           	dockerVersion = "17.03.2"

We'll need to decide if we want to point that at the newer version of docker. I feel we should just allow users to set the docker version for now, suggest they set Docker.Version to the 18.* version and change the default at a later time. Either way I'd suggest we do that in a separate PR.

mikesplain · 2019-02-11T19:47:27Z

/ok-to-test

mikesplain · 2019-02-11T19:47:42Z

/assign

mikesplain · 2019-02-11T19:51:19Z

We're currently limited in enforcing this because k/k 1.11 is only verified on 17.03.x. See https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.11.md#external-dependencies

rdrgmnzs · 2019-02-11T19:52:09Z

+1 on @mikesplain comment of not removing docker version and instead just adding new versions. Also let the users have to select the version and force and update at a later time.

mikesplain · 2019-02-11T20:04:58Z

e2e tests are broken because of aws right now.

gwohletz · 2019-02-11T22:27:01Z

Is there a reason ubuntu xenial is not being updated as well?

mikesplain

/lgtm
/approve

We should likely cherry-pick this into 1.11 and cut a patch release @justinsb

k8s-ci-robot · 2019-02-11T22:27:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jjo, mikesplain

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:

~~OWNERS~~ [mikesplain]

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

mikesplain · 2019-02-11T22:30:43Z

Good point @gwohletz, @jjo mind adding xenial too?

mikesplain · 2019-02-11T22:30:47Z

/hold

… Xenial, Bionic, Stretch)

zparnold · 2019-02-12T05:48:59Z

@jjo I thought the patched version of Docker is 18.09.2? https://github.com/docker/docker-ce/releases/tag/v18.09.2

Nevermind, just saw the other release. Sorry! 😅

zparnold · 2019-02-12T06:05:17Z

/test pull-kops-e2e-kubernetes-aws

mikesplain · 2019-02-12T13:29:55Z

FYI these e2e tests will likely continue to fail due to an issue with the aws account. We'll force merge it when we're ready. Thanks for the patience all.

zparnold · 2019-02-12T13:36:28Z

@mikesplain I was about to say the same! 😄 Looking at the test fail it seems account related. Is there someone at the CNCF we can contact for resolution in the account? Obv, higher priority goes to merging this fix.

aledbf · 2019-02-12T13:38:38Z

@zparnold please check kubernetes/kubernetes#73444 (comment)

mikesplain · 2019-02-12T13:47:45Z

Also added jessie patch here, should be merged after this one: #6461

eyalzek · 2019-02-12T13:50:44Z

Great work! Could you consider adding the rest of the patched version as mentioned here: #6459 (comment) ?

mikesplain · 2019-02-12T13:53:58Z

@eyalzek those are enterprise versions that we can't support as far as I know and I don't believe they're patching the older community editions 😞

eyalzek · 2019-02-12T15:19:06Z

@mikesplain ah damn I didn't realize that!

gaganapplatix · 2019-02-12T18:28:23Z

Any chance the new rancher binary for older versions of docker (specifically 17.03.2-ce) going to be supported (#6459 (comment)) or should we start testing with 18.06.2-ce for our clusters (considering we were still on the supported 17.03 versions before)?

zparnold · 2019-02-13T06:26:35Z

@zparnold please check kubernetes/kubernetes#73444 (comment)

Thanks! @aledbf

ivnilv · 2019-02-13T11:53:10Z

Hope we can get this merged in master soon :) 👍

chetanv-oi · 2019-02-13T14:09:56Z

Do the tests include spinning up GPU nodes as well. I am wondering if this approach - https://github.com/kubernetes/kops/tree/master/hooks/nvidia-device-plugin - will still work.

@dcwangmit01

elisiano · 2019-02-14T06:37:59Z

could someone let us know what's the timeline before this lands in a patch release?

please don't read my question as snarky, I'm genuinely asking because I would like to avoid workarounds like the ones listed in #6459 (with hooks) but at the same time, if this is gonna take a couple of weeks before released I will need to mitigate the threat on my clusters somehow.

thanks 👍

captainkerk · 2019-02-14T18:09:26Z

Honestly, I think using rancher's patched docker-runc is the best way to patch for this CVE, at least until kops supports kubernetes v1.12. The solution proposed in this PR is to use a version of docker that hasnt been verified with any version of kubernetes that kops supports. v1.12 is the first version to support docker 18.06.

It would be nice if kops could just use rancher's patched version of docker-runc (or maybe build your own) without us having to use a hook. To me, that seems like an interesting patch vector that should be considered?

zetaab · 2019-02-15T03:10:21Z

/retest

castroth

There are some errors on the CentOS configuration. To reproduce it:

Create the cluster
Let initial configurations finish and all PODs are running
Restart a Node

When the kops-configuration service kicks in, due to the issue I mentioned about container-seliniux package name and version, it will try to reinstall. As it is already installed rpm will fail with status code of 1 making the node unavailable

castroth · 2019-02-15T16:15:51Z

nodeup/pkg/model/docker.go

+	// 18.06.2 - CentOS / Rhel7 (two packages)
+	{
+		DockerVersion: "18.06.2",
+		Name:          "container-selinux-2",


The name of the package is actually container-selinux. If you let it as it is kops-configuration will keep retrying to install and will fail causing the Node to be lost.

Fixed in #6492. We actually get away with it because we don't double-check the package installed what we expected, but I started to fix it up in #6490

castroth · 2019-02-15T16:16:28Z

nodeup/pkg/model/docker.go

+		Name:          "container-selinux-2",
+		Distros:       []distros.Distribution{distros.DistributionRhel7, distros.DistributionCentos7},
+		Architectures: []Architecture{ArchitectureAmd64},
+		Version:       "18.06.2.ce",


Here you should have the container-selinux version 2.68. If you let it as it is kops-configuration will keep retrying to install and will fail causing the Node to be lost.

Thanks for spotting - addressed in #6492

zetaab · 2019-02-16T09:01:02Z

/test pull-kops-e2e-kubernetes-aws

Raffo · 2019-02-18T15:36:50Z

Is there going to be a new release of kops 1.10 / 1.11 with those changes? I think it's critical enough to be backported and it deserves a release.
Also, is there anyone already running those changes?

/cc @justinsb

bzuelke · 2019-02-18T16:34:58Z

Is there going to be a new release of kops 1.10 / 1.11 with those changes? I think it's critical enough to be backported and it deserves a release.
Also, is there anyone already running those changes?

/cc @justinsb

I agree with this 100%. It's awesome that it's been merged into master but what does that exactly mean for the release it's actually going to be available on? Don't really have a roadmap for kops 1.12 for this to be a thing. Would be great for a new release of 1.11.1 or something or make it back ported if possible. Thanks as always though :)

justinsb · 2019-02-20T17:40:33Z

Yes - we need to figure out what the right action is for 1.11. The choices seem to basically be "hotfix runc" or "run with a newer (maybe untested with 1.11) version of docker". Leaning towards the newer docker, but going to ask slack...

ghost · 2019-02-21T07:31:29Z

@justinsb For me the minimum requirement for kops 1.11 is to allow choosing a fixed (but not officially supported) version of Docker (see PR #6493). Whether or not that should be done by default is another question entirely, and there some risk analysis needs to happen.

elisiano · 2019-02-22T10:14:30Z

@justinsb although I would prefer the updated docker version as well I'm afraid this is gonna be delayed further given 18.06.2 did not actually address the runc problem on redhat-based OSes (moby/moby#38739 (comment)).

What's a bit troubling (for me) in this situation is that users are left without a clue as when there will be a kops release that is gonna address the issue.

This is now out for a week and every day I was checking the progress of this.

Could we have some visibility on the plan? ie: "this is gonna be released next week as 1.11.1" or "go ahead and patch your clusters with workarounds/hooks because this is not happening for X weeks".
I would even settle for a 1.12 release 🤣(pushing my luck here).

[jjo] update docker-ce 18.06 for CVE-2019-5736

49615d5

Fixes kubernetes#6459. * Update CoreOS, Debian Stretch and Ubuntu Bionic docker-ce packages to 18.06.2

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 11, 2019

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 11, 2019

k8s-ci-robot requested review from rdrgmnzs and robinpercy February 11, 2019 19:30

mikesplain suggested changes Feb 11, 2019

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 11, 2019

k8s-ci-robot assigned mikesplain Feb 11, 2019

add 18.06.2 entries instead of replacing 18.06.1 ones

b761a80

jjo changed the title ~~[jjo] update docker-ce 18.06 for CVE-2019-5736~~ [jjo] add docker-ce 18.06.2 for CVE-2019-5736 Feb 11, 2019

mikesplain approved these changes Feb 11, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 11, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 11, 2019

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 11, 2019

add Xenial, fix Stretch Version to 18.06.2~ce~3-0~debian (same as for…

6bb897d

… Xenial, Bionic, Stretch)

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 11, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 12, 2019

mikesplain mentioned this pull request Feb 12, 2019

Add jessie patch #6461

Merged

3h4x mentioned this pull request Feb 12, 2019

Docker Vulnerability CVE-2019-5736 #6459

Closed

drekle approved these changes Feb 13, 2019

View reviewed changes

castroth suggested changes Feb 15, 2019

View reviewed changes

k8s-ci-robot merged commit f69bbd7 into kubernetes:master Feb 16, 2019

giovadifiore mentioned this pull request Feb 17, 2019

Kops patch including new Docker version for CVE-2019-5736 #6476

Closed

k8s-ci-robot requested a review from justinsb February 18, 2019 15:36

[jjo] add docker-ce 18.06.2 for CVE-2019-5736 #6460

[jjo] add docker-ce 18.06.2 for CVE-2019-5736 #6460

Conversation

jjo commented Feb 11, 2019 • edited Loading

k8s-ci-robot commented Feb 11, 2019

jjo commented Feb 11, 2019

mikesplain left a comment • edited Loading

Choose a reason for hiding this comment

mikesplain Feb 11, 2019

Choose a reason for hiding this comment

jjo Feb 11, 2019

Choose a reason for hiding this comment

mikesplain commented Feb 11, 2019

mikesplain commented Feb 11, 2019

mikesplain commented Feb 11, 2019

mikesplain commented Feb 11, 2019

rdrgmnzs commented Feb 11, 2019

mikesplain commented Feb 11, 2019

gwohletz commented Feb 11, 2019

mikesplain left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 11, 2019

mikesplain commented Feb 11, 2019

mikesplain commented Feb 11, 2019

zparnold commented Feb 12, 2019 • edited Loading

zparnold commented Feb 12, 2019

mikesplain commented Feb 12, 2019

zparnold commented Feb 12, 2019

aledbf commented Feb 12, 2019

mikesplain commented Feb 12, 2019

eyalzek commented Feb 12, 2019

mikesplain commented Feb 12, 2019

eyalzek commented Feb 12, 2019

gaganapplatix commented Feb 12, 2019

zparnold commented Feb 13, 2019

ivnilv commented Feb 13, 2019

chetanv-oi commented Feb 13, 2019

elisiano commented Feb 14, 2019

captainkerk commented Feb 14, 2019

zetaab commented Feb 15, 2019

castroth left a comment

Choose a reason for hiding this comment

castroth Feb 15, 2019

Choose a reason for hiding this comment

justinsb Feb 20, 2019

Choose a reason for hiding this comment

castroth Feb 15, 2019

Choose a reason for hiding this comment

justinsb Feb 20, 2019

Choose a reason for hiding this comment

zetaab commented Feb 16, 2019

Raffo commented Feb 18, 2019

bzuelke commented Feb 18, 2019

justinsb commented Feb 20, 2019

ghost commented Feb 21, 2019

elisiano commented Feb 22, 2019 • edited Loading

jjo commented Feb 11, 2019 •

edited

Loading

mikesplain left a comment •

edited

Loading

zparnold commented Feb 12, 2019 •

edited

Loading

elisiano commented Feb 22, 2019 •

edited

Loading