Change current working directory after chroot

[legionus]

The syscall.Chroot does not change the current working directory, so that after the call '.' can be outside the tree rooted at '/'. In particular, the user can escape from a "chroot jail".

See chroot(2), openat(2) manpages.

[simon3z]

@enoodle @ilackarms can you review? Thanks.

[ilackarms]

I'm not sure this change is necessary. The purpose of chroot IIUC is to prevent the webdav server from exposing the content of the host filesystem to webdav clients when absolute-path symlinks exist inside the container:

log.Printf("!!!WARNING!!! It is insecure to serve the image content without changing")
log.Printf("root (--chroot). Absolute-path symlinks in the image can lead to disclose")
log.Printf("information of the hosting system.")

we're not chrooting in order to protect the host system from the image-inspector process. I haven't tested this manually, but this will certainly affect any exec calls we make later in Image inspector (which we do in the case of OpenSCAP and ClamAV scanners).

is there a reason for which we would want to make this change?

[enoodle]

@ilackarms This is happening in the image server, after all the scanning was done. IIUC It is like another line of defense against an unknown attack that will go through the golang ~http~ webdav server and gain access to our process or something. Doesn't hurt to have.

[ilackarms]

@enoodle I still don't see the justification for it. It's not a practice I'm familiar with for HTTP servers to chroot + chdir to the root of the static content they serve.

My concern here is

• adding unnecessary code that could confuse future readers
• introducing technical debt (imagine being unaware of this chdir taking place, and running an exec some time after it somewhere else in the code. I imagine that would be pretty hard to debug!)

[mfojtik]

this is a security enhancement where once the webdav is running to server results you won't be able to stream the host filesystem files.

[legionus]

@mfojtik Actually, it's not a security enhancement. When someone call chroot, he must also call the chdir, because otherwise insulation will not be complete and the process will still have access to the entire filesystem.

[ilackarms]

@legionus i think the purpose is not to protect the filesystem from the process, but to protect the filesystem from webdav clients (by hiding files that might be exposed by absolute symlinks)

[legionus]

@ilackarms You can't protect the filesystem from webdav clients on 100% if you don't call chdir.

[ilackarms]

@pweil- thoughts on this?

[php-coder]

Just to show that in some cases it can lead to a real security holes: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5110

[pweil-]

@pweil- thoughts on this?

Based on my knowledge a chroot jail is not considered a security feature of the kernel (as specifically noted in the man page) and there are known issues with chroot when directories move.

That said, doing the chdir doesn't hurt and I would generally think it's better to try to be "safer".

adding unnecessary code that could confuse future readers

I think we can document well to mitigate this

introducing technical debt (imagine being unaware of this chdir taking place, and running an exec some time after it somewhere else in the code. I imagine that would be pretty hard to debug!)

this is interesting though. I would guess that relative commands are not the best strategy anyway but that doesn't mean someone won't use them. @legionus anything we can do to help with this?

[ilackarms]

@pweil- sounds reasonable. @legionus would you mind log output to alert the user that image inspector is performing a chdir here?

[legionus]

this is interesting though. I would guess that relative commands are not the best strategy anyway but that doesn't mean someone won't use them. @legionus anything we can do to help with this?

@pweil- No. If you decide to be inside chroot, you should not be able to invoke a program outside it.

Apparently my and your views on chroot(2) differ dramatically. If you cannot convince even the manpage, then I see no reason to change that.

[pweil-]

If you cannot convince even the manpage, then I see no reason to change that.

😄 this does not mean it isn't a good practice. Point is that we shouldn't consider this secure or a security feature - just something that makes it more difficult to mess things up.

So that said, why close this? It benefits us in that we're following the right steps to prevent exposure of host files. I see no reason why this shouldn't move forward.

Reopening

[legionus]

@pweil- I don't think there is a need for any messages. I brought the program to the expected behavior. If I specified --chroot option, then then obviously I lost the ability to call a programs outside of the chroot. If you want to run something outside of the host, this is should be done before the chroot(2). All programs that uses chroot(2) doing so.

[simo5]

While chroot is not a complete namespace isolation and therefore not a security feature it can be used for defense in depth. But that will only be true if you are sure to chdir inside the chroot. Chrooting without chdir is like waterproofing all your windows but leaving the door open.

I also do not think any warning is needed, I would like to think every developer would understand that exec commands trying to call outside the chroot are expected to fail.

[ilackarms]

I don't think there is a need for any messages. I brought the program to the expected behavior. If I specified --chroot option, then then obviously I lost the ability to call a programs outside of the chroot.

Since the user has to specifically request chroot, it makes sense that we don't don't need to log this behavior.