5

The purpose of the following portion of code is to poll on a socket fd-set and if data (ssl encrypted) is available, read it and decrypt it by openssl library. The underlying transport layer is TCP Stream, so data comes as stream (not packet).

Now, if more than one packets (lets assume 2 packets of length 85 bytes) are sent in quick succession from the peer, then TCP receive will return both the packets in same buffer with Number of Bytes received as 170. So, we have one buffer that carries 2 ssl encrypted packets (or n number of packets). For ssl decryption, we need to call BIO_write() to write the buffer into ssl_bio and then ssl_read() to retrieve the decrypted buffer. But though BIO_write() is writing 170 bytes into the bio, it seems like ssl_read() is only returning one decrypted packet (43 bytes). There is no error returned. How to know if there is still unprocessed bytes in the bio. Is there any way out or is there any bug in the code?

The code is working fine when single packets are received in tcp recv().

int iReadyFds = poll( PollFdSet, iFdCount, iTimeout);

for(iFdIndx = 0; iFdIndx < (iFdCount) && (iReadyFds>0); ++iFdIndx)
{
    if((PollFdSet[iFdIndx].events == 0) ||
       (PollFdSet[iFdIndx].fd == 0) ||
       (PollFdSet[iFdIndx].revents != POLLIN)
       )
    {
        continue;
    }

    /* we have data to read */
    int iMsgLen = 0;
    int iFd = PollFdSet[iFdIndx].fd;


    /*This is TCP Receive. Returns 170 bytes*/
    iRcvdBytes = recv( iSocketId, ( void* )pcInBuffer, PN_TCP_STREAM_MAX_RX_BUFF_SIZE, 0 );
    
    /*Writing into SSL BIO, this will be retrieved by ssl_read*/
    /*iNoOFBytes  = 170*/
    iNoOFBytes = BIO_write(m_pRead_bio, pcInBuffer, iRcvdBytes);

    if(iNoOFBytes <= 0)
    {
        printf("Error");
        return -1;
    }
    
    char* pcDecodedBuff = (char*)malloc(1024);
    
    /*here it returns 43 bytes of decrypted buffer(1 packet). the other packet vanishes*/
    iReadData = SSL_read(m_psSSL, pcDecodedBuff, 1024);

    if ((iReadData == -1) || (iReadData == 0))
    {
        error = SSL_get_error(psPskTls->m_psSSL, iReadData);

        if(error == SSL_ERROR_ZERO_RETURN
        || error == SSL_ERROR_NONE
        || error == SSL_ERROR_WANT_READ)
        {
            printf("Error");
        }
    }

    iReadyFds--;
}
Ammar Faizi
  • 1,393
  • 2
  • 11
  • 26
AmiyaG
  • 170
  • 2
  • 7

3 Answers3

5

OpenSSL will, normally, just read and decrypt one record at a time. Calling SSL_read again will give you the next record. If you don't know if there is another record to read you can ask the underlying transport if it is currently "readable" - or just call SSL_read() anyway and handle the error (if using non-blocking IO).

In some circumstances (such as if you are using the "read_ahead" capability), OpenSSL may buffer some data internally. You can find out if there is buffered internal data using SSL_has_pending() (for OpenSSL 1.1.0) or SSL_pending() (for OpenSSL 1.0.2+). See https://www.openssl.org/docs/man1.1.0/ssl/SSL_has_pending.html.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
Matt Caswell
  • 8,167
  • 25
  • 28
  • Good job on linking to the reference. I took the liberty of splitting the paragraph for readability. – David C. Rankin Mar 10 '17 at 14:32
  • Thanks Matt for such a quick reply. It looks like SSl_pending() is what i was looking for. I'll comeback after checking it in my code. Thanks again for the help. – AmiyaG Mar 11 '17 at 08:10
  • unfortunately, SSL_pending() hasn't worked out. But Matt's suggestion of multiple ssl_read() worked in this case. I had that solution with me, but was reluctant to use it as the requirement was of high speed data transaction and making unnecessary ssl_read() would cost me precious time. But now it seems there is no other way around. – AmiyaG Mar 18 '17 at 06:13
  • Most people just do a "poll" or "select" or similar on the underlying transport to see if it is readable first. – Matt Caswell Mar 18 '17 at 11:31
  • Matt, I've done a poll and got a socket that has something to read. I've done a TCP recv() to read the whole data and it returned n number of packets together. I'm writing them in ssl bio, but when reading them back ssl_read() only returning one processed buffer and there is no indication if there is anything left to process and read from bio. That was the problem. using a poll at this point is pointless, as it already returned everything it got. – AmiyaG Mar 18 '17 at 18:02
  • 1
    Ah, ok. Typically the read and write BIOs connected to the SSL object read and write directly from the network. Re-reading your original code sample I now see a subtlety I had originally missed. You are reading data off the network and then pushing it into the read BIO to be subsequently picked up by the SSL object. So I assume your read BIO is actually a mem BIO?? If so then you can use BIO_pending() on the read BIO to see if it still has readable data left for the SSL object to pick up. – Matt Caswell Mar 19 '17 at 11:14
  • Just a clarification, if I don't use read ahead, can I expect SSL_write() to write one record on the wire and on the other side SSL_read() to read one record? Could this assumption be broken in the future? My experiments show that SSL_read reads one record and SSL_write writes one record. – zapstar Jan 10 '18 at 12:00
  • Just an addition to previous comment: To be more specific, for writes I would be disabling partial writes and read will pass in a bigger buffer than the record size. – zapstar Jan 10 '18 at 12:18
  • SSL_write() will write as many records as it needs to to write out the data you are trying to send. If it won't fit inside one record (16k max) then it will use two (or more). SSL_read() typically only reads one record at a time but there are various scenarios where this might not hold (read_ahead is one of those but there are others). The dev version has a capability to negotiate the max record size to something smaller during the handshake so this could make a difference too (e.g. for writes). In short I don't think you can rely on your assumption. – Matt Caswell Jan 10 '18 at 19:20
4

Well after a few experiments and reading openssl documents, I've been able to resolve the issue. To my understanding, at reasonably high speed(more than 1000 application data transactions over ssl connection per second) this issue will certainly occur in cases where asynchronous openssl implementations are used.

Coming to the answer, according to openssl implementation -

  1. There are two bios associated with a ssl connection(or context) - read bio and write bio
  2. read bio is where BIO_write() writes received buffer and from which the successive ssl_read() reads unprocessed encrypted buffer and decrypts it into plain text.
  3. write bio is where ssl_write(), upon taking a plain text and encrypting it, puts the encrypted buffer. BIO_read() reads from this bio and the buffer BIO_read() returns is ready for sending into the network.

As this question is related to ssl_read(), we are continuing our discussion from 2nd point above. To my understanding, ssl_read() actually executes openssl's internal fsm that reads and processes buffer from read_bio one packet at a time. The rest of the buffer(if any), rests in the read bio as unprocessed buffer. So, ssl_pending() will never return anything, as there is no processed buffer left to read. The only way to find out if anything is left to process and read in the bio is making successive ssl_read() calls until it returns 0 bytes received.

The modified code should look something like this -

int iReadyFds = poll( PollFdSet, iFdCount, iTimeout);

for(iFdIndx = 0; iFdIndx < (iFdCount) && (iReadyFds>0); ++iFdIndx)
{
    if((PollFdSet[iFdIndx].events == 0) ||
       (PollFdSet[iFdIndx].fd == 0) ||
       (PollFdSet[iFdIndx].revents != POLLIN)
       )
    {
        continue;
    }

    /* we have data to read */
    int iMsgLen = 0;
    int iFd = PollFdSet[iFdIndx].fd;


    /*This is TCP Receive. Returns 170 bytes*/
    iRcvdBytes = recv( iSocketId, ( void* )pcInBuffer, PN_TCP_STREAM_MAX_RX_BUFF_SIZE, 0 );

    /*Writing into SSL BIO, this will be retrieved by ssl_read*/
    /*iNoOFBytes  = 170*/
    iNoOFBytes = BIO_write(m_pRead_bio, pcInBuffer, iRcvdBytes);

    if(iNoOFBytes <= 0)
    {
        printf("Error");
        return -1;
    }

    char* pcDecodedBuff = (char*)malloc(1024);

    /*here it returns 43 bytes of decrypted buffer(1 packet). 
    So we keep on reading until all the packets are processed and read*/
    while(iReadData = SSL_read(m_psSSL, pcDecodedBuff, 1024) > 0)
    {
        doSomething(pcDecodedBuff, iReadData);**
    }

    if ((iReadData == -1) || (iReadData == 0))
    {
        error = SSL_get_error(psPskTls->m_psSSL, iReadData);

        if(error == SSL_ERROR_ZERO_RETURN
        || error == SSL_ERROR_NONE
        || error == SSL_ERROR_WANT_READ)
        {
            printf("Error");
        }
    }

    iReadyFds--;
}

Check how successive ssl_read() has been used to make sure there is no unprocessed data left in the read_BIO.

while(iReadData = SSL_read(m_psSSL, pcDecodedBuff, 1024) > 0) { doSomething(pcDecodedBuff, iReadData); }

Using this code, the problem I was facing got solved. Hope it will help others too.

AmiyaG
  • 170
  • 2
  • 7
  • 1
    This is still incorrect. Firstly you are still waiting for SSL_read to return, only this time in a loop. It may or may not always return. Also this statement `while(iReadData = SSL_read(m_psSSL, pcDecodedBuff, 1024) > 0)` is totally incorrect logic. Should be `while((iReadData = SSL_read(m_psSSL, pcDecodedBuff, 1024)) > 0)` not that this solves the issue entirely. –  Oct 05 '21 at 19:00
  • Do you mind please showing how you are init your BIO objects? Also how are you init the socket? –  Oct 05 '21 at 20:33
  • SSL_read() is not itself a blocking call. Whether it blocks or not entirely depends on the underlying BIO. The BIO can be either blocking or non-blocking. If your BIO is non blocking, SSL_read() will retrun immediately with number of bytes read. Please do remember, the error handling in this code snippet is just an example. It may not be an error if there is nothing to read for multiple reads. – AmiyaG Oct 08 '21 at 03:26
3

I'm also using memory BIO's to use SSL with non-blocking sockets (using poll). The code I ended up with looks similar to your solutions, but with one extra step; after the call to SSL_get_error you need to check if the SSL has requested a write operation. An SSL_read can result in a the SSL object needing to perform a socket write; this can happen if the peer requested renegotiaton.

Below is a snippet from the full code at ssl_server_nonblock.c ... its the part perform the BIO_write and SSL_read

/* Process SSL bytes received from the peer. The data needs to be fed into the
   SSL object to be unencrypted.  On success returns 0, on SSL error -1. */
int on_read_cb(char* src, size_t len) {
char buf[DEFAULT_BUF_SIZE]; /* used for copying bytes out of SSL/BIO */
enum sslstatus status;
int n;

while (len > 0) {
  n = BIO_write(client.rbio, src, len);

  if (n<=0)
    return -1; /* if BIO write fails, assume unrecoverable */

  src += n;
  len -= n;

  if (!SSL_is_init_finished(client.ssl)) {
    n = SSL_accept(client.ssl);
    status = get_sslstatus(client.ssl, n);

    /* Did SSL request to write bytes? */
    if (status == SSLSTATUS_WANT_IO)
      do {
        n = BIO_read(client.wbio, buf, sizeof(buf));
        if (n > 0)
          queue_encrypted_bytes(buf, n);
        else if (!BIO_should_retry(client.wbio))
          return -1;
      } while (n>0);

    if (status == SSLSTATUS_FAIL)
      return -1;

    if (!SSL_is_init_finished(client.ssl))
      return 0;
  }

  /* The encrypted data is now in the input bio so now we can perform actual
   * read of unencrypted data. */

  do {
    n = SSL_read(client.ssl, buf, sizeof(buf));
    if (n > 0)
      client.do_something(buf, (size_t)n);
  } while (n > 0);

  status = get_sslstatus(client.ssl, n);

  /* Did SSL request to write bytes? This can happen if peer has requested SSL
   * renegotiation. */
  if (status == SSLSTATUS_WANT_IO)
    do {
      n = BIO_read(client.wbio, buf, sizeof(buf));
      if (n > 0)
        queue_encrypted_bytes(buf, n);
      else if (!BIO_should_retry(client.wbio))
        return -1;
    } while (n>0);

  if (status == SSLSTATUS_FAIL)
    return -1;
}

Darren Smith
  • 2,261
  • 16
  • 16
  • Perform write operation on what data precisely? `SSL_read()` does renegotiation implicitly. –  Oct 05 '21 at 20:05