Synss / python-mbedtls

Cryptographic library with an mbed TLS back end
MIT License
79 stars 28 forks source link

Renegotiate Issues #49

Closed Jnetops closed 2 years ago

Jnetops commented 2 years ago

NOTE: Please use stackoverflow for support questions. This repository's issues are reserved for feature requests and bug reports.

I am submitting a …

Description

Currently, I am trying to initiate a DTLS handshake in which the server handles things in a very particular way. The flow is this: Client Hello (client) Hello Verify Request (server) Client Hello + cookie (client) Hello Request (server) Client Hello #no cookie# (client) Server Hello etc...

I believe what I need access too is the ability to set this to 1 _tls.mbedtls_ssl_conf_renegotiation(&self._ctx, 0) line 446 tls.pyx

Current behavior

Renegotiation is disabled and I believe that is what I need enabled. I would also love if the bindings for mbedtls legacy negotiation options where accessible

Expected behavior

Allow users to modify these values when setting up a context for tls or dtls

Steps to reproduce

Using default DTLS config provided in docs, works as expected until renegotiation occurs.

Other information

If this is something that is already available, please provide info on how to achieve this, thank you in advance.

Jnetops commented 2 years ago

If this is a stupid bug report I apologize. If this functionality exists please, if you don't mind, advise me on how I would use it. Otherwise, if you need further information I can submit anything you need for a use case, including pcap of how a typical handshake with this particular server occurs vs how this library currently handles it etc.

Synss commented 2 years ago

Hi @Jnetops! No need to apologize. 😉 I will have a look at this in the next few days.

Jnetops commented 2 years ago

OK no problem. The error I get is a WantReadError() with the current state being HandshakeStep.SERVER_HELLO. This is totally understandable since once a CLIENT_HELLO is sent with cookie, the expectation is a SERVER_HELLO should follow. If there was a way to manually set state to HELLO_REQUEST so it attempts to read the right state, or ability to trigger renegotiation on HELLO_REQUEST etc etc that would be super.

Synss commented 2 years ago

It looks like mbedtls_ssl_conf_renegotiation() is indeed what you need but, according to upstream documentation, the option is dangerous and easily misused, as you might have seen already

If you don't need renegotiation, it's probably better to disable it, since it has been associated with security issues in the past and is easy to misuse/misunderstand.

https://github.com/ARMmbed/mbedtls/blob/master/include/mbedtls/mbedtls_config.h#L1428-L1432

It is recommended to always disable renegotation unless you know you need it and you know what you're doing. In the past, there have been several issues associated with renegotiation or a poor understanding of its properties.

https://github.com/ARMmbed/mbedtls/blob/master/include/mbedtls/ssl.h#L3333-L3336

Actually, I realize https://github.com/Synss/python-mbedtls/blob/master/src/mbedtls/tls.pyx#L446 is useless in my code and I will remove the line. I think it is best if you patch the library yourself and make sure that

    # https://github.com/Synss/python-mbedtls/blob/master/src/mbedtls/tls.pxd#L305-L307
    void mbedtls_ssl_conf_renegotiation(
        mbedtls_ssl_config *conf,
        int renegotiation)

remains as-is and

           # https://github.com/Synss/python-mbedtls/blob/master/src/mbedtls/tls.pyx#L446
           _tls.mbedtls_ssl_conf_renegotiation(&self._ctx, 0)

is either set with a parameter or set to 1

           # or if you want to always enable renegotiation, which is not recommended
           _tls.mbedtls_ssl_conf_renegotiation(&self._ctx, 1)