Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for querying the negotiated TLS version. The Quickening #244

Merged
merged 18 commits into from
May 30, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
2015-05-27 Jim Shaver <dcypherd@gmail.com>

* OpenSSL/SSL.py, : Add ``get_protocol_version()`` and
``get_protocol_version_name()`` to ``Connection``.
Based on work from Rich Moore.

2015-05-02 Jim Shaver <dcypherd@gmail.com>

* .travis.yml, setup.py, tox.ini: Removed support for Python 3.2.
Expand Down
25 changes: 25 additions & 0 deletions OpenSSL/SSL.py
Original file line number Diff line number Diff line change
Expand Up @@ -1883,6 +1883,31 @@ def get_cipher_version(self):
return version.decode("utf-8")


def get_protocol_version_name(self):
"""
Obtain the protocol version of the current connection.

:returns: The TLS version of the current connection, for example
the value for TLS 1.2 would be ``TLSv1.2``or ``Unknown``
for connections that were not successfully established.
:rtype: :py:class:`unicode`
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that’s not true anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

208438c now bytes

"""
version = _ffi.string(_lib.SSL_get_version(self._ssl))
return version.decode("utf-8")
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not sure about this one. I believe we decided to keep pyOpenSSL’s APIs bytes-based except for paths.

So unless someone can enlighten me on what I’m missing, I’m gonna claim this ought to be bytes.

Copy link
Member

Choose a reason for hiding this comment

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

Dunno, the name of the protocol version definitely seems textual to me.

On Mon, Apr 27, 2015 at 10:16 AM, Hynek Schlawack notifications@github.com
wrote:

In OpenSSL/SSL.py
#244 (comment):

@@ -1883,6 +1883,18 @@ def get_cipher_version(self):
return version.decode("utf-8")

  • def get_protocol_version(self):
  •    """
    
  •    Obtain the protocol version of the current connection.
    
  •    :returns: The TLS version of the current connection, for example
    
  •        the value for TLS 1.2 would be 0x303.
    
  •    :rtype: :py:class:`unicode`
    
  •    """
    
  •    version = _ffi.string(_lib.SSL_get_version(self._ssl))
    
  •    return version.decode("utf-8")
    

I’m not sure about this one. I believe we decided to keep pyOpenSSL’s APIs
bytes-based except for paths.

So unless someone can enlighten me on what I’m missing, I’m gonna claim
this ought to be bytes.


Reply to this email directly or view it on GitHub
https://github.com/pyca/pyopenssl/pull/244/files#r29149506.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand your argument correctly, you are saying that when you query for TLS version you should receive something like 0x769 as a response? The whole python3 bytes v. unicode thing is still new to me... What was the thinking behind trying to stick with bytes?

Plenty of things in SSL.py are unicode strings though, including much of the related cipher information. Should a developer really have to get out their OpenSSL decoder ring everytime she wants to query a cipher? I think TLS_RSA_WITH_AES_256_CBC_SHA256 makes a lot more sense than 0x3d, as long as the upstream doesn't change the formatting of those strings.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the first paragraph; what I’m saying is that the docstring is plain wrong. There’s no way you get anything resembling “0x303” back. You get a string like "TLSv1" back. Try it and pdb in your test. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My apologies, I should have looked at the code you were commenting on more closely! I thought I had updated it from the way it was initially proposed in #184. Fixed in 6ac2e2a.



def get_protocol_version(self):
"""
Obtain the protocol version of the current connection.

:returns: The TLS version of the current connection, for example
the value for TLS 1 would be 0x769.
:rtype: :py:class:`int`
"""
version = _lib.SSL_version(self._ssl)
return version


@_requires_npn
def get_next_proto_negotiated(self):
"""
Expand Down
30 changes: 30 additions & 0 deletions OpenSSL/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,36 @@ def test_get_cipher_bits(self):
self.assertEqual(server_cipher_bits, client_cipher_bits)


def test_get_protocol_version_name(self):
"""
:py:obj:`Connection.get_protocol_version_name()` returns a string
giving the protocol version of the current connection.
"""
server, client = self._loopback()
client_protocol_version_name = client.get_protocol_version_name()
server_protocol_version_name = server.get_protocol_version_name()

self.assertIsInstance(server_protocol_version_name, text_type)
self.assertIsInstance(client_protocol_version_name, text_type)

self.assertEqual(server_protocol_version_name, client_protocol_version_name)


def test_get_protocol_version(self):
"""
:py:obj:`Connection.get_protocol_version()` returns an integer
giving the protocol version of the current connection.
"""
server, client = self._loopback()
client_protocol_version = client.get_protocol_version()
server_protocol_version = server.get_protocol_version()

self.assertIsInstance(server_protocol_version, int)
self.assertIsInstance(client_protocol_version, int)

self.assertEqual(server_protocol_version, client_protocol_version)



class ConnectionGetCipherListTests(TestCase):
"""
Expand Down
15 changes: 15 additions & 0 deletions doc/api/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,21 @@ Connection objects have the following methods:
but not it returns the entire list in one go.


.. py:method:: Connection.get_protocol_version()

Retrieve the version of the SSL or TLS protocol used by the Connection.
For example, it will return ``0x769`` for connections made over TLS
version 1.


.. py:method:: Connection.get_protocol_version_name()

Retrieve the version of the SSL or TLS protocol used by the Connection as
a unicode string. For example, it will return ``TLSv1`` for connections
made over TLS version 1, or ``Unknown`` for connections that were not
successfully established.


.. py:method:: Connection.get_client_ca_list()

Retrieve the list of preferred client certificate issuers sent by the server
Expand Down