2010-07-01 19:08:22

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH] security: move LSM xattrnames to xattr.h

Make the security extended attributes names global. Updated to move
the remaining Smack xattrs.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
include/linux/capability.h | 3 ---
include/linux/xattr.h | 14 ++++++++++++++
security/selinux/hooks.c | 3 ---
security/smack/smack.h | 10 ----------
4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 39e5ff5..90012b9 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -49,9 +49,6 @@ typedef struct __user_cap_data_struct {
} __user *cap_user_data_t;


-#define XATTR_CAPS_SUFFIX "capability"
-#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
-
#define VFS_CAP_REVISION_MASK 0xFF000000
#define VFS_CAP_REVISION_SHIFT 24
#define VFS_CAP_FLAGS_MASK ~VFS_CAP_REVISION_MASK
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 0cfa1e9..f1e5bde 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -33,6 +33,20 @@
#define XATTR_USER_PREFIX "user."
#define XATTR_USER_PREFIX_LEN (sizeof (XATTR_USER_PREFIX) - 1)

+/* Security namespace */
+#define XATTR_SELINUX_SUFFIX "selinux"
+#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
+
+#define XATTR_SMACK_SUFFIX "SMACK64"
+#define XATTR_SMACK_IPIN "SMACK64IPIN"
+#define XATTR_SMACK_IPOUT "SMACK64IPOUT"
+#define XATTR_NAME_SMACK XATTR_SECURITY_PREFIX XATTR_SMACK_SUFFIX
+#define XATTR_NAME_SMACKIPIN XATTR_SECURITY_PREFIX XATTR_SMACK_IPIN
+#define XATTR_NAME_SMACKIPOUT XATTR_SECURITY_PREFIX XATTR_SMACK_IPOUT
+
+#define XATTR_CAPS_SUFFIX "capability"
+#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
+
struct inode;
struct dentry;

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0f524b7..85338f0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -87,9 +87,6 @@
#include "netlabel.h"
#include "audit.h"

-#define XATTR_SELINUX_SUFFIX "selinux"
-#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
-
#define NUM_SEL_MNT_OPTS 5

extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
diff --git a/security/smack/smack.h b/security/smack/smack.h
index c6e9aca..43ae747 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -123,16 +123,6 @@ struct smack_known {
#define SMK_FSHAT "smackfshat="
#define SMK_FSROOT "smackfsroot="

-/*
- * xattr names
- */
-#define XATTR_SMACK_SUFFIX "SMACK64"
-#define XATTR_SMACK_IPIN "SMACK64IPIN"
-#define XATTR_SMACK_IPOUT "SMACK64IPOUT"
-#define XATTR_NAME_SMACK XATTR_SECURITY_PREFIX XATTR_SMACK_SUFFIX
-#define XATTR_NAME_SMACKIPIN XATTR_SECURITY_PREFIX XATTR_SMACK_IPIN
-#define XATTR_NAME_SMACKIPOUT XATTR_SECURITY_PREFIX XATTR_SMACK_IPOUT
-
#define SMACK_CIPSO_OPTION "-CIPSO"

/*
--
1.6.6.1


2010-07-02 00:16:08

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Thu, 1 Jul 2010, Mimi Zohar wrote:

> Make the security extended attributes names global. Updated to move
> the remaining Smack xattrs.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

--
James Morris
<[email protected]>

2010-11-03 17:00:54

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Tuesday, October 12, 2010 09:40:34 am Mimi Zohar wrote:
> On Tue, 2010-10-12 at 09:19 -0400, Steve Grubb wrote:
> > On Tuesday, October 12, 2010 09:06:09 am Mimi Zohar wrote:
> > > On Tue, 2010-10-12 at 14:14 +0300, Ozan Çağlayan wrote:
> > > > Cuma 02 Temmuz 2010 günü (saat 03:16:01) James Morris şunları yazmıştı:
> > > > > On Thu, 1 Jul 2010, Mimi Zohar wrote:
> > > > > > Make the security extended attributes names global. Updated to
> > > > > > move the remaining Smack xattrs.
> > > > > >
> > > > > > Signed-off-by: Mimi Zohar <[email protected]>
> > > > > > Acked-by: Serge Hallyn <[email protected]>
> > > >
> > > > This drops
> > > >
> > > > #define XATTR_CAPS_SUFFIX "capability"
> > > > #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> > > >
> > > > definitions from capability.h and puts them in xattr.h's #ifdef
> > > > __KERNEL__ section making them invisible to userspace like libcap-ng
> > > > causing build failures.
> > > >
> > > > Am I wrong?
> > >
> > > You're correct. It's the same reason that cap-ng.c has to define
> > > 'security'.
> > >
> > > #ifdef VFS_CAP_U32
> > >
> > > #include <attr/xattr.h>
> > > #define XATTR_SECURITY_PREFIX "security."
> > >
> > > Am cc'ing Steve.
> >
> > So does this mean I need to provide more definitions for libcap-ng to
> > work with future kernels or are you asking my opinion? My opinion is
> > that if user space needs it to work correctly, please let it be
> > available so I don't have to make my own define which may be inaccurate
> > one day.
> >
> > Thanks,
> > -Steve
>
> Before making any changes to the kernel xattr.h, I want to understand
> the reason for two xattr.h files, one in /usr/include/linux/ and the
> other in /usr/include/xattr/. /usr/include/linux/xattr.h contains those
> elements not defined as __kernel__, while /usr/include/xattr/xattr.h
> contains that and other definitions. Will changing the kernel xattr.h
> version change both?
>
> As long as we're making this change, should 'security' also be defined
> outside of the __kernel__ definitions?

I guess no one fixed this before 2.6.36 was finalized. Removing the define has broke user
space compilation for anything that works on file based capabilities. I can define it
myself, but if the kernel folks ever change the string, then we have more than just a
compile problem, we have runtime problems because I can no longer use the correct
string.

So, what was the gain for breaking user space?

-Steve

2010-11-03 17:38:47

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Wed, 2010-11-03 at 13:00 -0400, Steve Grubb wrote:
> On Tuesday, October 12, 2010 09:40:34 am Mimi Zohar wrote:
> > On Tue, 2010-10-12 at 09:19 -0400, Steve Grubb wrote:
> > > On Tuesday, October 12, 2010 09:06:09 am Mimi Zohar wrote:
> > > > On Tue, 2010-10-12 at 14:14 +0300, Ozan Çağlayan wrote:
> > > > > Cuma 02 Temmuz 2010 günü (saat 03:16:01) James Morris şunları yazmıştı:
> > > > > > On Thu, 1 Jul 2010, Mimi Zohar wrote:
> > > > > > > Make the security extended attributes names global. Updated to
> > > > > > > move the remaining Smack xattrs.
> > > > > > >
> > > > > > > Signed-off-by: Mimi Zohar <[email protected]>
> > > > > > > Acked-by: Serge Hallyn <[email protected]>
> > > > >
> > > > > This drops
> > > > >
> > > > > #define XATTR_CAPS_SUFFIX "capability"
> > > > > #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> > > > >
> > > > > definitions from capability.h and puts them in xattr.h's #ifdef
> > > > > __KERNEL__ section making them invisible to userspace like libcap-ng
> > > > > causing build failures.
> > > > >
> > > > > Am I wrong?
> > > >
> > > > You're correct. It's the same reason that cap-ng.c has to define
> > > > 'security'.
> > > >
> > > > #ifdef VFS_CAP_U32
> > > >
> > > > #include <attr/xattr.h>
> > > > #define XATTR_SECURITY_PREFIX "security."
> > > >
> > > > Am cc'ing Steve.
> > >
> > > So does this mean I need to provide more definitions for libcap-ng to
> > > work with future kernels or are you asking my opinion? My opinion is
> > > that if user space needs it to work correctly, please let it be
> > > available so I don't have to make my own define which may be inaccurate
> > > one day.
> > >
> > > Thanks,
> > > -Steve
> >
> > Before making any changes to the kernel xattr.h, I want to understand
> > the reason for two xattr.h files, one in /usr/include/linux/ and the
> > other in /usr/include/xattr/. /usr/include/linux/xattr.h contains those
> > elements not defined as __kernel__, while /usr/include/xattr/xattr.h
> > contains that and other definitions. Will changing the kernel xattr.h
> > version change both?
> >
> > As long as we're making this change, should 'security' also be defined
> > outside of the __kernel__ definitions?
>
> I guess no one fixed this before 2.6.36 was finalized. Removing the define has broke user
> space compilation for anything that works on file based capabilities. I can define it
> myself, but if the kernel folks ever change the string, then we have more than just a
> compile problem, we have runtime problems because I can no longer use the correct
> string.
>
> So, what was the gain for breaking user space?
>
> -Steve

Sorry I dropped the ball. Was expecting some kind of response to my
question above, and then forgot about it.

All of the 'security' xattrs were moved to fsmagic.h, including
capability. Not only those that EVM protects, but others like
XATTR_NAME_SMACKIPIN/OUT (based on Casey's request).

Mimi

2010-11-03 17:57:59

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Wednesday, November 03, 2010 01:38:33 pm Mimi Zohar wrote:
> > > As long as we're making this change, should 'security' also be defined
> > > outside of the __kernel__ definitions?
> >
> > I guess no one fixed this before 2.6.36 was finalized. Removing the
> > define has broke user space compilation for anything that works on file
> > based capabilities. I can define it myself, but if the kernel folks ever
> > change the string, then we have more than just a compile problem, we
> > have runtime problems because I can no longer use the correct string.
> >
> > So, what was the gain for breaking user space?
> >
> > -Steve
>
> Sorry I dropped the ball. Was expecting some kind of response to my
> question above, and then forgot about it.
>
> All of the 'security' xattrs were moved to fsmagic.h, including
> capability. Not only those that EVM protects, but others like
> XATTR_NAME_SMACKIPIN/OUT (based on Casey's request).

If user space has to know the exact contents of a string in order to do something that
the kernel understands, then its part of a public API. I've made my own define and
released a new copy of libcap-ng. So, if the contents of the string ever change, or
becomes deprecated, you'll now have user space apps using the old values no matter
what.

-Steve

2010-11-03 19:02:14

by Eric Paris

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Wed, Nov 3, 2010 at 1:57 PM, Steve Grubb <[email protected]> wrote:
> On Wednesday, November 03, 2010 01:38:33 pm Mimi Zohar wrote:
>> > > As long as we're making this change, should 'security' also be defined
>> > > outside of the __kernel__ definitions?
>> >
>> > I guess no one fixed this before 2.6.36 was finalized. Removing the
>> > define has broke user ?space compilation for anything that works on file
>> > based capabilities. I can define it myself, but if the kernel folks ever
>> > change the string, then we have more than just a compile problem, we
>> > have runtime problems because I can no longer use the correct string.
>> >
>> > So, what was the gain for breaking user space?
>> >
>> > -Steve
>>
>> Sorry I dropped the ball. Was expecting some kind of response to my
>> question above, and then forgot about it.
>>
>> All of the 'security' xattrs were moved to fsmagic.h, including
>> capability. Not only those that EVM protects, but others like
>> XATTR_NAME_SMACKIPIN/OUT (based on Casey's request).
>
> If user space has to know the exact contents of a string in order to do something that
> the kernel understands, then its part of a public API. I've made my own define and
> released a new copy of libcap-ng. So, if the contents of the string ever change, or
> becomes deprecated, you'll now have user space apps using the old values no matter
> what.

You're right Steve, it is ABI, we broke it, and we can fix it. What
are you having to define and what are you including. What files did
you used to get these defines from?

-Eric

2010-11-03 19:47:05

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Wednesday, November 03, 2010 03:02:11 pm Eric Paris wrote:
> On Wed, Nov 3, 2010 at 1:57 PM, Steve Grubb <[email protected]> wrote:
> > On Wednesday, November 03, 2010 01:38:33 pm Mimi Zohar wrote:
> >> > > As long as we're making this change, should 'security' also be
> >> > > defined outside of the __kernel__ definitions?
> >> >
> >> > I guess no one fixed this before 2.6.36 was finalized. Removing the
> >> > define has broke user space compilation for anything that works on
> >> > file based capabilities. I can define it myself, but if the kernel
> >> > folks ever change the string, then we have more than just a compile
> >> > problem, we have runtime problems because I can no longer use the
> >> > correct string.
> >> >
> >> > So, what was the gain for breaking user space?
> >> >
> >> > -Steve
> >>
> >> Sorry I dropped the ball. Was expecting some kind of response to my
> >> question above, and then forgot about it.
> >>
> >> All of the 'security' xattrs were moved to fsmagic.h, including
> >> capability. Not only those that EVM protects, but others like
> >> XATTR_NAME_SMACKIPIN/OUT (based on Casey's request).
> >
> > If user space has to know the exact contents of a string in order to do
> > something that the kernel understands, then its part of a public API.
> > I've made my own define and released a new copy of libcap-ng. So, if the
> > contents of the string ever change, or becomes deprecated, you'll now
> > have user space apps using the old values no matter what.
>
> You're right Steve, it is ABI, we broke it, and we can fix it. What
> are you having to define and what are you including. What files did
> you used to get these defines from?

I did this:

#define XATTR_CAPS_SUFFIX "capability"
#define XATTR_SECURITY_PREFIX "security."
#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX

2 of them came from capability.h. The other was not public sometime in the past.

-Steve

2010-11-03 20:26:37

by Eric Paris

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Tue, Oct 12, 2010 at 9:40 AM, Mimi Zohar <[email protected]> wrote:
> On Tue, 2010-10-12 at 09:19 -0400, Steve Grubb wrote:

> Before making any changes to the kernel xattr.h, I want to understand
> the reason for two xattr.h files, one in /usr/include/linux/ and the
> other in /usr/include/xattr/. ?/usr/include/linux/xattr.h contains those
> elements not defined as __kernel__, while /usr/include/xattr/xattr.h
> contains that and other definitions. ?Will changing the kernel xattr.h
> version change both?

I don't know what /usr/include/xattr/ is but /usr/include/linux/ is
where the kernel-headers are supposed to end up. I'm guessing
/usr/include/xattr/ is the glibc function definitions? (my glibc
function definitions for xattrs are in /usr/include/sys/xattr.h)

I sent a patch to export them all. I didn't cc stable since it
clearly change more than it needs to, but we really should send
something to stable....

-Eric

2010-11-03 20:45:24

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH] security: move LSM xattrnames to xattr.h

On Wed, 2010-11-03 at 16:26 -0400, Eric Paris wrote:
> On Tue, Oct 12, 2010 at 9:40 AM, Mimi Zohar <[email protected]> wrote:
> > On Tue, 2010-10-12 at 09:19 -0400, Steve Grubb wrote:
>
> > Before making any changes to the kernel xattr.h, I want to understand
> > the reason for two xattr.h files, one in /usr/include/linux/ and the
> > other in /usr/include/xattr/. /usr/include/linux/xattr.h contains those
> > elements not defined as __kernel__, while /usr/include/xattr/xattr.h
> > contains that and other definitions. Will changing the kernel xattr.h
> > version change both?
>
> I don't know what /usr/include/xattr/ is but /usr/include/linux/ is
> where the kernel-headers are supposed to end up. I'm guessing
> /usr/include/xattr/ is the glibc function definitions? (my glibc
> function definitions for xattrs are in /usr/include/sys/xattr.h)
>
> I sent a patch to export them all. I didn't cc stable since it
> clearly change more than it needs to, but we really should send
> something to stable....
>
> -Eric

Thanks Eric. Don't know what happened, but /usr/include/xattr/xattr.h is
gone. /usr/include/sys/xattr.h contains the names of the user space
xattr tools, not the kernel version. I'll create the patch for stable.

thanks,

Mimi