On Sun, Sep 27, 2015 at 05:17:54PM -0700, Doug Hogan wrote:
> On Sun, Sep 27, 2015 at 09:11:22PM +0200, Landry Breuil wrote:
> > On Sat, Sep 26, 2015 at 03:34:53PM +0200, Jona Joachim wrote:
> > > Hi,
> > > weboob has a runtime dependency for SSLv3. The attached diff replaces
> > > SSLv3 usage with SSLv23.
> > > 
> > > 2015-09-26 15:02:52,557:ERROR:weboob:1.0:ouiboube.py:450:load_backends
> > > Unable to load module "cic": 'module' object has no attribute
> > > 'PROTOCOL_SSLv3'
> > > 
> > > This is already fixed in the upstream git repo.
> > 
> > I dont see this in
> > https://github.com/laurentb/weboob/blob/master/weboob/deprecated/browser/browser.py#L783
> 
> PROTOCOL_SSLv3 was replaced with PROTOCOL_SSLv23 in commit: c42f4b61d83b
> However, I think that change and the previous code was a mistake.
> Despite the name, they should only be using SSLv23.
> 
> > > +-    _PROTOCOLS = [ssl.PROTOCOL_TLSv1, ssl.PROTOCOL_SSLv3]
> > > ++    _PROTOCOLS = [ssl.PROTOCOL_SSLv23]
> > 
> > Im not sure removing TLSv1 makes sense... dont we want to deprecate
> > SSLv2 and SSLv3 here ? What upstream commit are you reffering to ?
> 
> OpenSSL has an unfortunately named 'SSLv23_*' method that really means
> highest supported protocol (SSL or TLS).  You can optionally disable
> protocols by settings options SSL_OP_NO_*.  We gutted SSLv2 and SSLv3
> support in LibreSSL so 'SSLv23' means TLS v1.0 or higher for us by
> default.
> 
> Upstream weboob changed the above line in git to:
> 
> _PROTOCOLS = [ssl.PROTOCOL_TLSv1, ssl.PROTOCOL_SSLv23]
> 
> It looks like their intention is to try TLS and if that fails, fall
> back on SSL.  The way they are doing this has unintended consequences
> and it's not necessary to have multiple tries.
> 
> 'TLSv1' is another unfortunate name that means TLS v1.0 only and not
> v1.*.  The _PROTOCOLS above will try TLS v1.0 specifically and if that
> fails, fall back on what SSLv23 allows.  In practice, 'TLSv1' will
> select TLS v1.0 even if TLS v1.0 through v1.2 are supported by the
> server.  If 'SSLv23' was used, it will select the highest supported
> protocol in common between the client and the server (but subject to
> SSL_OP_NO_*).
> 
> Upstream should remove _PROTOCOLS and always use SSLv23.  It already
> does what they want.

Oh $DEITY so much nonsense in there... anyway, commited the fix.

Landry

Reply via email to