LinuxLists.cc - [PATCH 1/1] security: introduce fs caps

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Quoting Alexey Dobriyan ([email protected]):
> On Wed, Nov 08, 2006 at 04:24:53PM -0600, Serge E. Hallyn wrote:
> > +struct vfs_cap_data_struct {
>
> "_struct" suffix is useless here: "struct" is already typed.

Good point.

> > + __u32 version;
> > + __u32 effective;
> > + __u32 permitted;
> > + __u32 inheritable;
> > +};
> > +
> > +static inline void convert_to_le(struct vfs_cap_data_struct *cap)
> > +{
> > + cap->version = le32_to_cpu(cap->version);
> > + cap->effective = le32_to_cpu(cap->effective);
> > + cap->permitted = le32_to_cpu(cap->permitted);
> > + cap->inheritable = le32_to_cpu(cap->inheritable);
> > +}
>
> This pretty much defeats sparse endian checking. You will get warnings
> regardless of whether u32 or le32 are used.

But I don't get any sparse warnings with make C=1. Am I doing something
wrong? Will it give warnings only on some architectures?

thanks,
-serge

2006-11-09 05:27:42

by Alexey Dobriyan

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Wed, Nov 08, 2006 at 05:52:03PM -0600, Serge E. Hallyn wrote:
> > > + __u32 version;
> > > + __u32 effective;
> > > + __u32 permitted;
> > > + __u32 inheritable;
> > > +};
> > > +
> > > +static inline void convert_to_le(struct vfs_cap_data_struct *cap)
> > > +{
> > > + cap->version = le32_to_cpu(cap->version);
> > > + cap->effective = le32_to_cpu(cap->effective);
> > > + cap->permitted = le32_to_cpu(cap->permitted);
> > > + cap->inheritable = le32_to_cpu(cap->inheritable);
> > > +}
> >
> > This pretty much defeats sparse endian checking. You will get warnings
> > regardless of whether u32 or le32 are used.
>
> But I don't get any sparse warnings with make C=1. Am I doing something
> wrong?

You need (temporarily) to use CF:

make C=1 CF=-D__CHECK_ENDIAN__ ...

2006-11-09 06:17:12

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

2006-11-09 06:20:53

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Sorry, I should have noticed and fixed this much sooner. This
patch is against the latest full fscaps patch which I'm replying
to.

From: Serge E. Hallyn <[email protected]>
Date: Thu, 9 Nov 2006 00:01:49 -0600
Subject: security: file caps: fix unused variable warnings

Address warnings of unused variables at cap_bprm_set_security
when file capabilities are disabled, and simultaneously clean
up the code a little, by pulling the new code into a helper
function.

Rename vfs_cap_data_struct to remove redundant '_struct'.

Signed-off-by: Serge E. Hallyn <[email protected]>
---
security/commoncap.c | 73 ++++++++++++++++++++++++++++----------------------
1 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 6f5e46c..4b50b4d 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -109,16 +109,17 @@ void cap_capset_set (struct task_struct
target->cap_permitted = *permitted;
}

+#ifdef CONFIG_SECURITY_FS_CAPABILITIES
#define XATTR_CAPS_SUFFIX "capability"
#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
-struct vfs_cap_data_struct {
+struct vfs_cap_data {
__u32 version;
__u32 effective;
__u32 permitted;
__u32 inheritable;
};

-static inline void convert_to_le(struct vfs_cap_data_struct *cap)
+static inline void convert_to_le(struct vfs_cap_data *cap)
{
cap->version = le32_to_cpu(cap->version);
cap->effective = le32_to_cpu(cap->effective);
@@ -126,7 +127,7 @@ static inline void convert_to_le(struct
cap->inheritable = le32_to_cpu(cap->inheritable);
}

-static int check_cap_sanity(struct vfs_cap_data_struct *cap)
+static int check_cap_sanity(struct vfs_cap_data *cap)
{
int i;

@@ -149,39 +150,14 @@ static int check_cap_sanity(struct vfs_c
return 0;
}

-int cap_bprm_set_security (struct linux_binprm *bprm)
+/* Locate any VFS capabilities: */
+static int set_file_caps(struct linux_binprm *bprm)
{
struct dentry *dentry;
ssize_t rc;
- struct vfs_cap_data_struct cap_struct;
+ struct vfs_cap_data cap_struct;
struct inode *inode;

- /* Copied from fs/exec.c:prepare_binprm. */
-
- cap_clear (bprm->cap_inheritable);
- cap_clear (bprm->cap_permitted);
- cap_clear (bprm->cap_effective);
-
- /* To support inheritance of root-permissions and suid-root
- * executables under compatibility mode, we raise all three
- * capability sets for the file.
- *
- * If only the real uid is 0, we only raise the inheritable
- * and permitted sets of the executable file.
- */
-
- if (!issecure (SECURE_NOROOT)) {
- if (bprm->e_uid == 0 || current->uid == 0) {
- cap_set_full (bprm->cap_inheritable);
- cap_set_full (bprm->cap_permitted);
- }
- if (bprm->e_uid == 0)
- cap_set_full (bprm->cap_effective);
- }
-
-#ifdef CONFIG_SECURITY_FS_CAPABILITIES
- /* Locate any VFS capabilities: */
-
dentry = dget(bprm->file->f_dentry);
inode = dentry->d_inode;
if (!inode->i_op || !inode->i_op->getxattr) {
@@ -216,9 +192,42 @@ #ifdef CONFIG_SECURITY_FS_CAPABILITIES
bprm->cap_permitted = cap_struct.permitted;
bprm->cap_inheritable = cap_struct.inheritable;

-#endif
return 0;
}
+#else
+static int set_file_caps(struct linux_binprm *bprm)
+{
+ return 0;
+}
+#endif
+
+int cap_bprm_set_security (struct linux_binprm *bprm)
+{
+ /* Copied from fs/exec.c:prepare_binprm. */
+
+ cap_clear (bprm->cap_inheritable);
+ cap_clear (bprm->cap_permitted);
+ cap_clear (bprm->cap_effective);
+
+ /* To support inheritance of root-permissions and suid-root
+ * executables under compatibility mode, we raise all three
+ * capability sets for the file.
+ *
+ * If only the real uid is 0, we only raise the inheritable
+ * and permitted sets of the executable file.
+ */
+
+ if (!issecure (SECURE_NOROOT)) {
+ if (bprm->e_uid == 0 || current->uid == 0) {
+ cap_set_full (bprm->cap_inheritable);
+ cap_set_full (bprm->cap_permitted);
+ }
+ if (bprm->e_uid == 0)
+ cap_set_full (bprm->cap_effective);
+ }
+
+ return set_file_caps(bprm);
+}

void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
{
--
1.4.1

2006-11-09 09:34:31

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Page http://www.friedhoff.org/fscaps.html updated ...
Kernel 2.6.18.2 updated ...
System keeps on humming ...
Is anyone else using/testing the patch? Please give feedback ...
Thanks ...

Chris

On Thu, 9 Nov 2006 00:10:21 -0600
"Serge E. Hallyn" <[email protected]> wrote:

> Sorry, I should have noticed and fixed this much sooner. This
> patch is against the latest full fscaps patch which I'm replying
> to.
>
> From: Serge E. Hallyn <[email protected]>
> Date: Thu, 9 Nov 2006 00:01:49 -0600
> Subject: security: file caps: fix unused variable warnings
>
> Address warnings of unused variables at cap_bprm_set_security
> when file capabilities are disabled, and simultaneously clean
> up the code a little, by pulling the new code into a helper
> function.
>
> Rename vfs_cap_data_struct to remove redundant '_struct'.
>
> Signed-off-by: Serge E. Hallyn <[email protected]>
> ---
> security/commoncap.c | 73 ++++++++++++++++++++++++++++----------------------
> 1 files changed, 41 insertions(+), 32 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6f5e46c..4b50b4d 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -109,16 +109,17 @@ void cap_capset_set (struct task_struct
> target->cap_permitted = *permitted;
> }
>
> +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> #define XATTR_CAPS_SUFFIX "capability"
> #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> -struct vfs_cap_data_struct {
> +struct vfs_cap_data {
> __u32 version;
> __u32 effective;
> __u32 permitted;
> __u32 inheritable;
> };
>
> -static inline void convert_to_le(struct vfs_cap_data_struct *cap)
> +static inline void convert_to_le(struct vfs_cap_data *cap)
> {
> cap->version = le32_to_cpu(cap->version);
> cap->effective = le32_to_cpu(cap->effective);
> @@ -126,7 +127,7 @@ static inline void convert_to_le(struct
> cap->inheritable = le32_to_cpu(cap->inheritable);
> }
>
> -static int check_cap_sanity(struct vfs_cap_data_struct *cap)
> +static int check_cap_sanity(struct vfs_cap_data *cap)
> {
> int i;
>
> @@ -149,39 +150,14 @@ static int check_cap_sanity(struct vfs_c
> return 0;
> }
>
> -int cap_bprm_set_security (struct linux_binprm *bprm)
> +/* Locate any VFS capabilities: */
> +static int set_file_caps(struct linux_binprm *bprm)
> {
> struct dentry *dentry;
> ssize_t rc;
> - struct vfs_cap_data_struct cap_struct;
> + struct vfs_cap_data cap_struct;
> struct inode *inode;
>
> - /* Copied from fs/exec.c:prepare_binprm. */
> -
> - cap_clear (bprm->cap_inheritable);
> - cap_clear (bprm->cap_permitted);
> - cap_clear (bprm->cap_effective);
> -
> - /* To support inheritance of root-permissions and suid-root
> - * executables under compatibility mode, we raise all three
> - * capability sets for the file.
> - *
> - * If only the real uid is 0, we only raise the inheritable
> - * and permitted sets of the executable file.
> - */
> -
> - if (!issecure (SECURE_NOROOT)) {
> - if (bprm->e_uid == 0 || current->uid == 0) {
> - cap_set_full (bprm->cap_inheritable);
> - cap_set_full (bprm->cap_permitted);
> - }
> - if (bprm->e_uid == 0)
> - cap_set_full (bprm->cap_effective);
> - }
> -
> -#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> - /* Locate any VFS capabilities: */
> -
> dentry = dget(bprm->file->f_dentry);
> inode = dentry->d_inode;
> if (!inode->i_op || !inode->i_op->getxattr) {
> @@ -216,9 +192,42 @@ #ifdef CONFIG_SECURITY_FS_CAPABILITIES
> bprm->cap_permitted = cap_struct.permitted;
> bprm->cap_inheritable = cap_struct.inheritable;
>
> -#endif
> return 0;
> }
> +#else
> +static int set_file_caps(struct linux_binprm *bprm)
> +{
> + return 0;
> +}
> +#endif
> +
> +int cap_bprm_set_security (struct linux_binprm *bprm)
> +{
> + /* Copied from fs/exec.c:prepare_binprm. */
> +
> + cap_clear (bprm->cap_inheritable);
> + cap_clear (bprm->cap_permitted);
> + cap_clear (bprm->cap_effective);
> +
> + /* To support inheritance of root-permissions and suid-root
> + * executables under compatibility mode, we raise all three
> + * capability sets for the file.
> + *
> + * If only the real uid is 0, we only raise the inheritable
> + * and permitted sets of the executable file.
> + */
> +
> + if (!issecure (SECURE_NOROOT)) {
> + if (bprm->e_uid == 0 || current->uid == 0) {
> + cap_set_full (bprm->cap_inheritable);
> + cap_set_full (bprm->cap_permitted);
> + }
> + if (bprm->e_uid == 0)
> + cap_set_full (bprm->cap_effective);
> + }
> +
> + return set_file_caps(bprm);
> +}
>
> void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
> {
> --
> 1.4.1
>

--------------------
Chris Friedhoff
[email protected]

2006-11-09 14:47:34

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Thu, Nov 09, 2006 at 10:33:49AM +0100, Chris Friedhoff wrote:
| Page http://www.friedhoff.org/fscaps.html updated ...
| Kernel 2.6.18.2 updated ...
| System keeps on humming ...
| Is anyone else using/testing the patch? Please give feedback ...
| Thanks ...
I am just starting to test it out. I'll let you know how it goes.
Thanks!
Bill

|
| Chris
|
|
| On Thu, 9 Nov 2006 00:10:21 -0600
| "Serge E. Hallyn" <[email protected]> wrote:
|
| > Sorry, I should have noticed and fixed this much sooner. This
| > patch is against the latest full fscaps patch which I'm replying
| > to.
| >
| > From: Serge E. Hallyn <[email protected]>
| > Date: Thu, 9 Nov 2006 00:01:49 -0600
| > Subject: security: file caps: fix unused variable warnings
| >
| > Address warnings of unused variables at cap_bprm_set_security
| > when file capabilities are disabled, and simultaneously clean
| > up the code a little, by pulling the new code into a helper
| > function.
| >
| > Rename vfs_cap_data_struct to remove redundant '_struct'.
| >
| > Signed-off-by: Serge E. Hallyn <[email protected]>
| > ---
| > security/commoncap.c | 73 ++++++++++++++++++++++++++++----------------------
| > 1 files changed, 41 insertions(+), 32 deletions(-)
| >
| > diff --git a/security/commoncap.c b/security/commoncap.c
| > index 6f5e46c..4b50b4d 100644
| > --- a/security/commoncap.c
| > +++ b/security/commoncap.c
| > @@ -109,16 +109,17 @@ void cap_capset_set (struct task_struct
| > target->cap_permitted = *permitted;
| > }
| >
| > +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
| > #define XATTR_CAPS_SUFFIX "capability"
| > #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
| > -struct vfs_cap_data_struct {
| > +struct vfs_cap_data {
| > __u32 version;
| > __u32 effective;
| > __u32 permitted;
| > __u32 inheritable;
| > };
| >
| > -static inline void convert_to_le(struct vfs_cap_data_struct *cap)
| > +static inline void convert_to_le(struct vfs_cap_data *cap)
| > {
| > cap->version = le32_to_cpu(cap->version);
| > cap->effective = le32_to_cpu(cap->effective);
| > @@ -126,7 +127,7 @@ static inline void convert_to_le(struct
| > cap->inheritable = le32_to_cpu(cap->inheritable);
| > }
| >
| > -static int check_cap_sanity(struct vfs_cap_data_struct *cap)
| > +static int check_cap_sanity(struct vfs_cap_data *cap)
| > {
| > int i;
| >
| > @@ -149,39 +150,14 @@ static int check_cap_sanity(struct vfs_c
| > return 0;
| > }
| >
| > -int cap_bprm_set_security (struct linux_binprm *bprm)
| > +/* Locate any VFS capabilities: */
| > +static int set_file_caps(struct linux_binprm *bprm)
| > {
| > struct dentry *dentry;
| > ssize_t rc;
| > - struct vfs_cap_data_struct cap_struct;
| > + struct vfs_cap_data cap_struct;
| > struct inode *inode;
| >
| > - /* Copied from fs/exec.c:prepare_binprm. */
| > -
| > - cap_clear (bprm->cap_inheritable);
| > - cap_clear (bprm->cap_permitted);
| > - cap_clear (bprm->cap_effective);
| > -
| > - /* To support inheritance of root-permissions and suid-root
| > - * executables under compatibility mode, we raise all three
| > - * capability sets for the file.
| > - *
| > - * If only the real uid is 0, we only raise the inheritable
| > - * and permitted sets of the executable file.
| > - */
| > -
| > - if (!issecure (SECURE_NOROOT)) {
| > - if (bprm->e_uid == 0 || current->uid == 0) {
| > - cap_set_full (bprm->cap_inheritable);
| > - cap_set_full (bprm->cap_permitted);
| > - }
| > - if (bprm->e_uid == 0)
| > - cap_set_full (bprm->cap_effective);
| > - }
| > -
| > -#ifdef CONFIG_SECURITY_FS_CAPABILITIES
| > - /* Locate any VFS capabilities: */
| > -
| > dentry = dget(bprm->file->f_dentry);
| > inode = dentry->d_inode;
| > if (!inode->i_op || !inode->i_op->getxattr) {
| > @@ -216,9 +192,42 @@ #ifdef CONFIG_SECURITY_FS_CAPABILITIES
| > bprm->cap_permitted = cap_struct.permitted;
| > bprm->cap_inheritable = cap_struct.inheritable;
| >
| > -#endif
| > return 0;
| > }
| > +#else
| > +static int set_file_caps(struct linux_binprm *bprm)
| > +{
| > + return 0;
| > +}
| > +#endif
| > +
| > +int cap_bprm_set_security (struct linux_binprm *bprm)
| > +{
| > + /* Copied from fs/exec.c:prepare_binprm. */
| > +
| > + cap_clear (bprm->cap_inheritable);
| > + cap_clear (bprm->cap_permitted);
| > + cap_clear (bprm->cap_effective);
| > +
| > + /* To support inheritance of root-permissions and suid-root
| > + * executables under compatibility mode, we raise all three
| > + * capability sets for the file.
| > + *
| > + * If only the real uid is 0, we only raise the inheritable
| > + * and permitted sets of the executable file.
| > + */
| > +
| > + if (!issecure (SECURE_NOROOT)) {
| > + if (bprm->e_uid == 0 || current->uid == 0) {
| > + cap_set_full (bprm->cap_inheritable);
| > + cap_set_full (bprm->cap_permitted);
| > + }
| > + if (bprm->e_uid == 0)
| > + cap_set_full (bprm->cap_effective);
| > + }
| > +
| > + return set_file_caps(bprm);
| > +}
| >
| > void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
| > {
| > --
| > 1.4.1
| >
|
|
| --------------------
| Chris Friedhoff
| [email protected]
| -
| To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
| the body of a message to [email protected]
| More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Bill O'Donnell
SGI
651.683.3079
[email protected]

2006-11-13 16:43:35

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Quoting Alexey Dobriyan ([email protected]):
> On Wed, Nov 08, 2006 at 05:52:03PM -0600, Serge E. Hallyn wrote:
> > > > + __u32 version;
> > > > + __u32 effective;
> > > > + __u32 permitted;
> > > > + __u32 inheritable;
> > > > +};
> > > > +
> > > > +static inline void convert_to_le(struct vfs_cap_data_struct *cap)
> > > > +{
> > > > + cap->version = le32_to_cpu(cap->version);
> > > > + cap->effective = le32_to_cpu(cap->effective);
> > > > + cap->permitted = le32_to_cpu(cap->permitted);
> > > > + cap->inheritable = le32_to_cpu(cap->inheritable);
> > > > +}
> > >
> > > This pretty much defeats sparse endian checking. You will get warnings
> > > regardless of whether u32 or le32 are used.
> >
> > But I don't get any sparse warnings with make C=1. Am I doing something
> > wrong?
>
> You need (temporarily) to use CF:
>
> make C=1 CF=-D__CHECK_ENDIAN__ ...

The following patch on top of the existing one eliminates the warning.
Does it appear consistent with what you were suggesting?

If it is ok, I'll resend the full patch.

Thank you for that help.

-serge

From: Serge E. Hallyn <[email protected]>
Subject: [PATCH 1/1] filecaps: fix endianness warnings

Fix endianness warnings as suggested by Alexey Dobriyan.

Signed-off-by: Serge E. Hallyn <[email protected]>
---
include/linux/capability.h | 21 +++++++++++++++++++++
security/commoncap.c | 39 ++++++++++++++++-----------------------
2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 76a6e87..c54b201 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -39,12 +39,33 @@ typedef struct __user_cap_data_struct {
__u32 permitted;
__u32 inheritable;
} __user *cap_user_data_t;
+
+
+#ifdef CONFIG_SECURITY_FS_CAPABILITIES
+#define XATTR_CAPS_SUFFIX "capability"
+#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
+struct vfs_cap_data_disk {
+ __le32 version;
+ __le32 effective;
+ __le32 permitted;
+ __le32 inheritable;
+};
+#endif

#ifdef __KERNEL__

#include <linux/spinlock.h>
#include <asm/current.h>

+#ifdef CONFIG_SECURITY_FS_CAPABILITIES
+struct vfs_cap_data {
+ __u32 version;
+ __u32 effective;
+ __u32 permitted;
+ __u32 inheritable;
+};
+#endif
+
/* #define STRICT_CAP_T_TYPECHECKS */

#ifdef STRICT_CAP_T_TYPECHECKS
diff --git a/security/commoncap.c b/security/commoncap.c
index 4b50b4d..c1ef43f 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -110,21 +110,13 @@ void cap_capset_set (struct task_struct
}

#ifdef CONFIG_SECURITY_FS_CAPABILITIES
-#define XATTR_CAPS_SUFFIX "capability"
-#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
-struct vfs_cap_data {
- __u32 version;
- __u32 effective;
- __u32 permitted;
- __u32 inheritable;
-};
-
-static inline void convert_to_le(struct vfs_cap_data *cap)
+static inline void cap_from_disk(struct vfs_cap_data_disk *dcap,
+ struct vfs_cap_data *cap)
{
- cap->version = le32_to_cpu(cap->version);
- cap->effective = le32_to_cpu(cap->effective);
- cap->permitted = le32_to_cpu(cap->permitted);
- cap->inheritable = le32_to_cpu(cap->inheritable);
+ cap->version = le32_to_cpu(dcap->version);
+ cap->effective = le32_to_cpu(dcap->effective);
+ cap->permitted = le32_to_cpu(dcap->permitted);
+ cap->inheritable = le32_to_cpu(dcap->inheritable);
}

static int check_cap_sanity(struct vfs_cap_data *cap)
@@ -155,7 +147,8 @@ static int set_file_caps(struct linux_bi
{
struct dentry *dentry;
ssize_t rc;
- struct vfs_cap_data cap_struct;
+ struct vfs_cap_data_disk dcaps;
+ struct vfs_cap_data caps;
struct inode *inode;

dentry = dget(bprm->file->f_dentry);
@@ -165,8 +158,8 @@ static int set_file_caps(struct linux_bi
return 0;
}

- rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &cap_struct,
- sizeof(cap_struct));
+ rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &dcaps,
+ sizeof(dcaps));
dput(dentry);

if (rc == -ENODATA)
@@ -178,19 +171,19 @@ static int set_file_caps(struct linux_bi
return rc;
}

- if (rc != sizeof(cap_struct)) {
+ if (rc != sizeof(dcaps)) {
printk(KERN_NOTICE "%s: got wrong size for getxattr (%d)\n",
__FUNCTION__, rc);
return -EPERM;
}

- convert_to_le(&cap_struct);
- if (check_cap_sanity(&cap_struct))
+ cap_from_disk(&dcaps, &caps);
+ if (check_cap_sanity(&caps))
return -EPERM;

- bprm->cap_effective = cap_struct.effective;
- bprm->cap_permitted = cap_struct.permitted;
- bprm->cap_inheritable = cap_struct.inheritable;
+ bprm->cap_effective = caps.effective;
+ bprm->cap_permitted = caps.permitted;
+ bprm->cap_inheritable = caps.inheritable;

return 0;
}
--
1.4.1

2006-11-13 21:04:55

by Alexey Dobriyan

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Mon, Nov 13, 2006 at 10:43:26AM -0600, Serge E. Hallyn wrote:
> Quoting Alexey Dobriyan ([email protected]):
> > On Wed, Nov 08, 2006 at 05:52:03PM -0600, Serge E. Hallyn wrote:
> > > > > + __u32 version;
> > > > > + __u32 effective;
> > > > > + __u32 permitted;
> > > > > + __u32 inheritable;
> > > > > +};
> > > > > +
> > > > > +static inline void convert_to_le(struct vfs_cap_data_struct *cap)
> > > > > +{
> > > > > + cap->version = le32_to_cpu(cap->version);
> > > > > + cap->effective = le32_to_cpu(cap->effective);
> > > > > + cap->permitted = le32_to_cpu(cap->permitted);
> > > > > + cap->inheritable = le32_to_cpu(cap->inheritable);
> > > > > +}
> > > >
> > > > This pretty much defeats sparse endian checking. You will get warnings
> > > > regardless of whether u32 or le32 are used.
> > >
> > > But I don't get any sparse warnings with make C=1. Am I doing something
> > > wrong?
> >
> > You need (temporarily) to use CF:
> >
> > make C=1 CF=-D__CHECK_ENDIAN__ ...
>
> The following patch on top of the existing one eliminates the warning.
> Does it appear consistent with what you were suggesting?

> If it is ok, I'll resend the full patch.

Yes, that's it, modulo:

> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> +#ifdef CONFIG_SECURITY_FS_CAPABILITIES

This is exportable header, so no CONFIG_*

> +#define XATTR_CAPS_SUFFIX "capability"
> +#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> +struct vfs_cap_data_disk {
> + __le32 version;
> + __le32 effective;
> + __le32 permitted;
> + __le32 inheritable;
> +};
> +#endif
>
> #ifdef __KERNEL__
>
> #include <linux/spinlock.h>
> #include <asm/current.h>
>
> +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> +struct vfs_cap_data {
> + __u32 version;
> + __u32 effective;
> + __u32 permitted;
> + __u32 inheritable;
> +};

Now you're in kernel, so you can happily use u32 without undescores.

> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -155,7 +147,8 @@ static int set_file_caps(struct linux_bi
> {
> struct dentry *dentry;
> ssize_t rc;

> @@ -178,19 +171,19 @@ static int set_file_caps(struct linux_bi
> return rc;
> }
>
> - if (rc != sizeof(cap_struct)) {
> + if (rc != sizeof(dcaps)) {
> printk(KERN_NOTICE "%s: got wrong size for getxattr (%d)\n",
> __FUNCTION__, rc);

rc is ssize_t, so "%zd".

2006-11-13 21:54:05

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Thu, Nov 09, 2006 at 10:33:49AM +0100, Chris Friedhoff wrote:
| Page http://www.friedhoff.org/fscaps.html updated ...
| Kernel 2.6.18.2 updated ...
| System keeps on humming ...
| Is anyone else using/testing the patch? Please give feedback ...

Most likely a cockpit error, but I'm having trouble when I give the
capability to ping (using the userexample from your fscaps page):

$ uname -a
Linux certify 2.6.19-rc3 #3 SMP PREEMPT Mon Nov 13 14:40:54 CST 2006 ia64

$ sudo chmod 711 /bin/ping
$ ping -c 1 localhost
ping: icmp open socket: Operation not permitted

$ sudo setfcaps cap_net_raw=ep /bin/ping
/bin/ping: Function not implemented (errno=38)

Any help is appreciated.

Bill

---
Bill O'Donnell
SGI
651.683.3079
[email protected]

2006-11-14 03:04:46

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Quoting Alexey Dobriyan ([email protected]):
> On Mon, Nov 13, 2006 at 10:43:26AM -0600, Serge E. Hallyn wrote:
> > Quoting Alexey Dobriyan ([email protected]):
> > > On Wed, Nov 08, 2006 at 05:52:03PM -0600, Serge E. Hallyn wrote:
> > > > > > + __u32 version;
> > > > > > + __u32 effective;
> > > > > > + __u32 permitted;
> > > > > > + __u32 inheritable;
> > > > > > +};
> > > > > > +
> > > > > > +static inline void convert_to_le(struct vfs_cap_data_struct *cap)
> > > > > > +{
> > > > > > + cap->version = le32_to_cpu(cap->version);
> > > > > > + cap->effective = le32_to_cpu(cap->effective);
> > > > > > + cap->permitted = le32_to_cpu(cap->permitted);
> > > > > > + cap->inheritable = le32_to_cpu(cap->inheritable);
> > > > > > +}
> > > > >
> > > > > This pretty much defeats sparse endian checking. You will get warnings
> > > > > regardless of whether u32 or le32 are used.
> > > >
> > > > But I don't get any sparse warnings with make C=1. Am I doing something
> > > > wrong?
> > >
> > > You need (temporarily) to use CF:
> > >
> > > make C=1 CF=-D__CHECK_ENDIAN__ ...
> >
> > The following patch on top of the existing one eliminates the warning.
> > Does it appear consistent with what you were suggesting?
>
> > If it is ok, I'll resend the full patch.
>
> Yes, that's it, modulo:

Thanks, comments integrated with the exception of:

> > --- a/include/linux/capability.h
> > +++ b/include/linux/capability.h
> > +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
>
> This is exportable header, so no CONFIG_*
>
> > +#define XATTR_CAPS_SUFFIX "capability"
> > +#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> > +struct vfs_cap_data_disk {
> > + __le32 version;
> > + __le32 effective;
> > + __le32 permitted;
> > + __le32 inheritable;
> > +};
> > +#endif
> >
> > #ifdef __KERNEL__
> >
> > #include <linux/spinlock.h>
> > #include <asm/current.h>
> >
> > +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> > +struct vfs_cap_data {
> > + __u32 version;
> > + __u32 effective;
> > + __u32 permitted;
> > + __u32 inheritable;
> > +};
>
> Now you're in kernel, so you can happily use u32 without undescores.

The rest of the file already uses __u32, so for consistency I'd
rather stick with __u32, unless there's a reason why it's really
discouraged.

Will send new patch in just a bit.

thanks,
-serge

> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
> > @@ -155,7 +147,8 @@ static int set_file_caps(struct linux_bi
> > {
> > struct dentry *dentry;
> > ssize_t rc;
>
> > @@ -178,19 +171,19 @@ static int set_file_caps(struct linux_bi
> > return rc;
> > }
> >
> > - if (rc != sizeof(cap_struct)) {
> > + if (rc != sizeof(dcaps)) {
> > printk(KERN_NOTICE "%s: got wrong size for getxattr (%d)\n",
> > __FUNCTION__, rc);
>
> rc is ssize_t, so "%zd".

2006-11-14 05:25:37

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Quoting Bill O'Donnell ([email protected]):
> On Thu, Nov 09, 2006 at 10:33:49AM +0100, Chris Friedhoff wrote:
> | Page http://www.friedhoff.org/fscaps.html updated ...
> | Kernel 2.6.18.2 updated ...
> | System keeps on humming ...
> | Is anyone else using/testing the patch? Please give feedback ...
>
> Most likely a cockpit error, but I'm having trouble when I give the
> capability to ping (using the userexample from your fscaps page):
>
> $ uname -a
> Linux certify 2.6.19-rc3 #3 SMP PREEMPT Mon Nov 13 14:40:54 CST 2006 ia64
>
> $ sudo chmod 711 /bin/ping
> $ ping -c 1 localhost
> ping: icmp open socket: Operation not permitted
>
> $ sudo setfcaps cap_net_raw=ep /bin/ping
> /bin/ping: Function not implemented (errno=38)
>
> Any help is appreciated.

Hmm, two things which come to mind are (a) do you have extended
attributes compiled into your kernel and (b) is sudo properly set
up.

But for (a) to be the case, you should be getting EOPNOTZSPUP (98),
not ENOSYS (38).

Could you send me a copy of your .config, tell me which filesystem
you are using, and send the /tmp/straceout after doing

strace -o/tmp/straceout -f setfcaps cap_net_raw=ep /bin/ping

as root?

thanks,
-serge

2006-11-14 13:53:00

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Attachments:

(No filename) (1.46 kB)
.config.certify (28.13 kB)
straceout (2.59 kB)
Download all attachments

2006-11-14 15:23:13

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Quoting Bill O'Donnell ([email protected]):
> 8102 execve("/sbin/setfcaps", ["setfcaps", "cap_net_raw=ep", "/bin/ping"], [/* 67 vars */]) = 0
> 8102 brk(0) = 0x6000000000004000
> 8102 uname({sys="Linux", node="certify", ...}) = 0
> 8102 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory)
> 8102 open("/etc/ld.so.cache", O_RDONLY) = 3
> 8102 fstat(3, {st_mode=S_IFREG|0644, st_size=111415, ...}) = 0
> 8102 mmap(NULL, 111415, PROT_READ, MAP_PRIVATE, 3, 0) = 0x200000000004c000
> 8102 close(3) = 0
> 8102 open("/lib/libcap.so.1", O_RDONLY) = 3
> 8102 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0002\0\1\0\0\0\340\25"..., 832) = 832
> 8102 fstat(3, {st_mode=S_IFREG|0755, st_size=22672, ...}) = 0
> 8102 mmap(NULL, 85800, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x2000000000068000
> 8102 madvise(0x2000000000068000, 85800, MADV_SEQUENTIAL|0x1) = 0
> 8102 mprotect(0x2000000000070000, 49152, PROT_NONE) = 0
> 8102 mmap(0x200000000007c000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x4000) = 0x200000000007c000
> 8102 close(3) = 0
> 8102 open("/lib/libc.so.6.1", O_RDONLY) = 3
> 8102 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0002\0\1\0\0\0\3609\2"..., 832) = 832
> 8102 fstat(3, {st_mode=S_IFREG|0755, st_size=2590313, ...}) = 0
> 8102 mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2000000000080000
> 8102 mmap(NULL, 2416624, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x2000000000084000
> 8102 madvise(0x2000000000084000, 2416624, MADV_SEQUENTIAL|0x1) = 0
> 8102 mprotect(0x20000000002bc000, 49152, PROT_NONE) = 0
> 8102 mmap(0x20000000002c8000, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x234000) = 0x20000000002c8000
> 8102 mmap(0x20000000002d0000, 8176, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x20000000002d0000
> 8102 close(3) = 0
> 8102 mmap(NULL, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x20000000002d4000
> 8102 mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x20000000002dc000
> 8102 munmap(0x200000000004c000, 111415) = 0
> 8102 brk(0) = 0x6000000000004000
> 8102 brk(0x6000000000028000) = 0x6000000000028000
> 8102 capget(0x19980330, 0, {0, 0, 0}) = -1 EINVAL (Invalid argument)

I don't see why this capget is returning -EINVAL. In fact I don't see
why it happens at all - cap_inode_setxattr would check
capable(CAP_SYS_ADMIN), but setxattr hasn't been called yet. Looking at
both libcap and setfcaps.c, I don't see where the capget comes from.

As for the -EINVAL, kernel/capability.c:sys_capget() returns -EINVAL if
the _LINUX_CAPABILITY_VERSION is wrong - you have 0x19980330 which is
correct - if pid < 0 - but you send in 0 - or if security_capget
returns -EINVAL, which cap_capget (and dummy_capget) don't do.

Kaigai, do you have any ideas?

thanks,
-serge

2006-11-14 17:28:52

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Attached the trace of
$ su -c "strace -o /tmp/stracesetfcapsout -f setfcaps
cap_net_raw=ep /bin/ping "
Here its working.
>From where are the setfcaps/getfcaps tools? Bill, have you compiled
them or are they from a package?

> $ uname -a
> Linux certify 2.6.19-rc3 #3 SMP PREEMPT Mon Nov 13 14:40:54 CST 2006
> ia64
Its an 64 bit system, right? Which distro are you using?

Chris

On Tue, 14 Nov 2006 09:23:07 -0600
"Serge E. Hallyn" <[email protected]> wrote:

> Quoting Bill O'Donnell ([email protected]):
> > 8102 execve("/sbin/setfcaps", ["setfcaps", "cap_net_raw=ep", "/bin/ping"], [/* 67 vars */]) = 0
> > 8102 brk(0) = 0x6000000000004000
> > 8102 uname({sys="Linux", node="certify", ...}) = 0
> > 8102 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory)
> > 8102 open("/etc/ld.so.cache", O_RDONLY) = 3
> > 8102 fstat(3, {st_mode=S_IFREG|0644, st_size=111415, ...}) = 0
> > 8102 mmap(NULL, 111415, PROT_READ, MAP_PRIVATE, 3, 0) = 0x200000000004c000
> > 8102 close(3) = 0
> > 8102 open("/lib/libcap.so.1", O_RDONLY) = 3
> > 8102 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0002\0\1\0\0\0\340\25"..., 832) = 832
> > 8102 fstat(3, {st_mode=S_IFREG|0755, st_size=22672, ...}) = 0
> > 8102 mmap(NULL, 85800, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x2000000000068000
> > 8102 madvise(0x2000000000068000, 85800, MADV_SEQUENTIAL|0x1) = 0
> > 8102 mprotect(0x2000000000070000, 49152, PROT_NONE) = 0
> > 8102 mmap(0x200000000007c000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x4000) = 0x200000000007c000
> > 8102 close(3) = 0
> > 8102 open("/lib/libc.so.6.1", O_RDONLY) = 3
> > 8102 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0002\0\1\0\0\0\3609\2"..., 832) = 832
> > 8102 fstat(3, {st_mode=S_IFREG|0755, st_size=2590313, ...}) = 0
> > 8102 mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2000000000080000
> > 8102 mmap(NULL, 2416624, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x2000000000084000
> > 8102 madvise(0x2000000000084000, 2416624, MADV_SEQUENTIAL|0x1) = 0
> > 8102 mprotect(0x20000000002bc000, 49152, PROT_NONE) = 0
> > 8102 mmap(0x20000000002c8000, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x234000) = 0x20000000002c8000
> > 8102 mmap(0x20000000002d0000, 8176, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x20000000002d0000
> > 8102 close(3) = 0
> > 8102 mmap(NULL, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x20000000002d4000
> > 8102 mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x20000000002dc000
> > 8102 munmap(0x200000000004c000, 111415) = 0
> > 8102 brk(0) = 0x6000000000004000
> > 8102 brk(0x6000000000028000) = 0x6000000000028000
> > 8102 capget(0x19980330, 0, {0, 0, 0}) = -1 EINVAL (Invalid argument)
>
> I don't see why this capget is returning -EINVAL. In fact I don't see
> why it happens at all - cap_inode_setxattr would check
> capable(CAP_SYS_ADMIN), but setxattr hasn't been called yet. Looking at
> both libcap and setfcaps.c, I don't see where the capget comes from.
>
> As for the -EINVAL, kernel/capability.c:sys_capget() returns -EINVAL if
> the _LINUX_CAPABILITY_VERSION is wrong - you have 0x19980330 which is
> correct - if pid < 0 - but you send in 0 - or if security_capget
> returns -EINVAL, which cap_capget (and dummy_capget) don't do.
>
> Kaigai, do you have any ideas?
>
> thanks,
> -serge

--------------------
Chris Friedhoff
[email protected]

Attachments:

stracesetfcapsout.gz (792.00 B)

2006-11-14 17:39:45

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Tue, Nov 14, 2006 at 06:28:18PM +0100, Chris Friedhoff wrote:
| Attached the trace of
| $ su -c "strace -o /tmp/stracesetfcapsout -f setfcaps
| cap_net_raw=ep /bin/ping "
| Here its working.
| From where are the setfcaps/getfcaps tools? Bill, have you compiled
| them or are they from a package?

I compiled and installed them to /sbin (Kagai's userspace libcap tools),
using his Makefile.

|
| > $ uname -a
| > Linux certify 2.6.19-rc3 #3 SMP PREEMPT Mon Nov 13 14:40:54 CST 2006
| > ia64
| Its an 64 bit system, right? Which distro are you using?

Yes. Its an Itanium based SGI Altix, with a SuSE SLES-10 distro.

Bill

|
|
| Chris
|
|
| On Tue, 14 Nov 2006 09:23:07 -0600
| "Serge E. Hallyn" <[email protected]> wrote:
|
| > Quoting Bill O'Donnell ([email protected]):
| > > 8102 execve("/sbin/setfcaps", ["setfcaps", "cap_net_raw=ep", "/bin/ping"], [/* 67 vars */]) = 0
| > > 8102 brk(0) = 0x6000000000004000
| > > 8102 uname({sys="Linux", node="certify", ...}) = 0
| > > 8102 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory)
| > > 8102 open("/etc/ld.so.cache", O_RDONLY) = 3
| > > 8102 fstat(3, {st_mode=S_IFREG|0644, st_size=111415, ...}) = 0
| > > 8102 mmap(NULL, 111415, PROT_READ, MAP_PRIVATE, 3, 0) = 0x200000000004c000
| > > 8102 close(3) = 0
| > > 8102 open("/lib/libcap.so.1", O_RDONLY) = 3
| > > 8102 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0002\0\1\0\0\0\340\25"..., 832) = 832
| > > 8102 fstat(3, {st_mode=S_IFREG|0755, st_size=22672, ...}) = 0
| > > 8102 mmap(NULL, 85800, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x2000000000068000
| > > 8102 madvise(0x2000000000068000, 85800, MADV_SEQUENTIAL|0x1) = 0
| > > 8102 mprotect(0x2000000000070000, 49152, PROT_NONE) = 0
| > > 8102 mmap(0x200000000007c000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x4000) = 0x200000000007c000
| > > 8102 close(3) = 0
| > > 8102 open("/lib/libc.so.6.1", O_RDONLY) = 3
| > > 8102 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0002\0\1\0\0\0\3609\2"..., 832) = 832
| > > 8102 fstat(3, {st_mode=S_IFREG|0755, st_size=2590313, ...}) = 0
| > > 8102 mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2000000000080000
| > > 8102 mmap(NULL, 2416624, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x2000000000084000
| > > 8102 madvise(0x2000000000084000, 2416624, MADV_SEQUENTIAL|0x1) = 0
| > > 8102 mprotect(0x20000000002bc000, 49152, PROT_NONE) = 0
| > > 8102 mmap(0x20000000002c8000, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x234000) = 0x20000000002c8000
| > > 8102 mmap(0x20000000002d0000, 8176, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x20000000002d0000
| > > 8102 close(3) = 0
| > > 8102 mmap(NULL, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x20000000002d4000
| > > 8102 mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x20000000002dc000
| > > 8102 munmap(0x200000000004c000, 111415) = 0
| > > 8102 brk(0) = 0x6000000000004000
| > > 8102 brk(0x6000000000028000) = 0x6000000000028000
| > > 8102 capget(0x19980330, 0, {0, 0, 0}) = -1 EINVAL (Invalid argument)
| >
| > I don't see why this capget is returning -EINVAL. In fact I don't see
| > why it happens at all - cap_inode_setxattr would check
| > capable(CAP_SYS_ADMIN), but setxattr hasn't been called yet. Looking at
| > both libcap and setfcaps.c, I don't see where the capget comes from.
| >
| > As for the -EINVAL, kernel/capability.c:sys_capget() returns -EINVAL if
| > the _LINUX_CAPABILITY_VERSION is wrong - you have 0x19980330 which is
| > correct - if pid < 0 - but you send in 0 - or if security_capget
| > returns -EINVAL, which cap_capget (and dummy_capget) don't do.
| >
| > Kaigai, do you have any ideas?
| >
| > thanks,
| > -serge
|
|
| --------------------
| Chris Friedhoff
| [email protected]

--
Bill O'Donnell
SGI
651.683.3079
[email protected]

2006-11-15 12:12:25

by Kohei KaiGai

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

Hi, Serge.

Serge E. Hallyn wrote:
> Quoting Bill O'Donnell ([email protected]):
>> 8102 execve("/sbin/setfcaps", ["setfcaps", "cap_net_raw=ep", "/bin/ping"], [/* 67 vars */]) = 0
- snip -
>> 8102 capget(0x19980330, 0, {0, 0, 0}) = -1 EINVAL (Invalid argument)
>
> I don't see why this capget is returning -EINVAL. In fact I don't see
> why it happens at all - cap_inode_setxattr would check
> capable(CAP_SYS_ADMIN), but setxattr hasn't been called yet. Looking at
> both libcap and setfcaps.c, I don't see where the capget comes from.
>
> As for the -EINVAL, kernel/capability.c:sys_capget() returns -EINVAL if
> the _LINUX_CAPABILITY_VERSION is wrong - you have 0x19980330 which is
> correct - if pid < 0 - but you send in 0 - or if security_capget
> returns -EINVAL, which cap_capget (and dummy_capget) don't do.
>
> Kaigai, do you have any ideas?

Bill said that he uses SLES10/ia64, so the version of libcap is different
from Fedora Core's one. 'libcap-1.92-499.4.src.rpm' is bandled.

Then, I found a strange code in libcap-1.92-499.4.src.rpm.

The setfcaps calls cap_from_text() which is defined in libcap to parse
the command line argument. It has the following function call chains:

cap_from_text()
-> cap_init()
-> _libcap_establish_api()

---- the definition of _libcap_establish_api() ----
void _libcap_establish_api(void)
{
struct __user_cap_header_struct ch;
struct __user_cap_data_struct cs;

if (_libcap_kernel_version) { <-- _libcap_kernel_version is 0 initially.
_cap_debug("already identified kernal api 0x%.8x",
_libcap_kernel_version);
return;
}

memset(&ch, 0, sizeof(ch));
memset(&cs, 0, sizeof(cs));

(void) capget(&ch, &cs); <-- (?)

switch (ch.version) {

case 0x19980330:
_libcap_kernel_version = 0x19980330;
_libcap_kernel_features = CAP_FEATURE_PROC;
break;

case 0x19990414:
_libcap_kernel_version = 0x19990414;
_libcap_kernel_features = CAP_FEATURE_PROC|CAP_FEATURE_FILE;
break;

default:
_libcap_kernel_version = 0x00000000;
_libcap_kernel_features = 0x00000000;
}

_cap_debug("version: %x, features: %x\n",
_libcap_kernel_version, _libcap_kernel_features);
}
---------------------------------------------------

capget() is called from _libcap_establish_api() with full-zeroed
__user_cap_header_struct object at first time.
The result of this, sys_capget() in kernel will return -EINVAL.
(Why did strace say the first argument is 0x19980330?)

Probably, Bill didn't update libcap.so.

But I can't recommend Bill to update libcap immediately.
As Hawk Xu said, it may cause a serious problem on the distro
except Fedora Core 6. :(

I have to recommend to use 'fscaps-1.0-kg.i386.rpm' now.
It includes the implementation of interaction between application and xattr.
(Of couse, it's one of the features which should be provided by libcap.)

Thanks,
--
Open Source Software Promotion Center, NEC
KaiGai Kohei <[email protected]>

2006-11-15 17:04:10

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Wed, Nov 15, 2006 at 09:08:55PM +0900, KaiGai Kohei wrote:
| Serge E. Hallyn wrote:
| >Quoting Bill O'Donnell ([email protected]):
| >>8102 execve("/sbin/setfcaps", ["setfcaps", "cap_net_raw=ep",
| >>"/bin/ping"], [/* 67 vars */]) = 0
| - snip -
| >>8102 capget(0x19980330, 0, {0, 0, 0}) = -1 EINVAL (Invalid argument)
| >
| >I don't see why this capget is returning -EINVAL. In fact I don't see
| >why it happens at all - cap_inode_setxattr would check
| >capable(CAP_SYS_ADMIN), but setxattr hasn't been called yet. Looking at
| >both libcap and setfcaps.c, I don't see where the capget comes from.
| >
| >As for the -EINVAL, kernel/capability.c:sys_capget() returns -EINVAL if
| >the _LINUX_CAPABILITY_VERSION is wrong - you have 0x19980330 which is
| >correct - if pid < 0 - but you send in 0 - or if security_capget
| >returns -EINVAL, which cap_capget (and dummy_capget) don't do.
| >
| >Kaigai, do you have any ideas?
|
| Bill said that he uses SLES10/ia64, so the version of libcap is different
| from Fedora Core's one. 'libcap-1.92-499.4.src.rpm' is bandled.
|
| Then, I found a strange code in libcap-1.92-499.4.src.rpm.
|
| The setfcaps calls cap_from_text() which is defined in libcap to parse
| the command line argument. It has the following function call chains:
|
| cap_from_text()
| -> cap_init()
| -> _libcap_establish_api()

- snip -

| capget() is called from _libcap_establish_api() with full-zeroed
| __user_cap_header_struct object at first time.
| The result of this, sys_capget() in kernel will return -EINVAL.
| (Why did strace say the first argument is 0x19980330?)
|
| Probably, Bill didn't update libcap.so.
No, I didn't...
certify:~/libcap-1.10/progs # ls -altr /lib/libcap*
-rwxr-xr-x 1 root root 22672 2006-06-16 09:56 /lib/libcap.so.1.92
-rw-r--r-- 1 root root 53363 2006-11-13 16:04 /lib/libcap.so.1.10
lrwxrwxrwx 1 root root 14 2006-11-13 16:04 /lib/libcap.so.1 ->libcap.so.1.92
lrwxrwxrwx 1 root root 11 2006-11-13 16:04 /lib/libcap.so -> libcap.so.1

|
| But I can't recommend Bill to update libcap immediately.
| As Hawk Xu said, it may cause a serious problem on the distro
| except Fedora Core 6. :(

What version of libcap is on FC6?

|
| I have to recommend to use 'fscaps-1.0-kg.i386.rpm' now.
| It includes the implementation of interaction between application and xattr.
| (Of couse, it's one of the features which should be provided by libcap.)

But that won't work on ia64 will it?

Thanks,
Bill

--
Bill O'Donnell
SGI
651.683.3079
[email protected]

2006-11-15 21:49:44

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

On Wed, 15 Nov 2006 11:06:34 -0600
"Bill O'Donnell" <[email protected]> wrote:

- snip -
> | Probably, Bill didn't update libcap.so.
> No, I didn't...
> certify:~/libcap-1.10/progs # ls -altr /lib/libcap*
> -rwxr-xr-x 1 root root 22672 2006-06-16 09:56 /lib/libcap.so.1.92
> -rw-r--r-- 1 root root 53363 2006-11-13 16:04 /lib/libcap.so.1.10
> lrwxrwxrwx 1 root root 14 2006-11-13 16:04 /lib/libcap.so.1 ->libcap.so.1.92
> lrwxrwxrwx 1 root root 11 2006-11-13 16:04 /lib/libcap.so -> libcap.so.1
>

Why is SLES10 using libcap-1.92?
(googling brought this page:
http://www.me.kernel.org/pub/linux/libs/security/linux-privs/old/kernel-2.3/)

> |
> | But I can't recommend Bill to update libcap immediately.
> | As Hawk Xu said, it may cause a serious problem on the distro
> | except Fedora Core 6. :(
>
> What version of libcap is on FC6?
>

Are there newer libcap versions then libcap-1.10 available?
(http://ftp.kernel.org/pub/linux/libs/security/linux-privs/kernel-2.4/)

> |
> | I have to recommend to use 'fscaps-1.0-kg.i386.rpm' now.
> | It includes the implementation of interaction between application and xattr.
> | (Of couse, it's one of the features which should be provided by libcap.)
>
> But that won't work on ia64 will it?

Kaigai also provides a srpm package to compile.
VFS Capability Support -> fscaps version 1.0 [SRPM]
(http://www.kaigai.gr.jp/pub/fscaps-1.0-kg.src.rpm)

Chris

--------------------
Chris Friedhoff
[email protected]

2006-11-16 14:45:29

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

2006-11-17 18:38:26

[permalink] [raw]

Subject: Re: [PATCH 1/1] security: introduce fs caps

> Good quesion. My gentoo ia32 machine uses libcap.so.1.10. Probably a FAQ,
> but is 1.92 actually older than 1.10?
looking at these locations, yes
http://www.me.kernel.org/pub/linux/libs/security/linux-privs/old/kernel-2.3/
http://ftp.kernel.org/pub/linux/libs/security/linux-privs/kernel-2.4/

> | Kaigai also provides a srpm package to compile.
> | VFS Capability Support -> fscaps version 1.0 [SRPM]
> | (http://www.kaigai.gr.jp/pub/fscaps-1.0-kg.src.rpm)
>
> I'll try that next.

I installed sles10 in qemu on my pentium-m laptop, compiled a kernel
with fscap support and installed fscaps-1.0-kg.i386.rpm and got "Invalid
argument (errno=22)" error.
trace output is attached

# id
uid=0(root) gid=0(root) groups=0(root)
# uname -a
Linux sles10 2.6.19-rc3-fscaps-a #2 Fri Nov 17 09:17:40 CET 2006 i686
i686 i386 GNU/Linux
# rpm -q fscaps
fscaps-1.0-kg
# zgrep -i capa /proc/config.gz
CONFIG_SECURITY_FS_CAPABILITIES=y
# setfcaps cap_net_raw=ep /bin/ping
/bin/ping: Invalid argument (errno=22)
# setfcaps cap_sys_module=ep /sbin/modprobe
/sbin/modprobe: Invalid argument (errno=22)
# ls -l /lib/libcap.so*
lrwxrwxrwx 1 root root 11 Nov 17 09:51 /lib/libcap.so -> libcap.so.1
lrwxrwxrwx 1 root root 14 Nov 16 11:57 /lib/libcap.so.1 ->
libcap.so.1.92 -rwxr-xr-x 1 root root 10456 Jun 16
15:14 /lib/libcap.so.1.92

--------------------
Chris Friedhoff
[email protected]

Attachments:

strace-ping-sles10.gz (929.00 B)

2006-11-17 19:12:32