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

[hyc@symas.com]

tedcheng@symas.com wrote:
> 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.

As Hallvard points out, all of this code was known to work at one time. 
Apparently a subsequent Solaris patchlevel has broken it. I don't see a 
portable way to conditionalize this behavior now.

> 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".

Nor should it. In this case whoever is using "//" in the path is 
misconfigured, as I already said before in private email.
>
> The revised patch is at:
> https://dl.dropboxusercontent.com/u/94235048/ITS7575.patch
>
> Ted C. Cheng
> Symas Corporation


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/