Re: (ITS#7575) Fixed send_cli_cred on platforms that do not support such functions
by h.b.furuseth＠usit.uio.no
Ted C. Cheng writes:
> I have revised the patch to (1) initialize rname buffer, (2) ignore
> the comparison "llen == rlen",
Might be OK, but...
> (3) compare st.st_mode only against S_IFIFO, but not S_ISUID|S_IRWXU.
No. You're getting proper use to work on Solaris, but that's the easy
part. That fchmod and corresponding mode test is there for a reason:
The code must also reject attempts to impersonate other users or groups.
A user could find and open a named socket/pipe owned by some other user,
or start someone's setuid program which opens a Unix domain socket and
then runs his program as his own user, or something like that. I don't
remember exactly. But OSes and OS releases differ about which of these
impersonation attempts might work if this code were insufficiently
paranoid, so you have quite some exporation work ahead of you if you
want to tweak this FD passing safely.
You can instead look for a mechanism with built-in credential passing,
apparently like Solaris "doors". Or look at what some other well-tested
and portable package does and suggest we steal its code. Or live with
the fact that SASL/EXTERNAL over ldapi:// is supported on your platform.
> The socket path comparison demands full-path match, e.g.,
> "/var/suum/run/socket" won't match against "/var/suum/run//socket".
--
Hallvard
10 years, 7 months
Re: (ITS#7575) Fixed send_cli_cred on platforms that do not support such functions
by hyc＠symas.com
tedcheng(a)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/
10 years, 7 months
Re: (ITS#7575) Fixed send_cli_cred on platforms that do not support such functions
by 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
10 years, 7 months
Re: (ITS#7575) Fixed send_cli_cred on platforms that do not support such functions
by h.b.furuseth＠usit.uio.no
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
10 years, 7 months
Re: (ITS#7575) Fixed send_cli_cred on platforms that do not support such functions
by h.b.furuseth＠usit.uio.no
tedcheng(a)symas.com writes:
> URL: https://dl.dropboxusercontent.com/u/94235048/fix_send_cli_cred.patch
>
> Importing fix from Symas #2230:
>
> On platforms, such as Solaris 8, that do not support functions for getting
> client credential, client sends a message and fd for server to derive euid/egid.
> The server-side logic for deriving client credential was buggy.
>
> This patch introduces sockaddr_un_cmp() to compare the socket path names,
> ignoring redundant "/" in the path, and only checks S_IFIFO mode. This patch has
> been tested on Solaris 8 and regression-tested fine on Solaris 10.
This patch is wrong:
- You cannot trust the path which the client wrote to the socket *by
hand* (label sendcred: in libldap/os-local.c:ldap_pvt_connect()), if
someone else than the owner and root has write access to the socket.
Could comment that in getpeereid.c, to prevent such a future patch.
- An initial "//" is significant on some systems and must not match "/".
Posix does not guarantee that sun_path[] is \0-terminated.
What problem is the complicated compare solving? Why not require that
the user spells the path like the server does, similar to how TLS
hostnames must be spelled the same way? You are not catching all
possible spellings of the path anyway: YOu do not reduce "/./" to "/"
and must not reduce "foo/../ to "" since foo can be a symlink.
--
Hallvard
10 years, 7 months
(ITS#7575) Fixed send_cli_cred on platforms that do not support such functions
by tedcheng＠symas.com
Full_Name: Ted C. Cheng
Version: HEAD
OS: Solaris 8
URL: https://dl.dropboxusercontent.com/u/94235048/fix_send_cli_cred.patch
Submission from: (NULL) (76.174.250.179)
Importing fix from Symas #2230:
On platforms, such as Solaris 8, that do not support functions for getting
client credential, client sends a message and fd for server to derive euid/egid.
The server-side logic for deriving client credential was buggy.
This patch introduces sockaddr_un_cmp() to compare the socket path names,
ignoring redundant "/" in the path, and only checks S_IFIFO mode. This patch has
been tested on Solaris 8 and regression-tested fine on Solaris 10.
10 years, 7 months
Re: (ITS#7574) slapo-unique not enforcing uniqueness
by quanah＠zimbra.com
--On Friday, April 12, 2013 5:48 PM +0000 quanah(a)zimbra.com wrote:
> --On Friday, April 12, 2013 5:43 PM +0000 quanah(a)OpenLDAP.org wrote:
>
>> Full_Name: Quanah Gibson-Mount
>> Version: RE24 4/12/2013
>> OS: Linux 2.6
>> URL: ftp://ftp.openldap.org/incoming/
>> Submission from: (NULL) (75.111.39.181)
>
> Also the following modify succeeds when it should fail:
>
> zimbra@zre-ldap002:~$ ldapsearch -LLL -x -H ldapi:/// -D cn=config -w
> zimbra dc=test zimbraID
> dn: dc=test,dc=com
> zimbraId: 03ccfb88-cbd8-47e5-94e7-0fb1fa60cacc
>
> zimbra@zre-ldap002:~$ ldapsearch -LLL -x -H ldapi:/// -D cn=config -w
> zimbra dc=example zimbraId
> dn: dc=example,dc=com
> zimbraId: 0c18e59d-fdd7-40b2-abff-134b118e6cf9
>
> zimbra@zre-ldap002:~$ ldapmodify -x -H ldapi:/// -D cn=config -w zimbra
> dn: dc=example,dc=com
> changetype: modify
> replace: zimbraId
> zimbraId: 03ccfb88-cbd8-47e5-94e7-0fb1fa60cacc
>
> modifying entry "dc=example,dc=com"
I found it has entirely to do with brand new entries since slapd was last
started. unique simply won't enforce on any data added after slapd was
started. Restarting slapd results in the correct behavior.
--Quanah
--
Quanah Gibson-Mount
Sr. Member of Technical Staff
Zimbra, Inc
A Division of VMware, Inc.
--------------------
Zimbra :: the leader in open source messaging and collaboration
10 years, 7 months
Re: (ITS#7574) slapo-unique not enforcing uniqueness
by quanah＠zimbra.com
--On Friday, April 12, 2013 5:43 PM +0000 quanah(a)OpenLDAP.org wrote:
> Full_Name: Quanah Gibson-Mount
> Version: RE24 4/12/2013
> OS: Linux 2.6
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (75.111.39.181)
Also the following modify succeeds when it should fail:
zimbra@zre-ldap002:~$ ldapsearch -LLL -x -H ldapi:/// -D cn=config -w
zimbra dc=test zimbraID
dn: dc=test,dc=com
zimbraId: 03ccfb88-cbd8-47e5-94e7-0fb1fa60cacc
zimbra@zre-ldap002:~$ ldapsearch -LLL -x -H ldapi:/// -D cn=config -w
zimbra dc=example zimbraId
dn: dc=example,dc=com
zimbraId: 0c18e59d-fdd7-40b2-abff-134b118e6cf9
zimbra@zre-ldap002:~$ ldapmodify -x -H ldapi:/// -D cn=config -w zimbra
dn: dc=example,dc=com
changetype: modify
replace: zimbraId
zimbraId: 03ccfb88-cbd8-47e5-94e7-0fb1fa60cacc
modifying entry "dc=example,dc=com"
zimbra@zre-ldap002:~$
--Quanah
--
Quanah Gibson-Mount
Sr. Member of Technical Staff
Zimbra, Inc
A Division of VMware, Inc.
--------------------
Zimbra :: the leader in open source messaging and collaboration
10 years, 7 months
(ITS#7574) slapo-unique not enforcing uniqueness
by quanah＠OpenLDAP.org
Full_Name: Quanah Gibson-Mount
Version: RE24 4/12/2013
OS: Linux 2.6
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (75.111.39.181)
In current RE24, when using the back-mdb backend, uniqueness is no longer
enforced. This worked correctly up until at least 2.4.33.
dn: olcOverlay={1}unique
objectClass: olcOverlayConfig
objectClass: olcUniqueConfig
olcOverlay: {1}unique
olcUniqueURI: ldap:///?mail?sub
olcUniqueURI: ldap:///?zimbraId?sub
olcUniqueURI: ldap:///?DKIMSelector?sub
so I force uniqueness on "DKIMSelector"
It is indexed eq:
olcDbIndex: DKIMSelector eq
However, adding data for it succeeds when it should be rejected:
zimbra@zre-ldap002:~$ ldapsearch -LLL -x -H ldapi:/// -D cn=config -w zimbra
DKIMSelector=ABCD DKIMSelector
dn: dc=test,dc=com
DKIMSelector: ABCD
dn: dc=example,dc=com
DKIMSelector: ABCD
10 years, 7 months
