projectatomic / docker

Docker - the open-source application container engine
http://www.docker.com
Apache License 2.0
81 stars 58 forks source link

CP cve fix part 2 (CVE-2018-15664) #342

Closed TomSweeneyRedHat closed 5 years ago

TomSweeneyRedHat commented 5 years ago

- What I did Touched up a segv for the definition of tar and added a root dir to two other tar commands that were necessary upon further review/testing.

- How I did it vi and a lot of blood, sweat and tears.

- How to verify it Full Docker conformance testing. Honestly I need to figure out how to test it fully myself.

- Description for the changelog Fix for CVE-2018-15664

- A picture of a cute animal (not mandatory but encouraged)

TomSweeneyRedHat commented 5 years ago

Follow up for #341

TomSweeneyRedHat commented 5 years ago

@nalind @vrothberg PTAL at this and #341. @edsantiago found a one second delay in docker ps over the latest in RHEL, but I used the prior commit to test and it was there too. Plus I don't think any of these changes are called by the ps command.

All other tests passed per Ed and this fixes the symlink cve issue.

mheon commented 5 years ago

Code changes LGTM to me

mrunalp commented 5 years ago

@rhatdan ptal

TomSweeneyRedHat commented 5 years ago

@nalind sent a couple of comments in IRC that I'm looking into ATM. Please hold on the merge until I finish looking.

the use of filepath.Base(absPath) as a chroot in daemon/archive.go:155 looks a little odd, but i'm not super-familiar with the code

a 'make help' at the top-level directory should list various targets that can be used to invoke some in-tree self-tests
TomSweeneyRedHat commented 5 years ago

I've reviewed @nalind 's comments, I think this is good to go as is. Removing the WIP.