Skip to content

Provide data for TLS channel binding #56760

Closed
Closed
Provide data for TLS channel binding#56760
Labels
stdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

Description

@Jajcus
Jajcus
mannequin
opened on Jul 13, 2011
BPO 12551
Nosy @jcea, @pitrou
Files
  • tls_channel_binding.patch
  • tls_channel_binding.patch: Patch updated with Antoine's suggestions
  • tls_channel_binding_alt.patch: Alternative version, CHANNEL_BINDING_TYPES instead of HAS_TLS_UNIQUE
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2011-07-20.23:13:47.516>
created_at = <Date 2011-07-13.11:33:25.161>
labels = ['type-feature', 'library']
title = 'Provide data for TLS channel binding'
updated_at = <Date 2011-07-20.23:13:47.515>
user = 'https://bugs.python.org/Jajcus'

    bugs.python.org fields:

    activity = <Date 2011-07-20.23:13:47.515>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2011-07-20.23:13:47.516>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-07-13.11:33:25.161>
creator = 'Jajcus'
dependencies = []
files = ['22646', '22651', '22652']
hgrepos = []
issue_num = 12551
keywords = ['patch']
message_count = 12.0
messages = ['140247', '140248', '140249', '140258', '140293', '140311', '140324', '140329', '140331', '140450', '140767', '140768']
nosy_count = 4.0
nosy_names = ['jcea', 'pitrou', 'Jajcus', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12551'
versions = ['Python 3.3']

    Activity

    1. Jajcus commented on Jul 13, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 13, 2011
      MannequinAuthor

      Recently IETF encourages using of the SCRAM-SHA-1-PLUS SASL authentication mechanism (5802) in new protocols. That is a requirement e.g. of the current XMPP specification (RFC6120). Any compliant implementation needs to support the 'SCRAM-SHA-1-PLUS' mechanism, and that requires obtaining the 'tls-unique' channel-binding data from a TLS connection used. Python doesn't provide this information and it seems the only detail stopping anyone from fully implementing XMPP or SCRAM-SHA-1-PLUS alone in Python.

      The 'tls-unique' channel binding is defined as:

      Description: The first TLS Finished message sent (note: the Finished
      struct, not the TLS record layer message containing it) in the most
      recent TLS handshake of the TLS connection being bound to

      …and is (they say), available via OpenSSL API. This should be exposed by the python SSLSocket object too.

      The other channel-binding data type, 'tls-server-end-point' can be computed using current Python API, but it is not enough for most uses ('tls-unique' is the required channel binding data in most cases) and still not trivial (one needs to ASN.1-decode the certificate to get the hash function name to compute proper digest).

    2. Jajcus
      added
      stdlibStandard Library Python modules in the Lib/ directory
      type-featureA feature request or enhancement
      on Jul 13, 2011

    3. pitrou commented on Jul 13, 2011

      @pitrou
      pitrou
      on Jul 13, 2011
      Member

      Interestingly (from rfc5929):

        This definition of 'tls-unique' means that a channel's bindings
  data may change over time, which in turn creates a synchronization
  problem should the channel's bindings data change between the time
  that the client initiates authentication with channel binding and
  the time that the server begins to process the client's first
  authentication message.  If that happens, the authentication
  attempt will fail spuriously.

      and is (they say), available via OpenSSL API

      Do you happen to know which API? I see no reference to tls-unique or channel binding, in either the OpenSSL website or the latest OpenSSL snapshot.

      According to some mailing-list message, we could use SSL_get_finished() and SSL_get_peer_finished(), but that still leaves us to figure out what to do with the info returned by these functions. It would be nice if there was some ready-to-use code (I'm not a crypto expert).

    4. Jajcus commented on Jul 13, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 13, 2011
      MannequinAuthor

      Do you happen to know which API?

      Not yet.

      I see no reference to tls-unique or channel binding, in either the OpenSSL website or the latest OpenSSL snapshot.

      Yes, I know it is not directly documented.

      It would be nice if there was some ready-to-use code (I'm not a crypto expert).

      _Maybe_ I will try to hack the python SSL code to make this work within my project. I will attach a patch here if it happens and gives any reusable results. As for now I cannot help any more, I am just reporting what is missing.

    5. Jajcus commented on Jul 13, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 13, 2011
      MannequinAuthor

      I skim-read the TLS specification, looked at the OpenSSL API and it seems it should be easy to implement. I am getting to work right now…

    6. Jajcus commented on Jul 13, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 13, 2011
      MannequinAuthor

      Here is a patch, ready for review. Seems to work, though I still need to check it with some other implementation.

      I have chosen not to expose another three OpenSSL functions (SSL_get_finished, SSL_get_peer_finished, SSL_session_reused), but provide API just for getting the channel binding. If OpenSSL provides a better API some day (gnutls already has a dedicated function), we can use that.

      The method added to SSLSocket - get_channel_binding() currently can return only the 'tls-unique' channel binding type, but can be easily extended for other types, which also may be easier to get from the C module.

    7. pitrou commented on Jul 13, 2011

      @pitrou
      pitrou
      on Jul 13, 2011
      Member

      Thank you, this looks mostly good.
      A couple of nits:

      +#if OPENSSL_VERSION_NUMBER >= 0x0090500fL
      +# define HAVE_OPENSSL_FINISHED 1
      +#else
      +# undef HAVE_OPENSSL_FINNISHED
      +#endif

      you have a typo in the #undef, also it would be more logical to have
      # define HAVE_OPENSSL_FINISHED 0
      instead.

      _ssl.c will not compile if OpenSSL is too old, because you lack some #if's (or #ifdef's) around PySSL_tls_unique_cb.

      Also, it would be nice to expose the availability of tls-unique as a public constant, as we already do for "ssl.HAS_SNI". ssl.HAS_TLS_UNIQUE?

      Similarly, you need to skip some of the tests when the functionality isn't available.
      And I think get_channel_binding() should raise NotImplementedError in that case.

    8. Jajcus commented on Jul 14, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 14, 2011
      MannequinAuthor

      Thanks for the quick review. Most of the problems are my oversights.

      I am not sure about that:

      And I think get_channel_binding() should raise NotImplementedError in that case.

      As the method is supposed to be extensible and 'tls-unique' may be just one of possible channel-binding types, then I think the same exception should be raised in case 'tls-unique' is requested and not implemented and when other, currently not implemented, channel binding type is requested. The get_channel_binding() method itself is always implemented, that is why I wonder if 'ValueError' is no better. Or 'NotImplementedError' for both 'tls-unique not implemented' and 'unknown channel binding'.

    9. Jajcus commented on Jul 14, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 14, 2011
      MannequinAuthor

      This is patch updated according to your suggestions, including raising NotImplementedError when 'tls-unique' is not available and with the ssl.HAS_TLS_UNIQUE constant added.

      It also includes an important fix to the data retrieval logic (one condition had to be reverted).

      Now the code is proven to work, by testing with another implementation (SCRAM-SHA-1-PLUS authentication in Isode M-Link 15.1a0).

      A alternative patch version will follow.

    10. Jajcus commented on Jul 14, 2011

      @Jajcus
      Jajcusmannequin
      on Jul 14, 2011
      MannequinAuthor

      This patch is functionally equivalent, but advertises 'tls-unique' support in a bit different way.

      HAS_TLS_UNIQUE is not exposed in the python 'ssl' module, instead a list 'CHANNEL_BINDING_TYPES' is provided (empty when 'tls-unique' is not supported). get_channel_binding raises ValueError if the argument is not on this list. This way the API can be extended to other channel binding types without adding new constants or functions. Adding a new channel binding type would not need any modifications in the API client code (if it is designed to use arbitrary cb types).

    11. pitrou commented on Jul 15, 2011

      @pitrou
      pitrou
      on Jul 15, 2011
      Member

      This patch is functionally equivalent, but advertises 'tls-unique'
      support in a bit different way.

      HAS_TLS_UNIQUE is not exposed in the python 'ssl' module, instead a
      list 'CHANNEL_BINDING_TYPES' is provided (empty when 'tls-unique' is
      not supported). get_channel_binding raises ValueError if the argument
      is not on this list. This way the API can be extended to other channel
      binding types without adding new constants or functions. Adding a new
      channel binding type would not need any modifications in the API
      client code (if it is designed to use arbitrary cb types).

      Thanks, this is a good idea. I'm trying to get advice on the
      openssl-users mailing-list about this and will commit if I don't get any
      contradicting info soon ;)

    12. python-dev commented on Jul 20, 2011

      @python-dev
      python-devmannequin
      on Jul 20, 2011
      Mannequin

      New changeset cb44fef5ea1d by Antoine Pitrou in branch 'default':
      Issue bpo-12551: Provide a get_channel_binding() method on SSL sockets so as
      http://hg.python.org/cpython/rev/cb44fef5ea1d

    13. pitrou commented on Jul 20, 2011

      @pitrou
      pitrou
      on Jul 20, 2011
      Member

      Patch is now committed. Thanks for your contribution!

    14. pitrou
      closed this as completedon Jul 20, 2011
    15. ezio-melotti
      transferred this issue fromon Apr 10, 2022

    16. Neustradamus commented on Jul 28, 2022

      @Neustradamus
      Neustradamus
      on Jul 28, 2022

      The commit is here by @Jajcus:

      For TLS 1.3 (the RFC has been released few days ago):

      @tiran has done it:

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      stdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions