* Seeking for upstreaming patches which floating in Gentoo
@ 2013-07-22 14:59 Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 1/6] fix compile error with heimdal support enabled Lan Yixun (dlan)
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs
Hi Ian:
I'm trying to push a few patches which floating in gentoo linux to
upstream.
My initial motivation is trying to work closely with upstream, and
aim to keep downstream package much more clean at next release.
I'm seeing tree possible results/feedbacks, a) accept patches, which
is perfect! b) need improvement, I can work on and polish. c) impossible
to accept, in this case, I'd like to hear your idea to decide
whether to drop or still carry on.
thanks
Dennis lan (dlan)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/6] fix compile error with heimdal support enabled
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
@ 2013-07-22 14:59 ` Lan Yixun (dlan)
2013-07-23 9:44 ` Ian Kent
2013-07-22 14:59 ` [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled Lan Yixun (dlan)
` (4 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs; +Cc: Lan Yixun (dlan)
From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
this patch instroduce a compatible layer between Heimdal and MTT Krb5.
And I slightly rework the original patch to make it more readable.
---
Upstream Discussion:
http://thread.gmane.org/gmane.linux.kernel.autofs/4203
Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=210762
Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
aclocal.m4 | 7 +++++++
modules/cyrus-sasl.c | 35 ++++++++++++++++++++++++++++++-----
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/aclocal.m4 b/aclocal.m4
index c5de159..7a8b03c 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -299,6 +299,13 @@ else
HAVE_KRB5=1
KRB5_LIBS=`$KRB5_CONFIG --libs`
KRB5_FLAGS=`$KRB5_CONFIG --cflags`
+
+ SAVE_CFLAGS=$CFLAGS
+ SAVE_LIBS=$LIBS
+ CFLAGS="$CFLAGS $KRB5_FLAGS"
+ LIBS="$LIBS $KRB5_LIBS"
+
+ AC_CHECK_FUNCS([krb5_principal_get_realm])
fi])
dnl --------------------------------------------------------------------------
diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
index 68f9242..6115f90 100644
--- a/modules/cyrus-sasl.c
+++ b/modules/cyrus-sasl.c
@@ -64,6 +64,31 @@
#endif
#endif
+#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+ const char **realm, int *len)
+{
+ *realm = krb5_principal_get_realm(context, princ);
+ *len = strlen(*realm);
+}
+#else
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+ const char **realm, int *len)
+{
+ const krb5_data *data;
+
+ data = krb5_princ_realm(context, princ);
+ if (data) {
+ *realm = data->data;
+ *len = data->length;
+ } else {
+ *realm = NULL;
+ *len = 0;
+ }
+}
+#endif
+
+
/*
* Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
* environment variable so that the library knows where to find it.
@@ -379,7 +404,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
krb5_principal tgs_princ, krb5_client_princ;
krb5_creds my_creds;
char *tgs_name;
- int status;
+ const char *realm_name;
+ int status, realm_length;
if (ctxt->kinit_done)
return 0;
@@ -450,12 +476,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
}
/* setup a principal for the ticket granting service */
+ _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
- krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
- krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
+ realm_length, realm_name,
strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
- krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
- krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
+ realm_length, realm_name,
0);
if (ret) {
error(logopt,
--
1.8.2.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 1/6] fix compile error with heimdal support enabled Lan Yixun (dlan)
@ 2013-07-22 14:59 ` Lan Yixun (dlan)
2013-07-23 9:50 ` Ian Kent
2013-07-22 14:59 ` [PATCH 3/6] fix compile error with LDAP enabled but SASL disabled Lan Yixun (dlan)
` (3 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs; +Cc: Lan Yixun (dlan)
From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
autofs will create symbol link mandatory no matter ldap support
is enabled or not. so, without this patch, lookup_ldaps.so will become
a dead link.
---
Upstream Discussion:
http://thread.gmane.org/gmane.linux.kernel.autofs/5371
Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
modules/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/modules/Makefile b/modules/Makefile
index c5deb24..4bb1096 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -74,7 +74,9 @@ install: all
-rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
+ifeq ($(SASL), 1)
ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
+endif
ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
ifeq ($(EXT2FS), 1)
ifeq ($(EXT3FS), 1)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/6] fix compile error with LDAP enabled but SASL disabled
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 1/6] fix compile error with heimdal support enabled Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled Lan Yixun (dlan)
@ 2013-07-22 14:59 ` Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c Lan Yixun (dlan)
` (2 subsequent siblings)
5 siblings, 0 replies; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs; +Cc: Lan Yixun (dlan)
From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
automount[10812]: open_lookup:90: cannot open lookup module ldap
(/usr/lib64/autofs/lookup_ldap.so: undefined symbol: parse_ldap_config)
---
Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=381315
Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
modules/lookup_ldap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index a2bfafd..b8970a6 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -1484,6 +1484,7 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
}
}
+#ifdef WITH_SASL
/*
* First, check to see if a preferred authentication method was
* specified by the user. parse_ldap_config will return error
@@ -1496,7 +1497,6 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
return 1;
}
-#ifdef WITH_SASL
/* Init the sasl callbacks */
if (!autofs_sasl_client_init(LOGOPT_NONE)) {
error(LOGOPT_ANY, "failed to init sasl client");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
` (2 preceding siblings ...)
2013-07-22 14:59 ` [PATCH 3/6] fix compile error with LDAP enabled but SASL disabled Lan Yixun (dlan)
@ 2013-07-22 14:59 ` Lan Yixun (dlan)
2013-07-23 10:17 ` Ian Kent
2013-07-22 14:59 ` [PATCH 5/6] add missing WITH_SASL " Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 6/6] fix typo, forced-shutdown should be force-shutdown Lan Yixun (dlan)
5 siblings, 1 reply; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs; +Cc: Lan Yixun (dlan)
From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
make "libxml/tree.h" controlled by WITH_SASL, also move it beind line of
#include "automount.h", since WITH_SASL is defined in config.h which is
included by automout.h
---
Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=468606
gcc -shared -O2 -pipe -march=core2 -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_REENTRANT
-D_FILE_OFFSET_BITS=
64 -I../include -I../lib -fPIC -D_GNU_SOURCE -DAUTOFS_LIB_DIR=\"/usr/lib64/autofs\"
-DAUTOFS_MAP_DIR=\"/e
tc/autofs\" -DLDAP_DEPRECATED=1 -o lookup_ldap.so \
lookup_ldap.c dclist.o base64.o \
../lib/autofs.a -lldap -llber -lresolv
: lookup_yp.so
lookup_ldap.c:31:25: fatal error: libxml/tree.h: No such file or directory
#include <libxml/tree.h>
^
compilation terminated.
Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
modules/lookup_ldap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index b8970a6..fad558d 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -28,7 +28,6 @@
#include <arpa/nameser.h>
#include <resolv.h>
#include <lber.h>
-#include <libxml/tree.h>
#define MODULE_LOOKUP
#include "automount.h"
@@ -36,6 +35,10 @@
#include "lookup_ldap.h"
#include "base64.h"
+#ifdef WITH_SASL
+#include <libxml/tree.h>
+#endif
+
#define MAPFMT_DEFAULT "sun"
#define MODPREFIX "lookup(ldap): "
--
1.8.2.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/6] add missing WITH_SASL in modules lookup_ldap.c
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
` (3 preceding siblings ...)
2013-07-22 14:59 ` [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c Lan Yixun (dlan)
@ 2013-07-22 14:59 ` Lan Yixun (dlan)
2013-07-23 10:35 ` Ian Kent
2013-07-22 14:59 ` [PATCH 6/6] fix typo, forced-shutdown should be force-shutdown Lan Yixun (dlan)
5 siblings, 1 reply; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs; +Cc: Lan Yixun (dlan)
From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
---
Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=361899
x86_64-pc-linux-gnu-gcc -Wl,-O1 -Wl,--as-needed -Wl,-O1 -Wl,--hash-style=gnu -Wl,--sort-common
-shared -O2 -pipe -march=core2 -mcx16 -msahf -mpopcnt -msse4.2 --param l1-cache-size=32 --param
l1-cache-line-size=64 --param l2-cache-size=8192 -frecord-gcc-switches -g
-Wimplicit-function-declaration -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_REENTRANT
-D_FILE_OFFSET_BITS=64 -I../include -I../lib -fPIC -D_GNU_SOURCE
-DAUTOFS_LIB_DIR=\"/usr/lib64/autofs\" -DAUTOFS_MAP_DIR=\"/etc/autofs\" -DLDAP_DEPRECATED=1 -o
lookup_ldap.so \
lookup_ldap.c dclist.o \
../lib/autofs.a -lldap -llber -lresolv
lookup_ldap.c: In function ‘do_connect’:
lookup_ldap.c:598:10: error: ‘struct lookup_context’ has no member named ‘extern_cert’
lookup_ldap.c:598:31: error: ‘struct lookup_context’ has no member named ‘extern_key’
lookup_ldap.c:599:41: error: ‘struct lookup_context’ has no member named ‘extern_cert’
lookup_ldap.c:600:40: error: ‘struct lookup_context’ has no member named ‘extern_key’
lookup_ldap.c: In function ‘free_context’:
lookup_ldap.c:1379:10: error: ‘struct lookup_context’ has no member named ‘extern_cert’
lookup_ldap.c:1380:12: error: ‘struct lookup_context’ has no member named ‘extern_cert’
lookup_ldap.c:1381:10: error: ‘struct lookup_context’ has no member named ‘extern_key’
lookup_ldap.c:1382:12: error: ‘struct lookup_context’ has no member named ‘extern_key’
make[1]: *** [lookup_ldap.so] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/var/tmp/portage/net-fs/autofs-5.0.5-r2/work/autofs-5.0.5/modules'
make: *** [daemon] Error 2
Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
modules/lookup_ldap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index fad558d..ddfb060 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -588,10 +588,12 @@ static LDAP *do_connect(unsigned logopt, const char *uri, struct lookup_context
{
LDAP *ldap;
+#ifdef WITH_SASL
if (ctxt->extern_cert && ctxt->extern_key) {
set_env(logopt, ENV_LDAPTLS_CERT, ctxt->extern_cert);
set_env(logopt, ENV_LDAPTLS_KEY, ctxt->extern_key);
}
+#endif
ldap = init_ldap_connection(logopt, uri, ctxt);
if (ldap) {
@@ -1393,10 +1395,12 @@ static void free_context(struct lookup_context *ctxt)
defaults_free_searchdns(ctxt->sdns);
if (ctxt->dclist)
free_dclist(ctxt->dclist);
+#ifdef WITH_SASL
if (ctxt->extern_cert)
free(ctxt->extern_cert);
if (ctxt->extern_key)
free(ctxt->extern_key);
+#endif
free(ctxt);
return;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/6] fix typo, forced-shutdown should be force-shutdown
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
` (4 preceding siblings ...)
2013-07-22 14:59 ` [PATCH 5/6] add missing WITH_SASL " Lan Yixun (dlan)
@ 2013-07-22 14:59 ` Lan Yixun (dlan)
2013-07-23 10:37 ` Ian Kent
5 siblings, 1 reply; 25+ messages in thread
From: Lan Yixun (dlan) @ 2013-07-22 14:59 UTC (permalink / raw)
To: autofs; +Cc: Lan Yixun (dlan)
From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
it will confuse user when they found what shows with "./configure --help"
differ from what it should be.
Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
configure.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure.in b/configure.in
index 559045a..0013a39 100644
--- a/configure.in
+++ b/configure.in
@@ -363,7 +363,7 @@ fi
#
# Enable forced shutdown on USR1 signal (unlink umounts all mounts).
#
-AC_ARG_ENABLE(forced-shutdown,
+AC_ARG_ENABLE(force-shutdown,
[ --enable-force-shutdown enable USR1 signal to force unlink umount of any
busy mounts during shutdown],,
enableval=no)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] fix compile error with heimdal support enabled
2013-07-22 14:59 ` [PATCH 1/6] fix compile error with heimdal support enabled Lan Yixun (dlan)
@ 2013-07-23 9:44 ` Ian Kent
2013-07-23 10:19 ` Dennis Lan (dlan)
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 9:44 UTC (permalink / raw)
To: Lan Yixun (dlan); +Cc: autofs
On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>
> this patch instroduce a compatible layer between Heimdal and MTT Krb5.
> And I slightly rework the original patch to make it more readable.
>
> ---
> Upstream Discussion:
> http://thread.gmane.org/gmane.linux.kernel.autofs/4203
>
> Gentoo Bugs:
> https://bugs.gentoo.org/show_bug.cgi?id=210762
>
> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> ---
> aclocal.m4 | 7 +++++++
> modules/cyrus-sasl.c | 35 ++++++++++++++++++++++++++++++-----
> 2 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/aclocal.m4 b/aclocal.m4
> index c5de159..7a8b03c 100644
> --- a/aclocal.m4
> +++ b/aclocal.m4
> @@ -299,6 +299,13 @@ else
> HAVE_KRB5=1
> KRB5_LIBS=`$KRB5_CONFIG --libs`
> KRB5_FLAGS=`$KRB5_CONFIG --cflags`
> +
> + SAVE_CFLAGS=$CFLAGS
> + SAVE_LIBS=$LIBS
> + CFLAGS="$CFLAGS $KRB5_FLAGS"
> + LIBS="$LIBS $KRB5_LIBS"
> +
> + AC_CHECK_FUNCS([krb5_principal_get_realm])
> fi])
>
> dnl --------------------------------------------------------------------------
> diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
> index 68f9242..6115f90 100644
> --- a/modules/cyrus-sasl.c
> +++ b/modules/cyrus-sasl.c
> @@ -64,6 +64,31 @@
> #endif
> #endif
>
> +#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
> + const char **realm, int *len)
> +{
> + *realm = krb5_principal_get_realm(context, princ);
> + *len = strlen(*realm);
So krb5_principal_get_realm() never fails, or does it return NULL, what
about strlen(NULL) .... SEGV.
> +}
> +#else
> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
> + const char **realm, int *len)
> +{
> + const krb5_data *data;
> +
> + data = krb5_princ_realm(context, princ);
> + if (data) {
> + *realm = data->data;
> + *len = data->length;
> + } else {
> + *realm = NULL;
> + *len = 0;
> + }
> +}
> +#endif
> +
> +
> /*
> * Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
> * environment variable so that the library knows where to find it.
> @@ -379,7 +404,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
> krb5_principal tgs_princ, krb5_client_princ;
> krb5_creds my_creds;
> char *tgs_name;
> - int status;
> + const char *realm_name;
> + int status, realm_length;
>
> if (ctxt->kinit_done)
> return 0;
> @@ -450,12 +476,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
> }
>
> /* setup a principal for the ticket granting service */
> + _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
> ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
> + realm_length, realm_name,
> strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
> + realm_length, realm_name,
> 0);
> if (ret) {
> error(logopt,
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-22 14:59 ` [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled Lan Yixun (dlan)
@ 2013-07-23 9:50 ` Ian Kent
2013-07-23 10:10 ` Ian Kent
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 9:50 UTC (permalink / raw)
To: Lan Yixun (dlan); +Cc: autofs
On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>
> autofs will create symbol link mandatory no matter ldap support
> is enabled or not. so, without this patch, lookup_ldaps.so will become
> a dead link.
I have added this to my patch queue.
It will be committed next time I push patches to the repo.
Thanks
>
> ---
> Upstream Discussion:
> http://thread.gmane.org/gmane.linux.kernel.autofs/5371
>
> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> ---
> modules/Makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/modules/Makefile b/modules/Makefile
> index c5deb24..4bb1096 100644
> --- a/modules/Makefile
> +++ b/modules/Makefile
> @@ -74,7 +74,9 @@ install: all
> -rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
> ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
> ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
> +ifeq ($(SASL), 1)
> ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
> +endif
> ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
> ifeq ($(EXT2FS), 1)
> ifeq ($(EXT3FS), 1)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-23 9:50 ` Ian Kent
@ 2013-07-23 10:10 ` Ian Kent
2013-07-23 10:30 ` Dennis Lan (dlan)
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:10 UTC (permalink / raw)
To: Lan Yixun (dlan); +Cc: autofs
On Tue, 2013-07-23 at 17:50 +0800, Ian Kent wrote:
> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> > From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
> >
> > autofs will create symbol link mandatory no matter ldap support
> > is enabled or not. so, without this patch, lookup_ldaps.so will become
> > a dead link.
>
> I have added this to my patch queue.
> It will be committed next time I push patches to the repo.
On second thoughts, what if WITH_LDAP is defined and WITH_SASL is not?
> Thanks
>
> >
> > ---
> > Upstream Discussion:
> > http://thread.gmane.org/gmane.linux.kernel.autofs/5371
> >
> > Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> > ---
> > modules/Makefile | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/modules/Makefile b/modules/Makefile
> > index c5deb24..4bb1096 100644
> > --- a/modules/Makefile
> > +++ b/modules/Makefile
> > @@ -74,7 +74,9 @@ install: all
> > -rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
> > ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
> > ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
> > +ifeq ($(SASL), 1)
> > ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
> > +endif
> > ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
> > ifeq ($(EXT2FS), 1)
> > ifeq ($(EXT3FS), 1)
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe autofs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c
2013-07-22 14:59 ` [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c Lan Yixun (dlan)
@ 2013-07-23 10:17 ` Ian Kent
2013-07-23 10:37 ` Dennis Lan (dlan)
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:17 UTC (permalink / raw)
To: Lan Yixun (dlan); +Cc: autofs
On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>
> make "libxml/tree.h" controlled by WITH_SASL, also move it beind line of
> #include "automount.h", since WITH_SASL is defined in config.h which is
> included by automout.h
Same again, WITH_LDAP defined and WITH_SASL not.
I agree there appears to be a problem but not sure what to do about it.
>
> ---
> Gentoo Bugs:
> https://bugs.gentoo.org/show_bug.cgi?id=468606
>
> gcc -shared -O2 -pipe -march=core2 -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_REENTRANT
> -D_FILE_OFFSET_BITS=
> 64 -I../include -I../lib -fPIC -D_GNU_SOURCE -DAUTOFS_LIB_DIR=\"/usr/lib64/autofs\"
> -DAUTOFS_MAP_DIR=\"/e
> tc/autofs\" -DLDAP_DEPRECATED=1 -o lookup_ldap.so \
> lookup_ldap.c dclist.o base64.o \
> ../lib/autofs.a -lldap -llber -lresolv
> : lookup_yp.so
> lookup_ldap.c:31:25: fatal error: libxml/tree.h: No such file or directory
> #include <libxml/tree.h>
> ^
> compilation terminated.
>
> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> ---
> modules/lookup_ldap.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
> index b8970a6..fad558d 100644
> --- a/modules/lookup_ldap.c
> +++ b/modules/lookup_ldap.c
> @@ -28,7 +28,6 @@
> #include <arpa/nameser.h>
> #include <resolv.h>
> #include <lber.h>
> -#include <libxml/tree.h>
>
> #define MODULE_LOOKUP
> #include "automount.h"
> @@ -36,6 +35,10 @@
> #include "lookup_ldap.h"
> #include "base64.h"
>
> +#ifdef WITH_SASL
> +#include <libxml/tree.h>
> +#endif
> +
> #define MAPFMT_DEFAULT "sun"
>
> #define MODPREFIX "lookup(ldap): "
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] fix compile error with heimdal support enabled
2013-07-23 9:44 ` Ian Kent
@ 2013-07-23 10:19 ` Dennis Lan (dlan)
2013-07-23 10:50 ` Ian Kent
0 siblings, 1 reply; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-23 10:19 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs
On Tue, Jul 23, 2013 at 5:44 PM, Ian Kent <raven@themaw.net> wrote:
> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>>
>> this patch instroduce a compatible layer between Heimdal and MTT Krb5.
>> And I slightly rework the original patch to make it more readable.
>>
>> ---
>> Upstream Discussion:
>> http://thread.gmane.org/gmane.linux.kernel.autofs/4203
>>
>> Gentoo Bugs:
>> https://bugs.gentoo.org/show_bug.cgi?id=210762
>>
>> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
>> ---
>> aclocal.m4 | 7 +++++++
>> modules/cyrus-sasl.c | 35 ++++++++++++++++++++++++++++++-----
>> 2 files changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/aclocal.m4 b/aclocal.m4
>> index c5de159..7a8b03c 100644
>> --- a/aclocal.m4
>> +++ b/aclocal.m4
>> @@ -299,6 +299,13 @@ else
>> HAVE_KRB5=1
>> KRB5_LIBS=`$KRB5_CONFIG --libs`
>> KRB5_FLAGS=`$KRB5_CONFIG --cflags`
>> +
>> + SAVE_CFLAGS=$CFLAGS
>> + SAVE_LIBS=$LIBS
>> + CFLAGS="$CFLAGS $KRB5_FLAGS"
>> + LIBS="$LIBS $KRB5_LIBS"
>> +
>> + AC_CHECK_FUNCS([krb5_principal_get_realm])
>> fi])
>>
>> dnl --------------------------------------------------------------------------
>> diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
>> index 68f9242..6115f90 100644
>> --- a/modules/cyrus-sasl.c
>> +++ b/modules/cyrus-sasl.c
>> @@ -64,6 +64,31 @@
>> #endif
>> #endif
>>
>> +#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
>> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
>> + const char **realm, int *len)
>> +{
>> + *realm = krb5_principal_get_realm(context, princ);
>> + *len = strlen(*realm);
>
> So krb5_principal_get_realm() never fails, or does it return NULL, what
> about strlen(NULL) .... SEGV.
>
>> +}
>> +#else
>> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
>> + const char **realm, int *len)
>> +{
>> + const krb5_data *data;
>> +
>> + data = krb5_princ_realm(context, princ);
>> + if (data) {
>> + *realm = data->data;
>> + *len = data->length;
>> + } else {
>> + *realm = NULL;
>> + *len = 0;
>> + }
>> +}
>> +#endif
>> +
>> +
>> /*
>> * Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
>> * environment variable so that the library knows where to find it.
>> @@ -379,7 +404,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
>> krb5_principal tgs_princ, krb5_client_princ;
>> krb5_creds my_creds;
>> char *tgs_name;
>> - int status;
>> + const char *realm_name;
>> + int status, realm_length;
>>
>> if (ctxt->kinit_done)
>> return 0;
>> @@ -450,12 +476,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
>> }
>>
>> /* setup a principal for the ticket granting service */
>> + _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
>> ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
>> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
>> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
>> + realm_length, realm_name,
>> strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
>> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
>> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
>> + realm_length, realm_name,
>> 0);
>> if (ret) {
>> error(logopt,
>
>
HI Ian:
As I looking into the code (krb5_principal_get_realm) which provided
by heimdal, there is no grantee that *realm will always be valid
(does't do internal check, and does't return "" empty string if fail).
But, as my knowledge here, I guess we should already grantee that
*realm is valid before calling this function,
It won't hurt if we add add a check here, so how about this?
*len = (*realm == NULL) ? 0 : strlen(*realm);
Dennis
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-23 10:10 ` Ian Kent
@ 2013-07-23 10:30 ` Dennis Lan (dlan)
2013-07-23 10:39 ` Ian Kent
2013-07-24 2:13 ` Ian Kent
0 siblings, 2 replies; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-23 10:30 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs
On Tue, Jul 23, 2013 at 6:10 PM, Ian Kent <raven@themaw.net> wrote:
> On Tue, 2013-07-23 at 17:50 +0800, Ian Kent wrote:
>> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> > From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>> >
>> > autofs will create symbol link mandatory no matter ldap support
>> > is enabled or not. so, without this patch, lookup_ldaps.so will become
>> > a dead link.
>>
>> I have added this to my patch queue.
>> It will be committed next time I push patches to the repo.
>
> On second thoughts, what if WITH_LDAP is defined and WITH_SASL is not?
>
HI Ian:
current logic is: if SASL is not enabled, then the symbol
lookup_ldaps.so (which link to lookup_ldap.so) will not be created.
what are you suggesting here? should symbol of lookup_ldaps.so be
controlled by LDAP ?
Thanks
Dennis
>> Thanks
>>
>> >
>> > ---
>> > Upstream Discussion:
>> > http://thread.gmane.org/gmane.linux.kernel.autofs/5371
>> >
>> > Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
>> > ---
>> > modules/Makefile | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/modules/Makefile b/modules/Makefile
>> > index c5deb24..4bb1096 100644
>> > --- a/modules/Makefile
>> > +++ b/modules/Makefile
>> > @@ -74,7 +74,9 @@ install: all
>> > -rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
>> > ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
>> > ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
>> > +ifeq ($(SASL), 1)
>> > ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
>> > +endif
>> > ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
>> > ifeq ($(EXT2FS), 1)
>> > ifeq ($(EXT3FS), 1)
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe autofs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/6] add missing WITH_SASL in modules lookup_ldap.c
2013-07-22 14:59 ` [PATCH 5/6] add missing WITH_SASL " Lan Yixun (dlan)
@ 2013-07-23 10:35 ` Ian Kent
2013-07-23 10:47 ` Dennis Lan (dlan)
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:35 UTC (permalink / raw)
To: Lan Yixun (dlan); +Cc: autofs
On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>
> ---
> Gentoo Bugs:
> https://bugs.gentoo.org/show_bug.cgi?id=361899
Not sure about this one either.
I have a patch in the queue that's been sitting for some time that does
this and some more. I'd like to include it in 5.0.8 but I'm not sure it
actually works properly. I need to go over it somewhat before I commit
it.
Here it is fyi:
autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl
From: Dustin Polke <DuPol@gmx.de>
See https://bugs.gentoo.org/show_bug.cgi?id=361899 for more info.
Edited by: Ian Kent <raven@themaw.net>
- fix parse_ldap_config() is needed by ldap but previously excluded.
- exclude other references to ctxt->extern_cert and ctxt->extern_key.
- prevent memory leak if present in config but not used.
- remove now unused set_env().
---
CHANGELOG | 1 +
include/lookup_ldap.h | 4 ++--
lib/Makefile | 4 ++++
modules/Makefile | 6 ++++--
modules/lookup_ldap.c | 14 ++++++++++++--
5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index 3228d6b..69fd1e7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -57,6 +57,7 @@
- fix a couple of compiler warnings.
- add after sssd dependency to unit file.
- dont start readmap unless ready.
+- fix compilation of lookup_ldap.c without sasl.
25/07/2012 autofs-5.0.7
=======================
diff --git a/include/lookup_ldap.h b/include/lookup_ldap.h
index 9a4ce73..f34c029 100644
--- a/include/lookup_ldap.h
+++ b/include/lookup_ldap.h
@@ -11,6 +11,8 @@
#include <krb5.h>
#endif
+#include <libxml/tree.h>
+
#include "list.h"
#include "dclist.h"
@@ -92,7 +94,6 @@ struct lookup_context {
};
-#ifdef WITH_SASL
#define LDAP_AUTH_CONF_FILE "test"
#define LDAP_TLS_DONT_USE 0
@@ -104,7 +105,6 @@ struct lookup_context {
#define LDAP_AUTH_REQUIRED 0x0002
#define LDAP_AUTH_AUTODETECT 0x0004
#define LDAP_NEED_AUTH (LDAP_AUTH_REQUIRED|LDAP_AUTH_AUTODETECT)
-#endif
#define LDAP_AUTH_USESIMPLE 0x0008
diff --git a/lib/Makefile b/lib/Makefile
index 5418009..4a5b712 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -24,6 +24,10 @@ CFLAGS += -I../include -fPIC -D_GNU_SOURCE
CFLAGS += -DAUTOFS_MAP_DIR=\"$(autofsmapdir)\"
CFLAGS += -DAUTOFS_CONF_DIR=\"$(autofsconfdir)\"
+ifeq ($(LDAP), 1)
+ CFLAGS += $(XML_FLAGS) $(XML_LIBS)
+endif
+
.PHONY: all install clean
all: autofs.a
diff --git a/modules/Makefile b/modules/Makefile
index c5deb24..2589ae0 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -45,10 +45,12 @@ endif
ifeq ($(LDAP), 1)
SRCS += lookup_ldap.c
MODS += lookup_ldap.so
+ LDAP_FLAGS += $(XML_FLAGS) -DLDAP_THREAD_SAFE
+ LIBLDAP += $(XML_LIBS)
ifeq ($(SASL), 1)
SASL_OBJ = cyrus-sasl.o cyrus-sasl-extern.o
- LDAP_FLAGS += $(SASL_FLAGS) $(XML_FLAGS) $(KRB5_FLAGS)
-DLDAP_THREAD_SAFE
- LIBLDAP += $(LIBSASL) $(XML_LIBS) $(KRB5_LIBS)
+ LDAP_FLAGS += $(SASL_FLAGS) $(KRB5_FLAGS)
+ LIBLDAP += $(LIBSASL) $(KRB5_LIBS)
endif
endif
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index a2bfafd..904cc21 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -66,6 +66,7 @@ struct ldap_search_params {
static int decode_percent_hack(const char *, char **);
+#ifdef HAVE_SASL
static int set_env(unsigned logopt, const char *name, const char *val)
{
int ret = setenv(name, val, 1);
@@ -75,6 +76,7 @@ static int set_env(unsigned logopt, const char *name,
const char *val)
}
return 1;
}
+#endif
#ifndef HAVE_LDAP_CREATE_PAGE_CONTROL
int ldap_create_page_control(LDAP *ldap, ber_int_t pagesize,
@@ -585,10 +587,12 @@ static LDAP *do_connect(unsigned logopt, const
char *uri, struct lookup_context
{
LDAP *ldap;
+#ifdef HAVE_SASL
if (ctxt->extern_cert && ctxt->extern_key) {
set_env(logopt, ENV_LDAPTLS_CERT, ctxt->extern_cert);
set_env(logopt, ENV_LDAPTLS_KEY, ctxt->extern_key);
}
+#endif
ldap = init_ldap_connection(logopt, uri, ctxt);
if (ldap) {
@@ -791,7 +795,6 @@ find_server:
return ldap;
}
-#ifdef WITH_SASL
int get_property(unsigned logopt, xmlNodePtr node, const char *prop,
char **value)
{
xmlChar *ret;
@@ -812,6 +815,7 @@ int get_property(unsigned logopt, xmlNodePtr node,
const char *prop, char **valu
return 0;
}
+#ifdef WITH_SASL
/*
* For plain text, login and digest-md5 authentication types, we need
* user and password credentials.
@@ -824,6 +828,7 @@ int authtype_requires_creds(const char *authtype)
return 1;
return 0;
}
+#endif
/*
* Returns:
@@ -1056,6 +1061,7 @@ auth_fail:
}
} else if (auth_required == LDAP_AUTH_REQUIRED &&
(authtype && !strncmp(authtype, "EXTERNAL", 8))) {
+#ifdef WITH_SASL
ret = get_property(logopt, root, "external_cert", &extern_cert);
ret |= get_property(logopt, root, "external_key", &extern_key);
/*
@@ -1074,6 +1080,7 @@ auth_fail:
if (extern_key)
free(extern_key);
}
+#endif
}
/*
@@ -1094,8 +1101,10 @@ auth_fail:
ctxt->secret = secret;
ctxt->client_princ = client_princ;
ctxt->client_cc = client_cc;
+#ifdef WITH_SASL
ctxt->extern_cert = extern_cert;
ctxt->extern_key = extern_key;
+#endif
debug(logopt, MODPREFIX
"ldap authentication configured with the following options:");
@@ -1127,7 +1136,6 @@ out:
return ret;
}
-#endif
/*
* Take an input string as specified in the master map, and break it
@@ -1390,10 +1398,12 @@ static void free_context(struct lookup_context
*ctxt)
defaults_free_searchdns(ctxt->sdns);
if (ctxt->dclist)
free_dclist(ctxt->dclist);
+#ifdef HAVE_SASL
if (ctxt->extern_cert)
free(ctxt->extern_cert);
if (ctxt->extern_key)
free(ctxt->extern_key);
+#endif
free(ctxt);
return;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] fix typo, forced-shutdown should be force-shutdown
2013-07-22 14:59 ` [PATCH 6/6] fix typo, forced-shutdown should be force-shutdown Lan Yixun (dlan)
@ 2013-07-23 10:37 ` Ian Kent
0 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:37 UTC (permalink / raw)
To: Lan Yixun (dlan); +Cc: autofs
On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>
> it will confuse user when they found what shows with "./configure --help"
> differ from what it should be.
This one looks sensible, ;)
>
> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> ---
> configure.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.in b/configure.in
> index 559045a..0013a39 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -363,7 +363,7 @@ fi
> #
> # Enable forced shutdown on USR1 signal (unlink umounts all mounts).
> #
> -AC_ARG_ENABLE(forced-shutdown,
> +AC_ARG_ENABLE(force-shutdown,
> [ --enable-force-shutdown enable USR1 signal to force unlink umount of any
> busy mounts during shutdown],,
> enableval=no)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c
2013-07-23 10:17 ` Ian Kent
@ 2013-07-23 10:37 ` Dennis Lan (dlan)
2013-07-23 10:42 ` Ian Kent
0 siblings, 1 reply; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-23 10:37 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs
On Tue, Jul 23, 2013 at 6:17 PM, Ian Kent <raven@themaw.net> wrote:
> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>>
>> make "libxml/tree.h" controlled by WITH_SASL, also move it beind line of
>> #include "automount.h", since WITH_SASL is defined in config.h which is
>> included by automout.h
>
> Same again, WITH_LDAP defined and WITH_SASL not.
> I agree there appears to be a problem but not sure what to do about it.
>
>>
>> ---
>> Gentoo Bugs:
>> https://bugs.gentoo.org/show_bug.cgi?id=468606
>>
>> gcc -shared -O2 -pipe -march=core2 -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_REENTRANT
>> -D_FILE_OFFSET_BITS=
>> 64 -I../include -I../lib -fPIC -D_GNU_SOURCE -DAUTOFS_LIB_DIR=\"/usr/lib64/autofs\"
>> -DAUTOFS_MAP_DIR=\"/e
>> tc/autofs\" -DLDAP_DEPRECATED=1 -o lookup_ldap.so \
>> lookup_ldap.c dclist.o base64.o \
>> ../lib/autofs.a -lldap -llber -lresolv
>> : lookup_yp.so
>> lookup_ldap.c:31:25: fatal error: libxml/tree.h: No such file or directory
>> #include <libxml/tree.h>
>> ^
>> compilation terminated.
>>
>> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
>> ---
>> modules/lookup_ldap.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
>> index b8970a6..fad558d 100644
>> --- a/modules/lookup_ldap.c
>> +++ b/modules/lookup_ldap.c
>> @@ -28,7 +28,6 @@
>> #include <arpa/nameser.h>
>> #include <resolv.h>
>> #include <lber.h>
>> -#include <libxml/tree.h>
>>
>> #define MODULE_LOOKUP
>> #include "automount.h"
>> @@ -36,6 +35,10 @@
>> #include "lookup_ldap.h"
>> #include "base64.h"
>>
>> +#ifdef WITH_SASL
>> +#include <libxml/tree.h>
>> +#endif
>> +
>> #define MAPFMT_DEFAULT "sun"
>>
>> #define MODPREFIX "lookup(ldap): "
>
>
HI Ian:
I haven't dig too much into the code.
As far as I know (could be wrong).. LDAP SASL auth needs libxml?
quote from configure.in (line 170)
# LDAP SASL auth needs libxml and Kerberos
AF_CHECK_LIBXML()
AF_CHECK_KRB5()
so libxml could be controlled by SASL, but not LDAP, right?
(Btw, SASL require LDAP enabled)
Dennis
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-23 10:30 ` Dennis Lan (dlan)
@ 2013-07-23 10:39 ` Ian Kent
2013-07-24 2:13 ` Ian Kent
1 sibling, 0 replies; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:39 UTC (permalink / raw)
To: Dennis Lan (dlan); +Cc: autofs
On Tue, 2013-07-23 at 18:30 +0800, Dennis Lan (dlan) wrote:
> On Tue, Jul 23, 2013 at 6:10 PM, Ian Kent <raven@themaw.net> wrote:
> > On Tue, 2013-07-23 at 17:50 +0800, Ian Kent wrote:
> >> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> >> > From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
> >> >
> >> > autofs will create symbol link mandatory no matter ldap support
> >> > is enabled or not. so, without this patch, lookup_ldaps.so will become
> >> > a dead link.
> >>
> >> I have added this to my patch queue.
> >> It will be committed next time I push patches to the repo.
> >
> > On second thoughts, what if WITH_LDAP is defined and WITH_SASL is not?
> >
>
> HI Ian:
> current logic is: if SASL is not enabled, then the symbol
> lookup_ldaps.so (which link to lookup_ldap.so) will not be created.
> what are you suggesting here? should symbol of lookup_ldaps.so be
> controlled by LDAP ?
Ha, yes, I was thinking ldap not ldaps, I'll check again.
> Thanks
>
> Dennis
>
> >> Thanks
> >>
> >> >
> >> > ---
> >> > Upstream Discussion:
> >> > http://thread.gmane.org/gmane.linux.kernel.autofs/5371
> >> >
> >> > Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> >> > ---
> >> > modules/Makefile | 2 ++
> >> > 1 file changed, 2 insertions(+)
> >> >
> >> > diff --git a/modules/Makefile b/modules/Makefile
> >> > index c5deb24..4bb1096 100644
> >> > --- a/modules/Makefile
> >> > +++ b/modules/Makefile
> >> > @@ -74,7 +74,9 @@ install: all
> >> > -rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
> >> > ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
> >> > ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
> >> > +ifeq ($(SASL), 1)
> >> > ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
> >> > +endif
> >> > ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
> >> > ifeq ($(EXT2FS), 1)
> >> > ifeq ($(EXT3FS), 1)
> >>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe autofs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c
2013-07-23 10:37 ` Dennis Lan (dlan)
@ 2013-07-23 10:42 ` Ian Kent
2013-07-23 10:49 ` Dennis Lan (dlan)
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:42 UTC (permalink / raw)
To: Dennis Lan (dlan); +Cc: autofs
On Tue, 2013-07-23 at 18:37 +0800, Dennis Lan (dlan) wrote:
> On Tue, Jul 23, 2013 at 6:17 PM, Ian Kent <raven@themaw.net> wrote:
> > On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> >> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
> >>
> >> make "libxml/tree.h" controlled by WITH_SASL, also move it beind line of
> >> #include "automount.h", since WITH_SASL is defined in config.h which is
> >> included by automout.h
> >
> > Same again, WITH_LDAP defined and WITH_SASL not.
> > I agree there appears to be a problem but not sure what to do about it.
> >
> >>
> >> ---
> >> Gentoo Bugs:
> >> https://bugs.gentoo.org/show_bug.cgi?id=468606
> >>
> >> gcc -shared -O2 -pipe -march=core2 -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_REENTRANT
> >> -D_FILE_OFFSET_BITS=
> >> 64 -I../include -I../lib -fPIC -D_GNU_SOURCE -DAUTOFS_LIB_DIR=\"/usr/lib64/autofs\"
> >> -DAUTOFS_MAP_DIR=\"/e
> >> tc/autofs\" -DLDAP_DEPRECATED=1 -o lookup_ldap.so \
> >> lookup_ldap.c dclist.o base64.o \
> >> ../lib/autofs.a -lldap -llber -lresolv
> >> : lookup_yp.so
> >> lookup_ldap.c:31:25: fatal error: libxml/tree.h: No such file or directory
> >> #include <libxml/tree.h>
> >> ^
> >> compilation terminated.
> >>
> >> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> >> ---
> >> modules/lookup_ldap.c | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
> >> index b8970a6..fad558d 100644
> >> --- a/modules/lookup_ldap.c
> >> +++ b/modules/lookup_ldap.c
> >> @@ -28,7 +28,6 @@
> >> #include <arpa/nameser.h>
> >> #include <resolv.h>
> >> #include <lber.h>
> >> -#include <libxml/tree.h>
> >>
> >> #define MODULE_LOOKUP
> >> #include "automount.h"
> >> @@ -36,6 +35,10 @@
> >> #include "lookup_ldap.h"
> >> #include "base64.h"
> >>
> >> +#ifdef WITH_SASL
> >> +#include <libxml/tree.h>
> >> +#endif
> >> +
> >> #define MAPFMT_DEFAULT "sun"
> >>
> >> #define MODPREFIX "lookup(ldap): "
> >
> >
> HI Ian:
> I haven't dig too much into the code.
> As far as I know (could be wrong).. LDAP SASL auth needs libxml?
>
> quote from configure.in (line 170)
>
> # LDAP SASL auth needs libxml and Kerberos
> AF_CHECK_LIBXML()
> AF_CHECK_KRB5()
>
> so libxml could be controlled by SASL, but not LDAP, right?
> (Btw, SASL require LDAP enabled)
That's what occurred to me but, after a quick look, I'm thinking the xml
config still needs to be read. That's why I agree there's a problem that
needs fixing.
I'll have to look at it along with that old Gentoo ldap patch I want to
commit.
>
> Dennis
> --
> To unsubscribe from this list: send the line "unsubscribe autofs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/6] add missing WITH_SASL in modules lookup_ldap.c
2013-07-23 10:35 ` Ian Kent
@ 2013-07-23 10:47 ` Dennis Lan (dlan)
0 siblings, 0 replies; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-23 10:47 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs, Dustin Polke
On Tue, Jul 23, 2013 at 6:35 PM, Ian Kent <raven@themaw.net> wrote:
> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>>
>> ---
>> Gentoo Bugs:
>> https://bugs.gentoo.org/show_bug.cgi?id=361899
>
> Not sure about this one either.
> I have a patch in the queue that's been sitting for some time that does
> this and some more. I'd like to include it in 5.0.8 but I'm not sure it
> actually works properly. I need to go over it somewhat before I commit
> it.
>
> Here it is fyi:
>
> autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl
>
> From: Dustin Polke <DuPol@gmx.de>
>
> See https://bugs.gentoo.org/show_bug.cgi?id=361899 for more info.
>
> Edited by: Ian Kent <raven@themaw.net>
> - fix parse_ldap_config() is needed by ldap but previously excluded.
> - exclude other references to ctxt->extern_cert and ctxt->extern_key.
> - prevent memory leak if present in config but not used.
> - remove now unused set_env().
> ---
> CHANGELOG | 1 +
> include/lookup_ldap.h | 4 ++--
> lib/Makefile | 4 ++++
> modules/Makefile | 6 ++++--
> modules/lookup_ldap.c | 14 ++++++++++++--
> 5 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/CHANGELOG b/CHANGELOG
> index 3228d6b..69fd1e7 100644
> --- a/CHANGELOG
> +++ b/CHANGELOG
> @@ -57,6 +57,7 @@
> - fix a couple of compiler warnings.
> - add after sssd dependency to unit file.
> - dont start readmap unless ready.
> +- fix compilation of lookup_ldap.c without sasl.
>
> 25/07/2012 autofs-5.0.7
> =======================
> diff --git a/include/lookup_ldap.h b/include/lookup_ldap.h
> index 9a4ce73..f34c029 100644
> --- a/include/lookup_ldap.h
> +++ b/include/lookup_ldap.h
> @@ -11,6 +11,8 @@
> #include <krb5.h>
> #endif
>
> +#include <libxml/tree.h>
> +
> #include "list.h"
> #include "dclist.h"
>
> @@ -92,7 +94,6 @@ struct lookup_context {
> };
>
>
> -#ifdef WITH_SASL
> #define LDAP_AUTH_CONF_FILE "test"
>
> #define LDAP_TLS_DONT_USE 0
> @@ -104,7 +105,6 @@ struct lookup_context {
> #define LDAP_AUTH_REQUIRED 0x0002
> #define LDAP_AUTH_AUTODETECT 0x0004
> #define LDAP_NEED_AUTH (LDAP_AUTH_REQUIRED|LDAP_AUTH_AUTODETECT)
> -#endif
>
> #define LDAP_AUTH_USESIMPLE 0x0008
>
> diff --git a/lib/Makefile b/lib/Makefile
> index 5418009..4a5b712 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -24,6 +24,10 @@ CFLAGS += -I../include -fPIC -D_GNU_SOURCE
> CFLAGS += -DAUTOFS_MAP_DIR=\"$(autofsmapdir)\"
> CFLAGS += -DAUTOFS_CONF_DIR=\"$(autofsconfdir)\"
>
> +ifeq ($(LDAP), 1)
> + CFLAGS += $(XML_FLAGS) $(XML_LIBS)
> +endif
> +
> .PHONY: all install clean
>
> all: autofs.a
> diff --git a/modules/Makefile b/modules/Makefile
> index c5deb24..2589ae0 100644
> --- a/modules/Makefile
> +++ b/modules/Makefile
> @@ -45,10 +45,12 @@ endif
> ifeq ($(LDAP), 1)
> SRCS += lookup_ldap.c
> MODS += lookup_ldap.so
> + LDAP_FLAGS += $(XML_FLAGS) -DLDAP_THREAD_SAFE
> + LIBLDAP += $(XML_LIBS)
> ifeq ($(SASL), 1)
> SASL_OBJ = cyrus-sasl.o cyrus-sasl-extern.o
> - LDAP_FLAGS += $(SASL_FLAGS) $(XML_FLAGS) $(KRB5_FLAGS)
> -DLDAP_THREAD_SAFE
> - LIBLDAP += $(LIBSASL) $(XML_LIBS) $(KRB5_LIBS)
> + LDAP_FLAGS += $(SASL_FLAGS) $(KRB5_FLAGS)
> + LIBLDAP += $(LIBSASL) $(KRB5_LIBS)
> endif
> endif
>
> diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
> index a2bfafd..904cc21 100644
> --- a/modules/lookup_ldap.c
> +++ b/modules/lookup_ldap.c
> @@ -66,6 +66,7 @@ struct ldap_search_params {
>
> static int decode_percent_hack(const char *, char **);
>
> +#ifdef HAVE_SASL
> static int set_env(unsigned logopt, const char *name, const char *val)
> {
> int ret = setenv(name, val, 1);
> @@ -75,6 +76,7 @@ static int set_env(unsigned logopt, const char *name,
> const char *val)
> }
> return 1;
> }
> +#endif
>
> #ifndef HAVE_LDAP_CREATE_PAGE_CONTROL
> int ldap_create_page_control(LDAP *ldap, ber_int_t pagesize,
> @@ -585,10 +587,12 @@ static LDAP *do_connect(unsigned logopt, const
> char *uri, struct lookup_context
> {
> LDAP *ldap;
>
> +#ifdef HAVE_SASL
> if (ctxt->extern_cert && ctxt->extern_key) {
> set_env(logopt, ENV_LDAPTLS_CERT, ctxt->extern_cert);
> set_env(logopt, ENV_LDAPTLS_KEY, ctxt->extern_key);
> }
> +#endif
>
> ldap = init_ldap_connection(logopt, uri, ctxt);
> if (ldap) {
> @@ -791,7 +795,6 @@ find_server:
> return ldap;
> }
>
> -#ifdef WITH_SASL
> int get_property(unsigned logopt, xmlNodePtr node, const char *prop,
> char **value)
> {
> xmlChar *ret;
> @@ -812,6 +815,7 @@ int get_property(unsigned logopt, xmlNodePtr node,
> const char *prop, char **valu
> return 0;
> }
>
> +#ifdef WITH_SASL
> /*
> * For plain text, login and digest-md5 authentication types, we need
> * user and password credentials.
> @@ -824,6 +828,7 @@ int authtype_requires_creds(const char *authtype)
> return 1;
> return 0;
> }
> +#endif
>
> /*
> * Returns:
> @@ -1056,6 +1061,7 @@ auth_fail:
> }
> } else if (auth_required == LDAP_AUTH_REQUIRED &&
> (authtype && !strncmp(authtype, "EXTERNAL", 8))) {
> +#ifdef WITH_SASL
> ret = get_property(logopt, root, "external_cert", &extern_cert);
> ret |= get_property(logopt, root, "external_key", &extern_key);
> /*
> @@ -1074,6 +1080,7 @@ auth_fail:
> if (extern_key)
> free(extern_key);
> }
> +#endif
> }
>
> /*
> @@ -1094,8 +1101,10 @@ auth_fail:
> ctxt->secret = secret;
> ctxt->client_princ = client_princ;
> ctxt->client_cc = client_cc;
> +#ifdef WITH_SASL
> ctxt->extern_cert = extern_cert;
> ctxt->extern_key = extern_key;
> +#endif
>
> debug(logopt, MODPREFIX
> "ldap authentication configured with the following options:");
> @@ -1127,7 +1136,6 @@ out:
>
> return ret;
> }
> -#endif
>
> /*
> * Take an input string as specified in the master map, and break it
> @@ -1390,10 +1398,12 @@ static void free_context(struct lookup_context
> *ctxt)
> defaults_free_searchdns(ctxt->sdns);
> if (ctxt->dclist)
> free_dclist(ctxt->dclist);
> +#ifdef HAVE_SASL
> if (ctxt->extern_cert)
> free(ctxt->extern_cert);
> if (ctxt->extern_key)
> free(ctxt->extern_key);
> +#endif
> free(ctxt);
>
> return;
>
>
Hi Ian
Dustin is the previous autofs maintainer for gentoo linux, I also CC
him, But not sure whether he will response (he may not active).
Haven't looked into your patch in detail. Basically what I've done
here is refactoring his patches. Breaking into small pieces for better
review..
Dennis
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c
2013-07-23 10:42 ` Ian Kent
@ 2013-07-23 10:49 ` Dennis Lan (dlan)
0 siblings, 0 replies; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-23 10:49 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs
On Tue, Jul 23, 2013 at 6:42 PM, Ian Kent <raven@themaw.net> wrote:
> On Tue, 2013-07-23 at 18:37 +0800, Dennis Lan (dlan) wrote:
>> On Tue, Jul 23, 2013 at 6:17 PM, Ian Kent <raven@themaw.net> wrote:
>> > On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> >> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>> >>
>> >> make "libxml/tree.h" controlled by WITH_SASL, also move it beind line of
>> >> #include "automount.h", since WITH_SASL is defined in config.h which is
>> >> included by automout.h
>> >
>> > Same again, WITH_LDAP defined and WITH_SASL not.
>> > I agree there appears to be a problem but not sure what to do about it.
>> >
>> >>
>> >> ---
>> >> Gentoo Bugs:
>> >> https://bugs.gentoo.org/show_bug.cgi?id=468606
>> >>
>> >> gcc -shared -O2 -pipe -march=core2 -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_REENTRANT
>> >> -D_FILE_OFFSET_BITS=
>> >> 64 -I../include -I../lib -fPIC -D_GNU_SOURCE -DAUTOFS_LIB_DIR=\"/usr/lib64/autofs\"
>> >> -DAUTOFS_MAP_DIR=\"/e
>> >> tc/autofs\" -DLDAP_DEPRECATED=1 -o lookup_ldap.so \
>> >> lookup_ldap.c dclist.o base64.o \
>> >> ../lib/autofs.a -lldap -llber -lresolv
>> >> : lookup_yp.so
>> >> lookup_ldap.c:31:25: fatal error: libxml/tree.h: No such file or directory
>> >> #include <libxml/tree.h>
>> >> ^
>> >> compilation terminated.
>> >>
>> >> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
>> >> ---
>> >> modules/lookup_ldap.c | 5 ++++-
>> >> 1 file changed, 4 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
>> >> index b8970a6..fad558d 100644
>> >> --- a/modules/lookup_ldap.c
>> >> +++ b/modules/lookup_ldap.c
>> >> @@ -28,7 +28,6 @@
>> >> #include <arpa/nameser.h>
>> >> #include <resolv.h>
>> >> #include <lber.h>
>> >> -#include <libxml/tree.h>
>> >>
>> >> #define MODULE_LOOKUP
>> >> #include "automount.h"
>> >> @@ -36,6 +35,10 @@
>> >> #include "lookup_ldap.h"
>> >> #include "base64.h"
>> >>
>> >> +#ifdef WITH_SASL
>> >> +#include <libxml/tree.h>
>> >> +#endif
>> >> +
>> >> #define MAPFMT_DEFAULT "sun"
>> >>
>> >> #define MODPREFIX "lookup(ldap): "
>> >
>> >
>> HI Ian:
>> I haven't dig too much into the code.
>> As far as I know (could be wrong).. LDAP SASL auth needs libxml?
>>
>> quote from configure.in (line 170)
>>
>> # LDAP SASL auth needs libxml and Kerberos
>> AF_CHECK_LIBXML()
>> AF_CHECK_KRB5()
>>
>> so libxml could be controlled by SASL, but not LDAP, right?
>> (Btw, SASL require LDAP enabled)
>
> That's what occurred to me but, after a quick look, I'm thinking the xml
> config still needs to be read. That's why I agree there's a problem that
> needs fixing.
>
> I'll have to look at it along with that old Gentoo ldap patch I want to
> commit.
>
>>
Hi Ian:
Good, if you have any progress, let me know ;_)
Thanks
>> Dennis
>> --
>> To unsubscribe from this list: send the line "unsubscribe autofs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] fix compile error with heimdal support enabled
2013-07-23 10:19 ` Dennis Lan (dlan)
@ 2013-07-23 10:50 ` Ian Kent
2013-07-24 10:25 ` Dennis Lan (dlan)
0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-23 10:50 UTC (permalink / raw)
To: Dennis Lan (dlan); +Cc: autofs
On Tue, 2013-07-23 at 18:19 +0800, Dennis Lan (dlan) wrote:
> On Tue, Jul 23, 2013 at 5:44 PM, Ian Kent <raven@themaw.net> wrote:
> > On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> >> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
> >>
> >> this patch instroduce a compatible layer between Heimdal and MTT Krb5.
> >> And I slightly rework the original patch to make it more readable.
> >>
> >> ---
> >> Upstream Discussion:
> >> http://thread.gmane.org/gmane.linux.kernel.autofs/4203
> >>
> >> Gentoo Bugs:
> >> https://bugs.gentoo.org/show_bug.cgi?id=210762
> >>
> >> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> >> ---
> >> aclocal.m4 | 7 +++++++
> >> modules/cyrus-sasl.c | 35 ++++++++++++++++++++++++++++++-----
> >> 2 files changed, 37 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/aclocal.m4 b/aclocal.m4
> >> index c5de159..7a8b03c 100644
> >> --- a/aclocal.m4
> >> +++ b/aclocal.m4
> >> @@ -299,6 +299,13 @@ else
> >> HAVE_KRB5=1
> >> KRB5_LIBS=`$KRB5_CONFIG --libs`
> >> KRB5_FLAGS=`$KRB5_CONFIG --cflags`
> >> +
> >> + SAVE_CFLAGS=$CFLAGS
> >> + SAVE_LIBS=$LIBS
> >> + CFLAGS="$CFLAGS $KRB5_FLAGS"
> >> + LIBS="$LIBS $KRB5_LIBS"
> >> +
> >> + AC_CHECK_FUNCS([krb5_principal_get_realm])
> >> fi])
> >>
> >> dnl --------------------------------------------------------------------------
> >> diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
> >> index 68f9242..6115f90 100644
> >> --- a/modules/cyrus-sasl.c
> >> +++ b/modules/cyrus-sasl.c
> >> @@ -64,6 +64,31 @@
> >> #endif
> >> #endif
> >>
> >> +#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
> >> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
> >> + const char **realm, int *len)
> >> +{
> >> + *realm = krb5_principal_get_realm(context, princ);
> >> + *len = strlen(*realm);
> >
> > So krb5_principal_get_realm() never fails, or does it return NULL, what
> > about strlen(NULL) .... SEGV.
> >
> >> +}
> >> +#else
> >> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
> >> + const char **realm, int *len)
> >> +{
> >> + const krb5_data *data;
> >> +
> >> + data = krb5_princ_realm(context, princ);
> >> + if (data) {
> >> + *realm = data->data;
> >> + *len = data->length;
> >> + } else {
> >> + *realm = NULL;
> >> + *len = 0;
> >> + }
> >> +}
> >> +#endif
> >> +
> >> +
> >> /*
> >> * Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
> >> * environment variable so that the library knows where to find it.
> >> @@ -379,7 +404,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
> >> krb5_principal tgs_princ, krb5_client_princ;
> >> krb5_creds my_creds;
> >> char *tgs_name;
> >> - int status;
> >> + const char *realm_name;
> >> + int status, realm_length;
> >>
> >> if (ctxt->kinit_done)
> >> return 0;
> >> @@ -450,12 +476,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
> >> }
> >>
> >> /* setup a principal for the ticket granting service */
> >> + _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
> >> ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
> >> + realm_length, realm_name,
> >> strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
> >> + realm_length, realm_name,
> >> 0);
> >> if (ret) {
> >> error(logopt,
> >
> >
> HI Ian:
> As I looking into the code (krb5_principal_get_realm) which provided
> by heimdal, there is no grantee that *realm will always be valid
> (does't do internal check, and does't return "" empty string if fail).
> But, as my knowledge here, I guess we should already grantee that
> *realm is valid before calling this function,
> It won't hurt if we add add a check here, so how about this?
>
> *len = (*realm == NULL) ? 0 : strlen(*realm);
Yeah, so we should be able to be sure it will always never return "".
That's probably a fair assumption ... although maybe *realm should be
initialized to NULL on entry to avoid odd arch specific initialization
issues.
>
> Dennis
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-23 10:30 ` Dennis Lan (dlan)
2013-07-23 10:39 ` Ian Kent
@ 2013-07-24 2:13 ` Ian Kent
2013-07-24 2:24 ` Dennis Lan (dlan)
1 sibling, 1 reply; 25+ messages in thread
From: Ian Kent @ 2013-07-24 2:13 UTC (permalink / raw)
To: Dennis Lan (dlan); +Cc: autofs
On Tue, 2013-07-23 at 18:30 +0800, Dennis Lan (dlan) wrote:
> On Tue, Jul 23, 2013 at 6:10 PM, Ian Kent <raven@themaw.net> wrote:
> > On Tue, 2013-07-23 at 17:50 +0800, Ian Kent wrote:
> >> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> >> > From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
> >> >
> >> > autofs will create symbol link mandatory no matter ldap support
> >> > is enabled or not. so, without this patch, lookup_ldaps.so will become
> >> > a dead link.
> >>
> >> I have added this to my patch queue.
> >> It will be committed next time I push patches to the repo.
> >
> > On second thoughts, what if WITH_LDAP is defined and WITH_SASL is not?
> >
>
> HI Ian:
> current logic is: if SASL is not enabled, then the symbol
> lookup_ldaps.so (which link to lookup_ldap.so) will not be created.
> what are you suggesting here? should symbol of lookup_ldaps.so be
> controlled by LDAP ?
Yes, I think so.
If lookup_ldap.so has been included in the build then ldaps:// uris
should be able to be used regardless of SASL, as long as the ldap client
library is configured for it, of course.
This is what I believe it should be:
autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled
From: Lan Yixun (dlan) <dennis.yxun@gmail.com>
autofs will create symbol link mandatory no matter ldap support
is enabled or not. so, without this patch, lookup_ldaps.so will become
a dead link.
Edited by: Ian Kent <raven@themaw.net>
- change check from SASL to LDAP since the ldaps lookup module may
still be used ldaps:// as long as LDAP support is built.
---
CHANGELOG | 1 +
modules/Makefile | 2 ++
2 files changed, 3 insertions(+)
diff --git a/CHANGELOG b/CHANGELOG
index 3c829cf..9755e25 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -59,6 +59,7 @@
- dont start readmap unless ready.
- fix typo forced-shutdown should be force-shutdown.
- fix hesiod check error and use correct $(LIBS) setting.
+- fix dead LDAP symbolic link when LDAP support is disabled.
25/07/2012 autofs-5.0.7
=======================
diff --git a/modules/Makefile b/modules/Makefile
index c5deb24..8c0df18 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -74,7 +74,9 @@ install: all
-rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
+ifeq ($(LDAP), 1)
ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
+endif
ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
ifeq ($(EXT2FS), 1)
ifeq ($(EXT3FS), 1)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled
2013-07-24 2:13 ` Ian Kent
@ 2013-07-24 2:24 ` Dennis Lan (dlan)
0 siblings, 0 replies; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-24 2:24 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs
On Wed, Jul 24, 2013 at 10:13 AM, Ian Kent <raven@themaw.net> wrote:
> On Tue, 2013-07-23 at 18:30 +0800, Dennis Lan (dlan) wrote:
>> On Tue, Jul 23, 2013 at 6:10 PM, Ian Kent <raven@themaw.net> wrote:
>> > On Tue, 2013-07-23 at 17:50 +0800, Ian Kent wrote:
>> >> On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> >> > From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>> >> >
>> >> > autofs will create symbol link mandatory no matter ldap support
>> >> > is enabled or not. so, without this patch, lookup_ldaps.so will become
>> >> > a dead link.
>> >>
>> >> I have added this to my patch queue.
>> >> It will be committed next time I push patches to the repo.
>> >
>> > On second thoughts, what if WITH_LDAP is defined and WITH_SASL is not?
>> >
>>
>> HI Ian:
>> current logic is: if SASL is not enabled, then the symbol
>> lookup_ldaps.so (which link to lookup_ldap.so) will not be created.
>> what are you suggesting here? should symbol of lookup_ldaps.so be
>> controlled by LDAP ?
>
> Yes, I think so.
>
> If lookup_ldap.so has been included in the build then ldaps:// uris
> should be able to be used regardless of SASL, as long as the ldap client
> library is configured for it, of course.
>
> This is what I believe it should be:
>
> autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled
>
> From: Lan Yixun (dlan) <dennis.yxun@gmail.com>
>
> autofs will create symbol link mandatory no matter ldap support
> is enabled or not. so, without this patch, lookup_ldaps.so will become
> a dead link.
>
> Edited by: Ian Kent <raven@themaw.net>
> - change check from SASL to LDAP since the ldaps lookup module may
> still be used ldaps:// as long as LDAP support is built.
> ---
> CHANGELOG | 1 +
> modules/Makefile | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/CHANGELOG b/CHANGELOG
> index 3c829cf..9755e25 100644
> --- a/CHANGELOG
> +++ b/CHANGELOG
> @@ -59,6 +59,7 @@
> - dont start readmap unless ready.
> - fix typo forced-shutdown should be force-shutdown.
> - fix hesiod check error and use correct $(LIBS) setting.
> +- fix dead LDAP symbolic link when LDAP support is disabled.
>
> 25/07/2012 autofs-5.0.7
> =======================
> diff --git a/modules/Makefile b/modules/Makefile
> index c5deb24..8c0df18 100644
> --- a/modules/Makefile
> +++ b/modules/Makefile
> @@ -74,7 +74,9 @@ install: all
> -rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
> ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
> ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
> +ifeq ($(LDAP), 1)
> ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
> +endif
> ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
> ifeq ($(EXT2FS), 1)
> ifeq ($(EXT3FS), 1)
>
>
Hi Ian:
I'm fine with this, thanks!!
Dennis Lan (dlan)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] fix compile error with heimdal support enabled
2013-07-23 10:50 ` Ian Kent
@ 2013-07-24 10:25 ` Dennis Lan (dlan)
2013-07-24 10:32 ` Ian Kent
0 siblings, 1 reply; 25+ messages in thread
From: Dennis Lan (dlan) @ 2013-07-24 10:25 UTC (permalink / raw)
To: Ian Kent; +Cc: autofs mailing list
On Tue, Jul 23, 2013 at 6:50 PM, Ian Kent <raven@themaw.net> wrote:
> On Tue, 2013-07-23 at 18:19 +0800, Dennis Lan (dlan) wrote:
>> On Tue, Jul 23, 2013 at 5:44 PM, Ian Kent <raven@themaw.net> wrote:
>> > On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
>> >> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
>> >>
>> >> this patch instroduce a compatible layer between Heimdal and MTT Krb5.
>> >> And I slightly rework the original patch to make it more readable.
>> >>
>> >> ---
>> >> Upstream Discussion:
>> >> http://thread.gmane.org/gmane.linux.kernel.autofs/4203
>> >>
>> >> Gentoo Bugs:
>> >> https://bugs.gentoo.org/show_bug.cgi?id=210762
>> >>
>> >> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
>> >> ---
>> >> aclocal.m4 | 7 +++++++
>> >> modules/cyrus-sasl.c | 35 ++++++++++++++++++++++++++++++-----
>> >> 2 files changed, 37 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/aclocal.m4 b/aclocal.m4
>> >> index c5de159..7a8b03c 100644
>> >> --- a/aclocal.m4
>> >> +++ b/aclocal.m4
>> >> @@ -299,6 +299,13 @@ else
>> >> HAVE_KRB5=1
>> >> KRB5_LIBS=`$KRB5_CONFIG --libs`
>> >> KRB5_FLAGS=`$KRB5_CONFIG --cflags`
>> >> +
>> >> + SAVE_CFLAGS=$CFLAGS
>> >> + SAVE_LIBS=$LIBS
>> >> + CFLAGS="$CFLAGS $KRB5_FLAGS"
>> >> + LIBS="$LIBS $KRB5_LIBS"
>> >> +
>> >> + AC_CHECK_FUNCS([krb5_principal_get_realm])
>> >> fi])
>> >>
>> >> dnl --------------------------------------------------------------------------
>> >> diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
>> >> index 68f9242..6115f90 100644
>> >> --- a/modules/cyrus-sasl.c
>> >> +++ b/modules/cyrus-sasl.c
>> >> @@ -64,6 +64,31 @@
>> >> #endif
>> >> #endif
>> >>
>> >> +#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
>> >> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
>> >> + const char **realm, int *len)
>> >> +{
>> >> + *realm = krb5_principal_get_realm(context, princ);
>> >> + *len = strlen(*realm);
>> >
>> > So krb5_principal_get_realm() never fails, or does it return NULL, what
>> > about strlen(NULL) .... SEGV.
>> >
>> >> +}
>> >> +#else
>> >> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
>> >> + const char **realm, int *len)
>> >> +{
>> >> + const krb5_data *data;
>> >> +
>> >> + data = krb5_princ_realm(context, princ);
>> >> + if (data) {
>> >> + *realm = data->data;
>> >> + *len = data->length;
>> >> + } else {
>> >> + *realm = NULL;
>> >> + *len = 0;
>> >> + }
>> >> +}
>> >> +#endif
>> >> +
>> >> +
>> >> /*
>> >> * Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
>> >> * environment variable so that the library knows where to find it.
>> >> @@ -379,7 +404,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
>> >> krb5_principal tgs_princ, krb5_client_princ;
>> >> krb5_creds my_creds;
>> >> char *tgs_name;
>> >> - int status;
>> >> + const char *realm_name;
>> >> + int status, realm_length;
>> >>
>> >> if (ctxt->kinit_done)
>> >> return 0;
>> >> @@ -450,12 +476,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
>> >> }
>> >>
>> >> /* setup a principal for the ticket granting service */
>> >> + _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
>> >> ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
>> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
>> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
>> >> + realm_length, realm_name,
>> >> strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
>> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
>> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
>> >> + realm_length, realm_name,
>> >> 0);
>> >> if (ret) {
>> >> error(logopt,
>> >
>> >
>> HI Ian:
>> As I looking into the code (krb5_principal_get_realm) which provided
>> by heimdal, there is no grantee that *realm will always be valid
>> (does't do internal check, and does't return "" empty string if fail).
>> But, as my knowledge here, I guess we should already grantee that
>> *realm is valid before calling this function,
>> It won't hurt if we add add a check here, so how about this?
>>
>> *len = (*realm == NULL) ? 0 : strlen(*realm);
>
> Yeah, so we should be able to be sure it will always never return "".
>
> That's probably a fair assumption ... although maybe *realm should be
> initialized to NULL on entry to avoid odd arch specific initialization
> issues.
>
>>
>> Dennis
>
>
HI Ian:
How about this one? for the three types of patches ;-)
which should this one belong to?
Thanks
Dennis Lan (dlan)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] fix compile error with heimdal support enabled
2013-07-24 10:25 ` Dennis Lan (dlan)
@ 2013-07-24 10:32 ` Ian Kent
0 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2013-07-24 10:32 UTC (permalink / raw)
To: Dennis Lan (dlan); +Cc: autofs mailing list
On Wed, 2013-07-24 at 18:25 +0800, Dennis Lan (dlan) wrote:
> On Tue, Jul 23, 2013 at 6:50 PM, Ian Kent <raven@themaw.net> wrote:
> > On Tue, 2013-07-23 at 18:19 +0800, Dennis Lan (dlan) wrote:
> >> On Tue, Jul 23, 2013 at 5:44 PM, Ian Kent <raven@themaw.net> wrote:
> >> > On Mon, 2013-07-22 at 22:59 +0800, Lan Yixun (dlan) wrote:
> >> >> From: "Lan Yixun (dlan)" <dennis.yxun@gmail.com>
> >> >>
> >> >> this patch instroduce a compatible layer between Heimdal and MTT Krb5.
> >> >> And I slightly rework the original patch to make it more readable.
> >> >>
> >> >> ---
> >> >> Upstream Discussion:
> >> >> http://thread.gmane.org/gmane.linux.kernel.autofs/4203
> >> >>
> >> >> Gentoo Bugs:
> >> >> https://bugs.gentoo.org/show_bug.cgi?id=210762
> >> >>
> >> >> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> >> >> ---
> >> >> aclocal.m4 | 7 +++++++
> >> >> modules/cyrus-sasl.c | 35 ++++++++++++++++++++++++++++++-----
> >> >> 2 files changed, 37 insertions(+), 5 deletions(-)
> >> >>
> >> >> diff --git a/aclocal.m4 b/aclocal.m4
> >> >> index c5de159..7a8b03c 100644
> >> >> --- a/aclocal.m4
> >> >> +++ b/aclocal.m4
> >> >> @@ -299,6 +299,13 @@ else
> >> >> HAVE_KRB5=1
> >> >> KRB5_LIBS=`$KRB5_CONFIG --libs`
> >> >> KRB5_FLAGS=`$KRB5_CONFIG --cflags`
> >> >> +
> >> >> + SAVE_CFLAGS=$CFLAGS
> >> >> + SAVE_LIBS=$LIBS
> >> >> + CFLAGS="$CFLAGS $KRB5_FLAGS"
> >> >> + LIBS="$LIBS $KRB5_LIBS"
> >> >> +
> >> >> + AC_CHECK_FUNCS([krb5_principal_get_realm])
> >> >> fi])
> >> >>
> >> >> dnl --------------------------------------------------------------------------
> >> >> diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
> >> >> index 68f9242..6115f90 100644
> >> >> --- a/modules/cyrus-sasl.c
> >> >> +++ b/modules/cyrus-sasl.c
> >> >> @@ -64,6 +64,31 @@
> >> >> #endif
> >> >> #endif
> >> >>
> >> >> +#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
> >> >> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
> >> >> + const char **realm, int *len)
> >> >> +{
> >> >> + *realm = krb5_principal_get_realm(context, princ);
> >> >> + *len = strlen(*realm);
> >> >
> >> > So krb5_principal_get_realm() never fails, or does it return NULL, what
> >> > about strlen(NULL) .... SEGV.
> >> >
> >> >> +}
> >> >> +#else
> >> >> +void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
> >> >> + const char **realm, int *len)
> >> >> +{
> >> >> + const krb5_data *data;
> >> >> +
> >> >> + data = krb5_princ_realm(context, princ);
> >> >> + if (data) {
> >> >> + *realm = data->data;
> >> >> + *len = data->length;
> >> >> + } else {
> >> >> + *realm = NULL;
> >> >> + *len = 0;
> >> >> + }
> >> >> +}
> >> >> +#endif
> >> >> +
> >> >> +
> >> >> /*
> >> >> * Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
> >> >> * environment variable so that the library knows where to find it.
> >> >> @@ -379,7 +404,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
> >> >> krb5_principal tgs_princ, krb5_client_princ;
> >> >> krb5_creds my_creds;
> >> >> char *tgs_name;
> >> >> - int status;
> >> >> + const char *realm_name;
> >> >> + int status, realm_length;
> >> >>
> >> >> if (ctxt->kinit_done)
> >> >> return 0;
> >> >> @@ -450,12 +476,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
> >> >> }
> >> >>
> >> >> /* setup a principal for the ticket granting service */
> >> >> + _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
> >> >> ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
> >> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
> >> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
> >> >> + realm_length, realm_name,
> >> >> strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
> >> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
> >> >> - krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
> >> >> + realm_length, realm_name,
> >> >> 0);
> >> >> if (ret) {
> >> >> error(logopt,
> >> >
> >> >
> >> HI Ian:
> >> As I looking into the code (krb5_principal_get_realm) which provided
> >> by heimdal, there is no grantee that *realm will always be valid
> >> (does't do internal check, and does't return "" empty string if fail).
> >> But, as my knowledge here, I guess we should already grantee that
> >> *realm is valid before calling this function,
> >> It won't hurt if we add add a check here, so how about this?
> >>
> >> *len = (*realm == NULL) ? 0 : strlen(*realm);
> >
> > Yeah, so we should be able to be sure it will always never return "".
> >
> > That's probably a fair assumption ... although maybe *realm should be
> > initialized to NULL on entry to avoid odd arch specific initialization
> > issues.
> >
> >>
> >> Dennis
> >
> >
>
> HI Ian:
> How about this one? for the three types of patches ;-)
> which should this one belong to?
All the patches in the series we are discussing, except for patch 9
since it's not finished yet, I want to commit next time I do one. I want
to finish patch 9 as well so I can commit it.
The patches are mostly straight forward so I'd like to include them in
the 5.0.8 release, which I want to do some time soon. It's overdue ...
> Thanks
>
> Dennis Lan (dlan)
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2013-07-24 10:32 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-22 14:59 Seeking for upstreaming patches which floating in Gentoo Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 1/6] fix compile error with heimdal support enabled Lan Yixun (dlan)
2013-07-23 9:44 ` Ian Kent
2013-07-23 10:19 ` Dennis Lan (dlan)
2013-07-23 10:50 ` Ian Kent
2013-07-24 10:25 ` Dennis Lan (dlan)
2013-07-24 10:32 ` Ian Kent
2013-07-22 14:59 ` [PATCH 2/6] fix dead LDAP symbol link with LDAP support disabled Lan Yixun (dlan)
2013-07-23 9:50 ` Ian Kent
2013-07-23 10:10 ` Ian Kent
2013-07-23 10:30 ` Dennis Lan (dlan)
2013-07-23 10:39 ` Ian Kent
2013-07-24 2:13 ` Ian Kent
2013-07-24 2:24 ` Dennis Lan (dlan)
2013-07-22 14:59 ` [PATCH 3/6] fix compile error with LDAP enabled but SASL disabled Lan Yixun (dlan)
2013-07-22 14:59 ` [PATCH 4/6] fix no "libxml/tree.h" error in modules lookup_ldap.c Lan Yixun (dlan)
2013-07-23 10:17 ` Ian Kent
2013-07-23 10:37 ` Dennis Lan (dlan)
2013-07-23 10:42 ` Ian Kent
2013-07-23 10:49 ` Dennis Lan (dlan)
2013-07-22 14:59 ` [PATCH 5/6] add missing WITH_SASL " Lan Yixun (dlan)
2013-07-23 10:35 ` Ian Kent
2013-07-23 10:47 ` Dennis Lan (dlan)
2013-07-22 14:59 ` [PATCH 6/6] fix typo, forced-shutdown should be force-shutdown Lan Yixun (dlan)
2013-07-23 10:37 ` Ian Kent
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.