krb5 commit: Add secure_getenv() support

Greg Hudson ghudson at mit.edu
Tue Apr 30 18:09:14 EDT 2019


https://github.com/krb5/krb5/commit/ff71934f40afd4ae536638fa626fcd9ab36daf75
commit ff71934f40afd4ae536638fa626fcd9ab36daf75
Author: Greg Hudson <ghudson at mit.edu>
Date:   Wed Apr 24 15:56:43 2019 -0400

    Add secure_getenv() support
    
    On systems with secure_getenv() (glibc 2.17+) use it directly.  For
    the fallback implementation, check the current process uids and gids
    in a library initializer, looking at the saved uid and gid where
    possible.  Include a comment about more aggressive approaches to
    detecting elevated privilege.
    
    ticket: 8800 (new)

 src/configure.ac                 |   16 +++++-
 src/include/k5-platform.h        |    9 +++
 src/util/support/Makefile.in     |   16 ++++--
 src/util/support/secure_getenv.c |  111 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 146 insertions(+), 6 deletions(-)

diff --git a/src/configure.ac b/src/configure.ac
index 87d2135..9bafee7 100644
--- a/src/configure.ac
+++ b/src/configure.ac
@@ -148,7 +148,7 @@ AC_SUBST(po)
 
 # for kdc
 AC_CHECK_HEADERS(sys/sockio.h ifaddrs.h unistd.h fnmatch.h)
-AC_CHECK_FUNCS(vsprintf vasprintf vsnprintf strlcpy fnmatch)
+AC_CHECK_FUNCS(vsprintf vasprintf vsnprintf strlcpy fnmatch secure_getenv)
 
 EXTRA_SUPPORT_SYMS=
 AC_CHECK_FUNC(strlcpy,
@@ -208,6 +208,18 @@ KRB5_NEED_PROTO([#include <string.h>
 #include <stdlib.h>
 ],swab,1)
 
+AC_CHECK_FUNC(secure_getenv,
+[SECURE_GETENV_ST_OBJ=
+SECURE_GETENV_OBJ=
+SECURE_GETENV_INIT=],
+[SECURE_GETENV_ST_OBJ=secure_getenv.o
+SECURE_GETENV_OBJ='$(OUTPRE)secure_getenv.$(OBJEXT)'
+SECURE_GETENV_INIT=k5_secure_getenv_init
+EXTRA_SUPPORT_SYMS="$EXTRA_SUPPORT_SYMS k5_secure_getenv"])
+AC_SUBST(SECURE_GETENV_OBJ)
+AC_SUBST(SECURE_GETENV_ST_OBJ)
+AC_SUBST(SECURE_GETENV_INIT)
+
 AC_PROG_AWK
 KRB5_AC_INET6
 KRB5_SOCKADDR_SA_LEN
@@ -442,7 +454,7 @@ AC_PROG_LEX
 AC_C_CONST
 AC_HEADER_DIRENT
 AC_FUNC_STRERROR_R
-AC_CHECK_FUNCS(strdup setvbuf seteuid setresuid setreuid setegid setresgid setregid setsid flock fchmod chmod strptime geteuid setenv unsetenv getenv gmtime_r localtime_r bswap16 bswap64 mkstemp getusershell access getcwd srand48 srand srandom stat strchr strerror timegm explicit_bzero explicit_memset)
+AC_CHECK_FUNCS(strdup setvbuf seteuid setresuid setreuid setegid setresgid setregid setsid flock fchmod chmod strptime geteuid setenv unsetenv getenv gmtime_r localtime_r bswap16 bswap64 mkstemp getusershell access getcwd srand48 srand srandom stat strchr strerror timegm explicit_bzero explicit_memset getresuid getresgid)
 
 AC_CHECK_FUNC(mkstemp,
 [MKSTEMP_ST_OBJ=
diff --git a/src/include/k5-platform.h b/src/include/k5-platform.h
index 1fcd68e..1124931 100644
--- a/src/include/k5-platform.h
+++ b/src/include/k5-platform.h
@@ -45,6 +45,7 @@
  * + path manipulation
  * + _, N_, dgettext, bindtextdomain (for localization)
  * + getopt_long
+ * + secure_getenv
  * + fetching filenames from a directory
  */
 
@@ -1134,6 +1135,14 @@ extern int k5_getopt_long(int nargc, char **nargv, char *options,
 #define getopt_long k5_getopt_long
 #endif /* HAVE_GETOPT_LONG */
 
+#if defined(_WIN32)
+/* On Windows there is never a need to ignore the process environment. */
+#define secure_getenv getenv
+#elif !defined(HAVE_SECURE_GETENV)
+#define secure_getenv k5_secure_getenv
+extern char *k5_secure_getenv(const char *name);
+#endif
+
 /* Set *fnames_out to a null-terminated list of filenames within dirname,
  * sorted according to strcmp().  Return 0 on success, or ENOENT/ENOMEM. */
 int k5_dir_filenames(const char *dirname, char ***fnames_out);
diff --git a/src/util/support/Makefile.in b/src/util/support/Makefile.in
index db7b030..86d5a95 100644
--- a/src/util/support/Makefile.in
+++ b/src/util/support/Makefile.in
@@ -15,7 +15,7 @@ LIBBASE=krb5support
 LIBMAJOR=@SUPPORTLIB_MAJOR@
 LIBMINOR=1
 
-LIBINITFUNC=krb5int_thread_support_init
+LIBINITFUNC=krb5int_thread_support_init @SECURE_GETENV_INIT@
 LIBFINIFUNC=krb5int_thread_support_fini
 
 GETTIMEOFDAY_ST_OBJ= @GETTIMEOFDAY_ST_OBJ@
@@ -53,6 +53,11 @@ GETOPT_LONG_OBJ= @GETOPT_LONG_OBJ@
 ##DOS##GETOPT_LONG_ST_OBJ= getopt_long.o
 ##DOS##GETOPT_LONG_OBJ= $(OUTPRE)getopt_long.$(OBJEXT)
 
+SECURE_GETENV_ST_OBJ= @SECURE_GETENV_ST_OBJ@
+SECURE_GETENV_OBJ= @SECURE_GETENV_OBJ@
+##DOS##SECURE_GETENV_ST_OBJ=
+##DOS##SECURE_GETENV_OBJ=
+
 IPC_ST_OBJ=
 IPC_OBJ=
 ##DOS##IPC_ST_OBJ= ipc_stream.o
@@ -93,7 +98,8 @@ STLIBOBJS= \
 	$(PRINTF_ST_OBJ) \
 	$(MKSTEMP_ST_OBJ) \
 	$(GETOPT_ST_OBJ) \
-	$(GETOPT_LONG_ST_OBJ)
+	$(GETOPT_LONG_ST_OBJ) \
+	$(SECURE_GETENV_OBJ)
 
 LIBOBJS= \
 	$(OUTPRE)threads.$(OBJEXT) \
@@ -121,7 +127,8 @@ LIBOBJS= \
 	$(PRINTF_OBJ) \
 	$(MKSTEMP_OBJ) \
 	$(GETOPT_OBJ) \
-	$(GETOPT_LONG_OBJ)
+	$(GETOPT_LONG_OBJ) \
+	$(SECURE_GETENV_OBJ)
 
 SRCS=\
 	$(srcdir)/threads.c \
@@ -156,7 +163,8 @@ SRCS=\
 	$(srcdir)/t_utf8.c \
 	$(srcdir)/t_utf16.c \
 	$(srcdir)/getopt.c \
-	$(srcdir)/getopt_long.c
+	$(srcdir)/getopt_long.c \
+	$(srcdir)/secure_getenv.c
 
 SHLIB_EXPDEPS =
 # Add -lm if dumping thread stats, for sqrt.
diff --git a/src/util/support/secure_getenv.c b/src/util/support/secure_getenv.c
new file mode 100644
index 0000000..6df0591
--- /dev/null
+++ b/src/util/support/secure_getenv.c
@@ -0,0 +1,111 @@
+/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/* util/support/secure_getenv.c - secure_getenv() portability support */
+/*
+ * Copyright (C) 2019 by the Massachusetts Institute of Technology.
+ * All rights reserved.
+ *
+ * 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.
+ *
+ * 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 HOLDER 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.
+ */
+
+/*
+ * This file contains the fallback implementation for secure_getenv(), which is
+ * currently only provided by glibc 2.17 and later.  The goal is to ignore the
+ * environment if this process is (or previously was) running at elevated
+ * privilege compared to the calling process.
+ *
+ * In this fallback version we compare the real and effective uid/gid, and also
+ * compare the saved uid/gid if possible.  These comparisons detect a setuid or
+ * setgid process which is still running with elevated privilege; if we can
+ * fetch the saved uid/gid, we also detect a process which has temporarily
+ * dropped privilege with seteuid() or setegid().  These comparisons do not
+ * detect the case where a setuid or setgid process has permanently dropped
+ * privilege before the library initializer ran; this is not ideal because such
+ * a process may possess a privileged resource or have privileged information
+ * in its address space.
+ *
+ * Heimdal also looks at the ELF aux vector in /proc/self/auxv to determine the
+ * starting uid/euid/gid/euid on Solaris/Illumos and NetBSD.  On FreeBSD this
+ * approach can determine the executable path to do a stat() check.  We do not
+ * go to this length due to the amount of code required.
+ *
+ * The BSDs and Solaris provide issetugid(), but the FreeBSD and NetBSD
+ * versions are not useful; they return true if a non-setuid/setgid executable
+ * is run by root and drops privilege, such as Apache httpd.  We do not want to
+ * ignore the process environment in this case.
+ *
+ * On some platforms a process may have elevated privilege via mechanisms other
+ * than setuid/setgid.  glibc's secure_getenv() should detect these cases on
+ * Linux; we do not detect them in this fallback version.
+ */
+
+#include "k5-platform.h"
+
+static int elevated_privilege = 0;
+
+MAKE_INIT_FUNCTION(k5_secure_getenv_init);
+
+int
+k5_secure_getenv_init()
+{
+    int saved_errno = errno;
+
+#ifdef HAVE_GETRESUID
+    {
+        uid_t r, e, s;
+        if (getresuid(&r, &e, &s) == 0) {
+            if (r != e || r != s)
+                elevated_privilege = 1;
+        }
+    }
+#else
+    if (getuid() != geteuid())
+        elevated_privilege = 1;
+#endif
+
+#ifdef HAVE_GETRESGID
+    {
+        gid_t r, e, s;
+        if (!elevated_privilege && getresgid(&r, &e, &s) == 0) {
+            if (r != e || r != s)
+                elevated_privilege = 1;
+        }
+    }
+#else
+    if (!elevated_privilege && getgid() != getegid())
+        elevated_privilege = 1;
+#endif
+
+    errno = saved_errno;
+    return 0;
+}
+
+char *
+k5_secure_getenv(const char *name)
+{
+    if (CALL_INIT_FUNCTION(k5_secure_getenv_init) != 0)
+        return NULL;
+    return elevated_privilege ? NULL : getenv(name);
+}


More information about the cvs-krb5 mailing list