[PATCH] Makes https+pycurl urls works with self-certified hosts
John Arbash Meinel
john at arbash-meinel.com
Mon Aug 21 15:54:46 BST 2006
Vincent LADEUIL wrote:
>>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:
> >> <cough> Setting HHTPS_PROXY (note caps) works. <blush>
> jam> Well, I hope it is 'HTTPS_PROXY' :)
v- The first line of this change seems accidental, you don't really want
the extra whitespace, do you?
Also, you are again using Tabs, rather than space characters.
> ua_str = 'bzr/%s (pycurl)' % (bzrlib.__version__,)
> curl.setopt(pycurl.USERAGENT, ua_str)
> curl.setopt(pycurl.HTTPHEADER, headers)
> - curl.setopt(pycurl.FOLLOWLOCATION, 1) # follow redirect responses
> + curl.setopt(pycurl.FOLLOWLOCATION, 1) # follow redirect responses
> + # FIXME: The following allows the use of self-certified
> + # hosts. This should not be activated without the user
> + # understanding the consequences (there is a risk to
> + # connect to rogue hosts). This should certainly be
> + # handled better but that keep the relevant informations
> + # at the right place for now.
> + if os.getenv('BZR_ACCEPT_SELF_CERTIFIED_HOSTS') is not None:
> + curl.setopt(pycurl.SSL_VERIFYHOST, 1)
> + curl.setopt(pycurl.SSL_VERIFYPEER, False)
I think it would be better to keep a list somewhere, of what hosts have
been accepted. Rather than just blindly accepting all self-certified
hosts. This has pretty big implications for man-in-the-middle attacks.
If you allow self-certified across the board, than a man-in-the-middle
has no problem inserting themselves, and the user won't even be alerted
to the fact that the certified host's key has changed.
So I would prefer a user prompt (through bzrlib.ui.ui_factory) in the
case that the self certificate fails.
I'm not sure where you would exactly need to put this check. But I think
it would end up being around one of the get/has handlers. Because you
don't know that it fails to validate until you have actually made a
request. But then if it does fail, you catch the correct exception (I
would guess CURLE_SSL_PEER_CERTIFICATE (51) look in _pycurl_errors.py).
Now, in the short term, it might be okay to expose a big thunk like
this. But we can make it slightly better with:
BZR_ACCEPT_SELF_CERTIFIED_HOSTS = 'host1:host2:host3:host4'
And then internally you use:
hosts = os.getenv('BZR_ACCEPT_SELF_CERTIFIED_HOSTS', '').split(':')
if self._host in hosts:
So final feeling:
1) You need to get rid of the tabs
2) If you use an env var, have it explicitly list the allowed hosts. It
might also be reasonable to put this into ~/.bazaar/bazaar.conf, and
then you can access it with a_config = bzrlib.config.GlobalConfig().
Though it is a small layering violation to have a Transport know about
bzrlib.config. We've never really stated how we want to handle global
state information like this.
3) I would prefer handling this on a case-by-case, with a user-prompt.
But (2) is reasonably secure, since you have to explicitly enable a host
as being okay for self certified. It still leaves you open to
man-in-the-middle on that host, but at least it doesn't open you up to
man-in-the-middle on every other host.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060821/5c360526/attachment.pgp
More information about the bazaar