| Message ID | 20090810172850.7946.25175.stgit@flek.lan |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Aug 10, 2009 at 1:28 PM, Paul Moore<paul.moore@hp.com> wrote: > Add support for the new TUN LSM hooks: security_tun_dev_create(), > security_tun_dev_post_create() and security_tun_dev_attach(). This includes > the addition of a new object class, tun_socket, which represents the socks > associated with TUN devices. The _tun_dev_create() and _tun_dev_post_create() > hooks are fairly similar to the standard socket functions but _tun_dev_attach() > is a bit special. The _tun_dev_attach() is unique because it involves a > domain attaching to an existing TUN device and its associated tun_socket > object, an operation which does not exist with standard sockets and most > closely resembles a relabel operation. Looks good to me, feel free to add my Ack -Eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 11 August 2009 04:36:22 pm Eric Paris wrote: > On Mon, Aug 10, 2009 at 1:28 PM, Paul Moore<paul.moore@hp.com> wrote: > > Add support for the new TUN LSM hooks: security_tun_dev_create(), > > security_tun_dev_post_create() and security_tun_dev_attach(). This > > includes the addition of a new object class, tun_socket, which represents > > the socks associated with TUN devices. The _tun_dev_create() and > > _tun_dev_post_create() hooks are fairly similar to the standard socket > > functions but _tun_dev_attach() is a bit special. The _tun_dev_attach() > > is unique because it involves a domain attaching to an existing TUN > > device and its associated tun_socket object, an operation which does not > > exist with standard sockets and most closely resembles a relabel > > operation. > > Looks good to me, feel free to add my Ack Thanks, I added both acks.
Quoting Paul Moore (paul.moore@hp.com): > Add support for the new TUN LSM hooks: security_tun_dev_create(), > security_tun_dev_post_create() and security_tun_dev_attach(). This includes > the addition of a new object class, tun_socket, which represents the socks > associated with TUN devices. The _tun_dev_create() and _tun_dev_post_create() > hooks are fairly similar to the standard socket functions but _tun_dev_attach() > is a bit special. The _tun_dev_attach() is unique because it involves a > domain attaching to an existing TUN device and its associated tun_socket > object, an operation which does not exist with standard sockets and most > closely resembles a relabel operation. > > -- > > NOTE: This relies on some changes to the policy to add the new object class > and its associated permissions, I will ensure that the policy is sorted > and merged before pushing this patch upstream. Also, you will notice > that the new tun_socket object class simply inherits the base socket > object class, thoughts? > --- > > security/selinux/hooks.c | 60 +++++++++++++++++++++++++++- > security/selinux/include/av_inherit.h | 1 > security/selinux/include/av_permissions.h | 22 ++++++++++ > security/selinux/include/class_to_string.h | 1 > security/selinux/include/flask.h | 1 > 5 files changed, 83 insertions(+), 2 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 15c2a08..fc7caa0 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -13,8 +13,8 @@ > * Eric Paris <eparis@redhat.com> > * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc. > * <dgoeddel@trustedcs.com> > - * Copyright (C) 2006, 2007 Hewlett-Packard Development Company, L.P. > - * Paul Moore <paul.moore@hp.com> > + * Copyright (C) 2006, 2007, 2009 Hewlett-Packard Development Company, L.P. > + * Paul Moore <paul.moore@hp.com> > * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. > * Yuichi Nakamura <ynakam@hitachisoft.jp> > * > @@ -4296,6 +4296,59 @@ static void selinux_req_classify_flow(const struct request_sock *req, > fl->secid = req->secid; > } > > +static int selinux_tun_dev_create(void) > +{ > + u32 sid = current_sid(); > + > + /* we aren't taking into account the "sockcreate" SID since the socket > + * that is being created here is not a socket in the traditional sense, > + * instead it is a private sock, accessible only to the kernel, and > + * representing a wide range of network traffic spanning multiple > + * connections unlike traditional sockets - check the TUN driver to > + * get a better understanding of why this socket is special */ > + > + return avc_has_perm(sid, sid, SECCLASS_TUN_SOCKET, TUN_SOCKET__CREATE, > + NULL); > +} > + > +static void selinux_tun_dev_post_create(struct sock *sk) > +{ > + struct sk_security_struct *sksec = sk->sk_security; > + > + /* we don't currently perform any NetLabel based labeling here and it > + * isn't clear that we would want to do so anyway; while we could apply > + * labeling without the support of the TUN user the resulting labeled > + * traffic from the other end of the connection would almost certainly > + * cause confusion to the TUN user that had no idea network labeling > + * protocols were being used */ > + > + /* see the comments in selinux_tun_dev_create() about why we don't use > + * the sockcreate SID here */ > + > + sksec->sid = current_sid(); > + sksec->sclass = SECCLASS_TUN_SOCKET; > +} > + > +static int selinux_tun_dev_attach(struct sock *sk) > +{ > + struct sk_security_struct *sksec = sk->sk_security; > + u32 sid = current_sid(); > + int err; > + > + err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET, > + TUN_SOCKET__RELABELFROM, NULL); > + if (err) > + return err; > + err = avc_has_perm(sid, sid, SECCLASS_RAWIP_SOCKET, Was RAWIP on purpose here? > + TUN_SOCKET__RELABELTO, NULL); > + if (err) > + return err; > + > + sksec->sid = sid; > + > + return 0; > +} IIUC it is possible for multiple processes to attach to the same tun device. Will it get confusing/incorrect to have each attach potentially (if tasks have different sids) relabel? > static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb) > { > int err = 0; > @@ -5464,6 +5517,9 @@ static struct security_operations selinux_ops = { > .inet_csk_clone = selinux_inet_csk_clone, > .inet_conn_established = selinux_inet_conn_established, > .req_classify_flow = selinux_req_classify_flow, > + .tun_dev_create = selinux_tun_dev_create, > + .tun_dev_post_create = selinux_tun_dev_post_create, > + .tun_dev_attach = selinux_tun_dev_attach, > > #ifdef CONFIG_SECURITY_NETWORK_XFRM > .xfrm_policy_alloc_security = selinux_xfrm_policy_alloc, > diff --git a/security/selinux/include/av_inherit.h b/security/selinux/include/av_inherit.h > index 8377a4b..abedcd7 100644 > --- a/security/selinux/include/av_inherit.h > +++ b/security/selinux/include/av_inherit.h > @@ -15,6 +15,7 @@ > S_(SECCLASS_KEY_SOCKET, socket, 0x00400000UL) > S_(SECCLASS_UNIX_STREAM_SOCKET, socket, 0x00400000UL) > S_(SECCLASS_UNIX_DGRAM_SOCKET, socket, 0x00400000UL) > + S_(SECCLASS_TUN_SOCKET, socket, 0x00400000UL) > S_(SECCLASS_IPC, ipc, 0x00000200UL) > S_(SECCLASS_SEM, ipc, 0x00000200UL) > S_(SECCLASS_MSGQ, ipc, 0x00000200UL) > diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h > index d645192..0b41ad5 100644 > --- a/security/selinux/include/av_permissions.h > +++ b/security/selinux/include/av_permissions.h > @@ -423,6 +423,28 @@ > #define UNIX_DGRAM_SOCKET__RECV_MSG 0x00080000UL > #define UNIX_DGRAM_SOCKET__SEND_MSG 0x00100000UL > #define UNIX_DGRAM_SOCKET__NAME_BIND 0x00200000UL > +#define TUN_SOCKET__IOCTL 0x00000001UL > +#define TUN_SOCKET__READ 0x00000002UL > +#define TUN_SOCKET__WRITE 0x00000004UL > +#define TUN_SOCKET__CREATE 0x00000008UL > +#define TUN_SOCKET__GETATTR 0x00000010UL > +#define TUN_SOCKET__SETATTR 0x00000020UL > +#define TUN_SOCKET__LOCK 0x00000040UL > +#define TUN_SOCKET__RELABELFROM 0x00000080UL > +#define TUN_SOCKET__RELABELTO 0x00000100UL > +#define TUN_SOCKET__APPEND 0x00000200UL > +#define TUN_SOCKET__BIND 0x00000400UL > +#define TUN_SOCKET__CONNECT 0x00000800UL > +#define TUN_SOCKET__LISTEN 0x00001000UL > +#define TUN_SOCKET__ACCEPT 0x00002000UL > +#define TUN_SOCKET__GETOPT 0x00004000UL > +#define TUN_SOCKET__SETOPT 0x00008000UL > +#define TUN_SOCKET__SHUTDOWN 0x00010000UL > +#define TUN_SOCKET__RECVFROM 0x00020000UL > +#define TUN_SOCKET__SENDTO 0x00040000UL > +#define TUN_SOCKET__RECV_MSG 0x00080000UL > +#define TUN_SOCKET__SEND_MSG 0x00100000UL > +#define TUN_SOCKET__NAME_BIND 0x00200000UL > #define PROCESS__FORK 0x00000001UL > #define PROCESS__TRANSITION 0x00000002UL > #define PROCESS__SIGCHLD 0x00000004UL > diff --git a/security/selinux/include/class_to_string.h b/security/selinux/include/class_to_string.h > index 21ec786..7ab9299 100644 > --- a/security/selinux/include/class_to_string.h > +++ b/security/selinux/include/class_to_string.h > @@ -77,3 +77,4 @@ > S_(NULL) > S_(NULL) > S_("kernel_service") > + S_("tun_socket") > diff --git a/security/selinux/include/flask.h b/security/selinux/include/flask.h > index 882f27d..f248500 100644 > --- a/security/selinux/include/flask.h > +++ b/security/selinux/include/flask.h > @@ -53,6 +53,7 @@ > #define SECCLASS_PEER 68 > #define SECCLASS_CAPABILITY2 69 > #define SECCLASS_KERNEL_SERVICE 74 > +#define SECCLASS_TUN_SOCKET 75 > > /* > * Security identifier indices for initial entities > > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 12 August 2009 06:14:40 pm Serge E. Hallyn wrote: > Quoting Paul Moore (paul.moore@hp.com): > > +static int selinux_tun_dev_attach(struct sock *sk) > > +{ > > + struct sk_security_struct *sksec = sk->sk_security; > > + u32 sid = current_sid(); > > + int err; > > + > > + err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET, > > + TUN_SOCKET__RELABELFROM, NULL); > > + if (err) > > + return err; > > + err = avc_has_perm(sid, sid, SECCLASS_RAWIP_SOCKET, > > Was RAWIP on purpose here? Nope, a mistake on my part that I hadn't caught yet. Thanks. > > + TUN_SOCKET__RELABELTO, NULL); > > + if (err) > > + return err; > > + > > + sksec->sid = sid; > > + > > + return 0; > > +} > > IIUC it is possible for multiple processes to attach to the same > tun device. Will it get confusing/incorrect to have each attach > potentially (if tasks have different sids) relabel? I may be reading the code wrong, but in drivers/net/tun.c:tun_attach() the code checks to see if the TUN device is already in use and if it is then the attach fails with -EBUSY (check where the tun_device->tfile is examined). I believe this should ensure that only one process at a time has access to the TUN device so we shouldn't have to worry about a TUN socket getting relabeled while it is currently in use. As far as persistent TUN devices getting relabeled when a new process attaches to them, that is what we are trying to accomplish here so that the network traffic being sent via the TUN device is labeled according to the currently attached process; this is consistent with how SELinux currently labels locally generated outbound traffic - outbound packets inherit their security label from the sending process via the originating socket/sock.
Quoting Paul Moore (paul.moore@hp.com): > On Wednesday 12 August 2009 06:14:40 pm Serge E. Hallyn wrote: > > Quoting Paul Moore (paul.moore@hp.com): > > > +static int selinux_tun_dev_attach(struct sock *sk) > > > +{ > > > + struct sk_security_struct *sksec = sk->sk_security; > > > + u32 sid = current_sid(); > > > + int err; > > > + > > > + err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET, > > > + TUN_SOCKET__RELABELFROM, NULL); > > > + if (err) > > > + return err; > > > + err = avc_has_perm(sid, sid, SECCLASS_RAWIP_SOCKET, > > > > Was RAWIP on purpose here? > > Nope, a mistake on my part that I hadn't caught yet. Thanks. > > > > + TUN_SOCKET__RELABELTO, NULL); > > > + if (err) > > > + return err; > > > + > > > + sksec->sid = sid; > > > + > > > + return 0; > > > +} > > > > IIUC it is possible for multiple processes to attach to the same > > tun device. Will it get confusing/incorrect to have each attach > > potentially (if tasks have different sids) relabel? > > I may be reading the code wrong, but in drivers/net/tun.c:tun_attach() the > code checks to see if the TUN device is already in use and if it is then the > attach fails with -EBUSY (check where the tun_device->tfile is examined). I Ah yes, you're right - I saw the check for (ifr->ifr_flags & IFF_TUN_EXCL) in the attach path in tun_set_iff, and missed this one. > believe this should ensure that only one process at a time has access to the > TUN device so we shouldn't have to worry about a TUN socket getting relabeled > while it is currently in use. As far as persistent TUN devices getting > relabeled when a new process attaches to them, that is what we are trying to > accomplish here so that the network traffic being sent via the TUN device is > labeled according to the currently attached process; this is consistent with > how SELinux currently labels locally generated outbound traffic - outbound > packets inherit their security label from the sending process via the > originating socket/sock. Ok, thanks. To my untrained eye the class addition looks right too, so with the trivial change: Acked-by: Serge Hallyn <serue@us.ibm.com> thanks, -serge -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 15c2a08..fc7caa0 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -13,8 +13,8 @@ * Eric Paris <eparis@redhat.com> * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc. * <dgoeddel@trustedcs.com> - * Copyright (C) 2006, 2007 Hewlett-Packard Development Company, L.P. - * Paul Moore <paul.moore@hp.com> + * Copyright (C) 2006, 2007, 2009 Hewlett-Packard Development Company, L.P. + * Paul Moore <paul.moore@hp.com> * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. * Yuichi Nakamura <ynakam@hitachisoft.jp> * @@ -4296,6 +4296,59 @@ static void selinux_req_classify_flow(const struct request_sock *req, fl->secid = req->secid; } +static int selinux_tun_dev_create(void) +{ + u32 sid = current_sid(); + + /* we aren't taking into account the "sockcreate" SID since the socket + * that is being created here is not a socket in the traditional sense, + * instead it is a private sock, accessible only to the kernel, and + * representing a wide range of network traffic spanning multiple + * connections unlike traditional sockets - check the TUN driver to + * get a better understanding of why this socket is special */ + + return avc_has_perm(sid, sid, SECCLASS_TUN_SOCKET, TUN_SOCKET__CREATE, + NULL); +} + +static void selinux_tun_dev_post_create(struct sock *sk) +{ + struct sk_security_struct *sksec = sk->sk_security; + + /* we don't currently perform any NetLabel based labeling here and it + * isn't clear that we would want to do so anyway; while we could apply + * labeling without the support of the TUN user the resulting labeled + * traffic from the other end of the connection would almost certainly + * cause confusion to the TUN user that had no idea network labeling + * protocols were being used */ + + /* see the comments in selinux_tun_dev_create() about why we don't use + * the sockcreate SID here */ + + sksec->sid = current_sid(); + sksec->sclass = SECCLASS_TUN_SOCKET; +} + +static int selinux_tun_dev_attach(struct sock *sk) +{ + struct sk_security_struct *sksec = sk->sk_security; + u32 sid = current_sid(); + int err; + + err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET, + TUN_SOCKET__RELABELFROM, NULL); + if (err) + return err; + err = avc_has_perm(sid, sid, SECCLASS_RAWIP_SOCKET, + TUN_SOCKET__RELABELTO, NULL); + if (err) + return err; + + sksec->sid = sid; + + return 0; +} + static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb) { int err = 0; @@ -5464,6 +5517,9 @@ static struct security_operations selinux_ops = { .inet_csk_clone = selinux_inet_csk_clone, .inet_conn_established = selinux_inet_conn_established, .req_classify_flow = selinux_req_classify_flow, + .tun_dev_create = selinux_tun_dev_create, + .tun_dev_post_create = selinux_tun_dev_post_create, + .tun_dev_attach = selinux_tun_dev_attach, #ifdef CONFIG_SECURITY_NETWORK_XFRM .xfrm_policy_alloc_security = selinux_xfrm_policy_alloc, diff --git a/security/selinux/include/av_inherit.h b/security/selinux/include/av_inherit.h index 8377a4b..abedcd7 100644 --- a/security/selinux/include/av_inherit.h +++ b/security/selinux/include/av_inherit.h @@ -15,6 +15,7 @@ S_(SECCLASS_KEY_SOCKET, socket, 0x00400000UL) S_(SECCLASS_UNIX_STREAM_SOCKET, socket, 0x00400000UL) S_(SECCLASS_UNIX_DGRAM_SOCKET, socket, 0x00400000UL) + S_(SECCLASS_TUN_SOCKET, socket, 0x00400000UL) S_(SECCLASS_IPC, ipc, 0x00000200UL) S_(SECCLASS_SEM, ipc, 0x00000200UL) S_(SECCLASS_MSGQ, ipc, 0x00000200UL) diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h index d645192..0b41ad5 100644 --- a/security/selinux/include/av_permissions.h +++ b/security/selinux/include/av_permissions.h @@ -423,6 +423,28 @@ #define UNIX_DGRAM_SOCKET__RECV_MSG 0x00080000UL #define UNIX_DGRAM_SOCKET__SEND_MSG 0x00100000UL #define UNIX_DGRAM_SOCKET__NAME_BIND 0x00200000UL +#define TUN_SOCKET__IOCTL 0x00000001UL +#define TUN_SOCKET__READ 0x00000002UL +#define TUN_SOCKET__WRITE 0x00000004UL +#define TUN_SOCKET__CREATE 0x00000008UL +#define TUN_SOCKET__GETATTR 0x00000010UL +#define TUN_SOCKET__SETATTR 0x00000020UL +#define TUN_SOCKET__LOCK 0x00000040UL +#define TUN_SOCKET__RELABELFROM 0x00000080UL +#define TUN_SOCKET__RELABELTO 0x00000100UL +#define TUN_SOCKET__APPEND 0x00000200UL +#define TUN_SOCKET__BIND 0x00000400UL +#define TUN_SOCKET__CONNECT 0x00000800UL +#define TUN_SOCKET__LISTEN 0x00001000UL +#define TUN_SOCKET__ACCEPT 0x00002000UL +#define TUN_SOCKET__GETOPT 0x00004000UL +#define TUN_SOCKET__SETOPT 0x00008000UL +#define TUN_SOCKET__SHUTDOWN 0x00010000UL +#define TUN_SOCKET__RECVFROM 0x00020000UL +#define TUN_SOCKET__SENDTO 0x00040000UL +#define TUN_SOCKET__RECV_MSG 0x00080000UL +#define TUN_SOCKET__SEND_MSG 0x00100000UL +#define TUN_SOCKET__NAME_BIND 0x00200000UL #define PROCESS__FORK 0x00000001UL #define PROCESS__TRANSITION 0x00000002UL #define PROCESS__SIGCHLD 0x00000004UL diff --git a/security/selinux/include/class_to_string.h b/security/selinux/include/class_to_string.h index 21ec786..7ab9299 100644 --- a/security/selinux/include/class_to_string.h +++ b/security/selinux/include/class_to_string.h @@ -77,3 +77,4 @@ S_(NULL) S_(NULL) S_("kernel_service") + S_("tun_socket") diff --git a/security/selinux/include/flask.h b/security/selinux/include/flask.h index 882f27d..f248500 100644 --- a/security/selinux/include/flask.h +++ b/security/selinux/include/flask.h @@ -53,6 +53,7 @@ #define SECCLASS_PEER 68 #define SECCLASS_CAPABILITY2 69 #define SECCLASS_KERNEL_SERVICE 74 +#define SECCLASS_TUN_SOCKET 75 /* * Security identifier indices for initial entities
Add support for the new TUN LSM hooks: security_tun_dev_create(), security_tun_dev_post_create() and security_tun_dev_attach(). This includes the addition of a new object class, tun_socket, which represents the socks associated with TUN devices. The _tun_dev_create() and _tun_dev_post_create() hooks are fairly similar to the standard socket functions but _tun_dev_attach() is a bit special. The _tun_dev_attach() is unique because it involves a domain attaching to an existing TUN device and its associated tun_socket object, an operation which does not exist with standard sockets and most closely resembles a relabel operation. -- NOTE: This relies on some changes to the policy to add the new object class and its associated permissions, I will ensure that the policy is sorted and merged before pushing this patch upstream. Also, you will notice that the new tun_socket object class simply inherits the base socket object class, thoughts? --- security/selinux/hooks.c | 60 +++++++++++++++++++++++++++- security/selinux/include/av_inherit.h | 1 security/selinux/include/av_permissions.h | 22 ++++++++++ security/selinux/include/class_to_string.h | 1 security/selinux/include/flask.h | 1 5 files changed, 83 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html