[PATCH] Makes https+pycurl urls works with self-certified hosts

[John Arbash Meinel]

PING

You submitted this a while ago, and I gave a bit of feedback. Have you
decided to not worry about it, or what are you thinking to do with this
change?

John
=:->


John Arbash Meinel wrote:
> Vincent LADEUIL wrote:
>>>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:
>> <snip/>
>>
>>     >>  <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:
>   curl.setopt(...)
> 
> 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.
> 
> John
> =:->
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061031/bcb1070d/attachment.pgp