zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.71k stars 6.54k forks source link

samples/mbedtls_sslclient: Discards TLS records, handshake does not work #7502

Closed vakulgarg closed 6 years ago

vakulgarg commented 6 years ago

I encountered a scenario where TLS handshake got broken. The mbedtls stack discarded tls records assuming corrupt data.

Digging further, I found that the way the incoming TLS records have been fed to mbedtls stack is wrong. Specifically, in function tcp_rx() (inside mbedtls_sslclient/src/tcp.c), the function always copies 'read_bytes' of data in input buffer 'buf' even if the requested input len given in 'size' parameter is less than 'read_bytes'. This could cause buffer overflow. Further, mbedtls stack disards whatever extra (i.e. read_bytes - size) that has been copied in tcp_rx() in 'buf'.

Perhaps, it would have been simpler and user-friendly, if instead of net_context apis, POSIX style sockets had been used in this example.

Mras2an commented 6 years ago

Hi, I have the same issue!

pfalcon commented 6 years ago

Note that there're examples how to use mbedTLS with POSIX sockets: https://github.com/zephyrproject-rtos/zephyr/pull/5985 .

Mras2an commented 6 years ago

Tkx pfalcon, May you tell me were I can find your example code? I would like open a socket with mbedtls on the NXP FRDM-K64F board. I have tested the mbedtls_sslclient but it didn't work. Is it possible to use POSIX sockets into this example?

jukkar commented 6 years ago

Perhaps, it would have been simpler and user-friendly, if instead of net_context apis, POSIX style sockets had been used in this example.

When this sample application was created, there was no bsd socket support in zephyr. You are welcome to send patches that convert this app to use bsd sockets.

jukkar commented 6 years ago

The application has bitrotted, I will send fixed prj.conf file shortly.

I followed the steps described in the README file but used qemu instead of real device.

Starting the server in host side:

./programs/ssl/ssl_server2 psk=000102030405060708090a0b0c0d0e0f server_addr=192.0.2.2

  . Seeding the random number generator... ok
  . Bind on tcp://192.0.2.2:4433/ ... ok
  . Setting up the SSL/TLS structure... ok
  . Waiting for a remote connection ... ok
  . Performing the SSL/TLS handshake... ok
    [ Protocol is TLSv1.2 ]
    [ Ciphersuite is TLS-PSK-WITH-AES-256-CCM-8 ]
    [ Record expansion is 21 ]
  < Read from client: 18 bytes read

GET / HTTP/1.0

  > Write to client: 139 bytes written in 1 fragments

HTTP/1.0 200 OK
Content-Type: text/html

<h2>mbed TLS Test Server</h2>
<p>Successful connection using: TLS-PSK-WITH-AES-256-CCM-8</p>

  . Closing the connection... done
  . Waiting for a remote connection ...^C interrupted by signal
  . Cleaning up... done.

And the zephyr side:

***** Booting Zephyr OS v1.11.0-1168-g03f9f66496 *****
Zephyr Shell, Zephyr version: 1.11.99
Type 'help' for a list of available commands
shell> 
  . Seeding the random number generator... ok
  . Setting up the SSL/TLS structure... ok
  . Connecting to tcp 192.0.2.2... ok
  . Performing the SSL/TLS handshake... ok
  > Write to server: ok
  . Closing the connection... done
QEMU: Terminated
[100%] Built target run
Mras2an commented 6 years ago

tkx jukkar, I will test with QEMU.

xianrenqiu commented 6 years ago

I did a test in the qemu_x86, only a few times can run successfully, but most of them fail. @Mras2an has it been successful in the QEMU environment?

In addition, I also tried to use the configuration file provided by mbedtls(config-suite-b.h), The server certificate will be verified,but the handshake was not successful. @jukkar is there have a similar sample program? thanks

jukkar commented 6 years ago

is there have a similar sample program?

For example echo-client and http_client samples have TLS supported.

Note that there are many things that can go wrong with TLS, one major thing is to use correct certs. Because of this the echo-[client | server] and http_[client | server] applications have been setup so that they should work against the relevant counter parts found in net-tools project (see README.md file in that project for details).

xianrenqiu commented 6 years ago

Thanks for your feedback.

As you said, I tried using the echo_client example with prj_qemu_x86_tls.conf config file. The first run can be successful (Sometimes there is a thread crash), ctrl+a+x closes the QEMU echo_client program, and runs again, it will be failed(probability). If there was no such situation, It might be my environmental problem.

Mras2an commented 6 years ago

I did more test with my NXP FRDM-K64F board and my issue is after the server hello. The NXP FRDM-K64F board never send the "Key Exchange". It's like if the board has a problem with the "server hello" frame but without indicating an error. I keep watching...

log: New TCP connection #1: 192.168.2.66(45945) <-> 192.168.2.15(60303) 1 1 0.2943 (0.2943) C>S Handshake ClientHello Version 3.3 cipher suites Unknown value 0x3d TLS_RSA_WITH_AES_256_CBC_SHA Unknown value 0x3c TLS_RSA_WITH_AES_128_CBC_SHA Unknown value 0x8d Unknown value 0xae Unknown value 0x8c Unknown value 0xff compression methods NULL 1 2 0.3522 (0.0578) S>C Handshake ServerHello Version 3.3 session_id[32]= 5a f9 ac 03 3d dc 3e 75 f9 41 2c 64 fe 06 d8 eb 0a 25 f7 55 29 d9 d4 27 5a 10 a4 99 ce 71 d3 50 cipherSuite TLS_RSA_WITH_AES_256_CBC_SHA compressionMethod NULL Certificate CertificateRequest certificate_types rsa_sign certificate_types dss_sign ...

vakulgarg commented 6 years ago

@Mras2an, does my pull request #7675 fix your problem?

Mras2an commented 6 years ago

@vakulgarg, I will test as soon as possible.

Mras2an commented 6 years ago

@vakulgarg, with your pull request the board send the clientHello but the server don't send the serverHello...

vakulgarg commented 6 years ago

I tested mbedtls_sslclient application on nxp-frdm-k64f with an 'openssl s_server' on my linux machine. Can you check what is happening at the server end? Does it discard clientHello?

pfalcon commented 6 years ago

@vakulgarg : What's the state of this issue? Did @jukkar's PR https://github.com/zephyrproject-rtos/zephyr/pull/7549 fixed this sample. If not, this ticket should be reopened. And if yes, why do we need https://github.com/zephyrproject-rtos/zephyr/pull/7675 ?

vakulgarg commented 6 years ago

I reported that because of buffer overflows, the handshake is failing. PR #7549 increased number of buffers in configuration and didn't address the code flaw I described. It might have just masked some other issue in network stack.

Hence from my point of view, the problem remains un-addressed and this ticket should be re-opened.

7675 addresses the issue I reported.

pfalcon commented 6 years ago

@vakulgarg: Thanks for response, reopening.

linkmeyer commented 6 years ago

@jukkar , what's the status on getting this re-opened bug properly fixed?

rveerama1 commented 6 years ago

I did bit of investigation.. Reason for handshake failure is..

`ssl_tls.c:2498: |2| ssl->f_send() returned 54 (-0xffffffca) ssl_tls.c:2525: |2| <= flush output ssl_tls.c:2924: |2| <= write record ssl_cli.c:1088: |2| <= write client hello

ssl_cli.c:3389: |2| client state: 2 ssl_tls.c:2473: |2| => flush output ssl_tls.c:2485: |2| <= flush output

ssl_cli.c:1481: |2| => parse server hello ssl_tls.c:3811: |2| => read record ssl_tls.c:2254: |2| => fetch input ssl_tls.c:2415: |2| in_left: 0, nb_want: 5

[net/conn] [DBG] net_conn_input: (0x004085b8): Check TCP listener for pkt 0x0040c324 src port 4433 dst port 49302 family 2 chksum 0x7a66 len 40 [net/conn] [DBG] net_conn_input: (0x004085b8): [0] match found cb 0x000153bc ud 0x00408340 rank 0x33 [net/tcp] [DBG] _tcp_established: (0x004085b8): DATA received from 192.0.2.2 port 4433

[net/conn] [DBG] net_conn_input: (0x004085b8): Check TCP listener for pkt 0x0040c2d8 src port 4433 dst port 49302 family 2 chksum 0x2b41 len 126 [net/conn] [DBG] net_conn_input: (0x004085b8): [0] match found cb 0x000153bc ud 0x00408340 rank 0x33 [net/tcp] [DBG] _tcp_established: (0x004085b8): DATA received from 192.0.2.2 port 4433 [net/ctx] [DBG] net_context_packet_received: (0x004085b8): Set appdata 0x0040a676 to len 86 (total 126) [net/tcp] [DBG] net_tcp_trace: (0x004085b8): [0x00408840] pkt 0x0040befc src 49302 dst 4433 seq 0x4eb472c5 (1320448709) ack 0x53e2586c (1407342700/86) flags uAprsf win 1280 chk 0xe720 [net/tcp] [DBG] print_send_info: (0x004085b8): ACK sent to 192.0.2.2 port 4433

ssl_tls.c:2439: |2| in_left: 0, nb_want: 5 ssl_tls.c:2440: |2| ssl->f_recv(_timeout)() returned 86 (-0xffffffaa) ssl_tls.c:2452: |1| f_recv returned 86 bytes but only 5 were requested ssl_tls.c:3958: |1| mbedtls_ssl_fetch_input() returned -27648 (-0x6c00) ssl_tls.c:3822: |1| mbedtls_ssl_read_record_layer() returned -27648 (-0x6c00) ssl_cli.c:1488: |1| mbedtls_ssl_read_record() returned -27648 (-0x6c00) ssl_tls.c:6728: |2| <= handshake failed ! mbedtls_ssl_handshake returned -0xfffffff8`

samples/net/mbedtls_sslclient does not use net_app or sockets. Direct calls to mbedtls library. https://github.com/zephyrproject-rtos/zephyr/blob/master/samples/net/mbedtls_sslclient/src/tcp.c#L71 tcp_rx(.., size) function does not use 'size' variable at all.

https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/net/lib/app/net_app.c#L1852 net_app based rx function _net_app_ssl_mux() copy /feeds mbedtls buf based on size parameter.

Which is missing in "samples/net/mbedtls_sslclient" tcp_rx function. mbedtls expects 5 bytes but it returns whole packet (86 bytes).

These are very old samples. Better to use net_app based or latest sockets based api's to handle TLS stuff than application itself. Better to remove these samples, what do you say @jukkar ?

nashif commented 6 years ago

@jukkar any update on this one?

jukkar commented 6 years ago

The following mbedtls samples are using directly mbedtls APIs. As there are multiple ways these applications can fail, and as we have more proper TLS supported applications like samples/net/sockets/echo_[server|client], I propose that we deprecate these three applications. I will send a PR that removes them.

samples/net/mbedtls_sslclient
samples/net/mbedtls_dtlsserver
samples/net/mbedtls_dtlsclient
jukkar commented 6 years ago

The apps are removed by #9730

nashif commented 6 years ago

app removed