On Sat, 2015-08-29 at 15:26 +0200, Julien Cristau wrote: > Sorry for the delay in getting back to you. > > Please feel free to upload 0.9.4-3+deb8u1. Thanks. In the meantime, two more bugs arose in nss-pam-ldapd that I would like to fix in jessie. The first (#794686) is an RC bug with a one-line fix. The problem was that the exit code of the init script was wrong in some cases. I would also like to get #794068 fixed. This fixes password policy expiration. Instead of forcing the user to change their password, a warning message should be presented to the user instead. I have just uploaded the fix to unstable. Attached is a debdiff with the changes. Thanks, -- -- arthur - adejong@debian.org - http://people.debian.org/~adejong --
diff -Nru nss-pam-ldapd-0.9.4/debian/changelog nss-pam-ldapd-0.9.4/debian/changelog
--- nss-pam-ldapd-0.9.4/debian/changelog 2014-09-28 15:08:58.000000000 +0200
+++ nss-pam-ldapd-0.9.4/debian/changelog 2015-08-30 13:58:54.000000000 +0200
@@ -1,3 +1,14 @@
+nss-pam-ldapd (0.9.4-3+deb8u1) stable; urgency=low
+
+ * fix-issues-withdaemonising.patch, avoid-signal-race.patch: patches to
+ fix issues with daemonising nslcd and avoid a race condition in signal
+ handling during start-up (closes: #759544)
+ * ensure proper return code of init script (closes: #794686)
+ * fix-ppolicy-expiration-warnings.patch: fix password policy expiration
+ warnings (closes: #794068)
+
+ -- Arthur de Jong <adejong@debian.org> Sun, 30 Aug 2015 13:00:00 +0200
+
nss-pam-ldapd (0.9.4-3) unstable; urgency=low
* use-ip-range-for-tests.patch: use a different IP range for running the
diff -Nru nss-pam-ldapd-0.9.4/debian/nslcd.init nss-pam-ldapd-0.9.4/debian/nslcd.init
--- nss-pam-ldapd-0.9.4/debian/nslcd.init 2014-06-08 00:33:57.000000000 +0200
+++ nss-pam-ldapd-0.9.4/debian/nslcd.init 2015-08-30 13:53:17.000000000 +0200
@@ -174,5 +174,3 @@
exit 1
;;
esac
-
-exit 0
diff -Nru nss-pam-ldapd-0.9.4/debian/patches/avoid-signal-race.patch nss-pam-ldapd-0.9.4/debian/patches/avoid-signal-race.patch
--- nss-pam-ldapd-0.9.4/debian/patches/avoid-signal-race.patch 1970-01-01 01:00:00.000000000 +0100
+++ nss-pam-ldapd-0.9.4/debian/patches/avoid-signal-race.patch 2015-08-30 11:19:41.000000000 +0200
@@ -0,0 +1,71 @@
+From: Arthur de Jong <arthur@arthurdejong.org>
+Subject: Avoid signal race condition on start-up
+
+This only restores the signal mask after signal handlers are in place
+and the daemon has completely daemonised to avoid a race condition in
+the start-up phase of nslcd where a signal could be sent to nslcd
+causing it to quit or fail to write information to the parent process.
+
+This also block signals sooner in an attempt to avoid race conditions.
+
+Origin: upstream, http://arthurdejong.org/git/nss-pam-ldapd/commit/?id=1d3b19b1ecd3b10f36e8925e8a752a28e3e74b56
+Origin: upstream, http://arthurdejong.org/git/nss-pam-ldapd/commit/?id=530cc24c83dd5d2d347acb40d64c3ae06a43a293
+Bug-Debian: http://bugs.debian.org/759544
+
+--- a/nslcd/nslcd.c
++++ b/nslcd/nslcd.c
+@@ -648,6 +648,17 @@ int main(int argc, char *argv[])
+ #ifdef HAVE_PTHREAD_TIMEDJOIN_NP
+ struct timespec ts;
+ #endif /* HAVE_PTHREAD_TIMEDJOIN_NP */
++ /* block all these signals so our worker threads won't handle them */
++ sigemptyset(&signalmask);
++ sigaddset(&signalmask, SIGHUP);
++ sigaddset(&signalmask, SIGINT);
++ sigaddset(&signalmask, SIGQUIT);
++ sigaddset(&signalmask, SIGABRT);
++ sigaddset(&signalmask, SIGPIPE);
++ sigaddset(&signalmask, SIGTERM);
++ sigaddset(&signalmask, SIGUSR1);
++ sigaddset(&signalmask, SIGUSR2);
++ pthread_sigmask(SIG_BLOCK, &signalmask, &oldmask);
+ /* close all file descriptors (except stdin/out/err) */
+ daemonize_closefds();
+ /* parse the command line */
+@@ -785,17 +796,6 @@ int main(int argc, char *argv[])
+ }
+ log_log(LOG_DEBUG, "setuid(%d) done", (int)nslcd_cfg->uid);
+ }
+- /* block all these signals so our worker threads won't handle them */
+- sigemptyset(&signalmask);
+- sigaddset(&signalmask, SIGHUP);
+- sigaddset(&signalmask, SIGINT);
+- sigaddset(&signalmask, SIGQUIT);
+- sigaddset(&signalmask, SIGABRT);
+- sigaddset(&signalmask, SIGPIPE);
+- sigaddset(&signalmask, SIGTERM);
+- sigaddset(&signalmask, SIGUSR1);
+- sigaddset(&signalmask, SIGUSR2);
+- pthread_sigmask(SIG_BLOCK, &signalmask, &oldmask);
+ /* start worker threads */
+ log_log(LOG_INFO, "accepting connections");
+ nslcd_threads = (pthread_t *)malloc(nslcd_cfg->threads * sizeof(pthread_t));
+@@ -815,8 +815,7 @@ int main(int argc, char *argv[])
+ exit(EXIT_FAILURE);
+ }
+ }
+- pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+- /* install signalhandlers for some signals */
++ /* install signal handlers for some signals */
+ install_sighandler(SIGHUP, sig_handler);
+ install_sighandler(SIGINT, sig_handler);
+ install_sighandler(SIGQUIT, sig_handler);
+@@ -827,6 +826,8 @@ int main(int argc, char *argv[])
+ install_sighandler(SIGUSR2, SIG_IGN);
+ /* signal the starting process to exit because we can provide services now */
+ daemonize_ready(EXIT_SUCCESS, NULL);
++ /* enable receiving of signals */
++ pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+ /* wait until we received a signal */
+ while ((nslcd_receivedsignal == 0) || (nslcd_receivedsignal == SIGUSR1))
+ {
diff -Nru nss-pam-ldapd-0.9.4/debian/patches/fix-issues-withdaemonising.patch nss-pam-ldapd-0.9.4/debian/patches/fix-issues-withdaemonising.patch
--- nss-pam-ldapd-0.9.4/debian/patches/fix-issues-withdaemonising.patch 1970-01-01 01:00:00.000000000 +0100
+++ nss-pam-ldapd-0.9.4/debian/patches/fix-issues-withdaemonising.patch 2015-08-30 11:19:41.000000000 +0200
@@ -0,0 +1,93 @@
+From: Arthur de Jong <arthur@arthurdejong.org>
+Subject: Fix issues with daemonising
+
+This fixes a problem with a buffer that could end up padded with
+garbage.
+
+This also clarifies the code a bit and adds extra logging for errors
+that could occur during daemonising.
+
+Origin: upstream, http://arthurdejong.org/git/nss-pam-ldapd/commit/?id=a726d291b0f6794abec0a0192cf2b2a742648e4a
+
+--- a/nslcd/daemonize.c
++++ b/nslcd/daemonize.c
+@@ -57,13 +57,13 @@ void daemonize_closefds(void)
+ void daemonize_redirect_stdio(void)
+ {
+ /* close stdin, stdout and stderr */
+- close(0); /* stdin */
+- close(1); /* stdout */
+- close(2); /* stderr */
++ (void)close(0); /* stdin */
++ (void)close(1); /* stdout */
++ (void)close(2); /* stderr */
+ /* reconnect to /dev/null */
+- open("/dev/null", O_RDWR); /* stdin, fd=0 */
+- dup(0); /* stdout, fd=1 */
+- dup(0); /* stderr, fd=2 */
++ (void)open("/dev/null", O_RDWR); /* stdin, fd=0 */
++ (void)dup(0); /* stdout, fd=1 */
++ (void)dup(0); /* stderr, fd=2 */
+ }
+
+ /* try to fill the buffer until EOF or error */
+@@ -95,18 +95,27 @@ static int wait_for_response(int fd)
+ int i, l, rc;
+ char buffer[1024];
+ /* read return code */
++ errno = 0;
+ i = read_response(fd, (void *)&rc, sizeof(int));
++ log_log(LOG_DEBUG, "DEBUG: wait_for_response(): i=%d, rc=%d", i, rc);
+ if (i != sizeof(int))
++ {
++ log_log(LOG_ERR, "wait_for_response(): read_response() returned %d (expected %d)",
++ i, (int)sizeof(int));
++ if (errno == 0)
++ errno = ENODATA;
+ return -1;
++ }
+ /* read string length */
+ i = read_response(fd, (void *)&l, sizeof(int));
++ log_log(LOG_DEBUG, "DEBUG: wait_for_response(): i=%d, l=%d", i, l);
+ if ((i != sizeof(int)) || (l <= 0))
+ _exit(rc);
+ /* read string */
+ if ((size_t)l > (sizeof(buffer) - 1))
+ l = sizeof(buffer) - 1;
+ i = read_response(fd, buffer, l);
+- buffer[sizeof(buffer) - 1] = '\0';
++ buffer[l] = '\0';
+ if (i == l)
+ fprintf(stderr, "%s", buffer);
+ _exit(rc);
+@@ -200,22 +209,23 @@ int daemonize_daemon(void)
+
+ void daemonize_ready(int status, const char *message)
+ {
++ int l;
+ if (daemonizefd >= 0)
+ {
+ /* we ignore any errors writing */
+- write(daemonizefd, &status, sizeof(int));
++ (void)write(daemonizefd, &status, sizeof(int));
+ if ((message == NULL) || (message[0] == '\0'))
+ {
+- status = 0;
+- write(daemonizefd, &status, sizeof(int));
++ l = 0;
++ (void)write(daemonizefd, &l, sizeof(int));
+ }
+ else
+ {
+- status = strlen(message);
+- write(daemonizefd, &status, sizeof(int));
+- write(daemonizefd, message, status);
++ l = strlen(message);
++ (void)write(daemonizefd, &l, sizeof(int));
++ (void)write(daemonizefd, message, l);
+ }
+- close(daemonizefd);
++ (void)close(daemonizefd);
+ daemonizefd = -1;
+ }
+ }
diff -Nru nss-pam-ldapd-0.9.4/debian/patches/fix-ppolicy-expiration-warnings.patch nss-pam-ldapd-0.9.4/debian/patches/fix-ppolicy-expiration-warnings.patch
--- nss-pam-ldapd-0.9.4/debian/patches/fix-ppolicy-expiration-warnings.patch 1970-01-01 01:00:00.000000000 +0100
+++ nss-pam-ldapd-0.9.4/debian/patches/fix-ppolicy-expiration-warnings.patch 2015-08-30 13:54:18.000000000 +0200
@@ -0,0 +1,145 @@
+Description: Fix password policy warnings
+ Previously, when a password was about to expire, nslcd would incorrectly
+ signal the PAM module to force a password change. This changes this to
+ only present a message to the user.
+ This also ensures that the PAM module logs messages that are presented
+ to the user.
+Author: Arthur de Jong <arthur@arthurdejong.org>
+Origin: upstream, http://arthurdejong.org/git/nss-pam-ldapd/commit/?id=4302901a2708d55b24880b77437e3d782b0de1cb
+Origin: upstream, http://arthurdejong.org/git/nss-pam-ldapd/commit/?id=263a44340badb1e553c997f2dfb4986fb2f4c28b
+Origin: upstream, http://arthurdejong.org/git/nss-pam-ldapd/commit/?id=309f127416cd38f972d28b29f59e784ea5403785
+Bug-Debian: https://bugs.debian.org/794068
+
+--- a/nslcd/myldap.c
++++ b/nslcd/myldap.c
+@@ -5,7 +5,7 @@
+
+ Copyright (C) 1997-2006 Luke Howard
+ Copyright (C) 2006-2007 West Consulting
+- Copyright (C) 2006-2014 Arthur de Jong
++ Copyright (C) 2006-2015 Arthur de Jong
+
+ This library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+@@ -406,7 +406,7 @@ static int do_sasl_interact(LDAP UNUSED(
+ }
+
+ #if defined(HAVE_LDAP_SASL_BIND) && defined(LDAP_SASL_SIMPLE)
+-static void handle_ppasswd_controls(MYLDAP_SESSION *session, LDAP *ld, LDAPControl **ctrls)
++static void handle_ppolicy_controls(MYLDAP_SESSION *session, LDAP *ld, LDAPControl **ctrls)
+ {
+ int i;
+ int rc;
+@@ -434,10 +434,9 @@ static void handle_ppasswd_controls(MYLD
+ sec = atol(seconds);
+ log_log(LOG_DEBUG, "got LDAP_CONTROL_PWEXPIRING (password will expire in %ld seconds)",
+ sec);
+- /* return this warning to PAM */
+- if (session->policy_response == NSLCD_PAM_SUCCESS)
++ /* return this warning so PAM can present it to the user */
++ if (strlen(session->policy_message) == 0)
+ {
+- session->policy_response = NSLCD_PAM_NEW_AUTHTOK_REQD;
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "password will expire in %ld seconds", sec);
+ }
+@@ -453,9 +452,8 @@ static void handle_ppasswd_controls(MYLD
+ else
+ {
+ /* log returned control information */
+- if (error != PP_noError)
+- log_log(LOG_DEBUG, "got LDAP_CONTROL_PASSWORDPOLICYRESPONSE (%s)",
+- ldap_passwordpolicy_err2txt(error));
++ log_log(LOG_DEBUG, "got LDAP_CONTROL_PASSWORDPOLICYRESPONSE (%s)",
++ ldap_passwordpolicy_err2txt(error));
+ if (expire >= 0)
+ log_log(LOG_DEBUG, "got LDAP_CONTROL_PASSWORDPOLICYRESPONSE (password will expire in %d seconds)",
+ expire);
+@@ -467,7 +465,8 @@ static void handle_ppasswd_controls(MYLD
+ ((session->policy_response == NSLCD_PAM_SUCCESS) ||
+ (session->policy_response == NSLCD_PAM_NEW_AUTHTOK_REQD)))
+ {
+- session->policy_response = NSLCD_PAM_AUTHTOK_EXPIRED;
++ /* this means that the password has expired and must be reset */
++ session->policy_response = NSLCD_PAM_NEW_AUTHTOK_REQD;
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "%s", ldap_passwordpolicy_err2txt(error));
+ }
+@@ -475,6 +474,8 @@ static void handle_ppasswd_controls(MYLD
+ ((session->policy_response == NSLCD_PAM_SUCCESS) ||
+ (session->policy_response == NSLCD_PAM_NEW_AUTHTOK_REQD)))
+ {
++ /* this means that the account is locked and the user cannot log
++ in (the bind probably failed already) */
+ session->policy_response = NSLCD_PAM_ACCT_EXPIRED;
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "%s", ldap_passwordpolicy_err2txt(error));
+@@ -482,6 +483,8 @@ static void handle_ppasswd_controls(MYLD
+ else if ((error == PP_changeAfterReset) &&
+ (session->policy_response == NSLCD_PAM_SUCCESS))
+ {
++ /* this indicates that the password must be changed before the
++ user is allowed to perform any other operation */
+ session->policy_response = NSLCD_PAM_NEW_AUTHTOK_REQD;
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "%s", ldap_passwordpolicy_err2txt(error));
+@@ -490,22 +493,23 @@ static void handle_ppasswd_controls(MYLD
+ ((session->policy_response == NSLCD_PAM_SUCCESS) ||
+ (session->policy_response == NSLCD_PAM_NEW_AUTHTOK_REQD)))
+ {
++ /* any other error is assumed to mean that the operation failed */
+ session->policy_response = NSLCD_PAM_PERM_DENIED;
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "%s", ldap_passwordpolicy_err2txt(error));
+ }
+- else if ((expire >= 0) &&
+- ((session->policy_response == NSLCD_PAM_SUCCESS) ||
+- (session->policy_response == NSLCD_PAM_NEW_AUTHTOK_REQD)))
++ /* both expire and grace should just be warnings to the user */
++ if ((expire >= 0) && (strlen(session->policy_message) == 0))
+ {
+- session->policy_response = NSLCD_PAM_NEW_AUTHTOK_REQD;
++ /* if no other error has happened, this indicates that the password
++ will soon expire (number of seconds) */
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "Password will expire in %d seconds", expire);
+ }
+- else if ((grace >= 0) &&
+- (session->policy_response == NSLCD_PAM_SUCCESS))
++ else if ((grace >= 0) && (strlen(session->policy_message) == 0))
+ {
+- session->policy_response = NSLCD_PAM_NEW_AUTHTOK_REQD;
++ /* this indicates the number of grace logins that are left before
++ no further login attempts will be allowed */
+ mysnprintf(session->policy_message, sizeof(session->policy_message),
+ "Password expired, %d grace logins left", grace);
+ }
+@@ -580,7 +584,7 @@ static int do_ppolicy_bind(MYLDAP_SESSIO
+ /* handle any returned controls */
+ if (responsectrls != NULL)
+ {
+- handle_ppasswd_controls(session, ld, responsectrls);
++ handle_ppolicy_controls(session, ld, responsectrls);
+ ldap_controls_free(responsectrls);
+ }
+ /* return the result of the BIND operation */
+--- a/pam/pam.c
++++ b/pam/pam.c
+@@ -581,9 +581,17 @@ int pam_sm_acct_mgmt(pam_handle_t *pamh,
+ pam_syslog(pamh, LOG_DEBUG, "authorization succeeded");
+ /* present any informational messages to the user */
+ if ((authz_resp.msg[0] != '\0') && (!cfg.no_warn))
++ {
+ pam_info(pamh, "%s", authz_resp.msg);
++ pam_syslog(pamh, LOG_INFO, "%s; user=%s",
++ authz_resp.msg, username);
++ }
+ if ((ctx->saved_authz.msg[0] != '\0') && (!cfg.no_warn))
++ {
+ pam_info(pamh, "%s", ctx->saved_authz.msg);
++ pam_syslog(pamh, LOG_INFO, "%s; user=%s",
++ ctx->saved_authz.msg, username);
++ }
+ return PAM_SUCCESS;
+ }
+
diff -Nru nss-pam-ldapd-0.9.4/debian/patches/series nss-pam-ldapd-0.9.4/debian/patches/series
--- nss-pam-ldapd-0.9.4/debian/patches/series 2014-08-20 22:18:40.000000000 +0200
+++ nss-pam-ldapd-0.9.4/debian/patches/series 2015-08-30 13:54:57.000000000 +0200
@@ -1 +1,4 @@
use-ip-range-for-tests.patch
+fix-issues-withdaemonising.patch
+avoid-signal-race.patch
+fix-ppolicy-expiration-warnings.patch
Attachment:
signature.asc
Description: This is a digitally signed message part