Fix docker cp issues BZ1723491 & BZ1717087

[TomSweeneyRedHat]

Signed-off-by: TomSweeneyRedHat tsweeney@redhat.com

- What I did
There are two BZs https://bugzilla.redhat.com/show_bug.cgi?id=1723491 and https://bugzilla.redhat.com/show_bug.cgi?id=1717087 reporting an issue using docker cp to a container. Upon testing, further issues were discovered if the target in the container was a symlink. This code finds the target directory on the host's local container storage (/var/lib/docker/overlay2), resolves any symlinks and ensures the file gets copied to/from the container without any potential cve leakage.

- How I did it
VI and lots of blood, sweat and tears for this one.

- How to verify it

# docker run -t -i --name testctr --rm fedora bash

//Inside the container:
# ln -s /tmp /test

//Outside the container:
# touch testfile; docker cp testfile testctr:/test

//In the container,
# ls /tmp

// Outside the container,
# ls /tmp

//The file "testfile" should be shown in the ls within the container, but NOT with the ls outside the container.

# docker cp testctr:/test/testfile.txt  /tmp/newtestfile.txt
// /tmp on the host should have the file newtestfile.txt

- Description for the changelog
Addresses docker cp issue noted in BZ1717087 and BZ1723491

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

[TomSweeneyRedHat]

This is a WIP until I can have @edsantiago run tests on it. Please review prior, I believe it's gtg.

[TomSweeneyRedHat]

Follow on PR for #341 #342 and completes #347

[giuseppe]

should we ensure that after the Join the path is still under container.BaseFS?

Could happen that resolvedCtrDir has .. components, so it can potentially point outside of the basefs once we do the join?

[TomSweeneyRedHat]

I played with it tonight, and excellent spot. I think I've a solution for that, testing now.

[TomSweeneyRedHat]

Actually, I played more and add a symlink in the container like: ln -s /tmp ../../../../../../testy and that just resolved to the root directory of the container. However I did come up with a quick check that will make it completely safe, so I've added it.

[rhatdan]

@TomSweeneyRedHat I am confused by this, is this ready Now?

[TomSweeneyRedHat]

@rhatdan yes, ready to go given LGTM's which I don't have any atm.

[giuseppe]

LGTM

[vrothberg]

LGTM
Commit cec24f5 is still marked as WIP though.

[giuseppe]

this was fixed by another PR. Let's close it

[rhatdan]

@giuseppe So should we close this PR?

[TomSweeneyRedHat]

I think #353 by @giuseppe takes care of this. @giuseppe can you confirm?

[TomSweeneyRedHat]

I've looked through #353 from @giuseppe and have tested with it. It does appear to replace this one and solves a wart that this PR had in a specific pull sequence. I'm going to close this and we can resurrect it if we find out it's not necessary.