Merge pull request #5 from ipuustin/bug-fixes

Bug fixes.

Author: mythi

Makefile.am

@@ -3,10 +3,14 @@ groupcheck_SOURCES = groupcheck.c main.c
 groupcheck_CPPFLAGS = $(LIBSYSTEMD_CPPFLAGS)
 groupcheck_LDADD = $(LIBSYSTEMD_LIBS)
 
-noinst_PROGRAMS = test_groups test_directory
-test_groups_SOURCES = groupcheck.c test_groups.c
-test_groups_CPPFLAGS = $(LIBSYSTEMD_CPPFLAGS)
-test_groups_LDADD = $(LIBSYSTEMD_LIBS)
+noinst_PROGRAMS = test-process test-directory test-bus
+test_process_SOURCES = groupcheck.c test_process.c
+test_process_CPPFLAGS = $(LIBSYSTEMD_CPPFLAGS)
+test_process_LDADD = $(LIBSYSTEMD_LIBS)
+
+test_bus_SOURCES = groupcheck.c test_bus.c
+test_bus_CPPFLAGS = $(LIBSYSTEMD_CPPFLAGS)
+test_bus_LDADD = $(LIBSYSTEMD_LIBS)
 
 test_directory_SOURCES = groupcheck.c test_directory.c
 test_directory_CPPFLAGS = $(LIBSYSTEMD_CPPFLAGS)

configure.ac

@@ -1,4 +1,4 @@
-AC_INIT([groupcheck], 0.1)
+AC_INIT([groupcheck], 2.0)
 AM_INIT_AUTOMAKE
 AC_PROG_CC
 AC_CONFIG_FILES(Makefile)

groupcheck.c

@@ -31,20 +31,19 @@
 #define STAT_NAME_SIZE 32
 #define STAT_DATA_SIZE 256
 
-static int verify_start_time(struct subject *subject)
+int get_start_time(pid_t pid, uint64_t *start)
 {
-    /* Get the pid start time from /proc/stat and compare it with the value in
-     * the request. Return -1 if no match. */
+    /* Get the pid start time from /proc/stat. */
 
     char namebuf[STAT_NAME_SIZE];
     char databuf[STAT_DATA_SIZE];
     int r;
-    FILE *f;
+    FILE *f = NULL;
     char *p, *endp = NULL;
     int i;
     uint64_t start_time;
 
-    r = snprintf(namebuf, STAT_NAME_SIZE, "/proc/%d/stat", subject->data.p.pid);
+    r = snprintf(namebuf, STAT_NAME_SIZE, "/proc/%d/stat", pid);
     if (r < 0 || r >= STAT_NAME_SIZE)
         return -EINVAL;
 
@@ -54,28 +53,61 @@ static int verify_start_time(struct subject *subject)
         return -EINVAL;
 
     p = fgets(databuf, STAT_DATA_SIZE, f);
-    if (p == NULL)
-        return -EINVAL;
+    if (p == NULL) {
+        r = -EINVAL;
+        goto end;
+    }
 
     /* read the 22th field, which is the process start time in jiffies */
 
     /* skip over the "comm" field that has parentheses */
     p = strchr(p, ')');
 
-    if (*p == '\0')
-        return -EINVAL;
+    if (*p == '\0') {
+        r = -EINVAL;
+        goto end;
+    }
+    p++;
 
     /* That was the second field. Then skip over 19 more (20 spaces). */
 
     for (i = 0; i < 20; i++) {
         p = strchr(p, ' ');
-        if (*p == '\0')
-            return -EINVAL;
+        if (*p == '\0') {
+            r= -EINVAL;
+            goto end;
+        }
+        p++;
     }
 
     start_time = strtoul(p, &endp, 10);
-    if (endp != NULL)
-        return -EINVAL;
+
+    if (endp == NULL || *endp != ' ') {
+        r = -EINVAL;
+        goto end;
+    }
+
+    *start = start_time;
+    r = 0;
+
+end:
+    if (f)
+        fclose(f);
+
+    return r;
+}
+
+static int verify_start_time(struct subject *subject)
+{
+    int r;
+    uint64_t start_time = 0;
+
+    /* Compare pid start time with the value in the request. Return -1
+     * if no match. */
+
+    r = get_start_time(subject->data.p.pid, &start_time);
+    if (r < 0)
+        return r;
 
     if (start_time != subject->data.p.start_time)
         return -EINVAL;
@@ -93,7 +125,8 @@ bool check_allowed(sd_bus *bus, struct conf_data *conf_data,
     sd_bus_creds *creds = NULL;
     gid_t primary_gid;
     uint64_t mask = SD_BUS_CREDS_SUPPLEMENTARY_GIDS | SD_BUS_CREDS_AUGMENT
-            | SD_BUS_CREDS_PID | SD_BUS_CREDS_GID | SD_BUS_CREDS_UID;
+            | SD_BUS_CREDS_PID | SD_BUS_CREDS_GID | SD_BUS_CREDS_UID
+            | SD_BUS_CREDS_EUID;
     const gid_t *gids = NULL;
     int n_gids = 0;
     uid_t ruid, euid;
@@ -165,10 +198,8 @@ bool check_allowed(sd_bus *bus, struct conf_data *conf_data,
         break;
 
     case SUBJECT_KIND_SYSTEM_BUS_NAME:
-        if (bus == NULL) {
-            r = -EINVAL;
+        if (bus == NULL)
             goto end;
-        }
 
         r = sd_bus_get_name_creds(bus, subject->data.b.system_bus_name, mask, &creds);
         if (r < 0)
@@ -216,7 +247,7 @@ bool check_allowed(sd_bus *bus, struct conf_data *conf_data,
                 if (gids[j] == primary_gid) {
                     /* We only include supplementary gids in the check, not the
                        primary gid. This is to make it more difficult for
-                       processes to exec a setgid process to gain elevated
+                       processes to exec a setgid binary to gain elevated
                        group access. */
                        continue;
                 }
@@ -396,6 +427,22 @@ void print_decision(struct subject *subject, const char *action_id, bool allowed
     }
 }
 
+void print_config(struct conf_data *conf_data)
+{
+    int i, j;
+
+    if (conf_data == NULL)
+        return;
+
+    for (i = 0; i < conf_data->n_lines; i++) {
+        fprintf(stdout, "id: %s, groups: ", conf_data->lines[i].id);
+        for (j = 0; j < conf_data->lines[i].n_groups; j++) {
+            fprintf(stdout, "%s ", conf_data->lines[i].groups[j]);
+        }
+        fprintf(stdout, "\n");
+    }
+}
+
 static int method_check_authorization(sd_bus_message *m, void *userdata, sd_bus_error *ret_error)
 {
     int r;
@@ -598,7 +645,7 @@ static int property_backend_version(sd_bus *bus, const char *path,
         const char *interface, const char *property, sd_bus_message *reply,
         void *userdata, sd_bus_error *error)
 {
-    return sd_bus_message_append(reply, "s", "0.1");
+    return sd_bus_message_append(reply, "s", "2.0");
 }
 
 static int property_backend_features(sd_bus *bus, const char *path,
@@ -840,17 +887,3 @@ int load_directory(struct conf_data *conf_data, const char *dirname)
     closedir(dir);
     return r;
 }
-
-const char *find_policy_file()
-{
-    struct stat s;
-    const char *dynamic_conf = "/etc/groupcheck.policy";
-    const char *default_conf = "/usr/share/defaults/etc/groupcheck.policy";
-
-    if (stat(dynamic_conf, &s) == 0)
-        return dynamic_conf;
-    else if (stat(default_conf, &s) == 0)
-        return default_conf;
-
-    return NULL;
-}

groupcheck.h

@@ -69,13 +69,12 @@ struct subject {
  * parameters, "data" is an input parameter. */
 int initialize_bus(sd_bus **bus, sd_bus_slot **slot, struct conf_data *data);
 
-/* Return the policy file path from the search paths. */
-const char *find_policy_file();
-
 /* Load a policy file. The resulting struct must be freed by the caller. */
 int load_file(struct conf_data *conf_data, const char *filename);
 int load_directory(struct conf_data *conf_data, const char *filename);
 
 /* Exported for test programs. */
+int get_start_time(pid_t pid, uint64_t *start);
 void print_decision(struct subject *subject, const char *action_id, bool allowed);
+void print_config(struct conf_data *conf_data);
 bool check_allowed(sd_bus *bus, struct conf_data *conf_data, struct subject *subject, const char *action_id);

groupcheck.service

@@ -5,7 +5,7 @@ Description=groupcheck -- minimal polkit replacement
 User=groupcheck
 Type=dbus
 BusName=org.freedesktop.PolicyKit1
-ExecStart=/usr/sbin/groupcheck
+ExecStart=/usr/sbin/groupcheck -f /etc/groupcheck/groupcheck.policy
 
 [Install]
 WantedBy=multi-user.target

meson.build

@@ -2,7 +2,8 @@ project('groupcheck', 'c')
 
 systemd = dependency('libsystemd')
 
-executable('groupcheck', 'groupcheck.c', 'main.c', dependencies : systemd, install : true)
+executable('groupcheck', 'groupcheck.c', 'main.c', dependencies : systemd, install : true, install_dir : get_option('sbindir'))
 
-executable('test-groups', 'groupcheck.c', 'test_groups.c', dependencies : systemd)
+executable('test-process', 'groupcheck.c', 'test_process.c', dependencies : systemd)
+executable('test-bus', 'groupcheck.c', 'test_bus.c', dependencies : systemd)
 executable('test-directory', 'groupcheck.c', 'test_directory.c', dependencies : systemd)

test_bus.c

@@ -0,0 +1,162 @@
+/*
+ * groupcheck is a minimal polkit replacement for group-based authentication.
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <grp.h>
+#include <errno.h>
+
+#include <systemd/sd-bus.h>
+
+#include "groupcheck.h"
+
+int main(int argc, char *argv[])
+{
+    sd_bus *bus = NULL;
+    sd_bus_message *msg = NULL, *reply = NULL;
+    int r = -1, i;
+    const char *action_id;
+    const char *name = NULL;
+    bool *allowed;
+    gid_t supplementary_groups[argc];
+
+    if (argc < 2) {
+        fprintf(stderr, "Usage:\n\ttest_bus <action_id> [group1 group2 ...]\n");
+        return EXIT_FAILURE;
+    }
+
+    action_id = argv[1];
+
+    if (argc > 2) {
+        for (i = 0; i < argc-2; i++) {
+            struct group *grp;
+            grp = getgrnam(argv[i+2]);
+            if (grp == NULL) {
+                fprintf(stderr, "Error: group '%s' was not found.\n", argv[i+2]);
+                goto end;
+            }
+            supplementary_groups[i] = grp->gr_gid;
+        }
+        r = setgroups(argc-2, supplementary_groups);
+        if (r < 0) {
+            fprintf(stderr, "Error setting the supplementary groups: %s\n", strerror(errno));
+            goto end;
+        }
+    }
+
+    r = sd_bus_open_system(&bus);
+    if (r < 0) {
+        fprintf(stderr, "Error connecting to the system bus: %s\n", strerror(-r));
+        goto end;
+    }
+
+    r = sd_bus_get_unique_name(bus, &name);
+    if (r < 0) {
+        fprintf(stderr, "Error getting unique name: %s\n", strerror(-r));
+        goto end;
+    }
+
+    r = sd_bus_message_new_method_call(bus,
+            &msg,
+            "org.freedesktop.PolicyKit1",
+            "/org/freedesktop/PolicyKit1/Authority",
+            "org.freedesktop.PolicyKit1.Authority",
+            "CheckAuthorization");
+    if (r < 0) {
+        fprintf(stderr, "Error creating method call: %s\n", strerror(-r));
+        goto end;
+    }
+
+    r = sd_bus_message_open_container(msg, SD_BUS_TYPE_STRUCT, "sa{sv}");
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_append(msg, "s", "system-bus-name");
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_open_container(msg, SD_BUS_TYPE_ARRAY, "{sv}");
+    if (r < 0)
+        goto end;
+    
+    r = sd_bus_message_open_container(msg, SD_BUS_TYPE_DICT_ENTRY, "sv");
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_append(msg, "s", "name");
+    if (r < 0)
+        goto end;
+    
+    r = sd_bus_message_append(msg, "v", "s", name);
+    if (r < 0)
+        goto end;
+    
+    /* dict entry */
+    r = sd_bus_message_close_container(msg);
+    if (r < 0)
+        goto end;
+    
+    /* array */
+    r = sd_bus_message_close_container(msg);
+    if (r < 0)
+        goto end;
+
+    /* struct */
+    r = sd_bus_message_close_container(msg);
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_append(msg, "s", action_id);
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_append(msg, "a{ss}", 0, NULL);
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_append(msg, "u", 1);
+    if (r < 0)
+        goto end;
+
+    r = sd_bus_message_append(msg, "s", "");
+    if (r < 0)
+        goto end;
+    
+    r = sd_bus_call(bus, msg, 0, NULL, &reply);
+    if (r < 0) {
+        fprintf(stderr, "D-Bus method call failed: %s\n", strerror(-r));
+        goto end;
+    }
+
+    r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_STRUCT, "bba{ss}");
+    if (r < 0)
+        return r;
+
+    r = sd_bus_message_read(reply, "b", &allowed);
+    if (r < 0)
+        return r;
+
+    printf("Permission was %sgranted\n", allowed ? "" : "NOT ");
+
+end:
+
+    if (r < 0) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
+}