lkml.org 
[lkml]   [2010]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/5] secmark: make secmark object handling generic
From
Date
On Tue, 2010-10-12 at 11:40 -0400, Eric Paris wrote:
> Right now secmark has lots of direct selinux calls. Use all LSM calls and
> remove all SELinux specific knowledge. The only SELinux specific knowledge
> we leave is the mode. The only point is to make sure that other LSMs at
> least test this generic code before they assume it works. (They may also
> have to make changes if they do not represent labels as strings)

I'm sure you have, but I just want to make sure - you've tested this
change (and the others for that matter) against the existing iptables
userspace to make sure everything still works, right?

> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
>
> include/linux/netfilter/xt_SECMARK.h | 12 ++----
> include/linux/security.h | 25 +++++++++++++
> include/linux/selinux.h | 63 ----------------------------------
> net/netfilter/xt_CT.c | 1 -
> net/netfilter/xt_SECMARK.c | 51 +++++++++++++---------------
> security/capability.c | 17 +++++++++
> security/security.c | 18 ++++++++++
> security/selinux/exports.c | 49 --------------------------
> security/selinux/hooks.c | 24 +++++++++++++
> security/selinux/include/security.h | 1 +
> 10 files changed, 112 insertions(+), 149 deletions(-)
>
> diff --git a/include/linux/netfilter/xt_SECMARK.h b/include/linux/netfilter/xt_SECMARK.h
> index 6fcd344..989092b 100644
> --- a/include/linux/netfilter/xt_SECMARK.h
> +++ b/include/linux/netfilter/xt_SECMARK.h
> @@ -11,18 +11,12 @@
> * packets are being marked for.
> */
> #define SECMARK_MODE_SEL 0x01 /* SELinux */
> -#define SECMARK_SELCTX_MAX 256
> -
> -struct xt_secmark_target_selinux_info {
> - __u32 selsid;
> - char selctx[SECMARK_SELCTX_MAX];
> -};
> +#define SECMARK_SECCTX_MAX 256
>
> struct xt_secmark_target_info {
> __u8 mode;
> - union {
> - struct xt_secmark_target_selinux_info sel;
> - } u;
> + __u32 secid;
> + char secctx[SECMARK_SECCTX_MAX];
> };
>
> #endif /*_XT_SECMARK_H_target */
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 43771c8..f050119 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -960,6 +960,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * Sets the new child socket's sid to the openreq sid.
> * @inet_conn_established:
> * Sets the connection's peersid to the secmark on skb.
> + * @secmark_relabel_packet:
> + * check if the process should be allowed to relabel packets to the given secid
> + * @security_secmark_refcount_inc
> + * tells the LSM to increment the number of secmark labeling rules loaded
> + * @security_secmark_refcount_dec
> + * tells the LSM to decrement the number of secmark labeling rules loaded
> * @req_classify_flow:
> * Sets the flow's sid to the openreq sid.
> * @tun_dev_create:
> @@ -1596,6 +1602,9 @@ struct security_operations {
> struct request_sock *req);
> void (*inet_csk_clone) (struct sock *newsk, const struct request_sock *req);
> void (*inet_conn_established) (struct sock *sk, struct sk_buff *skb);
> + int (*secmark_relabel_packet) (u32 secid);
> + void (*secmark_refcount_inc) (void);
> + void (*secmark_refcount_dec) (void);
> void (*req_classify_flow) (const struct request_sock *req, struct flowi *fl);
> int (*tun_dev_create)(void);
> void (*tun_dev_post_create)(struct sock *sk);
> @@ -2556,6 +2565,9 @@ void security_inet_csk_clone(struct sock *newsk,
> const struct request_sock *req);
> void security_inet_conn_established(struct sock *sk,
> struct sk_buff *skb);
> +int security_secmark_relabel_packet(u32 secid);
> +void security_secmark_refcount_inc(void);
> +void security_secmark_refcount_dec(void);
> int security_tun_dev_create(void);
> void security_tun_dev_post_create(struct sock *sk);
> int security_tun_dev_attach(struct sock *sk);
> @@ -2710,6 +2722,19 @@ static inline void security_inet_conn_established(struct sock *sk,
> {
> }
>
> +static inline int security_secmark_relabel_packet(u32 secid)
> +{
> + return 0;
> +}
> +
> +static inline void security_secmark_refcount_inc(void)
> +{
> +}
> +
> +static inline void security_secmark_refcount_dec(void)
> +{
> +}
> +
> static inline int security_tun_dev_create(void)
> {
> return 0;
> diff --git a/include/linux/selinux.h b/include/linux/selinux.h
> index 82e0f26..44f4596 100644
> --- a/include/linux/selinux.h
> +++ b/include/linux/selinux.h
> @@ -21,74 +21,11 @@ struct kern_ipc_perm;
> #ifdef CONFIG_SECURITY_SELINUX
>
> /**
> - * selinux_string_to_sid - map a security context string to a security ID
> - * @str: the security context string to be mapped
> - * @sid: ID value returned via this.
> - *
> - * Returns 0 if successful, with the SID stored in sid. A value
> - * of zero for sid indicates no SID could be determined (but no error
> - * occurred).
> - */
> -int selinux_string_to_sid(char *str, u32 *sid);
> -
> -/**
> - * selinux_secmark_relabel_packet_permission - secmark permission check
> - * @sid: SECMARK ID value to be applied to network packet
> - *
> - * Returns 0 if the current task is allowed to set the SECMARK label of
> - * packets with the supplied security ID. Note that it is implicit that
> - * the packet is always being relabeled from the default unlabeled value,
> - * and that the access control decision is made in the AVC.
> - */
> -int selinux_secmark_relabel_packet_permission(u32 sid);
> -
> -/**
> - * selinux_secmark_refcount_inc - increments the secmark use counter
> - *
> - * SELinux keeps track of the current SECMARK targets in use so it knows
> - * when to apply SECMARK label access checks to network packets. This
> - * function incements this reference count to indicate that a new SECMARK
> - * target has been configured.
> - */
> -void selinux_secmark_refcount_inc(void);
> -
> -/**
> - * selinux_secmark_refcount_dec - decrements the secmark use counter
> - *
> - * SELinux keeps track of the current SECMARK targets in use so it knows
> - * when to apply SECMARK label access checks to network packets. This
> - * function decements this reference count to indicate that one of the
> - * existing SECMARK targets has been removed/flushed.
> - */
> -void selinux_secmark_refcount_dec(void);
> -
> -/**
> * selinux_is_enabled - is SELinux enabled?
> */
> bool selinux_is_enabled(void);
> #else
>
> -static inline int selinux_string_to_sid(const char *str, u32 *sid)
> -{
> - *sid = 0;
> - return 0;
> -}
> -
> -static inline int selinux_secmark_relabel_packet_permission(u32 sid)
> -{
> - return 0;
> -}
> -
> -static inline void selinux_secmark_refcount_inc(void)
> -{
> - return;
> -}
> -
> -static inline void selinux_secmark_refcount_dec(void)
> -{
> - return;
> -}
> -
> static inline bool selinux_is_enabled(void)
> {
> return false;
> diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> index 0cb6053..782e519 100644
> --- a/net/netfilter/xt_CT.c
> +++ b/net/netfilter/xt_CT.c
> @@ -9,7 +9,6 @@
> #include <linux/module.h>
> #include <linux/gfp.h>
> #include <linux/skbuff.h>
> -#include <linux/selinux.h>
> #include <linux/netfilter_ipv4/ip_tables.h>
> #include <linux/netfilter_ipv6/ip6_tables.h>
> #include <linux/netfilter/x_tables.h>
> diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
> index 364ad16..295054e 100644
> --- a/net/netfilter/xt_SECMARK.c
> +++ b/net/netfilter/xt_SECMARK.c
> @@ -14,8 +14,8 @@
> */
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> #include <linux/module.h>
> +#include <linux/security.h>
> #include <linux/skbuff.h>
> -#include <linux/selinux.h>
> #include <linux/netfilter/x_tables.h>
> #include <linux/netfilter/xt_SECMARK.h>
>
> @@ -39,9 +39,8 @@ secmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
>
> switch (mode) {
> case SECMARK_MODE_SEL:
> - secmark = info->u.sel.selsid;
> + secmark = info->secid;
> break;
> -
> default:
> BUG();
> }
> @@ -50,33 +49,33 @@ secmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
> return XT_CONTINUE;
> }
>
> -static int checkentry_selinux(struct xt_secmark_target_info *info)
> +static int checkentry_lsm(struct xt_secmark_target_info *info)
> {
> int err;
> - struct xt_secmark_target_selinux_info *sel = &info->u.sel;
>
> - sel->selctx[SECMARK_SELCTX_MAX - 1] = '\0';
> + info->secctx[SECMARK_SECCTX_MAX - 1] = '\0';
> + info->secid = 0;
>
> - err = selinux_string_to_sid(sel->selctx, &sel->selsid);
> + err = security_secctx_to_secid(info->secctx, strlen(info->secctx),
> + &info->secid);
> if (err) {
> if (err == -EINVAL)
> - pr_info("invalid SELinux context \'%s\'\n",
> - sel->selctx);
> + pr_info("invalid security context \'%s\'\n", info->secctx);
> return err;
> }
>
> - if (!sel->selsid) {
> - pr_info("unable to map SELinux context \'%s\'\n", sel->selctx);
> + if (!info->secid) {
> + pr_info("unable to map security context \'%s\'\n", info->secctx);
> return -ENOENT;
> }
>
> - err = selinux_secmark_relabel_packet_permission(sel->selsid);
> + err = security_secmark_relabel_packet(info->secid);
> if (err) {
> pr_info("unable to obtain relabeling permission\n");
> return err;
> }
>
> - selinux_secmark_refcount_inc();
> + security_secmark_refcount_inc();
> return 0;
> }
>
> @@ -100,16 +99,16 @@ static int secmark_tg_check(const struct xt_tgchk_param *par)
>
> switch (info->mode) {
> case SECMARK_MODE_SEL:
> - err = checkentry_selinux(info);
> - if (err)
> - return err;
> break;
> -
> default:
> pr_info("invalid mode: %hu\n", info->mode);
> return -EINVAL;
> }
>
> + err = checkentry_lsm(info);
> + if (err)
> + return err;
> +
> if (!mode)
> mode = info->mode;
> return 0;
> @@ -119,19 +118,19 @@ static void secmark_tg_destroy(const struct xt_tgdtor_param *par)
> {
> switch (mode) {
> case SECMARK_MODE_SEL:
> - selinux_secmark_refcount_dec();
> + security_secmark_refcount_dec();
> }
> }
>
> static struct xt_target secmark_tg_reg __read_mostly = {
> - .name = "SECMARK",
> - .revision = 0,
> - .family = NFPROTO_UNSPEC,
> - .checkentry = secmark_tg_check,
> - .destroy = secmark_tg_destroy,
> - .target = secmark_tg,
> - .targetsize = sizeof(struct xt_secmark_target_info),
> - .me = THIS_MODULE,
> + .name = "SECMARK",
> + .revision = 0,
> + .family = NFPROTO_UNSPEC,
> + .checkentry = secmark_tg_check,
> + .destroy = secmark_tg_destroy,
> + .target = secmark_tg,
> + .targetsize = sizeof(struct xt_secmark_target_info),
> + .me = THIS_MODULE,
> };
>
> static int __init secmark_tg_init(void)
> diff --git a/security/capability.c b/security/capability.c
> index 1e26299..806061b 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -681,7 +681,18 @@ static void cap_inet_conn_established(struct sock *sk, struct sk_buff *skb)
> {
> }
>
> +static int cap_secmark_relabel_packet(u32 secid)
> +{
> + return 0;
> +}
>
> +static void cap_secmark_refcount_inc(void)
> +{
> +}
> +
> +static void cap_secmark_refcount_dec(void)
> +{
> +}
>
> static void cap_req_classify_flow(const struct request_sock *req,
> struct flowi *fl)
> @@ -781,7 +792,8 @@ static int cap_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
>
> static int cap_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> {
> - return -EOPNOTSUPP;
> + *secid = 0;
> + return 0;
> }
>
> static void cap_release_secctx(char *secdata, u32 seclen)
> @@ -1023,6 +1035,9 @@ void __init security_fixup_ops(struct security_operations *ops)
> set_to_cap_if_null(ops, inet_conn_request);
> set_to_cap_if_null(ops, inet_csk_clone);
> set_to_cap_if_null(ops, inet_conn_established);
> + set_to_cap_if_null(ops, secmark_relabel_packet);
> + set_to_cap_if_null(ops, secmark_refcount_inc);
> + set_to_cap_if_null(ops, secmark_refcount_dec);
> set_to_cap_if_null(ops, req_classify_flow);
> set_to_cap_if_null(ops, tun_dev_create);
> set_to_cap_if_null(ops, tun_dev_post_create);
> diff --git a/security/security.c b/security/security.c
> index fe2da98..a8aa414 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1143,6 +1143,24 @@ void security_inet_conn_established(struct sock *sk,
> security_ops->inet_conn_established(sk, skb);
> }
>
> +int security_secmark_relabel_packet(u32 secid)
> +{
> + return security_ops->secmark_relabel_packet(secid);
> +}
> +EXPORT_SYMBOL(security_secmark_relabel_packet);
> +
> +void security_secmark_refcount_inc(void)
> +{
> + security_ops->secmark_refcount_inc();
> +}
> +EXPORT_SYMBOL(security_secmark_refcount_inc);
> +
> +void security_secmark_refcount_dec(void)
> +{
> + security_ops->secmark_refcount_dec();
> +}
> +EXPORT_SYMBOL(security_secmark_refcount_dec);
> +
> int security_tun_dev_create(void)
> {
> return security_ops->tun_dev_create();
> diff --git a/security/selinux/exports.c b/security/selinux/exports.c
> index c0a454a..9066438 100644
> --- a/security/selinux/exports.c
> +++ b/security/selinux/exports.c
> @@ -11,58 +11,9 @@
> * it under the terms of the GNU General Public License version 2,
> * as published by the Free Software Foundation.
> */
> -#include <linux/types.h>
> -#include <linux/kernel.h>
> #include <linux/module.h>
> -#include <linux/selinux.h>
> -#include <linux/fs.h>
> -#include <linux/ipc.h>
> -#include <asm/atomic.h>
>
> #include "security.h"
> -#include "objsec.h"
> -
> -/* SECMARK reference count */
> -extern atomic_t selinux_secmark_refcount;
> -
> -int selinux_string_to_sid(char *str, u32 *sid)
> -{
> - if (selinux_enabled)
> - return security_context_to_sid(str, strlen(str), sid);
> - else {
> - *sid = 0;
> - return 0;
> - }
> -}
> -EXPORT_SYMBOL_GPL(selinux_string_to_sid);
> -
> -int selinux_secmark_relabel_packet_permission(u32 sid)
> -{
> - if (selinux_enabled) {
> - const struct task_security_struct *__tsec;
> - u32 tsid;
> -
> - __tsec = current_security();
> - tsid = __tsec->sid;
> -
> - return avc_has_perm(tsid, sid, SECCLASS_PACKET,
> - PACKET__RELABELTO, NULL);
> - }
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(selinux_secmark_relabel_packet_permission);
> -
> -void selinux_secmark_refcount_inc(void)
> -{
> - atomic_inc(&selinux_secmark_refcount);
> -}
> -EXPORT_SYMBOL_GPL(selinux_secmark_refcount_inc);
> -
> -void selinux_secmark_refcount_dec(void)
> -{
> - atomic_dec(&selinux_secmark_refcount);
> -}
> -EXPORT_SYMBOL_GPL(selinux_secmark_refcount_dec);
>
> bool selinux_is_enabled(void)
> {
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f31b0a3..a6c063b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4290,6 +4290,27 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
> selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
> }
>
> +static int selinux_secmark_relabel_packet(u32 sid)
> +{
> + const struct task_security_struct *__tsec;
> + u32 tsid;
> +
> + __tsec = current_security();
> + tsid = __tsec->sid;
> +
> + return avc_has_perm(tsid, sid, SECCLASS_PACKET, PACKET__RELABELTO, NULL);
> +}
> +
> +static void selinux_secmark_refcount_inc(void)
> +{
> + atomic_inc(&selinux_secmark_refcount);
> +}
> +
> +static void selinux_secmark_refcount_dec(void)
> +{
> + atomic_dec(&selinux_secmark_refcount);
> +}
> +
> static void selinux_req_classify_flow(const struct request_sock *req,
> struct flowi *fl)
> {
> @@ -5545,6 +5566,9 @@ static struct security_operations selinux_ops = {
> .inet_conn_request = selinux_inet_conn_request,
> .inet_csk_clone = selinux_inet_csk_clone,
> .inet_conn_established = selinux_inet_conn_established,
> + .secmark_relabel_packet = selinux_secmark_relabel_packet,
> + .secmark_refcount_inc = selinux_secmark_refcount_inc,
> + .secmark_refcount_dec = selinux_secmark_refcount_dec,
> .req_classify_flow = selinux_req_classify_flow,
> .tun_dev_create = selinux_tun_dev_create,
> .tun_dev_post_create = selinux_tun_dev_post_create,
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 79df4a2..671273e 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -9,6 +9,7 @@
> #define _SELINUX_SECURITY_H_
>
> #include <linux/magic.h>
> +#include <linux/types.h>
> #include "flask.h"
>
> #define SECSID_NULL 0x00000000 /* unspecified SID */
>

--
paul moore
linux @ hp




\
 
 \ /
  Last update: 2010-10-13 00:59    [W:0.256 / U:0.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site