On 6/25/19 2:08 AM, Paul Gevers wrote: > Hi Shengjing, > > On 24-06-2019 00:28, Shengjing Zhu wrote: >> Now, with good reason... >> >> It tooks me enough hours today to figure out why the tests crash the host(as >> described in #929662, running out of pids). >> >> The bug is not from upstream. Previously a file was removed from >> upstream tarball, named engine/pkg/chrootarchive/archive_test.go, which >> has an important init func: >> >> func init() { >> reexec.Init() >> } >> >> All tests that rely on reexec need this func. The tests added by >> CVE-2018-15664 >> need it as well. Without this, the tests cause fork bomb. > Are you saying this file is only needed for testing? This file isn't > needed for docker.io itself? Why was it stripped in the first place?
The file `archive_test.go` contains tests that require root, hence have to be excluded from the test suite. In this case it was more convenient to just get rid of the file in debian/clean rather than to patch every tests that are in the file. As it turns out, it also contains helpers and functions that are used by other test files in the same directory, that's why we run into this issue. Test files are not involved in the build of the executable, so no, this file is not needed for docker.io itself. >> Well, after adding this func back, the tests run and the host doesn't >> crash. >> >> However the tests still can't pass in schroot, the log says: > [...] > >> Short version: these tests need privileged permission. > And your schroot doesn't provide those. How about any better container? > How about buildds? So today I just build the package in a container with root permission (also re-enabling the file engine/pkg/chrootarchive/archive_test.go that was removed in debian/clean). I can confirm that the test suite runs successfully: === RUN TestUntarWithMaliciousSymlinks --- PASS: TestUntarWithMaliciousSymlinks (0.10s) === RUN TestTarWithMaliciousSymlinks === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe_host-file === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe/_host-file === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe_ === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe/_ === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root_safe/host-file === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root_/safe/host-file === RUN TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root_ --- PASS: TestTarWithMaliciousSymlinks (0.02s) archive_unix_test.go:93: /tmp/TestTarWithMaliciousSymlinks466022653 --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe_host-file (0.00s) --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe/_host-file (0.00s) --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe_ (0.00s) --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root/safe/_ (0.00s) --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root_safe/host-file (0.00s) --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root_/safe/host-file (0.00s) --- PASS: TestTarWithMaliciousSymlinks//tmp/TestTarWithMaliciousSymlinks466022653/root_ (0.00s) PASS ok github.com/docker/docker/pkg/chrootarchive 3.262s As far as I can tell, the upload from Shengjing Zhu is correct and it fixes the CVE, which is the point of this unblock request. I agree that we could improve the testsuite regarding root tests. The right way to handle these tests is to check if the user is root, and bail out otherwise. It's done here and there in docker codebase, but there's always some places where it's not done, and we have to patch that to disable those tests for Debian. These patches could actually be re-worked and upstreamed. That's on my todolist. But that is out of scope of this unblock request. Best regards, Arnaud