Discussion:
[PATCH v2 4/4] smack: provide socketpair callback
David Herrmann
2018-05-04 14:28:22 UTC
Permalink
From: Tom Gundersen <***@jklm.no>

Make sure to implement the new socketpair callback so the SO_PEERSEC
call on socketpair(2)s will return correct information.

Signed-off-by: Tom Gundersen <***@jklm.no>
Signed-off-by: David Herrmann <***@gmail.com>
---
security/smack/smack_lsm.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0b414836bebd..dcb976f98df2 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2842,6 +2842,27 @@ static int smack_socket_post_create(struct socket *sock, int family,
return smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET);
}

+/**
+ * smack_socket_socketpair - create socket pair
+ * @socka: one socket
+ * @sockb: another socket
+ *
+ * Cross reference the peer labels for SO_PEERSEC
+ *
+ * Returns 0 on success, and error code otherwise
+ */
+static int smack_socket_socketpair(struct socket *socka,
+ struct socket *sockb)
+{
+ struct socket_smack *asp = socka->sk->sk_security;
+ struct socket_smack *bsp = sockb->sk->sk_security;
+
+ asp->smk_packet = bsp->smk_out;
+ bsp->smk_packet = asp->smk_out;
+
+ return 0;
+}
+
#ifdef SMACK_IPV6_PORT_LABELING
/**
* smack_socket_bind - record port binding information.
@@ -4724,6 +4745,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(unix_may_send, smack_unix_may_send),

LSM_HOOK_INIT(socket_post_create, smack_socket_post_create),
+ LSM_HOOK_INIT(socket_socketpair, smack_socket_socketpair),
#ifdef SMACK_IPV6_PORT_LABELING
LSM_HOOK_INIT(socket_bind, smack_socket_bind),
#endif
--
2.17.0
David Herrmann
2018-05-04 14:28:19 UTC
Permalink
Right now the LSM labels for socketpairs are always uninitialized,
since there is no security hook for the socketpair() syscall. This
patch adds the required hooks so LSMs can properly label socketpairs.
This allows SO_PEERSEC to return useful information on those sockets.

Note that the behavior of socketpair() can be emulated by creating a
listener socket, connecting to it, and then discarding the initial
listener socket. With this workaround, SO_PEERSEC would return the
caller's security context. However, with socketpair(), the uninitialized
context is returned unconditionally. This is unexpected and makes
socketpair() less useful in situations where the security context is
crucial to the application.

With the new socketpair-hook this disparity can be solved by making
socketpair() return the expected security context.

Acked-by: Serge Hallyn <***@hallyn.com>
Signed-off-by: Tom Gundersen <***@jklm.no>
Signed-off-by: David Herrmann <***@gmail.com>
---
include/linux/lsm_hooks.h | 7 +++++++
include/linux/security.h | 7 +++++++
security/security.c | 6 ++++++
3 files changed, 20 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9d0b286f3dba..8f1131c8dd54 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -757,6 +757,11 @@
* @type contains the requested communications type.
* @protocol contains the requested protocol.
* @kern set to 1 if a kernel socket.
+ * @socket_socketpair:
+ * Check permissions before creating a fresh pair of sockets.
+ * @socka contains the first socket structure.
+ * @sockb contains the second socket structure.
+ * Return 0 if permission is granted and the connection was established.
* @socket_bind:
* Check permission before socket protocol layer bind operation is
* performed and the socket @sock is bound to the address specified in the
@@ -1656,6 +1661,7 @@ union security_list_options {
int (*socket_create)(int family, int type, int protocol, int kern);
int (*socket_post_create)(struct socket *sock, int family, int type,
int protocol, int kern);
+ int (*socket_socketpair)(struct socket *socka, struct socket *sockb);
int (*socket_bind)(struct socket *sock, struct sockaddr *address,
int addrlen);
int (*socket_connect)(struct socket *sock, struct sockaddr *address,
@@ -1922,6 +1928,7 @@ struct security_hook_heads {
struct hlist_head unix_may_send;
struct hlist_head socket_create;
struct hlist_head socket_post_create;
+ struct hlist_head socket_socketpair;
struct hlist_head socket_bind;
struct hlist_head socket_connect;
struct hlist_head socket_listen;
diff --git a/include/linux/security.h b/include/linux/security.h
index 200920f521a1..4ff3ba457e56 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1191,6 +1191,7 @@ int security_unix_may_send(struct socket *sock, struct socket *other);
int security_socket_create(int family, int type, int protocol, int kern);
int security_socket_post_create(struct socket *sock, int family,
int type, int protocol, int kern);
+int security_socket_socketpair(struct socket *socka, struct socket *sockb);
int security_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen);
int security_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen);
int security_socket_listen(struct socket *sock, int backlog);
@@ -1262,6 +1263,12 @@ static inline int security_socket_post_create(struct socket *sock,
return 0;
}

+static inline int security_socket_socketpair(struct socket *socka,
+ struct socket *sockb)
+{
+ return 0;
+}
+
static inline int security_socket_bind(struct socket *sock,
struct sockaddr *address,
int addrlen)
diff --git a/security/security.c b/security/security.c
index 7bc2fde023a7..68f46d849abe 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1358,6 +1358,12 @@ int security_socket_post_create(struct socket *sock, int family,
protocol, kern);
}

+int security_socket_socketpair(struct socket *socka, struct socket *sockb)
+{
+ return call_int_hook(socket_socketpair, 0, socka, sockb);
+}
+EXPORT_SYMBOL(security_socket_socketpair);
+
int security_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
{
return call_int_hook(socket_bind, 0, sock, address, addrlen);
--
2.17.0
David Herrmann
2018-05-04 14:28:20 UTC
Permalink
Use the newly created LSM-hook for socketpair(). The default hook
return-value is 0, so behavior stays the same unless LSMs start using
this hook.

Acked-by: Serge Hallyn <***@hallyn.com>
Signed-off-by: Tom Gundersen <***@jklm.no>
Signed-off-by: David Herrmann <***@gmail.com>
---
net/socket.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/net/socket.c b/net/socket.c
index f10f1d947c78..667a7b397134 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1420,6 +1420,13 @@ int __sys_socketpair(int family, int type, int protocol, int __user *usockvec)
goto out;
}

+ err = security_socket_socketpair(sock1, sock2);
+ if (unlikely(err)) {
+ sock_release(sock2);
+ sock_release(sock1);
+ goto out;
+ }
+
err = sock1->ops->socketpair(sock1, sock2);
if (unlikely(err < 0)) {
sock_release(sock2);
--
2.17.0
David Miller
2018-05-04 17:31:08 UTC
Permalink
From: David Herrmann <***@gmail.com>
Date: Fri, 4 May 2018 16:28:20 +0200
Post by David Herrmann
Use the newly created LSM-hook for socketpair(). The default hook
return-value is 0, so behavior stays the same unless LSMs start using
this hook.
Acked-by: David S. Miller <***@davemloft.net>
David Herrmann
2018-05-04 14:28:21 UTC
Permalink
Make sure to implement the new socketpair callback so the SO_PEERSEC
call on socketpair(2)s will return correct information.

Acked-by: Serge Hallyn <***@hallyn.com>
Acked-by: Stephen Smalley <***@tycho.nsa.gov>
Signed-off-by: Tom Gundersen <***@jklm.no>
Signed-off-by: David Herrmann <***@gmail.com>
---
security/selinux/hooks.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4cafe6a19167..02ebd1585eaf 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4569,6 +4569,18 @@ static int selinux_socket_post_create(struct socket *sock, int family,
return err;
}

+static int selinux_socket_socketpair(struct socket *socka,
+ struct socket *sockb)
+{
+ struct sk_security_struct *sksec_a = socka->sk->sk_security;
+ struct sk_security_struct *sksec_b = sockb->sk->sk_security;
+
+ sksec_a->peer_sid = sksec_b->sid;
+ sksec_b->peer_sid = sksec_a->sid;
+
+ return 0;
+}
+
/* Range of port numbers used to automatically bind.
Need to determine whether we should perform a name_bind
permission check between the socket and the port number. */
@@ -6999,6 +7011,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {

LSM_HOOK_INIT(socket_create, selinux_socket_create),
LSM_HOOK_INIT(socket_post_create, selinux_socket_post_create),
+ LSM_HOOK_INIT(socket_socketpair, selinux_socket_socketpair),
LSM_HOOK_INIT(socket_bind, selinux_socket_bind),
LSM_HOOK_INIT(socket_connect, selinux_socket_connect),
LSM_HOOK_INIT(socket_listen, selinux_socket_listen),
--
2.17.0
James Morris
2018-05-04 19:51:43 UTC
Permalink
Hi
This is v2 of the socketpair(2) LSM hook introduction.
Thanks, all applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-general
--
James Morris
<***@namei.org>
Casey Schaufler
2018-05-04 22:01:19 UTC
Permalink
Post by David Herrmann
Make sure to implement the new socketpair callback so the SO_PEERSEC
call on socketpair(2)s will return correct information.
This doesn't look like it will cause any problems.
I've only been able to test it in a general way. I
haven't created specific tests, but it passes the
usual Smack use cases.
Post by David Herrmann
---
security/smack/smack_lsm.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0b414836bebd..dcb976f98df2 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2842,6 +2842,27 @@ static int smack_socket_post_create(struct socket *sock, int family,
return smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET);
}
+/**
+ * smack_socket_socketpair - create socket pair
+ *
+ * Cross reference the peer labels for SO_PEERSEC
+ *
+ * Returns 0 on success, and error code otherwise
+ */
+static int smack_socket_socketpair(struct socket *socka,
+ struct socket *sockb)
+{
+ struct socket_smack *asp = socka->sk->sk_security;
+ struct socket_smack *bsp = sockb->sk->sk_security;
+
+ asp->smk_packet = bsp->smk_out;
+ bsp->smk_packet = asp->smk_out;
+
+ return 0;
+}
+
#ifdef SMACK_IPV6_PORT_LABELING
/**
* smack_socket_bind - record port binding information.
@@ -4724,6 +4745,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(unix_may_send, smack_unix_may_send),
LSM_HOOK_INIT(socket_post_create, smack_socket_post_create),
+ LSM_HOOK_INIT(socket_socketpair, smack_socket_socketpair),
#ifdef SMACK_IPV6_PORT_LABELING
LSM_HOOK_INIT(socket_bind, smack_socket_bind),
#endif
Loading...