[PATCH] sslutil: python2.4 and 2.5 cannot verify certificates
Mads Kiilerich
mads at kiilerich.com
Mon Jun 13 16:39:53 UTC 2011
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
More information about the Mercurial-devel
mailing list