Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.25k stars 2.56k forks source link

Provided UDP socket library does not support multiple concurrent clients #1845

Open Wizzard033 opened 6 years ago

Wizzard033 commented 6 years ago

Bug (affects net_sockets.h only)

OS
Tested on Windows 10, but confirmed to not exist as a bug on Linux

mbed TLS build:
Version: 2.8.0

Description
If a UDP server accepts one client, the net_sockets.c UDP code assumes that all future incoming data will be from that one client. However, this makes it so other clients cannot connect to the server. I discovered this bug when attempting to have multiple clients connect to my UDP server at the same time, i.e. 2 concurrent connections.

It seems that the library doesn't even offer support for concurrent UDP connections as of now, despite it looking as if effort was made for it to. To fix this, the library must identify the source of the packet, and only read packets that are from the appropriate source for the given net context. This is accomplished by using "recvfrom" and "sendto" as opposed to "read" and "write".

I do not think this is a simple change for the net_sockets library. It would likely have to have its own form of UDP session management where either the user or the library itself detects client connection/disconnection. This is necessary so that the library can attain/clear the source(s) that it is expecting packets from.

For now, I'll just be using a different UDP socket library that supports multiple concurrent UDP connections. If I am misunderstanding the usage of the library, which I hope is the case, then feel free to correct me. However, as far as I can see, it just seems to be a quick'n'dirty test environment setup for DTLS. It does not actually work as a fully-featured UDP socket library on Windows 10.

Wizzard033 commented 6 years ago

I think the conversation in issue https://github.com/ARMmbed/mbedtls/issues/204 is relevant.

EDIT: The following seems unlikely due to example working

Here's some additional reading as well about the specific issue encountered by the library: https://stackoverflow.com/questions/5077887/so-reuseaddr-and-udp-behavior-in-windows

Wizzard033 commented 6 years ago

Updated the original post to fix a misconception I had

RonEld commented 6 years ago

@Wizzard033 Thank you for reporting this issue!

Why do you consider this a bug, and not a feature request? IMHO, this is an enhancement to current behavior.Please correct me it I am missing something.

Wizzard033 commented 6 years ago

There was effort made to keep the binding socket different than the client socket. This indicates that multiple concurrent client connections was thought about. It made me believe for a long time that I was doing something wrong until I debugged the library itself to find the problem. This is also exactly the type of thing that should be denoted too, i.e. the library specifically state that it isn't possible to connect multiple UDP clients on the same port concurrently. This is the first socket library I've seen that doesn't support this, and nowhere in the documentation does it seem to indicate it.

RonEld commented 6 years ago

Thanks, I agree that it should be denoted. Perhaps it's mostly a documentation bug. Nonetheless, it should be fixed. Thanks again!

Wizzard033 commented 6 years ago

On a side note: a server that rejects all connection attempts until the current client has disconnected isn't very useful. I imagine this makes the UDP socket library unusable for most DTLS projects.

ciarmcom commented 6 years ago

ARM Internal Ref: IOTSSL-2393

mpg commented 6 years ago

@Wizzard033 Thanks for your report! I agree with your analysis that the code makes attempts to make this work, so much that in fact I think it works for me on Linux.

Steps to reproduce

void read_puts( mbedtls_net_context *ctx ) { unsigned char buf[1024]; int ret;

memset( buf, 0, sizeof( buf ) );
ret = mbedtls_net_recv( ctx, buf, sizeof( buf ) );
printf( "received %d bytes:\n", ret );
assert( ret > 0 );
assert( (size_t) ret < sizeof( buf - 1 ) );
puts( (const char *) buf );

}

int main( void ) { mbedtls_net_context listen_fd, client1_fd, client2_fd;

mbedtls_net_init( &listen_fd );
mbedtls_net_init( &client1_fd );
mbedtls_net_init( &client2_fd );

assert( mbedtls_net_bind( &listen_fd, "0.0.0.0", "4433", MBEDTLS_NET_PROTO_UDP ) == 0 );

assert( mbedtls_net_accept( &listen_fd, &client1_fd, NULL, 0, NULL ) == 0 );
assert( mbedtls_net_accept( &listen_fd, &client2_fd, NULL, 0, NULL ) == 0 );

// order reversed on purpose
read_puts( &client2_fd );
read_puts( &client1_fd );

mbedtls_net_free( &listen_fd );
mbedtls_net_free( &client1_fd );
mbedtls_net_free( &client2_fd );

}

- compile it and run it:
```sh
make lib && gcc -Wall -Wextra -O1 -Iinclude udp_server.c library/libmbedtls.a -o udp_server && ./udp_server

Expected result

The terminal in which udp_server is running prints:

bar
foo

This shows that the two connections were accepted and the packets delivered to the correct connection (the server reads in a different order than it accepted in order to illustrate that).

Questions

  1. Do you agree that this shows the code is working as intended on Linux?
  2. Can you provide us more details about what you tried on Windows and how it fails? Ideally full steps so that we can try and reproduce ourselves.

In the meantime, you should be able to use any other socket library with our DTLS stack.

Wizzard033 commented 6 years ago

EDIT: The following seems unlikely due to example working

I'm going to give that example a try for sure, but just wanted to type up my theory of what is happening. In Winsock, the first socket to bind to a unicast address is the one that gets all the packets from it. If you bind 2 sockets to the same unicast (not broadcast address) then only the first bound socket receives anything at all. This includes when you use SO_REUSEADDR. I'm talking UDP here, so don't apply the same logic to TCP connections.

Anyways, when connect is called on the original binding socket as it is transformed into the client socket, then the socket begins to filter out any messages that aren't from the connected client. However, when the client socket is closed, then suddenly the newer binding socket is the socket that receives packets again. This is because it's the most recently bound socket to the unicast address. I tested this pretty thoroughly already in my own little code. One thing: both my personal Winsock tests and mbed TLS test were non-blocking, and so I think it's worth a shot to try your code too. I'll get to it soon.

Wizzard033 commented 6 years ago

I ran the example and it seems to work on Windows. I also changed it to non-blocking and it seems to work as well. Back to debugging I guess. All my original tests involve sending+receiving, via the full DTLS handshake.

#include <assert.h>
#include <stdio.h>
#include <string.h>
#include <mbedtls/net_sockets.h>

void read_puts( mbedtls_net_context *ctx )
{
    unsigned char buf[1024];
    int ret;

    memset( buf, 0, sizeof( buf ) );
    do {
        ret = mbedtls_net_recv( ctx, buf, sizeof( buf ) );
    } while (ret == MBEDTLS_ERR_SSL_WANT_READ);
    printf( "received %d bytes:\n", ret );
    assert( ret > 0 );
    assert( (size_t) ret < sizeof( buf ) - 1 );
    puts( (const char *) buf );
}

int main( void )
{
    mbedtls_net_context listen_fd, client1_fd, client2_fd;
    int ret;

    mbedtls_net_init( &listen_fd );
    mbedtls_net_init( &client1_fd );
    mbedtls_net_init( &client2_fd );

    assert( mbedtls_net_bind( &listen_fd, "0.0.0.0", "4433", MBEDTLS_NET_PROTO_UDP ) == 0 );

    mbedtls_net_set_nonblock ( &listen_fd );
    do {
        ret = mbedtls_net_accept( &listen_fd, &client1_fd, NULL, 0, NULL );
    } while (ret == MBEDTLS_ERR_SSL_WANT_READ);
    assert( ret == 0 );
    mbedtls_net_set_nonblock ( &client1_fd );
    mbedtls_net_set_nonblock ( &listen_fd );
    do {
        ret = mbedtls_net_accept( &listen_fd, &client2_fd, NULL, 0, NULL );
    } while (ret == MBEDTLS_ERR_SSL_WANT_READ);
    assert( ret == 0 );
    mbedtls_net_set_nonblock ( &client2_fd );

    // order reversed on purpose
    read_puts( &client2_fd );
    read_puts( &client1_fd );

    mbedtls_net_free( &listen_fd );
    mbedtls_net_free( &client1_fd );
    mbedtls_net_free( &client2_fd );
}
Wizzard033 commented 6 years ago

If I set my game client to send more than one datagram, the test works as if multiple clients have connected. That's not good. My client code is attempting to do a full DTLS handshake with the above server code.

Here is my client's log (format is ssl.handshake_step, , )

Attempting to connect to 127.0.0.1:4433...
ssl.handshake_step, 0, 16
ssl.handshake_step, 1, 16
- net.send
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.recv
ssl.handshake_step, 2, 16
- net.send
ssl.handshake_step, 2, 16
- net.recv
...

The server's print statements are

received 401 bytes:
■ 
received 401 bytes:
■ 

This means that, at least on Windows, sockets aren't filtered properly when connect is called.

mpg commented 6 years ago

I can't reproduce that kind of issue with the test UDP server posted above and netcat on Linux. if I run the server and then use netcat to send it multiple datagrams from the same source port, the server keeps waiting on the second net_accept(), until I send a datagram from a different source port.

So far I haven't seen any clear indication that the bug would lie in net_sockets.c rather than in your client or server code, and all the tests I have made would tend to show the code is behaving as expected. Granted, my tests were on Linux and yours are on Windows, so it's not impossible that there could be a platform-specific issue but I haven't seen evidence of that so far.

Please produce a reproducer with the code of a complete and reasonably minimal client based on net_socket.s and instructions for using it (in a similarly the code and instructions I provided with the test UDP server) that would could use to reproduce and confirm the bug.

I'm afraid we can't accept a bug report for an issue where the code looks correct if you don't provide a way for us to reproduce the issue.

Wizzard033 commented 6 years ago

As I said before, my client does the DTLS handshake to cause this issue. So, the example DTLS client will also work to cause this issue. I tested it as well to make sure, and yes, it's having the issue. I went ahead and pasted the client code below in case you can't manage to find it on your own.

Steps to reproduce (1) Be using an up-to-date version of Windows 10 x64 (2) Compile mbed TLS 2.8.0 (with examples) with the latest CMake and latest MinGW x86 (3) Compile the server I posted most recently with MinGW x86

For example, I compiled using the following automatically-generated commands
C:/MinGW/bin/g++.exe  -c  "C:/Users/Wizzard/Documents/Codelite/socket-test/src/server.cpp" -Wall -Wextra -O1 -D_DEBUG  -o ./socket-test/Release/up_src_server.cpp.o -I../extlibs/headers -I../src
C:/MinGW/bin/g++.exe -o ../bin/socket-test/socket-test @"socket-test.txt" -L../extlibs/libs-mingw -L../bin/socket-test  -lmbedtls

(4) Open the server that was just compiled (5) Open the DTLS client example that CMake was set to compile (6) Observe that the behavior is as I indicated: not working

By the way, I'm not the type to post bug reports unless I'm feeling confident that the issue is not with my own code. I spent a couple days debugging before posting this issue, and I also had a co-worker whom has a whole different Windows 10 setup in a different office help to make sure its not just my setup.

I feel its not my job to hunt down the specific cause of this issue and tell you everything about it. So, I won't be providing anything more than a way to reproduce the issue reliably. I've already spent way, way too long fixing my engine to work around net_sockets.c so that it can work for me, and I am very disappointed with the direction this issue has taken (i.e. not testing on Windows). If the bug is fixed, good for you guys. If not, well, it's no longer my problem as I have transitioned to another socket library that works for my needs.

/*
 *  Simple DTLS client demonstration program
 *
 *  Copyright (C) 2006-2015, ARM Limited, All Rights Reserved
 *  SPDX-License-Identifier: Apache-2.0
 *
 *  Licensed under the Apache License, Version 2.0 (the "License"); you may
 *  not use this file except in compliance with the License.
 *  You may obtain a copy of the License at
 *
 *  http://www.apache.org/licenses/LICENSE-2.0
 *
 *  Unless required by applicable law or agreed to in writing, software
 *  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
 *  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 *  See the License for the specific language governing permissions and
 *  limitations under the License.
 *
 *  This file is part of mbed TLS (https://tls.mbed.org)
 */

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
#include MBEDTLS_CONFIG_FILE
#endif

#if defined(MBEDTLS_PLATFORM_C)
#include "mbedtls/platform.h"
#else
#include <stdio.h>
#define mbedtls_printf     printf
#define mbedtls_fprintf    fprintf
#endif

#if !defined(MBEDTLS_SSL_CLI_C) || !defined(MBEDTLS_SSL_PROTO_DTLS) ||    \
    !defined(MBEDTLS_NET_C)  || !defined(MBEDTLS_TIMING_C) ||             \
    !defined(MBEDTLS_ENTROPY_C) || !defined(MBEDTLS_CTR_DRBG_C) ||        \
    !defined(MBEDTLS_X509_CRT_PARSE_C) || !defined(MBEDTLS_RSA_C) ||      \
    !defined(MBEDTLS_CERTS_C) || !defined(MBEDTLS_PEM_PARSE_C)
int main( void )
{
    mbedtls_printf( "MBEDTLS_SSL_CLI_C and/or MBEDTLS_SSL_PROTO_DTLS and/or "
            "MBEDTLS_NET_C and/or MBEDTLS_TIMING_C and/or "
            "MBEDTLS_ENTROPY_C and/or MBEDTLS_CTR_DRBG_C and/or "
            "MBEDTLS_X509_CRT_PARSE_C and/or MBEDTLS_RSA_C and/or "
            "MBEDTLS_CERTS_C and/or MBEDTLS_PEM_PARSE_C not defined.\n" );
    return( 0 );
}
#else

#include <string.h>

#include "mbedtls/net_sockets.h"
#include "mbedtls/debug.h"
#include "mbedtls/ssl.h"
#include "mbedtls/entropy.h"
#include "mbedtls/ctr_drbg.h"
#include "mbedtls/error.h"
#include "mbedtls/certs.h"
#include "mbedtls/timing.h"

#define SERVER_PORT "4433"
#define SERVER_ADDR "127.0.0.1" /* forces IPv4 */
#define MESSAGE     "Echo this"

#define READ_TIMEOUT_MS 1000
#define MAX_RETRY       5

#define DEBUG_LEVEL 0

static void my_debug( void *ctx, int level,
                      const char *file, int line,
                      const char *str )
{
    ((void) level);

    mbedtls_fprintf( (FILE *) ctx, "%s:%04d: %s", file, line, str );
    fflush(  (FILE *) ctx  );
}

int main( int argc, char *argv[] )
{
    int ret, len;
    mbedtls_net_context server_fd;
    uint32_t flags;
    unsigned char buf[1024];
    const char *pers = "dtls_client";
    int retry_left = MAX_RETRY;

    mbedtls_entropy_context entropy;
    mbedtls_ctr_drbg_context ctr_drbg;
    mbedtls_ssl_context ssl;
    mbedtls_ssl_config conf;
    mbedtls_x509_crt cacert;
    mbedtls_timing_delay_context timer;

    ((void) argc);
    ((void) argv);

#if defined(MBEDTLS_DEBUG_C)
    mbedtls_debug_set_threshold( DEBUG_LEVEL );
#endif

    /*
     * 0. Initialize the RNG and the session data
     */
    mbedtls_net_init( &server_fd );
    mbedtls_ssl_init( &ssl );
    mbedtls_ssl_config_init( &conf );
    mbedtls_x509_crt_init( &cacert );
    mbedtls_ctr_drbg_init( &ctr_drbg );

    mbedtls_printf( "\n  . Seeding the random number generator..." );
    fflush( stdout );

    mbedtls_entropy_init( &entropy );
    if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy,
                               (const unsigned char *) pers,
                               strlen( pers ) ) ) != 0 )
    {
        mbedtls_printf( " failed\n  ! mbedtls_ctr_drbg_seed returned %d\n", ret );
        goto exit;
    }

    mbedtls_printf( " ok\n" );

    /*
     * 0. Load certificates
     */
    mbedtls_printf( "  . Loading the CA root certificate ..." );
    fflush( stdout );

    ret = mbedtls_x509_crt_parse( &cacert, (const unsigned char *) mbedtls_test_cas_pem,
                          mbedtls_test_cas_pem_len );
    if( ret < 0 )
    {
        mbedtls_printf( " failed\n  !  mbedtls_x509_crt_parse returned -0x%x\n\n", -ret );
        goto exit;
    }

    mbedtls_printf( " ok (%d skipped)\n", ret );

    /*
     * 1. Start the connection
     */
    mbedtls_printf( "  . Connecting to udp/%s/%s...", SERVER_ADDR, SERVER_PORT );
    fflush( stdout );

    if( ( ret = mbedtls_net_connect( &server_fd, SERVER_ADDR,
                                         SERVER_PORT, MBEDTLS_NET_PROTO_UDP ) ) != 0 )
    {
        mbedtls_printf( " failed\n  ! mbedtls_net_connect returned %d\n\n", ret );
        goto exit;
    }

    mbedtls_printf( " ok\n" );

    /*
     * 2. Setup stuff
     */
    mbedtls_printf( "  . Setting up the DTLS structure..." );
    fflush( stdout );

    if( ( ret = mbedtls_ssl_config_defaults( &conf,
                   MBEDTLS_SSL_IS_CLIENT,
                   MBEDTLS_SSL_TRANSPORT_DATAGRAM,
                   MBEDTLS_SSL_PRESET_DEFAULT ) ) != 0 )
    {
        mbedtls_printf( " failed\n  ! mbedtls_ssl_config_defaults returned %d\n\n", ret );
        goto exit;
    }

    /* OPTIONAL is usually a bad choice for security, but makes interop easier
     * in this simplified example, in which the ca chain is hardcoded.
     * Production code should set a proper ca chain and use REQUIRED. */
    mbedtls_ssl_conf_authmode( &conf, MBEDTLS_SSL_VERIFY_OPTIONAL );
    mbedtls_ssl_conf_ca_chain( &conf, &cacert, NULL );
    mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg );
    mbedtls_ssl_conf_dbg( &conf, my_debug, stdout );

    if( ( ret = mbedtls_ssl_setup( &ssl, &conf ) ) != 0 )
    {
        mbedtls_printf( " failed\n  ! mbedtls_ssl_setup returned %d\n\n", ret );
        goto exit;
    }

    mbedtls_ssl_set_bio( &ssl, &server_fd,
                         mbedtls_net_send, mbedtls_net_recv, mbedtls_net_recv_timeout );

    mbedtls_ssl_set_timer_cb( &ssl, &timer, mbedtls_timing_set_delay,
                                            mbedtls_timing_get_delay );

    mbedtls_printf( " ok\n" );

    /*
     * 4. Handshake
     */
    mbedtls_printf( "  . Performing the DTLS handshake..." );
    fflush( stdout );

    do ret = mbedtls_ssl_handshake( &ssl );
    while( ret == MBEDTLS_ERR_SSL_WANT_READ ||
           ret == MBEDTLS_ERR_SSL_WANT_WRITE );

    if( ret != 0 )
    {
        mbedtls_printf( " failed\n  ! mbedtls_ssl_handshake returned -0x%x\n\n", -ret );
        goto exit;
    }

    mbedtls_printf( " ok\n" );

    /*
     * 5. Verify the server certificate
     */
    mbedtls_printf( "  . Verifying peer X.509 certificate..." );

    /* In real life, we would have used MBEDTLS_SSL_VERIFY_REQUIRED so that the
     * handshake would not succeed if the peer's cert is bad.  Even if we used
     * MBEDTLS_SSL_VERIFY_OPTIONAL, we would bail out here if ret != 0 */
    if( ( flags = mbedtls_ssl_get_verify_result( &ssl ) ) != 0 )
    {
        char vrfy_buf[512];

        mbedtls_printf( " failed\n" );

        mbedtls_x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), "  ! ", flags );

        mbedtls_printf( "%s\n", vrfy_buf );
    }
    else
        mbedtls_printf( " ok\n" );

    /*
     * 6. Write the echo request
     */
send_request:
    mbedtls_printf( "  > Write to server:" );
    fflush( stdout );

    len = sizeof( MESSAGE ) - 1;

    do ret = mbedtls_ssl_write( &ssl, (unsigned char *) MESSAGE, len );
    while( ret == MBEDTLS_ERR_SSL_WANT_READ ||
           ret == MBEDTLS_ERR_SSL_WANT_WRITE );

    if( ret < 0 )
    {
        mbedtls_printf( " failed\n  ! mbedtls_ssl_write returned %d\n\n", ret );
        goto exit;
    }

    len = ret;
    mbedtls_printf( " %d bytes written\n\n%s\n\n", len, MESSAGE );

    /*
     * 7. Read the echo response
     */
    mbedtls_printf( "  < Read from server:" );
    fflush( stdout );

    len = sizeof( buf ) - 1;
    memset( buf, 0, sizeof( buf ) );

    do ret = mbedtls_ssl_read( &ssl, buf, len );
    while( ret == MBEDTLS_ERR_SSL_WANT_READ ||
           ret == MBEDTLS_ERR_SSL_WANT_WRITE );

    if( ret <= 0 )
    {
        switch( ret )
        {
            case MBEDTLS_ERR_SSL_TIMEOUT:
                mbedtls_printf( " timeout\n\n" );
                if( retry_left-- > 0 )
                    goto send_request;
                goto exit;

            case MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY:
                mbedtls_printf( " connection was closed gracefully\n" );
                ret = 0;
                goto close_notify;

            default:
                mbedtls_printf( " mbedtls_ssl_read returned -0x%x\n\n", -ret );
                goto exit;
        }
    }

    len = ret;
    mbedtls_printf( " %d bytes read\n\n%s\n\n", len, buf );

    /*
     * 8. Done, cleanly close the connection
     */
close_notify:
    mbedtls_printf( "  . Closing the connection..." );

    /* No error checking, the connection might be closed already */
    do ret = mbedtls_ssl_close_notify( &ssl );
    while( ret == MBEDTLS_ERR_SSL_WANT_WRITE );
    ret = 0;

    mbedtls_printf( " done\n" );

    /*
     * 9. Final clean-ups and exit
     */
exit:

#ifdef MBEDTLS_ERROR_C
    if( ret != 0 )
    {
        char error_buf[100];
        mbedtls_strerror( ret, error_buf, 100 );
        mbedtls_printf( "Last error was: %d - %s\n\n", ret, error_buf );
    }
#endif

    mbedtls_net_free( &server_fd );

    mbedtls_x509_crt_free( &cacert );
    mbedtls_ssl_free( &ssl );
    mbedtls_ssl_config_free( &conf );
    mbedtls_ctr_drbg_free( &ctr_drbg );
    mbedtls_entropy_free( &entropy );

#if defined(_WIN32)
    mbedtls_printf( "  + Press Enter to exit this program.\n" );
    fflush( stdout ); getchar();
#endif

    /* Shell can not handle large exit numbers -> 1 for errors */
    if( ret < 0 )
        ret = 1;

    return( ret );
}
#endif /* MBEDTLS_SSL_CLI_C && MBEDTLS_SSL_PROTO_DTLS && MBEDTLS_NET_C &&
          MBEDTLD_TIMING_C && MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C &&
          MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_RSA_C && MBEDTLS_CERTS_C &&
          MBEDTLS_PEM_PARSE_C */
mpg commented 6 years ago

I went ahead and pasted the client code below in case you can't manage to find it on your own.

I'm sorry but I don't think that particular remark was appropriate.

Thanks however for clarifying how to reproduce the issue with a client that's available to us. I've confirmed the behaviour I'm observing on Linux is not the one you're observing on Windows, so at least part of the issue is platform-specific.

By the way, I'm not the type to post bug reports unless I'm feeling confident that the issue is not with my own code.

I didn't mean to imply that your report was not serious. It's just that everybody, with the best intent, qualifications and serious work, can make mistakes (as an illustration, our own bug count is far from zero). In the past, we've been fixing "bugs" we hadn't reproduced ourselves only to find out later that the issue was somewhere else (in one case, it was in the reporter's toolchain). Most importantly, we can't efficiently work on fixing this bug and test the fix if we can't reproduce the issue in the first place.

So, I won't be providing anything more than a way to reproduce the issue reliably

I don't think I've ever asked for more than that.

I am very disappointed with the direction this issue has taken (i.e. not testing on Windows).

Sorry, I should have communicated more clearly about what was happening here: since your initial report indicated "OS: all" and I remembered the code working on Linux, I was surprised and wanted to test that. Unfortunately I don't have access to a Windows machine myself, so I couldn't test on Windows too, but rest assured that we'll test on Windows too - it's just that nobody's got around to it yet.

Again, a necessary step for fixing the issue is understanding whether it is platform-specific, so while testing on Linux is obviously not sufficient, I think it was nonetheless necessary.

Also, it seems according to this comment of yours that even on windows, the issue doesn't always manifest in exactly the way you initially expected it too, so hopefully the discussion so far, while lacking the very important step of testing on Windows on our side, hasn't been entirely useless in clarifying what exactly the issue is.

I have transitioned to another socket library that works for my needs.

I'm happy to read that you found an alternative solution. Thanks for taking the time to report the issue anyway.

Wizzard033 commented 6 years ago

Thanks for clarifying your intent and the meaning behind your work on the issue so far. I appreciate it.

I'm afraid we can't accept a bug report for an issue where the code looks correct if you don't provide a way for us to reproduce the issue.

This comment made me think you were already going to disregard the bug without testing on Windows, and I felt as if I had provided abundant information to reproduce the bug prior to this statement. Although I thought that way, I can see how, without posting client code, you would think that the bug may not be reproduced reliably. So, your comment is somewhat reasonable in that regard. However, I feel that if you were testing on Windows 10, then this bug would be very straightforward to replicate, as the socket library does not have a complex API. Just accepting multiple datagrams from multiple sources reproduces the bug.

mpg commented 6 years ago

Sorry, I didn't mean to suggest we would consider rejecting this issue without testing on windows, but now that you mention it, I can see how my comment could give that impression. It's unfortunate that I've been the first to jump on this issue while I'm one of the few members on the team that can't easily test on the platform that we now understand the bug is specific to.

KollarRichard commented 4 years ago

Hi is there anything new regarding this issue?

Leon-Nagel commented 1 year ago

Hi @mpg

I've run into what seems like a similar issue to @Wizzard033.

You test example with two connections works on my windows setup, but extending it with one additional connection breaks the example code. This is however not the case under Linux (Ubuntu 23.04), where I can seemingly extend it with as many connections as I like. (Have only tested up to 5)

do you know if this issue is something that anyone will be looking into?

gilles-peskine-arm commented 1 year ago

No one is looking into this, and it's unlikely that we'll look into it any time soon. Most people working on Mbed TLS don't have Windows expertise and our focus is mostly on constrained devices.

If you know how to fix the bug, please submit a patch as a pull request. We'll try to find the expertise to review it.

Ideally the bug fix would come with a non-regression test, but I recognize that we have a test gap here: we don't have any test code with multi-client servers, and the net_sockets module doesn't have unit tests. So we'll test the fix manually.