Re: [pywbem-devel] TOCTOU issue (time of check, time of use)

[Michal M. <mi...@re...>]

The patch below has several flaws. Apart from one unnecessary connection
in connect() method, it searches for ca certificates on wrong places.
Recent ca-certificates uses ca-bundle files that are regenerated when
new trusted certificates are added to
     /etc/pki/ca-trust/source/anchors
(and update-ca-trust is run). For these directories cacertdir_rehash is 
no longer
used. Thus passing path to directory to load_verify_locations() is 
meaningless.

Moreover M2Crypto can not handle unicode strings. If data or some header
is a unicode string, m2crypto will accept it but the other side will 
receive rubbish.
Such cases must be prevented.

Attached is an improved version of previous patch.

Best regards,
Michal Minar
On 12/16/2013 04:40 PM, Michal Minář wrote:
> Hello,
>
> there is a security issue in making connection over ssl.
> Here is a snippet from cim_http.py:
>
>      124             try:
>      125                 from OpenSSL import SSL
>      126                 ctx = SSL.Context(SSL.SSLv3_METHOD)
>      127                 ctx.set_verify(SSL.VERIFY_PEER, verify_callback)
>      128                 ctx.set_default_verify_paths()
>      129                 # Add the key and certificate to the session
>      130                 if cert_file is not None and key_file is not 
> None:
>      131                   ctx.use_certificate_file(cert_file)
>      132                   ctx.use_privatekey_file(key_file)
>      133                 s = SSL.Connection(ctx, 
> socket.socket(socket.AF_INET,
>      134 socket.SOCK_STREAM))
>      135                 s.connect((host, port))
>      136                 s.do_handshake()
>      137                 s.shutdown()
>      138                 s.close()
>      139             except socket.error, arg:
>      140                 raise Error("Socket error: %s" % (arg,))
>      141             except socket.sslerror, arg:
>      142                 raise Error("SSL error: %s" % (arg,))
>
>      152         h = HTTPSConnection(host, port = port, key_file = 
> key_file,
>      153                                             cert_file = 
> cert_file)
>
> On lines 124-142 a connection to remote host is done just to validate
> peer's certificate. The connection is closed and a new one is created
> without any checks being done to peer's certificate and without any
> verification callback used.
>
> The second connection is therefore prune to MITM attack. In order to
> make it secure, the second connection needs to take ca_certs[1] argument
> either given by user or some system path. It also needs to check
> hostname in peer's certificate against the hostname we're trying to
> connect to. If we make the second connection properly secured, the first
> one looses any meaning and can be dropped.
>
> Attached is a patch that tries to solve these issues. It uses
> M2Crypto[2] library making it its new dependency. On the other hand
> pywbem no longer depends on pyOpenSSL. You may argue, that ssl or
> OpenSSL could be used to achieve the same thing. I have good reasons not
> to use them:
>
>     1. ssl (in standard python library) does not provide anything like:
>           ctx.set_default_verify_paths()
>        Moreover it does not accept directory as a trust store. Therefor
>        only path to particular pem file needs to be suplied. In case the
>        user uses self-signed certificate to secure its connection (which
>        cannot be verified against something like
>        /etc/ssl/certs/ca-bundle.crt), he would have to suply ca_certs
>        manually. That is not very user friendly and breaks the current
>        behaviour where system paths are searched.
>
>        On the other hand, with ssl we could easily check for matching
>        hostnames. There is a function match_hostname()[3] accepting the
>        output of socket.getpeercert() made exactly for this purpose.
>
>     2. OpenSSL has this very handy function:
>           ctx.set_default_verify_paths()
>        But unfortunatelly it does not verify hostname. We also can not
>        use match_hostname[3] like in previous case. If we really want to
>        use it, we'd have to somehow parse raw string obtained from
>        X509Extension object for subjectAltName extension. That would add
>        a lot of boiler plate for such an easy task.
>
> That's the reason why M2Crypto is used in this patch. It does both
> things good.
>
> Please comment on this patch. Maybe there's better solution for this
> problem. I'd like to have some acks from you before commiting this
> patch.
>
> Thank you, and Merry Christmas :).
> Michal Minar
>
> [1] Path to a CA pem file or to local trust store database.
> [2] http://www.heikkitoivonen.net/blog/2008/10/14/ssl-in-python-26/
> [3] https://pypi.python.org/pypi/backports.ssl_match_hostname
>
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
>
>
> _______________________________________________
> pywbem-devel mailing list
> pyw...@li...
> https://lists.sourceforge.net/lists/listinfo/pywbem-devel