strip_exif support for the OpenBSD sandbox

Change the strip_exif logic to work with the already existing OpenBSD
sandbox and allow ffmpeg and mogrify to be executed.

The previous strip_exif implementation relied on system(3), effectively
starting "/bin/sh" and executing the required tool within a shell
session. Making this work in the sandbox would require to allow
executing "/bin/sh", rendering the sandbox useless.

Thus, the code now starts determining the absolute path of the tools -
unless they are given as ffmpeg_path or mogrify_path - and allowing them
to be executed via unveil(2). Then, instead of the system(3) call, the
good old fork(2) and execve(2) dance is performed.

The sbox_enter code was made aware of strip_exif, which resulted in a
pledge(2) violation before when disable_email_notifications was set to
false. Furthermore, the detected paths of the tools are now allowed.

data.c

@@ -100,19 +100,12 @@ int srv_open(const char *basedir, int auto_upgrade)
 
                 if (auto_upgrade)
                     ret = snac_upgrade(&error);
-                else {
-                    if (xs_number_get(xs_dict_get(srv_config, "layout")) < disk_layout)
-                        error = xs_fmt("ERROR: disk layout changed - execute 'snac upgrade' first");
-                    else {
-                        if (!check_strip_tool()) {
-                            const char *mp = xs_dict_get(srv_config, "mogrify_path");
-                            if (mp == NULL) mp = "mogrify";
-                            error = xs_fmt("ERROR: strip_exif enabled but '%s' not found or not working (set 'mogrify_path' in server.json)", mp);
-                        }
-                        else
-                            ret = 1;
-                    }
-                }
+                else if (xs_number_get(xs_dict_get(srv_config, "layout")) < disk_layout)
+                    error = xs_fmt("ERROR: disk layout changed - execute 'snac upgrade' first");
+                else if (!check_strip_tool())
+                    error = xs_fmt("ERROR: strip_exif enabled but commands not found or working");
+                else
+                    ret = 1;
             }
 
         }

sandbox.c

@@ -13,6 +13,8 @@ void sbox_enter(const char *basedir)
         return;
     }
 
+    const xs_val *strip_exif = xs_dict_get(srv_config, "strip_exif");
+
     int smail;
     const char *url = xs_dict_get(srv_config, "smtp_url");
 
@@ -33,6 +35,11 @@ void sbox_enter(const char *basedir)
     if (*address == '/')
         unveil(address, "rwc");
 
+    if (strip_exif) {
+        unveil(xs_dict_get(srv_config, "ffmpeg_path"), "x");
+        unveil(xs_dict_get(srv_config, "mogrify_path"), "x");
+    }
+
     if (smail)
         unveil("/usr/sbin/sendmail",   "x");
 
@@ -45,7 +52,7 @@ void sbox_enter(const char *basedir)
     if (*address == '/')
         p = xs_str_cat(p, " unix");
 
-    if (smail)
+    if (smail || strip_exif)
         p = xs_str_cat(p, " exec");
 
     pledge(p, NULL);

snac.c

@@ -35,7 +35,9 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <limits.h>
+#include <stdio.h>
 #include <stdlib.h>
+#include <unistd.h>
 
 xs_str *srv_basedir = NULL;
 xs_dict *srv_config = NULL;
@@ -177,6 +179,46 @@ int check_password(const char *uid, const char *passwd, const char *hash)
 }
 
 
+char* findprog(const char *prog)
+/* find a prog in PATH and return the first match */
+{
+    char *path_env, *path, *dir, filename[PATH_MAX];
+    int len;
+    struct stat sbuf;
+
+    path_env = getenv("PATH");
+    if (!prog || !path_env)
+        return NULL;
+
+    path_env = strdup(path_env);
+    if (!path_env)
+        return NULL;
+    path = path_env;
+
+    while ((dir = strsep(&path, ":")) != NULL) {
+        /* empty entries as ./ instead of / */
+        if (*dir == '\0')
+            dir = ".";
+
+        /* strip trailing / */
+        len = strlen(dir);
+        while (len > 0 && dir[len-1] == '/')
+            dir[--len] = '\0';
+
+        len = snprintf(filename, sizeof(filename), "%s/%s", dir, prog);
+        if (len > 0 && len < (int) sizeof(filename) &&
+            (stat(filename, &sbuf) == 0) && S_ISREG(sbuf.st_mode) &&
+            access(filename, X_OK) == 0) {
+            free(path_env);
+            return strdup(filename);
+        }
+    }
+
+    free(path_env);
+    return NULL;
+}
+
+
 int strip_media(const char *fn)
 /* strips EXIF data from a file */
 {
@@ -207,23 +249,24 @@ int strip_media(const char *fn)
             xs_endswith(l_fn, ".gif")  || xs_endswith(l_fn, ".bmp")) {
 
             const char *mp = xs_dict_get(srv_config, "mogrify_path");
-            if (mp == NULL)
-                mp = "mogrify";
 
-            xs *cmd = xs_fmt("cd \"%s\" && %s -auto-orient -strip \"%s\" 2>/dev/null", srv_basedir, mp, r_fn);
-
-            ret = system(cmd);
+            pid_t pid = fork();
+            if (pid == -1) {
+                srv_log(xs_fmt("strip_media: cannot fork()"));
+                return -1;
+            } else if (pid == 0) {
+                chdir(srv_basedir);
+                execl(mp, "-auto-orient", "-strip", r_fn, (char*) NULL);
+                _exit(1);
+            }
 
-            if (ret != 0) {
-                int code = 0;
-                if (WIFEXITED(ret))
-                    code = WEXITSTATUS(ret);
-                
-                if (code == 127)
-                    srv_log(xs_fmt("strip_media: error stripping %s. '%s' not found (exit 127). Set 'mogrify_path' in server.json.", r_fn, mp));
-                else
-                    srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret));
+            if (waitpid(pid, &ret, 0) == -1) {
+                srv_log(xs_fmt("strip_media: cannot waitpid()"));
+                return -1;
             }
+
+            if (ret != 0)
+                srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret));
             else
                 srv_debug(1, xs_fmt("strip_media: stripped %s", r_fn));
         }
@@ -234,35 +277,33 @@ int strip_media(const char *fn)
             xs_endswith(l_fn, ".mkv") || xs_endswith(l_fn, ".avi")) {
 
             const char *fp = xs_dict_get(srv_config, "ffmpeg_path");
-            if (fp == NULL)
-                fp = "ffmpeg";
 
             /* ffmpeg cannot modify in-place, so we need a temp file */
             /* we must preserve valid extension for ffmpeg to guess the format */
             const char *ext = strrchr(r_fn, '.');
             if (ext == NULL) ext = "";
             xs *tmp_fn = xs_fmt("%s.tmp%s", r_fn, ext);
-            
-            /* -map_metadata -1 strips all global metadata */
-            /* -c copy copies input streams without re-encoding */
-            /* we don't silence stderr so we can debug issues */
-            /* we explicitly cd to srv_basedir to ensure relative paths work */
-            xs *cmd = xs_fmt("cd \"%s\" && %s -y -i \"%s\" -map_metadata -1 -c copy \"%s\"", srv_basedir, fp, r_fn, tmp_fn);
 
-            ret = system(cmd);
+            pid_t pid = fork();
+            if (pid == -1) {
+                srv_log(xs_fmt("strip_media: cannot fork()"));
+                return -1;
+            } else if (pid == 0) {
+                chdir(srv_basedir);
+                /* -map_metadata -1 strips all global metadata */
+                /* -c copy copies input streams without re-encoding */
+                execl(fp, "-y", "-i", r_fn, "-map_metadata", "-1", "-c", "copy", tmp_fn, (char*) NULL);
+                _exit(1);
+            }
+
+            if (waitpid(pid, &ret, 0) == -1) {
+                srv_log(xs_fmt("strip_media: cannot waitpid()"));
+                return -1;
+            }
 
             if (ret != 0) {
-                int code = 0;
-                if (WIFEXITED(ret))
-                    code = WEXITSTATUS(ret);
-                
-                if (code == 127)
-                    srv_log(xs_fmt("strip_media: error stripping %s. '%s' not found (exit 127). Set 'ffmpeg_path' in server.json.", r_fn, fp));
-                else {
-                    srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret));
-                    srv_log(xs_fmt("strip_media: command was: %s", cmd));
-                }
-                
+                srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret));
+
                 /* try to cleanup, just in case */
                 /* unlink needs full path too if we are not in basedir */
                 xs *full_tmp_fn = xs_fmt("%s/%s", srv_basedir, tmp_fn);
@@ -286,38 +327,37 @@ int strip_media(const char *fn)
 
 
 int check_strip_tool(void)
+/* check if strip_exif tools do exist and fix their absolute path */
 {
     const xs_val *v = xs_dict_get(srv_config, "strip_exif");
+    /* skip if unless strip_exif; return non-error */
+    if (xs_type(v) != XSTYPE_TRUE)
+        return 1;
+
     int ret = 1;
+    const char *progs[] = { "ffmpeg", "mogrify" };
 
-    if (xs_type(v) == XSTYPE_TRUE) {
-        /* check mogrify */
-        {
-            const char *mp = xs_dict_get(srv_config, "mogrify_path");
-            if (mp == NULL)
-                mp = "mogrify";
-
-            xs *cmd = xs_fmt("%s -version 2>/dev/null >/dev/null", mp);
-            
-            if (system(cmd) != 0) {
-                srv_log(xs_fmt("check_strip_tool: '%s' not working", mp));
-                ret = 0;
-            }
+    for (int i = 0; i < (int)(sizeof(progs) / sizeof(progs[0])); i++) {
+        xs_str *key = xs_fmt("%s_path", progs[i]);
+
+        const char *val = xs_dict_get(srv_config, key);
+        if (val == NULL) {
+            val = findprog(progs[i]);
+            if (val != NULL)
+                srv_debug(1, xs_fmt("check_strip_tool: found %s in PATH at %s", progs[i], val));
         }
 
-        /* check ffmpeg */
-        if (ret) {
-            const char *fp = xs_dict_get(srv_config, "ffmpeg_path");
-            if (fp == NULL)
-                fp = "ffmpeg";
-
-            xs *cmd = xs_fmt("%s -version 2>/dev/null >/dev/null", fp);
-            
-            if (system(cmd) != 0) {
-                srv_log(xs_fmt("check_strip_tool: '%s' not working", fp));
-                ret = 0;
-            }
+        if (val == NULL) {
+            srv_log(xs_fmt("check_strip_tool: %s not found in PATH", progs[i]));
+            ret = 0;
+        } else if (access(val, X_OK) != 0) {
+            srv_log(xs_fmt("check_strip_tool: %s '%s' is not executable", progs[i], val));
+            ret = 0;
+        } else {
+            srv_config = xs_dict_set(srv_config, key, val);
         }
+
+        xs_free(key);
     }
 
     return ret;

snac.h

@@ -107,6 +107,7 @@ int validate_uid(const char *uid);
 xs_str *hash_password(const char *uid, const char *passwd, const char *nonce);
 int check_password(const char *uid, const char *passwd, const char *hash);
 
+char* findprog(const char *prog);
 int strip_media(const char *fn);
 int check_strip_tool(void);