Winbind patch fixes broken client. Please have a look.

[Andrew Bartlett]

Hannes Schmidt wrote:
> 
> Even if this is complete crap, I would appreciate any feedback saying so.

Firstly, don't expect instant responses - particularly over the
weekend...

> A couple of days ago I posted a problem report to the samba list with some
> diagnostics. I seem to have found a
> solution on my own and provide a patch here. The problem is related to the
> fact that the winbind client code in
> libnss_winbind.so leaves the handle to the winbind pipe open, for obvious
> reasons. This is perfectly ok, but some
> programs get confused if this handle is 0,1 or 2, which they expect to be a
> handle to a different file. The patch
> makes the winbind client (libnss_winbind.so) code more robust.
> 
> Consider the following fragment of broken application code.
> 
> if( fork() == 0 ) {
>     /* child */
>     close( 0 );             /* next open() will return 0 (stdin) */
>     getpwent(...);        /* pid changed, so wb_common.c will re-open the
> pipe to winbindd, the handle will be 0 */
>     fd = open( ... );     /* returned fd is *not* 0 although the code below
> assumes this */
>     execve( ... );         /* executed program reads from stdin which -
> unintentionally - is the winbind pipe */
> } else {
>     /* parent */
>     ...
> }
> 
> Obviously, this code is broken and could be easily fixed by doing the
> getpwent() before the close(). Unfortunately
> the version of cron installed on my system is broken in a more complex but
> similar way. There is probably more
> broken code like this in other standard unix software, especially since it's
> such a subtle mistake.

It is rather - well spotted for figuring it out.

How do other nss libraries deal with this?  NIS and LDAP both open
sockets - find out if there is a standard solution to the problem. 
Personally, I don't like the idea of messing with a program's file
descriptors - but this is a problem we will have to cope with.

> The provided patch to wb_common.c makes sure that the handle to the winbindd
> pipe is not 0, 1 or 2.

For future reference, patches should be in 'diff -u' format, if
possible.

> *** /root/samba-2.2.5/source/nsswitch/wb_common.c Wed Jun 19 03:13:44 2002
> --- wb_common.c Thu Jul 25 16:36:39 2002
> ***************
> *** 94,99 ****
> --- 94,120 ----
>    }
>   }
> 
> + /* Returns a duplicate of the given file descriptor that is guranteed
> +    not to be 0, 1 or 2 (stdin, stdout or stderr respectively), unless
> +    an error occurs. If the given fd is invalid, it will be returned
> +    unchanged. If a different error occurs, the returned file handle
> +    will still be valid but it may be 0, 1 or 2. If a valid file
> +    handle is returned and the returned handle is different to the
> +    original one, the original one will be closed. If dup() is broken,
> +    this function might never return. */

I don't like that idea.  It should fail - infinite loops are bad...

> + int make_nonstd_fd(int fd)
> + {
> +  int saved_fd;
> +  if (0 >= fd && fd <= 2) {
> +      fd = dup(saved_fd = fd);
> +      if (fd == -1) return saved_fd;
> +      fd = make_nonstd_fd(fd);
> +      close(saved_fd);
> +  }
> +  return fd;
> + }
> +
>   /* Connect to winbindd socket */
> 
>   int winbind_open_pipe_sock(void)
> ***************
> *** 158,164 ****
>    if ((winbindd_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
>     return -1;
>    }
> !
>    if (connect(winbindd_fd, (struct sockaddr *)&sunaddr,
>         sizeof(sunaddr)) == -1) {
>     close_sock();
> --- 179,188 ----
>    if ((winbindd_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
>     return -1;
>    }
> !
> !  winbindd_fd = make_nonstd_fd( winbindd_fd );
> !
> !
>    if (connect(winbindd_fd, (struct sockaddr *)&sunaddr,
>         sizeof(sunaddr)) == -1) {
>     close_sock();
> ***************
> *** 167,172 ****
> --- 191,197 ----
> 
>    return winbindd_fd;
>   }
> +
> 
>   /* Write data to winbindd socket */

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net