diff mbox

Allow building libtirpc directly against GSSAPI

Message ID 1364308888.2660.119.camel@willson.li.ssimo.org (mailing list archive)
State New, archived
Headers show

Commit Message

Simo Sorce March 26, 2013, 2:41 p.m. UTC
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.

Comments

Steve Dickson March 26, 2013, 3:22 p.m. UTC | #1
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
Trond Myklebust March 26, 2013, 3:25 p.m. UTC | #2
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?
Simo Sorce March 26, 2013, 3:37 p.m. UTC | #3
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.
Trond Myklebust March 26, 2013, 3:43 p.m. UTC | #4
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?
Simo Sorce March 26, 2013, 3:49 p.m. UTC | #5
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.
Jim Rees March 26, 2013, 3:56 p.m. UTC | #6
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
Simo Sorce March 26, 2013, 4:56 p.m. UTC | #7
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.
Alex Dubov March 27, 2013, 1:14 a.m. UTC | #8
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
Steve Dickson April 2, 2013, 6:02 p.m. UTC | #9
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
diff mbox

Patch

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