critical fixes for perl-ldap 0.43
Peter Marschall
please consider pulling the commits in my pull request
https://github.com/gbarr/perl-ldap/pull/3
into master and release version 0.44 of perl-ldap really soon.
Reasons:
* 0.43 has a serious regression: commit 041d540 broke start_tls completely
and led to warnings being generated at every encrypted connection with
sslverify='none'.
This is fixed by commit a3c4f7f "un-break certificate verification"
BTW: this commit does The Right Thing(tm) and obsoletes commit 4dc845e
"Verify hostnames in TLS connections" in the next branch.
In my opinion we should not add additional compatibility flags for bug fixes:
not checking the host name in the sslverify != ' none' cases was definitely
a bug (allowing MITM attacks to go unnoticed)
Instead, I propose a note in the ChangeLog,
* lots of typo fixes in documentation
* extended documentation for Pre-Read & Post-Read controls
As bonus:
* Assertion Control implemented
* new control LDAP_CONTROL_PERMISSIVEMODIFY
Thanks
Peter
--
Peter Marschall
pe...@adpm.de
Peter Marschall
On Wednesday, 7. September 2011, Robert Threet wrote:
> I had a program that read the LDIF dump of my People container for
> years. In the past year, it has become unreliable. It was after
> patches to the Sun Directory and adding a 2md RW Master. Do you think
> this would fix that or is that another problem?
A) Please keep your mail on the list!
B) Please do not top-post!
C) To answer your question:
These fixes are completely unrelated to the problem you have.
Hint: compare the dates of your issues and when the regresion
was introduced.
Peter Marschall
On Thursday, 8. September 2011, Graham Barr wrote:
> > * 0.43 has a serious regression: commit 041d540 broke start_tls
> > completely
> >
> > and led to warnings being generated at every encrypted connection with
> > sslverify='none'.
> > This is fixed by commit a3c4f7f "un-break certificate verification"
> >
> > BTW: this commit does The Right Thing(tm) and obsoletes commit 4dc845e
> > "Verify hostnames in TLS connections" in the next branch.
>
Here are additional arguments:
* setting SSL_verifycn_scheme => 'ldap' (as it was done unconditionally in
0.43) automatically does a hostname verification
* commit a3c4f7f makes sure that this does not happen when sslverify => 'none'
avoiding that a warning shows up on every LDAPS connection
* for the sslverify => { 'require' | 'optional' } cases commit a3c4f7f
makes sure the host name is set, hence unbreaking start_tls()
* IMHO there's already an option to configure hostname verification: 'sslverify'
Setting it to 'none' will disable hostname verficiation, in the other cases
host names are checked automatically.
There's no need for another option doing the same thing
* IMHO not v erifying the host names in the 'optional' and 'require' cases
was a bug and a security risk. We should fix it instead of adding options
to continue keeping the users at risk.
Anyone wanting the old behaviour back, can set sslverify => 'none'
NOTE:
1) if someone switched to 0.43 they are bitten already
even in the sslverify => 'none' case
2) the only ones affected by this issue are those connecting to a
server where the hostname in the certificate does not match
the host name of the server they connect to.
In the non-matching cases, other clients that adhere to the standard
were complaining already.
So the number of these cases should be fairly low.
(if the host names match, then the new behaviour is identical to
the old one: everything works without a problem)
* IMHO the option name 'check' is too generic.
* if I understand the connection / start_tls code correctly,
* in the LDAPS case there will be no connection if host name verification
fails. Hence there's no ned to disconnect.
* in the start_tls() case, it's a bit different
Here dissconnecting after a failed IO::socket::SSL->start_SSL
might be an option.
In my checks, any succeeding communication in the same session failed,
but closing the connection explicitly might make it clearer for the users.
* IMHO disconnect() is too generic too.
In the remaining case above, it should be changed so something like:
_drop_conn($self, LDAP_USER_CANCELED, "start_tls() failed");
Best regards
Chris Ridd
On 8 Sep 2011, at 14:26, Graham Barr wrote:
> On Sep 7, 2011, at 09:26 , Peter Marschall wrote:
>>
>> please consider pulling the commits in my pull request
>> https://github.com/gbarr/perl-ldap/pull/3
>> into master and release version 0.44 of perl-ldap really soon.
>>
>> Reasons:
>> * 0.43 has a serious regression: commit 041d540 broke start_tls completely
>> and led to warnings being generated at every encrypted connection with
>> sslverify='none'.
>> This is fixed by commit a3c4f7f "un-break certificate verification"
>>
>> BTW: this commit does The Right Thing(tm) and obsoletes commit 4dc845e
>> "Verify hostnames in TLS connections" in the next branch.
>
Chris