* [patch 0/1] NetLabel bugfix for 2.6.19 @ 2006-10-30 18:03 ` paul.moore 0 siblings, 0 replies; 10+ messages in thread From: paul.moore @ 2006-10-30 18:03 UTC (permalink / raw) To: netdev, selinux; +Cc: jmorris, sds, eparis Sorry, but another bugfix patch for NetLabel which I think should be included 2.6.19. The problem is that the SELinux reference policy is a bit more free in allowing applications to call setsockopt() than I had originally thought, which means that normal applications could add or tamper with the NetLabel/CIPSO options on a socket causing all sorts of nastiness. This patch should solve these problems. -- paul moore linux security @ hp -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 0/1] NetLabel bugfix for 2.6.19 @ 2006-10-30 18:03 ` paul.moore 0 siblings, 0 replies; 10+ messages in thread From: paul.moore @ 2006-10-30 18:03 UTC (permalink / raw) To: netdev, selinux; +Cc: jmorris, sds, eparis Sorry, but another bugfix patch for NetLabel which I think should be included 2.6.19. The problem is that the SELinux reference policy is a bit more free in allowing applications to call setsockopt() than I had originally thought, which means that normal applications could add or tamper with the NetLabel/CIPSO options on a socket causing all sorts of nastiness. This patch should solve these problems. -- paul moore linux security @ hp ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() 2006-10-30 18:03 ` paul.moore @ 2006-10-30 18:03 ` paul.moore -1 siblings, 0 replies; 10+ messages in thread From: paul.moore @ 2006-10-30 18:03 UTC (permalink / raw) To: netdev, selinux; +Cc: jmorris, sds, eparis, Paul Moore From: Paul Moore <paul.moore@hp.com> This patch makes two changes to protect applications from either removing or tampering with the CIPSOv4 IP option on a socket. The first is the requirement that applications have the CAP_NET_RAW capability to set an IPOPT_CIPSO option on a socket; this prevents untrusted applications from setting their own CIPSOv4 security attributes on the packets they send. The second change is to SELinux and it prevents applications from setting any IPv4 options when there is an IPOPT_CIPSO option already present on the socket; this prevents applications from removing CIPSOv4 security attributes from the packets they send. Signed-off-by: Paul Moore <paul.moore@hp.com> --- net/ipv4/cipso_ipv4.c | 7 ++--- net/ipv4/ip_options.c | 2 - security/selinux/hooks.c | 8 +++++- security/selinux/include/selinux_netlabel.h | 10 +++++++ security/selinux/ss/services.c | 37 ++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) Index: net-2.6_sockopt/net/ipv4/cipso_ipv4.c =================================================================== --- net-2.6_sockopt.orig/net/ipv4/cipso_ipv4.c +++ net-2.6_sockopt/net/ipv4/cipso_ipv4.c @@ -1307,7 +1307,8 @@ int cipso_v4_socket_setattr(const struct /* We can't use ip_options_get() directly because it makes a call to * ip_options_get_alloc() which allocates memory with GFP_KERNEL and - * we can't block here. */ + * we won't always have CAP_NET_RAW even though we _always_ want to + * set the IPOPT_CIPSO option. */ opt_len = (buf_len + 3) & ~3; opt = kzalloc(sizeof(*opt) + opt_len, GFP_ATOMIC); if (opt == NULL) { @@ -1317,11 +1318,9 @@ int cipso_v4_socket_setattr(const struct memcpy(opt->__data, buf, buf_len); opt->optlen = opt_len; opt->is_data = 1; + opt->cipso = sizeof(struct iphdr); kfree(buf); buf = NULL; - ret_val = ip_options_compile(opt, NULL); - if (ret_val != 0) - goto socket_setattr_failure; sk_inet = inet_sk(sk); if (sk_inet->is_icsk) { Index: net-2.6_sockopt/net/ipv4/ip_options.c =================================================================== --- net-2.6_sockopt.orig/net/ipv4/ip_options.c +++ net-2.6_sockopt/net/ipv4/ip_options.c @@ -443,7 +443,7 @@ int ip_options_compile(struct ip_options opt->router_alert = optptr - iph; break; case IPOPT_CIPSO: - if (opt->cipso) { + if ((!skb && !capable(CAP_NET_RAW)) || opt->cipso) { pp_ptr = optptr; goto error; } Index: net-2.6_sockopt/security/selinux/hooks.c =================================================================== --- net-2.6_sockopt.orig/security/selinux/hooks.c +++ net-2.6_sockopt/security/selinux/hooks.c @@ -3313,7 +3313,13 @@ static int selinux_socket_getpeername(st static int selinux_socket_setsockopt(struct socket *sock,int level,int optname) { - return socket_has_perm(current, sock, SOCKET__SETOPT); + int err; + + err = socket_has_perm(current, sock, SOCKET__SETOPT); + if (err) + return err; + + return selinux_netlbl_socket_setsockopt(sock, level, optname); } static int selinux_socket_getsockopt(struct socket *sock, int level, Index: net-2.6_sockopt/security/selinux/include/selinux_netlabel.h =================================================================== --- net-2.6_sockopt.orig/security/selinux/include/selinux_netlabel.h +++ net-2.6_sockopt/security/selinux/include/selinux_netlabel.h @@ -53,6 +53,9 @@ void selinux_netlbl_sk_security_init(str void selinux_netlbl_sk_clone_security(struct sk_security_struct *ssec, struct sk_security_struct *newssec); int selinux_netlbl_inode_permission(struct inode *inode, int mask); +int selinux_netlbl_socket_setsockopt(struct socket *sock, + int level, + int optname); #else static inline void selinux_netlbl_cache_invalidate(void) { @@ -114,6 +117,13 @@ static inline int selinux_netlbl_inode_p { return 0; } + +static inline int selinux_netlbl_socket_setsockopt(struct socket *sock, + int level, + int optname) +{ + return 0; +} #endif /* CONFIG_NETLABEL */ #endif Index: net-2.6_sockopt/security/selinux/ss/services.c =================================================================== --- net-2.6_sockopt.orig/security/selinux/ss/services.c +++ net-2.6_sockopt/security/selinux/ss/services.c @@ -2682,4 +2682,41 @@ u32 selinux_netlbl_socket_getpeersec_dgr return peer_sid; } + +/** + * selinux_netlbl_socket_setsockopt - Do not allow users to remove a NetLabel + * @sock: the socket + * @level: the socket level or protocol + * @optname: the socket option name + * + * Description: + * Check the setsockopt() call and if the user is trying to replace the IP + * options on a socket and a NetLabel is in place for the socket deny the + * access; otherwise allow the access. Returns zero when the access is + * allowed, -EACCES when denied, and other negative values on error. + * + */ +int selinux_netlbl_socket_setsockopt(struct socket *sock, + int level, + int optname) +{ + int rc = 0; + struct inode *inode = SOCK_INODE(sock); + struct sk_security_struct *sksec = sock->sk->sk_security; + struct inode_security_struct *isec = inode->i_security; + struct netlbl_lsm_secattr secattr; + + mutex_lock(&isec->lock); + if (level == IPPROTO_IP && optname == IP_OPTIONS && + sksec->nlbl_state == NLBL_LABELED) { + netlbl_secattr_init(&secattr); + rc = netlbl_socket_getattr(sock, &secattr); + if (rc == 0 && (secattr.cache || secattr.mls_lvl_vld)) + rc = -EACCES; + netlbl_secattr_destroy(&secattr); + } + mutex_unlock(&isec->lock); + + return rc; +} #endif /* CONFIG_NETLABEL */ -- paul moore linux security @ hp -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() @ 2006-10-30 18:03 ` paul.moore 0 siblings, 0 replies; 10+ messages in thread From: paul.moore @ 2006-10-30 18:03 UTC (permalink / raw) To: netdev, selinux; +Cc: jmorris, sds, eparis, Paul Moore [-- Attachment #1: netlabel-sockopts --] [-- Type: text/plain, Size: 5732 bytes --] From: Paul Moore <paul.moore@hp.com> This patch makes two changes to protect applications from either removing or tampering with the CIPSOv4 IP option on a socket. The first is the requirement that applications have the CAP_NET_RAW capability to set an IPOPT_CIPSO option on a socket; this prevents untrusted applications from setting their own CIPSOv4 security attributes on the packets they send. The second change is to SELinux and it prevents applications from setting any IPv4 options when there is an IPOPT_CIPSO option already present on the socket; this prevents applications from removing CIPSOv4 security attributes from the packets they send. Signed-off-by: Paul Moore <paul.moore@hp.com> --- net/ipv4/cipso_ipv4.c | 7 ++--- net/ipv4/ip_options.c | 2 - security/selinux/hooks.c | 8 +++++- security/selinux/include/selinux_netlabel.h | 10 +++++++ security/selinux/ss/services.c | 37 ++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) Index: net-2.6_sockopt/net/ipv4/cipso_ipv4.c =================================================================== --- net-2.6_sockopt.orig/net/ipv4/cipso_ipv4.c +++ net-2.6_sockopt/net/ipv4/cipso_ipv4.c @@ -1307,7 +1307,8 @@ int cipso_v4_socket_setattr(const struct /* We can't use ip_options_get() directly because it makes a call to * ip_options_get_alloc() which allocates memory with GFP_KERNEL and - * we can't block here. */ + * we won't always have CAP_NET_RAW even though we _always_ want to + * set the IPOPT_CIPSO option. */ opt_len = (buf_len + 3) & ~3; opt = kzalloc(sizeof(*opt) + opt_len, GFP_ATOMIC); if (opt == NULL) { @@ -1317,11 +1318,9 @@ int cipso_v4_socket_setattr(const struct memcpy(opt->__data, buf, buf_len); opt->optlen = opt_len; opt->is_data = 1; + opt->cipso = sizeof(struct iphdr); kfree(buf); buf = NULL; - ret_val = ip_options_compile(opt, NULL); - if (ret_val != 0) - goto socket_setattr_failure; sk_inet = inet_sk(sk); if (sk_inet->is_icsk) { Index: net-2.6_sockopt/net/ipv4/ip_options.c =================================================================== --- net-2.6_sockopt.orig/net/ipv4/ip_options.c +++ net-2.6_sockopt/net/ipv4/ip_options.c @@ -443,7 +443,7 @@ int ip_options_compile(struct ip_options opt->router_alert = optptr - iph; break; case IPOPT_CIPSO: - if (opt->cipso) { + if ((!skb && !capable(CAP_NET_RAW)) || opt->cipso) { pp_ptr = optptr; goto error; } Index: net-2.6_sockopt/security/selinux/hooks.c =================================================================== --- net-2.6_sockopt.orig/security/selinux/hooks.c +++ net-2.6_sockopt/security/selinux/hooks.c @@ -3313,7 +3313,13 @@ static int selinux_socket_getpeername(st static int selinux_socket_setsockopt(struct socket *sock,int level,int optname) { - return socket_has_perm(current, sock, SOCKET__SETOPT); + int err; + + err = socket_has_perm(current, sock, SOCKET__SETOPT); + if (err) + return err; + + return selinux_netlbl_socket_setsockopt(sock, level, optname); } static int selinux_socket_getsockopt(struct socket *sock, int level, Index: net-2.6_sockopt/security/selinux/include/selinux_netlabel.h =================================================================== --- net-2.6_sockopt.orig/security/selinux/include/selinux_netlabel.h +++ net-2.6_sockopt/security/selinux/include/selinux_netlabel.h @@ -53,6 +53,9 @@ void selinux_netlbl_sk_security_init(str void selinux_netlbl_sk_clone_security(struct sk_security_struct *ssec, struct sk_security_struct *newssec); int selinux_netlbl_inode_permission(struct inode *inode, int mask); +int selinux_netlbl_socket_setsockopt(struct socket *sock, + int level, + int optname); #else static inline void selinux_netlbl_cache_invalidate(void) { @@ -114,6 +117,13 @@ static inline int selinux_netlbl_inode_p { return 0; } + +static inline int selinux_netlbl_socket_setsockopt(struct socket *sock, + int level, + int optname) +{ + return 0; +} #endif /* CONFIG_NETLABEL */ #endif Index: net-2.6_sockopt/security/selinux/ss/services.c =================================================================== --- net-2.6_sockopt.orig/security/selinux/ss/services.c +++ net-2.6_sockopt/security/selinux/ss/services.c @@ -2682,4 +2682,41 @@ u32 selinux_netlbl_socket_getpeersec_dgr return peer_sid; } + +/** + * selinux_netlbl_socket_setsockopt - Do not allow users to remove a NetLabel + * @sock: the socket + * @level: the socket level or protocol + * @optname: the socket option name + * + * Description: + * Check the setsockopt() call and if the user is trying to replace the IP + * options on a socket and a NetLabel is in place for the socket deny the + * access; otherwise allow the access. Returns zero when the access is + * allowed, -EACCES when denied, and other negative values on error. + * + */ +int selinux_netlbl_socket_setsockopt(struct socket *sock, + int level, + int optname) +{ + int rc = 0; + struct inode *inode = SOCK_INODE(sock); + struct sk_security_struct *sksec = sock->sk->sk_security; + struct inode_security_struct *isec = inode->i_security; + struct netlbl_lsm_secattr secattr; + + mutex_lock(&isec->lock); + if (level == IPPROTO_IP && optname == IP_OPTIONS && + sksec->nlbl_state == NLBL_LABELED) { + netlbl_secattr_init(&secattr); + rc = netlbl_socket_getattr(sock, &secattr); + if (rc == 0 && (secattr.cache || secattr.mls_lvl_vld)) + rc = -EACCES; + netlbl_secattr_destroy(&secattr); + } + mutex_unlock(&isec->lock); + + return rc; +} #endif /* CONFIG_NETLABEL */ -- paul moore linux security @ hp ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() 2006-10-30 18:03 ` paul.moore @ 2006-10-30 18:58 ` James Morris -1 siblings, 0 replies; 10+ messages in thread From: James Morris @ 2006-10-30 18:58 UTC (permalink / raw) To: Paul Moore; +Cc: netdev, selinux, jmorris, sds, eparis On Mon, 30 Oct 2006, paul.moore@hp.com wrote: > From: Paul Moore <paul.moore@hp.com> > > This patch makes two changes to protect applications from either removing or > tampering with the CIPSOv4 IP option on a socket. Thanks. Applied to: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-net-2.6.git -- James Morris <jmorris@namei.org> -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() @ 2006-10-30 18:58 ` James Morris 0 siblings, 0 replies; 10+ messages in thread From: James Morris @ 2006-10-30 18:58 UTC (permalink / raw) To: Paul Moore; +Cc: netdev, selinux, jmorris, sds, eparis On Mon, 30 Oct 2006, paul.moore@hp.com wrote: > From: Paul Moore <paul.moore@hp.com> > > This patch makes two changes to protect applications from either removing or > tampering with the CIPSOv4 IP option on a socket. Thanks. Applied to: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-net-2.6.git -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() 2006-10-30 18:03 ` paul.moore @ 2006-10-30 20:31 ` Eric Paris -1 siblings, 0 replies; 10+ messages in thread From: Eric Paris @ 2006-10-30 20:31 UTC (permalink / raw) To: paul.moore; +Cc: netdev, selinux, jmorris, sds On Mon, 2006-10-30 at 13:03 -0500, paul.moore@hp.com wrote: > plain text document attachment (netlabel-sockopts) > From: Paul Moore <paul.moore@hp.com> > > This patch makes two changes to protect applications from either removing or > tampering with the CIPSOv4 IP option on a socket. The first is the requirement > that applications have the CAP_NET_RAW capability to set an IPOPT_CIPSO option > on a socket; this prevents untrusted applications from setting their own > CIPSOv4 security attributes on the packets they send. The second change is to > SELinux and it prevents applications from setting any IPv4 options when there > is an IPOPT_CIPSO option already present on the socket; this prevents > applications from removing CIPSOv4 security attributes from the packets they > send. > --- net-2.6_sockopt.orig/security/selinux/ss/services.c > +++ net-2.6_sockopt/security/selinux/ss/services.c > @@ -2682,4 +2682,41 @@ u32 selinux_netlbl_socket_getpeersec_dgr > > return peer_sid; > } > + > +/** > + * selinux_netlbl_socket_setsockopt - Do not allow users to remove a NetLabel > + * @sock: the socket > + * @level: the socket level or protocol > + * @optname: the socket option name > + * > + * Description: > + * Check the setsockopt() call and if the user is trying to replace the IP > + * options on a socket and a NetLabel is in place for the socket deny the > + * access; otherwise allow the access. Returns zero when the access is > + * allowed, -EACCES when denied, and other negative values on error. > + * > + */ > +int selinux_netlbl_socket_setsockopt(struct socket *sock, > + int level, > + int optname) > +{ > + int rc = 0; > + struct inode *inode = SOCK_INODE(sock); > + struct sk_security_struct *sksec = sock->sk->sk_security; > + struct inode_security_struct *isec = inode->i_security; > + struct netlbl_lsm_secattr secattr; > + > + mutex_lock(&isec->lock); > + if (level == IPPROTO_IP && optname == IP_OPTIONS && > + sksec->nlbl_state == NLBL_LABELED) { > + netlbl_secattr_init(&secattr); > + rc = netlbl_socket_getattr(sock, &secattr); > + if (rc == 0 && (secattr.cache || secattr.mls_lvl_vld)) > + rc = -EACCES; > + netlbl_secattr_destroy(&secattr); > + } > + mutex_unlock(&isec->lock); > + > + return rc; > +} > #endif /* CONFIG_NETLABEL */ I probably should have ask this a while back but both here and in selinux_netlbl_socket_setsid() you are taking the isec->lock. As I originally understood the isec->lock it was just supposed to just prevent multiple tasks from initializing the isec information simultaneously while the isec information could be in an inconsistent state. After isec->initialized was set we didn't use the lock any more (as typically the only change inside the isec was the ->sid which couldn't even be 'in the middle'). I'm wondering what you are hoping to protect taking this lock. It doesn't seem like it would hurt anything, but i'd like to hear what you see it's purpose as being... (Then again it's just as unlikely that I understood what it was being used for pre-netlabel) -Eric -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() @ 2006-10-30 20:31 ` Eric Paris 0 siblings, 0 replies; 10+ messages in thread From: Eric Paris @ 2006-10-30 20:31 UTC (permalink / raw) To: paul.moore; +Cc: netdev, selinux, jmorris, sds On Mon, 2006-10-30 at 13:03 -0500, paul.moore@hp.com wrote: > plain text document attachment (netlabel-sockopts) > From: Paul Moore <paul.moore@hp.com> > > This patch makes two changes to protect applications from either removing or > tampering with the CIPSOv4 IP option on a socket. The first is the requirement > that applications have the CAP_NET_RAW capability to set an IPOPT_CIPSO option > on a socket; this prevents untrusted applications from setting their own > CIPSOv4 security attributes on the packets they send. The second change is to > SELinux and it prevents applications from setting any IPv4 options when there > is an IPOPT_CIPSO option already present on the socket; this prevents > applications from removing CIPSOv4 security attributes from the packets they > send. > --- net-2.6_sockopt.orig/security/selinux/ss/services.c > +++ net-2.6_sockopt/security/selinux/ss/services.c > @@ -2682,4 +2682,41 @@ u32 selinux_netlbl_socket_getpeersec_dgr > > return peer_sid; > } > + > +/** > + * selinux_netlbl_socket_setsockopt - Do not allow users to remove a NetLabel > + * @sock: the socket > + * @level: the socket level or protocol > + * @optname: the socket option name > + * > + * Description: > + * Check the setsockopt() call and if the user is trying to replace the IP > + * options on a socket and a NetLabel is in place for the socket deny the > + * access; otherwise allow the access. Returns zero when the access is > + * allowed, -EACCES when denied, and other negative values on error. > + * > + */ > +int selinux_netlbl_socket_setsockopt(struct socket *sock, > + int level, > + int optname) > +{ > + int rc = 0; > + struct inode *inode = SOCK_INODE(sock); > + struct sk_security_struct *sksec = sock->sk->sk_security; > + struct inode_security_struct *isec = inode->i_security; > + struct netlbl_lsm_secattr secattr; > + > + mutex_lock(&isec->lock); > + if (level == IPPROTO_IP && optname == IP_OPTIONS && > + sksec->nlbl_state == NLBL_LABELED) { > + netlbl_secattr_init(&secattr); > + rc = netlbl_socket_getattr(sock, &secattr); > + if (rc == 0 && (secattr.cache || secattr.mls_lvl_vld)) > + rc = -EACCES; > + netlbl_secattr_destroy(&secattr); > + } > + mutex_unlock(&isec->lock); > + > + return rc; > +} > #endif /* CONFIG_NETLABEL */ I probably should have ask this a while back but both here and in selinux_netlbl_socket_setsid() you are taking the isec->lock. As I originally understood the isec->lock it was just supposed to just prevent multiple tasks from initializing the isec information simultaneously while the isec information could be in an inconsistent state. After isec->initialized was set we didn't use the lock any more (as typically the only change inside the isec was the ->sid which couldn't even be 'in the middle'). I'm wondering what you are hoping to protect taking this lock. It doesn't seem like it would hurt anything, but i'd like to hear what you see it's purpose as being... (Then again it's just as unlikely that I understood what it was being used for pre-netlabel) -Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() 2006-10-30 20:31 ` Eric Paris @ 2006-10-30 20:58 ` Paul Moore -1 siblings, 0 replies; 10+ messages in thread From: Paul Moore @ 2006-10-30 20:58 UTC (permalink / raw) To: Eric Paris; +Cc: netdev, selinux, jmorris, sds Eric Paris wrote: > On Mon, 2006-10-30 at 13:03 -0500, paul.moore@hp.com wrote: > >>plain text document attachment (netlabel-sockopts) >>From: Paul Moore <paul.moore@hp.com> >> >>This patch makes two changes to protect applications from either removing or >>tampering with the CIPSOv4 IP option on a socket. The first is the requirement >>that applications have the CAP_NET_RAW capability to set an IPOPT_CIPSO option >>on a socket; this prevents untrusted applications from setting their own >>CIPSOv4 security attributes on the packets they send. The second change is to >>SELinux and it prevents applications from setting any IPv4 options when there >>is an IPOPT_CIPSO option already present on the socket; this prevents >>applications from removing CIPSOv4 security attributes from the packets they >>send. > > >>--- net-2.6_sockopt.orig/security/selinux/ss/services.c >>+++ net-2.6_sockopt/security/selinux/ss/services.c >>@@ -2682,4 +2682,41 @@ u32 selinux_netlbl_socket_getpeersec_dgr >> >> return peer_sid; >> } >>+ >>+/** >>+ * selinux_netlbl_socket_setsockopt - Do not allow users to remove a NetLabel >>+ * @sock: the socket >>+ * @level: the socket level or protocol >>+ * @optname: the socket option name >>+ * >>+ * Description: >>+ * Check the setsockopt() call and if the user is trying to replace the IP >>+ * options on a socket and a NetLabel is in place for the socket deny the >>+ * access; otherwise allow the access. Returns zero when the access is >>+ * allowed, -EACCES when denied, and other negative values on error. >>+ * >>+ */ >>+int selinux_netlbl_socket_setsockopt(struct socket *sock, >>+ int level, >>+ int optname) >>+{ >>+ int rc = 0; >>+ struct inode *inode = SOCK_INODE(sock); >>+ struct sk_security_struct *sksec = sock->sk->sk_security; >>+ struct inode_security_struct *isec = inode->i_security; >>+ struct netlbl_lsm_secattr secattr; >>+ >>+ mutex_lock(&isec->lock); >>+ if (level == IPPROTO_IP && optname == IP_OPTIONS && >>+ sksec->nlbl_state == NLBL_LABELED) { >>+ netlbl_secattr_init(&secattr); >>+ rc = netlbl_socket_getattr(sock, &secattr); >>+ if (rc == 0 && (secattr.cache || secattr.mls_lvl_vld)) >>+ rc = -EACCES; >>+ netlbl_secattr_destroy(&secattr); >>+ } >>+ mutex_unlock(&isec->lock); >>+ >>+ return rc; >>+} >> #endif /* CONFIG_NETLABEL */ > > I probably should have ask this a while back but both here and in > selinux_netlbl_socket_setsid() you are taking the isec->lock. As I > originally understood the isec->lock it was just supposed to just > prevent multiple tasks from initializing the isec information > simultaneously while the isec information could be in an inconsistent > state. After isec->initialized was set we didn't use the lock any more > (as typically the only change inside the isec was the ->sid which > couldn't even be 'in the middle'). I'm wondering what you are hoping to > protect taking this lock. It doesn't seem like it would hurt anything, > but i'd like to hear what you see it's purpose as being... (Then again > it's just as unlikely that I understood what it was being used for > pre-netlabel) Fair question. It was discussed a while back on either the SELinux or LSPP mailing list, I don't have a link handy but I'm sure you could find it with your favorite search engine (it was discussed back when it was a semaphore); I might be wrong but I think it was Stephen Smalley that suggested using the isec semaphore/mutex. Basically, inode_security_struct mutex also protects the sk_security_struct->nlbl_state field. You will see some cases where the nlbl_state field is set/checked without holding the mutex but in those cases only one thread should have access to the sock. Perhaps at some point it would make sense to protect this with some other locking mechanism (rcu might make the most sense) but considering the stage that 2.6.19 is at right now I think we should probably leave it alone right now. -- paul moore linux security @ hp -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() @ 2006-10-30 20:58 ` Paul Moore 0 siblings, 0 replies; 10+ messages in thread From: Paul Moore @ 2006-10-30 20:58 UTC (permalink / raw) To: Eric Paris; +Cc: netdev, selinux, jmorris, sds Eric Paris wrote: > On Mon, 2006-10-30 at 13:03 -0500, paul.moore@hp.com wrote: > >>plain text document attachment (netlabel-sockopts) >>From: Paul Moore <paul.moore@hp.com> >> >>This patch makes two changes to protect applications from either removing or >>tampering with the CIPSOv4 IP option on a socket. The first is the requirement >>that applications have the CAP_NET_RAW capability to set an IPOPT_CIPSO option >>on a socket; this prevents untrusted applications from setting their own >>CIPSOv4 security attributes on the packets they send. The second change is to >>SELinux and it prevents applications from setting any IPv4 options when there >>is an IPOPT_CIPSO option already present on the socket; this prevents >>applications from removing CIPSOv4 security attributes from the packets they >>send. > > >>--- net-2.6_sockopt.orig/security/selinux/ss/services.c >>+++ net-2.6_sockopt/security/selinux/ss/services.c >>@@ -2682,4 +2682,41 @@ u32 selinux_netlbl_socket_getpeersec_dgr >> >> return peer_sid; >> } >>+ >>+/** >>+ * selinux_netlbl_socket_setsockopt - Do not allow users to remove a NetLabel >>+ * @sock: the socket >>+ * @level: the socket level or protocol >>+ * @optname: the socket option name >>+ * >>+ * Description: >>+ * Check the setsockopt() call and if the user is trying to replace the IP >>+ * options on a socket and a NetLabel is in place for the socket deny the >>+ * access; otherwise allow the access. Returns zero when the access is >>+ * allowed, -EACCES when denied, and other negative values on error. >>+ * >>+ */ >>+int selinux_netlbl_socket_setsockopt(struct socket *sock, >>+ int level, >>+ int optname) >>+{ >>+ int rc = 0; >>+ struct inode *inode = SOCK_INODE(sock); >>+ struct sk_security_struct *sksec = sock->sk->sk_security; >>+ struct inode_security_struct *isec = inode->i_security; >>+ struct netlbl_lsm_secattr secattr; >>+ >>+ mutex_lock(&isec->lock); >>+ if (level == IPPROTO_IP && optname == IP_OPTIONS && >>+ sksec->nlbl_state == NLBL_LABELED) { >>+ netlbl_secattr_init(&secattr); >>+ rc = netlbl_socket_getattr(sock, &secattr); >>+ if (rc == 0 && (secattr.cache || secattr.mls_lvl_vld)) >>+ rc = -EACCES; >>+ netlbl_secattr_destroy(&secattr); >>+ } >>+ mutex_unlock(&isec->lock); >>+ >>+ return rc; >>+} >> #endif /* CONFIG_NETLABEL */ > > I probably should have ask this a while back but both here and in > selinux_netlbl_socket_setsid() you are taking the isec->lock. As I > originally understood the isec->lock it was just supposed to just > prevent multiple tasks from initializing the isec information > simultaneously while the isec information could be in an inconsistent > state. After isec->initialized was set we didn't use the lock any more > (as typically the only change inside the isec was the ->sid which > couldn't even be 'in the middle'). I'm wondering what you are hoping to > protect taking this lock. It doesn't seem like it would hurt anything, > but i'd like to hear what you see it's purpose as being... (Then again > it's just as unlikely that I understood what it was being used for > pre-netlabel) Fair question. It was discussed a while back on either the SELinux or LSPP mailing list, I don't have a link handy but I'm sure you could find it with your favorite search engine (it was discussed back when it was a semaphore); I might be wrong but I think it was Stephen Smalley that suggested using the isec semaphore/mutex. Basically, inode_security_struct mutex also protects the sk_security_struct->nlbl_state field. You will see some cases where the nlbl_state field is set/checked without holding the mutex but in those cases only one thread should have access to the sock. Perhaps at some point it would make sense to protect this with some other locking mechanism (rcu might make the most sense) but considering the stage that 2.6.19 is at right now I think we should probably leave it alone right now. -- paul moore linux security @ hp ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2006-10-30 20:58 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-10-30 18:03 [patch 0/1] NetLabel bugfix for 2.6.19 paul.moore 2006-10-30 18:03 ` paul.moore 2006-10-30 18:03 ` [patch 1/1] NetLabel: protect the CIPSOv4 socket option from setsockopt() paul.moore 2006-10-30 18:03 ` paul.moore 2006-10-30 18:58 ` James Morris 2006-10-30 18:58 ` James Morris 2006-10-30 20:31 ` Eric Paris 2006-10-30 20:31 ` Eric Paris 2006-10-30 20:58 ` Paul Moore 2006-10-30 20:58 ` Paul Moore
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.