RE: (ITS#6828) TLS fails to start when LDAP_OPT_CONNECT_ASYNC is used
by ipuleston@SonicWALL.com
There is something missing from the fix. I just had a problem where an asynchronous connect was failing and the result was continuous looping with repeated calls to ldap_sasl_bind.
The problem is that if the connect fails, ldap_int_poll called from ldap_int_check_async_open gets an error and returns -1, and then -1 is returned up the stack ldap_int_check_async_open -> ldap_send_initial_request -> ldap_sasl_bind. But when ldap_sasl_bind gets that -1 return from ldap_send_initial_request it returns ld->ld_errno, and nowhere have we set that. In fact, ld->ld_errno still contains LDAP_X_CONNECTING from when the initial connect was issued, and so ldap_sasl_bind returns LDAP_X_CONNECTING, and that is what leads to the infinite looping that I see.
What is missing is that in ldap_int_check_async_open this:
default:
return -1;
Should be changed to:
default:
ld->ld_errno = LDAP_CONNECT_ERROR;
return -1;
Ian
11 years, 11 months
RE: (ITS#6828) TLS fails to start when LDAP_OPT_CONNECT_ASYNC is used
by ipuleston@SonicWALL.com
Thanks Howard. I pulled your change out of GIT and tested it, and it works fine for me.
However, just as an FYI, this line in open.c results in a "suggest parentheses around && within ||" warning from gcc with the compiler options that we use to compile our OpenLDAP port, and that in turn results in a build failure (since we use -Werror):
if (rc == 0 && ld->ld_options.ldo_tls_mode == LDAP_OPT_X_TLS_HARD ||
strcmp( srv->lud_scheme, "ldaps" ) == 0 )
The parentheses that I had in that line in my patch avoided that.
Ian
11 years, 11 months
Re: (ITS#6828) TLS fails to start when LDAP_OPT_CONNECT_ASYNC is used
by hyc@symas.com
Ian Puleston wrote:
>> -----Original Message-----
>> From: Howard Chu [mailto:hyc@symas.com]
>> Sent: Wednesday, June 08, 2011 5:47 PM
>>
>> I'm pretty sure that this chunk can never do anything useful.
>> ...
>> The connection has just been created, asynchronously, so there's no way
>> that
>> the TLS layer was already started when it got here.
>
> After re-reviewing it I think you are correct. I put that in because I thought there may be a chance that ldap_int_tls_start could get called from ldap_int_open_connection if the connect completed very quickly in the underlying layers. But even if it did there has been no call to ldap_int_poll to check for it having completed. ldap_pvt_connect and then ldap_connect_to_host return -2 to ldap_int_open_connection on an async connect. And the latter then only calls ldap_int_tls_start if rc == 0, so that call will not happen.
>
>> Also, I suggest that you only check for CONNST_CONNECTING in the
>> callers, and do the TLS check in the check function
>
> Yes, good idea. I guess that the way I have it allows it to start up TLS immediately if using TLS and the connect completes immediately, but if not using TLS it will return LDAP_X_CONNECTING. What you are suggesting means that if not using TLS and the connect completes immediately then it will be able to go ahead and send the request rather than retuning LDAP_X_CONNECTING.
>
> I will update the fix and supply a new patch shortly.
No need, I already committed a fix to git. It is also in RE24.
>
> Thanks
> Ian
>
>
>
>
>
>
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
11 years, 11 months
RE: (ITS#6828) TLS fails to start when LDAP_OPT_CONNECT_ASYNC is used
by ipuleston@SonicWALL.com
> -----Original Message-----
> From: Howard Chu [mailto:hyc@symas.com]
> Sent: Wednesday, June 08, 2011 5:47 PM
>
> I'm pretty sure that this chunk can never do anything useful.
> ...
> The connection has just been created, asynchronously, so there's no way
> that
> the TLS layer was already started when it got here.
After re-reviewing it I think you are correct. I put that in because I thought there may be a chance that ldap_int_tls_start could get called from ldap_int_open_connection if the connect completed very quickly in the underlying layers. But even if it did there has been no call to ldap_int_poll to check for it having completed. ldap_pvt_connect and then ldap_connect_to_host return -2 to ldap_int_open_connection on an async connect. And the latter then only calls ldap_int_tls_start if rc == 0, so that call will not happen.
> Also, I suggest that you only check for CONNST_CONNECTING in the
> callers, and do the TLS check in the check function
Yes, good idea. I guess that the way I have it allows it to start up TLS immediately if using TLS and the connect completes immediately, but if not using TLS it will return LDAP_X_CONNECTING. What you are suggesting means that if not using TLS and the connect completes immediately then it will be able to go ahead and send the request rather than retuning LDAP_X_CONNECTING.
I will update the fix and supply a new patch shortly.
Thanks
Ian
11 years, 11 months
Re: (ITS#6978) Invalid indentation of splitted lines in LDIF input file causes SEGFAULT
by jvcelak@redhat.com
On Friday 24 June 2011 09:11:41, Howard Chu wrote:
> >> There is no file ldifutil.c in OpenLDAP 2.4.
> >>
> >> I don't know what you're testing against, but this bug report appears
> >> invalid. Closing.
> >
> > $ find -name ldifutil.c
> > ./libraries/libldap/ldifutil.c
> > ./libraries/libldap_r/ldifutil.c
> >
> > Tested against git master branch.
>
> Read again what I wrote. There is no file ldifutil.c in OpenLDAP 2.4. git
> master is not the same as the 2.4 releases.
Ouch, sorry. Then it is fixed only in master, not in RE24 (7cac590).
$ ./ldapmodify -a -x -f /tmp/invalid.ldif -d2048
ldif_parse_line: missing ':' after dc=com
ldapmodify: invalid format (line 2) entry: "cn=B,dc=my-domain,"
Segmentation fault (core dumped)
(gdb) bt
#0 __strcasecmp_l_ssse3 () at ../sysdeps/x86_64/strcmp.S:214
#1 0x000000000040789c in process_ldif_rec (rbuf=0x67464e "objectclass:
inetOrgPerson\nobjectclass: organizationalPerson\nobjectclass:
person\nobjectclass: top\ncn: B\nsn: B\nuid: B\nmail: b(a)example.org\n",
linenum=1) at ldapmodify.c:655
#2 0x00000000004067d5 in main (argc=6, argv=0x7fffffffdd88) at
ldapmodify.c:335
11 years, 11 months
Re: (ITS#6978) Invalid indentation of splitted lines in LDIF input file causes SEGFAULT
by hyc@symas.com
Jan Vcelak wrote:
> On Thursday 23 June 2011 22:41:20, Howard Chu wrote:
>> jvcelak(a)redhat.com wrote:
>>> Full_Name: Jan Vcelak
>>> Version: 2.5.25
>>
>> There is no OpenLDAP version 2.5.
>
> This is a typo. I mean 2.4.25. (But it an older issue, it was reported in RH
> bugzilla against 2.4.23.)
>
>> There is no file ldifutil.c in OpenLDAP 2.4.
>>
>> I don't know what you're testing against, but this bug report appears
>> invalid. Closing.
>
> $ find -name ldifutil.c
> ./libraries/libldap/ldifutil.c
> ./libraries/libldap_r/ldifutil.c
>
> Tested against git master branch.
Read again what I wrote. There is no file ldifutil.c in OpenLDAP 2.4. git
master is not the same as the 2.4 releases.
>> Your example doesn't SEGV for me. Anyway, I've committed a different patch
>> to master for this issue.
>
> Strange... Fedora Linux 15, 64bit. And clearly an uninitialized memory was
> accessed. But your fix is fine and works for me.
>
> Thank you.
>
> Jan
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
11 years, 11 months
Re: (ITS#6978) Invalid indentation of splitted lines in LDIF input file causes SEGFAULT
by jvcelak@redhat.com
On Thursday 23 June 2011 22:41:20, Howard Chu wrote:
> jvcelak(a)redhat.com wrote:
> > Full_Name: Jan Vcelak
> > Version: 2.5.25
>
> There is no OpenLDAP version 2.5.
This is a typo. I mean 2.4.25. (But it an older issue, it was reported in RH
bugzilla against 2.4.23.)
> There is no file ldifutil.c in OpenLDAP 2.4.
>
> I don't know what you're testing against, but this bug report appears
> invalid. Closing.
$ find -name ldifutil.c
./libraries/libldap/ldifutil.c
./libraries/libldap_r/ldifutil.c
Tested against git master branch.
> Your example doesn't SEGV for me. Anyway, I've committed a different patch
> to master for this issue.
Strange... Fedora Linux 15, 64bit. And clearly an uninitialized memory was
accessed. But your fix is fine and works for me.
Thank you.
Jan
11 years, 11 months
Re: (ITS#6977) ldapwhoami minor verbose bugfix patch submission
by hyc@symas.com
hyc(a)symas.com wrote:
> checker(a)d6.com wrote:
>> Full_Name: Chris Hecker
>> Version: 2.4.25
>> OS: centos 5.6
>> URL: ftp://ftp.openldap.org/incoming/
>> Submission from: (NULL) (98.248.126.162)
>>
>>
>> Hi, ldapwhoami erroneously prints the "Result: Success (0)" string even if -v is
>> not specified due to a minor bug in the code. Here is a fix. I am running
>> 2.3.43, but just checked the latest repository version and the bug is still
>> there (although at line 203 now), so I marked the version above as latest.
>>
>> Thanks,
>> Chris
>
> It seems to me the better fix would be to change ldap_parse_result() to not
> populate matcheddn or text when their values are zero-length. Kurt, any
> particular reason things should continue to work as they currently do?
OK, they work according to the C API draft. (sigh.) Client tools patched in
master.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
11 years, 11 months
Re: (ITS#6977) ldapwhoami minor verbose bugfix patch submission
by checker@d6.com
Yeah, I just decided to do the same test as the two enclosed if
statements to be 100% compatible.
Chris
On 2011/06/23 13:31, Howard Chu wrote:
> checker(a)d6.com wrote:
>> Full_Name: Chris Hecker
>> Version: 2.4.25
>> OS: centos 5.6
>> URL: ftp://ftp.openldap.org/incoming/
>> Submission from: (NULL) (98.248.126.162)
>>
>>
>> Hi, ldapwhoami erroneously prints the "Result: Success (0)" string
>> even if -v is
>> not specified due to a minor bug in the code. Here is a fix. I am running
>> 2.3.43, but just checked the latest repository version and the bug is
>> still
>> there (although at line 203 now), so I marked the version above as
>> latest.
>>
>> Thanks,
>> Chris
>
> It seems to me the better fix would be to change ldap_parse_result() to
> not populate matcheddn or text when their values are zero-length. Kurt,
> any particular reason things should continue to work as they currently do?
>
>>
>> --- old/openldap-2.3.43/clients/tools/ldapwhoami.c 2008-02-11
>> 17:24:07.000000000 -0600
>> +++ openldap-2.3.43/clients/tools/ldapwhoami.c 2011-06-21
>> 14:15:28.000000000
>> -0500
>> @@ -215,7 +215,7 @@
>>
>> skip:
>> if ( verbose || ( code != LDAP_SUCCESS ) ||
>> - matcheddn || text || refs || ctrls )
>> + (matcheddn&& *matcheddn) || (text&& *text) || refs || ctrls )
>> {
>> printf( _("Result: %s (%d)\n"), ldap_err2string( code ), code
>> );
>>
>>
>>
>
>
11 years, 11 months