Message ID | 1364308888.2660.119.camel@willson.li.ssimo.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 26/03/13 10:41, Simo Sorce wrote: > Libgssglue is not really useful anymore, it is a sort of middleman that > wraps the actual GSSAPI that is already pluggable/extensible via shared > modules. > > In particular libgssglue interferes with the workings of gss-proxy in my > case. > > The attached patch makes building against libgssglue optional and > defaults to not build against libgssglue and instead builds directly > against the native GSSAPI. > > ./configure --enable-gss > will now build against GSSAPI > > ./configure --enable-gss --with-gssglue > will keep building against libgssglue in case someone still needs it for > whatever reason. > > Simo. > Committed.... steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-03-26 at 10:41 -0400, Simo Sorce wrote: > Libgssglue is not really useful anymore, it is a sort of middleman that > wraps the actual GSSAPI that is already pluggable/extensible via shared > modules. > > In particular libgssglue interferes with the workings of gss-proxy in my > case. > > The attached patch makes building against libgssglue optional and > defaults to not build against libgssglue and instead builds directly > against the native GSSAPI. > > ./configure --enable-gss > will now build against GSSAPI > > ./configure --enable-gss --with-gssglue > will keep building against libgssglue in case someone still needs it for > whatever reason. > > Simo. > Won't that be a backward compatibility issue?
On Tue, 2013-03-26 at 15:25 +0000, Myklebust, Trond wrote: > On Tue, 2013-03-26 at 10:41 -0400, Simo Sorce wrote: > > Libgssglue is not really useful anymore, it is a sort of middleman that > > wraps the actual GSSAPI that is already pluggable/extensible via shared > > modules. > > > > In particular libgssglue interferes with the workings of gss-proxy in my > > case. > > > > The attached patch makes building against libgssglue optional and > > defaults to not build against libgssglue and instead builds directly > > against the native GSSAPI. > > > > ./configure --enable-gss > > will now build against GSSAPI > > > > ./configure --enable-gss --with-gssglue > > will keep building against libgssglue in case someone still needs it for > > whatever reason. > > > > Simo. > > > > Won't that be a backward compatibility issue? What are you worried about exactly ? Is there a use case we should know about ? Simo.
On Tue, 2013-03-26 at 11:37 -0400, Simo Sorce wrote: > On Tue, 2013-03-26 at 15:25 +0000, Myklebust, Trond wrote: > > On Tue, 2013-03-26 at 10:41 -0400, Simo Sorce wrote: > > > Libgssglue is not really useful anymore, it is a sort of middleman that > > > wraps the actual GSSAPI that is already pluggable/extensible via shared > > > modules. > > > > > > In particular libgssglue interferes with the workings of gss-proxy in my > > > case. > > > > > > The attached patch makes building against libgssglue optional and > > > defaults to not build against libgssglue and instead builds directly > > > against the native GSSAPI. > > > > > > ./configure --enable-gss > > > will now build against GSSAPI > > > > > > ./configure --enable-gss --with-gssglue > > > will keep building against libgssglue in case someone still needs it for > > > whatever reason. > > > > > > Simo. > > > > > > > Won't that be a backward compatibility issue? > > What are you worried about exactly ? Is there a use case we should know > about ? Building nfs-utils against existing older setups. There are a lot of users out there of 4-5 year old distros (including ones maintained by your employer). Why isn't it safe to assume that if someone has libgssglue installed, then we should be building nfs-utils against it?
On Tue, 2013-03-26 at 15:43 +0000, Myklebust, Trond wrote: > On Tue, 2013-03-26 at 11:37 -0400, Simo Sorce wrote: > > On Tue, 2013-03-26 at 15:25 +0000, Myklebust, Trond wrote: > > > On Tue, 2013-03-26 at 10:41 -0400, Simo Sorce wrote: > > > > Libgssglue is not really useful anymore, it is a sort of middleman that > > > > wraps the actual GSSAPI that is already pluggable/extensible via shared > > > > modules. > > > > > > > > In particular libgssglue interferes with the workings of gss-proxy in my > > > > case. > > > > > > > > The attached patch makes building against libgssglue optional and > > > > defaults to not build against libgssglue and instead builds directly > > > > against the native GSSAPI. > > > > > > > > ./configure --enable-gss > > > > will now build against GSSAPI > > > > > > > > ./configure --enable-gss --with-gssglue > > > > will keep building against libgssglue in case someone still needs it for > > > > whatever reason. > > > > > > > > Simo. > > > > > > > > > > Won't that be a backward compatibility issue? > > > > What are you worried about exactly ? Is there a use case we should know > > about ? > > Building nfs-utils against existing older setups. There are a lot of > users out there of 4-5 year old distros (including ones maintained by > your employer). > > Why isn't it safe to assume that if someone has libgssglue installed, > then we should be building nfs-utils against it? A foillowing patch for nfs-utils is coming, both the libtirpc and nfs-utils patches will make building against libgssglue optional. I see no value for selecting libgssglue by default though. libgssglue does nothing but load the underlying gssapi library by default anyway and adds nothing to that. I am leaving the option to compile against libgssglue exactly for the odd case where someone has a good reason to use it (very, very old systems). In that case all they need to do is to run configure with --enable-gss --with-gssglue Simo.
Doesn't nfs-utils build without gssglue if libgssglue is not installed? If that's true I see no reason to change the default. If you don't want gssglue, uninstall it. If you have it installed and want to build without it, use --without-gssglue. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-03-26 at 11:56 -0400, Jim Rees wrote: > Doesn't nfs-utils build without gssglue if libgssglue is not installed? Not really, both libtirpc needs --enable-gss, if yo pass that they check only for libgssglue right now and fail if not available. > If that's true I see no reason to change the default. If you don't want > gssglue, uninstall it. If you have it installed and want to build without > it, use --without-gssglue. I am changing the default to build against system GSSAPI libraries when --enable-gss is passed in, if you want to force to build against libgssglue for some specific reason you use --with-gssglue I believe building against system libraries instead of a shim is the better default in the general case. Simo.
Simo Sorce <simo@...> writes: Hi, If you've already mentioned the gssglue issue, there's a related one, namely, building nfs-utils against Heimdal. Currently, the out of the box Heimdal support is broken, and most of the breakage comes out of the gssglue. I'm looking at fixing nfs-utils to support Heimdal properly - currently my only remaining problem is to fix the configure and pkg-config scripts in both nfs- utils and libgssglue (if this one is not dropped for good, and I personally think it should be; small, icky library on no real use). Here is the code patch I'm using for my Heimdal build: diff -ur nfs-utils-1.2.6.orig/utils/gssd/context_lucid.c nfs-utils- 1.2.6/utils/gssd/context_lucid.c --- nfs-utils-1.2.6.orig/utils/gssd/context_lucid.c 2012-05-15 00:40:52.000000000 +1000 +++ nfs-utils-1.2.6/utils/gssd/context_lucid.c 2013-03-26 19:03:10.096586556 +1100 @@ -266,10 +266,10 @@ int retcode = 0; printerr(2, "DEBUG: %s: lucid version!\n", __FUNCTION__); - maj_stat = gss_export_lucid_sec_context(&min_stat, &ctx, - 1, &return_ctx); + maj_stat = gss_krb5_export_lucid_sec_context(&min_stat, &ctx, + - 1, &return_ctx); if (maj_stat != GSS_S_COMPLETE) { - pgsserr("gss_export_lucid_sec_context", + pgsserr("gss_krb5_export_lucid_sec_context", maj_stat, min_stat, &krb5oid); goto out_err; } @@ -302,9 +302,9 @@ else retcode = prepare_krb5_rfc4121_buffer(lctx, buf, endtime); - maj_stat = gss_free_lucid_sec_context(&min_stat, ctx, return_ctx); + maj_stat = gss_krb5_free_lucid_sec_context(&min_stat, ctx); if (maj_stat != GSS_S_COMPLETE) { - pgsserr("gss_free_lucid_sec_context", + pgsserr("gss_krb5_free_lucid_sec_context", maj_stat, min_stat, &krb5oid); printerr(0, "WARN: failed to free lucid sec context\n"); } diff -ur nfs-utils-1.2.6.orig/utils/gssd/krb5_util.c nfs-utils- 1.2.6/utils/gssd/krb5_util.c --- nfs-utils-1.2.6.orig/utils/gssd/krb5_util.c 2012-05-15 00:40:52.000000000 +1000 +++ nfs-utils-1.2.6/utils/gssd/krb5_util.c 2013-03-26 19:18:40.204045067 +1100 @@ -115,7 +115,7 @@ #include <errno.h> #include <time.h> #include <gssapi/gssapi.h> -#ifdef USE_PRIVATE_KRB5_FUNCTIONS +#if defined(USE_PRIVATE_KRB5_FUNCTIONS) || defined(HAVE_HEIMDAL) #include <gssapi/gssapi_krb5.h> #endif #include <krb5.h> @@ -936,9 +936,38 @@ { krb5_error_code ret; krb5_creds creds; - krb5_cc_cursor cur; int found = 0; +#if defined (HAVE_HEIMDAL) + krb5_creds pattern; + krb5_const_realm client_realm; + + krb5_cc_clear_mcred(&pattern); + + client_realm = krb5_principal_get_realm(context, principal); + + ret = krb5_make_principal(context, &pattern.server, + client_realm, KRB5_TGS_NAME, client_realm, + NULL); + if (ret) + krb5_err(context, 1, ret, "krb5_make_principal"); + pattern.client = principal; + + ret = krb5_cc_retrieve_cred(context, ccache, 0, &pattern, &creds); + krb5_free_principal(context, pattern.server); + if (ret) { + if (ret == KRB5_CC_END) + return 1; + krb5_err(context, 1, ret, "krb5_cc_retrieve_cred"); + } + + found = creds.times.endtime > time(NULL); + + krb5_free_cred_contents (context, &creds); +#else + krb5_cc_cursor cur; + + ret = krb5_cc_start_seq_get(context, ccache, &cur); if (ret) return 0; @@ -958,7 +987,7 @@ krb5_free_cred_contents(context, &creds); } krb5_cc_end_seq_get(context, ccache, &cur); - +#endif return found; } @@ -1278,7 +1307,7 @@ return strdup(error_message(code)); #else if (context != NULL) - return strdup(krb5_get_err_text(context, code)); + return strdup(krb5_get_error_message(context, code)); else return strdup(error_message(code)); #endif @@ -1347,11 +1376,11 @@ * list of supported enctypes, use local default here. */ if (krb5_enctypes == NULL || limit_to_legacy_enctypes) - maj_stat = gss_set_allowable_enctypes(&min_stat, credh, - &krb5oid, num_enctypes, enctypes); + maj_stat = gss_krb5_set_allowable_enctypes(&min_stat, credh, + num_enctypes, enctypes); else - maj_stat = gss_set_allowable_enctypes(&min_stat, credh, - &krb5oid, num_krb5_enctypes, krb5_enctypes); + maj_stat = gss_krb5_set_allowable_enctypes(&min_stat, credh, + num_krb5_enctypes, krb5_enctypes); if (maj_stat != GSS_S_COMPLETE) { pgsserr("gss_set_allowable_enctypes", -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
CC-ing Simo since he is not on this list... On 26/03/13 21:14, Alex Dubov wrote: > Simo Sorce <simo@...> writes: > > Hi, > > If you've already mentioned the gssglue issue, there's a related one, namely, > building nfs-utils against Heimdal. > > Currently, the out of the box Heimdal support is broken, and most of the > breakage comes out of the gssglue. > > I'm looking at fixing nfs-utils to support Heimdal properly - currently my only > remaining problem is to fix the configure and pkg-config scripts in both nfs- > utils and libgssglue (if this one is not dropped for good, and I personally > think it should be; small, icky library on no real use). > > Here is the code patch I'm using for my Heimdal build: > > diff -ur nfs-utils-1.2.6.orig/utils/gssd/context_lucid.c nfs-utils- > 1.2.6/utils/gssd/context_lucid.c > --- nfs-utils-1.2.6.orig/utils/gssd/context_lucid.c 2012-05-15 > 00:40:52.000000000 +1000 > +++ nfs-utils-1.2.6/utils/gssd/context_lucid.c 2013-03-26 19:03:10.096586556 > +1100 > @@ -266,10 +266,10 @@ > int retcode = 0; > > printerr(2, "DEBUG: %s: lucid version!\n", __FUNCTION__); > - maj_stat = gss_export_lucid_sec_context(&min_stat, &ctx, > - 1, &return_ctx); > + maj_stat = gss_krb5_export_lucid_sec_context(&min_stat, &ctx, > + - 1, &return_ctx); > if (maj_stat != GSS_S_COMPLETE) { > - pgsserr("gss_export_lucid_sec_context", > + pgsserr("gss_krb5_export_lucid_sec_context", > maj_stat, min_stat, &krb5oid); > goto out_err; > } > @@ -302,9 +302,9 @@ > else > retcode = prepare_krb5_rfc4121_buffer(lctx, buf, endtime); > > - maj_stat = gss_free_lucid_sec_context(&min_stat, ctx, return_ctx); > + maj_stat = gss_krb5_free_lucid_sec_context(&min_stat, ctx); > if (maj_stat != GSS_S_COMPLETE) { > - pgsserr("gss_free_lucid_sec_context", > + pgsserr("gss_krb5_free_lucid_sec_context", > maj_stat, min_stat, &krb5oid); > printerr(0, "WARN: failed to free lucid sec context\n"); > } > diff -ur nfs-utils-1.2.6.orig/utils/gssd/krb5_util.c nfs-utils- > 1.2.6/utils/gssd/krb5_util.c > --- nfs-utils-1.2.6.orig/utils/gssd/krb5_util.c 2012-05-15 00:40:52.000000000 > +1000 > +++ nfs-utils-1.2.6/utils/gssd/krb5_util.c 2013-03-26 19:18:40.204045067 > +1100 > @@ -115,7 +115,7 @@ > #include <errno.h> > #include <time.h> > #include <gssapi/gssapi.h> > -#ifdef USE_PRIVATE_KRB5_FUNCTIONS > +#if defined(USE_PRIVATE_KRB5_FUNCTIONS) || defined(HAVE_HEIMDAL) > #include <gssapi/gssapi_krb5.h> > #endif > #include <krb5.h> > @@ -936,9 +936,38 @@ > { > krb5_error_code ret; > krb5_creds creds; > - krb5_cc_cursor cur; > int found = 0; > > +#if defined (HAVE_HEIMDAL) > + krb5_creds pattern; > + krb5_const_realm client_realm; > + > + krb5_cc_clear_mcred(&pattern); > + > + client_realm = krb5_principal_get_realm(context, principal); > + > + ret = krb5_make_principal(context, &pattern.server, > + client_realm, KRB5_TGS_NAME, client_realm, > + NULL); > + if (ret) > + krb5_err(context, 1, ret, "krb5_make_principal"); > + pattern.client = principal; > + > + ret = krb5_cc_retrieve_cred(context, ccache, 0, &pattern, &creds); > + krb5_free_principal(context, pattern.server); > + if (ret) { > + if (ret == KRB5_CC_END) > + return 1; > + krb5_err(context, 1, ret, "krb5_cc_retrieve_cred"); > + } > + > + found = creds.times.endtime > time(NULL); > + > + krb5_free_cred_contents (context, &creds); > +#else > + krb5_cc_cursor cur; > + > + This bug huge ifdef is ugly... ;-) Can we redefine what check_for_tgt() contains depending on HAVE_HEIMDAL and HAVE_KRB5? > ret = krb5_cc_start_seq_get(context, ccache, &cur); > if (ret) > return 0; > @@ -958,7 +987,7 @@ > krb5_free_cred_contents(context, &creds); > } > krb5_cc_end_seq_get(context, ccache, &cur); > - > +#endif > return found; > } > > @@ -1278,7 +1307,7 @@ > return strdup(error_message(code)); > #else > if (context != NULL) > - return strdup(krb5_get_err_text(context, code)); > + return strdup(krb5_get_error_message(context, code)); Not sure why this is needed since they are both define in the krb5 libs Does krb5_get_error_message() give better error messages? steved. > else > return strdup(error_message(code)); > #endif > @@ -1347,11 +1376,11 @@ > * list of supported enctypes, use local default here. > */ > if (krb5_enctypes == NULL || limit_to_legacy_enctypes) > - maj_stat = gss_set_allowable_enctypes(&min_stat, credh, > - &krb5oid, num_enctypes, enctypes); > + maj_stat = gss_krb5_set_allowable_enctypes(&min_stat, credh, > + num_enctypes, enctypes); > else > - maj_stat = gss_set_allowable_enctypes(&min_stat, credh, > - &krb5oid, num_krb5_enctypes, > krb5_enctypes); > + maj_stat = gss_krb5_set_allowable_enctypes(&min_stat, credh, > + num_krb5_enctypes, krb5_enctypes); > if (maj_stat != GSS_S_COMPLETE) { > pgsserr("gss_set_allowable_enctypes", > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 8518f6857b928e9ec615f9ca5f79f98d13db4f6d Mon Sep 17 00:00:00 2001 From: Simo Sorce <simo@redhat.com> Date: Tue, 26 Mar 2013 10:15:56 -0400 Subject: [PATCH] Switch to use standard GSSAPI by default Make libgssglue configurable still but disabled by default. There is no reason to use libgssglue anymore, and modern gssapi supports all needed features for libtirpc and its dependencies. --- configure.ac | 23 +++++++++++++++++++---- src/Makefile.am | 4 ++-- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 40dce96..4a4adba 100644 --- a/configure.ac +++ b/configure.ac @@ -5,15 +5,30 @@ AC_CONFIG_SRCDIR([src/auth_des.c]) AC_CONFIG_MACRO_DIR([m4]) AC_ARG_ENABLE(gss,[ --enable-gss Turn on gss api], [case "${enableval}" in - yes) gss=true ; AC_CHECK_LIB([gssapi],[gss_init_sec_context]) ;; + yes) gss=true ;; no) gss=false ;; *) AC_MSG_ERROR(bad value ${enableval} for --enable-gss) ;; esac],[gss=false]) AM_CONDITIONAL(GSS, test x$gss = xtrue) +AC_ARG_WITH(gssglue, + [ --with-gssglue Use libgssglue], + [case "${enableval}" in + yes) gssglue=true ;; + no) gssglue=false ;; + *) AC_MSG_ERROR(bad value ${enableval} for --with-gssglue) ;; + esac], + [gssglue=false]) +AM_CONDITIONAL(USEGSSGLUE, test x$gssglue = xtrue) if test x$gss = xtrue; then - AC_DEFINE(HAVE_LIBGSSAPI, 1, []) - PKG_CHECK_MODULES(GSSGLUE, libgssglue, [], - AC_MSG_ERROR([Unable to locate information required to use libgssglue.])) + if test x$gssglue = xtrue; then + PKG_CHECK_MODULES(GSSAPI, libgssglue, [], + AC_MSG_ERROR([Unable to locate information required to use libgssglue.])) + else + GSSAPI_CFLAGS=`krb5-config --cflags gssapi` + GSSAPI_LIBS=`krb5-config --libs gssapi` + AC_SUBST([GSSAPI_CFLAGS]) + AC_SUBST([GSSAPI_LIBS]) + fi fi AC_ARG_ENABLE(ipv6, [AC_HELP_STRING([--disable-ipv6], [Disable IPv6 support @<:@default=no@:>@])], diff --git a/src/Makefile.am b/src/Makefile.am index 66350f5..2dd7768 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -58,8 +58,8 @@ libtirpc_la_SOURCES += xdr.c xdr_rec.c xdr_array.c xdr_float.c xdr_mem.c xdr_ref ## Secure-RPC if GSS libtirpc_la_SOURCES += auth_gss.c authgss_prot.c svc_auth_gss.c - libtirpc_la_LDFLAGS += $(GSSGLUE_LIBS) - libtirpc_la_CFLAGS = -DHAVE_RPCSEC_GSS $(GSSGLUE_CFLAGS) + libtirpc_la_LDFLAGS += $(GSSAPI_LIBS) + libtirpc_la_CFLAGS = -DHAVE_RPCSEC_GSS $(GSSAPI_CFLAGS) endif ## libtirpc_a_SOURCES += key_call.c key_prot_xdr.c getpublickey.c -- 1.8.1.4