ports/105684: [patch] net/nss_ldap pthread_atfork() bug

[Michael Bushkov]

>Number:         105684
>Category:       ports
>Synopsis:       [patch] net/nss_ldap pthread_atfork() bug
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-ports-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Mon Nov 20 11:00:31 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Michael Bushkov
>Release:        FreeBSD 6.1-RELEASE-p6 i386
>Organization:
Rostov State University
>Environment:
System: FreeBSD stinger.cc.rsu.ru 6.1-RELEASE-p6 FreeBSD 6.1-RELEASE-p6 #2: Mon Oct 2 12:14:54 MSD 2006 bushman at stinger.cc.rsu.ru:/usr/obj/usr/src/sys/STINGER_TUNED i386


>Description:
	
There is an issue with nss_ldap, which occurs when
specifying "bind_policy soft" in nss_ldap.conf.
Any program that uses nsswitch functions and fork()
syscall will work incorrectly with this option set
(sshd is good example - it won't let any user to
login into the system).

By default on FreeBSD nss_ldap is compiled with
pthread_atfork() function support. It uses
pthread_atfork() calls to detect whenever the process
has forked. When nss_ldap detects fork, it reopens
caonnection to the LDAP server. Unfortunately, this method
doesn't work on single-threaded applications -
nss_ldap doesn't reopen the connection and
both child and parent process read/write to the
same socket, which ends in malformed data read/written
to the LDAP server socket.

This error occurs only when "bind_policy soft"
is specified in "nss_ldap.conf" , because with this
policy nss_ldap doesn't try to rebind the connection
in case of error.

The simplest solution I see for this problem is to
turn off pthread_atfork detection in the configure
script - in this case nss_ldap will just compare
pids do detect if fork has happenned - this method seems
to work perfectly in both single- and multithreaded
applications, that use nss_ldap.

Dmitriy Kirhlarov <dkirhlarov at oilspace.com>
(who actually found the bug) has been using the patched version
of nss_ldap port for more than a month without
any patch-related issues.

The patch is included into the PR.
It adds one new file to the port:
files/patch-configure.in

How-To-Repeat:
Compile the following code and pass the name of any
group stored in ldap nsswitch source as the program's
argument. The output shouldn't contain any "failure" lines,
but it would, when "bind_policy soft" is specified in
nss_ldap.conf.

#include <grp.h>
#include <stdio.h>

int
main(int argc, char **argv)
{
        struct group *grp;
        int i;
        if (argc < 2)
                exit (-1);

        grp = getgrnam(argv[1]);
        printf("parent initial %s\n", grp != NULL ? grp->gr_name :
                "failure");
        switch (fork()) {
        case 0:
                grp = getgrnam(argv[1]);
                printf("child %s\n", grp != NULL ? grp->gr_name :
                        "failure");
                break;
        case -1:
                exit(-1);
                break;
        default:
                grp = getgrnam(argv[1]);
                printf("parent %s\n", grp != NULL ? grp->gr_name :
                        "failure");
                break;
        }

        return (0);
}

>How-To-Repeat:
>Fix:
--- nss_ldap_port_patch begins here ---
diff -urN nss_ldap.orig/Makefile nss_ldap/Makefile
--- nss_ldap.orig/Makefile	Mon Oct  9 18:18:50 2006
+++ nss_ldap/Makefile	Mon Oct  9 18:18:37 2006
@@ -7,6 +7,7 @@
 
 PORTNAME=	nss_ldap
 PORTVERSION=	1.${NSS_LDAP_VERSION}
+PORTREVISION=	1
 CATEGORIES=	net
 MASTER_SITES=	http://www.padl.com/download/ \
 		${MASTER_SITE_LOCAL}
diff -urN nss_ldap.orig/files/patch-configure nss_ldap/files/patch-configure
--- nss_ldap.orig/files/patch-configure	Mon Oct  9 18:18:50 2006
+++ nss_ldap/files/patch-configure	Mon Oct  9 18:16:31 2006
@@ -1,5 +1,5 @@
---- configure.orig	Sat May 27 17:06:27 2006
-+++ configure	Sat May 27 17:10:45 2006
+--- configure.orig	Thu Jun 22 06:39:26 2006
++++ configure	Mon Oct  9 18:15:29 2006
 @@ -1729,46 +1729,46 @@
  fi
  done
@@ -87,3 +87,65 @@
       for ac_hdr in irs.h
  do
  ac_safe=`echo "$ac_hdr" | sed 'y%./+-%__p_%'`
+@@ -3318,61 +3318,6 @@
+ ; return 0; }
+ EOF
+ if { (eval echo configure:3321: \"$ac_link\") 1>&5; (eval $ac_link) 2>&5; } && test -s conftest${ac_exeext}; then
+-  rm -rf conftest*
+-  eval "ac_cv_func_$ac_func=yes"
+-else
+-  echo "configure: failed program was:" >&5
+-  cat conftest.$ac_ext >&5
+-  rm -rf conftest*
+-  eval "ac_cv_func_$ac_func=no"
+-fi
+-rm -f conftest*
+-fi
+-
+-if eval "test \"`echo '$ac_cv_func_'$ac_func`\" = yes"; then
+-  echo "$ac_t""yes" 1>&6
+-    ac_tr_func=HAVE_`echo $ac_func | tr 'abcdefghijklmnopqrstuvwxyz' 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'`
+-  cat >> confdefs.h <<EOF
+-#define $ac_tr_func 1
+-EOF
+- 
+-else
+-  echo "$ac_t""no" 1>&6
+-fi
+-done
+-
+-for ac_func in pthread_atfork
+-do
+-echo $ac_n "checking for $ac_func""... $ac_c" 1>&6
+-echo "configure:3348: checking for $ac_func" >&5
+-if eval "test \"`echo '$''{'ac_cv_func_$ac_func'+set}'`\" = set"; then
+-  echo $ac_n "(cached) $ac_c" 1>&6
+-else
+-  cat > conftest.$ac_ext <<EOF
+-#line 3353 "configure"
+-#include "confdefs.h"
+-/* System header to define __stub macros and hopefully few prototypes,
+-    which can conflict with char $ac_func(); below.  */
+-#include <assert.h>
+-/* Override any gcc2 internal prototype to avoid an error.  */
+-/* We use char because int might match the return type of a gcc2
+-    builtin and then its argument prototype would still apply.  */
+-char $ac_func();
+-
+-int main() {
+-
+-/* 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_$ac_func) || defined (__stub___$ac_func)
+-choke me
+-#else
+-$ac_func();
+-#endif
+-
+-; return 0; }
+-EOF
+-if { (eval echo configure:3376: \"$ac_link\") 1>&5; (eval $ac_link) 2>&5; } && test -s conftest${ac_exeext}; then
+   rm -rf conftest*
+   eval "ac_cv_func_$ac_func=yes"
+ else
diff -urN nss_ldap.orig/files/patch-configure.in nss_ldap/files/patch-configure.in
--- nss_ldap.orig/files/patch-configure.in	Thu Jan  1 03:00:00 1970
+++ nss_ldap/files/patch-configure.in	Mon Oct  9 17:58:29 2006
@@ -0,0 +1,10 @@
+--- configure.in.orig	Mon Oct  9 17:56:31 2006
++++ configure.in	Mon Oct  9 17:56:39 2006
+@@ -194,7 +194,6 @@
+ AC_CHECK_FUNCS(snprintf)
+ AC_CHECK_FUNCS(gethostbyname)
+ AC_CHECK_FUNCS(nsdispatch)
+-AC_CHECK_FUNCS(pthread_atfork)
+ AC_CHECK_FUNCS(ether_aton)
+ AC_CHECK_FUNCS(ether_ntoa)
+ 
--- nss_ldap_port_patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted: