Separate socket SID than its creator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Stephen, James and Eric,

The attached is the v1 patches to compute socket SID on creation.

The propagation of the return code of security_transition_sid is a good practice and actually helps identifying the problem that the socket creator's role also needs to be able to type the socket role.

The class number(orig_tclass) used by SELinux kernel driver is checked against SECCLASS_XXX macros and if it's any one of those 22 socket classes both security_compute_sid and mls_comput_sid will preserve the scontext's role and MLS attribute as what they have been done for policydb.policy_class. (we are not checking access vectors so we don't have to concern about the unmapped, real class number allocated by checkpolicy)

I've also come up with a debug patch to make use of the security_sid_to_context which helps to reveal that the unix_dgram_socket object created by syslogd differs only in type from its creator and shares the same user, role and MLS at! tribute. The relevant changes to logging.pp used for debugging is also attached.

Please kindly help commenting and suggesting how I could further test these two patches.

Many thanks!

Best regards,
Harry


From 7fa2e0f6a6b537c5ff9f5586af35f3221db75de5 Mon Sep 17 00:00:00 2001
From: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
Date: Thu, 24 Feb 2011 13:10:13 +0800
Subject: [v1 PATCH 1/2] Compute SID for the newly created socket.

The security context for the newly created socket shares the same
user, role and MLS attribute as its creator but may have a different
type, which could be specified by a type_transition rule in the relevant
policy package.

Signed-off-by: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
---
 security/selinux/hooks.c |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c8d6992..f086541 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3644,9 +3644,15 @@ static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u16 family, u32 *sid)
 
 /* socket security operations */
 
-static u32 socket_sockcreate_sid(const struct task_security_struct *tsec)
+static int socket_sockcreate_sid(const struct task_security_struct *tsec,
+				 u16 secclass, u32 *socksid)
 {
-	return tsec->sockcreate_sid ? : tsec->sid;
+	if (tsec->sockcreate_sid > SECSID_NULL) {
+		*socksid = tsec->sockcreate_sid;
+		return 0;
+	}
+
+	return security_transition_sid(tsec->sid, tsec->sid, secclass, socksid);
 }
 
 static int sock_has_perm(struct task_struct *task, struct sock *sk, u32 perms)
@@ -3670,12 +3676,16 @@ static int selinux_socket_create(int family, int type,
 	const struct task_security_struct *tsec = current_security();
 	u32 newsid;
 	u16 secclass;
+	int rc;
 
 	if (kern)
 		return 0;
 
-	newsid = socket_sockcreate_sid(tsec);
 	secclass = socket_type_to_security_class(family, type, protocol);
+	rc = socket_sockcreate_sid(tsec, secclass, &newsid);
+	if (rc)
+		return rc;
+
 	return avc_has_perm(tsec->sid, newsid, secclass, SOCKET__CREATE, NULL);
 }
 
@@ -3687,12 +3697,16 @@ static int selinux_socket_post_create(struct socket *sock, int family,
 	struct sk_security_struct *sksec;
 	int err = 0;
 
+	isec->sclass = socket_type_to_security_class(family, type, protocol);
+
 	if (kern)
 		isec->sid = SECINITSID_KERNEL;
-	else
-		isec->sid = socket_sockcreate_sid(tsec);
+	else {
+		err = socket_sockcreate_sid(tsec, isec->sclass, &(isec->sid));
+		if (err)
+			return err;
+	}
 
-	isec->sclass = socket_type_to_security_class(family, type, protocol);
 	isec->initialized = 1;
 
 	if (sock->sk) {
-- 
1.7.0.4

From 2feb08d2c34cf61daebe56e9c9374059a9c832f2 Mon Sep 17 00:00:00 2001
From: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
Date: Sun, 27 Feb 2011 16:12:04 +0800
Subject: [v1 PATCH 2/2] Socket retains creator's user, role and MLS attribute.

The socket SID would be computed on creation and no longer inherit
its creator's SID by default. Socket may have a different type but
needs to retain the creator's role and MLS attribute in order not
to break labeled networking and network access control.

Signed-off-by: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
---
 security/selinux/ss/mls.c      |    5 ++-
 security/selinux/ss/mls.h      |    3 +-
 security/selinux/ss/services.c |   42 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 1ef8e4e..e961742 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -512,7 +512,8 @@ int mls_compute_sid(struct context *scontext,
 		    struct context *tcontext,
 		    u16 tclass,
 		    u32 specified,
-		    struct context *newcontext)
+		    struct context *newcontext,
+		    bool sock)
 {
 	struct range_trans rtr;
 	struct mls_range *r;
@@ -531,7 +532,7 @@ int mls_compute_sid(struct context *scontext,
 			return mls_range_set(newcontext, r);
 		/* Fallthrough */
 	case AVTAB_CHANGE:
-		if (tclass == policydb.process_class)
+		if ((tclass == policydb.process_class) || (sock == true))
 			/* Use the process MLS attributes. */
 			return mls_context_cpy(newcontext, scontext);
 		else
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index cd91526..037bf9d 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -49,7 +49,8 @@ int mls_compute_sid(struct context *scontext,
 		    struct context *tcontext,
 		    u16 tclass,
 		    u32 specified,
-		    struct context *newcontext);
+		    struct context *newcontext,
+		    bool sock);
 
 int mls_setup_user_range(struct context *fromcon, struct user_datum *user,
 			 struct context *usercon);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index a03cfaf..1adee26 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1343,6 +1343,42 @@ out:
 	return -EACCES;
 }
 
+static bool security_is_socket_class(u16 orig_tclass)
+{
+	bool sock = false;
+
+	switch (orig_tclass) {
+	case SECCLASS_SOCKET:
+	case SECCLASS_TCP_SOCKET:
+	case SECCLASS_UDP_SOCKET:
+	case SECCLASS_RAWIP_SOCKET:
+	case SECCLASS_NETLINK_SOCKET:
+	case SECCLASS_PACKET_SOCKET:
+	case SECCLASS_KEY_SOCKET:
+	case SECCLASS_UNIX_STREAM_SOCKET:
+	case SECCLASS_UNIX_DGRAM_SOCKET:
+	case SECCLASS_NETLINK_ROUTE_SOCKET:
+	case SECCLASS_NETLINK_FIREWALL_SOCKET:
+	case SECCLASS_NETLINK_TCPDIAG_SOCKET:
+	case SECCLASS_NETLINK_NFLOG_SOCKET:
+	case SECCLASS_NETLINK_XFRM_SOCKET:
+	case SECCLASS_NETLINK_SELINUX_SOCKET:
+	case SECCLASS_NETLINK_AUDIT_SOCKET:
+	case SECCLASS_NETLINK_IP6FW_SOCKET:
+	case SECCLASS_NETLINK_DNRT_SOCKET:
+	case SECCLASS_NETLINK_KOBJECT_UEVENT_SOCKET:
+	case SECCLASS_APPLETALK_SOCKET:
+	case SECCLASS_DCCP_SOCKET:
+	case SECCLASS_TUN_SOCKET:
+		sock = true;
+		break;
+	default:
+		break;
+	}
+
+	return sock;
+}
+
 static int security_compute_sid(u32 ssid,
 				u32 tsid,
 				u16 orig_tclass,
@@ -1357,6 +1393,7 @@ static int security_compute_sid(u32 ssid,
 	struct avtab_node *node;
 	u16 tclass;
 	int rc = 0;
+	bool sock = security_is_socket_class(orig_tclass);
 
 	if (!ss_initialized) {
 		switch (orig_tclass) {
@@ -1408,7 +1445,7 @@ static int security_compute_sid(u32 ssid,
 	}
 
 	/* Set the role and type to default values. */
-	if (tclass == policydb.process_class) {
+	if ((tclass == policydb.process_class) || (sock == true)) {
 		/* Use the current role and type of process. */
 		newcontext.role = scontext->role;
 		newcontext.type = scontext->type;
@@ -1460,7 +1497,8 @@ static int security_compute_sid(u32 ssid,
 
 	/* Set the MLS attributes.
 	   This is done last because it may allocate memory. */
-	rc = mls_compute_sid(scontext, tcontext, tclass, specified, &newcontext);
+	rc = mls_compute_sid(scontext, tcontext, tclass, specified,
+			     &newcontext, sock);
 	if (rc)
 		goto out_unlock;
 
-- 
1.7.0.4

From 045c025ed43ddb41a2f94aa6687ffd26a2e3529b Mon Sep 17 00:00:00 2001
From: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
Date: Sun, 27 Feb 2011 16:23:18 +0800
Subject: [PATCH 1/1] Debug separate socket SID.

With the help of this debug patch we can see that the socket created
by syslogd_t shares the same user, role and MLS attribute but has a
separate type:

[root/sysadm_r/s0@~]# getenforce
Enforcing
[root/sysadm_r/s0@~]# dmesg | tail -n 13
Pid: 397, comm: syslogd Not tainted 2.6.34.8 #9
Call Trace:
 [<c1247042>] socket_sockcreate_sid+0x52/0xc0
 [<c1247143>] selinux_socket_post_create+0x93/0x180
 [<c1240010>] security_socket_post_create+0x20/0x30
 [<c143bca2>] __sock_create+0x252/0x3d0
 [<c1072234>] ? audit_syscall_exit+0x2d4/0x300
 [<c143be87>] sock_create+0x37/0x50
 [<c143c0c6>] sys_socket+0x56/0xe0
 [<c143d27f>] sys_socketcall+0x8f/0x2f0
 [<c1525391>] system_call_done+0x0/0x4
tsec->sid: a9, context: system_u:system_r:syslogd_t:s15:c0.c1023
socksid: ad, context: system_u:system_r:syslogd_s_t:s15:c0.c1023
[root/sysadm_r/s0@~]#

Signed-off-by: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
---
 security/selinux/hooks.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f086541..8527a57 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3644,15 +3644,36 @@ static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u16 family, u32 *sid)
 
 /* socket security operations */
 
+void dump_stack(void);
+
 static int socket_sockcreate_sid(const struct task_security_struct *tsec,
 				 u16 secclass, u32 *socksid)
 {
+	int rc;
+	char *scontext;
+	u32 len;
+
 	if (tsec->sockcreate_sid > SECSID_NULL) {
 		*socksid = tsec->sockcreate_sid;
 		return 0;
 	}
 
-	return security_transition_sid(tsec->sid, tsec->sid, secclass, socksid);
+	rc = security_transition_sid(tsec->sid, tsec->sid, secclass, socksid);
+
+	if (rc == 0 && *socksid != tsec->sid) {
+		dump_stack();
+		security_sid_to_context(tsec->sid, &scontext, &len);
+		printk(KERN_INFO "tsec->sid: %x, context: %s\n",
+				  tsec->sid, scontext);
+		kfree(scontext);
+
+		security_sid_to_context(*socksid, &scontext, &len);
+		printk(KERN_INFO "socksid: %x, context: %s\n",
+				 *socksid, scontext);
+		kfree(scontext);
+	}
+
+	return rc;
 }
 
 static int sock_has_perm(struct task_struct *task, struct sock *sk, u32 perms)
-- 
1.7.0.4

From 44dd4c1ef2042ba6d5eb014b6c9826a07474953e Mon Sep 17 00:00:00 2001
From: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
Date: Thu, 24 Feb 2011 16:23:42 +0800
Subject: [PATCH 1/1] Specify a separate socket type for syslogd_t.

Use a type_transition rule to specify a separate type for unix_dgram_socket
object created by syslogd_t, so that the socket type alone could be added
to the mlstrustedobject attrbute to avoid below error message:

type=1400 audit(1298535101.654:868): avc:  denied  { sendto } for  pid=385 comm="klogd" path="/dev/log" scontext=system_u:object_r:klogd_t:s0 tcontext=system_u:object_r:syslogd_t:s15:c0.c1023 tclass=unix_dgram_socket

This helps to avoid adding syslogd_t to this attribute which also is the
label for all syslogd's procfs contents.

BTW, in SELinux kernel driver the security_transition_sid() should be
called to query above type_transition rule for the newly created socket,
which will retain the same user, role and MLS attribute as its creator.

Signed-off-by: Harry Ciao <qingtao.cao@xxxxxxxxxxxxx>
---
 policy/modules/system/logging.if |    4 ++--
 policy/modules/system/logging.te |   14 ++++++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/policy/modules/system/logging.if b/policy/modules/system/logging.if
index c7cfb62..92582b0 100644
--- a/policy/modules/system/logging.if
+++ b/policy/modules/system/logging.if
@@ -525,14 +525,14 @@ interface(`logging_log_filetrans',`
 #
 interface(`logging_send_syslog_msg',`
 	gen_require(`
-		type syslogd_t, devlog_t;
+		type syslogd_t, syslogd_s_t, devlog_t;
 	')
 
 	allow $1 devlog_t:lnk_file read_lnk_file_perms;
 	allow $1 devlog_t:sock_file write_sock_file_perms;
 
 	# the type of socket depends on the syslog daemon
-	allow $1 syslogd_t:unix_dgram_socket sendto;
+	allow $1 syslogd_s_t:unix_dgram_socket sendto;
 	allow $1 syslogd_t:unix_stream_socket connectto;
 	allow $1 self:unix_dgram_socket create_socket_perms;
 	allow $1 self:unix_stream_socket create_socket_perms;
diff --git a/policy/modules/system/logging.te b/policy/modules/system/logging.te
index 9b5a9ed..5bdedcb 100644
--- a/policy/modules/system/logging.te
+++ b/policy/modules/system/logging.te
@@ -61,6 +61,13 @@ type syslogd_t;
 type syslogd_exec_t;
 init_daemon_domain(syslogd_t, syslogd_exec_t)
 
+# PF_UNIX socket created by syslogd.
+# Any socket will retain the same user, role and MLS attribute as
+# its creator, thus the creator's role needs to type the socket type.
+type syslogd_s_t;
+role system_r types syslogd_s_t;
+mls_trusted_object(syslogd_s_t)
+
 type syslogd_initrc_exec_t;
 init_script_file(syslogd_initrc_exec_t)
 
@@ -344,15 +351,18 @@ dontaudit syslogd_t self:capability sys_tty_config;
 # setrlimit for syslog-ng
 allow syslogd_t self:process { signal_perms setpgid setrlimit };
 # receive messages to be logged
-allow syslogd_t self:unix_dgram_socket create_socket_perms;
+allow syslogd_t syslogd_s_t:unix_dgram_socket create_socket_perms;
 allow syslogd_t self:unix_stream_socket create_stream_socket_perms;
-allow syslogd_t self:unix_dgram_socket sendto;
+allow syslogd_t syslogd_s_t:unix_dgram_socket sendto;
 allow syslogd_t self:fifo_file rw_fifo_file_perms;
 allow syslogd_t self:udp_socket create_socket_perms;
 allow syslogd_t self:tcp_socket create_stream_socket_perms;
 
 allow syslogd_t syslog_conf_t:file read_file_perms;
 
+# PF_UNIX dgram socket created by syslogd_t labeled as syslogd_s_t
+type_transition syslogd_t syslogd_t:unix_dgram_socket syslogd_s_t;
+
 # Create and bind to /dev/log or /var/run/log.
 allow syslogd_t devlog_t:sock_file manage_sock_file_perms;
 files_pid_filetrans(syslogd_t, devlog_t, sock_file)
-- 
1.7.0.4


[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux