Use target process context to set socket context

[tedx]

Use target process context to set socket context when using SELinuxContextFromNet not systemds context. Currently when using the SELimuxContextFromNet option for a socket activated services systemd calls getcon_raw which returns init_t and uses the resulting context to compute the context to be passed to the setsockcreatecon call. A socket of type init_t is created and listened on and this means that SELinux policy cannot be written to control which processes (SELinux types) can connect to the socket since the ref policy allows all 'types' to connect to sockets of the type init_t. When security accessors see that any process can connect to a socket this raises serious concerns. I have spoken with SELinux contributors in person and on the mailing list and the consensus is that the best solution is to use the target executables context when computing the sockets context in all cases.

[cgzones]

mac_selinux_get_our_label() seems indeed bogus.

Additionally socket_determine_selinux_label() should, before computing the context based on ExecStart=, consider the SELinuxContext= setting of the associated service.

Untested suggestion:

diff --git a/src/core/socket.c b/src/core/socket.c
index 6e4718e998..cbf1cc38bc 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1411,6 +1411,7 @@ static int socket_determine_selinux_label(Socket *s, char **ret) {
 
         Unit *service;
         ExecCommand *c;
+        const char *exec_context;
         _cleanup_free_ char *path = NULL;
 
         r = socket_load_service_unit(s, -1, &service);
@@ -1419,6 +1420,12 @@ static int socket_determine_selinux_label(Socket *s, char **ret) {
         if (r < 0)
                 return r;
 
+        exec_context = SERVICE(service)->exec_context.selinux_context;
+        if (exec_context) {
+                *ret = strdup(exec_context);
+                return *ret ? 0 : -ENOMEM;
+        }
+
         c = SERVICE(service)->exec_command[SERVICE_EXEC_START];
         if (!c)
                 goto no_label;

Also maybe the documentation can be more clear about what context is affected:

diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 1f1f99800f..86afebe518 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -624,7 +624,8 @@
          <listitem><para>Takes a boolean argument. When true, systemd
          will attempt to figure out the SELinux label used for the
          instantiated service from the information handed by the peer
-         over the network. Note that only the security level is used
+        over the network. Note that the SELinux label of the socket
+        itself is not affected and that only the security level is used
          from the information provided by the peer. Other parts of the
          resulting SELinux context originate from either the target
          binary that is effectively triggered by socket unit or from

[poettering]

This code was originally added by @msekletar in 16115b0a7b7cdf08fb38084d857d572d8a9088dc

@msekletar any comment?

[poettering]

(this will need review from some selinux folks really, i don't grok the effect of this)

[karlwmacmillan]

I think I can count as an selinux person and discussed this approach with Ted. Conceptually this makes sense to me (labelling the sockets based on the context of the target process). I agree with @cgzones that it should respect SELinuxContext=. Basically, whatever the final context of the process will be. I don't have enough knowledge of systemd internals to really dig into the implementation unfortunately. But I did look at the patch and it seems to make sense.

My only question about this is around the permissions model. With socket activation, systemd is potentially performing privileged actions on behalf of these network daemons (binding and listening on sockets for example). When the sockets were labeled init_t the policy, in a non-ideal way, reflected that these processes were interacting with systemd in this way. After this change that will no longer be the case. I think the other network controls should kick in and allow whatever control is necessary (mainly the packet permissions or network ingress / egress). But just noting that change.

[cgzones]

With this change there is still one limitation: The original intend for the SELimuxContextFromNet= option was to set the MLS level according to the peer connecting to the socket. Since systemd creates the socket a priory, no information of the future peer is known, so the socket gets the default level, and systemd does not (currently) update the sockets security context in the event of an incoming connection.

[poettering]

+        exec_context = SERVICE(service)->exec_context.selinux_context;
+        if (exec_context) {
+                *ret = strdup(exec_context);
+                return *ret ? 0 : -ENOMEM;
+        }

Plesae submit a separate patch for that.

Note that as per coding style you should strdup() this first into a separate var, and only asssign to *ret on success. We have this rule that all our functions should leave return params untouched on failure, and initialize all on success.

[yuwata]

#24801 is merged. Please rebase this PR if still relevant.

[tedx]

I inadvertently did a sync on my fork which closed this PR and I'm not sure how to recover and reopen this request :(

[yuwata]

Please create new one.