p11-kit server socket permission question

[Benjamin Herrenschmidt]

Hi Folks !

I noticed p11-kit server does an unconditional umask(066) when creating
the server socket.

This means that even when specifying a group, the socket ends up
without group read or group write permission (funnily enough it does
have execute permission) for that group.

So we do end up doing a "chown()" to set the group of the socket, but
we still can't access it from this user or group unless it's
explicitely chmod'd.

Is this intentional ?

Additionally, we do this:

	if (uid != -1 && gid != -1) {
		rc = chown (socket_file, uid, gid);

Shouldn't this be a || instead of && ? Ie, allow to set either uid, gid
or both ? Right now if I specify only one, we don't get a chown at all.

Would this (yet untested) patch make sense ? I'll be testing it later
this week and send a patch here or a github PR, whatever is preferred,
but first maybe let me know if I'm completely off here ?

diff --git a/p11-kit/server.c b/p11-kit/server.c
index ce5bee3..3e0206b 100644
--- a/p11-kit/server.c
+++ b/p11-kit/server.c
@@ -60,6 +60,7 @@
 #include <sys/types.h>
 #include <sys/un.h>
 #include <sys/wait.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
 #ifdef WITH_SYSTEMD
@@ -296,6 +297,7 @@ create_unix_socket (const char *address,
        int rc, sd;
        struct sockaddr_un sa;
        const char *socket_file;
+       mode_t socket_mask;
 
        memset (&sa, 0, sizeof(sa));
        sa.sun_family = AF_UNIX;
@@ -312,7 +314,10 @@ create_unix_socket (const char *address,
                return -1;
        }
 
-       umask (066);
+       socket_mask = S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
+       if (gid != -1)
+               socket_mask &= ~(S_IRGRP | S_IWGRP);
+       umask (socket_mask);
        rc = bind (sd, (struct sockaddr *)&sa, SUN_LEN (&sa));
        if (rc == -1) {
                close (sd);
@@ -327,7 +332,7 @@ create_unix_socket (const char *address,
                return 1;
        }
 
-       if (uid != -1 && gid != -1) {
+       if (uid != -1 || gid != -1) {
                rc = chown (socket_file, uid, gid);
                if (rc == -1) {
                        close (sd);