[PATCH] sslutil: python2.4 and 2.5 cannot verify certificates

[Mads Kiilerich]

Stephen Thorne wrote, On 06/13/2011 03:32 AM:
> # HG changeset patch
> # User Stephen Thorne<stephen at thorne.id.au>
> # Date 1307928723 -36000
> # Node ID 131f3dcbb025b0f464f40f3cd9de52245d13a8e8
> # Parent  b72cef1b8b26262f627423c16b5c09396721b220
> sslutil: python2.4 and 2.5 cannot verify certificates

http://mercurial.selenic.com/wiki/ContributingChanges gives a good 
advice: "summarize the fix, not the problem"

> Two imports were omitted in the restructure of the code creating
> sslutil.py, socket and httplib are required when the 'ssl' module
> cannot be imported, restoring these imports allows mercurial to run
> on python2.4+2.5 again.

That looks ok - in a separate patch and with a better summary.

[I jump a bit:]
> Running mercurial on python2.4 or 2.5 with these import errors
> fixed
...
> If a hostfingerprint was added to the http configuration
> then an Abort would happen every time because socket.getpeercert() is
> not available and thus the fingerprint can't be verified.

"Abort" is often intentional and you would have to describe why the 
original intention was wrong, but in this case it crashed with an 
AttributeError, right? I think that should be described more clearly in 
the patch description and would be fine if in a separate patch.

> would result in a ui warning every time when accessing a https
> respository telling the user there was not hostfingerprint set in the
> configuration.

Yes, that hint didn't tell the full story in this case where it wouldn't 
solve the problem anyway.

It would however be nice if we didn't trick 2.4 users into configuring 
something that won't solve the problem anyway. Perhaps something like 
"warning: %s certificate not verified because of missing Python SSL 
capabilities".

But ...

> The warning has been downgraded to an info on python2.4+2.5 and no longer
> allows you to cause mercurial to simply Abort when you attempt to configure it
> to verify the certificate.

If Mercurial has been configured to do something and it for some reason 
can't do it then it shouldn't silently do something different - 
especially not when it is security related. This Abort has to stay.

And in the case where Mercurial hasn't (but should have) been configured 
to verify the certificate it should issue a warning - even if it is 
hard/impossible for the user to fix it. It is our duty to inform the 
user the he doesn't use SSL properly and is no more secure than if used 
plain HTTP.

/Mads