[Fusioninventory-devel] Fwd: LWP and certificate checking

[Guillaume Rousse]

Sorry, I confused both lists.

-------- Message original --------
Sujet: LWP and certificate checking
Date : Tue, 28 Jun 2011 11:33:07 +0200
De : Guillaume Rousse <guillomovitch at gmail.com>
Pour : FusionInventory User discussion
<fusioninventory-user at lists.alioth.debian.org>

Hello list.

I've been notified last friday of two problems with certificate checking
issues with stable release:
- the agent reject certificate using altSubjectNames
- the agent accept certificate delivered by untrusted certification
authority

So, I decided to set up a test suite with several scenarios, and test
various cases (2.1.x or 2.2.x branch, LWP 5.x or LWP 6). The test suite
tries the following cases:
- a trusted certificate, with correct hostname as CN -> OK
- a trusted certificate, with correct hostname as altSubjectNames -> OK
- a trusted certificate, with a joker hostname as CN -> OK
- a trusted certificate, with an incorrect hostname as CN -> NOK
- an untrusted certificate, with a correct hostname as CN -> NOK
The last two ones should succeed with an agent not performing ssl validation

The good news is that everything work as expected using LWP 6.x, but not
with LWP 5.x, so I spent sunday understanding why. Here the explanation.

LWP is able to use two different socket implementations, IO::Socket::SSL
and Net::SSL, to achieve SSL communication. Both of these
implementations are able to perform natively both kind of ssl checks
(hostname and certificate trust), but as LWP doesn't provide any kind of
abstraction over these features, they are just not used.

Instead, LWP provide a painful and incomplete hostname check feature,
whose only advantage is that it is implementation-neutral, using
If-SSL-Cert-Subject header. It is painful, because you have to create a
regexp yourself, wich is painful, error-prone (for instance, the current
code in 2.1.x is broken if you use a port number in the URL, because it
parses the URL itself instead), and does not follow RFC subtilies (for
instance, the * character is not supposed to be accepted in any
position). Moreover, it does not honour altSubjectName certificate
field, only subject.

So, the correct solution is to get rid of this 'feature', and instead
use native implementation of underlying code, through a custom HTTPS
protocol handler. And rather than being implementation-agnostic, it's
easier to force the implementation class used, instead of letting LWP
choose the one according to the context, as implemented in Net::HTTPS:
that's no use to setup Net::SSL settings ($ENV{HTTPS_CA_FILE}, etc...),
if IO::Socket::SSLis used instead, because already present in memory...

The 2.2.x branch has already been fixed, and the test suite proves
everything work as expected. The attached patches provide the same fix
for the 2.1.x, as well as the test suite itself, but as it is quite a
radical change, I'd prefer opinions before applying them. Basically,
they get rid of methods turnSSLOn and setSslRemoteHost, and instead add
a custom FusionInventory::Agent::HTTP::Protocol::https handler class,
overriding basic one with a single _extra_sock_opts method. That's both
simpler to read, more complete, and more efficient.

Also, the dirty trick of messing with UserAgent object internals is just
to allow coexistence of validating and non-validating agents in the same
process, as in the test suite, because there is not many ways to pass
the information 'check this one' to the handler at socket creation time.
-- 
BOFH excuse #444:

overflow error in /dev/null

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-force-use-of-IO-Socket-SSL-when-SSL-checking-is-need.patch
URL: <http://lists.alioth.debian.org/pipermail/fusioninventory-devel/attachments/20110628/3ffefedd/attachment-0002.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-test-ssl-connections-scenarios.patch
URL: <http://lists.alioth.debian.org/pipermail/fusioninventory-devel/attachments/20110628/3ffefedd/attachment-0003.ksh>