On 9/2/2021 1:06 PM, Vivek Goyal wrote:
> On Thu, Sep 02, 2021 at 11:55:11AM -0700, Casey Schaufler wrote:
>> On 9/2/2021 10:42 AM, Vivek Goyal wrote:
>>> On Thu, Sep 02, 2021 at 01:05:01PM -0400, Vivek Goyal wrote:
>>>> On Thu, Sep 02, 2021 at 08:43:50AM -0700, Casey Schaufler wrote:
>>>>> On 9/2/2021 8:22 AM, Vivek Goyal wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This is V3 of the patch. Previous versions were posted here.
>>>>>>
>>>>>> v2:
>>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]/
>>>>>> v1:
>>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]
>>>>>> +m/
>>>>>>
>>>>>> Changes since v2
>>>>>> ----------------
>>>>>> - Do not call inode_permission() for special files as file mode bits
>>>>>> on these files represent permissions to read/write from/to device
>>>>>> and not necessarily permission to read/write xattrs. In this case
>>>>>> now user.* extended xattrs can be read/written on special files
>>>>>> as long as caller is owner of file or has CAP_FOWNER.
>>>>>>
>>>>>> - Fixed "man xattr". Will post a patch in same thread little later. (J.
>>>>>> Bruce Fields)
>>>>>>
>>>>>> - Fixed xfstest 062. Changed it to run only on older kernels where
>>>>>> user extended xattrs are not allowed on symlinks/special files. Added
>>>>>> a new replacement test 648 which does exactly what 062. Just that
>>>>>> it is supposed to run on newer kernels where user extended xattrs
>>>>>> are allowed on symlinks and special files. Will post patch in
>>>>>> same thread (Ted Ts'o).
>>>>>>
>>>>>> Testing
>>>>>> -------
>>>>>> - Ran xfstest "./check -g auto" with and without patches and did not
>>>>>> notice any new failures.
>>>>>>
>>>>>> - Tested setting "user.*" xattr with ext4/xfs/btrfs/overlay/nfs
>>>>>> filesystems and it works.
>>>>>>
>>>>>> Description
>>>>>> ===========
>>>>>>
>>>>>> Right now we don't allow setting user.* xattrs on symlinks and special
>>>>>> files at all. Initially I thought that real reason behind this
>>>>>> restriction is quota limitations but from last conversation it seemed
>>>>>> that real reason is that permission bits on symlink and special files
>>>>>> are special and different from regular files and directories, hence
>>>>>> this restriction is in place. (I tested with xfs user quota enabled and
>>>>>> quota restrictions kicked in on symlink).
>>>>>>
>>>>>> This version of patch allows reading/writing user.* xattr on symlink and
>>>>>> special files if caller is owner or priviliged (has CAP_FOWNER) w.r.t inode.
>>>>> This part of your project makes perfect sense. There's no good
>>>>> security reason that you shouldn't set user.* xattrs on symlinks
>>>>> and/or special files.
>>>>>
>>>>> However, your virtiofs use case is unreasonable.
>>>> Ok. So we can merge this patch irrespective of the fact whether virtiofs
>>>> should make use of this mechanism or not, right?
>> I don't see a security objection. I did see that Andreas Gruenbacher
>> <[email protected]> has objections to the behavior.
>>
>>
>>>>>> Who wants to set user.* xattr on symlink/special files
>>>>>> -----------------------------------------------------
>>>>>> I have primarily two users at this point of time.
>>>>>>
>>>>>> - virtiofs daemon.
>>>>>>
>>>>>> - fuse-overlay. Giuseppe, seems to set user.* xattr attrs on unpriviliged
>>>>>> fuse-overlay as well and he ran into similar issue. So fuse-overlay
>>>>>> should benefit from this change as well.
>>>>>>
>>>>>> Why virtiofsd wants to set user.* xattr on symlink/special files
>>>>>> ----------------------------------------------------------------
>>>>>> In virtiofs, actual file server is virtiosd daemon running on host.
>>>>>> There we have a mode where xattrs can be remapped to something else.
>>>>>> For example security.selinux can be remapped to
>>>>>> user.virtiofsd.securit.selinux on the host.
>>>>> As I have stated before, this introduces a breach in security.
>>>>> It allows an unprivileged process on the host to manipulate the
>>>>> security state of the guest. This is horribly wrong. It is not
>>>>> sufficient to claim that the breach requires misconfiguration
>>>>> to exploit. Don't do this.
>>>> So couple of things.
>>>>
>>>> - Right now whole virtiofs model is relying on the fact that host
>>>> unpriviliged users don't have access to shared directory. Otherwise
>>>> guest process can simply drop a setuid root binary in shared directory
>>>> and unpriviliged process can execute it and take over host system.
>>>>
>>>> So if virtiofs makes use of this mechanism, we are well with-in
>>>> the existing constraints. If users don't follow the constraints,
>>>> bad things can happen.
>>>>
>>>> - I think Smalley provided a solution for your concern in other thread
>>>> we discussed this issue.
>>>>
>>>> https://lore.kernel.org/selinux/CAEjxPJ4411vL3+Ab-J0yrRTmXoEf8pVR3x3CSRgPjfzwiUcDtw@mail.gmail.com/T/#mddea4cec7a68c3ee5e8826d650020361030209d6
>>>>
>>>>
>>>> "So for example if the host policy says that only virtiofsd can set
>>>> attributes on those files, then the guest MAC labels along with all
>>>> the other attributes are protected against tampering by any other
>>>> process on the host."
>> You can't count on SELinux policy to address the issue on a
>> system running Smack.
>> Or any other user of system.* xattrs,
>> be they in the kernel or user space. You can't even count on
>> SELinux policy to be correct. virtiofs has to present a "safe"
>> situation regardless of how security.* xattrs are used and
>> regardless of which, if any, LSMs are configured. You can't
>> do that with user.* attributes.
> Lets take a step back. Your primary concern with using user.* xattrs
> by virtiofsd is that it can be modified by unprivileged users on host.
> And our solution to that problem is hide shared directory from
> unprivileged users.
You really don't see how fragile that is, do you? How a single
errant call to rename(), chmod() or chown() on the host can expose
the entire guest to exploitation. That's not even getting into
the bag of mount() tricks.
> In addition to that, LSMs on host can block setting "user.*" xattrs by
> virtiofsd domain only for additional protection.
Try thinking outside the SELinux box briefly, if you possibly can.
An LSM that implements just Bell & LaPadula isn't going to have a
"virtiofs domain". Neither is a Smack "3 domain" system. Smack doesn't
distinguish writing user xattrs from writing other file attributes
in policy. Your argument requires a fine grained policy a'la SELinux.
And an application specific SELinux policy at that.
> If LSMs are not configured,
> then hiding the directory is the solution.
It's not a solution at all. It's wishful thinking that
some admin is going to do absolutely everything right, will
never make a mistake and will never, ever, read the mount(2)
man page.
> So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> solution. I don't understand that part.
It comes back to your design, which is fundamentally flawed. You
can't store system security information in an attribute that can
be manipulated by untrusted entities. That's why we have system.*
xattrs. You want to have an attribute on the host that maps to a
security attribute on the guest. The host has to protect the attribute
on the guest with mechanisms of comparable strength as the guest's
mechanisms. Otherwise you can't trust the guest with host data.
It's a real shame that CAP_SYS_ADMIN is so scary. The capability
mechanism as implemented today won't scale to the hundreds of individual
capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
I'm not convinced that there isn't a way to accomplish what you're
trying to do without privilege, but this isn't it, and I don't know
what is. Sorry.
> Also if directory is not hidden, unprivileged users can change file
> data and other metadata.
I assumed that you've taken that into account. Are you saying that
isn't going to be done correctly either?
> Why that's not a concern and why there is
> so much of focus only security xattr.
As with an NFS mount, the assumption is that UID 567 (or its magically
mapped equivalent) has the same access rights on both the server/host
and client/guest. I'm not worried about the mode bits because they are
presented consistently on both machines. If, on the other hand, an
attribute used to determine access is security.esprit on the guest and
user.security.esprit on the host, the unprivileged user on the host
can defeat the privilege requirements on the guest. That's why.
> If you were to block modification
> of file then you will have rely on LSMs.
No. We're talking about the semantics of the xattr namespaces.
LSMs can further constrain access to xattrs, but the basic rules
of access to the user.* and security.* attributes are different
in any case. This is by design.
> And if LSMs are not configured,
> then we will rely on shared directory not being visible.
LSMs are not the problem. LSMs use security.* xattrs, which is why
they come up in the discussion.
> Can you please help me understand why hiding shared directory from
> unprivileged users is not a solution
Maybe you can describe the mechanism you use to "hide" a shared directory
on the host. If the filesystem is mounted on the host it seems unlikely
that you can provide a convincing argument for sufficient protection.
> (With both LSMs configured or
> not configured on host). That's a requirement for virtiofs anyway.
> And if we agree on that, then I don't see why using "user.*" xattrs
> for storing guest sercurity attributes is a problem.
>
> Thanks
> Vivek
>
On Thu, Sep 02, 2021 at 03:34:17PM -0700, Casey Schaufler wrote:
> On 9/2/2021 1:06 PM, Vivek Goyal wrote:
> > On Thu, Sep 02, 2021 at 11:55:11AM -0700, Casey Schaufler wrote:
> >> On 9/2/2021 10:42 AM, Vivek Goyal wrote:
> >>> On Thu, Sep 02, 2021 at 01:05:01PM -0400, Vivek Goyal wrote:
> >>>> On Thu, Sep 02, 2021 at 08:43:50AM -0700, Casey Schaufler wrote:
> >>>>> On 9/2/2021 8:22 AM, Vivek Goyal wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> This is V3 of the patch. Previous versions were posted here.
> >>>>>>
> >>>>>> v2:
> >>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]/
> >>>>>> v1:
> >>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]
> >>>>>> +m/
> >>>>>>
> >>>>>> Changes since v2
> >>>>>> ----------------
> >>>>>> - Do not call inode_permission() for special files as file mode bits
> >>>>>> on these files represent permissions to read/write from/to device
> >>>>>> and not necessarily permission to read/write xattrs. In this case
> >>>>>> now user.* extended xattrs can be read/written on special files
> >>>>>> as long as caller is owner of file or has CAP_FOWNER.
> >>>>>>
> >>>>>> - Fixed "man xattr". Will post a patch in same thread little later. (J.
> >>>>>> Bruce Fields)
> >>>>>>
> >>>>>> - Fixed xfstest 062. Changed it to run only on older kernels where
> >>>>>> user extended xattrs are not allowed on symlinks/special files. Added
> >>>>>> a new replacement test 648 which does exactly what 062. Just that
> >>>>>> it is supposed to run on newer kernels where user extended xattrs
> >>>>>> are allowed on symlinks and special files. Will post patch in
> >>>>>> same thread (Ted Ts'o).
> >>>>>>
> >>>>>> Testing
> >>>>>> -------
> >>>>>> - Ran xfstest "./check -g auto" with and without patches and did not
> >>>>>> notice any new failures.
> >>>>>>
> >>>>>> - Tested setting "user.*" xattr with ext4/xfs/btrfs/overlay/nfs
> >>>>>> filesystems and it works.
> >>>>>>
> >>>>>> Description
> >>>>>> ===========
> >>>>>>
> >>>>>> Right now we don't allow setting user.* xattrs on symlinks and special
> >>>>>> files at all. Initially I thought that real reason behind this
> >>>>>> restriction is quota limitations but from last conversation it seemed
> >>>>>> that real reason is that permission bits on symlink and special files
> >>>>>> are special and different from regular files and directories, hence
> >>>>>> this restriction is in place. (I tested with xfs user quota enabled and
> >>>>>> quota restrictions kicked in on symlink).
> >>>>>>
> >>>>>> This version of patch allows reading/writing user.* xattr on symlink and
> >>>>>> special files if caller is owner or priviliged (has CAP_FOWNER) w.r.t inode.
> >>>>> This part of your project makes perfect sense. There's no good
> >>>>> security reason that you shouldn't set user.* xattrs on symlinks
> >>>>> and/or special files.
> >>>>>
> >>>>> However, your virtiofs use case is unreasonable.
> >>>> Ok. So we can merge this patch irrespective of the fact whether virtiofs
> >>>> should make use of this mechanism or not, right?
> >> I don't see a security objection. I did see that Andreas Gruenbacher
> >> <[email protected]> has objections to the behavior.
> >>
> >>
> >>>>>> Who wants to set user.* xattr on symlink/special files
> >>>>>> -----------------------------------------------------
> >>>>>> I have primarily two users at this point of time.
> >>>>>>
> >>>>>> - virtiofs daemon.
> >>>>>>
> >>>>>> - fuse-overlay. Giuseppe, seems to set user.* xattr attrs on unpriviliged
> >>>>>> fuse-overlay as well and he ran into similar issue. So fuse-overlay
> >>>>>> should benefit from this change as well.
> >>>>>>
> >>>>>> Why virtiofsd wants to set user.* xattr on symlink/special files
> >>>>>> ----------------------------------------------------------------
> >>>>>> In virtiofs, actual file server is virtiosd daemon running on host.
> >>>>>> There we have a mode where xattrs can be remapped to something else.
> >>>>>> For example security.selinux can be remapped to
> >>>>>> user.virtiofsd.securit.selinux on the host.
> >>>>> As I have stated before, this introduces a breach in security.
> >>>>> It allows an unprivileged process on the host to manipulate the
> >>>>> security state of the guest. This is horribly wrong. It is not
> >>>>> sufficient to claim that the breach requires misconfiguration
> >>>>> to exploit. Don't do this.
> >>>> So couple of things.
> >>>>
> >>>> - Right now whole virtiofs model is relying on the fact that host
> >>>> unpriviliged users don't have access to shared directory. Otherwise
> >>>> guest process can simply drop a setuid root binary in shared directory
> >>>> and unpriviliged process can execute it and take over host system.
> >>>>
> >>>> So if virtiofs makes use of this mechanism, we are well with-in
> >>>> the existing constraints. If users don't follow the constraints,
> >>>> bad things can happen.
> >>>>
> >>>> - I think Smalley provided a solution for your concern in other thread
> >>>> we discussed this issue.
> >>>>
> >>>> https://lore.kernel.org/selinux/CAEjxPJ4411vL3+Ab-J0yrRTmXoEf8pVR3x3CSRgPjfzwiUcDtw@mail.gmail.com/T/#mddea4cec7a68c3ee5e8826d650020361030209d6
> >>>>
> >>>>
> >>>> "So for example if the host policy says that only virtiofsd can set
> >>>> attributes on those files, then the guest MAC labels along with all
> >>>> the other attributes are protected against tampering by any other
> >>>> process on the host."
> >> You can't count on SELinux policy to address the issue on a
> >> system running Smack.
> >> Or any other user of system.* xattrs,
> >> be they in the kernel or user space. You can't even count on
> >> SELinux policy to be correct. virtiofs has to present a "safe"
> >> situation regardless of how security.* xattrs are used and
> >> regardless of which, if any, LSMs are configured. You can't
> >> do that with user.* attributes.
> > Lets take a step back. Your primary concern with using user.* xattrs
> > by virtiofsd is that it can be modified by unprivileged users on host.
> > And our solution to that problem is hide shared directory from
> > unprivileged users.
>
> You really don't see how fragile that is, do you?
Yes, I am not sure why we are so opposed to the idea of using
user.* xattrs for storing the guest security.selinux xattrs by virtiofsd.
And I am trying to understand that. And this discussion should help.
With virtiofsd, we want to keep all shared directory trees in a
parent directory which has read/write/search permissions only for root
so that no unpriviliged process can get to files in shared directory
and do any of the operations.
For example, /var/lib/containers/storage setup by podman has
following permissions.
ll -d /var/lib/containers/storage/
drwx------. 10 root root 4096 Jun 18 2020 /var/lib/containers/storage/
Now I should be able to create /var/lib/containers/storage/shared1
directory and ask virtiofsd to export "share1" to guest. Unprivileged
process on host can not open any of the files in shared1 dir, hence
should not be able to modify any data/metadata associated with the
file.
If this assumption is correct, then I should be able to use "user.*"
xattrs without having to worry about unprivileged processes modifying
security labels of guest/nested-guest.
> How a single
> errant call to rename(), chmod() or chown() on the host can expose
> the entire guest to exploitation. That's not even getting into
> the bag of mount() tricks.
I am relying on unpriviliged processes not having permissions to
read/search in shared directory. And I guess I have to. Even if
I use "trusted.*" xattrs, what about file data. Unpriviliged
processes can modify file data and that's going to be equally
problematic, isn't it? So why are we so focussed only on
security label part of it.
You have mentioned that file data is not necessarily
that big a problem because UID 1000 on host and UID 1000 in guest
should be treated same. But I am not sure we can do that. In kata
container model, guest images are untrusted. So they can simply cook
up UID 1000 or UID 0 or any other UID. Now there can be use
cases where we are ready to trust guest and treat UID 1000
on host and guest in same way. But primary model I am focussing
on is that guest shared directory remains isolated from other
users on host.
>
>
> > In addition to that, LSMs on host can block setting "user.*" xattrs by
> > virtiofsd domain only for additional protection.
>
> Try thinking outside the SELinux box briefly, if you possibly can.
> An LSM that implements just Bell & LaPadula isn't going to have a
> "virtiofs domain". Neither is a Smack "3 domain" system. Smack doesn't
> distinguish writing user xattrs from writing other file attributes
> in policy. Your argument requires a fine grained policy a'la SELinux.
> And an application specific SELinux policy at that.
Ok, so does we have to have capability for every LSM to block write
to user xattr. I mean in above example, virtiofsd is relying on DAC so that
unprivileged processes can't modify user xattr labels. If we were to
use "trusted.*" xattr then we are relying on CAP_SYS_ADMIN.
IOW, it will be nice if one or more LSMs can provide mechanism fine
grained enough to block write to user xattr by unwanted application
and that provides extra level of security. But should it be mandatory?
>
> > If LSMs are not configured,
> > then hiding the directory is the solution.
>
> It's not a solution at all. It's wishful thinking that
> some admin is going to do absolutely everything right, will
> never make a mistake and will never, ever, read the mount(2)
> man page.
I agree that its easy to make mistakes. But making mistakes is already
disastrous. So lets say and admin makes mistake and unprivileged
processes can open/read/write files in shared directory on host.
Assume I am using "trusted" xattrs to store guest's security labels.
Now file's data can be modified on host in unwated way and be very
problematic.
Guest can drop a setuid root binary in shared directory and unprivileged
process on host can run it and take over the system.
Isn't that even bigger problem. So to me making sure shared directory
is not reachable by unprivileged processes is absolute must requirement
for virtiofsd (when running as root). If we break that assumption,
we already have much bigger problems.
>
> > So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> > solution. I don't understand that part.
>
> It comes back to your design, which is fundamentally flawed. You
> can't store system security information in an attribute that can
> be manipulated by untrusted entities.
You are assuming untrusted entities can have access to the shared dir. But
assumption in this model is, shared directory is not reachable by
unprivileged entities. If we break that requirement, there are much
bigger issues to deal with then just security attributes.
> That's why we have system.*
> xattrs. You want to have an attribute on the host that maps to a
> security attribute on the guest. The host has to protect the attribute
> on the guest with mechanisms of comparable strength as the guest's
> mechanisms. Otherwise you can't trust the guest with host data.
Ok, I understand your desire that security xattrs as seen by guest kernel
should be protected by something stronger than simply user xattr.
>
> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
> mechanism as implemented today won't scale to the hundreds of individual
> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
> I'm not convinced that there isn't a way to accomplish what you're
> trying to do without privilege, but this isn't it, and I don't know
> what is. Sorry.
>
> > Also if directory is not hidden, unprivileged users can change file
> > data and other metadata.
>
> I assumed that you've taken that into account. Are you saying that
> isn't going to be done correctly either?
I am relying on shared directory not accessible to unprivileged processes.
If that assumption is broken, I guess, all bets are off. Until and unless
one designs SELinux (or other LSM) policy in such a way so that
only virtiofsd can read/write to these shared directories.
I think Dan Walsh has got SELinux policy written for atleast kata
containers case.
So I guess we will rely on MAC to block unwanted access if users
make a mistake? Is that the idea? I guess that's why you want
a stronger mechansim to store guest security xattrs on host. If
users make a mistake then we have a fallback path?
>
> > Why that's not a concern and why there is
> > so much of focus only security xattr.
>
> As with an NFS mount, the assumption is that UID 567 (or its magically
> mapped equivalent) has the same access rights on both the server/host
> and client/guest. I'm not worried about the mode bits because they are
> presented consistently on both machines. If, on the other hand, an
> attribute used to determine access is security.esprit on the guest and
> user.security.esprit on the host, the unprivileged user on the host
> can defeat the privilege requirements on the guest. That's why.
Hmm..., so if a user id 1000 inside guest can't modify security
xattrs then same user on host should be allowed to bypass that
by modifying user xattr. I agree with that.
Just that I am relying on shared directory not being accessible
to uid 1000 on host and you don't want to rely on that because
users can easily make mistake. And that's why want to stronger
form of mechanism to store security xattrs.
>
> > If you were to block modification
> > of file then you will have rely on LSMs.
>
> No. We're talking about the semantics of the xattr namespaces.
> LSMs can further constrain access to xattrs, but the basic rules
> of access to the user.* and security.* attributes are different
> in any case. This is by design.
>
> > And if LSMs are not configured,
> > then we will rely on shared directory not being visible.
>
> LSMs are not the problem. LSMs use security.* xattrs, which is why
> they come up in the discussion.
>
> > Can you please help me understand why hiding shared directory from
> > unprivileged users is not a solution
>
> Maybe you can describe the mechanism you use to "hide" a shared directory
> on the host. If the filesystem is mounted on the host it seems unlikely
> that you can provide a convincing argument for sufficient protection.
I am relying on changing direcotry permissions to allow read/write/search
permission to root only.
Thanks
Vivek
On 9/3/2021 8:26 AM, Vivek Goyal wrote:
> On Thu, Sep 02, 2021 at 03:34:17PM -0700, Casey Schaufler wrote:
>> On 9/2/2021 1:06 PM, Vivek Goyal wrote:
>>> On Thu, Sep 02, 2021 at 11:55:11AM -0700, Casey Schaufler wrote:
>>>> On 9/2/2021 10:42 AM, Vivek Goyal wrote:
>>>>> On Thu, Sep 02, 2021 at 01:05:01PM -0400, Vivek Goyal wrote:
>>>>>> On Thu, Sep 02, 2021 at 08:43:50AM -0700, Casey Schaufler wrote:
>>>>>>> On 9/2/2021 8:22 AM, Vivek Goyal wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> This is V3 of the patch. Previous versions were posted here.
>>>>>>>>
>>>>>>>> v2:
>>>>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]/
>>>>>>>> v1:
>>>>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]
>>>>>>>> +m/
>>>>>>>>
>>>>>>>> Changes since v2
>>>>>>>> ----------------
>>>>>>>> - Do not call inode_permission() for special files as file mode bits
>>>>>>>> on these files represent permissions to read/write from/to device
>>>>>>>> and not necessarily permission to read/write xattrs. In this case
>>>>>>>> now user.* extended xattrs can be read/written on special files
>>>>>>>> as long as caller is owner of file or has CAP_FOWNER.
>>>>>>>>
>>>>>>>> - Fixed "man xattr". Will post a patch in same thread little later. (J.
>>>>>>>> Bruce Fields)
>>>>>>>>
>>>>>>>> - Fixed xfstest 062. Changed it to run only on older kernels where
>>>>>>>> user extended xattrs are not allowed on symlinks/special files. Added
>>>>>>>> a new replacement test 648 which does exactly what 062. Just that
>>>>>>>> it is supposed to run on newer kernels where user extended xattrs
>>>>>>>> are allowed on symlinks and special files. Will post patch in
>>>>>>>> same thread (Ted Ts'o).
>>>>>>>>
>>>>>>>> Testing
>>>>>>>> -------
>>>>>>>> - Ran xfstest "./check -g auto" with and without patches and did not
>>>>>>>> notice any new failures.
>>>>>>>>
>>>>>>>> - Tested setting "user.*" xattr with ext4/xfs/btrfs/overlay/nfs
>>>>>>>> filesystems and it works.
>>>>>>>>
>>>>>>>> Description
>>>>>>>> ===========
>>>>>>>>
>>>>>>>> Right now we don't allow setting user.* xattrs on symlinks and special
>>>>>>>> files at all. Initially I thought that real reason behind this
>>>>>>>> restriction is quota limitations but from last conversation it seemed
>>>>>>>> that real reason is that permission bits on symlink and special files
>>>>>>>> are special and different from regular files and directories, hence
>>>>>>>> this restriction is in place. (I tested with xfs user quota enabled and
>>>>>>>> quota restrictions kicked in on symlink).
>>>>>>>>
>>>>>>>> This version of patch allows reading/writing user.* xattr on symlink and
>>>>>>>> special files if caller is owner or priviliged (has CAP_FOWNER) w.r.t inode.
>>>>>>> This part of your project makes perfect sense. There's no good
>>>>>>> security reason that you shouldn't set user.* xattrs on symlinks
>>>>>>> and/or special files.
>>>>>>>
>>>>>>> However, your virtiofs use case is unreasonable.
>>>>>> Ok. So we can merge this patch irrespective of the fact whether virtiofs
>>>>>> should make use of this mechanism or not, right?
>>>> I don't see a security objection. I did see that Andreas Gruenbacher
>>>> <[email protected]> has objections to the behavior.
>>>>
>>>>
>>>>>>>> Who wants to set user.* xattr on symlink/special files
>>>>>>>> -----------------------------------------------------
>>>>>>>> I have primarily two users at this point of time.
>>>>>>>>
>>>>>>>> - virtiofs daemon.
>>>>>>>>
>>>>>>>> - fuse-overlay. Giuseppe, seems to set user.* xattr attrs on unpriviliged
>>>>>>>> fuse-overlay as well and he ran into similar issue. So fuse-overlay
>>>>>>>> should benefit from this change as well.
>>>>>>>>
>>>>>>>> Why virtiofsd wants to set user.* xattr on symlink/special files
>>>>>>>> ----------------------------------------------------------------
>>>>>>>> In virtiofs, actual file server is virtiosd daemon running on host.
>>>>>>>> There we have a mode where xattrs can be remapped to something else.
>>>>>>>> For example security.selinux can be remapped to
>>>>>>>> user.virtiofsd.securit.selinux on the host.
>>>>>>> As I have stated before, this introduces a breach in security.
>>>>>>> It allows an unprivileged process on the host to manipulate the
>>>>>>> security state of the guest. This is horribly wrong. It is not
>>>>>>> sufficient to claim that the breach requires misconfiguration
>>>>>>> to exploit. Don't do this.
>>>>>> So couple of things.
>>>>>>
>>>>>> - Right now whole virtiofs model is relying on the fact that host
>>>>>> unpriviliged users don't have access to shared directory. Otherwise
>>>>>> guest process can simply drop a setuid root binary in shared directory
>>>>>> and unpriviliged process can execute it and take over host system.
>>>>>>
>>>>>> So if virtiofs makes use of this mechanism, we are well with-in
>>>>>> the existing constraints. If users don't follow the constraints,
>>>>>> bad things can happen.
>>>>>>
>>>>>> - I think Smalley provided a solution for your concern in other thread
>>>>>> we discussed this issue.
>>>>>>
>>>>>> https://lore.kernel.org/selinux/CAEjxPJ4411vL3+Ab-J0yrRTmXoEf8pVR3x3CSRgPjfzwiUcDtw@mail.gmail.com/T/#mddea4cec7a68c3ee5e8826d650020361030209d6
>>>>>>
>>>>>>
>>>>>> "So for example if the host policy says that only virtiofsd can set
>>>>>> attributes on those files, then the guest MAC labels along with all
>>>>>> the other attributes are protected against tampering by any other
>>>>>> process on the host."
>>>> You can't count on SELinux policy to address the issue on a
>>>> system running Smack.
>>>> Or any other user of system.* xattrs,
>>>> be they in the kernel or user space. You can't even count on
>>>> SELinux policy to be correct. virtiofs has to present a "safe"
>>>> situation regardless of how security.* xattrs are used and
>>>> regardless of which, if any, LSMs are configured. You can't
>>>> do that with user.* attributes.
>>> Lets take a step back. Your primary concern with using user.* xattrs
>>> by virtiofsd is that it can be modified by unprivileged users on host.
>>> And our solution to that problem is hide shared directory from
>>> unprivileged users.
>> You really don't see how fragile that is, do you?
> Yes, I am not sure why we are so opposed to the idea of using
> user.* xattrs for storing the guest security.selinux xattrs by virtiofsd.
> And I am trying to understand that. And this discussion should help.
>
> With virtiofsd, we want to keep all shared directory trees in a
> parent directory which has read/write/search permissions only for root
> so that no unpriviliged process can get to files in shared directory
> and do any of the operations.
>
> For example, /var/lib/containers/storage setup by podman has
> following permissions.
>
> ll -d /var/lib/containers/storage/
> drwx------. 10 root root 4096 Jun 18 2020 /var/lib/containers/storage/
>
> Now I should be able to create /var/lib/containers/storage/shared1
> directory and ask virtiofsd to export "share1" to guest. Unprivileged
> process on host can not open any of the files in shared1 dir, hence
> should not be able to modify any data/metadata associated with the
> file.
>
> If this assumption is correct, then I should be able to use "user.*"
> xattrs without having to worry about unprivileged processes modifying
> security labels of guest/nested-guest.
The only attributes you can really count on to protect an object
are the attributes on the object itself. Path based protections are
not reliable.
>> How a single
>> errant call to rename(), chmod() or chown() on the host can expose
>> the entire guest to exploitation. That's not even getting into
>> the bag of mount() tricks.
> I am relying on unpriviliged processes not having permissions to
> read/search in shared directory. And I guess I have to. Even if
> I use "trusted.*" xattrs, what about file data. Unpriviliged
> processes can modify file data and that's going to be equally
> problematic, isn't it? So why are we so focussed only on
> security label part of it.
We are concentrating on your proposed clever mapping trick.
We are doing so because it won't work, and when it blows up
into a full security bruhaha someone is going to try putting
the blame on the xattr mechanism.
>
> You have mentioned that file data is not necessarily
> that big a problem because UID 1000 on host and UID 1000 in guest
> should be treated same. But I am not sure we can do that. In kata
> container model, guest images are untrusted. So they can simply cook
> up UID 1000 or UID 0 or any other UID. Now there can be use
> cases where we are ready to trust guest and treat UID 1000
> on host and guest in same way. But primary model I am focussing
> on is that guest shared directory remains isolated from other
> users on host.
I don't have the time just now to examine the rest of virtiofs.
I am quite afraid that you are making dangerous assumptions on
a number of things. It sure doesn't sound like you've thought
through all of the implications of sharing between the host and
guest.
>>> In addition to that, LSMs on host can block setting "user.*" xattrs by
>>> virtiofsd domain only for additional protection.
>> Try thinking outside the SELinux box briefly, if you possibly can.
>> An LSM that implements just Bell & LaPadula isn't going to have a
>> "virtiofs domain". Neither is a Smack "3 domain" system. Smack doesn't
>> distinguish writing user xattrs from writing other file attributes
>> in policy. Your argument requires a fine grained policy a'la SELinux.
>> And an application specific SELinux policy at that.
> Ok, so does we have to have capability for every LSM to block write
> to user xattr. I mean in above example, virtiofsd is relying on DAC so that
> unprivileged processes can't modify user xattr labels. If we were to
> use "trusted.*" xattr then we are relying on CAP_SYS_ADMIN.
>
> IOW, it will be nice if one or more LSMs can provide mechanism fine
> grained enough to block write to user xattr by unwanted application
> and that provides extra level of security. But should it be mandatory?
No. The behavior of user xattrs is just fine the way it is.
It works as designed.
>
>>> If LSMs are not configured,
>>> then hiding the directory is the solution.
>> It's not a solution at all. It's wishful thinking that
>> some admin is going to do absolutely everything right, will
>> never make a mistake and will never, ever, read the mount(2)
>> man page.
> I agree that its easy to make mistakes. But making mistakes is already
> disastrous. So lets say and admin makes mistake and unprivileged
> processes can open/read/write files in shared directory on host.
> Assume I am using "trusted" xattrs to store guest's security labels.
>
> Now file's data can be modified on host in unwated way and be very
> problematic.
>
> Guest can drop a setuid root binary in shared directory and unprivileged
> process on host can run it and take over the system.
>
> Isn't that even bigger problem. So to me making sure shared directory
> is not reachable by unprivileged processes is absolute must requirement
> for virtiofsd (when running as root). If we break that assumption,
> we already have much bigger problems.
I can't help but think you probably do already have much bigger problems.
>>> So why that's not a solution and only relying on CAP_SYS_ADMIN is the
>>> solution. I don't understand that part.
>> It comes back to your design, which is fundamentally flawed. You
>> can't store system security information in an attribute that can
>> be manipulated by untrusted entities.
> You are assuming untrusted entities can have access to the shared dir. But
> assumption in this model is, shared directory is not reachable by
> unprivileged entities. If we break that requirement, there are much
> bigger issues to deal with then just security attributes.
That's a bad assumption. You have all sorts of issues.
>
>> That's why we have system.*
>> xattrs. You want to have an attribute on the host that maps to a
>> security attribute on the guest. The host has to protect the attribute
>> on the guest with mechanisms of comparable strength as the guest's
>> mechanisms. Otherwise you can't trust the guest with host data.
> Ok, I understand your desire that security xattrs as seen by guest kernel
> should be protected by something stronger than simply user xattr.
>
>> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
>> mechanism as implemented today won't scale to the hundreds of individual
>> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
>> I'm not convinced that there isn't a way to accomplish what you're
>> trying to do without privilege, but this isn't it, and I don't know
>> what is. Sorry.
>>
>>> Also if directory is not hidden, unprivileged users can change file
>>> data and other metadata.
>> I assumed that you've taken that into account. Are you saying that
>> isn't going to be done correctly either?
> I am relying on shared directory not accessible to unprivileged processes.
> If that assumption is broken, I guess, all bets are off. Until and unless
> one designs SELinux (or other LSM) policy in such a way so that
> only virtiofsd can read/write to these shared directories.
>
> I think Dan Walsh has got SELinux policy written for atleast kata
> containers case.
>
> So I guess we will rely on MAC to block unwanted access if users
> make a mistake? Is that the idea? I guess that's why you want
> a stronger mechansim to store guest security xattrs on host. If
> users make a mistake then we have a fallback path?
>
>>> Why that's not a concern and why there is
>>> so much of focus only security xattr.
>> As with an NFS mount, the assumption is that UID 567 (or its magically
>> mapped equivalent) has the same access rights on both the server/host
>> and client/guest. I'm not worried about the mode bits because they are
>> presented consistently on both machines. If, on the other hand, an
>> attribute used to determine access is security.esprit on the guest and
>> user.security.esprit on the host, the unprivileged user on the host
>> can defeat the privilege requirements on the guest. That's why.
> Hmm..., so if a user id 1000 inside guest can't modify security
> xattrs then same user on host should be allowed to bypass that
> by modifying user xattr. I agree with that.
>
> Just that I am relying on shared directory not being accessible
> to uid 1000 on host and you don't want to rely on that because
> users can easily make mistake. And that's why want to stronger
> form of mechanism to store security xattrs.
>
>>> If you were to block modification
>>> of file then you will have rely on LSMs.
>> No. We're talking about the semantics of the xattr namespaces.
>> LSMs can further constrain access to xattrs, but the basic rules
>> of access to the user.* and security.* attributes are different
>> in any case. This is by design.
>>
>>> And if LSMs are not configured,
>>> then we will rely on shared directory not being visible.
>> LSMs are not the problem. LSMs use security.* xattrs, which is why
>> they come up in the discussion.
>>
>>> Can you please help me understand why hiding shared directory from
>>> unprivileged users is not a solution
>> Maybe you can describe the mechanism you use to "hide" a shared directory
>> on the host. If the filesystem is mounted on the host it seems unlikely
>> that you can provide a convincing argument for sufficient protection.
> I am relying on changing direcotry permissions to allow read/write/search
> permission to root only.
Then you have a real mess of problems coming your way. Sorry.
>
> Thanks
> Vivek
>
On Thu, Sep 02, 2021 at 03:34:17PM -0700, Casey Schaufler wrote:
> On 9/2/2021 1:06 PM, Vivek Goyal wrote:
> > On Thu, Sep 02, 2021 at 11:55:11AM -0700, Casey Schaufler wrote:
> >> On 9/2/2021 10:42 AM, Vivek Goyal wrote:
> >>> On Thu, Sep 02, 2021 at 01:05:01PM -0400, Vivek Goyal wrote:
> >>>> On Thu, Sep 02, 2021 at 08:43:50AM -0700, Casey Schaufler wrote:
> >>>>> On 9/2/2021 8:22 AM, Vivek Goyal wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> This is V3 of the patch. Previous versions were posted here.
> >>>>>>
> >>>>>> v2:
> >>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]/
> >>>>>> v1:
> >>>>>> https://lore.kernel.org/linux-fsdevel/[email protected]
> >>>>>> +m/
> >>>>>>
> >>>>>> Changes since v2
> >>>>>> ----------------
> >>>>>> - Do not call inode_permission() for special files as file mode bits
> >>>>>> on these files represent permissions to read/write from/to device
> >>>>>> and not necessarily permission to read/write xattrs. In this case
> >>>>>> now user.* extended xattrs can be read/written on special files
> >>>>>> as long as caller is owner of file or has CAP_FOWNER.
> >>>>>>
> >>>>>> - Fixed "man xattr". Will post a patch in same thread little later. (J.
> >>>>>> Bruce Fields)
> >>>>>>
> >>>>>> - Fixed xfstest 062. Changed it to run only on older kernels where
> >>>>>> user extended xattrs are not allowed on symlinks/special files. Added
> >>>>>> a new replacement test 648 which does exactly what 062. Just that
> >>>>>> it is supposed to run on newer kernels where user extended xattrs
> >>>>>> are allowed on symlinks and special files. Will post patch in
> >>>>>> same thread (Ted Ts'o).
> >>>>>>
> >>>>>> Testing
> >>>>>> -------
> >>>>>> - Ran xfstest "./check -g auto" with and without patches and did not
> >>>>>> notice any new failures.
> >>>>>>
> >>>>>> - Tested setting "user.*" xattr with ext4/xfs/btrfs/overlay/nfs
> >>>>>> filesystems and it works.
> >>>>>>
> >>>>>> Description
> >>>>>> ===========
> >>>>>>
> >>>>>> Right now we don't allow setting user.* xattrs on symlinks and special
> >>>>>> files at all. Initially I thought that real reason behind this
> >>>>>> restriction is quota limitations but from last conversation it seemed
> >>>>>> that real reason is that permission bits on symlink and special files
> >>>>>> are special and different from regular files and directories, hence
> >>>>>> this restriction is in place. (I tested with xfs user quota enabled and
> >>>>>> quota restrictions kicked in on symlink).
> >>>>>>
> >>>>>> This version of patch allows reading/writing user.* xattr on symlink and
> >>>>>> special files if caller is owner or priviliged (has CAP_FOWNER) w.r.t inode.
> >>>>> This part of your project makes perfect sense. There's no good
> >>>>> security reason that you shouldn't set user.* xattrs on symlinks
> >>>>> and/or special files.
> >>>>>
> >>>>> However, your virtiofs use case is unreasonable.
> >>>> Ok. So we can merge this patch irrespective of the fact whether virtiofs
> >>>> should make use of this mechanism or not, right?
> >> I don't see a security objection. I did see that Andreas Gruenbacher
> >> <[email protected]> has objections to the behavior.
> >>
> >>
> >>>>>> Who wants to set user.* xattr on symlink/special files
> >>>>>> -----------------------------------------------------
> >>>>>> I have primarily two users at this point of time.
> >>>>>>
> >>>>>> - virtiofs daemon.
> >>>>>>
> >>>>>> - fuse-overlay. Giuseppe, seems to set user.* xattr attrs on unpriviliged
> >>>>>> fuse-overlay as well and he ran into similar issue. So fuse-overlay
> >>>>>> should benefit from this change as well.
> >>>>>>
> >>>>>> Why virtiofsd wants to set user.* xattr on symlink/special files
> >>>>>> ----------------------------------------------------------------
> >>>>>> In virtiofs, actual file server is virtiosd daemon running on host.
> >>>>>> There we have a mode where xattrs can be remapped to something else.
> >>>>>> For example security.selinux can be remapped to
> >>>>>> user.virtiofsd.securit.selinux on the host.
> >>>>> As I have stated before, this introduces a breach in security.
> >>>>> It allows an unprivileged process on the host to manipulate the
> >>>>> security state of the guest. This is horribly wrong. It is not
> >>>>> sufficient to claim that the breach requires misconfiguration
> >>>>> to exploit. Don't do this.
> >>>> So couple of things.
> >>>>
> >>>> - Right now whole virtiofs model is relying on the fact that host
> >>>> unpriviliged users don't have access to shared directory. Otherwise
> >>>> guest process can simply drop a setuid root binary in shared directory
> >>>> and unpriviliged process can execute it and take over host system.
> >>>>
> >>>> So if virtiofs makes use of this mechanism, we are well with-in
> >>>> the existing constraints. If users don't follow the constraints,
> >>>> bad things can happen.
> >>>>
> >>>> - I think Smalley provided a solution for your concern in other thread
> >>>> we discussed this issue.
> >>>>
> >>>> https://lore.kernel.org/selinux/CAEjxPJ4411vL3+Ab-J0yrRTmXoEf8pVR3x3CSRgPjfzwiUcDtw@mail.gmail.com/T/#mddea4cec7a68c3ee5e8826d650020361030209d6
> >>>>
> >>>>
> >>>> "So for example if the host policy says that only virtiofsd can set
> >>>> attributes on those files, then the guest MAC labels along with all
> >>>> the other attributes are protected against tampering by any other
> >>>> process on the host."
> >> You can't count on SELinux policy to address the issue on a
> >> system running Smack.
> >> Or any other user of system.* xattrs,
> >> be they in the kernel or user space. You can't even count on
> >> SELinux policy to be correct. virtiofs has to present a "safe"
> >> situation regardless of how security.* xattrs are used and
> >> regardless of which, if any, LSMs are configured. You can't
> >> do that with user.* attributes.
> > Lets take a step back. Your primary concern with using user.* xattrs
> > by virtiofsd is that it can be modified by unprivileged users on host.
> > And our solution to that problem is hide shared directory from
> > unprivileged users.
>
> You really don't see how fragile that is, do you? How a single
> errant call to rename(), chmod() or chown() on the host can expose
> the entire guest to exploitation. That's not even getting into
> the bag of mount() tricks.
>
>
> > In addition to that, LSMs on host can block setting "user.*" xattrs by
> > virtiofsd domain only for additional protection.
>
> Try thinking outside the SELinux box briefly, if you possibly can.
> An LSM that implements just Bell & LaPadula isn't going to have a
> "virtiofs domain". Neither is a Smack "3 domain" system. Smack doesn't
> distinguish writing user xattrs from writing other file attributes
> in policy. Your argument requires a fine grained policy a'la SELinux.
> And an application specific SELinux policy at that.
>
> > If LSMs are not configured,
> > then hiding the directory is the solution.
>
> It's not a solution at all. It's wishful thinking that
> some admin is going to do absolutely everything right, will
> never make a mistake and will never, ever, read the mount(2)
> man page.
It's not worse than hoping that every admin is going to set up the
right permissions on a file backing the disk image of a Virtual
Machine.
And I'm no simply justifying one bad thing with another thing that's
even worse, I'm just trying to realign our expectations with the
current threat model of Virtualization in Linux. The host is protected
against the guest, but the guest is *not* implicitly protected against
the host. Everything depends on the host's admin good faith and even
better security practices.
This is the very reason why Virtualization-based Confidential
Computing technologies such as AMD SEV and Intel TDX are attracting so
much attention lately, as they aim to close the gap and also protect
guest against the host (as a necessary step to build trust on its
contents and behavior). But this is outside virtio-fs's scope.
Sergio.
* Casey Schaufler ([email protected]) wrote:
> On 9/2/2021 1:06 PM, Vivek Goyal wrote:
> > If LSMs are not configured,
> > then hiding the directory is the solution.
>
> It's not a solution at all. It's wishful thinking that
> some admin is going to do absolutely everything right, will
> never make a mistake and will never, ever, read the mount(2)
> man page.
That is why we run our virtiofsd with a sandbox setup and seccomp; and
frankly anything we can or could turn on we would.
> > So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> > solution. I don't understand that part.
>
> It comes back to your design, which is fundamentally flawed. You
> can't store system security information in an attribute that can
> be manipulated by untrusted entities. That's why we have system.*
> xattrs. You want to have an attribute on the host that maps to a
> security attribute on the guest. The host has to protect the attribute
> on the guest with mechanisms of comparable strength as the guest's
> mechanisms.
Can you just explain this line to me a bit more:
> Otherwise you can't trust the guest with host data.
Note we're not trying to trust the guest with the host data here;
we're trying to allow the guest to store the data on the host, while
trusting the host.
>
> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
> mechanism as implemented today won't scale to the hundreds of individual
> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
> I'm not convinced that there isn't a way to accomplish what you're
> trying to do without privilege, but this isn't it, and I don't know
> what is. Sorry.
>
> > Also if directory is not hidden, unprivileged users can change file
> > data and other metadata.
>
> I assumed that you've taken that into account. Are you saying that
> isn't going to be done correctly either?
>
> > Why that's not a concern and why there is
> > so much of focus only security xattr.
>
> As with an NFS mount, the assumption is that UID 567 (or its magically
> mapped equivalent) has the same access rights on both the server/host
> and client/guest. I'm not worried about the mode bits because they are
> presented consistently on both machines. If, on the other hand, an
> attribute used to determine access is security.esprit on the guest and
> user.security.esprit on the host, the unprivileged user on the host
> can defeat the privilege requirements on the guest. That's why.
We're OK with that; remember that the host can do wth it likes to the
guest anyway - it can just go in and poke at the guests RAM if it wants
to do something evil to the guest.
We wouldn't suggest using a scheme like this once you have
encrypted/protected guest RAM for example (SEV/TDX etc)
> > If you were to block modification
> > of file then you will have rely on LSMs.
>
> No. We're talking about the semantics of the xattr namespaces.
> LSMs can further constrain access to xattrs, but the basic rules
> of access to the user.* and security.* attributes are different
> in any case. This is by design.
I'm happy if you can suggest somewhere else to store the guests xattr
data other than in one of the hosts xattr's - the challenge is doing
that in a non-racy way, and making sure that the xattr's never get
associated with the wrong file as seen by a guest.
> > And if LSMs are not configured,
> > then we will rely on shared directory not being visible.
>
> LSMs are not the problem. LSMs use security.* xattrs, which is why
> they come up in the discussion.
>
> > Can you please help me understand why hiding shared directory from
> > unprivileged users is not a solution
>
> Maybe you can describe the mechanism you use to "hide" a shared directory
> on the host. If the filesystem is mounted on the host it seems unlikely
> that you can provide a convincing argument for sufficient protection.
Why? What can a guests fs mounted on the host, under one of the
directories that's already typically used for container fs's do - it's
already what fileservers, and existing container systems do.
Dave
> > (With both LSMs configured or
> > not configured on host). That's a requirement for virtiofs anyway.
> > And if we agree on that, then I don't see why using "user.*" xattrs
> > for storing guest sercurity attributes is a problem.
> >
> > Thanks
> > Vivek
> >
>
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
On 9/6/2021 7:55 AM, Dr. David Alan Gilbert wrote:
> * Casey Schaufler ([email protected]) wrote:
>> On 9/2/2021 1:06 PM, Vivek Goyal wrote:
>>> If LSMs are not configured,
>>> then hiding the directory is the solution.
>> It's not a solution at all. It's wishful thinking that
>> some admin is going to do absolutely everything right, will
>> never make a mistake and will never, ever, read the mount(2)
>> man page.
> That is why we run our virtiofsd with a sandbox setup and seccomp; and
> frankly anything we can or could turn on we would.
That doesn't address my concern at all. Being able to create an
environment in which a feature can be used safely does not make
the feature safe.
> So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> solution. I don't understand that part.
Sure you do. If you didn't, you wouldn't be so concerned about
requiring CAP_SYS_ADMIN. You're trying hard to avoid taking the
level of responsibility that running with privilege requires.
To do that, you're introducing a massive security hole, a backdoor
into the file system security attributes.
>> It comes back to your design, which is fundamentally flawed. You
>> can't store system security information in an attribute that can
>> be manipulated by untrusted entities. That's why we have system.*
>> xattrs. You want to have an attribute on the host that maps to a
>> security attribute on the guest. The host has to protect the attribute
>> on the guest with mechanisms of comparable strength as the guest's
>> mechanisms.
> Can you just explain this line to me a bit more:
>> Otherwise you can't trust the guest with host data.
> Note we're not trying to trust the guest with the host data here;
> we're trying to allow the guest to store the data on the host, while
> trusting the host.
But you can't trust the host! You're allowing unprivileged processes
on the host to modify security state of the guest.
>> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
>> mechanism as implemented today won't scale to the hundreds of individual
>> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
>> I'm not convinced that there isn't a way to accomplish what you're
>> trying to do without privilege, but this isn't it, and I don't know
>> what is. Sorry.
>>
>>> Also if directory is not hidden, unprivileged users can change file
>>> data and other metadata.
>> I assumed that you've taken that into account. Are you saying that
>> isn't going to be done correctly either?
>>
>>> Why that's not a concern and why there is
>>> so much of focus only security xattr.
>> As with an NFS mount, the assumption is that UID 567 (or its magically
>> mapped equivalent) has the same access rights on both the server/host
>> and client/guest. I'm not worried about the mode bits because they are
>> presented consistently on both machines. If, on the other hand, an
>> attribute used to determine access is security.esprit on the guest and
>> user.security.esprit on the host, the unprivileged user on the host
>> can defeat the privilege requirements on the guest. That's why.
> We're OK with that;
I understand that. I am not OK with that.
> remember that the host can do wth it likes to the
> guest anyway
We're not talking about "the host", we're talking about an
unprivileged user on the host.
> - it can just go in and poke at the guests RAM if it wants
> to do something evil to the guest.
> We wouldn't suggest using a scheme like this once you have
> encrypted/protected guest RAM for example (SEV/TDX etc)
>
>>> If you were to block modification
>>> of file then you will have rely on LSMs.
>> No. We're talking about the semantics of the xattr namespaces.
>> LSMs can further constrain access to xattrs, but the basic rules
>> of access to the user.* and security.* attributes are different
>> in any case. This is by design.
> I'm happy if you can suggest somewhere else to store the guests xattr
> data other than in one of the hosts xattr's - the challenge is doing
> that in a non-racy way, and making sure that the xattr's never get
> associated with the wrong file as seen by a guest.
I'm sorry, but I've got a bunch of other stuff on my plate.
I've already suggested implementing xattr namespaces a'la user
namespaces, but I understand that is beyond the scope of your
current needs, and has its own set of dragons.
>>> And if LSMs are not configured,
>>> then we will rely on shared directory not being visible.
>> LSMs are not the problem. LSMs use security.* xattrs, which is why
>> they come up in the discussion.
>>
>>> Can you please help me understand why hiding shared directory from
>>> unprivileged users is not a solution
>> Maybe you can describe the mechanism you use to "hide" a shared directory
>> on the host. If the filesystem is mounted on the host it seems unlikely
>> that you can provide a convincing argument for sufficient protection.
> Why?
Because 99-44/100% of admins out there aren't as skilled at "hiding"
data as you are. Many (I almost said "most". I'm still not sure which.)
of them don't even know how to use mode bits correctly.
> What can a guests fs mounted on the host, under one of the
> directories that's already typically used for container fs's do - it's
> already what fileservers, and existing container systems do.
>
> Dave
>
>
>
>>> (With both LSMs configured or
>>> not configured on host). That's a requirement for virtiofs anyway.
>>> And if we agree on that, then I don't see why using "user.*" xattrs
>>> for storing guest sercurity attributes is a problem.
>>>
>>> Thanks
>>> Vivek
>>>
On Mon, Sep 13, 2021 at 12:05:07PM -0700, Casey Schaufler wrote:
> On 9/6/2021 7:55 AM, Dr. David Alan Gilbert wrote:
> > * Casey Schaufler ([email protected]) wrote:
> >> On 9/2/2021 1:06 PM, Vivek Goyal wrote:
> >>> If LSMs are not configured,
> >>> then hiding the directory is the solution.
> >> It's not a solution at all. It's wishful thinking that
> >> some admin is going to do absolutely everything right, will
> >> never make a mistake and will never, ever, read the mount(2)
> >> man page.
> > That is why we run our virtiofsd with a sandbox setup and seccomp; and
> > frankly anything we can or could turn on we would.
>
> That doesn't address my concern at all. Being able to create an
> environment in which a feature can be used safely does not make
> the feature safe.
That's the requirement of virtiofs shared directory. It is a shared
directory, potentially being used by untrusted guest. So it should
not be accessible to unprivileged entities on host.
Same is the requirement for regular containers and that's why
podman (and possibly other container managers), make top level
storage directory only readable and searchable by root, so that
unpriveleged entities on host can not access container root filesystem
data.
I think similar requirements are there for idmapped mounts. One
will shift container images into user namespaces. And that means,
through idmapped mounts one can write files which will actually
show up as owned by root in original directory. And that original
directory should not be accessible to unprivileged users otherwise
user found an easy way to drop root owned files on system and
exectute these.
So more and more use cases are there which are relying on directory
not being accessible to unprivileged users.
I understand it is not going to be easy to get the configurations
right all the time. But orchestration tools can be helpful. container
managers can make sure kata container rootfs are not accessible.
libvirt can probably warn if shared directory is accessible to
unprivileged users. And all that will help in getting configuration
right.
>
>
> > So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> > solution. I don't understand that part.
>
> Sure you do. If you didn't, you wouldn't be so concerned about
> requiring CAP_SYS_ADMIN. You're trying hard to avoid taking the
> level of responsibility that running with privilege requires.
Running with minimal capabilities is always desired. So we want
to do away with CAP_SYS_ADMIN. And this reduces our risk in
case virtiofsd gets compromised.
I thought we had similar reasons that we did not want setuid
root binaries and wanted to give them limited set of capabilites
depending on what they are doing.
> To do that, you're introducing a massive security hole, a backdoor
> into the file system security attributes.
Shared directory is not accessible to unprivileged entities. If
configuration is not right, then it is user's problem and we
need to fix that.
>
> >> It comes back to your design, which is fundamentally flawed. You
> >> can't store system security information in an attribute that can
> >> be manipulated by untrusted entities. That's why we have system.*
> >> xattrs. You want to have an attribute on the host that maps to a
> >> security attribute on the guest. The host has to protect the attribute
> >> on the guest with mechanisms of comparable strength as the guest's
> >> mechanisms.
> > Can you just explain this line to me a bit more:
> >> Otherwise you can't trust the guest with host data.
> > Note we're not trying to trust the guest with the host data here;
> > we're trying to allow the guest to store the data on the host, while
> > trusting the host.
>
> But you can't trust the host! You're allowing unprivileged processes
> on the host to modify security state of the guest.
No we are not. Shared directory should not be accessible to
unpriviliged entities on host.
>
> >> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
> >> mechanism as implemented today won't scale to the hundreds of individual
> >> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
> >> I'm not convinced that there isn't a way to accomplish what you're
> >> trying to do without privilege, but this isn't it, and I don't know
> >> what is. Sorry.
> >>
> >>> Also if directory is not hidden, unprivileged users can change file
> >>> data and other metadata.
> >> I assumed that you've taken that into account. Are you saying that
> >> isn't going to be done correctly either?
> >>
> >>> Why that's not a concern and why there is
> >>> so much of focus only security xattr.
> >> As with an NFS mount, the assumption is that UID 567 (or its magically
> >> mapped equivalent) has the same access rights on both the server/host
> >> and client/guest. I'm not worried about the mode bits because they are
> >> presented consistently on both machines. If, on the other hand, an
> >> attribute used to determine access is security.esprit on the guest and
> >> user.security.esprit on the host, the unprivileged user on the host
> >> can defeat the privilege requirements on the guest. That's why.
> > We're OK with that;
>
> I understand that. I am not OK with that.
Unprivileged entities can't modify "user.viritiofs.security.selinux"
as shared directory is not accessible to them.
Sorry, I have to repeat this so many times because you are
completely ignoring this requirement saying users will get it
wrong. Sure if they do get it wrong, they need to fix it. But
that does not mean we start giving CAP_SYS_ADMIN to daemon.
If shared direcotry is accessible to unprivileged entities, game
is already over (setuid root binaries). Being able to modify
security attributes of a file seems like such a minor concern
in comparison.
>
> > remember that the host can do wth it likes to the
> > guest anyway
>
> We're not talking about "the host", we're talking about an
> unprivileged user on the host.
>
> > - it can just go in and poke at the guests RAM if it wants
> > to do something evil to the guest.
> > We wouldn't suggest using a scheme like this once you have
> > encrypted/protected guest RAM for example (SEV/TDX etc)
> >
> >>> If you were to block modification
> >>> of file then you will have rely on LSMs.
> >> No. We're talking about the semantics of the xattr namespaces.
> >> LSMs can further constrain access to xattrs, but the basic rules
> >> of access to the user.* and security.* attributes are different
> >> in any case. This is by design.
> > I'm happy if you can suggest somewhere else to store the guests xattr
> > data other than in one of the hosts xattr's - the challenge is doing
> > that in a non-racy way, and making sure that the xattr's never get
> > associated with the wrong file as seen by a guest.
>
> I'm sorry, but I've got a bunch of other stuff on my plate.
> I've already suggested implementing xattr namespaces a'la user
> namespaces, but I understand that is beyond the scope of your
> current needs, and has its own set of dragons.
>
> >>> And if LSMs are not configured,
> >>> then we will rely on shared directory not being visible.
> >> LSMs are not the problem. LSMs use security.* xattrs, which is why
> >> they come up in the discussion.
> >>
> >>> Can you please help me understand why hiding shared directory from
> >>> unprivileged users is not a solution
> >> Maybe you can describe the mechanism you use to "hide" a shared directory
> >> on the host. If the filesystem is mounted on the host it seems unlikely
> >> that you can provide a convincing argument for sufficient protection.
> > Why?
>
> Because 99-44/100% of admins out there aren't as skilled at "hiding"
> data as you are. Many (I almost said "most". I'm still not sure which.)
> of them don't even know how to use mode bits correctly.
We need to rely on orchestration tools to this by default.
podman already does that for containers. We probably need to
add something to libvirt too and warn users.
Vivek
On 9/14/2021 5:51 AM, Vivek Goyal wrote:
> Sorry, I have to repeat this so many times because you are
> completely ignoring this requirement saying users will get it
> wrong.
And I keep repeating myself because you are still wrong.
Configuration won't fix the problem. Tools won't fix the
problem. Your scheme is broken. Don't do it.
On Tue, Sep 14, 2021 at 8:52 AM Vivek Goyal <[email protected]> wrote:
> Same is the requirement for regular containers and that's why
> podman (and possibly other container managers), make top level
> storage directory only readable and searchable by root, so that
> unpriveleged entities on host can not access container root filesystem
> data.
Note--if that directory is on NFS, making it readable and searchable
by root is very weak protection, since it's often possible for an
attacker to guess filehandles and access objects without the need for
directory lookups.
--b.
On Tue, Sep 14, 2021 at 09:59:19AM -0400, Bruce Fields wrote:
> On Tue, Sep 14, 2021 at 8:52 AM Vivek Goyal <[email protected]> wrote:
> > Same is the requirement for regular containers and that's why
> > podman (and possibly other container managers), make top level
> > storage directory only readable and searchable by root, so that
> > unpriveleged entities on host can not access container root filesystem
> > data.
>
> Note--if that directory is on NFS, making it readable and searchable
> by root is very weak protection, since it's often possible for an
> attacker to guess filehandles and access objects without the need for
> directory lookups.
open_by_handle_at() requires CAP_DAC_READ_SEARCH. And if you have
CAP_DAC_READ_SEARCH, you don't need to even guess file handles. You
should be able to read/search through all directories, IIUC.
So how does one make sure that shared directory on host is not
accessible to unprivileged entities. If making directory accessible
to root only is weaker security, what are the options for stronger
security.
Vivek
On Tue, Sep 14, 2021 at 10:32 AM Vivek Goyal <[email protected]> wrote:
> open_by_handle_at() requires CAP_DAC_READ_SEARCH.
Or some sort of access to the network. If you can send rpc requests
to the nfs server that appear to be from someone with access to the
export, you can guess filehandles that allow access to objects under
that directory. You'll need access to particular objects, but you
won't need read or lookup access to the directory.
You can prevent that if you set things up right, but these
filehandle-issues are poorly understood, and people often forget to
take them into account.
--b.
> And if you have
> CAP_DAC_READ_SEARCH, you don't need to even guess file handles. You
> should be able to read/search through all directories, IIUC.
>
> So how does one make sure that shared directory on host is not
> accessible to unprivileged entities. If making directory accessible
> to root only is weaker security, what are the options for stronger
> security.
On Tue, Sep 14, 2021 at 10:32:13AM -0400, Vivek Goyal wrote:
Good morning, I hope the day is going well for everyone.
> On Tue, Sep 14, 2021 at 09:59:19AM -0400, Bruce Fields wrote:
> > On Tue, Sep 14, 2021 at 8:52 AM Vivek Goyal <[email protected]> wrote:
> > > Same is the requirement for regular containers and that's why
> > > podman (and possibly other container managers), make top level
> > > storage directory only readable and searchable by root, so that
> > > unpriveleged entities on host can not access container root filesystem
> > > data.
> >
> > Note--if that directory is on NFS, making it readable and searchable
> > by root is very weak protection, since it's often possible for an
> > attacker to guess filehandles and access objects without the need for
> > directory lookups.
> open_by_handle_at() requires CAP_DAC_READ_SEARCH. And if you have
> CAP_DAC_READ_SEARCH, you don't need to even guess file handles. You
> should be able to read/search through all directories, IIUC.
>
> So how does one make sure that shared directory on host is not
> accessible to unprivileged entities. If making directory accessible
> to root only is weaker security, what are the options for stronger
> security.
I've been watching this thread, with some interest, given what we have
been working on with respect to providing a new security framework
that merges IMA and LSM and external security co-processor technology.
Some observations based on those experiences and this thread.
Casey is an expert on MAC and capability based security systems,
unfortunately for our industry, particularly bog standard system
administrators, a rarefied set of skills. It may be helpful to
consider his concerns and position on the issues involved in the
framework of the number of systems that have, and blog posts that
recommend, setting 'selinux=0' on the kernel command-line.
I believe the best summary of his position on this issue, is the
notion that placing security labels, even in transitive form in user
accessible attributes, subordinates the security of the guest system,
regardless of the MAC policy it implements, to the DAC based policy on
the host system.
Given that, there are no legitimate security guarantees that are
inferrable based on the guest MAC policy.
A legitimate pundit, could and probably should question, in the face
of container filesystems and virtual machine images, whether any type
of inferrable security guarantees are possible, but that is a question
and argument for another day.
I didn't see any mention of EVM brought up in these discussions, which
may provide some options to improve the security integrity state of
the guest.
The 800 pound gorilla in the corner in all of this, is that inferrable
security guarantees in guests require a certifiable chain of trust
from the creator of the object to the kernel context that is making
the security gating decisions on the object. A hard to implement and
prove concept in bare metal trusted systems, let alone the myriad of
edge cases lurking in namespaced and virtual environments.
Which, in a nod to the other corner of the ring, may simply mean, with
our current state of deployable technology, you pay your money and
take your chances in these virtual environments. Which would in turn
support the notion of a minimum security, ie. DAC, based effort.
> Vivek
Have a good remainder of the week.
Dr. Greg
As always,
Dr. Greg Wettstein, Ph.D, Worker Autonomously self-defensive
Enjellic Systems Development, LLC IOT platforms and edge devices.
4206 N. 19th Ave.
Fargo, ND 58102
PH: 701-281-1686 EMAIL: [email protected]
------------------------------------------------------------------------------
"This place is so screwed up. It's just like the Titanic, only
we don't even have a band playing.
-- Terrance George Wieland
Resurrection.