2013-03-26 14:41:38

by Simo Sorce

[permalink] [raw]
Subject: Allow building libtirpc directly against GSSAPI

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.

--
Simo Sorce * Red Hat, Inc * New York


Attachments:
0001-Switch-to-use-standard-GSSAPI-by-default.patch (2.52 kB)

2013-03-26 15:37:58

by Simo Sorce

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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.

--
Simo Sorce * Red Hat, Inc * New York


2013-03-26 15:50:11

by Simo Sorce

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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.

--
Simo Sorce * Red Hat, Inc * New York


2013-03-26 16:57:07

by Simo Sorce

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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 * Red Hat, Inc * New York


2013-03-26 15:25:08

by Myklebust, Trond

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-03-27 01:39:50

by Alex Dubov

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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",



2013-03-26 15:43:58

by Myklebust, Trond

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-03-26 15:57:00

by Jim Rees

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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.

2013-03-26 15:22:31

by Steve Dickson

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI



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.

2013-04-02 18:02:45

by Steve Dickson

[permalink] [raw]
Subject: Re: Allow building libtirpc directly against GSSAPI

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>