[PATCH 00/25] Current autofs patch queue

[PATCH 00/25] Current autofs patch queue

[Ian Kent]

There are patches from several contributors I plan on committing
so I thought posting them to the list for final review would be
sensible.

I'm not sure when I'll commit the changes but I hope in the next
couple of days.

---

Dustin Polke (1):
      autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

Ian Kent (13):
      autofs-5.0.7 - don't override LDFLAGS in make rules
      autofs-5.0.7 - fix a couple of compiler warnings
      autofs-5.0.7 - add after sssd dependency to unit file
      autofs-5.0.7 - dont start readmap unless ready
      autofs-5.0.7 - fix dumpmaps multi output
      autofs-5.0.7 - try and cleanup after dumpmaps
      autofs-5.0.7 - teach dumpmaps to output simple key value pairs
      autofs-5.0.7 - fix syncronize handle_mounts() shutdown
      autofs-5.0.7 - fix fix wildcard multi map regression
      autofs-5.0.7 - improve timeout option description
      autofs-5.0.7 - only probe specific nfs version when requested
      autofs-5.0.7 - fix bad mkdir permission on create
      autofs-5.0.7 - setup program map env from macro table

Lan Yixun (dlan) (10):
      autofs-5.0.7 - fix compile error with heimdal support enabled
      autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown
      autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting
      autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled
      autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled
      autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones
      autofs-5.0.7 - remove hard-coded STRIP variable
      autofs-5.0.7 - use LIBS for link libraries
      autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other
      autofs-5.0.7 - fix occasional build error when enable parallel compiling

Leonardo Chiquitto (1):
      autofs-5.0.7 - fix crash due to thread unsafe use of libldap


 CHANGELOG                      |   23 ++++
 Makefile.conf.in               |    3 
 Makefile.rules                 |   34 +++--
 aclocal.m4                     |   19 ++-
 autofs.spec                    |    3 
 configure                      |  104 +++++++++++++++--
 configure.in                   |    2 
 daemon/Makefile                |    2 
 daemon/automount.c             |   71 +++++++++--
 daemon/state.c                 |   15 +-
 include/config.h.in            |    3 
 include/lookup_ldap.h          |    4 -
 include/macros.h               |    1 
 include/master.h               |    1 
 lib/Makefile                   |    6 +
 lib/macros.c                   |   28 ++++
 lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
 man/auto.master.5.in           |    5 +
 man/automount.8                |   23 +++-
 modules/Makefile               |   20 ++-
 modules/cyrus-sasl.c           |   39 +++++-
 modules/lookup_file.c          |   25 +++-
 modules/lookup_ldap.c          |   76 +++++++++++-
 modules/lookup_nisplus.c       |   26 +++-
 modules/lookup_program.c       |   20 +++
 modules/lookup_sss.c           |   22 +++-
 modules/lookup_yp.c            |   23 +++-
 modules/mount_bind.c           |    2 
 modules/mount_nfs.c            |   14 ++
 redhat/autofs.sysconfig.in     |    5 +
 samples/autofs.conf.default.in |    5 +
 samples/autofs.service.in      |    2 
 32 files changed, 753 insertions(+), 123 deletions(-)

-- 
Ian

[PATCH 01/25] autofs-5.0.7 - don't override LDFLAGS in make rules

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

Ensure that externally defined LDFLAGS is not overridden and ensure
binary is linked as RELRO in tar spec file.
---
 CHANGELOG      |    1 +
 Makefile.rules |    6 +++---
 autofs.spec    |    3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 5d90139..37eac72 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -53,6 +53,7 @@
 - fix probe each nfs version in turn for singleton mounts.
 - misc man page fixes.
 - fix add null check in parse_server_string().
+- don't override LDFLAGS in make rules.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/Makefile.rules b/Makefile.rules
index f2ba386..6b5b2bd 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -24,16 +24,16 @@ endif
 
 ifdef DEBUG
 CFLAGS    ?= -g -Wall -DDEBUG
-LDFLAGS   = -g
+LDFLAGS   ?= -g
 STRIP     = :
 else
 ifdef DONTSTRIP
 CFLAGS    ?= -O2 -g
-LDFLAGS   = -g
+LDFLAGS   ?= -g
 STRIP     = :
 else
 CFLAGS    ?= -O2 -Wall
-LDFLAGS   = -s
+LDFLAGS   ?= -s
 STRIP     = strip --strip-debug
 endif
 endif
diff --git a/autofs.spec b/autofs.spec
index a768e44..9a2bf6e 100644
--- a/autofs.spec
+++ b/autofs.spec
@@ -85,12 +85,13 @@ echo %{version}-%{release} > .version
 
 %build
 CFLAGS="$RPM_OPT_FLAGS -Wall" \
+LDFLAGS="-Wl,-z,now" \
 ./configure --libdir=%{_libdir} \
 	--disable-mount-locking \
 	--enable-ignore-busy \
 	%{?systemd_configure_arg:} \
 	%{?libtirpc_configure_arg:}
-CFLAGS="$RPM_OPT_FLAGS -Wall" make initdir=/etc/rc.d/init.d DONTSTRIP=1
+CFLAGS="$RPM_OPT_FLAGS -Wall" LDFLAGS="-Wl,-z,now" make initdir=/etc/rc.d/init.d DONTSTRIP=1
 
 %install
 rm -rf $RPM_BUILD_ROOT

[PATCH 02/25] autofs-5.0.7 - fix a couple of compiler warnings

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

Quiet a couple of ompiler warnings by making intention explicit using brackets.
---
 CHANGELOG            |    1 +
 modules/mount_bind.c |    2 +-
 modules/mount_nfs.c  |    4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 37eac72..7749f01 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -54,6 +54,7 @@
 - misc man page fixes.
 - fix add null check in parse_server_string().
 - don't override LDFLAGS in make rules.
+- fix a couple of compiler warnings.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/modules/mount_bind.c b/modules/mount_bind.c
index 61a773c..2b70104 100644
--- a/modules/mount_bind.c
+++ b/modules/mount_bind.c
@@ -208,7 +208,7 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 			      MODPREFIX
 			      "failed to create local mount %s -> %s",
 			      fullpath, what);
-			if (ap->flags & MOUNT_FLAG_GHOST && !status)
+			if ((ap->flags & MOUNT_FLAG_GHOST) && !status)
 				if (mkdir_path(fullpath, 0555) && errno != EEXIST) {
 					char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
 					error(ap->logopt,
diff --git a/modules/mount_nfs.c b/modules/mount_nfs.c
index 3d2ccea..adf2002 100644
--- a/modules/mount_nfs.c
+++ b/modules/mount_nfs.c
@@ -202,8 +202,8 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 	 */
 	if ((hosts && !hosts->next) &&
 	    mount_default_proto == 4 &&
-	    vers & NFS_VERS_MASK != 0 &&
-	    vers & NFS4_VERS_MASK != 0) {
+	    (vers & NFS_VERS_MASK) != 0 &&
+	    (vers & NFS4_VERS_MASK) != 0) {
 		unsigned int v4_probe_ok = 0;
 		struct host *tmp = new_host(hosts->name,
 					    hosts->addr, hosts->addr_len,

[PATCH 03/25] autofs-5.0.7 - add after sssd dependency to unit file

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

When using sss as a map source autofs can sometimes start before
sssd is ready.
---
 CHANGELOG                 |    1 +
 samples/autofs.service.in |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 7749f01..2734fe3 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -55,6 +55,7 @@
 - fix add null check in parse_server_string().
 - don't override LDFLAGS in make rules.
 - fix a couple of compiler warnings.
+- add after sssd dependency to unit file.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/samples/autofs.service.in b/samples/autofs.service.in
index 056ab5e..777463d 100644
--- a/samples/autofs.service.in
+++ b/samples/autofs.service.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=Automounts filesystems on demand
-After=network.target ypbind.service
+After=network.target ypbind.service sssd.service
 
 [Service]
 Type=forking

[PATCH 04/25] autofs-5.0.7 - dont start readmap unless ready

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

State transitions should only be made when an autofs point goes
to state ST_READY or when a new task is added to an empty task
queue.
---
 CHANGELOG      |    1 +
 daemon/state.c |   15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 2734fe3..3228d6b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -56,6 +56,7 @@
 - don't override LDFLAGS in make rules.
 - fix a couple of compiler warnings.
 - add after sssd dependency to unit file.
+- dont start readmap unless ready.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/daemon/state.c b/daemon/state.c
index ddc5556..8d81788 100644
--- a/daemon/state.c
+++ b/daemon/state.c
@@ -818,14 +818,13 @@ done:
 		new = st_alloc_task(ap, state);
 		if (new)
 			list_add(&new->list, head);
+		/* Added to empty state queue, kick state machine */
+		signaled = 1;
+		status = pthread_cond_signal(&cond);
+		if (status)
+			fatal(status);
 	}
 
-	/* Added task, encourage state machine */
-	signaled = 1;
-	status = pthread_cond_signal(&cond);
-	if (status)
-		fatal(status);
-
 	return 1;
 }
 
@@ -1131,6 +1130,10 @@ static void *st_queue_handler(void *arg)
 				task = list_entry(p, struct state_queue, list);
 				p = p->next;
 
+				/* Task may have been canceled before it started */
+				if (!task->thid && task->cancel)
+					goto remove;
+
 				if (!task->busy) {
 					/* Start a new task */
 					task->busy = 1;

[PATCH 05/25] autofs-5.0.7 - fix crash due to thread unsafe use of libldap

[Ian Kent]

From: Leonardo Chiquitto <leonardo.lists@gmail.com>

Add locking around LDAP initialization calls

To prevent corruption inside SSL and LDAP libraries, it's necessary to
serialize all calls to functions that initialize LDAP contexts.

How to reproduce the problem:

- Setup an LDAP server with SSL/TLS support enabled
- Configure AutoFS to fetch the maps from LDAP
- Make sure the OpenLDAP client library is configured to use SSL
  connections and "usetls" is set to yes in autofs_ldap_auth.conf.

In one directory handled by AutoFS (an indirect mount point), trigger in
parallel some dozens of invalid mounts (ie, try to access keys that do not
exist in the AutoFS map). Repeat until it crashes.

Here it always crashes in less than 20 minutes, normally inside OpenSSL.
Core dump inspection shows that internal SSL structures are corrupted,
with function pointers pointing to random addresses.

Trying to find similar reports on the web, I found this email from an
OpenLDAP developer (partial quote, emphasis mine) [1]:

"As far as I know, libldap is thread safe in the sense that multiple
threads can use separate LDAP* handles without running into concurrency
issues; *except for library initialization*, all accesses to common data
(i.e. global variables) is read-only."

[1]http://www.openldap.org/cgi-bin/wilma_hiliter/openldap-software/200606/msg00252.html

AutoFS implements no locking around LDAP initialization libraries and
it's quite common to see multiple threads executing ldap_initialize()
or ldap_start_tls_s() at the same time.
---
 CHANGELOG             |    1 +
 modules/lookup_ldap.c |   35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 3228d6b..fe232f4 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 crash due to thread unsafe use of libldap.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index a2bfafd..655e9fa 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -52,6 +52,12 @@ static struct ldap_schema common_schema[] = {
 };
 static unsigned int common_schema_count = sizeof(common_schema)/sizeof(struct ldap_schema);
 
+/*
+ * Initialization of LDAP and OpenSSL must be always serialized to
+ * avoid corruption of context structures inside these libraries.
+ */
+pthread_mutex_t ldapinit_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 struct ldap_search_params {
 	struct autofs_point *ap;
 	LDAP *ldap;
@@ -136,6 +142,22 @@ int ldap_parse_page_control(LDAP *ldap, LDAPControl **controls,
 }
 #endif /* HAVE_LDAP_PARSE_PAGE_CONTROL */
 
+static void ldapinit_mutex_lock(void)
+{
+	int status = pthread_mutex_lock(&ldapinit_mutex);
+	if (status)
+		fatal(status);
+	return;
+}
+
+static void ldapinit_mutex_unlock(void)
+{
+	int status = pthread_mutex_unlock(&ldapinit_mutex);
+	if (status)
+		fatal(status);
+	return;
+}
+
 static void uris_mutex_lock(struct lookup_context *ctxt)
 {
 	int status = pthread_mutex_lock(&ctxt->uris_mutex);
@@ -196,7 +218,7 @@ int unbind_ldap_connection(unsigned logopt, LDAP *ldap, struct lookup_context *c
 	return rv;
 }
 
-LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_context *ctxt)
+LDAP *__init_ldap_connection(unsigned logopt, const char *uri, struct lookup_context *ctxt)
 {
 	LDAP *ldap = NULL;
 	struct timeval timeout     = { ctxt->timeout, 0 };
@@ -277,6 +299,17 @@ LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_conte
 	return ldap;
 }
 
+LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_context *ctxt)
+{
+	LDAP *ldap;
+
+	ldapinit_mutex_lock();
+	ldap = __init_ldap_connection(logopt, uri, ctxt);
+	ldapinit_mutex_unlock();
+
+	return ldap;
+}
+
 static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt, const char *class, const char *key)
 {
 	char buf[MAX_ERR_BUF];

[PATCH 06/25] autofs-5.0.7 - fix compile error with heimdal support enabled

[Ian Kent]

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

Edited by: Ian Kent <raven@themaw.net>
- fix code indentation.
- update configure to include change.

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG            |    1 +
 aclocal.m4           |    7 ++++
 configure            |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/config.h.in  |    3 ++
 modules/cyrus-sasl.c |   39 ++++++++++++++++++++---
 5 files changed, 129 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index fe232f4..c77be18 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -58,6 +58,7 @@
 - add after sssd dependency to unit file.
 - dont start readmap unless ready.
 - fix crash due to thread unsafe use of libldap.
+- fix compile error with heimdal support enabled.
 
 25/07/2012 autofs-5.0.7
 =======================
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/configure b/configure
index 08b7ccc..1086eba 100755
--- a/configure
+++ b/configure
@@ -1559,6 +1559,73 @@ fi
 
 } # ac_fn_c_try_link
 
+# ac_fn_c_check_func LINENO FUNC VAR
+# ----------------------------------
+# Tests whether FUNC exists, setting the cache variable VAR accordingly
+ac_fn_c_check_func ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
+$as_echo_n "checking for $2... " >&6; }
+if eval \${$3+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $2 innocuous_$2
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $2 (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $2
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char $2 ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined __stub_$2 || defined __stub___$2
+choke me
+#endif
+
+int
+main ()
+{
+return $2 ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  eval "$3=yes"
+else
+  eval "$3=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+eval ac_res=\$$3
+	       { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+
+} # ac_fn_c_check_func
+
 # ac_fn_c_try_cpp LINENO
 # ----------------------
 # Try to preprocess conftest.$ac_ext, and return whether this succeeded.
@@ -4039,6 +4106,23 @@ $as_echo "yes" >&6; }
   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"
+
+  for ac_func in krb5_principal_get_realm
+do :
+  ac_fn_c_check_func "$LINENO" "krb5_principal_get_realm" "ac_cv_func_krb5_principal_get_realm"
+if test "x$ac_cv_func_krb5_principal_get_realm" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_KRB5_PRINCIPAL_GET_REALM 1
+_ACEOF
+
+fi
+done
+
 fi
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing versionsort" >&5
diff --git a/include/config.h.in b/include/config.h.in
index a2a05a8..8a52080 100644
--- a/include/config.h.in
+++ b/include/config.h.in
@@ -24,6 +24,9 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if you have the `krb5_principal_get_realm' function. */
+#undef HAVE_KRB5_PRINCIPAL_GET_REALM
+
 /* Define to 1 if you have the `ldap_create_page_control' function. */
 #undef HAVE_LDAP_CREATE_PAGE_CONTROL
 
diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
index 68f9242..b9c16f3 100644
--- a/modules/cyrus-sasl.c
+++ b/modules/cyrus-sasl.c
@@ -64,6 +64,35 @@
 #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);
+	if (*realm)
+		*len = strlen(*realm);
+	else
+		*len = 0;
+	return;
+#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;
+	}
+	return;
+}
+#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 +408,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 +480,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,

Re: [PATCH 06/25] autofs-5.0.7 - fix compile error with heimdal support enabled

[Dennis Lan (dlan)]

On Mon, Aug 19, 2013 at 9:12 AM, Ian Kent <raven@themaw.net> 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
>
> Edited by: Ian Kent <raven@themaw.net>
> - fix code indentation.
> - update configure to include change.
>
> Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> ---
>  CHANGELOG            |    1 +
>  aclocal.m4           |    7 ++++
>  configure            |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/config.h.in  |    3 ++
>  modules/cyrus-sasl.c |   39 ++++++++++++++++++++---
>  5 files changed, 129 insertions(+), 5 deletions(-)
>
> diff --git a/CHANGELOG b/CHANGELOG
> index fe232f4..c77be18 100644
> --- a/CHANGELOG
> +++ b/CHANGELOG
> @@ -58,6 +58,7 @@
>  - add after sssd dependency to unit file.
>  - dont start readmap unless ready.
>  - fix crash due to thread unsafe use of libldap.
> +- fix compile error with heimdal support enabled.
>
>  25/07/2012 autofs-5.0.7
>  =======================
> 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/configure b/configure
> index 08b7ccc..1086eba 100755
> --- a/configure
> +++ b/configure
> @@ -1559,6 +1559,73 @@ fi
>
>  } # ac_fn_c_try_link
>
> +# ac_fn_c_check_func LINENO FUNC VAR
> +# ----------------------------------
> +# Tests whether FUNC exists, setting the cache variable VAR accordingly
> +ac_fn_c_check_func ()
> +{
> +  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
> +  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
> +$as_echo_n "checking for $2... " >&6; }
> +if eval \${$3+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
> +   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
> +#define $2 innocuous_$2
> +
> +/* System header to define __stub macros and hopefully few prototypes,
> +    which can conflict with char $2 (); below.
> +    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
> +    <limits.h> exists even on freestanding compilers.  */
> +
> +#ifdef __STDC__
> +# include <limits.h>
> +#else
> +# include <assert.h>
> +#endif
> +
> +#undef $2
> +
> +/* Override any GCC internal prototype to avoid an error.
> +   Use char because int might match the return type of a GCC
> +   builtin and then its argument prototype would still apply.  */
> +#ifdef __cplusplus
> +extern "C"
> +#endif
> +char $2 ();
> +/* The GNU C library defines this for functions which it implements
> +    to always fail with ENOSYS.  Some functions are actually named
> +    something starting with __ and the normal name is an alias.  */
> +#if defined __stub_$2 || defined __stub___$2
> +choke me
> +#endif
> +
> +int
> +main ()
> +{
> +return $2 ();
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_link "$LINENO"; then :
> +  eval "$3=yes"
> +else
> +  eval "$3=no"
> +fi
> +rm -f core conftest.err conftest.$ac_objext \
> +    conftest$ac_exeext conftest.$ac_ext
> +fi
> +eval ac_res=\$$3
> +              { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
> +$as_echo "$ac_res" >&6; }
> +  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
> +
> +} # ac_fn_c_check_func
> +
>  # ac_fn_c_try_cpp LINENO
>  # ----------------------
>  # Try to preprocess conftest.$ac_ext, and return whether this succeeded.
> @@ -4039,6 +4106,23 @@ $as_echo "yes" >&6; }
>    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"
> +
> +  for ac_func in krb5_principal_get_realm
> +do :
> +  ac_fn_c_check_func "$LINENO" "krb5_principal_get_realm" "ac_cv_func_krb5_principal_get_realm"
> +if test "x$ac_cv_func_krb5_principal_get_realm" = xyes; then :
> +  cat >>confdefs.h <<_ACEOF
> +#define HAVE_KRB5_PRINCIPAL_GET_REALM 1
> +_ACEOF
> +
> +fi
> +done
> +
>  fi
>
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing versionsort" >&5
> diff --git a/include/config.h.in b/include/config.h.in
> index a2a05a8..8a52080 100644
> --- a/include/config.h.in
> +++ b/include/config.h.in
> @@ -24,6 +24,9 @@
>  /* Define to 1 if you have the <inttypes.h> header file. */
>  #undef HAVE_INTTYPES_H
>
> +/* Define to 1 if you have the `krb5_principal_get_realm' function. */
> +#undef HAVE_KRB5_PRINCIPAL_GET_REALM
> +
>  /* Define to 1 if you have the `ldap_create_page_control' function. */
>  #undef HAVE_LDAP_CREATE_PAGE_CONTROL
>
> diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
> index 68f9242..b9c16f3 100644
> --- a/modules/cyrus-sasl.c
> +++ b/modules/cyrus-sasl.c
> @@ -64,6 +64,35 @@
>  #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);
> +       if (*realm)
> +               *len = strlen(*realm);
> +       else
> +               *len = 0;
> +       return;

missed  '}' here

> +#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;
> +       }
> +       return;
> +}
> +#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 +408,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 +480,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,
>

Re: [PATCH 06/25] autofs-5.0.7 - fix compile error with heimdal support enabled

[Ian Kent]

On Tue, 2013-08-20 at 11:36 +0800, Dennis Lan (dlan) wrote:
> On Mon, Aug 19, 2013 at 9:12 AM, Ian Kent <raven@themaw.net> 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
> >
> > Edited by: Ian Kent <raven@themaw.net>
> > - fix code indentation.
> > - update configure to include change.
> >
> > Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
> > ---
> >  CHANGELOG            |    1 +
> >  aclocal.m4           |    7 ++++
> >  configure            |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/config.h.in  |    3 ++
> >  modules/cyrus-sasl.c |   39 ++++++++++++++++++++---
> >  5 files changed, 129 insertions(+), 5 deletions(-)
> >
> > diff --git a/CHANGELOG b/CHANGELOG
> > index fe232f4..c77be18 100644
> > --- a/CHANGELOG
> > +++ b/CHANGELOG
> > @@ -58,6 +58,7 @@
> >  - add after sssd dependency to unit file.
> >  - dont start readmap unless ready.
> >  - fix crash due to thread unsafe use of libldap.
> > +- fix compile error with heimdal support enabled.
> >
> >  25/07/2012 autofs-5.0.7
> >  =======================
> > 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/configure b/configure
> > index 08b7ccc..1086eba 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1559,6 +1559,73 @@ fi
> >
> >  } # ac_fn_c_try_link
> >
> > +# ac_fn_c_check_func LINENO FUNC VAR
> > +# ----------------------------------
> > +# Tests whether FUNC exists, setting the cache variable VAR accordingly
> > +ac_fn_c_check_func ()
> > +{
> > +  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
> > +  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
> > +$as_echo_n "checking for $2... " >&6; }
> > +if eval \${$3+:} false; then :
> > +  $as_echo_n "(cached) " >&6
> > +else
> > +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > +/* end confdefs.h.  */
> > +/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
> > +   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
> > +#define $2 innocuous_$2
> > +
> > +/* System header to define __stub macros and hopefully few prototypes,
> > +    which can conflict with char $2 (); below.
> > +    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
> > +    <limits.h> exists even on freestanding compilers.  */
> > +
> > +#ifdef __STDC__
> > +# include <limits.h>
> > +#else
> > +# include <assert.h>
> > +#endif
> > +
> > +#undef $2
> > +
> > +/* Override any GCC internal prototype to avoid an error.
> > +   Use char because int might match the return type of a GCC
> > +   builtin and then its argument prototype would still apply.  */
> > +#ifdef __cplusplus
> > +extern "C"
> > +#endif
> > +char $2 ();
> > +/* The GNU C library defines this for functions which it implements
> > +    to always fail with ENOSYS.  Some functions are actually named
> > +    something starting with __ and the normal name is an alias.  */
> > +#if defined __stub_$2 || defined __stub___$2
> > +choke me
> > +#endif
> > +
> > +int
> > +main ()
> > +{
> > +return $2 ();
> > +  ;
> > +  return 0;
> > +}
> > +_ACEOF
> > +if ac_fn_c_try_link "$LINENO"; then :
> > +  eval "$3=yes"
> > +else
> > +  eval "$3=no"
> > +fi
> > +rm -f core conftest.err conftest.$ac_objext \
> > +    conftest$ac_exeext conftest.$ac_ext
> > +fi
> > +eval ac_res=\$$3
> > +              { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
> > +$as_echo "$ac_res" >&6; }
> > +  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
> > +
> > +} # ac_fn_c_check_func
> > +
> >  # ac_fn_c_try_cpp LINENO
> >  # ----------------------
> >  # Try to preprocess conftest.$ac_ext, and return whether this succeeded.
> > @@ -4039,6 +4106,23 @@ $as_echo "yes" >&6; }
> >    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"
> > +
> > +  for ac_func in krb5_principal_get_realm
> > +do :
> > +  ac_fn_c_check_func "$LINENO" "krb5_principal_get_realm" "ac_cv_func_krb5_principal_get_realm"
> > +if test "x$ac_cv_func_krb5_principal_get_realm" = xyes; then :
> > +  cat >>confdefs.h <<_ACEOF
> > +#define HAVE_KRB5_PRINCIPAL_GET_REALM 1
> > +_ACEOF
> > +
> > +fi
> > +done
> > +
> >  fi
> >
> >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing versionsort" >&5
> > diff --git a/include/config.h.in b/include/config.h.in
> > index a2a05a8..8a52080 100644
> > --- a/include/config.h.in
> > +++ b/include/config.h.in
> > @@ -24,6 +24,9 @@
> >  /* Define to 1 if you have the <inttypes.h> header file. */
> >  #undef HAVE_INTTYPES_H
> >
> > +/* Define to 1 if you have the `krb5_principal_get_realm' function. */
> > +#undef HAVE_KRB5_PRINCIPAL_GET_REALM
> > +
> >  /* Define to 1 if you have the `ldap_create_page_control' function. */
> >  #undef HAVE_LDAP_CREATE_PAGE_CONTROL
> >
> > diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c
> > index 68f9242..b9c16f3 100644
> > --- a/modules/cyrus-sasl.c
> > +++ b/modules/cyrus-sasl.c
> > @@ -64,6 +64,35 @@
> >  #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);
> > +       if (*realm)
> > +               *len = strlen(*realm);
> > +       else
> > +               *len = 0;
> > +       return;
> 
> missed  '}' here

Fixed and pushed to temporary pending branch, patch also updated in
master branch.

Ian

[PATCH 07/25] autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

This will confuse people when they find what shows with "./configure --help"
is different from what is actually used.

Edited by: Ian Kent <raven@themaw.net>
- make description a little more readable.
- update configure to include change.

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG    |    1 +
 configure    |    8 ++++----
 configure.in |    2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index c77be18..5265f6c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -59,6 +59,7 @@
 - dont start readmap unless ready.
 - fix crash due to thread unsafe use of libldap.
 - fix compile error with heimdal support enabled.
+- fix typo forced-shutdown should be force-shutdown.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/configure b/configure
index 1086eba..b8e74b7 100755
--- a/configure
+++ b/configure
@@ -741,7 +741,7 @@ with_openldap
 with_sasl
 enable_ext_env
 enable_mount_locking
-enable_forced_shutdown
+enable_force_shutdown
 enable_ignore_busy
 '
       ac_precious_vars='build_alias
@@ -5637,9 +5637,9 @@ fi
 #
 # Enable forced shutdown on USR1 signal (unlink umounts all mounts).
 #
-# Check whether --enable-forced-shutdown was given.
-if test "${enable_forced_shutdown+set}" = set; then :
-  enableval=$enable_forced_shutdown;
+# Check whether --enable-force-shutdown was given.
+if test "${enable_force_shutdown+set}" = set; then :
+  enableval=$enable_force_shutdown;
 else
   enableval=no
 fi
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)

[PATCH 08/25] autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

the problem here is that hesiod checking always fail due to wrong $(LDFLAGS),
no matter hesiod library is installed or not. moving "-lhesiod" into
$(LIBS) solve this problem.

1) convert "char *" -> "void *" to fix the compile warning
2) put "-lhesiod -lresolv" into $(LIBS), not into $(LDFLAGS), otherwise we got link error

Edited by: Ian Kent <raven@themaw.net>
- update configure to include change.
---
 CHANGELOG  |    1 +
 aclocal.m4 |   12 ++++++------
 configure  |   12 ++++++------
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 5265f6c..647504b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -60,6 +60,7 @@
 - fix crash due to thread unsafe use of libldap.
 - fix compile error with heimdal support enabled.
 - fix typo forced-shutdown should be force-shutdown.
+- fix hesiod check error and use correct $(LIBS) setting.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/aclocal.m4 b/aclocal.m4
index 7a8b03c..637a775 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -316,20 +316,20 @@ dnl --------------------------------------------------------------------------
 AC_DEFUN([AF_CHECK_LIBHESIOD],
 [AC_MSG_CHECKING(for libhesiod)
 
-# save current ldflags
-af_check_hesiod_save_ldflags="$LDFLAGS"
-LDFLAGS="$LDFLAGS -lhesiod -lresolv"
+# save current libs
+af_check_hesiod_save_libs="$LIBS"
+LIBS="$LIBS -lhesiod -lresolv"
 
 AC_TRY_LINK(
   [ #include <hesiod.h> ],
-  [ char *c; hesiod_init(&c); ],
+  [ void *c; hesiod_init(&c); ],
   [ HAVE_HESIOD=1
     LIBHESIOD="$LIBHESIOD -lhesiod -lresolv"
     AC_MSG_RESULT(yes) ],
   [ AC_MSG_RESULT(no) ])
 
-# restore ldflags
-LDFLAGS="$af_check_hesiod_save_ldflags"
+# restore libs
+LIBS="$af_check_hesiod_save_libs"
 ])
 
 dnl --------------------------------------------------------------------------
diff --git a/configure b/configure
index b8e74b7..e6d5d4a 100755
--- a/configure
+++ b/configure
@@ -4305,9 +4305,9 @@ then
 	{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libhesiod" >&5
 $as_echo_n "checking for libhesiod... " >&6; }
 
-# save current ldflags
-af_check_hesiod_save_ldflags="$LDFLAGS"
-LDFLAGS="$LDFLAGS -lhesiod -lresolv"
+# save current libs
+af_check_hesiod_save_libs="$LIBS"
+LIBS="$LIBS -lhesiod -lresolv"
 
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
@@ -4315,7 +4315,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 int
 main ()
 {
- char *c; hesiod_init(&c);
+ void *c; hesiod_init(&c);
   ;
   return 0;
 }
@@ -4332,8 +4332,8 @@ fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext conftest.$ac_ext
 
-# restore ldflags
-LDFLAGS="$af_check_hesiod_save_ldflags"
+# restore libs
+LIBS="$af_check_hesiod_save_libs"
 
 	if test "$HAVE_HESIOD" == "1"; then
 

[PATCH 09/25] autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled

[Ian Kent]

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 by 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 647504b..296aaf8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -61,6 +61,7 @@
 - fix compile error with heimdal support enabled.
 - 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)

[PATCH 10/25] autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

this is need when libtirpc is enabled, otherwise autofs fail to start
due to it can't find symbol clnt_dg_create (which is provided by libtirpc)

autofs error log from /var/log/message:

Jul 23 12:11:58 ofire automount[25699]: open_mount:244: parse(sun): cannot open mount module
nfs (/usr/lib64/autofs/mount_nfs.so: undefined symbol: clnt_dg_create)
Jul 23 12:11:58 ofire automount[25699]: lookup(file): failed to open parse context

without this patch, mount_nfs.so fail link to libtirpc.so
 # ldd /usr/lib64/autofs/mount_nfs.so |grep tirpc

with this patch applied:
 # ldd /usr/lib64/autofs/mount_nfs.so |grep tirpc
        libtirpc.so.1 => /lib64/libtirpc.so.1 (0x00007f7d94920000)

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG        |    1 +
 modules/Makefile |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 296aaf8..679a469 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -62,6 +62,7 @@
 - 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.
+- add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/modules/Makefile b/modules/Makefile
index 8c0df18..e61e338 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -116,6 +116,6 @@ lookup_ldap.so: lookup_ldap.c dclist.o base64.o $(SASL_OBJ)
 
 mount_nfs.so: mount_nfs.c replicated.o
 	$(CC) $(SOLDFLAGS) $(CFLAGS) -o mount_nfs.so \
-		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBNSL)
+		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBNSL) $(TIRPCLIB)
 	$(STRIP) mount_nfs.so
 

[PATCH 11/25] autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

the logic here is:
 if user pass compiler variable, then use it, otherwise fallback to default value

this will provide most flexibility, so user can set this variable
it would be even useful for cross-compile environment.

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG        |    1 +
 Makefile.conf.in |    3 +++
 Makefile.rules   |    8 ++++----
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 679a469..8f349a2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -63,6 +63,7 @@
 - fix hesiod check error and use correct $(LIBS) setting.
 - fix dead LDAP symbolic link when LDAP support is disabled.
 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
+- use compiler determined by configure instead of hard-coded ones.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/Makefile.conf.in b/Makefile.conf.in
index 3766d45..cb53e66 100644
--- a/Makefile.conf.in
+++ b/Makefile.conf.in
@@ -109,3 +109,6 @@ initdir = @initdir@
 
 # Location of systemd unit files
 systemddir = @systemddir@
+
+# Use the compiler determined by configure instead of hard-coded ones
+CC := @CC@
diff --git a/Makefile.rules b/Makefile.rules
index 6b5b2bd..9aebd19 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -38,10 +38,10 @@ STRIP     = strip --strip-debug
 endif
 endif
 
-CC        = gcc
-CXX       = g++
-CXXFLAGS  = $(CFLAGS)
-LD        = ld
+CC        ?= gcc
+CXX       ?= g++
+CXXFLAGS  ?= $(CFLAGS)
+LD        ?= ld
 SOLDFLAGS = -shared
 
 CFLAGS += -D_REENTRANT -D_FILE_OFFSET_BITS=64

[PATCH 12/25] autofs-5.0.7 - remove hard-coded STRIP variable

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

according to autoconf manual[1], build script should allow user override the CFLAGS, LDFLAGS
varible (passed via ./configure). And actually, we should not directly use CFLAGS,
LDFLAGS, but have to create another one (eg. AUTOFS_CFLAGS). FYI, see section "Variable: CFLAGS",
the last paragraph.

Here, I slightly modify the code so that we can still fallback to the default one.

[1] http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Preset-Output-Variables.html#Preset-Output-Variables

Edited by: Ian Kent <raven@themaw.net>
- allow for previous LDFLAGS patch.
- rename patch to reflect what's actually done but leave the original
  description in place as a reminder.

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG      |    1 +
 Makefile.rules |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 8f349a2..ec9d7f0 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -64,6 +64,7 @@
 - fix dead LDAP symbolic link when LDAP support is disabled.
 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
 - use compiler determined by configure instead of hard-coded ones.
+- remove hard-coded STRIP variable.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/Makefile.rules b/Makefile.rules
index 9aebd19..d00ba3c 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -25,16 +25,16 @@ endif
 ifdef DEBUG
 CFLAGS    ?= -g -Wall -DDEBUG
 LDFLAGS   ?= -g
-STRIP     = :
+STRIP     ?= :
 else
 ifdef DONTSTRIP
 CFLAGS    ?= -O2 -g
 LDFLAGS   ?= -g
-STRIP     = :
+STRIP     ?= :
 else
 CFLAGS    ?= -O2 -Wall
 LDFLAGS   ?= -s
-STRIP     = strip --strip-debug
+STRIP     ?= strip --strip-debug
 endif
 endif
 

[PATCH 13/25] autofs-5.0.7 - use LIBS for link libraries

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

quote:
 "Don't use this variable to pass library names (-l) to the linker; use LIBS instead."

see the section "Variable: LDFLAGS" in:
http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/PreseOutput-Variables.html#Preset-Output-Variables

Edited by: Ian Kent <raven@themaw.net>
The variable usage changes for LDFLAGS and LIBS appear to be dependent
on each other so merge them to try and avoid breakage if not all are
used by someone. I also had changed the patch titles slightly when I
imported them but they should be easily recognisable.
- merge patches
  - pthread-link-library-should-be-put-into-LIBS-not-LDFLAGS
  - library-should-be-passed-via-variable-LIBS-not-LDFLAGS
  - accumulate-LIBS-variable-changes-otherwise-we-may-lose-early-settings
  - allow-user-to-pass-LDFLAGS-via-configure
- add LIBS to build rules, might fix DMALLOC build problem


Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG        |    1 +
 Makefile.rules   |   10 ++++++----
 daemon/Makefile  |    2 +-
 modules/Makefile |   12 ++++++------
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index ec9d7f0..3af7ece 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -65,6 +65,7 @@
 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
 - use compiler determined by configure instead of hard-coded ones.
 - remove hard-coded STRIP variable.
+- use LIBS for link libraries.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/Makefile.rules b/Makefile.rules
index d00ba3c..fa53459 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -45,17 +45,19 @@ LD        ?= ld
 SOLDFLAGS = -shared
 
 CFLAGS += -D_REENTRANT -D_FILE_OFFSET_BITS=64
-LDFLAGS += -lpthread
+LIBS += -lpthread
 
 ifdef TIRPCLIB
 CFLAGS += -I/usr/include/tirpc
-LDFLAGS += $(TIRPCLIB)
+LIBS += $(TIRPCLIB)
 endif
 
 ifdef DMALLOCLIB
-LDFLAGS += $(DMALLOCLIB)
+LIBS += $(DMALLOCLIB)
 endif
 
+LIBS += $(LIBNSL)
+
 # Standard rules
 
 .SUFFIXES: .c .o .s .so
@@ -67,5 +69,5 @@ endif
 	$(CC) $(CFLAGS) -S $<
 
 .c.so:
-	$(CC) $(SOLDFLAGS) $(CFLAGS) -o $*.so $< $(AUTOFS_LIB) $(DMALLOCLIB) $(LIBNSL)
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) -o $*.so $< $(AUTOFS_LIB) $(LIBS)
 	$(STRIP) $*.so
diff --git a/daemon/Makefile b/daemon/Makefile
index 9e9d635..6604c9a 100644
--- a/daemon/Makefile
+++ b/daemon/Makefile
@@ -20,7 +20,7 @@ CFLAGS += -DAUTOFS_FIFO_DIR=\"$(autofsfifodir)\"
 CFLAGS += -DAUTOFS_FLAG_DIR=\"$(autofsflagdir)\"
 CFLAGS += -DVERSION_STRING=\"$(version)\"
 LDFLAGS += -rdynamic
-LIBS = -ldl
+LIBS += -ldl
 
 ifeq ($(LDAP), 1)
     CFLAGS += $(XML_FLAGS)
diff --git a/modules/Makefile b/modules/Makefile
index e61e338..de01ebd 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -98,8 +98,8 @@ endif
 # Ad hoc compilation rules for modules which need auxilliary libraries
 #
 lookup_hesiod.so: lookup_hesiod.c
-	$(CC) $(SOLDFLAGS) $(CFLAGS) $(HESIOD_FLAGS) -o lookup_hesiod.so \
-		lookup_hesiod.c $(AUTOFS_LIB) $(LIBHESIOD) $(LIBRESOLV)
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) $(HESIOD_FLAGS) -o lookup_hesiod.so \
+		lookup_hesiod.c $(AUTOFS_LIB) $(LIBHESIOD) $(LIBRESOLV) $(LIBS)
 	$(STRIP) lookup_hesiod.so
 
 cyrus-sasl.o: cyrus-sasl.c
@@ -109,13 +109,13 @@ cyrus-sasl-extern.o: cyrus-sasl-extern.c
 	$(CC) $(CFLAGS) $(LDAP_FLAGS) -c $<
 
 lookup_ldap.so: lookup_ldap.c dclist.o base64.o $(SASL_OBJ)
-	$(CC) $(SOLDFLAGS) $(CFLAGS) $(LDAP_FLAGS) -o lookup_ldap.so \
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) $(LDAP_FLAGS) -o lookup_ldap.so \
 		lookup_ldap.c dclist.o base64.o $(SASL_OBJ) \
-		$(AUTOFS_LIB) $(LIBLDAP) $(LIBRESOLV)
+		$(AUTOFS_LIB) $(LIBLDAP) $(LIBRESOLV) $(LIBS)
 	$(STRIP) lookup_ldap.so
 
 mount_nfs.so: mount_nfs.c replicated.o
-	$(CC) $(SOLDFLAGS) $(CFLAGS) -o mount_nfs.so \
-		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBNSL) $(TIRPCLIB)
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) -o mount_nfs.so \
+		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBS)
 	$(STRIP) mount_nfs.so
 

[PATCH 14/25] autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

Gentoo build system expects unstripped elf image from the build system,
seperate this, will allow us to better control the strip operation.

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 CHANGELOG      |    1 +
 Makefile.rules |   10 ++++------
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 3af7ece..dd6fc1a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -66,6 +66,7 @@
 - use compiler determined by configure instead of hard-coded ones.
 - remove hard-coded STRIP variable.
 - use LIBS for link libraries.
+- unbundle NOTSTRIP from DEBUG so they dont depend on each other.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/Makefile.rules b/Makefile.rules
index fa53459..0754a00 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -25,18 +25,16 @@ endif
 ifdef DEBUG
 CFLAGS    ?= -g -Wall -DDEBUG
 LDFLAGS   ?= -g
-STRIP     ?= :
 else
+CFLAGS    ?= -O2 -Wall
+LDFLAGS   ?= -s
+endif
+
 ifdef DONTSTRIP
-CFLAGS    ?= -O2 -g
-LDFLAGS   ?= -g
 STRIP     ?= :
 else
-CFLAGS    ?= -O2 -Wall
-LDFLAGS   ?= -s
 STRIP     ?= strip --strip-debug
 endif
-endif
 
 CC        ?= gcc
 CXX       ?= g++

[PATCH 15/25] autofs-5.0.7 - fix occasional build error when enable parallel compiling

[Ian Kent]

From: Lan Yixun (dlan) <dennis.yxun@gmail.com>

The problem here we pass "-jX" ( X> 1) to "make", and "rpc_subs.c"
occasionally fail to compile due to it can't find "mount.h",
the root cause it that "mount.h" haven't generated by that time,
add this patch should fix this problem.
to make rpc_subs.o explicitly depend on "mount.h".

error message:

x86_64-pc-linux-gnu-gcc -O2 -pipe -march=core2  -D_REENTRANT
-D_FILE_OFFSET_BITS=64 -I/usr/include/tirp  c -D_REENTRANT
-D_FILE_OFFSET_BITS=64 -I/usr/include/tirpc -I../include -fPIC
-D_GNU_SOURCE -DAUTOFS_MA  P_DIR=\"/etc/autofs\"
-DAUTOFS_CONF_DIR=\"/etc/conf.d\" -c rpc_subs.c
rpc_subs.c:46:19: fatal error: mount.h: No such file or directory
  #include "mount.h"
                   ^
compilation terminated.
make[1]: *** [rpc_subs.o] Error 1
make[1]: *** Waiting for unfinished
jobs....

Signed-off-by: Lan Yixun (dlan) <dennis.yxun@gmail.com>
---
 lib/Makefile |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/Makefile b/lib/Makefile
index 5418009..314779b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -70,6 +70,8 @@ nss_tok.o: nss_tok.c nss_parse.tab.h
 
 nss_parse.tab.o: nss_parse.tab.c nss_parse.tab.h
 
+rpc_subs.o: mount.h
+
 install: all
 
 clean:

[PATCH 16/25] autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

[Ian Kent]

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 |   18 +++++++++++++-----
 5 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index dd6fc1a..5764cb0 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -67,6 +67,7 @@
 - remove hard-coded STRIP variable.
 - use LIBS for link libraries.
 - unbundle NOTSTRIP from DEBUG so they dont depend on each other.
+- 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 314779b..518b483 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 de01ebd..8610783 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 655e9fa..d05098f 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -72,6 +72,7 @@ struct ldap_search_params {
 
 static int decode_percent_hack(const char *, char **);
 
+#ifdef WITH_SASL
 static int set_env(unsigned logopt, const char *name, const char *val)
 {
 	int ret = setenv(name, val, 1);
@@ -81,6 +82,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,
@@ -205,9 +207,9 @@ int unbind_ldap_connection(unsigned logopt, LDAP *ldap, struct lookup_context *c
 {
 	int rv;
 
-#ifdef WITH_SASL
 	if (ctxt->use_tls == LDAP_TLS_RELEASE)
 		ctxt->use_tls = LDAP_TLS_INIT;
+#ifdef WITH_SASL
 	autofs_sasl_unbind(ctxt);
 #endif
 
@@ -265,7 +267,6 @@ LDAP *__init_ldap_connection(unsigned logopt, const char *uri, struct lookup_con
 		info(logopt, MODPREFIX "failed to set connection timeout to %d",
 		     net_timeout.tv_sec);
 
-#ifdef WITH_SASL
 	if (ctxt->use_tls) {
 		if (ctxt->version == 2) {
 			if (ctxt->tls_required) {
@@ -294,7 +295,6 @@ LDAP *__init_ldap_connection(unsigned logopt, const char *uri, struct lookup_con
 		}
 		ctxt->use_tls = LDAP_TLS_RELEASE;
 	}
-#endif
 
 	return ldap;
 }
@@ -618,10 +618,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) {
@@ -824,7 +826,6 @@ find_server:
 	return ldap;
 }
 
-#ifdef WITH_SASL
 int get_property(unsigned logopt, xmlNodePtr node, const char *prop, char **value)
 {
 	xmlChar *ret;
@@ -845,6 +846,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.
@@ -857,6 +859,7 @@ int authtype_requires_creds(const char *authtype)
 		return 1;
 	return 0;
 }
+#endif
 
 /*
  *  Returns:
@@ -1089,6 +1092,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);
 		/*
@@ -1107,6 +1111,7 @@ auth_fail:
 			if (extern_key)
 				free(extern_key);
 		}
+#endif
 	}
 
 	/*
@@ -1127,8 +1132,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:");
@@ -1160,7 +1167,6 @@ out:
 
 	return ret;
 }
-#endif
 
 /*
  *  Take an input string as specified in the master map, and break it
@@ -1423,10 +1429,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;

[PATCH 17/25] autofs-5.0.7 - fix dumpmaps multi output

[Ian Kent]

If the maps contain the old style "multi" type the dumped map information
isn't right.
---
 CHANGELOG    |    1 +
 lib/master.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 5764cb0..5e58aca 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -68,6 +68,7 @@
 - use LIBS for link libraries.
 - unbundle NOTSTRIP from DEBUG so they dont depend on each other.
 - fix compilation of lookup_ldap.c without sasl.
+- fix dumpmaps multi output.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/lib/master.c b/lib/master.c
index eca3523..df6c141 100644
--- a/lib/master.c
+++ b/lib/master.c
@@ -1281,6 +1281,54 @@ static void list_source_instances(struct map_source *source, struct map_source *
 	return;
 }
 
+static void print_map_info(struct map_source *source)
+{
+	int argc = source->argc;
+	int i, multi, map_num;
+
+	multi = (source->type && !strcmp(source->type, "multi"));
+	map_num = 1;
+	for (i = 0; i < argc; i++) {
+		if (source->argv[i] && *source->argv[i] != '-') {
+			if (!multi)
+				printf("  map: %s\n", source->argv[i]);
+			else
+				printf("  map[%i]: %s\n", map_num, source->argv[i]);
+			i++;
+		}
+
+		if (i >= argc)
+			return;
+
+		if (!strcmp(source->argv[i], "--"))
+			continue;
+
+		if (source->argv[i]) {
+			int need_newline = 0;
+			int j;
+
+			if (!multi)
+				printf("  arguments:");
+			else
+				printf("  arguments[%i]:", map_num);
+
+			for (j = i; j < source->argc; j++) {
+				if (!strcmp(source->argv[j], "--"))
+					break;
+				printf(" %s", source->argv[j]);
+				i++;
+				need_newline = 1;
+			}
+			if (need_newline)
+				printf("\n");
+		}
+		if (multi)
+			map_num++;
+	}
+
+	return;
+}
+
 int master_show_mounts(struct master *master)
 {
 	struct list_head *p, *head;
@@ -1311,7 +1359,6 @@ int master_show_mounts(struct master *master)
 		struct autofs_point *ap;
 		time_t now = time(NULL);
 		unsigned int count = 0;
-		int i;
 
 		this = list_entry(p, struct master_mapent, list);
 		p = p->next;
@@ -1357,20 +1404,10 @@ int master_show_mounts(struct master *master)
 			}
 
 			if (source->argc >= 1) {
-				i = 0;
-				if (source->argv[0] && *source->argv[0] != '-') {
-					printf("  map: %s\n", source->argv[0]);
-					i = 1;
-				}
+				print_map_info(source);
 				if (count && ap->type == LKP_INDIRECT)
 					printf("  duplicate indirect map entry"
 					       " will be ignored at run time\n");
-				if (source->argc > 1) {
-					printf("  arguments: ");
-					for (; i < source->argc; i++)
-						printf("%s ", source->argv[i]);
-					printf("\n");
-				}
 			}
 
 			printf("\n");

[PATCH 18/25] autofs-5.0.7 - try and cleanup after dumpmaps

[Ian Kent]

Try and cleanup a little after dumping maps.
It's not really necessary but but can help by reducing the noise
from valgrind when checking code.
---
 CHANGELOG          |    1 +
 daemon/automount.c |   13 +++++++++++++
 lib/master.c       |    2 ++
 3 files changed, 16 insertions(+)

diff --git a/CHANGELOG b/CHANGELOG
index 5e58aca..1583c76 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -69,6 +69,7 @@
 - unbundle NOTSTRIP from DEBUG so they dont depend on each other.
 - fix compilation of lookup_ldap.c without sasl.
 - fix dumpmaps multi output.
+- try and cleanup after dumpmaps.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/daemon/automount.c b/daemon/automount.c
index 1d0b64e..6849272 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -2136,6 +2136,8 @@ int main(int argc, char *argv[])
 	}
 
 	if (dumpmaps) {
+		struct master_mapent *entry;
+		struct list_head *head, *p;
 		struct mapent_cache *nc;
 
 		open_log();
@@ -2152,6 +2154,17 @@ int main(int argc, char *argv[])
 
 		lookup_nss_read_master(master_list, 0);
 		master_show_mounts(master_list);
+
+		head = &master_list->mounts;
+		p = head->next;
+		while (p != head) {
+			entry = list_entry(p, struct master_mapent, list);
+			p = p->next;
+			master_free_mapent_sources(entry, 1);
+			master_free_mapent(entry);
+		}
+		master_kill(master_list);
+
 		exit(0);
 	}
 
diff --git a/lib/master.c b/lib/master.c
index df6c141..734485e 100644
--- a/lib/master.c
+++ b/lib/master.c
@@ -1426,6 +1426,8 @@ int master_show_mounts(struct master *master)
 			source = source->next;
 		}
 
+		lookup_close_lookup(ap);
+
 		printf("\n");
 	}
 

[PATCH 19/25] autofs-5.0.7 - teach dumpmaps to output simple key value pairs

[Ian Kent]

The dumpmaps option doesn't allow maps to be output in <key, value>
pairs suitable for use as a file map.

This could be useful to save current maps as a backup for emergency
use.

If the dumpmaps option is given and is followed by two parameters,
"<map type> <map name>" then simple <key, value> pairs that would
be read in by a map read are printed to stdout if the given map type
and map name are found in the map configuration.

If the map is an LDAP map and there is more than one map of same name
in different base dns only the first map encountered by autofs will
be listed.

If the map type is an old style multi-map and any one of the map
names in the multi-map entry matches the given map name the entries
that would be used by autofs for the whole multi-map will be listed.
---
 CHANGELOG          |    1 
 daemon/automount.c |   51 +++++++++++---
 include/master.h   |    1 
 lib/master.c       |  187 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/automount.8    |   18 ++++-
 5 files changed, 243 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 1583c76..07f9d6d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -70,6 +70,7 @@
 - fix compilation of lookup_ldap.c without sasl.
 - fix dumpmaps multi output.
 - try and cleanup after dumpmaps.
+- teach dumpmaps to output simple key value pairs.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/daemon/automount.c b/daemon/automount.c
index 6849272..2ce49bf 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -1725,7 +1725,8 @@ static void usage(void)
 		"	-f --foreground do not fork into background\n"
 		"	-r --random-multimount-selection\n"
 		"			use ramdom replicated server selection\n"
-		"	-m --dumpmaps	dump automounter maps and exit\n"
+		"	-m --dumpmaps [<map type> <map name]"
+		"			dump automounter maps and exit\n"
 		"	-n --negative-timeout n\n"
 		"			set the timeout for failed key lookups.\n"
 		"	-O --global-options\n"
@@ -2125,22 +2126,33 @@ int main(int argc, char *argv[])
 			program);
 #endif
 
-	if (argc == 0)
-		master_list = master_new(NULL, timeout, ghost);
-	else
-		master_list = master_new(argv[0], timeout, ghost);
-
-	if (!master_list) {
-		printf("%s: can't create master map %s", program, argv[0]);
-		exit(1);
-	}
-
 	if (dumpmaps) {
 		struct master_mapent *entry;
 		struct list_head *head, *p;
 		struct mapent_cache *nc;
+		const char *type = NULL;
+		const char *name = NULL;
+		const char *master = NULL;
+
+		if (argc > 0) {
+			if (argc >= 2) {
+				type = argv[0];
+				name = argv[1];
+			}
+			if (argc == 3)
+				master = argv[2];
+		}
+
+		if (master)
+			master_list = master_new(NULL, timeout, ghost);
+		else
+			master_list = master_new(master, timeout, ghost);
+		if (!master_list) {
+			printf("%s: can't create master map", program);
+			exit(1);
+		}
 
-		open_log();
+		log_to_stderr();
 
 		master_init_scan();
 
@@ -2153,7 +2165,10 @@ int main(int argc, char *argv[])
 		master_list->nc = nc;
 
 		lookup_nss_read_master(master_list, 0);
-		master_show_mounts(master_list);
+		if (type)
+			dump_map(master_list, type, name);
+		else
+			master_show_mounts(master_list);
 
 		head = &master_list->mounts;
 		p = head->next;
@@ -2168,6 +2183,16 @@ int main(int argc, char *argv[])
 		exit(0);
 	}
 
+	if (argc == 0)
+		master_list = master_new(NULL, timeout, ghost);
+	else
+		master_list = master_new(argv[0], timeout, ghost);
+
+	if (!master_list) {
+		printf("%s: can't create master map %s", program, argv[0]);
+		exit(1);
+	}
+
 	become_daemon(foreground, daemon_check);
 
 	if (pthread_attr_init(&th_attr)) {
diff --git a/include/master.h b/include/master.h
index 824707c..e77d5ba 100644
--- a/include/master.h
+++ b/include/master.h
@@ -112,6 +112,7 @@ int master_submount_list_empty(struct autofs_point *ap);
 int master_notify_submount(struct autofs_point *, const char *path, enum states);
 void master_notify_state_change(struct master *, int);
 int master_mount_mounts(struct master *, time_t, int);
+int dump_map(struct master *, const char *, const char *);
 int master_show_mounts(struct master *);
 extern inline unsigned int master_get_logopt(void);
 int master_list_empty(struct master *);
diff --git a/lib/master.c b/lib/master.c
index 734485e..964b547 100644
--- a/lib/master.c
+++ b/lib/master.c
@@ -1329,6 +1329,193 @@ static void print_map_info(struct map_source *source)
 	return;
 }
 
+static int match_type(const char *source, const char *type)
+{
+	if (!strcmp(source, type))
+		return 1;
+	/* Sources file and files are synonymous */
+	if (!strncmp(source, type, 4) && (strlen(source) <= 5))
+		return 1;
+	return 0;
+}
+
+static char *get_map_name(char *string)
+{
+	char *name, *tmp;
+	char *start, *end, *base;
+
+	tmp = strdup(string);
+	if (!tmp) {
+		printf("error: allocation failure: %s\n", strerror(errno));
+		return NULL;
+	}
+
+	base = basename(tmp);
+	end = strchr(base, ',');
+	if (end)
+		*end = '\0';
+	start = strchr(tmp, '=');
+	if (start)
+		start++;
+	else {
+		char *colon = strrchr(base, ':');
+		if (colon)
+			start = ++colon;
+		else
+			start = base;
+	}
+
+	name = strdup(start);
+	if (!name)
+		printf("error: allocation failure: %s\n", strerror(errno));
+	free(tmp);
+
+	return name;
+}
+
+static int match_name(struct map_source *source, const char *name)
+{
+	int argc = source->argc;
+	int ret = 0;
+	int i;
+
+	/*
+	 * This can't work for old style "multi" type sources since
+	 * there's no way to know from which map the cache entry came
+	 * from and duplicate entries are ignored at map read time.
+	 * All we can really do is list all the entries for the given
+	 * multi map if one of its map names matches.
+	 */
+	for (i = 0; i < argc; i++) {
+		if (i == 0 || !strcmp(source->argv[i], "--")) {
+			if (i != 0) {
+				i++;
+				if (i >= argc)
+					break;
+			}
+
+			if (source->argv[i] && *source->argv[i] != '-') {
+				char *map = get_map_name(source->argv[i]);
+				if (!map)
+					break;
+				if (!strcmp(map, name)) {
+					ret = 1;
+					free(map);
+					break;
+				}
+				free(map);
+			}
+		}
+	}
+
+	return ret;
+}
+
+int dump_map(struct master *master, const char *type, const char *name)
+{
+	struct list_head *p, *head;
+
+	if (list_empty(&master->mounts)) {
+		printf("no master map entries found\n");
+		return 1;
+	}
+
+	head = &master->mounts;
+	p = head->next;
+	while (p != head) {
+		struct map_source *source;
+		struct master_mapent *this;
+		struct autofs_point *ap;
+		time_t now = time(NULL);
+
+		this = list_entry(p, struct master_mapent, list);
+		p = p->next;
+
+		ap = this->ap;
+
+		/*
+		 * Ensure we actually read indirect map entries so we can
+		 * list them. The map reads won't read any indirect map
+		 * entries (other than those in a file map) unless the
+		 * browse option is set.
+		 */
+		if (ap->type == LKP_INDIRECT)
+			ap->flags |= MOUNT_FLAG_GHOST;
+
+		/* Read the map content into the cache */
+		if (lookup_nss_read_map(ap, NULL, now))
+			lookup_prune_cache(ap, now);
+		else {
+			printf("failed to read map\n");
+			lookup_close_lookup(ap);
+			continue;
+		}
+
+		if (!this->maps) {
+			printf("no map sources found for %s\n", ap->path);
+			lookup_close_lookup(ap);
+			continue;
+		}
+
+		source = this->maps;
+		while (source) {
+			struct map_source *instance;
+			struct mapent *me;
+
+			instance = NULL;
+			if (source->type) {
+				if (!match_type(source->type, type)) {
+					source = source->next;
+					continue;
+				}
+				if (!match_name(source, name)) {
+					source = source->next;
+					continue;
+				}
+				instance = source;
+			} else {
+				struct map_source *map;
+
+				map = source->instance;
+				while (map) {
+					if (!match_type(map->type, type)) {
+						map = map->next;
+						continue;
+					}
+					if (!match_name(map, name)) {
+						map = map->next;
+						continue;
+					}
+					instance = map;
+					break;
+				}
+			}
+
+			if (!instance) {
+				source = source->next;
+				lookup_close_lookup(ap);
+				continue;
+			}
+
+			me = cache_lookup_first(source->mc);
+			if (!me)
+				printf("no keys found in map\n");
+			else {
+				do {
+					if (me->source == instance)
+						printf("%s\t%s\n", me->key, me->mapent);
+				} while ((me = cache_lookup_next(source->mc, me)));
+			}
+
+			lookup_close_lookup(ap);
+			return 1;
+		}
+		lookup_close_lookup(ap);
+	}
+
+	return 0;
+}
+
 int master_show_mounts(struct master *master)
 {
 	struct list_head *p, *head;
diff --git a/man/automount.8 b/man/automount.8
index 844b876..48f5c5a 100644
--- a/man/automount.8
+++ b/man/automount.8
@@ -57,8 +57,22 @@ Run the daemon in the foreground and log to stderr instead of syslog."
 Enables the use of ramdom selection when choosing a host from a
 list of replicated servers.
 .TP
-.I "\-m, \-\-dumpmaps"
-Dump configured automounter maps, then exit.
+.I "\-m, \-\-dumpmaps [<map type> <map name]"
+With no parameters, list information about the configured automounter
+maps, then exit.
+
+If the dumpmaps option is given and is followed by two parameters,
+"<map type> <map name>" then simple "<key, value>" pairs that would
+be read in by a map read are printed to stdout if the given map type
+and map name are found in the map configuration.
+
+If the map is an LDAP map and there is more than one map of same name
+in different base dns only the first map encountered by autofs will
+be listed.
+
+If the map type is an old style multi-map and any one of the map
+names in the multi-map entry matches the given map name the entries
+that would be used by autofs for the whole multi-map will be listed.
 .TP
 .I "\-O, \-\-global-options"
 Allows the specification of global mount options used for all master

[PATCH 20/25] autofs-5.0.7 - fix syncronize handle_mounts() shutdown

[Ian Kent]

A recent change to fix a problem where automount would exit when re-reading
the master map if more than on entry had been removed causes automount to
ignore HUP signals if the mount list is empty.

Removing that check and testing shows that the remainder of the change still
fixes the original problem.
---
 CHANGELOG          |    1 +
 daemon/automount.c |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 07f9d6d..618ac20 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -71,6 +71,7 @@
 - fix dumpmaps multi output.
 - try and cleanup after dumpmaps.
 - teach dumpmaps to output simple key value pairs.
+- fix syncronize handle_mounts() shutdown.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/daemon/automount.c b/daemon/automount.c
index 2ce49bf..97c726a 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -1286,7 +1286,7 @@ static int do_hup_signal(struct master *master, time_t age)
 
 	master_mutex_lock();
 	/* Already doing a map read or shutdown or no mounts */
-	if (master->reading || list_empty(&master->mounts)) {
+	if (master->reading) {
 		status = pthread_mutex_unlock(&mrc.mutex);
 		if (status)
 			fatal(status);

[PATCH 21/25] autofs-5.0.7 - fix fix wildcard multi map regression

[Ian Kent]

A recent patch to fix a wildcard multi map mount regression has a
side effect of causing a deadlock at startup when trying to re-connect
to existing mounts.

The patch required the map entry cache write lock be taken so the cache
could be updated. But when starting and trying to re-connect to existing
mounts there's no need to update the cache.
---
 CHANGELOG                |    1 +
 modules/lookup_file.c    |   25 ++++++++++++++++++++-----
 modules/lookup_ldap.c    |   23 +++++++++++++++++++----
 modules/lookup_nisplus.c |   26 +++++++++++++++++++++-----
 modules/lookup_sss.c     |   22 ++++++++++++++++++----
 modules/lookup_yp.c      |   23 +++++++++++++++++++----
 6 files changed, 98 insertions(+), 22 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 618ac20..c3f12e5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -72,6 +72,7 @@
 - try and cleanup after dumpmaps.
 - teach dumpmaps to output simple key value pairs.
 - fix syncronize handle_mounts() shutdown.
+- fix fix wildcard multi map regression.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/modules/lookup_file.c b/modules/lookup_file.c
index 4b4ee89..83ef048 100644
--- a/modules/lookup_file.c
+++ b/modules/lookup_file.c
@@ -1042,7 +1042,7 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return NSS_STATUS_UNAVAIL;
 		}
 
-		cache_writelock(mc);
+		cache_readlock(mc);
 		me = cache_lookup_first(mc);
 		if (me && st.st_mtime <= me->age) {
 			/*
@@ -1084,7 +1084,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 		}
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 do_cache_lookup:
 	me = cache_lookup(mc, key);
 	/*
@@ -1102,10 +1113,11 @@ do_cache_lookup:
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (!(ap->flags & MOUNT_FLAG_REMOUNT) &&
+		    ap->type == LKP_INDIRECT && *me->key == '*') {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -1130,6 +1142,9 @@ do_cache_lookup:
 	ret = ctxt->parse->parse_mount(ap, key, key_len,
 				       mapent, ctxt->parse->context);
 	if (ret) {
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		cache_update_negative(mc, source, key, ap->negative_timeout);
 		cache_unlock(mc);
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index d05098f..2ab1e8c 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -3016,7 +3016,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -3028,10 +3039,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (!(ap->flags & MOUNT_FLAG_REMOUNT) &&
+		    ap->type == LKP_INDIRECT && *me->key == '*') {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -3053,6 +3065,9 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	ret = ctxt->parse->parse_mount(ap, key, key_len,
 				       mapent, ctxt->parse->context);
 	if (ret) {
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		cache_update_negative(mc, source, key, ap->negative_timeout);
 		cache_unlock(mc);
diff --git a/modules/lookup_nisplus.c b/modules/lookup_nisplus.c
index ef942a7..08878bb 100644
--- a/modules/lookup_nisplus.c
+++ b/modules/lookup_nisplus.c
@@ -561,7 +561,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -573,10 +584,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (!(ap->flags & MOUNT_FLAG_REMOUNT) &&
+		    ap->type == LKP_INDIRECT && *me->key == '*') {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -603,6 +615,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 		time_t now = time(NULL);
 		int rv = CHE_OK;
 
+		free(mapent);
+
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		me = cache_lookup_distinct(mc, key);
 		if (!me)
@@ -612,7 +629,6 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			me->status = time(NULL) + ap->negative_timeout;
 		}
 		cache_unlock(mc);
-		free(mapent);
 		return NSS_STATUS_TRYAGAIN;
 	}
 	free(mapent);
diff --git a/modules/lookup_sss.c b/modules/lookup_sss.c
index 1fe740b..4a9cfd2 100644
--- a/modules/lookup_sss.c
+++ b/modules/lookup_sss.c
@@ -635,7 +635,17 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_readlock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_writelock(mc);
+	else
+		cache_readlock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -647,10 +657,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (ap->type == LKP_INDIRECT && *me->key == '*' &&
+		   !(ap->flags & MOUNT_FLAG_REMOUNT)) {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -672,6 +683,9 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	ret = ctxt->parse->parse_mount(ap, key, key_len,
 				       mapent, ctxt->parse->context);
 	if (ret) {
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		cache_update_negative(mc, source, key, ap->negative_timeout);
 		cache_unlock(mc);
diff --git a/modules/lookup_yp.c b/modules/lookup_yp.c
index e99e3c0..4d1848e 100644
--- a/modules/lookup_yp.c
+++ b/modules/lookup_yp.c
@@ -662,7 +662,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -674,10 +685,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (ap->type == LKP_INDIRECT && *me->key == '*' &&
+		   !(ap->flags & MOUNT_FLAG_REMOUNT)) {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -698,6 +710,9 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 		ret = ctxt->parse->parse_mount(ap, key, key_len,
 					       mapent, ctxt->parse->context);
 		if (ret) {
+			/* Don't update negative cache when re-connecting */
+			if (ap->flags & MOUNT_FLAG_REMOUNT)
+				return NSS_STATUS_TRYAGAIN;
 			cache_writelock(mc);
 			cache_update_negative(mc, source, key, ap->negative_timeout);
 			cache_unlock(mc);

[PATCH 22/25] autofs-5.0.7 - improve timeout option description

[Ian Kent]

The description of the configuration option is a little misleading.
So add some explaination about the internal default and the reason
the default installed configuration overrides it.
---
 CHANGELOG                      |    1 +
 man/auto.master.5.in           |    5 ++++-
 man/automount.8                |    5 ++++-
 redhat/autofs.sysconfig.in     |    5 ++++-
 samples/autofs.conf.default.in |    5 ++++-
 5 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index c3f12e5..9639703 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -73,6 +73,7 @@
 - teach dumpmaps to output simple key value pairs.
 - fix syncronize handle_mounts() shutdown.
 - fix fix wildcard multi map regression.
+- improve timeout option description.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/man/auto.master.5.in b/man/auto.master.5.in
index 7907ffa..65a86b2 100644
--- a/man/auto.master.5.in
+++ b/man/auto.master.5.in
@@ -212,7 +212,10 @@ configuration file
 They are:
 .TP
 .B TIMEOUT
-sets the default mount timeout (program default 600).
+Sets the default mount timeout in seconds. The internal program
+default is 10 minutes, but the default installed configuration
+overrides this and sets the timeout to 5 minutes to be consistent
+with earlier autofs releases.
 .TP
 .B NEGATIVE_TIMEOUT
 Set the default timeout for caching failed key lookups (program default
diff --git a/man/automount.8 b/man/automount.8
index 48f5c5a..abb40bc 100644
--- a/man/automount.8
+++ b/man/automount.8
@@ -29,10 +29,13 @@ Print brief help on program usage.
 .I "\-p, \-\-pid-file"
 Write the pid of the daemon to the specified file.
 .TP
-.I "\-t, \-\-timeout"
+.I "\-t <seconds>, \-\-timeout <seconds>"
 Set the global minimum timeout, in seconds, until directories
 are unmounted. The default is 10 minutes. Setting the timeout
 to zero disables umounts completely.
+The internal program default is 10 minutes, but the default
+installed configuration overrides this and sets the timeout
+to 5 minutes to be consistent with earlier autofs releases.
 .TP
 .I "\-n <seconds>, \-\-negative\-timeout <seconds>"
 Set the default timeout for caching failed key lookups. The default is 60 seconds.
diff --git a/redhat/autofs.sysconfig.in b/redhat/autofs.sysconfig.in
index a8992c4..9d8d704 100644
--- a/redhat/autofs.sysconfig.in
+++ b/redhat/autofs.sysconfig.in
@@ -5,7 +5,10 @@
 #
 #MASTER_MAP_NAME="auto.master"
 #
-# TIMEOUT - set the default mount timeout (default 600).
+# TIMEOUT - set the default mount timeout in secons. The internal
+#	    program default is 10 minutes, but the default installed
+#	    configuration overrides this and sets the timeout to 5
+#	    minutes to be consistent with earlier autofs releases.
 #
 TIMEOUT=300
 #
diff --git a/samples/autofs.conf.default.in b/samples/autofs.conf.default.in
index 1da89cf..695b010 100644
--- a/samples/autofs.conf.default.in
+++ b/samples/autofs.conf.default.in
@@ -5,7 +5,10 @@
 #
 #MASTER_MAP_NAME="auto.master"
 #
-# TIMEOUT - set the default mount timeout (default 600).
+# TIMEOUT - set the default mount timeout in secons. The internal
+#	    program default is 10 minutes, but the default installed
+#	    configuration overrides this and sets the timeout to 5
+#	    minutes to be consistent with earlier autofs releases.
 #
 TIMEOUT=300
 #

[PATCH 23/25] autofs-5.0.7 - only probe specific nfs version when requested

[Ian Kent]

If a specific NFS version is given as an option the set the probe flags
to probe only that version.
---
 CHANGELOG           |    1 +
 modules/mount_nfs.c |   10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 9639703..2499f1c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -74,6 +74,7 @@
 - fix syncronize handle_mounts() shutdown.
 - fix fix wildcard multi map regression.
 - improve timeout option description.
+- only probe specific nfs version if requested.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/modules/mount_nfs.c b/modules/mount_nfs.c
index adf2002..5a582ef 100644
--- a/modules/mount_nfs.c
+++ b/modules/mount_nfs.c
@@ -146,7 +146,15 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 				if (strncmp("vers=4", cp, o_len) == 0 ||
 				    strncmp("nfsvers=4", cp, o_len) == 0)
 					vers = NFS4_VERS_MASK | TCP_SUPPORTED;
-				else if (strstr(cp, "port=") == cp &&
+				else if (strncmp("vers=3", cp, o_len) == 0 ||
+					 strncmp("nfsvers=3", cp, o_len) == 0) {
+					vers &= ~(NFS4_VERS_MASK | NFS_VERS_MASK);
+					vers |= NFS3_REQUESTED;
+				} else if (strncmp("vers=2", cp, o_len) == 0 ||
+					 strncmp("nfsvers=2", cp, o_len) == 0) {
+					vers &= ~(NFS4_VERS_MASK | NFS_VERS_MASK);
+					vers |= NFS2_REQUESTED;
+				} else if (strstr(cp, "port=") == cp &&
 					 o_len - 5 < 25) {
 					char optport[25];
 

[PATCH 24/25] autofs-5.0.7 - fix bad mkdir permission on create

[Ian Kent]

Reported by Gordon Lack (gordon[dot]m[dot]lack[at]gsk[dot]com).

If the automount daemon needs to create a directory (hierarchy) for an
automount and it is started up with a umask of 027 (or similar) then it
creates unusable directories (permission == 550).
---
 CHANGELOG          |    1 +
 daemon/automount.c |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 2499f1c..3a3fec1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -75,6 +75,7 @@
 - fix fix wildcard multi map regression.
 - improve timeout option description.
 - only probe specific nfs version if requested.
+- fix bad mkdir permission on create.
 
 25/07/2012 autofs-5.0.7
 =======================
diff --git a/daemon/automount.c b/daemon/automount.c
index 97c726a..6b55103 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -122,7 +122,10 @@ static int do_mkdir(const char *parent, const char *path, mode_t mode)
 		status = statfs(parent, &fs);
 	if ((status != -1 && fs.f_type == (__SWORD_TYPE) AUTOFS_SUPER_MAGIC) ||
 	    contained_in_local_fs(path)) {
-		if (mkdir(path, mode) == -1) {
+		int mask = umask(0022);
+		int ret = mkdir(path, mode);
+		void umask(mask);
+		if (ret == -1) {
 			errno = EACCES;
 			return 0;
 		}

Re: [PATCH 24/25] autofs-5.0.7 - fix bad mkdir permission on create

[Ian Kent]

On Mon, 2013-08-19 at 09:14 +0800, Ian Kent wrote:
> Reported by Gordon Lack (gordon[dot]m[dot]lack[at]gsk[dot]com).
> 
> If the automount daemon needs to create a directory (hierarchy) for an
> automount and it is started up with a umask of 027 (or similar) then it
> creates unusable directories (permission == 550).
> ---
>  CHANGELOG          |    1 +
>  daemon/automount.c |    5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/CHANGELOG b/CHANGELOG
> index 2499f1c..3a3fec1 100644
> --- a/CHANGELOG
> +++ b/CHANGELOG
> @@ -75,6 +75,7 @@
>  - fix fix wildcard multi map regression.
>  - improve timeout option description.
>  - only probe specific nfs version if requested.
> +- fix bad mkdir permission on create.
>  
>  25/07/2012 autofs-5.0.7
>  =======================
> diff --git a/daemon/automount.c b/daemon/automount.c
> index 97c726a..6b55103 100644
> --- a/daemon/automount.c
> +++ b/daemon/automount.c
> @@ -122,7 +122,10 @@ static int do_mkdir(const char *parent, const char *path, mode_t mode)
>  		status = statfs(parent, &fs);
>  	if ((status != -1 && fs.f_type == (__SWORD_TYPE) AUTOFS_SUPER_MAGIC) ||
>  	    contained_in_local_fs(path)) {
> -		if (mkdir(path, mode) == -1) {
> +		int mask = umask(0022);
> +		int ret = mkdir(path, mode);
> +		void umask(mask);

OK, so that void should be (void).

> +		if (ret == -1) {
>  			errno = EACCES;
>  			return 0;
>  		}
> 
> --
> 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

[PATCH 25/25] autofs-5.0.7 - setup program map env from macro table

[Ian Kent]

The ability to pass parameters to program maps, in some way, is needed.
Standard autofs specifies that program maps have one argument so passing
parameters as arguments shouldn't be done.

This patch sets the existing macro table definitions (for both global and
local table) as environment variables before calling the map. The values
are not checked after return so, at this stage, program maps can't change
macro definitions.
---
 include/macros.h         |    1 +
 lib/macros.c             |   28 ++++++++++++++++++++++++++++
 modules/lookup_program.c |   20 ++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/include/macros.h b/include/macros.h
index a73a4a7..5077b5d 100644
--- a/include/macros.h
+++ b/include/macros.h
@@ -40,5 +40,6 @@ void macro_free_global_table(void);
 void macro_free_table(struct substvar *table);
 const struct substvar *
 macro_findvar(const struct substvar *table, const char *str, int len);
+void macro_setenv(const struct substvar *table);
 
 #endif
diff --git a/lib/macros.c b/lib/macros.c
index 32b70bf..33c2ada 100644
--- a/lib/macros.c
+++ b/lib/macros.c
@@ -421,3 +421,31 @@ macro_findvar(const struct substvar *table, const char *str, int len)
 	return NULL;
 }
 
+/* Set environment from macro variable table */
+void macro_setenv(const struct substvar *table)
+{
+	const struct substvar *sv = system_table;
+	const struct substvar *lv = table;
+
+	/*
+	 * First set environment from global table, matching local
+	 * variables will overwrite these.
+	 */
+	while (sv) {
+		if (sv->def)
+			setenv(sv->def, sv->val, 1);
+		sv = sv->next;
+	}
+
+	error(LOGOPT_ANY, "table %p", table);
+	dump_table(table);
+
+	/* Next set environment from the local table */
+	while (lv) {
+		if (lv->def)
+			setenv(lv->def, lv->val, 1);
+		lv = lv->next;
+	}
+
+	return;
+}
diff --git a/modules/lookup_program.c b/modules/lookup_program.c
index 2457108..7e22b38 100644
--- a/modules/lookup_program.c
+++ b/modules/lookup_program.c
@@ -36,9 +36,17 @@
 
 struct lookup_context {
 	const char *mapname;
+	char *mapfmt;
 	struct parse_mod *parse;
 };
 
+struct parse_context {
+	char *optstr;		/* Mount options */
+	char *macros;		/* Map wide macro defines */
+	struct substvar *subst;	/* $-substitutions */
+	int slashify_colons;	/* Change colons to slashes? */
+};
+
 int lookup_version = AUTOFS_LOOKUP_VERSION;	/* Required by protocol */
 
 int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **context)
@@ -79,6 +87,8 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
 	if (!mapfmt)
 		mapfmt = MAPFMT_DEFAULT;
 
+	ctxt->mapfmt = strdup(mapfmt);
+
 	ctxt->parse = open_parse(mapfmt, MODPREFIX, argc - 1, argv + 1);
 	if (!ctxt->parse) {
 		logmsg(MODPREFIX "failed to open parse context");
@@ -255,6 +265,14 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			warn(ap->logopt,
 			     MODPREFIX "failed to set PWD to %s for map %s",
 			     ap->path, ctxt->mapname);
+		/*
+		 * MAPFMT_DEFAULT must be "sun" for ->parse_init() to have setup
+		 * the macro table.
+		 */
+		if (ctxt->mapfmt && strcmp(ctxt->mapfmt, "MAPFMT_DEFAULT")) {
+			struct parse_context *pctxt = (struct parse_context *) ctxt->parse->context;
+			macro_setenv(pctxt->subst);
+		}
 		execl(ctxt->mapname, ctxt->mapname, name, NULL);
 		_exit(255);	/* execl() failed */
 	}
@@ -448,6 +466,8 @@ int lookup_done(void *context)
 {
 	struct lookup_context *ctxt = (struct lookup_context *) context;
 	int rv = close_parse(ctxt->parse);
+	if (ctxt->mapfmt)
+		free(ctxt->mapfmt);
 	free(ctxt);
 	return rv;
 }

Re: [PATCH 00/25] Current autofs patch queue

[Dennis Lan (dlan)]

On Mon, Aug 19, 2013 at 9:11 AM, Ian Kent <raven@themaw.net> wrote:
> There are patches from several contributors I plan on committing
> so I thought posting them to the list for final review would be
> sensible.
>
> I'm not sure when I'll commit the changes but I hope in the next
> couple of days.
>
> ---
>
> Dustin Polke (1):
>       autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl
>
> Ian Kent (13):
>       autofs-5.0.7 - don't override LDFLAGS in make rules
>       autofs-5.0.7 - fix a couple of compiler warnings
>       autofs-5.0.7 - add after sssd dependency to unit file
>       autofs-5.0.7 - dont start readmap unless ready
>       autofs-5.0.7 - fix dumpmaps multi output
>       autofs-5.0.7 - try and cleanup after dumpmaps
>       autofs-5.0.7 - teach dumpmaps to output simple key value pairs
>       autofs-5.0.7 - fix syncronize handle_mounts() shutdown
>       autofs-5.0.7 - fix fix wildcard multi map regression
>       autofs-5.0.7 - improve timeout option description
>       autofs-5.0.7 - only probe specific nfs version when requested
>       autofs-5.0.7 - fix bad mkdir permission on create
>       autofs-5.0.7 - setup program map env from macro table
>
> Lan Yixun (dlan) (10):
>       autofs-5.0.7 - fix compile error with heimdal support enabled
>       autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown
>       autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting
>       autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled
>       autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled
>       autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones
>       autofs-5.0.7 - remove hard-coded STRIP variable
>       autofs-5.0.7 - use LIBS for link libraries
>       autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other
>       autofs-5.0.7 - fix occasional build error when enable parallel compiling
>
> Leonardo Chiquitto (1):
>       autofs-5.0.7 - fix crash due to thread unsafe use of libldap
>
>
>  CHANGELOG                      |   23 ++++
>  Makefile.conf.in               |    3
>  Makefile.rules                 |   34 +++--
>  aclocal.m4                     |   19 ++-
>  autofs.spec                    |    3
>  configure                      |  104 +++++++++++++++--
>  configure.in                   |    2
>  daemon/Makefile                |    2
>  daemon/automount.c             |   71 +++++++++--
>  daemon/state.c                 |   15 +-
>  include/config.h.in            |    3
>  include/lookup_ldap.h          |    4 -
>  include/macros.h               |    1
>  include/master.h               |    1
>  lib/Makefile                   |    6 +
>  lib/macros.c                   |   28 ++++
>  lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
>  man/auto.master.5.in           |    5 +
>  man/automount.8                |   23 +++-
>  modules/Makefile               |   20 ++-
>  modules/cyrus-sasl.c           |   39 +++++-
>  modules/lookup_file.c          |   25 +++-
>  modules/lookup_ldap.c          |   76 +++++++++++-
>  modules/lookup_nisplus.c       |   26 +++-
>  modules/lookup_program.c       |   20 +++
>  modules/lookup_sss.c           |   22 +++-
>  modules/lookup_yp.c            |   23 +++-
>  modules/mount_bind.c           |    2
>  modules/mount_nfs.c            |   14 ++
>  redhat/autofs.sysconfig.in     |    5 +
>  samples/autofs.conf.default.in |    5 +
>  samples/autofs.service.in      |    2
>  32 files changed, 753 insertions(+), 123 deletions(-)
>
> --
> Ian


HI Ian:
 May I suggest you pushing this patch serials to one test branch
(for-next?) in the public repository?
I find this much more convenient for me to test (instead of fetch all
the patches and apply myself).
Since this branch is experimental, you can delete it later or rebase
to master branch.

Dennis Lan (dlan)

Re: [PATCH 00/25] Current autofs patch queue

[Ian Kent]

On Mon, 2013-08-19 at 13:30 +0800, Dennis Lan (dlan) wrote:
> On Mon, Aug 19, 2013 at 9:11 AM, Ian Kent <raven@themaw.net> wrote:
> > There are patches from several contributors I plan on committing
> > so I thought posting them to the list for final review would be
> > sensible.
> >
> > I'm not sure when I'll commit the changes but I hope in the next
> > couple of days.
> >
> > ---
> >
> > Dustin Polke (1):
> >       autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl
> >
> > Ian Kent (13):
> >       autofs-5.0.7 - don't override LDFLAGS in make rules
> >       autofs-5.0.7 - fix a couple of compiler warnings
> >       autofs-5.0.7 - add after sssd dependency to unit file
> >       autofs-5.0.7 - dont start readmap unless ready
> >       autofs-5.0.7 - fix dumpmaps multi output
> >       autofs-5.0.7 - try and cleanup after dumpmaps
> >       autofs-5.0.7 - teach dumpmaps to output simple key value pairs
> >       autofs-5.0.7 - fix syncronize handle_mounts() shutdown
> >       autofs-5.0.7 - fix fix wildcard multi map regression
> >       autofs-5.0.7 - improve timeout option description
> >       autofs-5.0.7 - only probe specific nfs version when requested
> >       autofs-5.0.7 - fix bad mkdir permission on create
> >       autofs-5.0.7 - setup program map env from macro table
> >
> > Lan Yixun (dlan) (10):
> >       autofs-5.0.7 - fix compile error with heimdal support enabled
> >       autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown
> >       autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting
> >       autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled
> >       autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled
> >       autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones
> >       autofs-5.0.7 - remove hard-coded STRIP variable
> >       autofs-5.0.7 - use LIBS for link libraries
> >       autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other
> >       autofs-5.0.7 - fix occasional build error when enable parallel compiling
> >
> > Leonardo Chiquitto (1):
> >       autofs-5.0.7 - fix crash due to thread unsafe use of libldap
> >
> >
> >  CHANGELOG                      |   23 ++++
> >  Makefile.conf.in               |    3
> >  Makefile.rules                 |   34 +++--
> >  aclocal.m4                     |   19 ++-
> >  autofs.spec                    |    3
> >  configure                      |  104 +++++++++++++++--
> >  configure.in                   |    2
> >  daemon/Makefile                |    2
> >  daemon/automount.c             |   71 +++++++++--
> >  daemon/state.c                 |   15 +-
> >  include/config.h.in            |    3
> >  include/lookup_ldap.h          |    4 -
> >  include/macros.h               |    1
> >  include/master.h               |    1
> >  lib/Makefile                   |    6 +
> >  lib/macros.c                   |   28 ++++
> >  lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
> >  man/auto.master.5.in           |    5 +
> >  man/automount.8                |   23 +++-
> >  modules/Makefile               |   20 ++-
> >  modules/cyrus-sasl.c           |   39 +++++-
> >  modules/lookup_file.c          |   25 +++-
> >  modules/lookup_ldap.c          |   76 +++++++++++-
> >  modules/lookup_nisplus.c       |   26 +++-
> >  modules/lookup_program.c       |   20 +++
> >  modules/lookup_sss.c           |   22 +++-
> >  modules/lookup_yp.c            |   23 +++-
> >  modules/mount_bind.c           |    2
> >  modules/mount_nfs.c            |   14 ++
> >  redhat/autofs.sysconfig.in     |    5 +
> >  samples/autofs.conf.default.in |    5 +
> >  samples/autofs.service.in      |    2
> >  32 files changed, 753 insertions(+), 123 deletions(-)
> >
> > --
> > Ian
> 
> 
> HI Ian:
>  May I suggest you pushing this patch serials to one test branch
> (for-next?) in the public repository?
> I find this much more convenient for me to test (instead of fetch all
> the patches and apply myself).
> Since this branch is experimental, you can delete it later or rebase
> to master branch.

Not sure I've done this correctly but try updating and checkout branch
pending-temp-v0.

Ian

Re: [PATCH 00/25] Current autofs patch queue

[Dennis Lan (dlan)]

On Tue, Aug 20, 2013 at 10:55 AM, Ian Kent <raven@themaw.net> wrote:
> On Mon, 2013-08-19 at 13:30 +0800, Dennis Lan (dlan) wrote:
>> On Mon, Aug 19, 2013 at 9:11 AM, Ian Kent <raven@themaw.net> wrote:
>> > There are patches from several contributors I plan on committing
>> > so I thought posting them to the list for final review would be
>> > sensible.
>> >
>> > I'm not sure when I'll commit the changes but I hope in the next
>> > couple of days.
>> >
>> > ---
>> >
>> > Dustin Polke (1):
>> >       autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl
>> >
>> > Ian Kent (13):
>> >       autofs-5.0.7 - don't override LDFLAGS in make rules
>> >       autofs-5.0.7 - fix a couple of compiler warnings
>> >       autofs-5.0.7 - add after sssd dependency to unit file
>> >       autofs-5.0.7 - dont start readmap unless ready
>> >       autofs-5.0.7 - fix dumpmaps multi output
>> >       autofs-5.0.7 - try and cleanup after dumpmaps
>> >       autofs-5.0.7 - teach dumpmaps to output simple key value pairs
>> >       autofs-5.0.7 - fix syncronize handle_mounts() shutdown
>> >       autofs-5.0.7 - fix fix wildcard multi map regression
>> >       autofs-5.0.7 - improve timeout option description
>> >       autofs-5.0.7 - only probe specific nfs version when requested
>> >       autofs-5.0.7 - fix bad mkdir permission on create
>> >       autofs-5.0.7 - setup program map env from macro table
>> >
>> > Lan Yixun (dlan) (10):
>> >       autofs-5.0.7 - fix compile error with heimdal support enabled
>> >       autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown
>> >       autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting
>> >       autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled
>> >       autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled
>> >       autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones
>> >       autofs-5.0.7 - remove hard-coded STRIP variable
>> >       autofs-5.0.7 - use LIBS for link libraries
>> >       autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other
>> >       autofs-5.0.7 - fix occasional build error when enable parallel compiling
>> >
>> > Leonardo Chiquitto (1):
>> >       autofs-5.0.7 - fix crash due to thread unsafe use of libldap
>> >
>> >
>> >  CHANGELOG                      |   23 ++++
>> >  Makefile.conf.in               |    3
>> >  Makefile.rules                 |   34 +++--
>> >  aclocal.m4                     |   19 ++-
>> >  autofs.spec                    |    3
>> >  configure                      |  104 +++++++++++++++--
>> >  configure.in                   |    2
>> >  daemon/Makefile                |    2
>> >  daemon/automount.c             |   71 +++++++++--
>> >  daemon/state.c                 |   15 +-
>> >  include/config.h.in            |    3
>> >  include/lookup_ldap.h          |    4 -
>> >  include/macros.h               |    1
>> >  include/master.h               |    1
>> >  lib/Makefile                   |    6 +
>> >  lib/macros.c                   |   28 ++++
>> >  lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
>> >  man/auto.master.5.in           |    5 +
>> >  man/automount.8                |   23 +++-
>> >  modules/Makefile               |   20 ++-
>> >  modules/cyrus-sasl.c           |   39 +++++-
>> >  modules/lookup_file.c          |   25 +++-
>> >  modules/lookup_ldap.c          |   76 +++++++++++-
>> >  modules/lookup_nisplus.c       |   26 +++-
>> >  modules/lookup_program.c       |   20 +++
>> >  modules/lookup_sss.c           |   22 +++-
>> >  modules/lookup_yp.c            |   23 +++-
>> >  modules/mount_bind.c           |    2
>> >  modules/mount_nfs.c            |   14 ++
>> >  redhat/autofs.sysconfig.in     |    5 +
>> >  samples/autofs.conf.default.in |    5 +
>> >  samples/autofs.service.in      |    2
>> >  32 files changed, 753 insertions(+), 123 deletions(-)
>> >
>> > --
>> > Ian
>>
>>
>> HI Ian:
>>  May I suggest you pushing this patch serials to one test branch
>> (for-next?) in the public repository?
>> I find this much more convenient for me to test (instead of fetch all
>> the patches and apply myself).
>> Since this branch is experimental, you can delete it later or rebase
>> to master branch.
>
> Not sure I've done this correctly but try updating and checkout branch
> pending-temp-v0.
>
> Ian
>
>
HI Ian:
  what you 've done is correct!
  I tested this branch, so far so good..

 you can use "git push origin :refs/heads/pending-temp-v0" to delete
this branch in the remote repository
or "git push --force origin HEAD:refs/heads/pending-temp-v0" to
override the remote branch with new commit (just in case you need to
do this)

Dennis Lan (dlan)

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 08/19/2013 03:11 AM, Ian Kent wrote:
> There are patches from several contributors I plan on committing
> so I thought posting them to the list for final review would be
> sensible.

I am wondering about my auto.smb patch (see
http://www.spinics.net/lists/autofs/msg00402.html). Back then, you said

> 
> All I'm really saying is that, the program map returns a string that is
> then used as the map entry, so when it is parsed the variables $UID,
> $GID, etc. will be present in the macro lookup table as ${UID} (and $UID
> etc. works) ... so maybe these same variables should be provided in the
> environment of the program.
> 
> It isn't entirely clear if that is needed since those macro values may
> be returned in the map entry text with things like echo "/some/string
> \ ... ${UID} ..." etc. for later translation.
> 
> Really, a revised patch depends on this question being answered first. 

Nobody has spoken up on this list saying that additional environment
variables would need to be set. I am wondering what else needs to be
done in order to merge this (IMO very useful) functionality.

Martin

-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

I am wondering about my auto.smb patch (see http://www.spinics.net/lists/autofs/msg00402.html).

I can add a comment about that link...


> > OTOH, setting the uid to
> > the caller is definitely what should be done, IMHO.

That only makes sense if the result is only going to be visible to that one individual.

Linux is a multi-user system, so having automounts that depend on who made the request to do the mount makes no sense if the result is going to be visible to everyone.

I've not seen a system whereby different people have different views of what is mounted.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 09/02/2013 12:41 PM, Gordon Lack wrote:
> I am wondering about my auto.smb patch (see http://www.spinics.net/lists/autofs/msg00402.html).
> 
> I can add a comment about that link...
> 
> 
>>> OTOH, setting the uid to
>>> the caller is definitely what should be done, IMHO.
> 
> That only makes sense if the result is only going to be visible to that one individual.
> 
> Linux is a multi-user system, so having automounts that depend on who made the request to do the mount makes no sense if the result is going to be visible to everyone.
> I've not seen a system whereby different people have different views
> of what is mounted.

I'm not sure what you mean. Users can create user-specific CIFS mounts
today with "sudo mount -t cifs -o sec=krb5,cruid=$UID,...". The
intention of my patch is just to enable autofs to create such mounts.

It would certainly be reasonable to setup autofs such that a separate
mount point is used for each user, but that's a configuration issue. In
as properly configured environment, the "multiuser" option of mount.cifs
can be used to deal with local multiuser/permission issues.

Regards
Martin

P.S.: You are correct that there is an issue with autofs caching the
list of mount points if the list of mountable shares returned by a
server depends on the credentials provided. AFAICS that can be fixed by
configuring the such that all users can see the same shares (but not
necessarily access/mount them all).


> 
> ________________________________
> 
> This e-mail was sent by GlaxoSmithKline Services Unlimited
> (registered in England and Wales No. 1047315), which is a
> member of the GlaxoSmithKline group of companies. The
> registered address of GlaxoSmithKline Services Unlimited
> is 980 Great West Road, Brentford, Middlesex TW8 9GS.
> 


-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

>> I'm not sure what you mean.

Just that I've not seen it, so thanks for the example.

>> Users can create user-specific CIFS mounts today with "sudo mount -t cifs -o sec=krb5,cruid=$UID,...".
>> The intention of my patch is just to enable autofs to create such mounts.

But that leaves the mount permission dependent on who make the first call.

>> P.S.: You are correct that there is an issue with autofs caching the list of mount points if the
>> list of mountable shares returned by a server depends on the credentials provided. AFAICS that can
>> be fixed by configuring the such that all users can see the same shares (but not necessarily
>> access/mount them all).

And once you've done that the UID that needs to be used for each of these mounts is mount-specific,
not "who caused the mount"-specific. Which is why I see a problem with it.



________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 09/02/2013 01:13 PM, Gordon Lack wrote:
>>> I'm not sure what you mean.
> 
> Just that I've not seen it, so thanks for the example.
> 
>>> Users can create user-specific CIFS mounts today with "sudo mount -t cifs -o sec=krb5,cruid=$UID,...".
>>> The intention of my patch is just to enable autofs to create such mounts.
> 
> But that leaves the mount permission dependent on who make the first call.

True. But that holds in the manual "mount -t cifs ..." case as well.

>>> P.S.: You are correct that there is an issue with autofs caching the list of mount points if the
>>> list of mountable shares returned by a server depends on the credentials provided. AFAICS that can
>>> be fixed by configuring the such that all users can see the same shares (but not necessarily
>>> access/mount them all).
> 
> And once you've done that the UID that needs to be used for each of these mounts is mount-specific,
> not "who caused the mount"-specific. Which is why I see a problem with it.

Do you have security concerns, or is it just that you don't consider it
useful? I can just say that I find it very useful working on a Linux
workstation in an AD-dominated environment. It's much more practical
than using any of the "Network Browsing" utilities provided by GNOME and
the like. Just my personal opinion.

I admit I have been using it mostly on my workstation, where I am the
only user.

Martin

> 
> 
> 
> ________________________________
> 
> This e-mail was sent by GlaxoSmithKline Services Unlimited
> (registered in England and Wales No. 1047315), which is a
> member of the GlaxoSmithKline group of companies. The
> registered address of GlaxoSmithKline Services Unlimited
> is 980 Great West Road, Brentford, Middlesex TW8 9GS.
> 


-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

>> But that leaves the mount permission dependent on who make the first call.

> True. But that holds in the manual "mount -t cifs ..." case as well.

The manual mount will be done by a specific individual who (hopefully) knows what they are doing.

An automount can be done by a non-determined account and so have a non-determinate outcome once you put per-caller variables into the rule.

>> And once you've done that the UID that needs to be used for each of
>> these mounts is mount-specific, not "who caused the mount"-specific. Which is why I see a problem with it.

> Do you have security concerns, or is it just that you don't consider it useful?

Both.
Its presence would encourage its use.
You will then find users who set it up for themselves and then get confused when another account has made the mount and access is wrong, but everything "looks" correct.
Mind you - the security issue is about using cifs mounts on a system which has the potential for >1 concurrent user anyway.

> But I admit I have been using it mostly on my workstation, where I am the only user.

So you can set-up the rules so that they contain just your id.
And make sure you lock-out all other users once you have anything mounted.


________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 09/02/2013 02:55 PM, Gordon Lack wrote:
>>> But that leaves the mount permission dependent on who make the first call.
> 
>> True. But that holds in the manual "mount -t cifs ..." case as well.
> 
> The manual mount will be done by a specific individual who (hopefully) knows what they are doing.
> 
> An automount can be done by a non-determined account and so have a non-determinate outcome once you put per-caller variables into the rule.

What my patch does is to run the map script under the UID of the user
requesting the mount, rather than root. That is actually an improvement
of the security situation, AFAICS.

>>> And once you've done that the UID that needs to be used for each of
>>> these mounts is mount-specific, not "who caused the mount"-specific. Which is why I see a problem with it.
> 
>> Do you have security concerns, or is it just that you don't consider it useful?
> 
> Both.
> Its presence would encourage its use.
> You will then find users who set it up for themselves and then get confused when another account has made the mount and access is wrong, but everything "looks" correct.
> Mind you - the security issue is about using cifs mounts on a system which has the potential for >1 concurrent user anyway.

Please check the "multiuser" option of mount.cifs ("With this option,
the client ... creates a new session with the server using the user's
credentials whenever a new user accesses the mount"). With that option,
I see no major difference between CIFS and NFS automounts,
security-wise. IMO combining autofs and "multiuser" is exactly the
desired behavior in an AD environment. It lets the server decide access
rights based on the credentials provided.

>> But I admit I have been using it mostly on my workstation, where I am the only user.
> 
> So you can set-up the rules so that they contain just your id.
> And make sure you lock-out all other users once you have anything mounted.

I don't think that's necessary, see above.

Your arguments would apply equally well to users clicking on "Network
Environment" or similar in their GUI. It's a (sad) fact in life that
many of us have to work in Windows-dominated IT environments.

Martin

-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

>> What my patch does is to run the map script under the UID of the user requesting the mount, rather than root.
>> That is actually an improvement of the security situation, AFAICS.

Possibly not.

>> Please check the "multiuser" option of mount.cifs ("With this option, the client ... creates a new session with
>> the server using the user's credentials whenever a new user accesses the mount").

It also says:

           Furthermore, when unix extensions aren't in use and the
           administrator has not overriden ownership using the uid= or gid=
           options, ownership of files is presented as the current user
           accessing the share.

Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.

Unless cruid is different? (Never used it - or Kerberos - so don't know).

>> With that option, I see no major difference between CIFS and NFS automounts, security-wise.
>> IMO combining autofs and "multiuser" is exactly the desired behavior in an AD environment.
>> It lets the server decide access rights based on the credentials provided.

You could well be right.  I was just raising it as something that may lead to security issues in general.

>> It's a (sad) fact in life that many of us have to work in Windows-dominated IT environments.

True.  Although putting everything on network file servers which can farm out the same files using both cifs and nfs can simplify the clients.


________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 09/02/2013 03:41 PM, Gordon Lack wrote:
>>> What my patch does is to run the map script under the UID of the user requesting the mount, rather than root.
>>> That is actually an improvement of the security situation, AFAICS.
> 
> Possibly not.

Can you explain?

>>> Please check the "multiuser" option of mount.cifs ("With this option, the client ... creates a new session with
>>> the server using the user's credentials whenever a new user accesses the mount").
> 
> It also says:
> 
>            Furthermore, when unix extensions aren't in use and the
>            administrator has not overriden ownership using the uid= or gid=
>            options, ownership of files is presented as the current user
>            accessing the share.
> 
> Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.

Permission checks are done on the server.

On the client,  without unix extensions, the user/group IDs of files may
be displayed wrongly. That may confuse users because they may not be
able to open files listed as owned by themselves, but it's not a
security problem.

Martin

-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

>>>> What my patch does is to run the map script under the UID of the user requesting the mount, rather than root.
>>>> That is actually an improvement of the security situation, AFAICS.
>>
>> Possibly not.
>
> Can you explain?

It won't be an improvement if it enables something that is inherently insecure.

>> Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.
>
> Permission checks are done on the server.

Then why do you need to specify a specific UID at mount time?

> On the client,  without unix extensions, the user/group IDs of files may be displayed wrongly.
> That may confuse users because they may not be able to open files listed as owned by themselves, but
> it's not a security problem.

OK - so now I'm confused. you mentioned the multiuser option to mount.cifs as something that enables individual users to get
their own individual access permissions.  But you're also saying (above) that they won't.

It's this lack of specificity that worries me.

If we add this code
  it will enable this feature
  and remove this current anomaly/mis-feature
    which will provide this end-user enhancement.


________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 09/02/2013 04:20 PM, Gordon Lack wrote:
>>>>> What my patch does is to run the map script under the UID of the user requesting the mount, rather than root.
>>>>> That is actually an improvement of the security situation, AFAICS.
>>>
>>> Possibly not.
>>
>> Can you explain?
> 
> It won't be an improvement if it enables something that is inherently insecure.

I don't think that it's inherently insecure, at least not any more than
clicking on a network share in nautilus et al.

UID/GID mapping between client and server and permission control is a
general security challenge for Network file systems. That's not specific
to CIFS, NFS has similar problems. Yet aufofs has had the "-hosts" map
for a long time. My patch aims at something similar to '-hosts' for
CIFS, using kerberos credentials for access control. For both NFS and
CIFS, this kind of feature must not be enabled without proper security
considerations, but that shouldn't be used as an argument not to
implement the feature at all.

>>> Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.
>>
>> Permission checks are done on the server.
> 
> Then why do you need to specify a specific UID at mount time?

Depending on configuration, CIFS won't let you mount anything without
credentials. You may have to be authorized just to see the list of shares.

Once a user on a Windows client is logged in in AD (Kerberos), he will
usually see all relevant shares and be able to access them in various
ways. My goal is to provide a similarly easy access to users with AD
credentials on Linux clients, and one that works on the command line
too, not only from the GUI.

>> On the client,  without unix extensions, the user/group IDs of files may be displayed wrongly.
>> That may confuse users because they may not be able to open files listed as owned by themselves, but
>> it's not a security problem.
> 
> OK - so now I'm confused. you mentioned the multiuser option to mount.cifs as something that enables individual users to get
> their own individual access permissions.  But you're also saying (above) that they won't.

They will have individual access permissions. But without CIFS Unix
extensions, stat(2) will not return real Unix user and group IDs on
files or directories; instead every user will see his own user and group
ID. The real user and group IDs on the server are the Active Directory
Ids of the user owning the file.

Regards
Martin

-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

>> UID/GID mapping between client and server and permission control is a general security challenge for
>> Network file systems. That's not specific to CIFS, NFS has similar problems.

Global identities.  Don't set up Samba (etc.) to create local ones.

>> Yet aufofs has had the "-hosts" map for a long time.

Should _only_ ever be used on admin hosts for admin tasks as it hard-wires physical locations into pathnames (one of the things
which automount is able to remove).

>> Depending on configuration, CIFS won't let you mount anything without credentials. You may have to be authorized
>> just to see the list of shares.

So are you saying that the example you were giving, to use multiuser on a cifs mount, was so that the mount could
take place under the user's Kerberos credentials (which might work) rather than root's (who might not have them)?
But that once the mount was in place each user's access would be under their own credentials (assuming they have them)?

In which case I'd agree that this isn't a security issue.


________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Martin Wilck]

On 09/02/2013 05:08 PM, Gordon Lack wrote:

>>> Depending on configuration, CIFS won't let you mount anything without credentials. You may have to be authorized
>>> just to see the list of shares.
> 
> So are you saying that the example you were giving, to use multiuser on a cifs mount, was so that the mount could
> take place under the user's Kerberos credentials (which might work) rather than root's (who might not have them)?
> But that once the mount was in place each user's access would be under their own credentials (assuming they have them)?
> 
> In which case I'd agree that this isn't a security issue.

This is my understandning of how "multiuser" works, yes.

Martin

> 
> 
> ________________________________
> 
> This e-mail was sent by GlaxoSmithKline Services Unlimited
> (registered in England and Wales No. 1047315), which is a
> member of the GlaxoSmithKline group of companies. The
> registered address of GlaxoSmithKline Services Unlimited
> is 980 Great West Road, Brentford, Middlesex TW8 9GS.
> 


-- 
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			martin.wilck@ts.fujitsu.com
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint

RE: [PATCH 00/25] Current autofs patch queue

[Gordon Lack]

>> In which case I'd agree that this isn't a security issue.

> This is my understandning of how "multiuser" works, yes.

OK.

I forsee some testing being done on my little ARM/MIPS systems at home...just don't wait for results....



________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited
(registered in England and Wales No. 1047315), which is a
member of the GlaxoSmithKline group of companies. The
registered address of GlaxoSmithKline Services Unlimited
is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Re: [PATCH 00/25] Current autofs patch queue

[Ian Kent]

On Mon, 2013-09-02 at 12:34 +0200, Martin Wilck wrote:
> On 08/19/2013 03:11 AM, Ian Kent wrote:
> > There are patches from several contributors I plan on committing
> > so I thought posting them to the list for final review would be
> > sensible.
> 
> I am wondering about my auto.smb patch (see
> http://www.spinics.net/lists/autofs/msg00402.html). Back then, you said
> 
> > 
> > All I'm really saying is that, the program map returns a string that is
> > then used as the map entry, so when it is parsed the variables $UID,
> > $GID, etc. will be present in the macro lookup table as ${UID} (and $UID
> > etc. works) ... so maybe these same variables should be provided in the
> > environment of the program.
> > 
> > It isn't entirely clear if that is needed since those macro values may
> > be returned in the map entry text with things like echo "/some/string
> > \ ... ${UID} ..." etc. for later translation.
> > 
> > Really, a revised patch depends on this question being answered first. 
> 
> Nobody has spoken up on this list saying that additional environment
> variables would need to be set. I am wondering what else needs to be
> done in order to merge this (IMO very useful) functionality.

I like the idea of the setuid() but it's probably more convenient for me
to set the standard macro variables (that are set when the entry is
parsed) in the environment before running the program map. Also setuid()
changes what some may expect to be the case so that's probably not a
good idea.

One of the current patches in the queue sets existing macro variables in
the environment, in order to pass parameters to a program map, so all
that would need to be done is a call to a function that adds the
standard variables to the macro table for that to happen.

I posted a question about the second patch and will wait for the answer
to that.


> 
> Martin
>