Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions
On 3/10/2011 10:30 PM, Stefan Hajnoczi wrote:
> On Fri, Mar 11, 2011 at 5:54 AM, Venkateswararao Jujjuri (JV)
> wrote:
>> On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
>>> On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar wrote:
Add chroot functionality for systemcalls that can operate on a file
using relative directory file descriptor.
>>>
>>> I suspect the relative directory approach is broken and escapes the
>>> chroot. Here's why:
>>>
>>> The request is local_chmod(fs_ctx, "/..", credp). dirname("/..") is
>>> "/" and basename("..") is "..".
>>
>> We should never receive protocol operations with relative path.
>> Client should always resolve to full path and send the request.
>> If the client is malicious this scenario can be be possible.. but in that
>> case
>> it is fine to fail the operation.
>
> What I haven't audited yet is whether symlinks can be abused in any of
> these *at(2) operations.
Reading symlink sends only contents to the client. So a symlink can contain
anything.
But when the fully populated path comes we avoid the potential symlink issue by
opening
the entire dir in chrooted process.
>
> The *at(2) approach seems like a shortcut to avoid implementing
> individual chroot protocol requests/responses for stat(2) and friends.
> But it carries the risk that if we don't use NOFOLLOW then we can be
> tricked into escaping the "chroot" because we're performing the
> operation outside the chroot.
I would agree with your observation. With *at(2) we need the following:
1. The path should never have ".." May be we can check it early enough and fail.
2. Get the pfd from the chroot_thread
3. use NO_FOLLOW.
If we do all three then we are fool prof.
>
> I'll take a look later today to make sure all operations safe traverse
> paths outside the chroot.
If we move all the operations to chroot, we are essentially serializing all
operations.
But the code looks lot cleaner though.
- JV
>
> Stefan
>
Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions
On Fri, Mar 11, 2011 at 5:54 AM, Venkateswararao Jujjuri (JV)
wrote:
> On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
>> On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar wrote:
>>> Add chroot functionality for systemcalls that can operate on a file
>>> using relative directory file descriptor.
>>
>> I suspect the relative directory approach is broken and escapes the
>> chroot. Here's why:
>>
>> The request is local_chmod(fs_ctx, "/..", credp). dirname("/..") is
>> "/" and basename("..") is "..".
>
> We should never receive protocol operations with relative path.
> Client should always resolve to full path and send the request.
> If the client is malicious this scenario can be be possible.. but in that case
> it is fine to fail the operation.
What I haven't audited yet is whether symlinks can be abused in any of
these *at(2) operations.
The *at(2) approach seems like a shortcut to avoid implementing
individual chroot protocol requests/responses for stat(2) and friends.
But it carries the risk that if we don't use NOFOLLOW then we can be
tricked into escaping the "chroot" because we're performing the
operation outside the chroot.
I'll take a look later today to make sure all operations safe traverse
paths outside the chroot.
Stefan
Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions
On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
> On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar wrote:
>> Add chroot functionality for systemcalls that can operate on a file
>> using relative directory file descriptor.
>
> I suspect the relative directory approach is broken and escapes the
> chroot. Here's why:
>
> The request is local_chmod(fs_ctx, "/..", credp). dirname("/..") is
> "/" and basename("..") is "..".
We should never receive protocol operations with relative path.
Client should always resolve to full path and send the request.
If the client is malicious this scenario can be be possible.. but in that case
it is fine to fail the operation.
Thanks,
JV
> I'm not 100% sure of the semantics but I suspect that chmodat(dir_fd,
> "..", ...) does not honor the chroot since your current task is not
> inside the chroot. If so, then you can manipulate the parent
> directory of the chroot using some of the operations added in this
> patch.
>
> The safe solution is to perform all operations inside the chroot.
> This will require extending the chroot socket protocol.
>
> Stefan
>
[Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions
On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar wrote:
> Add chroot functionality for systemcalls that can operate on a file
> using relative directory file descriptor.
I suspect the relative directory approach is broken and escapes the
chroot. Here's why:
The request is local_chmod(fs_ctx, "/..", credp). dirname("/..") is
"/" and basename("..") is "..".
I'm not 100% sure of the semantics but I suspect that chmodat(dir_fd,
"..", ...) does not honor the chroot since your current task is not
inside the chroot. If so, then you can manipulate the parent
directory of the chroot using some of the operations added in this
patch.
The safe solution is to perform all operations inside the chroot.
This will require extending the chroot socket protocol.
Stefan
