From 1d78d1af3da7eeb15aa1f054b740f31a12f48f31 Mon Sep 17 00:00:00 2001

From: Simo Sorce <simo@redhat.com>

Date: Sat, 16 Nov 2013 17:08:06 -0500

Subject: [PATCH 1/3] config: Do not modify const strings

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Take a copy here, the option string is const and strtok_r() is not a safe

function as it may change the string it manipulates.

Reviewed-by: Günther Deschner <gdeschner@redhat.com>

---

 proxy/src/gp_config.c | 10 +++++++++-

 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c

index e21e70d..63f264e 100644

--- a/proxy/src/gp_config.c

+++ b/proxy/src/gp_config.c

@@ -209,6 +209,7 @@ static int load_services(struct gp_config *cfg, struct gp_ini_context *ctx)

     int num_sec;

     char *secname = NULL;

     const char *value;

+    char *vcopy;

     char *token;

     char *handle;

     int valnum;

@@ -318,7 +319,12 @@ static int load_services(struct gp_config *cfg, struct gp_ini_context *ctx)

                 goto done;

             }

-            token = strtok_r(no_const(value), ", ", &handle);

+            vcopy = strdup(value);

+            if (!vcopy) {

+                ret = ENOMEM;

+                goto done;

+            }

+            token = strtok_r(vcopy, ", ", &handle);

             do {

                 ret = strcmp(value, "krb5");

@@ -329,6 +335,7 @@ static int load_services(struct gp_config *cfg, struct gp_ini_context *ctx)

                     } else {

                         GPERROR("Failed to read krb5 config for %s.\n",

                                 secname);

+                        safefree(vcopy);

                         return ret;

                     }

                 } else {

@@ -338,6 +345,7 @@ static int load_services(struct gp_config *cfg, struct gp_ini_context *ctx)

                 token = strtok_r(NULL, ", ", &handle);

             } while (token != NULL);

+            safefree(vcopy);

             if (cfg->svcs[n]->mechs == 0) {

                 GPDEBUG("No mechs found for [%s], ignoring.\n", secname);

-- 

1.8.3.1

From a272091dfd568cb96738cc96ea01bbf7f24ee62c Mon Sep 17 00:00:00 2001

From: Simo Sorce <simo@redhat.com>

Date: Sat, 16 Nov 2013 18:54:28 -0500

Subject: [PATCH 2/3] creds: Allow admins to define only client creds

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

When a service is configured with cred_usage = initiate it is

ok to allow only client credentials to be defined.

Reviewed-by: Günther Deschner <gdeschner@redhat.com>

---

 proxy/src/gp_creds.c | 7 ++++++-

 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c

index 60c4e12..1ac1fac 100644

--- a/proxy/src/gp_creds.c

+++ b/proxy/src/gp_creds.c

@@ -376,7 +376,12 @@ static int gp_get_cred_environment(struct gp_call_ctx *gpcall,

      * if any. */

     if (use_service_keytab) {

         if (k_num == -1) {

-            ret = EINVAL;

+            if (ck_num == -1) {

+                ret = EINVAL;

+            } else {

+                /* allow a service to define only the client keytab */

+                ret = 0;

+            }

             goto done;

         }

         if (ck_num == -1) {

-- 

1.8.3.1

From 23f4ee4359d10f66e1938ce6b1d92d3cc77865ff Mon Sep 17 00:00:00 2001

From: Simo Sorce <simo@redhat.com>

Date: Wed, 20 Nov 2013 11:58:22 -0500

Subject: [PATCH 3/3] Use secure_getenv in client and mechglue module

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

proxymehc.so may be used in setuid binaries so follow best security

practices and use secure_getenv() if available.

Fallback to poorman emulation when secure_getenv() is not available.

Resolves: https://fedorahosted.org/gss-proxy/ticket/110

Reviewed-by: Günther Deschner <gdeschner@redhat.com>

---

 proxy/Makefile.am               |  7 ++++---

 proxy/configure.ac              |  2 ++

 proxy/src/client/gpm_common.c   |  2 +-

 proxy/src/gp_common.h           |  1 +

 proxy/src/gp_util.c             | 20 ++++++++++++++++++++

 proxy/src/mechglue/gss_plugin.c |  4 ++--

 6 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/proxy/Makefile.am b/proxy/Makefile.am

index 065be6e..c946421 100644

--- a/proxy/Makefile.am

+++ b/proxy/Makefile.am

@@ -102,7 +102,9 @@ GP_RPCCLI_OBJ = \

     src/client/gpm_wrap.c \

     src/client/gpm_unwrap.c \

     src/client/gpm_wrap_size_limit.c \

-    src/client/gpm_common.c

+    src/client/gpm_common.c \

+    src/gp_util.c

+

 GP_MECHGLUE_OBJ = \

     src/mechglue/gpp_accept_sec_context.c \

     src/mechglue/gpp_acquire_cred.c \

@@ -114,8 +116,7 @@ GP_MECHGLUE_OBJ = \

     src/mechglue/gpp_indicate_mechs.c \

     src/mechglue/gpp_priv_integ.c \

     src/mechglue/gpp_misc.c \

-    src/mechglue/gss_plugin.c \

-    src/gp_util.c

+    src/mechglue/gss_plugin.c

 dist_noinst_HEADERS = \

     rpcgen/gp_rpc.h \

diff --git a/proxy/configure.ac b/proxy/configure.ac

index b75a1ef..a0cc4ef 100644

--- a/proxy/configure.ac

+++ b/proxy/configure.ac

@@ -149,6 +149,8 @@ AC_CHECK_LIB(gssrpc, gssrpc_xdrmem_create,,

              [$GSSAPI_LIBS $GSSRPC_LIBS])

 AC_SUBST([GSSRPC_LIBS])

+AC_CHECK_FUNCS([__secure_getenv secure_getenv])

+

 WITH_INITSCRIPT

 if test x$initscript = xsystemd; then

     WITH_SYSTEMD_UNIT_DIR

diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c

index df1f5a1..74296da 100644

--- a/proxy/src/client/gpm_common.c

+++ b/proxy/src/client/gpm_common.c

@@ -68,7 +68,7 @@ static int get_pipe_name(struct gpm_ctx *gpmctx, char *name)

     const char *socket;

     int ret;

-    socket = getenv("GSSPROXY_SOCKET");

+    socket = gp_getenv("GSSPROXY_SOCKET");

     if (!socket) {

         socket = GP_SOCKET_NAME;

     }

diff --git a/proxy/src/gp_common.h b/proxy/src/gp_common.h

index 9e4ae81..b5c525f 100644

--- a/proxy/src/gp_common.h

+++ b/proxy/src/gp_common.h

@@ -67,6 +67,7 @@

 bool gp_same(const char *a, const char *b);

 bool gp_boolean_is_true(const char *s);

+char *gp_getenv(const char *name);

 #include "rpcgen/gss_proxy.h"

diff --git a/proxy/src/gp_util.c b/proxy/src/gp_util.c

index 8400da1..a6c870f 100644

--- a/proxy/src/gp_util.c

+++ b/proxy/src/gp_util.c

@@ -23,8 +23,10 @@

    DEALINGS IN THE SOFTWARE.

 */

+#include "config.h"

 #include <stdbool.h>

 #include <string.h>

+#include <stdlib.h>

 bool gp_same(const char *a, const char *b)

 {

@@ -46,3 +48,21 @@ bool gp_boolean_is_true(const char *s)

     return false;

 }

+

+char *gp_getenv(const char *name)

+{

+#if HAVE_SECURE_GETENV

+    return secure_getenv(name);

+#elif HAVE___SECURE_GETENV

+    return __secure_getenv(name);

+#else

+#include <unistd.h>

+#include <sys/types.h>

+#warning secure_getenv not available, falling back to poorman emulation

+    if ((getuid() == geteuid()) &&

+        (getgid() == getegid())) {

+        return getenv(name);

+    }

+    return NULL;

+#endif

+}

diff --git a/proxy/src/mechglue/gss_plugin.c b/proxy/src/mechglue/gss_plugin.c

index 5b40df9..372ab2e 100644

--- a/proxy/src/mechglue/gss_plugin.c

+++ b/proxy/src/mechglue/gss_plugin.c

@@ -64,7 +64,7 @@ enum gpp_behavior gpp_get_behavior(void)

     char *envval;

     if (behavior == GPP_UNINITIALIZED) {

-        envval = getenv("GSSPROXY_BEHAVIOR");

+        envval = gp_getenv("GSSPROXY_BEHAVIOR");

         if (envval) {

             if (strcmp(envval, "LOCAL_ONLY") == 0) {

                 behavior = GPP_LOCAL_ONLY;

@@ -102,7 +102,7 @@ gss_OID_set gss_mech_interposer(gss_OID mech_type)

     /* avoid looping in the gssproxy daemon by avoiding to interpose

      * any mechanism */

-    envval = getenv("GSS_USE_PROXY");

+    envval = gp_getenv("GSS_USE_PROXY");

     if (!envval) {

         return NULL;

     }

-- 

1.8.3.1