Fix credential renewal and impersonator checking for m_a_g

This commit is contained in:
Robbie Harwood 2017-03-14 19:00:23 +00:00
parent d3c2500ea7
commit 9f866b6ca3
4 changed files with 538 additions and 1 deletions

View File

@ -0,0 +1,236 @@
From 0e04be2c1398dac40c50910a59157eed0ad5a7e4 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Tue, 14 Mar 2017 10:43:17 -0400
Subject: [PATCH] Allow connection to self when impersonator set
If the target of a context establishment is the impersonator itself,
then allow it. This kind of context establishment is used by tools like
mod_auth_gssapi to be able to inspect the ticket just obtained via
impersonation and it is basically a noop as the acceptor and the
impersonator are the same entitiy.
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Merges: #172
(cherry picked from commit eada55e831d12b42d3be3a555ff4e133bed7f594)
---
proxy/src/gp_creds.c | 57 ++++++++++++++++++++++++++++++++-----
proxy/src/gp_rpc_creds.h | 3 +-
proxy/src/gp_rpc_init_sec_context.c | 2 +-
proxy/tests/t_impersonate.py | 35 ++++++++++++++++-------
4 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c
index 95a1c48..7d89b06 100644
--- a/proxy/src/gp_creds.c
+++ b/proxy/src/gp_creds.c
@@ -883,7 +883,8 @@ static uint32_t get_impersonator_name(uint32_t *min, gss_cred_id_t cred,
}
} else if (ret_maj == GSS_S_UNAVAILABLE) {
/* Not supported by krb5 library yet, fallback to raw krb5 calls */
- /* TODO: Remove once we set a required dependency on MIT 1.15+ */
+ /* TODO: Remove once we set a minimum required dependency on a
+ * release that supports this call */
ret_maj = get_impersonator_fallback(&ret_min, cred, impersonator);
if (ret_maj == GSS_S_FAILURE) {
if (ret_min == KRB5_CC_NOTFOUND) {
@@ -899,9 +900,47 @@ done:
return ret_maj;
}
+static uint32_t check_impersonator_name(uint32_t *min,
+ gss_name_t target_name,
+ const char *impersonator)
+{
+ gss_name_t canon_name = NULL;
+ gss_buffer_desc buf;
+ uint32_t ret_maj = 0;
+ uint32_t ret_min = 0;
+ uint32_t discard;
+ bool match;
+
+ ret_maj = gss_canonicalize_name(&discard, target_name, &gp_mech_krb5,
+ &canon_name);
+ if (ret_maj != GSS_S_COMPLETE) {
+ *min = ret_min;
+ return ret_maj;
+ }
+
+ ret_maj = gss_display_name(&discard, canon_name, &buf, NULL);
+ gss_release_name(&discard, &canon_name);
+ if (ret_maj != GSS_S_COMPLETE) {
+ *min = ret_min;
+ return ret_maj;
+ }
+
+ match = (strncmp(impersonator, buf.value, buf.length) == 0) &&
+ (strlen(impersonator) == buf.length);
+ gss_release_buffer(&discard, &buf);
+
+ *min = 0;
+ if (match) {
+ return GSS_S_COMPLETE;
+ } else {
+ return GSS_S_UNAUTHORIZED;
+ }
+}
+
uint32_t gp_cred_allowed(uint32_t *min,
struct gp_call_ctx *gpcall,
- gss_cred_id_t cred)
+ gss_cred_id_t cred,
+ gss_name_t target_name)
{
char *impersonator = NULL;
uint32_t ret_maj = 0;
@@ -924,11 +963,11 @@ uint32_t gp_cred_allowed(uint32_t *min,
if (ret_maj) goto done;
/* if we find an impersonator entry we bail as that is not authorized,
- * if it were then gpcall->service->allow_const_deleg would have caused
- * the ealier check to return GSS_S_COMPLETE already */
+ * *unless* the target is the impersonator itself! If the operation
+ * were authorized then gpcall->service->allow_const_deleg would have
+ * caused the ealier check to return GSS_S_COMPLETE already */
if (impersonator != NULL) {
- ret_min = 0;
- ret_maj = GSS_S_UNAUTHORIZED;
+ ret_maj = check_impersonator_name(&ret_min, target_name, impersonator);
}
done:
@@ -937,7 +976,11 @@ done:
GPDEBUGN(2, "Unauthorized impersonator credentials detected\n");
break;
case GSS_S_COMPLETE:
- GPDEBUGN(2, "No impersonator credentials detected\n");
+ if (impersonator) {
+ GPDEBUGN(2, "Credentials allowed for 'self'\n");
+ } else {
+ GPDEBUGN(2, "No impersonator credentials detected\n");
+ }
break;
default:
GPDEBUG("Failure while checking credentials\n");
diff --git a/proxy/src/gp_rpc_creds.h b/proxy/src/gp_rpc_creds.h
index 54fe482..c116e53 100644
--- a/proxy/src/gp_rpc_creds.h
+++ b/proxy/src/gp_rpc_creds.h
@@ -34,7 +34,8 @@ uint32_t gp_add_krb5_creds(uint32_t *min,
uint32_t gp_cred_allowed(uint32_t *min,
struct gp_call_ctx *gpcall,
- gss_cred_id_t cred);
+ gss_cred_id_t cred,
+ gss_name_t target_name);
void gp_filter_flags(struct gp_call_ctx *gpcall, uint32_t *flags);
diff --git a/proxy/src/gp_rpc_init_sec_context.c b/proxy/src/gp_rpc_init_sec_context.c
index 767a3ff..413e2ec 100644
--- a/proxy/src/gp_rpc_init_sec_context.c
+++ b/proxy/src/gp_rpc_init_sec_context.c
@@ -108,7 +108,7 @@ int gp_init_sec_context(struct gp_call_ctx *gpcall,
}
}
- ret_maj = gp_cred_allowed(&ret_min, gpcall, ich);
+ ret_maj = gp_cred_allowed(&ret_min, gpcall, ich, target_name);
if (ret_maj) {
goto done;
}
diff --git a/proxy/tests/t_impersonate.py b/proxy/tests/t_impersonate.py
index 3e25962..29f9a41 100755
--- a/proxy/tests/t_impersonate.py
+++ b/proxy/tests/t_impersonate.py
@@ -34,19 +34,20 @@ IMPERSONATE_CONF_TEMPLATE = '''
'''
-def run_cmd(testdir, env, conf, name, socket, cmd, expected_failure):
+def run_cmd(testdir, env, conf, name, socket, cmd, keytab, expected_failure):
logfile = conf['logfile']
testenv = env.copy()
testenv.update({'KRB5CCNAME': os.path.join(testdir, 't' + conf['prefix'] +
'_impersonate.ccache'),
- 'KRB5_KTNAME': os.path.join(testdir, PROXY_KTNAME),
+ 'KRB5_KTNAME': os.path.join(testdir, keytab),
'KRB5_TRACE': os.path.join(testdir, 't' + conf['prefix'] +
'_impersonate.trace'),
'GSS_USE_PROXY': 'yes',
'GSSPROXY_SOCKET': socket,
'GSSPROXY_BEHAVIOR': 'REMOTE_FIRST'})
+ print("\nTesting: [%s]" % (name,), file=logfile)
print("[COMMAND]\n%s\n[ENVIRONMENT]\n%s\n" % (cmd, testenv), file=logfile)
logfile.flush()
@@ -74,45 +75,59 @@ def run(testdir, env, conf):
rets = []
# Test all permitted
+ msg = "Impersonate"
socket = os.path.join(testdir, 'impersonate.socket')
cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, PROXY_GSS,
path_prefix + 'impersonate.cache']
- r = run_cmd(testdir, env, conf, "Impersonate", socket, cmd, False)
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, PROXY_KTNAME, False)
rets.append(r)
- #Test fail
+ #Test self fail
+ msg = "Impersonate fail self"
socket = os.path.join(testdir, 'impersonate-proxyonly.socket')
cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, PROXY_GSS,
path_prefix + 'impersonate.cache']
- r = run_cmd(testdir, env, conf, "Impersonate fail self", socket, cmd, True)
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, PROXY_KTNAME, True)
rets.append(r)
- #Test fail
+ #Test proxy fail
+ msg = "Impersonate fail proxy"
socket = os.path.join(testdir, 'impersonate-selfonly.socket')
cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, PROXY_GSS,
path_prefix + 'impersonate.cache']
- r = run_cmd(testdir, env, conf, "Impersonate fail proxy", socket, cmd, True)
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, PROXY_KTNAME, True)
rets.append(r)
#Test s4u2self half succeed
+ msg = "s4u2self delegation"
socket = os.path.join(testdir, 'impersonate-selfonly.socket')
cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, PROXY_GSS,
path_prefix + 'impersonate.cache', 's4u2self']
- r = run_cmd(testdir, env, conf, "s4u2self delegation", socket, cmd, False)
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, PROXY_KTNAME, False)
+ rets.append(r)
+
+ #Test proxy to self succeed
+ msg = "Impersonate to self"
+ socket = os.path.join(testdir, 'impersonate-selfonly.socket')
+ cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, HOST_GSS,
+ path_prefix + 'impersonate.cache', 's4u2proxy']
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, SVC_KTNAME, False)
rets.append(r)
#Test s4u2proxy half fail
+ msg = "s4u2proxy fail"
socket = os.path.join(testdir, 'impersonate-selfonly.socket')
cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, PROXY_GSS,
path_prefix + 'impersonate.cache', 's4u2proxy']
- r = run_cmd(testdir, env, conf, "s4u2proxy fail", socket, cmd, True)
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, PROXY_KTNAME, True)
rets.append(r)
#Test s4u2proxy half succeed
+ msg = "s4u2proxy"
socket = os.path.join(testdir, 'impersonate-proxyonly.socket')
cmd = ["./tests/t_impersonate", USR_NAME, HOST_GSS, PROXY_GSS,
path_prefix + 'impersonate.cache', 's4u2proxy']
- r = run_cmd(testdir, env, conf, "s4u2proxy", socket, cmd, False)
+ r = run_cmd(testdir, env, conf, msg, socket, cmd, PROXY_KTNAME, False)
rets.append(r)
# Reset back gssproxy conf

View File

@ -0,0 +1,216 @@
From 37d1667ad0cc91f46a493281e62775cc8bbe3b5b Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Tue, 14 Mar 2017 10:20:08 -0400
Subject: [PATCH] Change impersonator check code
In MIT 1.15 we now have a native way to check for an impersonator,
implement the use of that function but still keep the fallback for
earlier krb5 versions that do not support this method for now.
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Merges: #172
(cherry picked from commit 73b50c0b2799f0aed53337a6516b8e1a27279ebf)
---
proxy/configure.ac | 3 ++
proxy/src/gp_creds.c | 147 ++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 112 insertions(+), 38 deletions(-)
diff --git a/proxy/configure.ac b/proxy/configure.ac
index 63c0edf..c52dbb6 100644
--- a/proxy/configure.ac
+++ b/proxy/configure.ac
@@ -131,6 +131,9 @@ AC_CHECK_LIB(gssapi_krb5, gss_export_cred,,
[AC_MSG_ERROR([GSSAPI library does not support gss_export_cred])],
[$GSSAPI_LIBS])
+AC_CHECK_DECLS([GSS_KRB5_GET_CRED_IMPERSONATOR], [], [],
+ [[#include <gssapi/gssapi_krb5.h>]])
+
AC_SUBST([KRB5_CFLAGS])
AC_SUBST([KRB5_LIBS])
AC_SUBST([GSSAPI_CFLAGS])
diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c
index 171a724..95a1c48 100644
--- a/proxy/src/gp_creds.c
+++ b/proxy/src/gp_creds.c
@@ -773,9 +773,9 @@ void gp_filter_flags(struct gp_call_ctx *gpcall, uint32_t *flags)
*flags &= ~gpcall->service->filter_flags;
}
-uint32_t gp_cred_allowed(uint32_t *min,
- struct gp_call_ctx *gpcall,
- gss_cred_id_t cred)
+
+static uint32_t get_impersonator_fallback(uint32_t *min, gss_cred_id_t cred,
+ char **impersonator)
{
uint32_t ret_maj = 0;
uint32_t ret_min = 0;
@@ -785,22 +785,6 @@ uint32_t gp_cred_allowed(uint32_t *min,
krb5_data config;
int err;
- if (cred == GSS_C_NO_CREDENTIAL) {
- return GSS_S_CRED_UNAVAIL;
- }
-
- if (gpcall->service->trusted ||
- gpcall->service->impersonate ||
- gpcall->service->allow_const_deleg) {
-
- GPDEBUGN(2, "Credentials allowed by configuration\n");
- *min = 0;
- return GSS_S_COMPLETE;
- }
-
- /* FIXME: krb5 specific code, should get an oid registerd to query the
- * cred with gss_inquire_cred_by_oid() or similar instead */
-
err = krb5_init_context(&context);
if (err) {
ret_min = err;
@@ -835,21 +819,116 @@ uint32_t gp_cred_allowed(uint32_t *min,
goto done;
}
+ err = krb5_cc_get_config(context, ccache, NULL, "proxy_impersonator",
+ &config);
+ if (err == 0) {
+ *impersonator = strndup(config.data, config.length);
+ if (!*impersonator) {
+ ret_min = ENOMEM;
+ ret_maj = GSS_S_FAILURE;
+ } else {
+ ret_min = 0;
+ ret_maj = GSS_S_COMPLETE;
+ }
+ krb5_free_data_contents(context, &config);
+ } else {
+ ret_min = err;
+ ret_maj = GSS_S_FAILURE;
+ }
+
+done:
+ if (context) {
+ if (ccache) {
+ krb5_cc_destroy(context, ccache);
+ }
+ krb5_free_context(context);
+ }
+ free(memcache);
+
+ *min = ret_min;
+ return ret_maj;
+}
+
+#if !HAVE_DECL_GSS_KRB5_GET_CRED_IMPERSONATOR
+gss_OID_desc impersonator_oid = {
+ 11, discard_const("\x2a\x86\x48\x86\xf7\x12\x01\x02\x02\x05\x0e")
+};
+const gss_OID GSS_KRB5_GET_CRED_IMPERSONATOR = &impersonator_oid;
+#endif
+
+static uint32_t get_impersonator_name(uint32_t *min, gss_cred_id_t cred,
+ char **impersonator)
+{
+ gss_buffer_set_t bufset = GSS_C_NO_BUFFER_SET;
+ uint32_t ret_maj = 0;
+ uint32_t ret_min = 0;
+ uint32_t discard;
+
+ *impersonator = NULL;
+
+ ret_maj = gss_inquire_cred_by_oid(&ret_min, cred,
+ GSS_KRB5_GET_CRED_IMPERSONATOR,
+ &bufset);
+ if (ret_maj == GSS_S_COMPLETE) {
+ if (bufset->count == 0) {
+ ret_min = ENOENT;
+ ret_maj = GSS_S_COMPLETE;
+ goto done;
+ }
+ *impersonator = strndup(bufset->elements[0].value,
+ bufset->elements[0].length);
+ if (!*impersonator) {
+ ret_min = ENOMEM;
+ ret_maj = GSS_S_FAILURE;
+ }
+ } else if (ret_maj == GSS_S_UNAVAILABLE) {
+ /* Not supported by krb5 library yet, fallback to raw krb5 calls */
+ /* TODO: Remove once we set a required dependency on MIT 1.15+ */
+ ret_maj = get_impersonator_fallback(&ret_min, cred, impersonator);
+ if (ret_maj == GSS_S_FAILURE) {
+ if (ret_min == KRB5_CC_NOTFOUND) {
+ ret_min = ENOENT;
+ ret_maj = GSS_S_COMPLETE;
+ }
+ }
+ }
+
+done:
+ (void)gss_release_buffer_set(&discard, &bufset);
+ *min = ret_min;
+ return ret_maj;
+}
+
+uint32_t gp_cred_allowed(uint32_t *min,
+ struct gp_call_ctx *gpcall,
+ gss_cred_id_t cred)
+{
+ char *impersonator = NULL;
+ uint32_t ret_maj = 0;
+ uint32_t ret_min = 0;
+
+ if (cred == GSS_C_NO_CREDENTIAL) {
+ return GSS_S_CRED_UNAVAIL;
+ }
+
+ if (gpcall->service->trusted ||
+ gpcall->service->impersonate ||
+ gpcall->service->allow_const_deleg) {
+
+ GPDEBUGN(2, "Credentials allowed by configuration\n");
+ *min = 0;
+ return GSS_S_COMPLETE;
+ }
+
+ ret_maj = get_impersonator_name(&ret_min, cred, &impersonator);
+ if (ret_maj) goto done;
+
/* if we find an impersonator entry we bail as that is not authorized,
* if it were then gpcall->service->allow_const_deleg would have caused
* the ealier check to return GSS_S_COMPLETE already */
- err = krb5_cc_get_config(context, ccache, NULL, "proxy_impersonator",
- &config);
- if (!err) {
- krb5_free_data_contents(context, &config);
+ if (impersonator != NULL) {
ret_min = 0;
ret_maj = GSS_S_UNAUTHORIZED;
- } else if (err != KRB5_CC_NOTFOUND) {
- ret_min = err;
- ret_maj = GSS_S_FAILURE;
- } else {
- ret_min = 0;
- ret_maj = GSS_S_COMPLETE;
}
done:
@@ -864,15 +943,7 @@ done:
GPDEBUG("Failure while checking credentials\n");
break;
}
- if (context) {
- /* NOTE: destroy only if we created a MEMORY ccache */
- if (ccache) {
- if (memcache) krb5_cc_destroy(context, ccache);
- else krb5_cc_close(context, ccache);
- }
- krb5_free_context(context);
- }
- free(memcache);
+ free(impersonator);
*min = ret_min;
return ret_maj;
}

View File

@ -0,0 +1,75 @@
From fc748ba83eb29f10fd44b6572b04709fa27dc587 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Mon, 13 Mar 2017 08:06:12 -0400
Subject: [PATCH] Properly renew expired credentials
When a caller imports expired credentials, we aim to actually renew them
if we can. However due to incorrect checks and not clearing of the
ret_maj variable after checks we end up returning an error instead.
Also fix mechglue to also save and properly report the first call errors
when both remote and local fail.
Resolves: #170
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
(cherry picked from commit dc462321226f59ceaab0d3db47446a694a8ecba2)
---
proxy/src/gp_creds.c | 14 +++++++++-----
proxy/src/mechglue/gpp_acquire_cred.c | 5 +++++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c
index 5d84904..171a724 100644
--- a/proxy/src/gp_creds.c
+++ b/proxy/src/gp_creds.c
@@ -629,8 +629,12 @@ uint32_t gp_add_krb5_creds(uint32_t *min,
ret_maj = gp_check_cred(&ret_min, in_cred, desired_name, cred_usage);
if (ret_maj == GSS_S_COMPLETE) {
return GSS_S_COMPLETE;
- } else if (ret_maj != GSS_S_CREDENTIALS_EXPIRED &&
- ret_maj != GSS_S_NO_CRED) {
+ } else if (ret_maj == GSS_S_CREDENTIALS_EXPIRED ||
+ ret_maj == GSS_S_NO_CRED) {
+ /* continue and try to obtain new creds */
+ ret_maj = 0;
+ ret_min = 0;
+ } else {
*min = ret_min;
return GSS_S_CRED_UNAVAIL;
}
@@ -639,14 +643,14 @@ uint32_t gp_add_krb5_creds(uint32_t *min,
if (acquire_type == ACQ_NORMAL) {
ret_min = gp_get_cred_environment(gpcall, desired_name, &req_name,
&cred_usage, &cred_store);
+ if (ret_min) {
+ ret_maj = GSS_S_CRED_UNAVAIL;
+ }
} else if (desired_name) {
ret_maj = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
}
if (ret_maj) {
goto done;
- } else if (ret_min) {
- ret_maj = GSS_S_CRED_UNAVAIL;
- goto done;
}
if (!try_impersonate(gpcall->service, cred_usage, acquire_type)) {
diff --git a/proxy/src/mechglue/gpp_acquire_cred.c b/proxy/src/mechglue/gpp_acquire_cred.c
index d876699..514fdd1 100644
--- a/proxy/src/mechglue/gpp_acquire_cred.c
+++ b/proxy/src/mechglue/gpp_acquire_cred.c
@@ -186,6 +186,11 @@ OM_uint32 gssi_acquire_cred_from(OM_uint32 *minor_status,
}
if (behavior == GPP_REMOTE_FIRST) {
+ if (maj != GSS_S_COMPLETE) {
+ /* save errors */
+ tmaj = maj;
+ tmin = min;
+ }
/* So remote failed, but we can fallback to local, try that */
maj = acquire_local(&min, NULL, name,
time_req, desired_mechs, cred_usage, cred_store,

View File

@ -1,6 +1,6 @@
Name: gssproxy
Version: 0.7.0
Release: 1%{?dist}
Release: 2%{?dist}
Summary: GSSAPI Proxy
Group: System Environment/Libraries
@ -14,6 +14,9 @@ BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
%global gpstatedir %{_localstatedir}/lib/gssproxy
### Patches ###
Patch0: Properly-renew-expired-credentials.patch
Patch1: Change-impersonator-check-code.patch
Patch2: Allow-connection-to-self-when-impersonator-set.patch
### Dependencies ###
Requires: krb5-libs >= 1.12.0
@ -49,6 +52,10 @@ A proxy for GSSAPI credential handling
%prep
%setup -q
%patch0 -p2 -b .Properly-renew-expired-credentials
%patch1 -p2 -b .Change-impersonator-check-code
%patch2 -p2 -b .Allow-connection-to-self-when-impersonator-set
%build
@ -107,6 +114,9 @@ rm -rf %{buildroot}
%systemd_postun_with_restart gssproxy.service
%changelog
* Tue Mar 14 2017 Robbie Harwood <rharwood@redhat.com> - 0.7.0-2
- Fix credential renewal and impersonator checking for m_a_g
* Tue Mar 07 2017 Robbie Harwood <rharwood@redhat.com> - 0.7.0-1
- New upstream release - 0.7.0