Re: (ITS#7575) Fixed send_cli_cred on platforms that do not support such functions

[tedcheng@symas.com]

On Apr 13, 2013, at 12:24 AM, Hallvard Breien Furuseth wrote:

> Looking again at the st_mode compare: I'm pretty sure the old
> code worked somewhere, at some time.  Maybe somewhere it still
> does, and expecting a different fstat():st_mode will break that.
> Unless something changed in OpenLDAP itself?
> 
> Anyway, after re-reading ITS#4893 I'd be very wary about tweaking
> this code.  Looks like at least I just gave up at the end, but
> followup#9 suggests using Solaris "doors".  IIRC I did look at at
> the supposedly more general STREAMS pipes and found myself wading
> into portability issues or kernel tuning params or whateveer.
> 
> -- 
> Hallvard


There are several issues on Solaris 8:

1. getsockname(s, (struct sockaddr *)&lname, &llen)

The llen parameter does not return the actual size of the socket address,
but the original buffer size. As a result, the comparison
!memcmp(&lname, &rname, llen) compares the whole buffer.
Since rname is not initialized to 0 for the entire buffer, the garbage
characters following the socket path name always fail the comparison.

+               if( err == 0 && st.st_mode == mode && llen == rlen
+                        && !memcmp(&lname, &rname, llen))

2. The checking "llen == rlen" always fails as well.

3. The fchmod( fds[0], S_ISUID|S_IRWXU) on the client side does not change
the mode to include S_ISUID|S_IRWXU. The mode stays as only
S_IFIFO:

(gdb) next
81              if (rc = fchmod( fds[0], S_ISUID|S_IRWXU ) < 0) {
(gdb) print st
$2 = {...st_mode = 4096, ...} 
(gdb) next
85              (void) fstat(fds[0], &st);
(gdb) next
86              write( fds[1], sa, salen );
(gdb) print st
$3 = {...st_mode = 4096, ...}

Trying to compare (st.st_mode ==  S_IFIFO|S_ISUID|S_IRWXU) 
always fails.

I have revised the patch to (1) initialize rname buffer, (2) ignore 
the comparison "llen == rlen", (3) compare st.st_mode only against
S_IFIFO, but not S_ISUID|S_IRWXU.

The socket path comparison demands full-path match, e.g., "/var/suum/run/socket"
won't match against "/var/suum/run//socket".

The revised patch is at:
https://dl.dropboxusercontent.com/u/94235048/ITS7575.patch

Ted C. Cheng
Symas Corporation