2007-02-23 05:19:15

by Serge E. Hallyn

[permalink] [raw]
Subject: [PATCH 2/2] file caps: accomodate future 64-bit caps

Here is another attempt. This format is compatible with
KaiGai's current tools.

Tested on s390 with 32 and 64-bit caps stored in the xattrs.

-serge


From: "Serge E. Hallyn" <[email protected]>
Subject: [PATCH 2/2] file caps: accomodate future 64-bit caps

As the capability set changes and distributions start tagging
binaries with capabilities, we would like for running an older
kernel to not necessarily make those binaries unusable.

(1. Rename CONFIG_SECURITY_FS_CAPABILITIES to
CONFIG_SECURITY_FILE_CAPABILITIES)
2. Introduce CONFIG_SECURITY_FILE_CAPABILITIES_STRICTXATTR
which, when set, prevents loading binaries with capabilities
set which the kernel doesn't know about. When not set,
such capabilities run, ignoring the unknown caps.
3. To accomodate 64-bit caps, specify that capabilities are
stored as
u32 version; u32 eff0; u32 perm0; u32 inh0;
u32 eff1; u32 perm1; u32 inh1; (etc)

Signed-off-by: Serge E. Hallyn <[email protected]>

---

include/linux/capability.h | 23 ++++++
security/Kconfig | 12 +++
security/commoncap.c | 157 ++++++++++++++++++++++++++++----------------
3 files changed, 131 insertions(+), 61 deletions(-)

987fe7fcd60aaea6aaa86e6eb24a35f8bf2bdc68
diff --git a/include/linux/capability.h b/include/linux/capability.h
index 2776886..4dbfef3 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -44,11 +44,28 @@ typedef struct __user_cap_data_struct {

#define XATTR_CAPS_SUFFIX "capability"
#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
+
+/* size of caps that we work with */
+#define XATTR_CAPS_SZ (4*sizeof(__le32))
+
+/*
+ * data[] is organized as:
+ * effective[0]
+ * permitted[0]
+ * inheritable[0]
+ * effective[1]
+ * ...
+ * this way we can just read as much of the on-disk capability as
+ * we know should exist and know we'll get the data we'll need.
+ */
struct vfs_cap_data_disk {
__le32 version;
- __le32 effective;
- __le32 permitted;
- __le32 inheritable;
+ __le32 data[]; /* eff[0], perm[0], inh[0], eff[1], ... */
+};
+
+struct vfs_cap_data_disk_v1 {
+ __le32 version;
+ __le32 data[3]; /* eff[0], perm[0], inh[0] */
};

#ifdef __KERNEL__
diff --git a/security/Kconfig b/security/Kconfig
index bc5b1be..3d5de26 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -88,7 +88,7 @@ config SECURITY_CAPABILITIES
This enables the "default" Linux capabilities functionality.
If you are unsure how to answer this question, answer Y.

-config SECURITY_FS_CAPABILITIES
+config SECURITY_FILE_CAPABILITIES
bool "File POSIX Capabilities"
depends on SECURITY=n || SECURITY_CAPABILITIES!=n
default n
@@ -98,6 +98,16 @@ config SECURITY_FS_CAPABILITIES

If in doubt, answer N.

+config SECURITY_FILE_CAPABILITIES_STRICTXATTR
+ bool "Refuse to run files with unknown caps"
+ depends on SECURITY_FILE_CAPABILITIES
+ default y
+ help
+ Refuse to run files which have unknown capabilities set
+ in the security.capability xattr. This could prevent
+ running important binaries from an updated distribution
+ on an older kernel.
+
config SECURITY_ROOTPLUG
tristate "Root Plug Support"
depends on USB && SECURITY
diff --git a/security/commoncap.c b/security/commoncap.c
index be86acb..86894be 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -110,36 +110,73 @@ void cap_capset_set (struct task_struct
target->cap_permitted = *permitted;
}

-#ifdef CONFIG_SECURITY_FS_CAPABILITIES
-static inline void cap_from_disk(struct vfs_cap_data_disk *dcap,
- struct vfs_cap_data *cap)
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES_STRICTXATTR
+static int check_cap_sanity(struct vfs_cap_data_disk *dcap, int size)
{
- 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);
+ int word, bit;
+ u32 eff, inh, perm;
+ int sz = (size-1)/3;
+
+ word = CAP_NUMCAPS / 32;
+ bit = CAP_NUMCAPS % 32;
+
+ eff = le32_to_cpu(dcap->data[3*word]);
+ perm = le32_to_cpu(dcap->data[3*word+1]);
+ inh = le32_to_cpu(dcap->data[3*word+2]);
+
+ while (word < sz) {
+ if (bit == 32) {
+ bit = 0;
+ word++;
+ if (word >= sz)
+ break;
+ eff = le32_to_cpu(dcap->data[3*word]);
+ perm = le32_to_cpu(dcap->data[3*word+1]);
+ inh = le32_to_cpu(dcap->data[3*word+2]);
+ continue;
+ }
+ if (eff & CAP_TO_MASK(bit))
+ return -EINVAL;
+ if (inh & CAP_TO_MASK(bit))
+ return -EINVAL;
+ if (perm & CAP_TO_MASK(bit))
+ return -EINVAL;
+ bit++;
+ }
+
+ return 0;
}
+#else
+static int check_cap_sanity(struct vfs_cap_data_disk *dcap, int sz)
+{ return 0 }
+#endif

-static int check_cap_sanity(struct vfs_cap_data *cap)
+static inline int cap_from_disk(struct vfs_cap_data_disk *dcap,
+ struct linux_binprm *bprm, int size)
{
- int i;
+ int rc, version;

- if (cap->version != _LINUX_CAPABILITY_VERSION)
- return -EPERM;
+ version = le32_to_cpu(dcap->version);
+ if (version != _LINUX_CAPABILITY_VERSION)
+ return -EINVAL;

- for (i = CAP_NUMCAPS; i < 8*sizeof(cap->effective); i++) {
- if (cap->effective & CAP_TO_MASK(i))
- return -EPERM;
- }
- for (i = CAP_NUMCAPS; i < 8*sizeof(cap->permitted); i++) {
- if (cap->permitted & CAP_TO_MASK(i))
- return -EPERM;
- }
- for (i = CAP_NUMCAPS; i < 8*sizeof(cap->inheritable); i++) {
- if (cap->inheritable & CAP_TO_MASK(i))
- return -EPERM;
+ size /= sizeof(u32);
+ if ((size-1)%3) {
+ printk(KERN_WARNING "%s: size is an invalid size (%d)\n",
+ __FUNCTION__, size);
+ return -EINVAL;
}

+ rc = check_cap_sanity(dcap, size);
+ if (rc)
+ return rc;
+
+ bprm->cap_effective = le32_to_cpu(dcap->data[0]);
+ bprm->cap_permitted = le32_to_cpu(dcap->data[1]);
+ bprm->cap_inheritable = le32_to_cpu(dcap->data[2]);
+
return 0;
}

@@ -147,52 +184,58 @@ static int check_cap_sanity(struct vfs_c
static int set_file_caps(struct linux_binprm *bprm)
{
struct dentry *dentry;
- ssize_t rc;
- struct vfs_cap_data_disk dcaps;
- struct vfs_cap_data caps;
+ int rc;
+ struct vfs_cap_data_disk_v1 v1caps;
+ struct vfs_cap_data_disk *dcaps;
struct inode *inode;
- int err;

+ dcaps = (struct vfs_cap_data_disk *)&v1caps;
if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)
return 0;

dentry = dget(bprm->file->f_dentry);
inode = dentry->d_inode;
- if (!inode->i_op || !inode->i_op->getxattr) {
- dput(dentry);
- return 0;
- }
-
- rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &dcaps,
- sizeof(dcaps));
- dput(dentry);
-
- if (rc == -ENODATA)
- return 0;
-
- if (rc < 0) {
- printk(KERN_NOTICE "%s: Error (%zd) getting xattr\n",
- __FUNCTION__, rc);
- return rc;
+ rc = 0;
+ if (!inode->i_op || !inode->i_op->getxattr)
+ goto out;
+
+ rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, dcaps,
+ XATTR_CAPS_SZ);
+ if (rc == -ENODATA || rc == -EOPNOTSUPP) {
+ rc = 0;
+ goto out;
+ }
+ if (rc == -ERANGE) {
+ int size;
+ size = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, NULL, 0);
+ if (size <= 0) { /* shouldn't ever happen */
+ rc = -EINVAL;
+ goto out;
+ }
+ dcaps = kmalloc(size, GFP_KERNEL);
+ if (!dcaps) {
+ rc = -ENOMEM;
+ goto out;
+ }
+ rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, dcaps,
+ size);
}
-
- if (rc != sizeof(dcaps)) {
- printk(KERN_NOTICE "%s: got wrong size for getxattr (%zd)\n",
- __FUNCTION__, rc);
- return -EPERM;
+ if (rc < 0)
+ goto out;
+ if (rc < sizeof(struct vfs_cap_data_disk_v1)) {
+ rc = -EINVAL;
+ goto out;
}

- cap_from_disk(&dcaps, &caps);
- err = check_cap_sanity(&caps);
- if (err)
- return err;
-
- bprm->cap_effective = caps.effective;
- bprm->cap_permitted = caps.permitted;
- bprm->cap_inheritable = caps.inheritable;
+ rc = cap_from_disk(dcaps, bprm, rc);

- return 0;
+out:
+ dput(dentry);
+ if ((void *)dcaps != (void *)&v1caps)
+ kfree(dcaps);
+ return rc;
}
+
#else
static inline int set_file_caps(struct linux_binprm *bprm)
{
@@ -399,7 +442,7 @@ int cap_task_post_setuid (uid_t old_ruid
return 0;
}

-#ifdef CONFIG_SECURITY_FS_CAPABILITIES
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
/*
* Rationale: code calling task_setscheduler, task_setioprio, and
* task_setnice, assumes that
--
1.1.6


2007-02-23 15:43:53

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 2/2] file caps: accomodate future 64-bit caps

(Clearly the noop version of check_cap_sanity() needs a semicolon.
If there are no complaints about this approach in general I will send
an updated patch. And hopefully I can find a kbd without
broken ; and .)

-serge

Quoting Serge E. Hallyn ([email protected]):
> Here is another attempt. This format is compatible with
> KaiGai's current tools.
>
> Tested on s390 with 32 and 64-bit caps stored in the xattrs.
>
> -serge
>
>
> From: "Serge E. Hallyn" <[email protected]>
> Subject: [PATCH 2/2] file caps: accomodate future 64-bit caps
>
> As the capability set changes and distributions start tagging
> binaries with capabilities, we would like for running an older
> kernel to not necessarily make those binaries unusable.
>
> (1. Rename CONFIG_SECURITY_FS_CAPABILITIES to
> CONFIG_SECURITY_FILE_CAPABILITIES)
> 2. Introduce CONFIG_SECURITY_FILE_CAPABILITIES_STRICTXATTR
> which, when set, prevents loading binaries with capabilities
> set which the kernel doesn't know about. When not set,
> such capabilities run, ignoring the unknown caps.
> 3. To accomodate 64-bit caps, specify that capabilities are
> stored as
> u32 version; u32 eff0; u32 perm0; u32 inh0;
> u32 eff1; u32 perm1; u32 inh1; (etc)
>
> Signed-off-by: Serge E. Hallyn <[email protected]>
>
> ---
>
> include/linux/capability.h | 23 ++++++
> security/Kconfig | 12 +++
> security/commoncap.c | 157 ++++++++++++++++++++++++++++----------------
> 3 files changed, 131 insertions(+), 61 deletions(-)
>
> 987fe7fcd60aaea6aaa86e6eb24a35f8bf2bdc68
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 2776886..4dbfef3 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -44,11 +44,28 @@ typedef struct __user_cap_data_struct {
>
> #define XATTR_CAPS_SUFFIX "capability"
> #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> +
> +/* size of caps that we work with */
> +#define XATTR_CAPS_SZ (4*sizeof(__le32))
> +
> +/*
> + * data[] is organized as:
> + * effective[0]
> + * permitted[0]
> + * inheritable[0]
> + * effective[1]
> + * ...
> + * this way we can just read as much of the on-disk capability as
> + * we know should exist and know we'll get the data we'll need.
> + */
> struct vfs_cap_data_disk {
> __le32 version;
> - __le32 effective;
> - __le32 permitted;
> - __le32 inheritable;
> + __le32 data[]; /* eff[0], perm[0], inh[0], eff[1], ... */
> +};
> +
> +struct vfs_cap_data_disk_v1 {
> + __le32 version;
> + __le32 data[3]; /* eff[0], perm[0], inh[0] */
> };
>
> #ifdef __KERNEL__
> diff --git a/security/Kconfig b/security/Kconfig
> index bc5b1be..3d5de26 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -88,7 +88,7 @@ config SECURITY_CAPABILITIES
> This enables the "default" Linux capabilities functionality.
> If you are unsure how to answer this question, answer Y.
>
> -config SECURITY_FS_CAPABILITIES
> +config SECURITY_FILE_CAPABILITIES
> bool "File POSIX Capabilities"
> depends on SECURITY=n || SECURITY_CAPABILITIES!=n
> default n
> @@ -98,6 +98,16 @@ config SECURITY_FS_CAPABILITIES
>
> If in doubt, answer N.
>
> +config SECURITY_FILE_CAPABILITIES_STRICTXATTR
> + bool "Refuse to run files with unknown caps"
> + depends on SECURITY_FILE_CAPABILITIES
> + default y
> + help
> + Refuse to run files which have unknown capabilities set
> + in the security.capability xattr. This could prevent
> + running important binaries from an updated distribution
> + on an older kernel.
> +
> config SECURITY_ROOTPLUG
> tristate "Root Plug Support"
> depends on USB && SECURITY
> diff --git a/security/commoncap.c b/security/commoncap.c
> index be86acb..86894be 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -110,36 +110,73 @@ void cap_capset_set (struct task_struct
> target->cap_permitted = *permitted;
> }
>
> -#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> -static inline void cap_from_disk(struct vfs_cap_data_disk *dcap,
> - struct vfs_cap_data *cap)
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> +
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES_STRICTXATTR
> +static int check_cap_sanity(struct vfs_cap_data_disk *dcap, int size)
> {
> - 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);
> + int word, bit;
> + u32 eff, inh, perm;
> + int sz = (size-1)/3;
> +
> + word = CAP_NUMCAPS / 32;
> + bit = CAP_NUMCAPS % 32;
> +
> + eff = le32_to_cpu(dcap->data[3*word]);
> + perm = le32_to_cpu(dcap->data[3*word+1]);
> + inh = le32_to_cpu(dcap->data[3*word+2]);
> +
> + while (word < sz) {
> + if (bit == 32) {
> + bit = 0;
> + word++;
> + if (word >= sz)
> + break;
> + eff = le32_to_cpu(dcap->data[3*word]);
> + perm = le32_to_cpu(dcap->data[3*word+1]);
> + inh = le32_to_cpu(dcap->data[3*word+2]);
> + continue;
> + }
> + if (eff & CAP_TO_MASK(bit))
> + return -EINVAL;
> + if (inh & CAP_TO_MASK(bit))
> + return -EINVAL;
> + if (perm & CAP_TO_MASK(bit))
> + return -EINVAL;
> + bit++;
> + }
> +
> + return 0;
> }
> +#else
> +static int check_cap_sanity(struct vfs_cap_data_disk *dcap, int sz)
> +{ return 0 }
> +#endif
>
> -static int check_cap_sanity(struct vfs_cap_data *cap)
> +static inline int cap_from_disk(struct vfs_cap_data_disk *dcap,
> + struct linux_binprm *bprm, int size)
> {
> - int i;
> + int rc, version;
>
> - if (cap->version != _LINUX_CAPABILITY_VERSION)
> - return -EPERM;
> + version = le32_to_cpu(dcap->version);
> + if (version != _LINUX_CAPABILITY_VERSION)
> + return -EINVAL;
>
> - for (i = CAP_NUMCAPS; i < 8*sizeof(cap->effective); i++) {
> - if (cap->effective & CAP_TO_MASK(i))
> - return -EPERM;
> - }
> - for (i = CAP_NUMCAPS; i < 8*sizeof(cap->permitted); i++) {
> - if (cap->permitted & CAP_TO_MASK(i))
> - return -EPERM;
> - }
> - for (i = CAP_NUMCAPS; i < 8*sizeof(cap->inheritable); i++) {
> - if (cap->inheritable & CAP_TO_MASK(i))
> - return -EPERM;
> + size /= sizeof(u32);
> + if ((size-1)%3) {
> + printk(KERN_WARNING "%s: size is an invalid size (%d)\n",
> + __FUNCTION__, size);
> + return -EINVAL;
> }
>
> + rc = check_cap_sanity(dcap, size);
> + if (rc)
> + return rc;
> +
> + bprm->cap_effective = le32_to_cpu(dcap->data[0]);
> + bprm->cap_permitted = le32_to_cpu(dcap->data[1]);
> + bprm->cap_inheritable = le32_to_cpu(dcap->data[2]);
> +
> return 0;
> }
>
> @@ -147,52 +184,58 @@ static int check_cap_sanity(struct vfs_c
> static int set_file_caps(struct linux_binprm *bprm)
> {
> struct dentry *dentry;
> - ssize_t rc;
> - struct vfs_cap_data_disk dcaps;
> - struct vfs_cap_data caps;
> + int rc;
> + struct vfs_cap_data_disk_v1 v1caps;
> + struct vfs_cap_data_disk *dcaps;
> struct inode *inode;
> - int err;
>
> + dcaps = (struct vfs_cap_data_disk *)&v1caps;
> if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)
> return 0;
>
> dentry = dget(bprm->file->f_dentry);
> inode = dentry->d_inode;
> - if (!inode->i_op || !inode->i_op->getxattr) {
> - dput(dentry);
> - return 0;
> - }
> -
> - rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &dcaps,
> - sizeof(dcaps));
> - dput(dentry);
> -
> - if (rc == -ENODATA)
> - return 0;
> -
> - if (rc < 0) {
> - printk(KERN_NOTICE "%s: Error (%zd) getting xattr\n",
> - __FUNCTION__, rc);
> - return rc;
> + rc = 0;
> + if (!inode->i_op || !inode->i_op->getxattr)
> + goto out;
> +
> + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, dcaps,
> + XATTR_CAPS_SZ);
> + if (rc == -ENODATA || rc == -EOPNOTSUPP) {
> + rc = 0;
> + goto out;
> + }
> + if (rc == -ERANGE) {
> + int size;
> + size = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, NULL, 0);
> + if (size <= 0) { /* shouldn't ever happen */
> + rc = -EINVAL;
> + goto out;
> + }
> + dcaps = kmalloc(size, GFP_KERNEL);
> + if (!dcaps) {
> + rc = -ENOMEM;
> + goto out;
> + }
> + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, dcaps,
> + size);
> }
> -
> - if (rc != sizeof(dcaps)) {
> - printk(KERN_NOTICE "%s: got wrong size for getxattr (%zd)\n",
> - __FUNCTION__, rc);
> - return -EPERM;
> + if (rc < 0)
> + goto out;
> + if (rc < sizeof(struct vfs_cap_data_disk_v1)) {
> + rc = -EINVAL;
> + goto out;
> }
>
> - cap_from_disk(&dcaps, &caps);
> - err = check_cap_sanity(&caps);
> - if (err)
> - return err;
> -
> - bprm->cap_effective = caps.effective;
> - bprm->cap_permitted = caps.permitted;
> - bprm->cap_inheritable = caps.inheritable;
> + rc = cap_from_disk(dcaps, bprm, rc);
>
> - return 0;
> +out:
> + dput(dentry);
> + if ((void *)dcaps != (void *)&v1caps)
> + kfree(dcaps);
> + return rc;
> }
> +
> #else
> static inline int set_file_caps(struct linux_binprm *bprm)
> {
> @@ -399,7 +442,7 @@ int cap_task_post_setuid (uid_t old_ruid
> return 0;
> }
>
> -#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> /*
> * Rationale: code calling task_setscheduler, task_setioprio, and
> * task_setnice, assumes that
> --
> 1.1.6
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/