Skip to content

Commit

Permalink
set check_hostname attribute before setiing verify_mode.
Browse files Browse the repository at this point in the history
  • Loading branch information
@liris
liris committed Apr 20, 2015
1 parent ced3c3e commit b96a2e8
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions websocket/_http.py
View file
Expand Up @@ -124,10 +124,11 @@ def _can_use_sni():

def _wrap_sni_socket(sock, sslopt, hostname, check_hostname):
context = ssl.SSLContext(sslopt.get('ssl_version', ssl.PROTOCOL_SSLv23))
context.load_verify_locations(cafile=sslopt.get('ca_certs', None))
context.verify_mode = sslopt['cert_reqs']

if HAVE_CONTEXT_CHECK_HOSTNAME:
context.check_hostname = check_hostname
context.load_verify_locations(cafile=sslopt.get('ca_certs', None))
context.verify_mode = sslopt['cert_reqs']
if 'ciphers' in sslopt:
context.set_ciphers(sslopt['ciphers'])

Expand Down

6 comments on commit b96a2e8

@KenjiTakahashi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I did it on purpose. verify_mode defaults to CERT_NONE and setting check_hostname to True before changing it will raise exception.

@KenjiTakahashi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @liris

@liris
Copy link
Collaborator Author

@liris liris commented on b96a2e8 Apr 20, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KenjiTakahashi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. But it is checked both ways:

  1. trying to set verify_mode = CERT_NONE checks for check_hostname == False.
  2. trying to set check_hostname = True checks for verify_mode != CERT_NONE.

Where by default check_hostname = False and verify_mode = CERT_NONE.

Case 1. is prevented by making sure that function param check_hostname is always False when cert_reqs is CERT_NONE (it's down below, at :150).
To prevent case 2. we have to set verify_mode before check_hostname, otherwise this happens:

In [1]: import websocket

In [2]: websocket.create_connection("wss://echo.websocket.org").status
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-2-f86ccfe5ac08> in <module>()
----> 1 websocket.create_connection("wss://echo.websocket.org").status

/home/kenji/.virtualenvs/snipy3/lib/python3.4/site-packages/websocket/_core.py in create_connection(url, timeout, **options)
    102                         skip_utf8_validation=skip_utf8_validation)
    103     websock.settimeout(timeout if timeout is not None else getdefaulttimeout())
--> 104     websock.connect(url, **options)
    105     return websock
    106 

/home/kenji/.virtualenvs/snipy3/lib/python3.4/site-packages/websocket/_core.py in connect(self, url, **options)

--> 253         self.sock, addrs = connect(url, self.sock_opt, proxy_info(**options))
    254 
    255         try:

/home/kenji/.virtualenvs/snipy3/lib/python3.4/site-packages/websocket/_http.py in connect(url, options, proxy)
     67         if is_secure:
     68             if HAVE_SSL:
---> 69                 sock = _ssl_socket(sock, options.sslopt, hostname)
     70             else:
     71                 raise WebSocketException("SSL not available.")

/home/kenji/.virtualenvs/snipy3/lib/python3.4/site-packages/websocket/_http.py in _ssl_socket(sock, user_sslopt, hostname)
    151 
    152     if _can_use_sni():
--> 153         sock = _wrap_sni_socket(sock, sslopt, hostname, check_hostname)
    154     else:
    155         sslopt.pop('check_hostname', True)

/home/kenji/.virtualenvs/snipy3/lib/python3.4/site-packages/websocket/_http.py in _wrap_sni_socket(sock, sslopt, hostname, check_hostname)
    127 
    128     if HAVE_CONTEXT_CHECK_HOSTNAME:
--> 129         context.check_hostname = check_hostname
    130     context.load_verify_locations(cafile=sslopt.get('ca_certs', None))
    131     context.verify_mode = sslopt['cert_reqs']

ValueError: check_hostname needs a SSL context with either CERT_OPTIONAL or CERT_REQUIRED

IMHO it is badly thought out in the stdlib, but we have to live with this.

@liris
Copy link
Collaborator Author

@liris liris commented on b96a2e8 Apr 21, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.
578b57d to fix this.

@KenjiTakahashi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be OK now, thanks.

Please sign in to comment.