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