Bug#930293: unblock: docker.io/18.09.1+dfsg1-7

[Arnaud Rebillout <arnaud.rebillout@collabora.com>]

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