[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Tomeu Vizoso]

Labelling of files in a NFSv4.2 currently fails with ENOTSUPP because
the mount point doesn't have SBLABEL_MNT.

Add specific condition for NFS4 filesystems so it gets correctly
labeled.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: J. Bruce Fields <bfields@redhat.com>

---

Hi,

cannot remotely say that I currently understand how selinux is expected
to work within NFS mounts, but this change allowed me to fully boot AOSP
with its rootfs and ramdisk on a single NFS share.

Thanks,

Tomeu
---
 security/selinux/hooks.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0c2ac318aa7f..71cd1d8c67c2 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -485,6 +485,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
 		!strcmp(sb->s_type->name, "debugfs") ||
 		!strcmp(sb->s_type->name, "tracefs") ||
 		!strcmp(sb->s_type->name, "rootfs") ||
+		!strcmp(sb->s_type->name, "nfs4") ||
 		(selinux_policycap_cgroupseclabel &&
 		 (!strcmp(sb->s_type->name, "cgroup") ||
 		  !strcmp(sb->s_type->name, "cgroup2")));
-- 
2.9.3

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Tomeu Vizoso]

Labelling of files in a NFSv4.2 currently fails with ENOTSUPP because
the mount point doesn't have SBLABEL_MNT.

Add specific condition for NFS4 filesystems so it gets correctly
labeled.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: J. Bruce Fields <bfields@redhat.com>

---

Hi,

cannot remotely say that I currently understand how selinux is expected
to work within NFS mounts, but this change allowed me to fully boot AOSP
with its rootfs and ramdisk on a single NFS share.

Thanks,

Tomeu
---
 security/selinux/hooks.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0c2ac318aa7f..71cd1d8c67c2 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -485,6 +485,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
 		!strcmp(sb->s_type->name, "debugfs") ||
 		!strcmp(sb->s_type->name, "tracefs") ||
 		!strcmp(sb->s_type->name, "rootfs") ||
+		!strcmp(sb->s_type->name, "nfs4") ||
 		(selinux_policycap_cgroupseclabel &&
 		 (!strcmp(sb->s_type->name, "cgroup") ||
 		  !strcmp(sb->s_type->name, "cgroup2")));
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[J. Bruce Fields]

On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> Labelling of files in a NFSv4.2 currently fails with ENOTSUPP because
> the mount point doesn't have SBLABEL_MNT.
> 
> Add specific condition for NFS4 filesystems so it gets correctly
> labeled.

Huh.  Looking at the code, I think this is meant to be handled by the
SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some time
ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is it a
recent regression (in which case, what's the latest kernel that worked
for you)?

--b.

> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: J. Bruce Fields <bfields@redhat.com>
> 
> ---
> 
> Hi,
> 
> cannot remotely say that I currently understand how selinux is expected
> to work within NFS mounts, but this change allowed me to fully boot AOSP
> with its rootfs and ramdisk on a single NFS share.
> 
> Thanks,
> 
> Tomeu
> ---
>  security/selinux/hooks.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0c2ac318aa7f..71cd1d8c67c2 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -485,6 +485,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
>  		!strcmp(sb->s_type->name, "debugfs") ||
>  		!strcmp(sb->s_type->name, "tracefs") ||
>  		!strcmp(sb->s_type->name, "rootfs") ||
> +		!strcmp(sb->s_type->name, "nfs4") ||
>  		(selinux_policycap_cgroupseclabel &&
>  		 (!strcmp(sb->s_type->name, "cgroup") ||
>  		  !strcmp(sb->s_type->name, "cgroup2")));
> -- 
> 2.9.3
> 

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[J. Bruce Fields]

On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> Labelling of files in a NFSv4.2 currently fails with ENOTSUPP because
> the mount point doesn't have SBLABEL_MNT.
> 
> Add specific condition for NFS4 filesystems so it gets correctly
> labeled.

Huh.  Looking at the code, I think this is meant to be handled by the
SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some time
ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is it a
recent regression (in which case, what's the latest kernel that worked
for you)?

--b.

> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: J. Bruce Fields <bfields@redhat.com>
> 
> ---
> 
> Hi,
> 
> cannot remotely say that I currently understand how selinux is expected
> to work within NFS mounts, but this change allowed me to fully boot AOSP
> with its rootfs and ramdisk on a single NFS share.
> 
> Thanks,
> 
> Tomeu
> ---
>  security/selinux/hooks.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0c2ac318aa7f..71cd1d8c67c2 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -485,6 +485,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
>  		!strcmp(sb->s_type->name, "debugfs") ||
>  		!strcmp(sb->s_type->name, "tracefs") ||
>  		!strcmp(sb->s_type->name, "rootfs") ||
> +		!strcmp(sb->s_type->name, "nfs4") ||
>  		(selinux_policycap_cgroupseclabel &&
>  		 (!strcmp(sb->s_type->name, "cgroup") ||
>  		  !strcmp(sb->s_type->name, "cgroup2")));
> -- 
> 2.9.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Tomeu Vizoso]

On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com> wrote:
> On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
>> Labelling of files in a NFSv4.2 currently fails with ENOTSUPP because
>> the mount point doesn't have SBLABEL_MNT.
>>
>> Add specific condition for NFS4 filesystems so it gets correctly
>> labeled.
>
> Huh.  Looking at the code, I think this is meant to be handled by the
> SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some time
> ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is it a
> recent regression (in which case, what's the latest kernel that worked
> for you)?

I have seen this on 4.11-rc4, but I never tried to get this working before.

I will try to find time to see why SECURITY_FS_USE_NATIVE isn't working here.

Thanks,

Tomeu

> --b.
>
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Cc: J. Bruce Fields <bfields@redhat.com>
>>
>> ---
>>
>> Hi,
>>
>> cannot remotely say that I currently understand how selinux is expected
>> to work within NFS mounts, but this change allowed me to fully boot AOSP
>> with its rootfs and ramdisk on a single NFS share.
>>
>> Thanks,
>>
>> Tomeu
>> ---
>>  security/selinux/hooks.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 0c2ac318aa7f..71cd1d8c67c2 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -485,6 +485,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
>>               !strcmp(sb->s_type->name, "debugfs") ||
>>               !strcmp(sb->s_type->name, "tracefs") ||
>>               !strcmp(sb->s_type->name, "rootfs") ||
>> +             !strcmp(sb->s_type->name, "nfs4") ||
>>               (selinux_policycap_cgroupseclabel &&
>>                (!strcmp(sb->s_type->name, "cgroup") ||
>>                 !strcmp(sb->s_type->name, "cgroup2")));
>> --
>> 2.9.3
>>

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Tomeu Vizoso]

On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com> wrote:
> On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
>> Labelling of files in a NFSv4.2 currently fails with ENOTSUPP because
>> the mount point doesn't have SBLABEL_MNT.
>>
>> Add specific condition for NFS4 filesystems so it gets correctly
>> labeled.
>
> Huh.  Looking at the code, I think this is meant to be handled by the
> SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some time
> ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is it a
> recent regression (in which case, what's the latest kernel that worked
> for you)?

I have seen this on 4.11-rc4, but I never tried to get this working before.

I will try to find time to see why SECURITY_FS_USE_NATIVE isn't working here.

Thanks,

Tomeu

> --b.
>
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Cc: J. Bruce Fields <bfields@redhat.com>
>>
>> ---
>>
>> Hi,
>>
>> cannot remotely say that I currently understand how selinux is expected
>> to work within NFS mounts, but this change allowed me to fully boot AOSP
>> with its rootfs and ramdisk on a single NFS share.
>>
>> Thanks,
>>
>> Tomeu
>> ---
>>  security/selinux/hooks.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 0c2ac318aa7f..71cd1d8c67c2 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -485,6 +485,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
>>               !strcmp(sb->s_type->name, "debugfs") ||
>>               !strcmp(sb->s_type->name, "tracefs") ||
>>               !strcmp(sb->s_type->name, "rootfs") ||
>> +             !strcmp(sb->s_type->name, "nfs4") ||
>>               (selinux_policycap_cgroupseclabel &&
>>                (!strcmp(sb->s_type->name, "cgroup") ||
>>                 !strcmp(sb->s_type->name, "cgroup2")));
>> --
>> 2.9.3
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Stephen Smalley]

On Thu, 2017-03-30 at 09:49 +0200, Tomeu Vizoso wrote:
> On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com>
> wrote:
> > On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> > > Labelling of files in a NFSv4.2 currently fails with ENOTSUPP
> > > because
> > > the mount point doesn't have SBLABEL_MNT.
> > > 
> > > Add specific condition for NFS4 filesystems so it gets correctly
> > > labeled.
> > 
> > Huh.  Looking at the code, I think this is meant to be handled by
> > the
> > SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some
> > time
> > ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is it a
> > recent regression (in which case, what's the latest kernel that
> > worked
> > for you)?
> 
> I have seen this on 4.11-rc4, but I never tried to get this working
> before.
> 
> I will try to find time to see why SECURITY_FS_USE_NATIVE isn't
> working here.

Does your exports file specify the "security_label" option, e.g.
/path/to/dir example.com(rw,security_label)

It appears that with recent kernels that is now required; otherwise,
the mount defaults to not enabling native labeling and all of the files
are treated as having a single, fixed label defined by the client
policy (and hence setxattr is not supported).  This was kernel commit
32ddd944a056c786f6acdd95ed29e994adc613a2.  I don't recall seeing any
discussion of this on selinux list.  I understand the rationale, but it
seems like a user-visible regression and at the very least, it seems
odd that they didn't just use "seclabel" as the kernel does in
/proc/mounts to signify a filesystem that supports security labeling by
userspace.

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Stephen Smalley]

On Thu, 2017-03-30 at 09:49 +0200, Tomeu Vizoso wrote:
> On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com>
> wrote:
> > On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> > > Labelling of files in a NFSv4.2 currently fails with ENOTSUPP
> > > because
> > > the mount point doesn't have SBLABEL_MNT.
> > > 
> > > Add specific condition for NFS4 filesystems so it gets correctly
> > > labeled.
> > 
> > Huh.??Looking at the code, I think this is meant to be handled by
> > the
> > SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some
> > time
> > ago by 9fc2b4b436cf.??What kernel are you seeing this on???Is it a
> > recent regression (in which case, what's the latest kernel that
> > worked
> > for you)?
> 
> I have seen this on 4.11-rc4, but I never tried to get this working
> before.
> 
> I will try to find time to see why SECURITY_FS_USE_NATIVE isn't
> working here.

Does your exports file specify the "security_label" option, e.g.
/path/to/dir example.com(rw,security_label)

It appears that with recent kernels that is now required; otherwise,
the mount defaults to not enabling native labeling and all of the files
are treated as having a single, fixed label defined by the client
policy (and hence setxattr is not supported).  This was kernel commit
32ddd944a056c786f6acdd95ed29e994adc613a2.  I don't recall seeing any
discussion of this on selinux list.  I understand the rationale, but it
seems like a user-visible regression and at the very least, it seems
odd that they didn't just use "seclabel" as the kernel does in
/proc/mounts to signify a filesystem that supports security labeling by
userspace.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[J. Bruce Fields]

On Thu, Mar 30, 2017 at 01:27:07PM -0400, Stephen Smalley wrote:
> On Thu, 2017-03-30 at 09:49 +0200, Tomeu Vizoso wrote:
> > On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com>
> > wrote:
> > > On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> > > > Labelling of files in a NFSv4.2 currently fails with ENOTSUPP
> > > > because
> > > > the mount point doesn't have SBLABEL_MNT.
> > > > 
> > > > Add specific condition for NFS4 filesystems so it gets correctly
> > > > labeled.
> > > 
> > > Huh.  Looking at the code, I think this is meant to be handled by
> > > the
> > > SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some
> > > time
> > > ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is it a
> > > recent regression (in which case, what's the latest kernel that
> > > worked
> > > for you)?
> > 
> > I have seen this on 4.11-rc4, but I never tried to get this working
> > before.
> > 
> > I will try to find time to see why SECURITY_FS_USE_NATIVE isn't
> > working here.
> 
> Does your exports file specify the "security_label" option, e.g.
> /path/to/dir example.com(rw,security_label)

Oops, right, that should have been the first thing I asked about....

> It appears that with recent kernels that is now required; otherwise,
> the mount defaults to not enabling native labeling and all of the files
> are treated as having a single, fixed label defined by the client
> policy (and hence setxattr is not supported).  This was kernel commit
> 32ddd944a056c786f6acdd95ed29e994adc613a2.  I don't recall seeing any
> discussion of this on selinux list.  I understand the rationale, but it
> seems like a user-visible regression

It is.  I also want to keep new protocol upgrades free of user
regressions, which the 4.1->4.2 upgrade is in most cases if we turn on
security labeling by default.  So I was stuck choosing between two
regresisons, and figured 4.2 user depending on security labeling was
still the much rarer case.

So I'd like to keep security labeling off by default, but if there's
anything I can do to smooth the transition obviously that's good.

> and at the very least, it seems odd that they didn't just use
> "seclabel" as the kernel does in /proc/mounts to signify a filesystem
> that supports security labeling by userspace.

I see logic in sb_finish_set_opts() that sets SBLABEL_MNT in the
selinux_is_sblabel_mnt() case.  Doesn't that mean "seclabel" shows up in
/proc/mounts when we nfs sets SECURITY_LSM_NATIVE_LABELS?

I may not understand your comment, I'm pretty unfamiliar with this area.

--b.

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[J. Bruce Fields]

On Thu, Mar 30, 2017 at 01:27:07PM -0400, Stephen Smalley wrote:
> On Thu, 2017-03-30 at 09:49 +0200, Tomeu Vizoso wrote:
> > On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com>
> > wrote:
> > > On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> > > > Labelling of files in a NFSv4.2 currently fails with ENOTSUPP
> > > > because
> > > > the mount point doesn't have SBLABEL_MNT.
> > > > 
> > > > Add specific condition for NFS4 filesystems so it gets correctly
> > > > labeled.
> > > 
> > > Huh.??Looking at the code, I think this is meant to be handled by
> > > the
> > > SECURITY_FS_USE_NATIVE case--there was a similar failure fixed some
> > > time
> > > ago by 9fc2b4b436cf.??What kernel are you seeing this on???Is it a
> > > recent regression (in which case, what's the latest kernel that
> > > worked
> > > for you)?
> > 
> > I have seen this on 4.11-rc4, but I never tried to get this working
> > before.
> > 
> > I will try to find time to see why SECURITY_FS_USE_NATIVE isn't
> > working here.
> 
> Does your exports file specify the "security_label" option, e.g.
> /path/to/dir example.com(rw,security_label)

Oops, right, that should have been the first thing I asked about....

> It appears that with recent kernels that is now required; otherwise,
> the mount defaults to not enabling native labeling and all of the files
> are treated as having a single, fixed label defined by the client
> policy (and hence setxattr is not supported).  This was kernel commit
> 32ddd944a056c786f6acdd95ed29e994adc613a2.  I don't recall seeing any
> discussion of this on selinux list.  I understand the rationale, but it
> seems like a user-visible regression

It is.  I also want to keep new protocol upgrades free of user
regressions, which the 4.1->4.2 upgrade is in most cases if we turn on
security labeling by default.  So I was stuck choosing between two
regresisons, and figured 4.2 user depending on security labeling was
still the much rarer case.

So I'd like to keep security labeling off by default, but if there's
anything I can do to smooth the transition obviously that's good.

> and at the very least, it seems odd that they didn't just use
> "seclabel" as the kernel does in /proc/mounts to signify a filesystem
> that supports security labeling by userspace.

I see logic in sb_finish_set_opts() that sets SBLABEL_MNT in the
selinux_is_sblabel_mnt() case.  Doesn't that mean "seclabel" shows up in
/proc/mounts when we nfs sets SECURITY_LSM_NATIVE_LABELS?

I may not understand your comment, I'm pretty unfamiliar with this area.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Stephen Smalley]

On Thu, 2017-03-30 at 13:41 -0400, J. Bruce Fields wrote:
> On Thu, Mar 30, 2017 at 01:27:07PM -0400, Stephen Smalley wrote:
> > On Thu, 2017-03-30 at 09:49 +0200, Tomeu Vizoso wrote:
> > > On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com>
> > > wrote:
> > > > On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> > > > > Labelling of files in a NFSv4.2 currently fails with ENOTSUPP
> > > > > because
> > > > > the mount point doesn't have SBLABEL_MNT.
> > > > > 
> > > > > Add specific condition for NFS4 filesystems so it gets
> > > > > correctly
> > > > > labeled.
> > > > 
> > > > Huh.  Looking at the code, I think this is meant to be handled
> > > > by
> > > > the
> > > > SECURITY_FS_USE_NATIVE case--there was a similar failure fixed
> > > > some
> > > > time
> > > > ago by 9fc2b4b436cf.  What kernel are you seeing this on?  Is
> > > > it a
> > > > recent regression (in which case, what's the latest kernel that
> > > > worked
> > > > for you)?
> > > 
> > > I have seen this on 4.11-rc4, but I never tried to get this
> > > working
> > > before.
> > > 
> > > I will try to find time to see why SECURITY_FS_USE_NATIVE isn't
> > > working here.
> > 
> > Does your exports file specify the "security_label" option, e.g.
> > /path/to/dir example.com(rw,security_label)
> 
> Oops, right, that should have been the first thing I asked about....
> 
> > It appears that with recent kernels that is now required;
> > otherwise,
> > the mount defaults to not enabling native labeling and all of the
> > files
> > are treated as having a single, fixed label defined by the client
> > policy (and hence setxattr is not supported).  This was kernel
> > commit
> > 32ddd944a056c786f6acdd95ed29e994adc613a2.  I don't recall seeing
> > any
> > discussion of this on selinux list.  I understand the rationale,
> > but it
> > seems like a user-visible regression
> 
> It is.  I also want to keep new protocol upgrades free of user
> regressions, which the 4.1->4.2 upgrade is in most cases if we turn
> on
> security labeling by default.  So I was stuck choosing between two
> regresisons, and figured 4.2 user depending on security labeling was
> still the much rarer case.
> 
> So I'd like to keep security labeling off by default, but if there's
> anything I can do to smooth the transition obviously that's good.

Yes, I understand - wish though that it could have been communicated
better, e.g. on selinux list (unless I just missed it somehow).

> 
> > and at the very least, it seems odd that they didn't just use
> > "seclabel" as the kernel does in /proc/mounts to signify a
> > filesystem
> > that supports security labeling by userspace.
> 
> I see logic in sb_finish_set_opts() that sets SBLABEL_MNT in the
> selinux_is_sblabel_mnt() case.  Doesn't that mean "seclabel" shows up
> in
> /proc/mounts when we nfs sets SECURITY_LSM_NATIVE_LABELS?
> 
> I may not understand your comment, I'm pretty unfamiliar with this
> area.

Correct, I just meant it seems potentially confusing to users to use
"security_label" in exports when we show it as "seclabel" in
/proc/mounts.  I know, they are totally different namespaces (in the
conventional sense), but consistency might have been more user-
friendly.

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[Stephen Smalley]

On Thu, 2017-03-30 at 13:41 -0400, J. Bruce Fields wrote:
> On Thu, Mar 30, 2017 at 01:27:07PM -0400, Stephen Smalley wrote:
> > On Thu, 2017-03-30 at 09:49 +0200, Tomeu Vizoso wrote:
> > > On 29 March 2017 at 23:34, J. Bruce Fields <bfields@redhat.com>
> > > wrote:
> > > > On Wed, Mar 29, 2017 at 05:27:23PM +0200, Tomeu Vizoso wrote:
> > > > > Labelling of files in a NFSv4.2 currently fails with ENOTSUPP
> > > > > because
> > > > > the mount point doesn't have SBLABEL_MNT.
> > > > > 
> > > > > Add specific condition for NFS4 filesystems so it gets
> > > > > correctly
> > > > > labeled.
> > > > 
> > > > Huh.??Looking at the code, I think this is meant to be handled
> > > > by
> > > > the
> > > > SECURITY_FS_USE_NATIVE case--there was a similar failure fixed
> > > > some
> > > > time
> > > > ago by 9fc2b4b436cf.??What kernel are you seeing this on???Is
> > > > it a
> > > > recent regression (in which case, what's the latest kernel that
> > > > worked
> > > > for you)?
> > > 
> > > I have seen this on 4.11-rc4, but I never tried to get this
> > > working
> > > before.
> > > 
> > > I will try to find time to see why SECURITY_FS_USE_NATIVE isn't
> > > working here.
> > 
> > Does your exports file specify the "security_label" option, e.g.
> > /path/to/dir example.com(rw,security_label)
> 
> Oops, right, that should have been the first thing I asked about....
> 
> > It appears that with recent kernels that is now required;
> > otherwise,
> > the mount defaults to not enabling native labeling and all of the
> > files
> > are treated as having a single, fixed label defined by the client
> > policy (and hence setxattr is not supported).??This was kernel
> > commit
> > 32ddd944a056c786f6acdd95ed29e994adc613a2.??I don't recall seeing
> > any
> > discussion of this on selinux list.??I understand the rationale,
> > but it
> > seems like a user-visible regression
> 
> It is.??I also want to keep new protocol upgrades free of user
> regressions, which the 4.1->4.2 upgrade is in most cases if we turn
> on
> security labeling by default.??So I was stuck choosing between two
> regresisons, and figured 4.2 user depending on security labeling was
> still the much rarer case.
> 
> So I'd like to keep security labeling off by default, but if there's
> anything I can do to smooth the transition obviously that's good.

Yes, I understand - wish though that it could have been communicated
better, e.g. on selinux list (unless I just missed it somehow).

> 
> > and at the very least, it seems odd that they didn't just use
> > "seclabel" as the kernel does in /proc/mounts to signify a
> > filesystem
> > that supports security labeling by userspace.
> 
> I see logic in sb_finish_set_opts() that sets SBLABEL_MNT in the
> selinux_is_sblabel_mnt() case.??Doesn't that mean "seclabel" shows up
> in
> /proc/mounts when we nfs sets SECURITY_LSM_NATIVE_LABELS?
> 
> I may not understand your comment, I'm pretty unfamiliar with this
> area.

Correct, I just meant it seems potentially confusing to users to use
"security_label" in exports when we show it as "seclabel" in
/proc/mounts.  I know, they are totally different namespaces (in the
conventional sense), but consistency might have been more user-
friendly.


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[J. Bruce Fields]

On Thu, Mar 30, 2017 at 01:52:14PM -0400, Stephen Smalley wrote:
> On Thu, 2017-03-30 at 13:41 -0400, J. Bruce Fields wrote:
> > It is.  I also want to keep new protocol upgrades free of user
> > regressions, which the 4.1->4.2 upgrade is in most cases if we turn
> > on
> > security labeling by default.  So I was stuck choosing between two
> > regresisons, and figured 4.2 user depending on security labeling was
> > still the much rarer case.
> > 
> > So I'd like to keep security labeling off by default, but if there's
> > anything I can do to smooth the transition obviously that's good.
> 
> Yes, I understand - wish though that it could have been communicated
> better, e.g. on selinux list (unless I just missed it somehow).

No, I didn't think of it, apologies, I agree that would have been
smarter.

> > > and at the very least, it seems odd that they didn't just use
> > > "seclabel" as the kernel does in /proc/mounts to signify a
> > > filesystem
> > > that supports security labeling by userspace.
> > 
> > I see logic in sb_finish_set_opts() that sets SBLABEL_MNT in the
> > selinux_is_sblabel_mnt() case.  Doesn't that mean "seclabel" shows up
> > in
> > /proc/mounts when we nfs sets SECURITY_LSM_NATIVE_LABELS?
> > 
> > I may not understand your comment, I'm pretty unfamiliar with this
> > area.
> 
> Correct, I just meant it seems potentially confusing to users to use
> "security_label" in exports when we show it as "seclabel" in
> /proc/mounts.  I know, they are totally different namespaces (in the
> conventional sense), but consistency might have been more user-
> friendly.

Oh, got it.

We've had problems when NFS client mount and server export options are
spelled the same but have subtle differences in semantics (I'm thinking
of "async").  But maybe that wouldn't have been an issue here.

--b.

[PATCH] selinux: Fix SBLABEL_MNT for NFS mounts

[J. Bruce Fields]

On Thu, Mar 30, 2017 at 01:52:14PM -0400, Stephen Smalley wrote:
> On Thu, 2017-03-30 at 13:41 -0400, J. Bruce Fields wrote:
> > It is.??I also want to keep new protocol upgrades free of user
> > regressions, which the 4.1->4.2 upgrade is in most cases if we turn
> > on
> > security labeling by default.??So I was stuck choosing between two
> > regresisons, and figured 4.2 user depending on security labeling was
> > still the much rarer case.
> > 
> > So I'd like to keep security labeling off by default, but if there's
> > anything I can do to smooth the transition obviously that's good.
> 
> Yes, I understand - wish though that it could have been communicated
> better, e.g. on selinux list (unless I just missed it somehow).

No, I didn't think of it, apologies, I agree that would have been
smarter.

> > > and at the very least, it seems odd that they didn't just use
> > > "seclabel" as the kernel does in /proc/mounts to signify a
> > > filesystem
> > > that supports security labeling by userspace.
> > 
> > I see logic in sb_finish_set_opts() that sets SBLABEL_MNT in the
> > selinux_is_sblabel_mnt() case.??Doesn't that mean "seclabel" shows up
> > in
> > /proc/mounts when we nfs sets SECURITY_LSM_NATIVE_LABELS?
> > 
> > I may not understand your comment, I'm pretty unfamiliar with this
> > area.
> 
> Correct, I just meant it seems potentially confusing to users to use
> "security_label" in exports when we show it as "seclabel" in
> /proc/mounts.  I know, they are totally different namespaces (in the
> conventional sense), but consistency might have been more user-
> friendly.

Oh, got it.

We've had problems when NFS client mount and server export options are
spelled the same but have subtle differences in semantics (I'm thinking
of "async").  But maybe that wouldn't have been an issue here.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[Scott Mayhew]

Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
The problem is related to how filesystems are mounted in NFSv4.

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.

NFS servers with a modern nfs-utils package will automatically create a
pseudo fs to fill in the gaps (including the root itself) leading up to
the actual export, so it is uncommon these days for an NFS server to
have an explicit fsid=0 export.

Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
flag on an initialized superblock would ensure that SBLABEL_MNT is set
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option.

Scott Mayhew (2):
  selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
    initialized superblock
  nfs: update labeling behavior on a superblock when submounting

 fs/nfs/super.c           | 23 ++++++++++++++++++++++-
 security/selinux/hooks.c |  4 ++--
 2 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.9.3

[PATCH RFC 1/2] selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already initialized superblock

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.

Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
flag on an initialized superblock will ensure that SBLABEL_MNT is set
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 security/selinux/hooks.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..366ab86 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -730,7 +730,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	 * will be used for both mounts)
 	 */
 	if ((sbsec->flags & SE_SBINITIALIZED) && (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)
-	    && (num_opts == 0))
+	    && (num_opts == 0) && !(kern_flags & SECURITY_LSM_NATIVE_LABELS))
 		goto out;
 
 	root_isec = backing_inode_security_novalidate(root);
@@ -797,7 +797,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 		}
 	}
 
-	if (sbsec->flags & SE_SBINITIALIZED) {
+	if (sbsec->flags & SE_SBINITIALIZED && !(kern_flags & SECURITY_LSM_NATIVE_LABELS)) {
 		/* previously mounted with options, but not on this attempt? */
 		if ((sbsec->flags & SE_MNTMASK) && !num_opts)
 			goto out_double_mount;
-- 
2.9.3

[PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Scott Mayhew]

When the client traverses from filesystem exported without the
"security_label" option to one exported with the "security_label"
option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
security_sb_set_mnt_opts() so that the new superblock has SBLABEL_MNT
set in its security mount options.  Otherwise, attempts to set security
labels via setxattr over NFSv4.2 will fail.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..d7a3b89 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+	struct security_mnt_opts opts;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+	if (error)
+		goto err;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(NFS_SB(mount_info->cloned->sb)->caps & NFS_CAP_SECURITY_LABEL)) {
+		memset(&opts, 0, sizeof(opts));
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+		error = security_sb_set_mnt_opts(s, &opts, kflags, &kflags_out);
+		if (error)
+			goto err;
+
+		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+			NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+	}
+err:
+	return error;
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
-- 
2.9.3

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> When the client traverses from filesystem exported without the
> "security_label" option to one exported with the "security_label"
> option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> security_sb_set_mnt_opts() so that the new superblock has SBLABEL_MNT
> set in its security mount options.  Otherwise, attempts to set
> security
> labels via setxattr over NFSv4.2 will fail.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..d7a3b89 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +	struct security_mnt_opts opts;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +	if (error)
> +		goto err;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(NFS_SB(mount_info->cloned->sb)->caps &
> NFS_CAP_SECURITY_LABEL)) {
> +		memset(&opts, 0, sizeof(opts));
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +		error = security_sb_set_mnt_opts(s, &opts, kflags,
> &kflags_out);
> +		if (error)
> +			goto err;
> +
> +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +			NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	}
> +err:
> +	return error;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);

Could this clobber a context set via context= mount option?

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Scott Mayhew]

On Fri, 26 May 2017, Stephen Smalley wrote:

> On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > When the client traverses from filesystem exported without the
> > "security_label" option to one exported with the "security_label"
> > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > security_sb_set_mnt_opts() so that the new superblock has SBLABEL_MNT
> > set in its security mount options.  Otherwise, attempts to set
> > security
> > labels via setxattr over NFSv4.2 will fail.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..d7a3b89 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +	struct security_mnt_opts opts;
> > +
> >  	/* clone any lsm security options from the parent to the new
> > sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +	if (error)
> > +		goto err;
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > NFS_CAP_SECURITY_LABEL)) {
> > +		memset(&opts, 0, sizeof(opts));
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +		error = security_sb_set_mnt_opts(s, &opts, kflags,
> > &kflags_out);
> > +		if (error)
> > +			goto err;
> > +
> > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +			NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +	}
> > +err:
> > +	return error;
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> 
> Could this clobber a context set via context= mount option?

Argh, yes I suppose it could.  In my first attempt to fix this, I added
a security_sb_get_mnt_opts() hook to get the original mount options and
then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
security_sb_set_mnt_opts().  When I saw that security_sb_set_mnt_opts()
wouldn't allow me to change a superblock that had already been
initialized, I got rid of the hook and added the check in patch 1...
maybe a combination of the two is needed?

Testing it again now, I'm not sure the context= mount option is working
correctly with the latest kernel.

-Scott

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> On Fri, 26 May 2017, Stephen Smalley wrote:
> 
> > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > When the client traverses from filesystem exported without the
> > > "security_label" option to one exported with the "security_label"
> > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > security_sb_set_mnt_opts() so that the new superblock has
> > > SBLABEL_MNT
> > > set in its security mount options.  Otherwise, attempts to set
> > > security
> > > labels via setxattr over NFSv4.2 will fail.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 2f3822a..d7a3b89 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > *mntroot,
> > >  			  struct nfs_mount_info *mount_info)
> > >  {
> > > +	int error;
> > > +	unsigned long kflags = 0, kflags_out = 0;
> > > +	struct security_mnt_opts opts;
> > > +
> > >  	/* clone any lsm security options from the parent to the
> > > new
> > > sb */
> > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > rpc_ops->dir_inode_ops)
> > > 
> > >  		return -ESTALE;
> > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > NFS_CAP_SECURITY_LABEL)) {
> > > +		memset(&opts, 0, sizeof(opts));
> > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > +
> > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > kflags,
> > > &kflags_out);
> > > +		if (error)
> > > +			goto err;
> > > +
> > > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > +			NFS_SB(s)->caps &=
> > > ~NFS_CAP_SECURITY_LABEL;
> > > +	}
> > > +err:
> > > +	return error;
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > 
> > Could this clobber a context set via context= mount option?
> 
> Argh, yes I suppose it could.  In my first attempt to fix this, I
> added
> a security_sb_get_mnt_opts() hook to get the original mount options
> and
> then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
> security_sb_set_mnt_opts().  When I saw that
> security_sb_set_mnt_opts()
> wouldn't allow me to change a superblock that had already been
> initialized, I got rid of the hook and added the check in patch 1...
> maybe a combination of the two is needed?

Could we instead extend the security_sb_clone_mnt_opts() hook interface
to pass flags, and just handle it inside of the
selinux_sb_clone_mnt_opts() hook?  Then we don't have to change
selinux_sb_set_mnt_opts() handling, which may be called for mount(2)
from userspace, not just from NFS client code.  You might need to
refactor it to share common code, but we shouldn't alter its logic when
called normally.


> Testing it again now, I'm not sure the context= mount option is
> working
> correctly with the latest kernel.

Hmm...if not, then that's another regression that will need to be
addressed.

[PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.

Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c            | 18 +++++++++++++++++-
 include/linux/lsm_hooks.h |  4 +++-
 include/linux/security.h  |  8 ++++++--
 security/security.c       |  7 +++++--
 security/selinux/hooks.c  | 43 ++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..6a11535 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags, &kflags_out);
+	if (error)
+		goto err;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+err:
+	return error;
+
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e..2f54bfb 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1388,7 +1388,9 @@ union security_list_options {
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
-					struct super_block *newsb);
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags);
 	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..a55ae9c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb);
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags);
 int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
@@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
 }
 
 static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-					      struct super_block *newsb)
+					      struct super_block *newsb,
+					      unsigned long kern_flags,
+					      unsigned long *set_kern_flags)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index b9fea39..7b70ea2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 EXPORT_SYMBOL(security_sb_set_mnt_opts);
 
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb)
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags)
 {
-	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
+	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
+				kern_flags, set_kern_flags);
 }
 EXPORT_SYMBOL(security_sb_clone_mnt_opts);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..80d9acf 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block *sb)
 		       sb->s_id, sb->s_type->name);
 
 	sbsec->flags |= SE_SBINITIALIZED;
+
+	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
+	   leave the flag untouched because sb_clone_mnt_opts might be handing
+	   us a superblock that needs the flag to be cleared. */
 	if (selinux_is_sblabel_mnt(sb))
 		sbsec->flags |= SBLABEL_MNT;
+	else
+		sbsec->flags &= ~SBLABEL_MNT;
 
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
@@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
 }
 
 static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
-					struct super_block *newsb)
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags)
 {
+	int rc = 0;
 	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
 	struct superblock_security_struct *newsbsec = newsb->s_security;
 
@@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	 * mount options.  thus we can safely deal with this superblock later
 	 */
 	if (!ss_initialized)
-		return 0;
+		goto out;
+
+	if (kern_flags && !set_kern_flags) {
+		/* Specifying internal flags without providing a place to
+		 * place the results is not allowed */
+		rc = -EINVAL;
+		goto out;
+	}
 
 	/* how can we clone if the old one wasn't set up?? */
 	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
 
 	/* if fs is reusing a sb, make sure that the contexts match */
-	if (newsbsec->flags & SE_SBINITIALIZED)
-		return selinux_cmp_sb_context(oldsb, newsb);
+	if (newsbsec->flags & SE_SBINITIALIZED) {
+		rc = selinux_cmp_sb_context(oldsb, newsb);
+		goto out;
+	}
 
 	mutex_lock(&newsbsec->lock);
 
@@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	newsbsec->def_sid = oldsbsec->def_sid;
 	newsbsec->behavior = oldsbsec->behavior;
 
+	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
+			&& !(kern_flags & SECURITY_LSM_NATIVE_LABELS)
+			&& !set_context) {
+		rc = security_fs_use(newsb);
+		if (rc)
+			goto out_unlock;
+	}
+
+	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
+		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
+		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+	}
+
 	if (set_context) {
 		u32 sid = oldsbsec->mntpoint_sid;
 
@@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	}
 
 	sb_finish_set_opts(newsb);
+out_unlock:
 	mutex_unlock(&newsbsec->lock);
-	return 0;
+out:
+	return rc;
 }
 
 static int selinux_parse_opts_str(char *options,
-- 
2.9.3

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Thu, 01 Jun 2017, Scott Mayhew wrote:

> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after cloning
> the security mount options.  As a result, setxattr's of security labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 18 +++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 43 ++++++++++++++++++++++++++++++++++++++-----
>  5 files changed, 69 insertions(+), 11 deletions(-)

With this patch, the behavior and flags fields of the superblock_security_struct
and the nfs_server->caps all have the correct values when crossing from an
export without the "security_label" option to one with the "security_label"
option and vice versa.

Here /export/export1 and /export/export1/export2/export3 have the
"security_label" option; /export and /export/export1/export2 do not:

[root@rhel7u4 ~]# exportfs -v
/export       	<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,secure,no_root_squash,no_all_squash)
/export/export1
		<world>(rw,sync,wdelay,hide,no_subtree_check,security_label,sec=sys,secure,no_root_squash,no_all_squash)
/export/export1/export2
		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,secure,no_root_squash,no_all_squash)
/export/export1/export2/export3
		<world>(rw,sync,wdelay,hide,no_subtree_check,security_label,sec=sys,secure,no_root_squash,no_all_squash)


[root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
[root@fedora25 ~]# ls -lZ /mnt/t
total 8
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09 export1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file3
drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21 12:58 scratch
drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21 12:55 test
[root@fedora25 ~]# ls -lZ /mnt/t/export1
total 20
drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0  4096 May 30 14:09 export2
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
total 20
drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30 14:08 export3
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
total 16
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30 14:08 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30 14:08 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30 14:08 file3
drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30 14:02 lost+found

I can do chcon on /mnt/t/export1 and /mnt/t/export1/export2/export3
but not /mnt/t or /mnt/t/export1/export2:

[root@fedora25 ~]# chcon -t usr_t /mnt/t/file1
chcon: failed to change context of '/mnt/t/file1' to ‘system_u:object_r:usr_t:s0’: Operation not supported
[root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/file1
[root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/file1
chcon: failed to change context of '/mnt/t/export1/export2/file1' to ‘system_u:object_r:usr_t:s0’: Operation not supported
[root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/export3/file1
[root@fedora25 ~]# ls -lZ /mnt/t/file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09 /mnt/t/file1
[root@fedora25 ~]# ls -lZ /mnt/t/export1/file1
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:09 /mnt/t/export1/file1
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09 /mnt/t/export1/export2/file1
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3/file1
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:08 /mnt/t/export1/export2/export3/file1
[root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/file1
[root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/export2/export3/file1

/mnt/t/export1 and /mnt/t/export1/export2/export3 have the "seclabel"
mount option; /mnt/t and /mnt/t/export1/export do not:

[root@fedora25 ~]# cat /proc/mounts
...
rhel7u4:/export /mnt/t nfs4 rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1 /mnt/t/export1 nfs4 rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4 rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2/export3 /mnt/t/export1/export2/export3 nfs4 rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0

The capabilities flags has NFS_CAP_SECURITY_LABEL set on /mnt/t/export1 and
/mnt/t/export1/export2/export3; it is not set on /mnt/t and /mnt/t/export1/export2:

[root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
device rhel7u4:/export mounted on /mnt/t with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype nfs4 statvers=1.1
	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2 mounted on /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2/export3 mounted on /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
[root@fedora25 ~]# printf "%x\n" $(( 0x1fbffdf & (1<<18) ))
0
[root@fedora25 ~]# printf "%x\n" $(( 0x1ffffdf & (1<<18) ))
40000
[root@fedora25 ~]# 

The context= mount option now works correctly:

[root@fedora25 ~]# mount -o v4.2,context="system_u:object_r:user_home_t:s0" rhel7u4:/export /mnt/t
[root@fedora25 ~]# ls -lZ /mnt/t
total 8
drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0 4096 May 30 14:09 export1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30 14:09 file3
drwxrwxrwx. 3 root root system_u:object_r:user_home_t:s0   22 Apr 21 12:58 scratch
drwxrwxrwx. 7 root root system_u:object_r:user_home_t:s0 4096 Apr 21 12:55 test
[root@fedora25 ~]# ls -lZ /mnt/t/export1
total 20
drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0  4096 May 30 14:09 export2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
total 20
drwxr-xr-x. 3 root root system_u:object_r:user_home_t:s0  4096 May 30 14:08 export3
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
total 16
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:08 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:08 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:08 file3
drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30 14:02 lost+found

[root@fedora25 ~]# cat /proc/mounts
...
rhel7u4:/export /mnt/t nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1 /mnt/t/export1 nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2/export3 /mnt/t/export1/export2/export3 nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0

[root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
device rhel7u4:/export mounted on /mnt/t with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2 mounted on /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2/export3 mounted on /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255

-Scott

> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..6a11535 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags, &kflags_out);
> +	if (error)
> +		goto err;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +err:
> +	return error;
> +
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..2f54bfb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1388,7 +1388,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..a55ae9c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					      struct super_block *newsb)
> +					      struct super_block *newsb,
> +					      unsigned long kern_flags,
> +					      unsigned long *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..7b70ea2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..80d9acf 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block *sb)
>  		       sb->s_id, sb->s_type->name);
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> +	   leave the flag untouched because sb_clone_mnt_opts might be handing
> +	   us a superblock that needs the flag to be cleared. */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
>  	struct superblock_security_struct *newsbsec = newsb->s_security;
>  
> @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	 * mount options.  thus we can safely deal with this superblock later
>  	 */
>  	if (!ss_initialized)
> -		return 0;
> +		goto out;
> +
> +	if (kern_flags && !set_kern_flags) {
> +		/* Specifying internal flags without providing a place to
> +		 * place the results is not allowed */
> +		rc = -EINVAL;
> +		goto out;
> +	}
>  
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
>  	/* if fs is reusing a sb, make sure that the contexts match */
> -	if (newsbsec->flags & SE_SBINITIALIZED)
> -		return selinux_cmp_sb_context(oldsb, newsb);
> +	if (newsbsec->flags & SE_SBINITIALIZED) {
> +		rc = selinux_cmp_sb_context(oldsb, newsb);
> +		goto out;
> +	}
>  
>  	mutex_lock(&newsbsec->lock);
>  
> @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> +			&& !(kern_flags & SECURITY_LSM_NATIVE_LABELS)
> +			&& !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out_unlock;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out_unlock:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +out:
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 10:55 -0400, Scott Mayhew wrote:
> On Thu, 01 Jun 2017, Scott Mayhew wrote:
> 
> > When an NFSv4 client performs a mount operation, it first mounts
> > the
> > NFSv4 root and then does path walk to the exported path and
> > performs a
> > submount on that, cloning the security mount options from the
> > root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts
> > mounted
> > with the context= mount option will not show the correct labels
> > because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> > 
> > Allowing the NFSv4 client to enable or disable
> > SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct
> > value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if
> > SECURITY_LSM_NATIVE_LABELS is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels
> > to
> > be displayed for NFSv4.2 mounts mounted with the context= mount
> > option.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 18 +++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 43
> > ++++++++++++++++++++++++++++++++++++++-----
> >  5 files changed, 69 insertions(+), 11 deletions(-)
> 
> With this patch, the behavior and flags fields of the
> superblock_security_struct
> and the nfs_server->caps all have the correct values when crossing
> from an
> export without the "security_label" option to one with the
> "security_label"
> option and vice versa.
> 
> Here /export/export1 and /export/export1/export2/export3 have the
> "security_label" option; /export and /export/export1/export2 do not:
> 
> [root@rhel7u4 ~]# exportfs -v
> /export       	<world>(rw,sync,wdelay,hide,no_subtree_check,se
> c=sys,secure,no_root_squash,no_all_squash)
> /export/export1
> 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> abel,sec=sys,secure,no_root_squash,no_all_squash)
> /export/export1/export2
> 		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,se
> cure,no_root_squash,no_all_squash)
> /export/export1/export2/export3
> 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> abel,sec=sys,secure,no_root_squash,no_all_squash)
> 
> 
> [root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
> [root@fedora25 ~]# ls -lZ /mnt/t
> total 8
> drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09
> export1

Shouldn't this be the label of export1 from the server, since export1
is exported with security_label?

> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09
> file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09
> file2
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09
> file3
> drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21 12:58
> scratch
> drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21 12:55
> test
> [root@fedora25 ~]# ls -lZ /mnt/t/export1
> total 20
> drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0  4096 May 30
> 14:09 export2

Shouldn't this be system_u:object_r:nfs_t:s0, like the rest of export2?

> -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30
> 14:09 file3
> drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30
> 14:02 lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> total 20
> drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30 14:08
> export3

Shouldn't this be the label of export3 from the server, since it is
exported with security_label?

> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09
> file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09
> file2
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09
> file3
> drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30 14:02
> lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> total 16
> -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30
> 14:08 file1
> -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30
> 14:08 file2
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30
> 14:08 file3
> drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30
> 14:02 lost+found
> 
> I can do chcon on /mnt/t/export1 and /mnt/t/export1/export2/export3
> but not /mnt/t or /mnt/t/export1/export2:
> 
> [root@fedora25 ~]# chcon -t usr_t /mnt/t/file1
> chcon: failed to change context of '/mnt/t/file1' to
> ‘system_u:object_r:usr_t:s0’: Operation not supported
> [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/file1
> [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/file1
> chcon: failed to change context of '/mnt/t/export1/export2/file1' to
> ‘system_u:object_r:usr_t:s0’: Operation not supported
> [root@fedora25 ~]# chcon -t usr_t
> /mnt/t/export1/export2/export3/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> /mnt/t/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/file1
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:09
> /mnt/t/export1/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> /mnt/t/export1/export2/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3/file1
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:08
> /mnt/t/export1/export2/export3/file1
> [root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/file1
> [root@fedora25 ~]# chcon -t etc_t
> /mnt/t/export1/export2/export3/file1
> 
> /mnt/t/export1 and /mnt/t/export1/export2/export3 have the "seclabel"
> mount option; /mnt/t and /mnt/t/export1/export do not:
> 
> [root@fedora25 ~]# cat /proc/mounts
> ...
> rhel7u4:/export /mnt/t nfs4
> rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=
> tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,loc
> al_lock=none,addr=192.168.122.83 0 0
> rhel7u4:/export/export1 /mnt/t/export1 nfs4
> rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,ha
> rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.12
> 2.236,local_lock=none,addr=192.168.122.83 0 0
> rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=
> tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,loc
> al_lock=none,addr=192.168.122.83 0 0
> rhel7u4:/export/export1/export2/export3
> /mnt/t/export1/export2/export3 nfs4
> rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,ha
> rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.12
> 2.236,local_lock=none,addr=192.168.122.83 0 0
> 
> The capabilities flags has NFS_CAP_SECURITY_LABEL set on
> /mnt/t/export1 and
> /mnt/t/export1/export2/export3; it is not set on /mnt/t and
> /mnt/t/export1/export2:
> 
> [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype
> nfs4 statvers=1.1
> 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2 mounted on
> /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2/export3 mounted on
> /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> [root@fedora25 ~]# printf "%x\n" $(( 0x1fbffdf & (1<<18) ))
> 0
> [root@fedora25 ~]# printf "%x\n" $(( 0x1ffffdf & (1<<18) ))
> 40000
> [root@fedora25 ~]# 
> 
> The context= mount option now works correctly:
> 
> [root@fedora25 ~]# mount -o
> v4.2,context="system_u:object_r:user_home_t:s0" rhel7u4:/export
> /mnt/t
> [root@fedora25 ~]# ls -lZ /mnt/t
> total 8
> drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0 4096 May 30
> 14:09 export1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30
> 14:09 file3
> drwxrwxrwx. 3 root root system_u:object_r:user_home_t:s0   22 Apr 21
> 12:58 scratch
> drwxrwxrwx. 7 root root system_u:object_r:user_home_t:s0 4096 Apr 21
> 12:55 test
> [root@fedora25 ~]# ls -lZ /mnt/t/export1
> total 20
> drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0  4096 May 30
> 14:09 export2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file3
> drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30
> 14:02 lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> total 20
> drwxr-xr-x. 3 root root system_u:object_r:user_home_t:s0  4096 May 30
> 14:08 export3
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file3
> drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30
> 14:02 lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> total 16
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:08 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:08 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:08 file3
> drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30
> 14:02 lost+found
> 
> [root@fedora25 ~]# cat /proc/mounts
> ...
> rhel7u4:/export /mnt/t nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> rhel7u4:/export/export1 /mnt/t/export1 nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> rhel7u4:/export/export1/export2/export3
> /mnt/t/export1/export2/export3 nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> 
> [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype
> nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2 mounted on
> /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2/export3 mounted on
> /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> 
> -Scott
> 
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..6a11535 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +
> >  	/* clone any lsm security options from the parent to the
> > new sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, 
> > s, kflags, &kflags_out);
> > +	if (error)
> > +		goto err;
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +err:
> > +	return error;
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >  
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..2f54bfb 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1388,7 +1388,9 @@ union security_list_options {
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -					struct super_block
> > *newsb);
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags);
> >  	int (*sb_parse_opts_str)(char *options, struct
> > security_mnt_opts *opts);
> >  	int (*dentry_init_security)(struct dentry *dentry, int
> > mode,
> >  					const struct qstr *name,
> > void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..a55ae9c 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb);
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct
> > security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > @@ -581,7 +583,9 @@ static inline int
> > security_sb_set_mnt_opts(struct super_block *sb,
> >  }
> >  
> >  static inline int security_sb_clone_mnt_opts(const struct
> > super_block *oldsb,
> > -					      struct super_block
> > *newsb)
> > +					      struct super_block
> > *newsb,
> > +					      unsigned long
> > kern_flags,
> > +					      unsigned long
> > *set_kern_flags)
> >  {
> >  	return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..7b70ea2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct
> > super_block *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >  
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb)
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags)
> >  {
> > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +				kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >  
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..80d9acf 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct
> > super_block *sb)
> >  		       sb->s_id, sb->s_type->name);
> >  
> >  	sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +	/* Explicitly set or clear SBLABEL_MNT.  It's not
> > sufficient to simply
> > +	   leave the flag untouched because sb_clone_mnt_opts
> > might be handing
> > +	   us a superblock that needs the flag to be cleared. */
> >  	if (selinux_is_sblabel_mnt(sb))
> >  		sbsec->flags |= SBLABEL_MNT;
> > +	else
> > +		sbsec->flags &= ~SBLABEL_MNT;
> >  
> >  	/* Initialize the root inode. */
> >  	rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct
> > super_block *oldsb,
> >  }
> >  
> >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > *oldsb,
> > -					struct super_block *newsb)
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags)
> >  {
> > +	int rc = 0;
> >  	const struct superblock_security_struct *oldsbsec = oldsb-
> > >s_security;
> >  	struct superblock_security_struct *newsbsec = newsb-
> > >s_security;
> >  
> > @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	 * mount options.  thus we can safely deal with this
> > superblock later
> >  	 */
> >  	if (!ss_initialized)
> > -		return 0;
> > +		goto out;
> > +
> > +	if (kern_flags && !set_kern_flags) {
> > +		/* Specifying internal flags without providing a
> > place to
> > +		 * place the results is not allowed */
> > +		rc = -EINVAL;
> > +		goto out;
> > +	}
> >  
> >  	/* how can we clone if the old one wasn't set up?? */
> >  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> >  
> >  	/* if fs is reusing a sb, make sure that the contexts
> > match */
> > -	if (newsbsec->flags & SE_SBINITIALIZED)
> > -		return selinux_cmp_sb_context(oldsb, newsb);
> > +	if (newsbsec->flags & SE_SBINITIALIZED) {
> > +		rc = selinux_cmp_sb_context(oldsb, newsb);
> > +		goto out;
> > +	}
> >  
> >  	mutex_lock(&newsbsec->lock);
> >  
> > @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	newsbsec->def_sid = oldsbsec->def_sid;
> >  	newsbsec->behavior = oldsbsec->behavior;
> >  
> > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> > +			&& !(kern_flags &
> > SECURITY_LSM_NATIVE_LABELS)
> > +			&& !set_context) {
> > +		rc = security_fs_use(newsb);
> > +		if (rc)
> > +			goto out_unlock;
> > +	}
> > +
> > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS &&
> > !set_context) {
> > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +	}
> > +
> >  	if (set_context) {
> >  		u32 sid = oldsbsec->mntpoint_sid;
> >  
> > @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	}
> >  
> >  	sb_finish_set_opts(newsb);
> > +out_unlock:
> >  	mutex_unlock(&newsbsec->lock);
> > -	return 0;
> > +out:
> > +	return rc;
> >  }
> >  
> >  static int selinux_parse_opts_str(char *options,
> > -- 
> > 2.9.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 14:08 -0400, Stephen Smalley wrote:
> On Thu, 2017-06-01 at 10:55 -0400, Scott Mayhew wrote:
> > On Thu, 01 Jun 2017, Scott Mayhew wrote:
> > 
> > > When an NFSv4 client performs a mount operation, it first mounts
> > > the
> > > NFSv4 root and then does path walk to the exported path and
> > > performs a
> > > submount on that, cloning the security mount options from the
> > > root's
> > > superblock to the submount's superblock in the process.
> > > 
> > > Unless the NFS server has an explicit fsid=0 export with the
> > > "security_label" option, the NFSv4 root superblock will not have
> > > SBLABEL_MNT set, and neither will the submount superblock after
> > > cloning
> > > the security mount options.  As a result, setxattr's of security
> > > labels
> > > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts
> > > mounted
> > > with the context= mount option will not show the correct labels
> > > because
> > > the nfs_server->caps flags of the cloned superblock will still
> > > have
> > > NFS_CAP_SECURITY_LABEL set.
> > > 
> > > Allowing the NFSv4 client to enable or disable
> > > SECURITY_LSM_NATIVE_LABELS
> > > behavior will ensure that the SBLABEL_MNT flag has the correct
> > > value
> > > when the client traverses from an exported path without the
> > > "security_label" option to one with the "security_label" option
> > > and
> > > vice versa.  Similarly, checking to see if
> > > SECURITY_LSM_NATIVE_LABELS is
> > > set upon return from security_sb_clone_mnt_opts() and clearing
> > > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels
> > > to
> > > be displayed for NFSv4.2 mounts mounted with the context= mount
> > > option.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c            | 18 +++++++++++++++++-
> > >  include/linux/lsm_hooks.h |  4 +++-
> > >  include/linux/security.h  |  8 ++++++--
> > >  security/security.c       |  7 +++++--
> > >  security/selinux/hooks.c  | 43
> > > ++++++++++++++++++++++++++++++++++++++-----
> > >  5 files changed, 69 insertions(+), 11 deletions(-)
> > 
> > With this patch, the behavior and flags fields of the
> > superblock_security_struct
> > and the nfs_server->caps all have the correct values when crossing
> > from an
> > export without the "security_label" option to one with the
> > "security_label"
> > option and vice versa.
> > 
> > Here /export/export1 and /export/export1/export2/export3 have the
> > "security_label" option; /export and /export/export1/export2 do
> > not:
> > 
> > [root@rhel7u4 ~]# exportfs -v
> > /export       	<world>(rw,sync,wdelay,hide,no_subtree_check,
> > se
> > c=sys,secure,no_root_squash,no_all_squash)
> > /export/export1
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > /export/export1/export2
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,se
> > cure,no_root_squash,no_all_squash)
> > /export/export1/export2/export3
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > 
> > 
> > [root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
> > [root@fedora25 ~]# ls -lZ /mnt/t
> > total 8
> > drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30
> > 14:09
> > export1
> 
> Shouldn't this be the label of export1 from the server, since export1
> is exported with security_label?

BTW, I don't think your patch introduces this bug (assuming it is a
bug), so this isn't an obstacle to your patch proceeding, but unless I
misunderstand, it is another bug that needs to be fixed (in this case,
with the initialization of the root inode label for a NFS mount).

> 
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30
> > 14:09
> > file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30
> > 14:09
> > file2
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30
> > 14:09
> > file3
> > drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21
> > 12:58
> > scratch
> > drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21
> > 12:55
> > test
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1
> > total 20
> > drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0  4096 May
> > 30
> > 14:09 export2
> 
> Shouldn't this be system_u:object_r:nfs_t:s0, like the rest of
> export2?
> 
> > -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May
> > 30
> > 14:09 file3
> > drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> > total 20
> > drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30
> > 14:08
> > export3
> 
> Shouldn't this be the label of export3 from the server, since it is
> exported with security_label?
> 
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30
> > 14:09
> > file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30
> > 14:09
> > file2
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30
> > 14:09
> > file3
> > drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30
> > 14:02
> > lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> > total 16
> > -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May
> > 30
> > 14:08 file1
> > -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May
> > 30
> > 14:08 file2
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May
> > 30
> > 14:08 file3
> > drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > 
> > I can do chcon on /mnt/t/export1 and /mnt/t/export1/export2/export3
> > but not /mnt/t or /mnt/t/export1/export2:
> > 
> > [root@fedora25 ~]# chcon -t usr_t /mnt/t/file1
> > chcon: failed to change context of '/mnt/t/file1' to
> > ‘system_u:object_r:usr_t:s0’: Operation not supported
> > [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/file1
> > [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/file1
> > chcon: failed to change context of '/mnt/t/export1/export2/file1'
> > to
> > ‘system_u:object_r:usr_t:s0’: Operation not supported
> > [root@fedora25 ~]# chcon -t usr_t
> > /mnt/t/export1/export2/export3/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> > /mnt/t/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/file1
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:09
> > /mnt/t/export1/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> > /mnt/t/export1/export2/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3/file1
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:08
> > /mnt/t/export1/export2/export3/file1
> > [root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/file1
> > [root@fedora25 ~]# chcon -t etc_t
> > /mnt/t/export1/export2/export3/file1
> > 
> > /mnt/t/export1 and /mnt/t/export1/export2/export3 have the
> > "seclabel"
> > mount option; /mnt/t and /mnt/t/export1/export do not:
> > 
> > [root@fedora25 ~]# cat /proc/mounts
> > ...
> > rhel7u4:/export /mnt/t nfs4
> > rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,prot
> > o=
> > tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,l
> > oc
> > al_lock=none,addr=192.168.122.83 0 0
> > rhel7u4:/export/export1 /mnt/t/export1 nfs4
> > rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,
> > ha
> > rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.
> > 12
> > 2.236,local_lock=none,addr=192.168.122.83 0 0
> > rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> > rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,prot
> > o=
> > tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,l
> > oc
> > al_lock=none,addr=192.168.122.83 0 0
> > rhel7u4:/export/export1/export2/export3
> > /mnt/t/export1/export2/export3 nfs4
> > rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,
> > ha
> > rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.
> > 12
> > 2.236,local_lock=none,addr=192.168.122.83 0 0
> > 
> > The capabilities flags has NFS_CAP_SECURITY_LABEL set on
> > /mnt/t/export1 and
> > /mnt/t/export1/export2/export3; it is not set on /mnt/t and
> > /mnt/t/export1/export2:
> > 
> > [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> > device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> > statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1 mounted on /mnt/t/export1 with
> > fstype
> > nfs4 statvers=1.1
> > 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2 mounted on
> > /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2/export3 mounted on
> > /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > [root@fedora25 ~]# printf "%x\n" $(( 0x1fbffdf & (1<<18) ))
> > 0
> > [root@fedora25 ~]# printf "%x\n" $(( 0x1ffffdf & (1<<18) ))
> > 40000
> > [root@fedora25 ~]# 
> > 
> > The context= mount option now works correctly:
> > 
> > [root@fedora25 ~]# mount -o
> > v4.2,context="system_u:object_r:user_home_t:s0" rhel7u4:/export
> > /mnt/t
> > [root@fedora25 ~]# ls -lZ /mnt/t
> > total 8
> > drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0 4096 May
> > 30
> > 14:09 export1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May
> > 30
> > 14:09 file3
> > drwxrwxrwx. 3 root root system_u:object_r:user_home_t:s0   22 Apr
> > 21
> > 12:58 scratch
> > drwxrwxrwx. 7 root root system_u:object_r:user_home_t:s0 4096 Apr
> > 21
> > 12:55 test
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1
> > total 20
> > drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0  4096 May
> > 30
> > 14:09 export2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file3
> > drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> > total 20
> > drwxr-xr-x. 3 root root system_u:object_r:user_home_t:s0  4096 May
> > 30
> > 14:08 export3
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file3
> > drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> > total 16
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:08 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:08 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:08 file3
> > drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > 
> > [root@fedora25 ~]# cat /proc/mounts
> > ...
> > rhel7u4:/export /mnt/t nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > rhel7u4:/export/export1 /mnt/t/export1 nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > rhel7u4:/export/export1/export2/export3
> > /mnt/t/export1/export2/export3 nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > 
> > [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> > device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> > statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1 mounted on /mnt/t/export1 with
> > fstype
> > nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2 mounted on
> > /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2/export3 mounted on
> > /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > 
> > -Scott
> > 
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 2f3822a..6a11535 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > *mntroot,
> > >  			  struct nfs_mount_info *mount_info)
> > >  {
> > > +	int error;
> > > +	unsigned long kflags = 0, kflags_out = 0;
> > > +
> > >  	/* clone any lsm security options from the parent to the
> > > new sb */
> > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > rpc_ops->dir_inode_ops)
> > > 
> > >  		return -ESTALE;
> > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > +
> > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb, 
> > > s, kflags, &kflags_out);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > > +err:
> > > +	return error;
> > > +
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > >  
> > > diff --git a/include/linux/lsm_hooks.h
> > > b/include/linux/lsm_hooks.h
> > > index 080f34e..2f54bfb 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -1388,7 +1388,9 @@ union security_list_options {
> > >  				unsigned long kern_flags,
> > >  				unsigned long *set_kern_flags);
> > >  	int (*sb_clone_mnt_opts)(const struct super_block
> > > *oldsb,
> > > -					struct super_block
> > > *newsb);
> > > +					struct super_block
> > > *newsb,
> > > +					unsigned long
> > > kern_flags,
> > > +					unsigned long
> > > *set_kern_flags);
> > >  	int (*sb_parse_opts_str)(char *options, struct
> > > security_mnt_opts *opts);
> > >  	int (*dentry_init_security)(struct dentry *dentry, int
> > > mode,
> > >  					const struct qstr *name,
> > > void **ctx,
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index af675b5..a55ae9c 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct
> > > super_block
> > > *sb,
> > >  				unsigned long kern_flags,
> > >  				unsigned long *set_kern_flags);
> > >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > > -				struct super_block *newsb);
> > > +				struct super_block *newsb,
> > > +				unsigned long kern_flags,
> > > +				unsigned long *set_kern_flags);
> > >  int security_sb_parse_opts_str(char *options, struct
> > > security_mnt_opts *opts);
> > >  int security_dentry_init_security(struct dentry *dentry, int
> > > mode,
> > >  					const struct qstr *name,
> > > void **ctx,
> > > @@ -581,7 +583,9 @@ static inline int
> > > security_sb_set_mnt_opts(struct super_block *sb,
> > >  }
> > >  
> > >  static inline int security_sb_clone_mnt_opts(const struct
> > > super_block *oldsb,
> > > -					      struct super_block
> > > *newsb)
> > > +					      struct super_block
> > > *newsb,
> > > +					      unsigned long
> > > kern_flags,
> > > +					      unsigned long
> > > *set_kern_flags)
> > >  {
> > >  	return 0;
> > >  }
> > > diff --git a/security/security.c b/security/security.c
> > > index b9fea39..7b70ea2 100644
> > > --- a/security/security.c
> > > +++ b/security/security.c
> > > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct
> > > super_block *sb,
> > >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> > >  
> > >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > > -				struct super_block *newsb)
> > > +				struct super_block *newsb,
> > > +				unsigned long kern_flags,
> > > +				unsigned long *set_kern_flags)
> > >  {
> > > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb,
> > > newsb);
> > > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > > +				kern_flags, set_kern_flags);
> > >  }
> > >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> > >  
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index e67a526..80d9acf 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct
> > > super_block *sb)
> > >  		       sb->s_id, sb->s_type->name);
> > >  
> > >  	sbsec->flags |= SE_SBINITIALIZED;
> > > +
> > > +	/* Explicitly set or clear SBLABEL_MNT.  It's not
> > > sufficient to simply
> > > +	   leave the flag untouched because sb_clone_mnt_opts
> > > might be handing
> > > +	   us a superblock that needs the flag to be cleared. */
> > >  	if (selinux_is_sblabel_mnt(sb))
> > >  		sbsec->flags |= SBLABEL_MNT;
> > > +	else
> > > +		sbsec->flags &= ~SBLABEL_MNT;
> > >  
> > >  	/* Initialize the root inode. */
> > >  	rc = inode_doinit_with_dentry(root_inode, root);
> > > @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const
> > > struct
> > > super_block *oldsb,
> > >  }
> > >  
> > >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > > *oldsb,
> > > -					struct super_block
> > > *newsb)
> > > +					struct super_block
> > > *newsb,
> > > +					unsigned long
> > > kern_flags,
> > > +					unsigned long
> > > *set_kern_flags)
> > >  {
> > > +	int rc = 0;
> > >  	const struct superblock_security_struct *oldsbsec =
> > > oldsb-
> > > > s_security;
> > > 
> > >  	struct superblock_security_struct *newsbsec = newsb-
> > > > s_security;
> > > 
> > >  
> > > @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> > > struct super_block *oldsb,
> > >  	 * mount options.  thus we can safely deal with this
> > > superblock later
> > >  	 */
> > >  	if (!ss_initialized)
> > > -		return 0;
> > > +		goto out;
> > > +
> > > +	if (kern_flags && !set_kern_flags) {
> > > +		/* Specifying internal flags without providing a
> > > place to
> > > +		 * place the results is not allowed */
> > > +		rc = -EINVAL;
> > > +		goto out;
> > > +	}
> > >  
> > >  	/* how can we clone if the old one wasn't set up?? */
> > >  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> > >  
> > >  	/* if fs is reusing a sb, make sure that the contexts
> > > match */
> > > -	if (newsbsec->flags & SE_SBINITIALIZED)
> > > -		return selinux_cmp_sb_context(oldsb, newsb);
> > > +	if (newsbsec->flags & SE_SBINITIALIZED) {
> > > +		rc = selinux_cmp_sb_context(oldsb, newsb);
> > > +		goto out;
> > > +	}
> > >  
> > >  	mutex_lock(&newsbsec->lock);
> > >  
> > > @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> > > struct super_block *oldsb,
> > >  	newsbsec->def_sid = oldsbsec->def_sid;
> > >  	newsbsec->behavior = oldsbsec->behavior;
> > >  
> > > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> > > +			&& !(kern_flags &
> > > SECURITY_LSM_NATIVE_LABELS)
> > > +			&& !set_context) {
> > > +		rc = security_fs_use(newsb);
> > > +		if (rc)
> > > +			goto out_unlock;
> > > +	}
> > > +
> > > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS &&
> > > !set_context) {
> > > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > > +	}
> > > +
> > >  	if (set_context) {
> > >  		u32 sid = oldsbsec->mntpoint_sid;
> > >  
> > > @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> > > struct super_block *oldsb,
> > >  	}
> > >  
> > >  	sb_finish_set_opts(newsb);
> > > +out_unlock:
> > >  	mutex_unlock(&newsbsec->lock);
> > > -	return 0;
> > > +out:
> > > +	return rc;
> > >  }
> > >  
> > >  static int selinux_parse_opts_str(char *options,
> > > -- 
> > > 2.9.3
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> > > nfs" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.htm
> > > l

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Thu, 01 Jun 2017, Stephen Smalley wrote:

> On Thu, 2017-06-01 at 10:55 -0400, Scott Mayhew wrote:
> > On Thu, 01 Jun 2017, Scott Mayhew wrote:
> > 
> > > When an NFSv4 client performs a mount operation, it first mounts
> > > the
> > > NFSv4 root and then does path walk to the exported path and
> > > performs a
> > > submount on that, cloning the security mount options from the
> > > root's
> > > superblock to the submount's superblock in the process.
> > > 
> > > Unless the NFS server has an explicit fsid=0 export with the
> > > "security_label" option, the NFSv4 root superblock will not have
> > > SBLABEL_MNT set, and neither will the submount superblock after
> > > cloning
> > > the security mount options.  As a result, setxattr's of security
> > > labels
> > > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts
> > > mounted
> > > with the context= mount option will not show the correct labels
> > > because
> > > the nfs_server->caps flags of the cloned superblock will still have
> > > NFS_CAP_SECURITY_LABEL set.
> > > 
> > > Allowing the NFSv4 client to enable or disable
> > > SECURITY_LSM_NATIVE_LABELS
> > > behavior will ensure that the SBLABEL_MNT flag has the correct
> > > value
> > > when the client traverses from an exported path without the
> > > "security_label" option to one with the "security_label" option and
> > > vice versa.  Similarly, checking to see if
> > > SECURITY_LSM_NATIVE_LABELS is
> > > set upon return from security_sb_clone_mnt_opts() and clearing
> > > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels
> > > to
> > > be displayed for NFSv4.2 mounts mounted with the context= mount
> > > option.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c            | 18 +++++++++++++++++-
> > >  include/linux/lsm_hooks.h |  4 +++-
> > >  include/linux/security.h  |  8 ++++++--
> > >  security/security.c       |  7 +++++--
> > >  security/selinux/hooks.c  | 43
> > > ++++++++++++++++++++++++++++++++++++++-----
> > >  5 files changed, 69 insertions(+), 11 deletions(-)
> > 
> > With this patch, the behavior and flags fields of the
> > superblock_security_struct
> > and the nfs_server->caps all have the correct values when crossing
> > from an
> > export without the "security_label" option to one with the
> > "security_label"
> > option and vice versa.
> > 
> > Here /export/export1 and /export/export1/export2/export3 have the
> > "security_label" option; /export and /export/export1/export2 do not:
> > 
> > [root@rhel7u4 ~]# exportfs -v
> > /export       	<world>(rw,sync,wdelay,hide,no_subtree_check,se
> > c=sys,secure,no_root_squash,no_all_squash)
> > /export/export1
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > /export/export1/export2
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,se
> > cure,no_root_squash,no_all_squash)
> > /export/export1/export2/export3
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > 
> > 
> > [root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
> > [root@fedora25 ~]# ls -lZ /mnt/t
> > total 8
> > drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09
> > export1
> 
> Shouldn't this be the label of export1 from the server, since export1
> is exported with security_label?
> 
<snip/>

That's my fault, I actually didn't have labels on all the directories on
the server.

[root@rhel7u4 ~]# ls -laZ /export/export1
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 .
drwxrwxrwx. root root unconfined_u:object_r:usr_t:s0   ..
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 export2
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:unlabeled_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 .
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 ..
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 export3
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:unlabeled_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2/export3
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 .
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 ..
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:unlabeled_t:s0 lost+found

Setting the missing labels...

[root@rhel7u4 ~]# chcon -t usr_t /export/export1
[root@rhel7u4 ~]# chcon -t usr_t /export/export1/export2
[root@rhel7u4 ~]# chcon -t usr_t /export/export1/export2/export3
[root@rhel7u4 ~]# chcon -t lost_found_t /export/export1/lost+found
[root@rhel7u4 ~]# chcon -t lost_found_t /export/export1/export2/lost+found
[root@rhel7u4 ~]# chcon -t lost_found_t /export/export1/export2/export3/lost+found

[root@rhel7u4 ~]# ls -laZ /export/export1
drwxr-xr-x. root root system_u:object_r:usr_t:s0       .
drwxrwxrwx. root root unconfined_u:object_r:usr_t:s0   ..
drwxr-xr-x. root root system_u:object_r:usr_t:s0       export2
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:lost_found_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2
drwxr-xr-x. root root system_u:object_r:usr_t:s0       .
drwxr-xr-x. root root system_u:object_r:usr_t:s0       ..
drwxr-xr-x. root root system_u:object_r:usr_t:s0       export3
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:lost_found_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2/export3
drwxr-xr-x. root root system_u:object_r:usr_t:s0       .
drwxr-xr-x. root root system_u:object_r:usr_t:s0       ..
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:lost_found_t:s0 lost+found

Even after setting the labels on the NFS server, the behavior on the client
appears strange until you remember that the client doesn't actually know about
label support on the export until it steps on the mountpoint and triggers the
submount.  I don't know if I'd call it a bug or not... more of a peculiarity :)

Here we're doing READDIR with the NFS filehandle of /export.  Since /export is
not exported with the security_label option, the attrmask in the READDIR is not
requesting security labels... so export1 shows nfs_t even though
/export/export1 is exported with the security_label option:

[root@fedora25 ~]# ls -laZ /mnt/t
total 8
drwxrwxrwx. 5 root root system_u:object_r:nfs_t:s0   87 Jun  1 09:10 .
drwxr-xr-x. 5 root root system_u:object_r:mnt_t:s0   42 May 22 08:45 ..
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09 export1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file3
drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21 12:58 scratch
drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21 12:55 test

Here we're doing a READDIR with the NFS filehandle of /export/export1, which
triggers the mount of /export/export1.  Since /export/export1 is exported with
the security_label option, the attrmask in the READDIR is requesting security
labels.  Note that '.' now shows usr_t instead of nfs_t.  However, since
/export/export1/export2 is not exported with the security_label option, the NFS
server does not return a security label in the attributes for export2 in the
READDIR reply... so it's displayed as unlabeled_t:

[root@fedora25 ~]# ls -laZ /mnt/t/export1
total 24
drwxr-xr-x. 4 root root system_u:object_r:usr_t:s0         4096 May 30 14:09 .
drwxrwxrwx. 5 root root system_u:object_r:nfs_t:s0           87 Jun  1 09:10 ..
drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0   4096 May 30 14:09 export2
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0            0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0            0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0            0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:lost_found_t:s0 16384 May 30 14:02 lost+found

Here we're doing doing a READDIR with the NFS filehandle of /export/export1/export2,
which triggers the mount of /export/export1/export2.  Since /export/export1/export2
is not exported with the security_label option, the attrmask in the READDIR is not
requesting security labels, and everything (including both '.' and 'export3') is
displayed as nfs_t:

[root@fedora25 ~]# ls -laZ /mnt/t/export1/export2
total 28
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0  4096 May 30 14:09 .
drwxr-xr-x. 4 root root system_u:object_r:usr_t:s0  4096 May 30 14:09 ..
drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30 14:08 export3
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30 14:02 lost+found

Here we're doing a READDIR with the NFS filehandle of /export1/export2/export3,
which triggers the mount of /export1/export2/export3.  Since /export1/export2/export3
is exported with the security_label option, the attrmask in the READDIR is
requesting security labels, and '.' now shows as usr_t:

[root@fedora25 ~]# ls -laZ /mnt/t/export1/export2/export3
total 24
drwxr-xr-x. 3 root root system_u:object_r:usr_t:s0         4096 May 30 14:08 .
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0         4096 May 30 14:09 ..
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0            0 May 30 14:08 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0            0 May 30 14:08 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0            0 May 30 14:08 file3
drwx------. 2 root root system_u:object_r:lost_found_t:s0 16384 May 30 14:02 lost+found

After triggering all those mounts, the labels for export1 and export3 match
what I set on the server:

[root@fedora25 ~]# ls -ldZ /mnt/t
drwxrwxrwx. 5 root root system_u:object_r:nfs_t:s0 87 Jun  1 09:10 /mnt/t
[root@fedora25 ~]# ls -ldZ /mnt/t/export1
drwxr-xr-x. 4 root root system_u:object_r:usr_t:s0 4096 May 30 14:09 /mnt/t/export1
[root@fedora25 ~]# ls -ldZ /mnt/t/export1/export2
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09 /mnt/t/export1/export2
[root@fedora25 ~]# ls -ldZ /mnt/t/export1/export2/export3
drwxr-xr-x. 3 root root system_u:object_r:usr_t:s0 4096 May 30 14:08 /mnt/t/export1/export2/export3
[root@fedora25 ~]# 

-Scott

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 10:46 -0400, Scott Mayhew wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels
> because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable
> SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount
> option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 18 +++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 43
> ++++++++++++++++++++++++++++++++++++++-----
>  5 files changed, 69 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..6a11535 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s, kflags, &kflags_out);
> +	if (error)
> +		goto err;

Not sure this is justified; coding style says to just return directly
if no cleanup is required.

> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +err:
> +	return error;
> +
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..2f54bfb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1388,7 +1388,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct
> security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..a55ae9c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct
> security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct
> super_block *oldsb,
> -					      struct super_block
> *newsb)
> +					      struct super_block
> *newsb,
> +					      unsigned long
> kern_flags,
> +					      unsigned long
> *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..7b70ea2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..80d9acf 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block
> *sb)
>  		       sb->s_id, sb->s_type->name);
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> to simply
> +	   leave the flag untouched because sb_clone_mnt_opts might
> be handing
> +	   us a superblock that needs the flag to be cleared. */

Coding style (and checkpatch.pl) prefer a different style for multi-
line comments.

>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct
> super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block
> *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb-
> >s_security;
>  	struct superblock_security_struct *newsbsec = newsb-
> >s_security;
>  
> @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	 * mount options.  thus we can safely deal with this
> superblock later
>  	 */
>  	if (!ss_initialized)
> -		return 0;
> +		goto out;

Likewise, don't see the point of this since no cleanup is required, and
as per coding style.

> +
> +	if (kern_flags && !set_kern_flags) {
> +		/* Specifying internal flags without providing a
> place to
> +		 * place the results is not allowed */
> +		rc = -EINVAL;
> +		goto out;

Ditto, just return directly.

> +	}
>  
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
>  	/* if fs is reusing a sb, make sure that the contexts match
> */
> -	if (newsbsec->flags & SE_SBINITIALIZED)
> -		return selinux_cmp_sb_context(oldsb, newsb);
> +	if (newsbsec->flags & SE_SBINITIALIZED) {
> +		rc = selinux_cmp_sb_context(oldsb, newsb);
> +		goto out;
> +	}

And again.

>  
>  	mutex_lock(&newsbsec->lock);
>  
> @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> +			&& !(kern_flags &
> SECURITY_LSM_NATIVE_LABELS)
> +			&& !set_context) {

I prefer ending the prior line with &&, not beginning the next one with
it. Also indentation of the continuation lines seems excessive.

> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out_unlock;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out_unlock:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +out:
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Thu, 01 Jun 2017, Stephen Smalley wrote:

> On Thu, 2017-06-01 at 10:46 -0400, Scott Mayhew wrote:
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> > with the context= mount option will not show the correct labels
> > because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> > 
> > Allowing the NFSv4 client to enable or disable
> > SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> > is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> > be displayed for NFSv4.2 mounts mounted with the context= mount
> > option.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 18 +++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 43
> > ++++++++++++++++++++++++++++++++++++++-----
> >  5 files changed, 69 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..6a11535 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +
> >  	/* clone any lsm security options from the parent to the new
> > sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s, kflags, &kflags_out);
> > +	if (error)
> > +		goto err;
> 
> Not sure this is justified; coding style says to just return directly
> if no cleanup is required.
> 
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +err:
> > +	return error;
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >  
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..2f54bfb 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1388,7 +1388,9 @@ union security_list_options {
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -					struct super_block *newsb);
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags);
> >  	int (*sb_parse_opts_str)(char *options, struct
> > security_mnt_opts *opts);
> >  	int (*dentry_init_security)(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..a55ae9c 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb);
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct
> > security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> > super_block *sb,
> >  }
> >  
> >  static inline int security_sb_clone_mnt_opts(const struct
> > super_block *oldsb,
> > -					      struct super_block
> > *newsb)
> > +					      struct super_block
> > *newsb,
> > +					      unsigned long
> > kern_flags,
> > +					      unsigned long
> > *set_kern_flags)
> >  {
> >  	return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..7b70ea2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >  
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb)
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags)
> >  {
> > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +				kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >  
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..80d9acf 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block
> > *sb)
> >  		       sb->s_id, sb->s_type->name);
> >  
> >  	sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> > to simply
> > +	   leave the flag untouched because sb_clone_mnt_opts might
> > be handing
> > +	   us a superblock that needs the flag to be cleared. */
> 
> Coding style (and checkpatch.pl) prefer a different style for multi-
> line comments.
> 
> >  	if (selinux_is_sblabel_mnt(sb))
> >  		sbsec->flags |= SBLABEL_MNT;
> > +	else
> > +		sbsec->flags &= ~SBLABEL_MNT;
> >  
> >  	/* Initialize the root inode. */
> >  	rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct
> > super_block *oldsb,
> >  }
> >  
> >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > *oldsb,
> > -					struct super_block *newsb)
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags)
> >  {
> > +	int rc = 0;
> >  	const struct superblock_security_struct *oldsbsec = oldsb-
> > >s_security;
> >  	struct superblock_security_struct *newsbsec = newsb-
> > >s_security;
> >  
> > @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	 * mount options.  thus we can safely deal with this
> > superblock later
> >  	 */
> >  	if (!ss_initialized)
> > -		return 0;
> > +		goto out;
> 
> Likewise, don't see the point of this since no cleanup is required, and
> as per coding style.
> 
> > +
> > +	if (kern_flags && !set_kern_flags) {
> > +		/* Specifying internal flags without providing a
> > place to
> > +		 * place the results is not allowed */
> > +		rc = -EINVAL;
> > +		goto out;
> 
> Ditto, just return directly.
> 
> > +	}
> >  
> >  	/* how can we clone if the old one wasn't set up?? */
> >  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> >  
> >  	/* if fs is reusing a sb, make sure that the contexts match
> > */
> > -	if (newsbsec->flags & SE_SBINITIALIZED)
> > -		return selinux_cmp_sb_context(oldsb, newsb);
> > +	if (newsbsec->flags & SE_SBINITIALIZED) {
> > +		rc = selinux_cmp_sb_context(oldsb, newsb);
> > +		goto out;
> > +	}
> 
> And again.
> 
> >  
> >  	mutex_lock(&newsbsec->lock);
> >  
> > @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	newsbsec->def_sid = oldsbsec->def_sid;
> >  	newsbsec->behavior = oldsbsec->behavior;
> >  
> > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> > +			&& !(kern_flags &
> > SECURITY_LSM_NATIVE_LABELS)
> > +			&& !set_context) {
> 
> I prefer ending the prior line with &&, not beginning the next one with
> it. Also indentation of the continuation lines seems excessive.
> 
> > +		rc = security_fs_use(newsb);
> > +		if (rc)
> > +			goto out_unlock;
> > +	}
> > +
> > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> > {
> > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +	}
> > +
> >  	if (set_context) {
> >  		u32 sid = oldsbsec->mntpoint_sid;
> >  
> > @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	}
> >  
> >  	sb_finish_set_opts(newsb);
> > +out_unlock:
> >  	mutex_unlock(&newsbsec->lock);
> > -	return 0;
> > +out:
> > +	return rc;
> >  }
> >  
> >  static int selinux_parse_opts_str(char *options,

Again, my fault... I should've run checkpatch.pl.

I'll address all these and re-send.

-Scott

[PATCH v2] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.

Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c            | 18 +++++++++++++++++-
 include/linux/lsm_hooks.h |  4 +++-
 include/linux/security.h  |  8 ++++++--
 security/security.c       |  7 +++++--
 security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
 5 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..ffded39 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
+			&kflags_out);
+	if (error)
+		return error;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+	return error;
+
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e..2f54bfb 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1388,7 +1388,9 @@ union security_list_options {
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
-					struct super_block *newsb);
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags);
 	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..a55ae9c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb);
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags);
 int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
@@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
 }
 
 static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-					      struct super_block *newsb)
+					      struct super_block *newsb,
+					      unsigned long kern_flags,
+					      unsigned long *set_kern_flags)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index b9fea39..7b70ea2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 EXPORT_SYMBOL(security_sb_set_mnt_opts);
 
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb)
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags)
 {
-	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
+	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
+				kern_flags, set_kern_flags);
 }
 EXPORT_SYMBOL(security_sb_clone_mnt_opts);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..d8d5d35 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -529,8 +529,16 @@ static int sb_finish_set_opts(struct super_block *sb)
 		       sb->s_id, sb->s_type->name);
 
 	sbsec->flags |= SE_SBINITIALIZED;
+
+	/*
+	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
+	 * leave the flag untouched because sb_clone_mnt_opts might be handing
+	 * us a superblock that needs the flag to be cleared.
+	 */
 	if (selinux_is_sblabel_mnt(sb))
 		sbsec->flags |= SBLABEL_MNT;
+	else
+		sbsec->flags &= ~SBLABEL_MNT;
 
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
@@ -963,8 +971,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
 }
 
 static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
-					struct super_block *newsb)
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags)
 {
+	int rc = 0;
 	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
 	struct superblock_security_struct *newsbsec = newsb->s_security;
 
@@ -979,6 +990,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	if (!ss_initialized)
 		return 0;
 
+	/*
+	 * Specifying internal flags without providing a place to
+	 * place the results is not allowed.
+	 */
+	if (kern_flags && !set_kern_flags)
+		return -EINVAL;
+
 	/* how can we clone if the old one wasn't set up?? */
 	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
 
@@ -994,6 +1012,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	newsbsec->def_sid = oldsbsec->def_sid;
 	newsbsec->behavior = oldsbsec->behavior;
 
+	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
+		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
+		rc = security_fs_use(newsb);
+		if (rc)
+			goto out;
+	}
+
+	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
+		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
+		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+	}
+
 	if (set_context) {
 		u32 sid = oldsbsec->mntpoint_sid;
 
@@ -1013,8 +1043,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	}
 
 	sb_finish_set_opts(newsb);
+out:
 	mutex_unlock(&newsbsec->lock);
-	return 0;
+	return rc;
 }
 
 static int selinux_parse_opts_str(char *options,
-- 
2.9.3

Re: [PATCH v2] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 16:59 -0400, Scott Mayhew wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels
> because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable
> SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount
> option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 18 +++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 64 insertions(+), 8 deletions(-)

What tree is this against? Doesn't apply cleanly on selinux #next.

> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..ffded39 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s, kflags,
> +			&kflags_out);
> +	if (error)
> +		return error;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	return error;

This can just be return 0, right?

> +
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..2f54bfb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1388,7 +1388,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct
> security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..a55ae9c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct
> security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct
> super_block *oldsb,
> -					      struct super_block
> *newsb)
> +					      struct super_block
> *newsb,
> +					      unsigned long
> kern_flags,
> +					      unsigned long
> *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..7b70ea2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..d8d5d35 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -529,8 +529,16 @@ static int sb_finish_set_opts(struct super_block
> *sb)
>  		       sb->s_id, sb->s_type->name);
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/*
> +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> to simply
> +	 * leave the flag untouched because sb_clone_mnt_opts might
> be handing
> +	 * us a superblock that needs the flag to be cleared.
> +	 */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -963,8 +971,11 @@ static int selinux_cmp_sb_context(const struct
> super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block
> *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb-
> >s_security;
>  	struct superblock_security_struct *newsbsec = newsb-
> >s_security;
>  
> @@ -979,6 +990,13 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	if (!ss_initialized)
>  		return 0;
>  
> +	/*
> +	 * Specifying internal flags without providing a place to
> +	 * place the results is not allowed.
> +	 */
> +	if (kern_flags && !set_kern_flags)
> +		return -EINVAL;
> +
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
> @@ -994,6 +1012,18 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) &&
> !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1013,8 +1043,9 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,

Re: [PATCH v2] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Fri, 02 Jun 2017, Stephen Smalley wrote:

> On Thu, 2017-06-01 at 16:59 -0400, Scott Mayhew wrote:
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> > with the context= mount option will not show the correct labels
> > because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> > 
> > Allowing the NFSv4 client to enable or disable
> > SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> > is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> > be displayed for NFSv4.2 mounts mounted with the context= mount
> > option.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 18 +++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
> >  5 files changed, 64 insertions(+), 8 deletions(-)
> 
> What tree is this against? Doesn't apply cleanly on selinux #next.

It's against Linus' mainline tree.
> 
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..ffded39 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +
> >  	/* clone any lsm security options from the parent to the new
> > sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s, kflags,
> > +			&kflags_out);
> > +	if (error)
> > +		return error;
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +	return error;
> 
> This can just be return 0, right?

Yes.
> 
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >  
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..2f54bfb 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1388,7 +1388,9 @@ union security_list_options {
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -					struct super_block *newsb);
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags);
> >  	int (*sb_parse_opts_str)(char *options, struct
> > security_mnt_opts *opts);
> >  	int (*dentry_init_security)(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..a55ae9c 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb);
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct
> > security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> > super_block *sb,
> >  }
> >  
> >  static inline int security_sb_clone_mnt_opts(const struct
> > super_block *oldsb,
> > -					      struct super_block
> > *newsb)
> > +					      struct super_block
> > *newsb,
> > +					      unsigned long
> > kern_flags,
> > +					      unsigned long
> > *set_kern_flags)
> >  {
> >  	return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..7b70ea2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >  
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb)
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags)
> >  {
> > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +				kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >  
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..d8d5d35 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -529,8 +529,16 @@ static int sb_finish_set_opts(struct super_block
> > *sb)
> >  		       sb->s_id, sb->s_type->name);
> >  
> >  	sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +	/*
> > +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> > to simply
> > +	 * leave the flag untouched because sb_clone_mnt_opts might
> > be handing
> > +	 * us a superblock that needs the flag to be cleared.
> > +	 */
> >  	if (selinux_is_sblabel_mnt(sb))
> >  		sbsec->flags |= SBLABEL_MNT;
> > +	else
> > +		sbsec->flags &= ~SBLABEL_MNT;
> >  
> >  	/* Initialize the root inode. */
> >  	rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -963,8 +971,11 @@ static int selinux_cmp_sb_context(const struct
> > super_block *oldsb,
> >  }
> >  
> >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > *oldsb,
> > -					struct super_block *newsb)
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags)
> >  {
> > +	int rc = 0;
> >  	const struct superblock_security_struct *oldsbsec = oldsb-
> > >s_security;
> >  	struct superblock_security_struct *newsbsec = newsb-
> > >s_security;
> >  
> > @@ -979,6 +990,13 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	if (!ss_initialized)
> >  		return 0;
> >  
> > +	/*
> > +	 * Specifying internal flags without providing a place to
> > +	 * place the results is not allowed.
> > +	 */
> > +	if (kern_flags && !set_kern_flags)
> > +		return -EINVAL;
> > +
> >  	/* how can we clone if the old one wasn't set up?? */
> >  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> >  
> > @@ -994,6 +1012,18 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	newsbsec->def_sid = oldsbsec->def_sid;
> >  	newsbsec->behavior = oldsbsec->behavior;
> >  
> > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> > +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) &&
> > !set_context) {
> > +		rc = security_fs_use(newsb);
> > +		if (rc)
> > +			goto out;
> > +	}
> > +
> > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> > {
> > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +	}
> > +
> >  	if (set_context) {
> >  		u32 sid = oldsbsec->mntpoint_sid;
> >  
> > @@ -1013,8 +1043,9 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	}
> >  
> >  	sb_finish_set_opts(newsb);
> > +out:
> >  	mutex_unlock(&newsbsec->lock);
> > -	return 0;
> > +	return rc;
> >  }
> >  
> >  static int selinux_parse_opts_str(char *options,

[PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.

Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c            | 17 ++++++++++++++++-
 include/linux/lsm_hooks.h |  4 +++-
 include/linux/security.h  |  8 ++++++--
 security/security.c       |  7 +++++--
 security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
 5 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..b8e0735 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
+			&kflags_out);
+	if (error)
+		return error;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 68d91e4..3cc9d77 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1409,7 +1409,9 @@ union security_list_options {
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
-					struct super_block *newsb);
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags);
 	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
diff --git a/include/linux/security.h b/include/linux/security.h
index 549cb82..b44e954 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb);
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags);
 int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
@@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
 }
 
 static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-					      struct super_block *newsb)
+					      struct super_block *newsb,
+					      unsigned long kern_flags,
+					      unsigned long *set_kern_flags)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index 714433e..3013237 100644
--- a/security/security.c
+++ b/security/security.c
@@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 EXPORT_SYMBOL(security_sb_set_mnt_opts);
 
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb)
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags)
 {
-	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
+	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
+				kern_flags, set_kern_flags);
 }
 EXPORT_SYMBOL(security_sb_clone_mnt_opts);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9926adb..9cc042d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
 	}
 
 	sbsec->flags |= SE_SBINITIALIZED;
+
+	/*
+	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
+	 * leave the flag untouched because sb_clone_mnt_opts might be handing
+	 * us a superblock that needs the flag to be cleared.
+	 */
 	if (selinux_is_sblabel_mnt(sb))
 		sbsec->flags |= SBLABEL_MNT;
+	else
+		sbsec->flags &= ~SBLABEL_MNT;
 
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
@@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
 }
 
 static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
-					struct super_block *newsb)
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags)
 {
+	int rc = 0;
 	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
 	struct superblock_security_struct *newsbsec = newsb->s_security;
 
@@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	if (!ss_initialized)
 		return 0;
 
+	/*
+	 * Specifying internal flags without providing a place to
+	 * place the results is not allowed.
+	 */
+	if (kern_flags && !set_kern_flags)
+		return -EINVAL;
+
 	/* how can we clone if the old one wasn't set up?? */
 	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
 
@@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	newsbsec->def_sid = oldsbsec->def_sid;
 	newsbsec->behavior = oldsbsec->behavior;
 
+	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
+		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
+		rc = security_fs_use(newsb);
+		if (rc)
+			goto out;
+	}
+
+	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
+		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
+		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+	}
+
 	if (set_context) {
 		u32 sid = oldsbsec->mntpoint_sid;
 
@@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	}
 
 	sb_finish_set_opts(newsb);
+out:
 	mutex_unlock(&newsbsec->lock);
-	return 0;
+	return rc;
 }
 
 static int selinux_parse_opts_str(char *options,
-- 
2.9.3

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Mon, 05 Jun 2017, Scott Mayhew wrote:

> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after cloning
> the security mount options.  As a result, setxattr's of security labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 17 ++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 63 insertions(+), 8 deletions(-)

This is against the "next" branch of
git@github.com:SELinuxProject/selinux-kernel.git.

-Scott

> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..b8e0735 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
> +			&kflags_out);
> +	if (error)
> +		return error;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 68d91e4..3cc9d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1409,7 +1409,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 549cb82..b44e954 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					      struct super_block *newsb)
> +					      struct super_block *newsb,
> +					      unsigned long kern_flags,
> +					      unsigned long *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index 714433e..3013237 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9926adb..9cc042d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
>  	}
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/*
> +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> +	 * leave the flag untouched because sb_clone_mnt_opts might be handing
> +	 * us a superblock that needs the flag to be cleared.
> +	 */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
>  	struct superblock_security_struct *newsbsec = newsb->s_security;
>  
> @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	if (!ss_initialized)
>  		return 0;
>  
> +	/*
> +	 * Specifying internal flags without providing a place to
> +	 * place the results is not allowed.
> +	 */
> +	if (kern_flags && !set_kern_flags)
> +		return -EINVAL;
> +
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
> @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Mon, 2017-06-05 at 11:45 -0400, Scott Mayhew wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels
> because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable
> SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount
> option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>

Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
Resolves: https://github.com/SELinuxProject/selinux-kernel/issues/35

> ---
>  fs/nfs/super.c            | 17 ++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..b8e0735 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s, kflags,
> +			&kflags_out);
> +	if (error)
> +		return error;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 68d91e4..3cc9d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1409,7 +1409,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct
> security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 549cb82..b44e954 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct
> security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct
> super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct
> super_block *oldsb,
> -					      struct super_block
> *newsb)
> +					      struct super_block
> *newsb,
> +					      unsigned long
> kern_flags,
> +					      unsigned long
> *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index 714433e..3013237 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9926adb..9cc042d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block
> *sb)
>  	}
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/*
> +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> to simply
> +	 * leave the flag untouched because sb_clone_mnt_opts might
> be handing
> +	 * us a superblock that needs the flag to be cleared.
> +	 */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct
> super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block
> *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb-
> >s_security;
>  	struct superblock_security_struct *newsbsec = newsb-
> >s_security;
>  
> @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	if (!ss_initialized)
>  		return 0;
>  
> +	/*
> +	 * Specifying internal flags without providing a place to
> +	 * place the results is not allowed.
> +	 */
> +	if (kern_flags && !set_kern_flags)
> +		return -EINVAL;
> +
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
> @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) &&
> !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Paul Moore]

On Mon, Jun 5, 2017 at 11:45 AM, Scott Mayhew <smayhew@redhat.com> wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
>
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after cloning
> the security mount options.  As a result, setxattr's of security labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
>
> Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount option.
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 17 ++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 63 insertions(+), 8 deletions(-)

Thanks for sorting this out Scott and Stephen.

NFS folks, any objections to this patch?  If not, I'd like to pull
this into the SELinux tree but I'd like to have an ACK from you before
I do.

> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..b8e0735 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
>                           struct nfs_mount_info *mount_info)
>  {
> +       int error;
> +       unsigned long kflags = 0, kflags_out = 0;
> +
>         /* clone any lsm security options from the parent to the new sb */
>         if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
>                 return -ESTALE;
> -       return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> +
> +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +               kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +       error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
> +                       &kflags_out);
> +       if (error)
> +               return error;
> +
> +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +               !(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +               NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +       return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 68d91e4..3cc9d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1409,7 +1409,9 @@ union security_list_options {
>                                 unsigned long kern_flags,
>                                 unsigned long *set_kern_flags);
>         int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -                                       struct super_block *newsb);
> +                                       struct super_block *newsb,
> +                                       unsigned long kern_flags,
> +                                       unsigned long *set_kern_flags);
>         int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
>         int (*dentry_init_security)(struct dentry *dentry, int mode,
>                                         const struct qstr *name, void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 549cb82..b44e954 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>                                 unsigned long kern_flags,
>                                 unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                               struct super_block *newsb);
> +                               struct super_block *newsb,
> +                               unsigned long kern_flags,
> +                               unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>                                         const struct qstr *name, void **ctx,
> @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
>  }
>
>  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                                             struct super_block *newsb)
> +                                             struct super_block *newsb,
> +                                             unsigned long kern_flags,
> +                                             unsigned long *set_kern_flags)
>  {
>         return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index 714433e..3013237 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                               struct super_block *newsb)
> +                               struct super_block *newsb,
> +                               unsigned long kern_flags,
> +                               unsigned long *set_kern_flags)
>  {
> -       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +                               kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9926adb..9cc042d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
>         }
>
>         sbsec->flags |= SE_SBINITIALIZED;
> +
> +       /*
> +        * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> +        * leave the flag untouched because sb_clone_mnt_opts might be handing
> +        * us a superblock that needs the flag to be cleared.
> +        */
>         if (selinux_is_sblabel_mnt(sb))
>                 sbsec->flags |= SBLABEL_MNT;
> +       else
> +               sbsec->flags &= ~SBLABEL_MNT;
>
>         /* Initialize the root inode. */
>         rc = inode_doinit_with_dentry(root_inode, root);
> @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
>  }
>
>  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                                       struct super_block *newsb)
> +                                       struct super_block *newsb,
> +                                       unsigned long kern_flags,
> +                                       unsigned long *set_kern_flags)
>  {
> +       int rc = 0;
>         const struct superblock_security_struct *oldsbsec = oldsb->s_security;
>         struct superblock_security_struct *newsbsec = newsb->s_security;
>
> @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>         if (!ss_initialized)
>                 return 0;
>
> +       /*
> +        * Specifying internal flags without providing a place to
> +        * place the results is not allowed.
> +        */
> +       if (kern_flags && !set_kern_flags)
> +               return -EINVAL;
> +
>         /* how can we clone if the old one wasn't set up?? */
>         BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>
> @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>         newsbsec->def_sid = oldsbsec->def_sid;
>         newsbsec->behavior = oldsbsec->behavior;
>
> +       if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +               !(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
> +               rc = security_fs_use(newsb);
> +               if (rc)
> +                       goto out;
> +       }
> +
> +       if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> +               newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +               *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +       }
> +
>         if (set_context) {
>                 u32 sid = oldsbsec->mntpoint_sid;
>
> @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>         }
>
>         sb_finish_set_opts(newsb);
> +out:
>         mutex_unlock(&newsbsec->lock);
> -       return 0;
> +       return rc;
>  }
>
>  static int selinux_parse_opts_str(char *options,
> --
> 2.9.3
>



-- 
paul moore
www.paul-moore.com

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[J . Bruce Fields]

On Mon, Jun 05, 2017 at 05:21:55PM -0400, Paul Moore wrote:
> On Mon, Jun 5, 2017 at 11:45 AM, Scott Mayhew <smayhew@redhat.com> wrote:
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> >
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after cloning
> > the security mount options.  As a result, setxattr's of security labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> > with the context= mount option will not show the correct labels because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> >
> > Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> > be displayed for NFSv4.2 mounts mounted with the context= mount option.
> >
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 17 ++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
> >  5 files changed, 63 insertions(+), 8 deletions(-)
> 
> Thanks for sorting this out Scott and Stephen.
> 
> NFS folks, any objections to this patch?  If not, I'd like to pull
> this into the SELinux tree but I'd like to have an ACK from you before
> I do.

Looks OK to me, but I think it's Trond or Anna (added to cc) that you
want the ACK from.

--b.

> 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..b8e0735 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
> >                           struct nfs_mount_info *mount_info)
> >  {
> > +       int error;
> > +       unsigned long kflags = 0, kflags_out = 0;
> > +
> >         /* clone any lsm security options from the parent to the new sb */
> >         if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
> >                 return -ESTALE;
> > -       return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> > +
> > +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +               kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +       error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
> > +                       &kflags_out);
> > +       if (error)
> > +               return error;
> > +
> > +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +               !(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +               NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +       return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 68d91e4..3cc9d77 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1409,7 +1409,9 @@ union security_list_options {
> >                                 unsigned long kern_flags,
> >                                 unsigned long *set_kern_flags);
> >         int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -                                       struct super_block *newsb);
> > +                                       struct super_block *newsb,
> > +                                       unsigned long kern_flags,
> > +                                       unsigned long *set_kern_flags);
> >         int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
> >         int (*dentry_init_security)(struct dentry *dentry, int mode,
> >                                         const struct qstr *name, void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index 549cb82..b44e954 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
> >                                 unsigned long kern_flags,
> >                                 unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                               struct super_block *newsb);
> > +                               struct super_block *newsb,
> > +                               unsigned long kern_flags,
> > +                               unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >                                         const struct qstr *name, void **ctx,
> > @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
> >  }
> >
> >  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                                             struct super_block *newsb)
> > +                                             struct super_block *newsb,
> > +                                             unsigned long kern_flags,
> > +                                             unsigned long *set_kern_flags)
> >  {
> >         return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index 714433e..3013237 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                               struct super_block *newsb)
> > +                               struct super_block *newsb,
> > +                               unsigned long kern_flags,
> > +                               unsigned long *set_kern_flags)
> >  {
> > -       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +                               kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 9926adb..9cc042d 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
> >         }
> >
> >         sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +       /*
> > +        * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> > +        * leave the flag untouched because sb_clone_mnt_opts might be handing
> > +        * us a superblock that needs the flag to be cleared.
> > +        */
> >         if (selinux_is_sblabel_mnt(sb))
> >                 sbsec->flags |= SBLABEL_MNT;
> > +       else
> > +               sbsec->flags &= ~SBLABEL_MNT;
> >
> >         /* Initialize the root inode. */
> >         rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
> >  }
> >
> >  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                                       struct super_block *newsb)
> > +                                       struct super_block *newsb,
> > +                                       unsigned long kern_flags,
> > +                                       unsigned long *set_kern_flags)
> >  {
> > +       int rc = 0;
> >         const struct superblock_security_struct *oldsbsec = oldsb->s_security;
> >         struct superblock_security_struct *newsbsec = newsb->s_security;
> >
> > @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> >         if (!ss_initialized)
> >                 return 0;
> >
> > +       /*
> > +        * Specifying internal flags without providing a place to
> > +        * place the results is not allowed.
> > +        */
> > +       if (kern_flags && !set_kern_flags)
> > +               return -EINVAL;
> > +
> >         /* how can we clone if the old one wasn't set up?? */
> >         BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> >
> > @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> >         newsbsec->def_sid = oldsbsec->def_sid;
> >         newsbsec->behavior = oldsbsec->behavior;
> >
> > +       if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> > +               !(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
> > +               rc = security_fs_use(newsb);
> > +               if (rc)
> > +                       goto out;
> > +       }
> > +
> > +       if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> > +               newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +               *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +       }
> > +
> >         if (set_context) {
> >                 u32 sid = oldsbsec->mntpoint_sid;
> >
> > @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> >         }
> >
> >         sb_finish_set_opts(newsb);
> > +out:
> >         mutex_unlock(&newsbsec->lock);
> > -       return 0;
> > +       return rc;
> >  }
> >
> >  static int selinux_parse_opts_str(char *options,
> > --
> > 2.9.3
> >
> 
> 
> 
> -- 
> paul moore
> www.paul-moore.com

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Paul Moore]

On Mon, Jun 5, 2017 at 8:46 PM, J . Bruce Fields <bfields@fieldses.org> wrote:
> On Mon, Jun 05, 2017 at 05:21:55PM -0400, Paul Moore wrote:
>> On Mon, Jun 5, 2017 at 11:45 AM, Scott Mayhew <smayhew@redhat.com> wrote:
>> > When an NFSv4 client performs a mount operation, it first mounts the
>> > NFSv4 root and then does path walk to the exported path and performs a
>> > submount on that, cloning the security mount options from the root's
>> > superblock to the submount's superblock in the process.
>> >
>> > Unless the NFS server has an explicit fsid=0 export with the
>> > "security_label" option, the NFSv4 root superblock will not have
>> > SBLABEL_MNT set, and neither will the submount superblock after cloning
>> > the security mount options.  As a result, setxattr's of security labels
>> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
>> > with the context= mount option will not show the correct labels because
>> > the nfs_server->caps flags of the cloned superblock will still have
>> > NFS_CAP_SECURITY_LABEL set.
>> >
>> > Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
>> > behavior will ensure that the SBLABEL_MNT flag has the correct value
>> > when the client traverses from an exported path without the
>> > "security_label" option to one with the "security_label" option and
>> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
>> > set upon return from security_sb_clone_mnt_opts() and clearing
>> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
>> > be displayed for NFSv4.2 mounts mounted with the context= mount option.
>> >
>> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
>> > ---
>> >  fs/nfs/super.c            | 17 ++++++++++++++++-
>> >  include/linux/lsm_hooks.h |  4 +++-
>> >  include/linux/security.h  |  8 ++++++--
>> >  security/security.c       |  7 +++++--
>> >  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>> >  5 files changed, 63 insertions(+), 8 deletions(-)
>>
>> Thanks for sorting this out Scott and Stephen.
>>
>> NFS folks, any objections to this patch?  If not, I'd like to pull
>> this into the SELinux tree but I'd like to have an ACK from you before
>> I do.
>
> Looks OK to me, but I think it's Trond or Anna (added to cc) that you
> want the ACK from.

It's been about four days with not comments from the NFS folks so I'm
just going to go ahead and merge this into the selinux/next branch so
we can get some more widespread testing.  NFS folks, if you want to
object please do so a week or two before the next merge window opens,
otherwise I'm going to send this patch upstream.

Thanks Scott for working on this patch, and everyone else for their
comments, testing, and review.

-- 
paul moore
www.paul-moore.com

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> On Fri, 26 May 2017, Stephen Smalley wrote:
> 
> > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > When the client traverses from filesystem exported without the
> > > "security_label" option to one exported with the "security_label"
> > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > security_sb_set_mnt_opts() so that the new superblock has
> > > SBLABEL_MNT
> > > set in its security mount options.  Otherwise, attempts to set
> > > security
> > > labels via setxattr over NFSv4.2 will fail.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 2f3822a..d7a3b89 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > *mntroot,
> > >  			  struct nfs_mount_info *mount_info)
> > >  {
> > > +	int error;
> > > +	unsigned long kflags = 0, kflags_out = 0;
> > > +	struct security_mnt_opts opts;
> > > +
> > >  	/* clone any lsm security options from the parent to the
> > > new
> > > sb */
> > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > rpc_ops->dir_inode_ops)
> > > 
> > >  		return -ESTALE;
> > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > NFS_CAP_SECURITY_LABEL)) {
> > > +		memset(&opts, 0, sizeof(opts));
> > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > +
> > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > kflags,
> > > &kflags_out);
> > > +		if (error)
> > > +			goto err;
> > > +
> > > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > +			NFS_SB(s)->caps &=
> > > ~NFS_CAP_SECURITY_LABEL;
> > > +	}
> > > +err:
> > > +	return error;
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > 
> > Could this clobber a context set via context= mount option?
> 
> Argh, yes I suppose it could.  In my first attempt to fix this, I
> added
> a security_sb_get_mnt_opts() hook to get the original mount options
> and
> then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
> security_sb_set_mnt_opts().  When I saw that
> security_sb_set_mnt_opts()
> wouldn't allow me to change a superblock that had already been
> initialized, I got rid of the hook and added the check in patch 1...
> maybe a combination of the two is needed?
> 
> Testing it again now, I'm not sure the context= mount option is
> working
> correctly with the latest kernel.

Looks like you are correct,
https://github.com/SELinuxProject/selinux-kernel/issues/35

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[J . Bruce Fields]

On Tue, May 30, 2017 at 10:38:45AM -0400, Stephen Smalley wrote:
> On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> > On Fri, 26 May 2017, Stephen Smalley wrote:
> > 
> > > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > > When the client traverses from filesystem exported without the
> > > > "security_label" option to one exported with the "security_label"
> > > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > > security_sb_set_mnt_opts() so that the new superblock has
> > > > SBLABEL_MNT
> > > > set in its security mount options.  Otherwise, attempts to set
> > > > security
> > > > labels via setxattr over NFSv4.2 will fail.
> > > > 
> > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > > ---
> > > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > > index 2f3822a..d7a3b89 100644
> > > > --- a/fs/nfs/super.c
> > > > +++ b/fs/nfs/super.c
> > > > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > > *mntroot,
> > > >  			  struct nfs_mount_info *mount_info)
> > > >  {
> > > > +	int error;
> > > > +	unsigned long kflags = 0, kflags_out = 0;
> > > > +	struct security_mnt_opts opts;
> > > > +
> > > >  	/* clone any lsm security options from the parent to the
> > > > new
> > > > sb */
> > > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > > rpc_ops->dir_inode_ops)
> > > > 
> > > >  		return -ESTALE;
> > > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > > >sb,
> > > > s);
> > > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > > >sb,
> > > > s);
> > > > +	if (error)
> > > > +		goto err;
> > > > +
> > > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > > NFS_CAP_SECURITY_LABEL)) {
> > > > +		memset(&opts, 0, sizeof(opts));
> > > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > > +
> > > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > > kflags,
> > > > &kflags_out);
> > > > +		if (error)
> > > > +			goto err;
> > > > +
> > > > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > > +			NFS_SB(s)->caps &=
> > > > ~NFS_CAP_SECURITY_LABEL;
> > > > +	}
> > > > +err:
> > > > +	return error;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > > 
> > > Could this clobber a context set via context= mount option?
> > 
> > Argh, yes I suppose it could.  In my first attempt to fix this, I
> > added
> > a security_sb_get_mnt_opts() hook to get the original mount options
> > and
> > then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
> > security_sb_set_mnt_opts().  When I saw that
> > security_sb_set_mnt_opts()
> > wouldn't allow me to change a superblock that had already been
> > initialized, I got rid of the hook and added the check in patch 1...
> > maybe a combination of the two is needed?
> > 
> > Testing it again now, I'm not sure the context= mount option is
> > working
> > correctly with the latest kernel.
> 
> Looks like you are correct,
> https://github.com/SELinuxProject/selinux-kernel/issues/35

Ugh.  So, to make sure I understand: the desired behavior is that in the
case the client mounts with a context= option, behavior is exactly as if
the client or server didn't support the new security labeling protocol.
That would make sense to me.

--b.

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Tue, 2017-05-30 at 15:40 -0400, J . Bruce Fields wrote:
> On Tue, May 30, 2017 at 10:38:45AM -0400, Stephen Smalley wrote:
> > On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> > > On Fri, 26 May 2017, Stephen Smalley wrote:
> > > 
> > > > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > > > When the client traverses from filesystem exported without
> > > > > the
> > > > > "security_label" option to one exported with the
> > > > > "security_label"
> > > > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > > > security_sb_set_mnt_opts() so that the new superblock has
> > > > > SBLABEL_MNT
> > > > > set in its security mount options.  Otherwise, attempts to
> > > > > set
> > > > > security
> > > > > labels via setxattr over NFSv4.2 will fail.
> > > > > 
> > > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > > > ---
> > > > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > > > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > > > index 2f3822a..d7a3b89 100644
> > > > > --- a/fs/nfs/super.c
> > > > > +++ b/fs/nfs/super.c
> > > > > @@ -2544,10 +2544,31 @@
> > > > > EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > > > >  int nfs_clone_sb_security(struct super_block *s, struct
> > > > > dentry
> > > > > *mntroot,
> > > > >  			  struct nfs_mount_info *mount_info)
> > > > >  {
> > > > > +	int error;
> > > > > +	unsigned long kflags = 0, kflags_out = 0;
> > > > > +	struct security_mnt_opts opts;
> > > > > +
> > > > >  	/* clone any lsm security options from the parent to
> > > > > the
> > > > > new
> > > > > sb */
> > > > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > > > rpc_ops->dir_inode_ops)
> > > > > 
> > > > >  		return -ESTALE;
> > > > > -	return security_sb_clone_mnt_opts(mount_info-
> > > > > >cloned-
> > > > > > sb,
> > > > > 
> > > > > s);
> > > > > +	error = security_sb_clone_mnt_opts(mount_info-
> > > > > >cloned-
> > > > > > sb,
> > > > > 
> > > > > s);
> > > > > +	if (error)
> > > > > +		goto err;
> > > > > +
> > > > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > > > NFS_CAP_SECURITY_LABEL)) {
> > > > > +		memset(&opts, 0, sizeof(opts));
> > > > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > > > +
> > > > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > > > kflags,
> > > > > &kflags_out);
> > > > > +		if (error)
> > > > > +			goto err;
> > > > > +
> > > > > +		if (!(kflags_out &
> > > > > SECURITY_LSM_NATIVE_LABELS))
> > > > > +			NFS_SB(s)->caps &=
> > > > > ~NFS_CAP_SECURITY_LABEL;
> > > > > +	}
> > > > > +err:
> > > > > +	return error;
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > > > 
> > > > Could this clobber a context set via context= mount option?
> > > 
> > > Argh, yes I suppose it could.  In my first attempt to fix this, I
> > > added
> > > a security_sb_get_mnt_opts() hook to get the original mount
> > > options
> > > and
> > > then passed that along with the SECURITY_LSM_NATIVE_LABELS flag
> > > to
> > > security_sb_set_mnt_opts().  When I saw that
> > > security_sb_set_mnt_opts()
> > > wouldn't allow me to change a superblock that had already been
> > > initialized, I got rid of the hook and added the check in patch
> > > 1...
> > > maybe a combination of the two is needed?
> > > 
> > > Testing it again now, I'm not sure the context= mount option is
> > > working
> > > correctly with the latest kernel.
> > 
> > Looks like you are correct,
> > https://github.com/SELinuxProject/selinux-kernel/issues/35
> 
> Ugh.  So, to make sure I understand: the desired behavior is that in
> the
> case the client mounts with a context= option, behavior is exactly as
> if
> the client or server didn't support the new security labeling
> protocol.
> That would make sense to me.

Yes, that's correct.  And in theory that is what nfs_set_sb_security()
is trying to do by clearing NFS_CAP_SECURITY_LABEL if
SECURITY_LSM_NATIVE_LABELS was not set by the security hook.

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[Stephen Smalley]

On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> The problem is related to how filesystems are mounted in NFSv4.

What kernel version and what is a reproducer for the problem?  I don't
seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.

> 
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.
> 
> NFS servers with a modern nfs-utils package will automatically create
> a
> pseudo fs to fill in the gaps (including the root itself) leading up
> to
> the actual export, so it is uncommon these days for an NFS server to
> have an explicit fsid=0 export.
> 
> Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> flag on an initialized superblock would ensure that SBLABEL_MNT is
> set
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option.
> 
> Scott Mayhew (2):
>   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
>     initialized superblock
>   nfs: update labeling behavior on a superblock when submounting
> 
>  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
>  security/selinux/hooks.c |  4 ++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[J . Bruce Fields]

On Fri, May 26, 2017 at 10:48:17AM -0400, Stephen Smalley wrote:
> On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> > The problem is related to how filesystems are mounted in NFSv4.
> 
> What kernel version and what is a reproducer for the problem?  I don't
> seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.

Basically just mount an export with security_label set, mount over NFS,
"ls -Z" will (correctly) show you the server-side security labels, but
"chcon" will fail with ENOTSUP.

If that's not reproducing, maybe you chould show us "exports -v" and
"mount" output--it might not reproduce if you have an "fsid=0" export or
if you're mounting with a protocol version older than 4.2.

--b.

> 
> > 
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.
> > 
> > NFS servers with a modern nfs-utils package will automatically create
> > a
> > pseudo fs to fill in the gaps (including the root itself) leading up
> > to
> > the actual export, so it is uncommon these days for an NFS server to
> > have an explicit fsid=0 export.
> > 
> > Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> > flag on an initialized superblock would ensure that SBLABEL_MNT is
> > set
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option.
> > 
> > Scott Mayhew (2):
> >   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
> >     initialized superblock
> >   nfs: update labeling behavior on a superblock when submounting
> > 
> >  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
> >  security/selinux/hooks.c |  4 ++--
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[J . Bruce Fields]

On Fri, May 26, 2017 at 11:17:22AM -0400, J . Bruce Fields wrote:
> On Fri, May 26, 2017 at 10:48:17AM -0400, Stephen Smalley wrote:
> > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> > > The problem is related to how filesystems are mounted in NFSv4.
> > 
> > What kernel version and what is a reproducer for the problem?  I don't
> > seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.
> 
> Basically just mount an export with security_label set, mount over NFS,
> "ls -Z" will (correctly) show you the server-side security labels, but
> "chcon" will fail with ENOTSUP.
> 
> If that's not reproducing, maybe you chould show us "exports -v" and
> "mount" output--it might not reproduce if you have an "fsid=0" export or
> if you're mounting with a protocol version older than 4.2.

Oh, and, sorry, kernel version: I think you just need something recent
enough to support "security_label", which went into 4.11.

--b.

> 
> --b.
> 
> > 
> > > 
> > > When an NFSv4 client performs a mount operation, it first mounts the
> > > NFSv4 root and then does path walk to the exported path and performs
> > > a
> > > submount on that, cloning the security mount options from the root's
> > > superblock to the submount's superblock in the process.
> > > 
> > > Unless the NFS server has an explicit fsid=0 export with the
> > > "security_label" option, the NFSv4 root superblock will not have
> > > SBLABEL_MNT set, and neither will the submount superblock after
> > > cloning
> > > the security mount options.  As a result, setxattr's of security
> > > labels
> > > over NFSv4.2 will fail.
> > > 
> > > NFS servers with a modern nfs-utils package will automatically create
> > > a
> > > pseudo fs to fill in the gaps (including the root itself) leading up
> > > to
> > > the actual export, so it is uncommon these days for an NFS server to
> > > have an explicit fsid=0 export.
> > > 
> > > Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> > > flag on an initialized superblock would ensure that SBLABEL_MNT is
> > > set
> > > when the client traverses from an exported path without the
> > > "security_label" option to one with the "security_label" option.
> > > 
> > > Scott Mayhew (2):
> > >   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
> > >     initialized superblock
> > >   nfs: update labeling behavior on a superblock when submounting
> > > 
> > >  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
> > >  security/selinux/hooks.c |  4 ++--
> > >  2 files changed, 24 insertions(+), 3 deletions(-)
> > > 

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[Scott Mayhew]

On Fri, 26 May 2017, Stephen Smalley wrote:

> On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> > The problem is related to how filesystems are mounted in NFSv4.
> 
> What kernel version and what is a reproducer for the problem?  I don't
> seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.

I'm testing with Fedora 25 but with the upstream kernel.  The Fedora
kernel doesn't have commit 32ddd944 ("nfsd: opt in to labeled nfs per
export") yet.

-Scott
> 
> > 
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.
> > 
> > NFS servers with a modern nfs-utils package will automatically create
> > a
> > pseudo fs to fill in the gaps (including the root itself) leading up
> > to
> > the actual export, so it is uncommon these days for an NFS server to
> > have an explicit fsid=0 export.
> > 
> > Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> > flag on an initialized superblock would ensure that SBLABEL_MNT is
> > set
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option.
> > 
> > Scott Mayhew (2):
> >   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
> >     initialized superblock
> >   nfs: update labeling behavior on a superblock when submounting
> > 
> >  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
> >  security/selinux/hooks.c |  4 ++--
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> >