openssl / openssl

TLS/SSL and crypto library
https://www.openssl.org
Apache License 2.0
25.59k stars 10.09k forks source link

Openssl renegotiation triggered by client failure #20891

Closed u279897 closed 1 year ago

u279897 commented 1 year ago

I create ssl_ctx , SSL structure and perform first handshake as follows:

From thread t1:

SSL_CTX *tls_ctx = SSL_CTX_new(TLS_client_method())));
// set configuration and callbacks

SSL_CTX_set_msg_callback(tls_ctx, tls_msg_clb);
SSL_CTX_set_cipher_list(tls_ctx, cipher_list);
SSL_CTX_set_ciphersuites(tls_ctx, cipher_suites);
SSL_CTX_set_min_proto_version(tls_ctx, min_proto_version);
SSL_CTX_set_max_proto_version(tls_ctx, max_proto_version);
SSL_CTX_set_verify(tls_ctx, SSL_VERIFY_PEER, tlsauth_verify_callback);
// get peer certificate

SSL_CTX_use_certificate_file(tls_ctx, ..)
SSL_CTX_use_PrivateKey_file(tls_ctx, ..)
SSL_CTX_load_verify_locations(tls_ctx, ..)

SSL *tls = SSL_new(tls_ctx);
SSL_set_fd(tls, sock);

SSL_set_mode(tls, SSL_MODE_AUTO_RETRY);

/* perform the  first handshake */
    int ret;
    int waitMax = 10; // wait for max 10 seconds.
    while (((ret = SSL_connect(tls)) == -1) and (SSL_get_error(tls, ret) == SSL_ERROR_WANT_READ)) {
        usleep(100); 
        if ((time(nullptr) - startTime) >= waitMax)
        {
                std::cout<< " failed due to timeout"; 
                return;
        }
       if (ret != 1)
       {
           std::cout<< " ret !=1";  // **2
           return;
      }

     if (not verifyServerCertificate(tls))
     {
           std::cout<< " failed to verify server ceritiface"; // I never encountered this.
          return;
     }

/ I have a session succesfully established over TLSv1.2 with following openssl version: OPENSSL_VERSION_TEXT: OpenSSL 3.0.8 7 Feb 2023 After a certain period I want to trigger renegotiation from client side as follows /

I send application data and trigger the renegotiation from Thread t2:

int ret;
if ((ret = SSL_renegotiate(tls)) <= 0)
{
    std::cout<< "failed with SSL_renegotiate"; // never fails here
    return;
}

int ssl_get_error;
int waitMax = 10; // wait for max 10 seconds.
while ((ret = SSL_connect(tls)) != 1)
{
    ssl_get_error = SSL_get_error(tls, ret);
    if (ssl_get_error != SSL_ERROR_WANT_READ)
    {
          std::cout << " Failure ssl_get_error: " << ssl_get_error;
          break;
    }
    if ((time(nullptr) - startTime) >= waitMax)
    {
         std::cout<< " failed due to timeout"; // **1
         return;
    }
}

if (ret != 1)
{
    std::cout<< " ret !=1";  // **2
    return;
}

if (not verifyServerCertificate(tls))
{
     std::cout<< " failed to verify server ceritiface"; // I never encountered this.
     return;
}

I trigger the renegotiation following 3-4 seconds SSL structure is created, in that time application succesfully sends data using tls. It sometimes works and renegotiation is performed even multiple times succesfully but sometimes it doesn't. I usually encounter timeout or the 10 seconds ends without SSL_connect succeding with following SSL_get_error outputs:


SSL_get_error: 2 with timeout // most frequent // **1
SSL_get_error: 1 -> SSL_ERROR_SSL: cipher operation failed/wrong version number // least frequent // **2
SSL_get_error: 1 -> SSL_ERROR_SSL: unknown error

I basically just use SSL_renegotiate and SSL_connect which runs max 10 seconds until it keep receiving SSL_ERROR_WANT_READ. My SSL server is a pyton tls socket server

What might be wrong with my code, how could it be improved? How could I find out why it doesn't work sometimes ?

t8m commented 1 year ago

It is not fully clear to me from the description - are you trying to use the same tls object from multiple threads? If so, you would have to properly lock the access to the object as simultaneous calls on a single object from multiple threads will cause race conditions and memory corruption.

u279897 commented 1 year ago

thanks for the clue, SSL object is created on thread t1, application data is sent on thread T2, and renegotiation is triggered on main thread, with potential data race between T2 and main thread.

u279897 commented 1 year ago

@t8m I updated the issue description with more details regarding the settings and using of SSL object, Im not using it anymore from multiple threads but I still have the same issue, could you please give any hint what might be still wrong ? Is there an alternative solution to perform the renegotiation that might work ?

t8m commented 1 year ago

You should not call SSL_connect() after SSL_renegotiate(). SSL_renegotiate() should be called on an already connected SSL object.

u279897 commented 1 year ago

Thank you @t8m, I think it works using only SSL_renegotiate(). As far as I understand it will schedule a renegotiation and it will be performed automatically so no need of explicit call of SSL_connect()/SSL_do_handshake when client wants to renegotiate over existing TLS connection -> please confirm

  1. How could I know when the renegotiation is done in order to validate again the server certificate ?
  2. what will happen if the renegotiation fails, how is the application layer supposed to know about the failure ?
t8m commented 1 year ago

You can use SSL_renegotiate_pending() to check whether the renegotiation is still pending or it was done. AFAIK if the renegotiation fails, the SSL connection will end up in an error state so it will not be possible to do further reads/writes through it.

u279897 commented 1 year ago

@t8m, I want to use a dummy SSL_write_ex to trigger renegotiation following SSL_renegotiate, then wait until SSL_renegotiate_pending returns 1, at the end of it a new dummy SSL_write_ex to see whether renegotiation was succesful or not, but the SSL_write_ex meant to trigger renegotiation returns < 1.

 ```
         // triggger renegotiation
        int res = SSL_write_ex(tls, (NULL), 0, 0);
        if (res < 1)
        {
            std::cout<< " Renegotiate trigger SSL_write failed";  // fails here
        }

        // wait until SSL_renegotiate_pending is 1 max 10s

        // c
        res = SSL_write_ex(sslPtr, (NULL), 0, 0);
        if (res < 1)
        {
            std::cout << " Caannot write in SSL object meaning renegotiation failed!";
            return; 
        }
       // renegotiation ended succesfull I can verify server certificate

Could you please tell me it this approach can be used and if so why the first SSL_write_ex returns < 1 ? 
Thank you!
t8m commented 1 year ago

Do you use non-blocking sockets? If so, the SSL_write() might return until there is the handshake data from the peer on the socket.

u279897 commented 1 year ago

yes, I use non-blocking sockets, is any other way to trigger the renegotiation right after it was scheduled instead of waiting for an SSL_write which is called for application data ?

t8m commented 1 year ago

SSL_do_handshake() can do it. However with non-blocking sockets you still need a loop to allow for the handshake data to arrive from the peer.

u279897 commented 1 year ago

how could I do that, allowing the handshake data to arrive from the peer ? is any opensource project example for that ?

t8m commented 1 year ago

how could I do that, allowing the handshake data to arrive from the peer ? is any opensource project example for that ?

You have a SSL_connect() loop in you opening comment in this issue, this would be very much similar using SSL_do_handshake() instead of SSL_connect().

u279897 commented 1 year ago

@t8m I did try also with SSL_do_handshake instead of SSL_connect but it doesn't work, Do I need to add more changes to it ? Thanks.

if ((ret = SSL_renegotiate(tls)) <= 0)
{
    std::cout<< "failed with SSL_renegotiate";
    return;
}

int ssl_get_error;
int waitMax = 10; // wait for max 10 seconds.
while ((ret = SSL_do_handshake(tls)) != 1)
{
    ssl_get_error = SSL_get_error(tls, ret);
    if (ssl_get_error != SSL_ERROR_WANT_READ)
    {
          std::cout << " Failure ssl_get_error: " << ssl_get_error;
          break;
    }
    if ((time(nullptr) - startTime) >= waitMax)
    {
         std::cout<< " failed due to timeout"; // **1
         return;
    }
}

if (ret != 1)
{
    std::cout<< " failed";
    return;
}
t8m commented 1 year ago

What do you get returned from the SSL_get_error() call?

u279897 commented 1 year ago

using select on SSL fd for read and write based on SSL_get_error() solved my issue