New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect return value in ssl_write_real() #839

Closed
davidwu2000 opened this Issue Mar 7, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@davidwu2000

davidwu2000 commented Mar 7, 2017

I have a simple fix (below) but could not push to repository

commit f6ac560b040b99b4f9bd120e7477ad85461607d3
Author: David Wu <davidwu@arcturusnetworks.com>
Date:   Tue Mar 7 14:42:21 2017 -0500

    fix the incorrect return value in ssl_write_real()

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index abad0b3..d680b97 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -6796,8 +6796,8 @@ static int ssl_write_real( mbedtls_ssl_context *ssl,
         if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 )
         {
             MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flush_output", ret );
-            return( ret );
         }
+        return( ret );
     }
     else
     {
@davidwu2000

This comment has been minimized.

davidwu2000 commented Mar 7, 2017

here is the function with the fix -

static int ssl_write_real( mbedtls_ssl_context *ssl,
                           const unsigned char *buf, size_t len )
{
    int ret;
#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH)
    size_t max_len = mbedtls_ssl_get_max_frag_len( ssl );

    if( len > max_len )
    {
#if defined(MBEDTLS_SSL_PROTO_DTLS)
        if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
        {
            MBEDTLS_SSL_DEBUG_MSG( 1, ( "fragment larger than the (negotiated) "
                                "maximum fragment length: %d > %d",
                                len, max_len ) );
            return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
        }
        else
#endif
            len = max_len;
    }
#endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */

    if( ssl->out_left != 0 )
    {
        if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 )
        {
            MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flush_output", ret );
        }
        return( ret );
    }
    else
    {
        ssl->out_msglen  = len;
        ssl->out_msgtype = MBEDTLS_SSL_MSG_APPLICATION_DATA;
        memcpy( ssl->out_msg, buf, len );

        if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 )
        {
            MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret );
            return( ret );
        }
    }

    return( (int) len );
}
@sbutcher-arm

This comment has been minimized.

Collaborator

sbutcher-arm commented Mar 11, 2017

Your fix does look right. Thanks we'll look into this.

@ciarmcom

This comment has been minimized.

Member

ciarmcom commented Mar 13, 2017

ARM Internal Ref: IOTSSL-1255

@sbutcher-arm

This comment has been minimized.

Collaborator

sbutcher-arm commented Jun 29, 2018

We've updated the documentation to clarify the correct usage of the API, in PR #1006 for development, and backported it to #1768 for mbedtls-2.7 and #1769 for mbedtls-2.1.

We believe this explains the correct behaviour and therefore I'm closing this issue.

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