PR#172: Allow to establish context to self for impersonators - gssproxy

gssproxy

#172 Allow to establish context to self for impersonators

Merged 6 years ago by simo. Opened 7 years ago by simo.

simo/gssproxy target_check into master

Allow connection to self when impersonator set

Simo Sorce • 7 years ago

3100822

Change impersonator check code

Simo Sorce • 7 years ago

c0cd897

proxy/configure.ac

file modified

		`@@ -131,6 +131,9 @@`
		`[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])`

proxy/src/gp_creds.c

file modified

+152 -38

		`@@ -769,9 +769,9 @@`
		`*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;`
		`@@ -781,22 +781,6 @@`
		`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;`
		`@@ -831,21 +815,155 @@`
		`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) {`
		`+ 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);`
		`- ret_min = 0;`
		`- ret_maj = GSS_S_UNAUTHORIZED;`
		`- } else if (err != KRB5_CC_NOTFOUND) {`
		`+ } 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 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) {`
		`+ ret_min = ENOENT;`
		`+ ret_maj = GSS_S_COMPLETE;`
		`+ }`
		`+ }`
		`+ }`
		`+`
		`+ done:`
		`+ (void)gss_release_buffer_set(&discard, &bufset);`
		`+ *min = ret_min;`
		`+ 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 {`
		`- ret_min = 0;`
		`- ret_maj = GSS_S_COMPLETE;`
		`+ return GSS_S_UNAUTHORIZED;`
		`+ }`
		`+ }`
		`+`
		`+ uint32_t gp_cred_allowed(uint32_t *min,`
		`+ struct gp_call_ctx *gpcall,`
		`+ gss_cred_id_t cred,`
		`+ gss_name_t target_name)`
		`+ {`
		`+ 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,`
		`+ * 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_maj = check_impersonator_name(&ret_min, target_name, impersonator);`
		`}`

		`done:`
		`@@ -854,21 +972,17 @@`
		`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");`
		`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;`
		`}`

proxy/src/gp_rpc_creds.h

file modified

+2 -1

		`@@ -34,7 +34,8 @@`

		`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);`

proxy/src/gp_rpc_init_sec_context.c

file modified

+1 -1

		`@@ -108,7 +108,7 @@`
		`}`
		`}`

		`- 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;`
		`}`

proxy/tests/t_impersonate.py

file modified

+25 -10

		`@@ -34,19 +34,20 @@`

		`'''`

		`- 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 @@`
		`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`

simo commented 7 years ago

Impersonators may need to establish context to themselves, usually to allow gssapi to get access to the ticket and export named attributes on the "client" name.
Given it is harmless to allow connections to self always allow it in the impersonator credential case.

simo commented 7 years ago

Please don't push this yet, I figured out a simpler/better method to deal with this problem

rebased

7 years ago

simo commented 7 years ago

Ok, I changed this PR to not require the admin to set krb5_principal to allow self, as that is error prone and did not account for keytabs with multiple keys. It also had side effects people may not ant to deal with again in case of keytabs with multiple keys.
Instead check who actually is the impersonator in the ticket and allow context establishment to that specific target whatever name it is.

rharwood commented on line 113 of proxy/src/gp_creds.c 7 years ago

It's not in 1.15. Please test a version that has it, and remove the reference here to 1.15. (At some point in the future after we check the version of krb5 in autotools, we can talk about removing the fallback entirely.)

rharwood commented on line 159 of proxy/src/gp_creds.c 7 years ago

The contents of this if block are candidates for being their own function.

simo commented 7 years ago

I rebased to a version that should address all your comments.
I also tested with a krb5 version that implements the new call to test impersonator and it also works as expected.

2 new commits added