[QUESTION] MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY treated as error

[mitchellhansen-wyze]

Hello!

I have a program that integrates this library. When calling the final Kvs_putMediaDoWork in a stream, it will receive a -0x7880:MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY error from the mbedtls layers.

my_program.c

        if (Kvs_putMediaDoWork(kvs->put_media_handle) != KVS_ERRNO_NONE)
        {
            LOG_ERROR("Failed in Kvs_putMediaDoWork");
            ret = EXIT_FAILURE;
            break;
        }

netio.c @ line 311

        n = mbedtls_ssl_read(&(pxNet->xSsl), pBuffer, uBufferSize);
        if (n < 0 || n > uBufferSize)
        {
            LogError("SSL recv error %d", n);
            xRes = KVS_ERRNO_FAIL;
        }

The error message from KVS:

 File:/.../src/source/net/netio.c Func:NetIo_recv Line:314 SSL recv error -30848
Error: Time:Mon Jul 11 22:29:13 2022
 File:/.../src/source/restful/kvs/restapi_kvs.c Func:Kvs_putMediaDoWork Line:1216 Failed to receive

Discussion about this specific mbedtls API : https://github.com/Mbed-TLS/mbedtls/issues/2173

My program will attempt to reinitialize the library when Kvs_putMediaDoWork returns bad error codes. But this MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY error (according to the linked discussion) is not a fatal error, and should not affect program behaviour.

Tweaking this function to :

        n = mbedtls_ssl_read(&(pxNet->xSsl), pBuffer, uBufferSize);
        if (n == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY)
        {
            *puBytesReceived = n;
        }
        else if (n < 0 || n > uBufferSize)
        {
            LogError("SSL recv error %d, %d", n, MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY);
            xRes = KVS_ERRNO_FAIL;
        }
        else
        {
            *puBytesReceived = n;
        }

Stops the error from occurring, and lets my program re-initialize only when there are fatal errors within the function.

Though oddly enough, this now exposes another issue, in which mbedtls_ssl_read will return -76 (MBEDTLS_ERR_NET_RECV_FAILED) on occasion, again causing my code to re-initialize the module

 File:/.../src/source/net/netio.c Func:NetIo_recv Line:318 SSL recv error -76
Error: Time:Mon Jul 11 23:47:15 2022
 File:/.../src/source/restful/kvs/restapi_kvs.c Func:Kvs_putMediaDoWork Line:1216 Failed to receive

Is this change (to ignore MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY) an acceptable change to upstream to this repo? And, has any other user of this repo run into these problems?

[weichihl]

@mitchellhansen-wyze From my understanding, you don't want your program to re-initialize when a MbedTLS error happens in Kvs_putMediaDoWork, right? There are several possible MbedTLS error codes while error happens in mbedtls_ssl_read, and MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY is one of them. So even if we don't treat MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY as an error, other actual MbedTls errors still cause your program re-initialize. In my opinion, I tend to cache the error code (Ex. MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY) and the error type (Ex. MbedTLS error) in the KVS library, and then the application can get it and decide what to do with the error. Is that sound good to you?

[mitchellhansen-wyze]

@weichihl

From my understanding, you don't want your program to re-initialize when a MbedTLS error happens in Kvs_putMediaDoWork, right?

More or less, yes. Mainly I need Kvs_putMediaDoWork to either return only fatal errors, or return a more descriptive error code than just a KVS_ERRNO_NONE or KVS_ERRNO_FAIL.

I like your suggestion, where Kvs_putMediaDoWork would return the same error codes, but the underlying error would be cached in some sort of errno like structure. Then I can query it, and decide on my end whether or not the error was fatal.

Are you maintaining this repository @weichihl? is it possible that this improvement could be applied?

[weichihl]

@mitchellhansen-wyze Yeah, I can make the change. I'll draft the solution and attach the PR here once it's available.

[mitchellhansen-wyze]

Excellent, thank you @weichihl!

[weichihl]

@mitchellhansen-wyze I've merged the PR. Please check if this helps, and feel free to reopen this issue if you need more help.