credentials_krb5: use gss_acquire_cred for client-side GSSAPI use case

Alexander Bokovoy ab at samba.org
Fri Mar 3 18:29:28 UTC 2017


On pe, 03 maalis 2017, Alexander Bokovoy wrote:
> On pe, 03 maalis 2017, Alexander Bokovoy wrote:
> > On pe, 03 maalis 2017, Stefan Metzmacher wrote:
> > > Am 03.03.2017 um 13:07 schrieb Alexander Bokovoy:
> > > > On pe, 03 maalis 2017, Stefan Metzmacher wrote:
> > > >> Hi Alexander,
> > > >>
> > > >>>>>> Attached patch is needed for upcoming FreeIPA 4.5 release to allow use
> > > >>>>>> of Samba Python bindings in a privile separation mode provided by
> > > >>>>>> GSS-proxy (https://pagure.io/gssproxy). FreeIPA bug is here:
> > > >>>>>> https://pagure.io/freeipa/issue/6671, Samba bug is
> > > >>>>>> https://bugzilla.samba.org/show_bug.cgi?id=12611
> > > >>>>>>
> > > >>>>>> Please see more details in the commit message.
> > > >>>>>
> > > >>>>> Please have a look at
> > > >>>>> https://bugzilla.samba.org/show_bug.cgi?id=12480
> > > >>>>> for the reasons why we can't use gss_acquire_cred().
> > > >>>>>
> > > >>>>> There needs to be another solution, sorry.
> > > >>>>
> > > >>>> As gss_acquire_cred_from() seems to be handled by gssproxy,
> > > >>>> I guess we need a wrapper in lib/krb5_wrap/gss_samba.[ch]
> > > >>>> that uses gss_acquire_cred_from() if available and
> > > >>>> gss_krb5_import_cred() otherwise.
> > > >>>>
> > > >>>> And that wrapper needs to be used everywhere we currently
> > > >>>> use gss_krb5_import_cred(). It should also hide the mess
> > > >>>> we currently use in gse_init_server() to work arround
> > > >>>> the broken gss_krb5_import_cred() server side.
> > > >>> That was my initial idea but the problem here is that we deal with it 
> > > >>> differently in different contexts. In client context we use GSSAPI creds
> > > >>> while in other places we use KRB5 ccache handle.
> > > >>>
> > > >>> This particular code path is isolated from other use of
> > > >>> gss_krb5_import_cred(). And it does not need to specify explicit
> > > >>> credential cache because it is running in a client context and expects
> > > >>> to use the default ccache.
> > > >>
> > > >> That's wrong this code path operates on multiple MEMORY caches
> > > >> in smbtorture.
> > > >>
> > > >> gss_acquire_cred() never used again for initiator creds
> > > >> and only for acceptor creds as a fallback for a broken
> > > >> gss_krb5_import_cred().
> > > >>
> > > >> So all places should use the wrapper that uses
> > > >> gss_acquire_cred_from() if available. It should
> > > >> otherwise use gss_krb5_import_cred() with
> > > >> a fallback only for the keytab only case
> > > >> to gsskrb5_register_acceptor_identity()/gss_acquire_cred().
> > > > So, what about Andreas' code from
> > > > https://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/master-cli_creds
> > > > ?
> > > 
> > > I know I also had some of this in my master3-libsmb-tmp,
> > > but skipped it as it was not required yet.
> > > 
> > > > The only issue I see there is that when gensec_gssapi calls into
> > > > cli_credentials_get_client_gss_creds() with NULL ccache, it wants to get
> > > > the creds from the default ccache but Andreas' code will try to create a
> > > > new MEMORY: ccache. This is wrong for the use case we have with
> > > > cli_credentials_get_client_gss_creds() because this ccache is empty and
> > > > default one contains actual credentials.
> > > 
> > > I think I'd preferr to have a wrapper that behaves like
> > > gss_krb5_import_cred() on heimdal, just
> > > 
> > > uint32_t smb_gss_krb5_import_cred(OM_uint32 *minor_status,
> > >                      krb5_ccache id,
> > >                      krb5_principal keytab_principal,
> > >                      krb5_keytab keytab,
> > >                      gss_cred_id_t *cred);
> > > 
> > > And change the callers just to use it without any additional
> > > logic change.
> > > 
> > > That would be much easier to understand.
> > ACK. Will do that.
> I pushed current patchset to
> https://git.samba.org/?p=ab/samba.git/.git;a=shortlog;h=refs/heads/master-gss_acquire_cred_from
> 
> I'm running tests right now. Will submit final patch once they pass.
Final patch is attached.

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From d3c4036005b23005544afd9305c6e3c9beb25723 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab at samba.org>
Date: Fri, 3 Mar 2017 17:08:09 +0200
Subject: [PATCH 1/5] gssapi: check for gss_acquire_cred_from

---
 wscript_configure_system_mitkrb5 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/wscript_configure_system_mitkrb5 b/wscript_configure_system_mitkrb5
index 06a9821..d3e8ebf 100644
--- a/wscript_configure_system_mitkrb5
+++ b/wscript_configure_system_mitkrb5
@@ -92,6 +92,7 @@ conf.CHECK_FUNCS_IN('''
        gsskrb5_extract_authz_data_from_sec_context
        gss_krb5_export_lucid_sec_context
        gss_import_cred gss_export_cred
+       gss_acquire_cred_from
        ''', 'gssapi gssapi_krb5')
 conf.CHECK_VARIABLE('GSS_KRB5_CRED_NO_CI_FLAGS_X', headers=possible_gssapi_headers)
 conf.CHECK_FUNCS_IN('krb5_mk_req_extended krb5_kt_compare', 'krb5')
-- 
2.9.3


>From a3bd2e3e5b9f9d96ca423833ad51e0ba515ec76d Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab at samba.org>
Date: Fri, 3 Mar 2017 16:14:57 +0200
Subject: [PATCH 2/5] lib/krb5_wrap: add smb_gss_krb5_import_cred wrapper

Wrap gss_krb5_import_cred() to allow re-implementing it with
gss_acquire_cred_from() for newer MIT versions. gss_acquire_cred_from()
works fine with GSSAPI interposer (GSS-proxy) while
gss_krb5_import_cred() is not interposed yet.

The wrapper has additional parameter, krb5_context handle, to facilitate
with credentials cache name discovery. All our callers to
gss_krb5_import_cred() already have krb5 context handy.
---
 lib/krb5_wrap/gss_samba.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/krb5_wrap/gss_samba.h | 13 +++++++
 2 files changed, 105 insertions(+)

diff --git a/lib/krb5_wrap/gss_samba.c b/lib/krb5_wrap/gss_samba.c
index b444633..fc8f3b9 100644
--- a/lib/krb5_wrap/gss_samba.c
+++ b/lib/krb5_wrap/gss_samba.c
@@ -48,4 +48,96 @@ int smb_gss_oid_equal(const gss_OID first_oid, const gss_OID second_oid)
 }
 #endif /* !HAVE_GSS_OID_EQUAL */
 
+
+/* wrapper around gss_krb5_import_cred() that prefers to use gss_acquire_cred_from()
+ * if this GSSAPI extension is available. gss_acquire_cred_from() is properly
+ * interposed by GSSPROXY while gss_krb5_import_cred() is not.
+ *
+ * This wrapper requires a proper krb5_context to resolve ccache name.
+ * All gss_krb5_import_cred() callers in Samba already have krb5_context available. */
+uint32_t smb_gss_krb5_import_cred(uint32_t *minor_status, krb5_context ctx,
+				  krb5_ccache id, krb5_principal keytab_principal,
+				  krb5_keytab keytab, gss_cred_id_t *cred)
+{
+	uint32_t major_status = 0;
+
+#if HAVE_GSS_ACQUIRE_CRED_FROM
+	gss_key_value_element_desc ccache_element = {
+		.key = "ccache",
+		.value = NULL,
+	};
+
+	gss_key_value_element_desc keytab_element = {
+		.key = "keytab",
+		.value = NULL,
+	};
+
+	gss_key_value_element_desc elements[2];
+
+	gss_key_value_set_desc cred_store = {
+		.elements = &ccache_element,
+		.count = 1,
+	};
+
+	gss_OID_set mech_set = GSS_C_NO_OID_SET;
+	gss_cred_usage_t cred_usage = GSS_C_INITIATE;
+
+	if (id != NULL) {
+		major_status = krb5_cc_get_full_name(ctx,
+						     id,
+						     discard_const(&ccache_element.value));
+		if (major_status != 0) {
+			return major_status;
+		}
+	}
+
+	if (keytab != NULL) {
+		keytab_element.value = malloc(4096);
+		if (!keytab_element.value) {
+			return ENOMEM;
+		}
+		major_status = krb5_kt_get_name(ctx,
+						keytab,
+						discard_const(keytab_element.value), 4096);
+		if (major_status != 0) {
+			free(discard_const(keytab_element.value));
+			return major_status;
+		}
+		cred_usage = GSS_C_ACCEPT;
+		cred_store.elements = &keytab_element;
+	}
+
+	if (id != NULL && keytab != NULL) {
+		elements[0] = ccache_element;
+		elements[1] = keytab_element;
+
+		cred_store.elements = elements;
+		cred_store.count = 2;
+		cred_usage = GSS_C_BOTH;
+	}
+
+	major_status = gss_acquire_cred_from(minor_status,
+					     GSS_C_NO_NAME,
+					     0,
+					     mech_set,
+					     cred_usage,
+					     &cred_store,
+					     cred,
+					     NULL,
+					     NULL);
+
+	krb5_free_string(ctx, discard_const(ccache_element.value));
+	if (keytab_element.value != NULL) {
+		free(discard_const(keytab_element.value));
+	}
+#else
+	major_status = gss_krb5_import_cred(minor_status,
+					id,
+					keytab_principal,
+					keytab, cred);
+#endif
+	return major_status;
+}
+
+
 #endif /* HAVE_GSSAPI */
diff --git a/lib/krb5_wrap/gss_samba.h b/lib/krb5_wrap/gss_samba.h
index 5319932..89aee34 100644
--- a/lib/krb5_wrap/gss_samba.h
+++ b/lib/krb5_wrap/gss_samba.h
@@ -25,6 +25,7 @@
 #ifdef HAVE_GSSAPI
 
 #include "system/gssapi.h"
+#include "krb5_samba.h"
 
 #if defined(HAVE_GSS_OID_EQUAL)
 #define smb_gss_oid_equal gss_oid_equal
@@ -32,5 +33,17 @@
 int smb_gss_oid_equal(const gss_OID first_oid, const gss_OID second_oid);
 #endif /* HAVE_GSS_OID_EQUAL */
 
+/* wrapper around gss_krb5_import_cred() that prefers to use gss_acquire_cred_from()
+ * if this GSSAPI extension is available. gss_acquire_cred_from() is properly
+ * interposed by GSS-proxy while gss_krb5_import_cred() is not.
+ *
+ * This wrapper requires a proper krb5_context to resolve the ccache name for
+ * gss_acquire_cred_from().
+ *
+ * All gss_krb5_import_cred() callers in Samba already have krb5_context available. */
+uint32_t smb_gss_krb5_import_cred(OM_uint32 *minor_status, krb5_context ctx,
+				  krb5_ccache id, krb5_principal keytab_principal,
+				  krb5_keytab keytab, gss_cred_id_t *cred);
+
 #endif /* HAVE_GSSAPI */
 #endif /* _GSS_SAMBA_H */
-- 
2.9.3


>From 884528075f32b0747ebffbddd50616da9f2a83e0 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab at samba.org>
Date: Fri, 3 Mar 2017 16:57:13 +0200
Subject: [PATCH 3/5] credentials_krb5: convert to use smb_gss_krb5_import_cred

---
 auth/credentials/credentials_krb5.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/auth/credentials/credentials_krb5.c b/auth/credentials/credentials_krb5.c
index d2a655e..6544e42 100644
--- a/auth/credentials/credentials_krb5.c
+++ b/auth/credentials/credentials_krb5.c
@@ -717,8 +717,9 @@ _PUBLIC_ int cli_credentials_get_client_gss_creds(struct cli_credentials *cred,
 		return ENOMEM;
 	}
 
-	maj_stat = gss_krb5_import_cred(&min_stat, ccache->ccache, NULL, NULL, 
-					&gcc->creds);
+	maj_stat = smb_gss_krb5_import_cred(&min_stat, ccache->smb_krb5_context->krb5_context,
+					    ccache->ccache, NULL, NULL,
+					    &gcc->creds);
 	if ((maj_stat == GSS_S_FAILURE) &&
 	    (min_stat == (OM_uint32)KRB5_CC_END ||
 	     min_stat == (OM_uint32)KRB5_CC_NOTFOUND ||
@@ -735,8 +736,9 @@ _PUBLIC_ int cli_credentials_get_client_gss_creds(struct cli_credentials *cred,
 			return ret;
 		}
 
-		maj_stat = gss_krb5_import_cred(&min_stat, ccache->ccache, NULL, NULL,
-						&gcc->creds);
+		maj_stat = smb_gss_krb5_import_cred(&min_stat, ccache->smb_krb5_context->krb5_context,
+						    ccache->ccache, NULL, NULL,
+						    &gcc->creds);
 
 	}
 
@@ -747,7 +749,7 @@ _PUBLIC_ int cli_credentials_get_client_gss_creds(struct cli_credentials *cred,
 		} else {
 			ret = EINVAL;
 		}
-		(*error_string) = talloc_asprintf(cred, "gss_krb5_import_cred failed: %s", error_message(ret));
+		(*error_string) = talloc_asprintf(cred, "smb_gss_krb5_import_cred failed: %s", error_message(ret));
 		return ret;
 	}
 
@@ -1215,12 +1217,14 @@ _PUBLIC_ int cli_credentials_get_server_gss_creds(struct cli_credentials *cred,
 
 	if (ktc->password_based || obtained < CRED_SPECIFIED) {
 		/* This creates a GSSAPI cred_id_t for match-by-key with only the keytab set */
-		maj_stat = gss_krb5_import_cred(&min_stat, NULL, NULL, ktc->keytab,
-						&gcc->creds);
+		maj_stat = smb_gss_krb5_import_cred(&min_stat, smb_krb5_context->krb5_context,
+						    NULL, NULL, ktc->keytab,
+						    &gcc->creds);
 	} else {
 		/* This creates a GSSAPI cred_id_t with the principal and keytab set, matching by name */
-		maj_stat = gss_krb5_import_cred(&min_stat, NULL, princ, ktc->keytab,
-						&gcc->creds);
+		maj_stat = smb_gss_krb5_import_cred(&min_stat, smb_krb5_context->krb5_context,
+						    NULL, princ, ktc->keytab,
+						    &gcc->creds);
 	}
 	if (maj_stat) {
 		if (min_stat) {
-- 
2.9.3


>From eb2ca7e0eed8042e0ad4f2927cb56e6f233fe95a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab at samba.org>
Date: Fri, 3 Mar 2017 16:57:50 +0200
Subject: [PATCH 4/5] libads: convert to use smb_gss_krb5_import_cred

---
 source3/libads/sasl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/libads/sasl.c b/source3/libads/sasl.c
index 8570788..30127fa 100644
--- a/source3/libads/sasl.c
+++ b/source3/libads/sasl.c
@@ -372,7 +372,7 @@ static ADS_STATUS ads_init_gssapi_cred(ADS_STRUCT *ads, gss_cred_id_t *cred)
 		goto done;
 	}
 
-	maj = gss_krb5_import_cred(&min, kccache, NULL, NULL, cred);
+	maj = smb_gss_krb5_import_cred(&min, kctx, kccache, NULL, NULL, cred);
 	if (maj != GSS_S_COMPLETE) {
 		status = ADS_ERROR_GSS(maj, min);
 		goto done;
-- 
2.9.3


>From d9ee506d3721a8d259413ee1ce1ed5041168b563 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab at samba.org>
Date: Fri, 3 Mar 2017 16:58:14 +0200
Subject: [PATCH 5/5] s3-gse: convert to use smb_gss_krb5_import_cred

---
 source3/librpc/crypto/gse.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/source3/librpc/crypto/gse.c b/source3/librpc/crypto/gse.c
index abf20bc..f4238f3 100644
--- a/source3/librpc/crypto/gse.c
+++ b/source3/librpc/crypto/gse.c
@@ -252,11 +252,12 @@ static NTSTATUS gse_init_client(TALLOC_CTX *mem_ctx,
 	/* TODO: get krb5 ticket using username/password, if no valid
 	 * one already available in ccache */
 
-	gss_maj = gss_krb5_import_cred(&gss_min,
-				       gse_ctx->ccache,
-				       NULL, /* keytab_principal */
-				       NULL, /* keytab */
-				       &gse_ctx->creds);
+	gss_maj = smb_gss_krb5_import_cred(&gss_min,
+					   gse_ctx->k5ctx,
+					   gse_ctx->ccache,
+					   NULL, /* keytab_principal */
+					   NULL, /* keytab */
+					   &gse_ctx->creds);
 	if (gss_maj) {
 		char *ccache = NULL;
 		int kret;
@@ -268,7 +269,7 @@ static NTSTATUS gse_init_client(TALLOC_CTX *mem_ctx,
 			ccache = NULL;
 		}
 
-		DEBUG(5, ("gss_krb5_import_cred ccache[%s] failed with [%s] -"
+		DEBUG(5, ("smb_gss_krb5_import_cred ccache[%s] failed with [%s] -"
 			  "the caller may retry after a kinit.\n",
 			  ccache, gse_errstr(gse_ctx, gss_maj, gss_min)));
 		SAFE_FREE(ccache);
@@ -430,12 +431,13 @@ static NTSTATUS gse_init_server(TALLOC_CTX *mem_ctx,
 	}
 
 	/* This creates a GSSAPI cred_id_t with the keytab set */
-	gss_maj = gss_krb5_import_cred(&gss_min, NULL, NULL, gse_ctx->keytab, 
-				       &gse_ctx->creds);
+	gss_maj = smb_gss_krb5_import_cred(&gss_min, gse_ctx->k5ctx,
+					   NULL, NULL, gse_ctx->keytab,
+					   &gse_ctx->creds);
 
 	if (gss_maj != 0
 	    && gss_maj != (GSS_S_CALL_BAD_STRUCTURE|GSS_S_BAD_NAME)) {
-		DEBUG(0, ("gss_krb5_import_cred failed with [%s]\n",
+		DEBUG(0, ("smb_gss_krb5_import_cred failed with [%s]\n",
 			  gse_errstr(gse_ctx, gss_maj, gss_min)));
 		status = NT_STATUS_INTERNAL_ERROR;
 		goto done;
-- 
2.9.3



More information about the samba-technical mailing list