common: Use secure_getenv() implementation when setuid

In anything security sensitive, use secure_getenv() implementation
for retrieving environment variables.

diff --git a/common/Makefile.am b/common/Makefile.am
index 47162ddb7a88419c13fa3b272f36d54c3cc6e40b..b053ec0df21a87fe288f1579c5b0cf722e7c68e9 100644 (file)
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -99,7 +99,13 @@ test_tests_LDADD = $(common_LIBS)
 test_url_SOURCES = common/test-url.c
 test_url_LDADD = $(common_LIBS)
 
-noinst_PROGRAMS += frob-getauxval
+noinst_PROGRAMS += \
+       frob-getauxval \
+       frob-getenv \
+       $(NULL)
 
 frob_getauxval_SOURCES = common/frob-getauxval.c
 frob_getauxval_LDADD = $(common_LIBS)
+
+frob_getenv_SOURCES = common/frob-getenv.c
+frob_getenv_LDADD = $(common_LIBS)

diff --git a/common/compat.c b/common/compat.c
index ce0ccabd5b92d7bae66df162827bf54c5067ff17..768bb7d17874aa43321eac59ff8550c0084d9946 100644 (file)
--- a/common/compat.c
+++ b/common/compat.c
@@ -845,6 +845,14 @@ getauxval (unsigned long type)
 
 #endif /* HAVE_GETAUXVAL */
 
+char *
+secure_getenv (const char *name)
+{
+       if (getauxval (AT_SECURE))
+               return NULL;
+       return getenv (name);
+}
+
 #ifndef HAVE_STRERROR_R
 
 int

diff --git a/common/compat.h b/common/compat.h
index 477137072e899a43a0b3ed9c7e66de051d871837..6483d4f2318dae9e2335d4381120580353873a10 100644 (file)
--- a/common/compat.h
+++ b/common/compat.h
@@ -317,6 +317,8 @@ unsigned long     getauxval (unsigned long type);
 
 #endif /* !HAVE_GETAUXVAL */
 
+char *            secure_getenv (const char *name);
+
 #ifndef HAVE_STRERROR_R
 
 int         strerror_r      (int errnum,

diff --git a/common/debug.c b/common/debug.c
index 1fbdc7f4c8454e3779f2bed0b1c451bc3e652e18..47933fa8af1a01a6bdd10116a5b9dd7c1e90292e 100644 (file)
--- a/common/debug.c
+++ b/common/debug.c
@@ -36,6 +36,7 @@
 
 #include "config.h"
 
+#include "compat.h"
 #include "debug.h"
 
 #include <assert.h>
@@ -76,7 +77,7 @@ parse_environ_flags (void)
        const char *q;
        int i;
 
-       env = getenv ("P11_KIT_STRICT");
+       env = secure_getenv ("P11_KIT_STRICT");
        if (env && env[0] != '\0')
                debug_strict = true;
 

diff --git a/common/frob-getenv.c b/common/frob-getenv.c
new file mode 100644 (file)
index 0000000..a36594a
--- /dev/null
+++ b/common/frob-getenv.c
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2014 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *     * Redistributions of source code must retain the above
+ *       copyright notice, this list of conditions and the
+ *       following disclaimer.
+ *     * Redistributions in binary form must reproduce the
+ *       above copyright notice, this list of conditions and
+ *       the following disclaimer in the documentation and/or
+ *       other materials provided with the distribution.
+ *     * The names of contributors to this software may not be
+ *       used to endorse or promote products derived from this
+ *       software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ * Author: Stef Walter <stefw@gnome.org>
+ */
+
+#include "config.h"
+#include "compat.h"
+
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int
+main (int argc,
+      char *argv[])
+{
+       int ret;
+       const char *val;
+
+fprintf (stderr, "calling secure_getenv(%s) getenv(%s) = %s\n", argv[1], argv[1], getenv(argv[1]));
+       val = secure_getenv (argv[1]);
+       if (val == NULL) {
+               printf ("%s=NULL\n", argv[1]);
+               return 0;
+       }
+
+       ret = atoi (val);
+       if (ret == 0) {
+               fprintf (stderr, "usage: frob-getenv VAR");
+               abort ();
+       }
+
+       printf ("%s=%d\n", argv[1], ret);
+       return ret;
+}

diff --git a/common/test-compat.c b/common/test-compat.c
index 42471ae272d0047c0848422f06c27996a4f67351..3af33ac5aab4ac7a458b575e0bc4f45b2e14216b 100644 (file)
--- a/common/test-compat.c
+++ b/common/test-compat.c
@@ -83,6 +83,32 @@ test_getauxval (void)
        free (path);
 }
 
+static void
+test_secure_getenv (void)
+{
+       const char *args[] = { BUILDDIR "/frob-getenv", "BLAH", NULL };
+       char *path;
+       int ret;
+
+       setenv ("BLAH", "5", 1);
+
+       ret = p11_test_run_child (args, true);
+       assert_num_eq (ret, 5);
+
+       path = p11_test_copy_setgid (args[0]);
+       if (path == NULL)
+               return;
+
+       args[0] = path;
+       ret = p11_test_run_child (args, true);
+       assert_num_cmp (ret, ==, 0);
+
+/*     if (unlink (path) < 0)
+               assert_fail ("unlink failed", strerror (errno));
+               */
+       free (path);
+}
+
 static void
 test_mmap (void)
 {
@@ -110,6 +136,7 @@ main (int argc,
        /* Don't run this test when under fakeroot */
        if (!getenv ("FAKED_MODE")) {
                p11_test (test_getauxval, "/compat/getauxval");
+               p11_test (test_secure_getenv, "/compat/secure_getenv");
        }
        p11_test (test_mmap, "/compat/mmap");
 #endif

diff --git a/common/test.c b/common/test.c
index 5a1cf721c9ed320b454899fd56a1a78bec60b52d..9605d03a93c727bd2dcc9135c30009a8a420e0e7 100644 (file)
--- a/common/test.c
+++ b/common/test.c
@@ -322,7 +322,7 @@ expand_tempdir (const char *name)
 {
        const char *env;
 
-       env = getenv ("TMPDIR");
+       env = secure_getenv ("TMPDIR");
        if (env && env[0]) {
                return p11_path_build (env, name, NULL);
 

diff --git a/configure.ac b/configure.ac
index b316d54dfbb454913660662de623ae12d1e3f4ed..c151c9a673163a5a279580040ce997e829dea691 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -88,7 +88,7 @@ if test "$os_unix" = "yes"; then
        AC_CHECK_HEADERS([sys/resource.h])
        AC_CHECK_MEMBERS([struct dirent.d_type],,,[#include <dirent.h>])
        AC_CHECK_FUNCS([getprogname getexecname basename mkstemp mkdtemp])
-       AC_CHECK_FUNCS([getauxval issetugid getresuid])
+       AC_CHECK_FUNCS([getauxval issetugid getresuid secure_getenv])
        AC_CHECK_FUNCS([strnstr memdup strndup strerror_r])
        AC_CHECK_FUNCS([asprintf vasprintf vsnprintf])
        AC_CHECK_FUNCS([timegm])