Merge pull request #849 from paullouisageneau/cleanup-tls

Clean up and make TLS and DTLS transports more consistent

README.md

@@ -1,19 +1,20 @@
 # libdatachannel - C/C++ WebRTC network library
 
 [![License: MPL 2.0](https://img.shields.io/badge/License-MPL_2.0-blue.svg)](https://www.mozilla.org/en-US/MPL/2.0/)
-[![Build with OpenSSL](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-openssl.yml/badge.svg)](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-openssl.yml)
 [![Build with GnuTLS](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-gnutls.yml/badge.svg)](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-gnutls.yml)
-[![Gitter](https://badges.gitter.im/libdatachannel/community.svg)](https://gitter.im/libdatachannel/community?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
-[![Discord](https://img.shields.io/discord/903257095539925006?logo=discord)](https://discord.gg/jXAP8jp3Nn)
+[![Build with Mbed TLS](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-mbedtls.yml/badge.svg)](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-mbedtls.yml)
+[![Build with OpenSSL](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-openssl.yml/badge.svg)](https://github.com/paullouisageneau/libdatachannel/actions/workflows/build-openssl.yml)
 
 [![AUR package](https://repology.org/badge/version-for-repo/aur/libdatachannel.svg)](https://repology.org/project/libdatachannel/versions) [![FreeBSD port](https://repology.org/badge/version-for-repo/freebsd/libdatachannel.svg)](https://repology.org/project/libdatachannel/versions) [![Vcpkg package](https://repology.org/badge/version-for-repo/vcpkg/libdatachannel.svg)](https://repology.org/project/libdatachannel/versions)
+[![Gitter](https://badges.gitter.im/libdatachannel/community.svg)](https://gitter.im/libdatachannel/community?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
+[![Discord](https://img.shields.io/discord/903257095539925006?logo=discord)](https://discord.gg/jXAP8jp3Nn)
 
 libdatachannel is a standalone implementation of WebRTC Data Channels, WebRTC Media Transport, and WebSockets in C++17 with C bindings for POSIX platforms (including GNU/Linux, Android, FreeBSD, Apple macOS and iOS) and Microsoft Windows. WebRTC is a W3C and IETF standard enabling real-time peer-to-peer data and media exchange between two devices.
 
 The library aims at being both straightforward and lightweight with minimal external dependencies, to enable direct connectivity between native applications and web browsers without the pain of importing Google's bloated [reference library](https://webrtc.googlesource.com/src/). The interface consists of somewhat simplified versions of the JavaScript WebRTC and WebSocket APIs present in browsers, in order to ease the design of cross-environment applications.
 
 It can be compiled with multiple backends:
-- The security layer can be provided through [OpenSSL](https://www.openssl.org/) or [GnuTLS](https://www.gnutls.org/).
+- The security layer can be provided through [GnuTLS](https://www.gnutls.org/), [Mbed TLS](https://www.trustedfirmware.org/projects/mbed-tls/), or [OpenSSL](https://www.openssl.org/).
 - The connectivity for WebRTC can be provided through my ad-hoc ICE library [libjuice](https://github.com/paullouisageneau/libjuice) as submodule or through [libnice](https://github.com/libnice/libnice).
 
 The WebRTC stack is fully compatible with browsers like Firefox and Chromium, see [Compatibility](#Compatibility) below. Additionally, code using Data Channels and WebSockets from the library may be compiled as is to WebAssembly for browsers with [datachannel-wasm](https://github.com/paullouisageneau/datachannel-wasm).
@@ -24,12 +25,12 @@ libdatachannel is available on [AUR](https://aur.archlinux.org/packages/libdatac
 
 ## Dependencies
 
-- [GnuTLS](https://www.gnutls.org/) or [OpenSSL](https://www.openssl.org/)
+- [GnuTLS](https://www.gnutls.org/), [Mbed TLS](https://www.trustedfirmware.org/projects/mbed-tls/), or [OpenSSL](https://www.openssl.org/)
 - [usrsctp](https://github.com/sctplab/usrsctp) (as submodule by default)
 - [Plog](https://github.com/SergiusTheBest/plog) (as submodule by default)
 - [libjuice](https://github.com/paullouisageneau/libjuice) (as submodule by default) or [libnice](https://nice.freedesktop.org/) as an ICE backend.
 - [libsrtp](https://github.com/cisco/libsrtp) (as submodule by default) required if compiled with media support.
-- [Nlohmann JSON](https://github.com/nlohmann/json) (as submodule by default) required to build examples.
+- [nlohmann JSON](https://github.com/nlohmann/json) (as submodule by default) required to build examples.
 
 ## Building
 

src/impl/dtlstransport.cpp

@@ -209,7 +209,7 @@ void DtlsTransport::doRecv() {
 					throw std::runtime_error("MTU is too low");
 				}
 
-			} while (!gnutls::check(ret, "DTLS handshake failed")); // Re-call on non-fatal error
+			} while (!gnutls::check(ret, "Handshake failed")); // Re-call on non-fatal error
 
 			// RFC 8261: DTLS MUST support sending messages larger than the current path MTU
 			// See https://www.rfc-editor.org/rfc/rfc8261.html#section-5
@@ -263,9 +263,14 @@ void DtlsTransport::doRecv() {
 
 	gnutls_bye(mSession, GNUTLS_SHUT_WR);
 
-	PLOG_INFO << "DTLS closed";
-	changeState(State::Disconnected);
-	recv(nullptr);
+	if (state() == State::Connected) {
+		PLOG_INFO << "DTLS closed";
+		changeState(State::Disconnected);
+		recv(nullptr);
+	} else {
+		PLOG_ERROR << "DTLS handshake failed";
+		changeState(State::Failed);
+	}
 }
 
 int DtlsTransport::CertificateCallback(gnutls_session_t session) {
@@ -413,6 +418,7 @@ DtlsTransport::DtlsTransport(shared_ptr<IceTransport> lower, certificate_ptr cer
 		mbedtls_ssl_set_export_keys_cb(&mSsl, DtlsTransport::ExportKeysCallback, this);
 		mbedtls_ssl_set_bio(&mSsl, this, WriteCallback, ReadCallback, NULL);
 		mbedtls_ssl_set_timer_cb(&mSsl, this, SetTimerCallback, GetTimerCallback);
+
 	} catch (...) {
 		mbedtls_entropy_free(&mEntropy);
 		mbedtls_ctr_drbg_free(&mDrbg);
@@ -524,6 +530,7 @@ void DtlsTransport::doRecv() {
 		if (state() == State::Connecting) {
 			while (true) {
 				auto ret = mbedtls_ssl_handshake(&mSsl);
+
 				if (ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
 					ThreadPool::Instance().schedule(mTimerSetAt + milliseconds(mFinMs),
 					                                [weak_this = weak_from_this()]() {
@@ -531,12 +538,14 @@ void DtlsTransport::doRecv() {
 							                                locked->doRecv();
 					                                });
 					return;
-				} else if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
-				           ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
-					continue;
 				}
 
-				mbedtls::check(ret);
+				if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
+				           ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS)
+					continue;
+
+				mbedtls::check(ret, "Handshake failed");
+
 				PLOG_INFO << "DTLS handshake finished";
 				changeState(State::Connected);
 				postHandshake();
@@ -551,17 +560,21 @@ void DtlsTransport::doRecv() {
 				    mbedtls_ssl_read(&mSsl, reinterpret_cast<unsigned char *>(buffer), bufferSize);
 				mMutex.unlock();
 
+				if (ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
+					return;
+				}
+
+				if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
+				           ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
+					continue;
+				}
+
 				if (ret == 0 || ret == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY) {
 					// Closed
 					PLOG_DEBUG << "DTLS connection cleanly closed";
 					break;
 				}
 
-				if (ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE ||
-				    ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
-				    ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
-					return;
-				}
 				mbedtls::check(ret);
 
 				auto *b = reinterpret_cast<byte *>(buffer);
@@ -572,9 +585,14 @@ void DtlsTransport::doRecv() {
 		PLOG_ERROR << "DTLS recv: " << e.what();
 	}
 
-	PLOG_INFO << "DTLS closed";
-	changeState(State::Disconnected);
-	recv(nullptr);
+	if (state() == State::Connected) {
+		PLOG_INFO << "DTLS closed";
+		changeState(State::Disconnected);
+		recv(nullptr);
+	} else {
+		PLOG_ERROR << "DTLS handshake failed";
+		changeState(State::Failed);
+	}
 }
 
 void DtlsTransport::ExportKeysCallback(void *ctx, mbedtls_ssl_key_export_type /*type*/,

src/impl/tlstransport.cpp

@@ -177,7 +177,7 @@ void TlsTransport::doRecv() {
 				if (ret == GNUTLS_E_AGAIN)
 					return;
 
-			} while (!gnutls::check(ret, "TLS handshake failed")); // Re-call on non-fatal error
+			} while (!gnutls::check(ret, "Handshake failed")); // Re-call on non-fatal error
 
 			PLOG_INFO << "TLS handshake finished";
 			changeState(State::Connected);
@@ -214,9 +214,14 @@ void TlsTransport::doRecv() {
 
 	gnutls_bye(mSession, GNUTLS_SHUT_WR);
 
-	PLOG_INFO << "TLS closed";
-	changeState(State::Disconnected);
-	recv(nullptr);
+	if (state() == State::Connected) {
+		PLOG_INFO << "TLS closed";
+		changeState(State::Disconnected);
+		recv(nullptr);
+	} else {
+		PLOG_ERROR << "TLS handshake failed";
+		changeState(State::Failed);
+	}
 }
 
 ssize_t TlsTransport::WriteCallback(gnutls_transport_ptr_t ptr, const void *data, size_t len) {
@@ -339,6 +344,7 @@ TlsTransport::TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<HttpProx
 
 		mbedtls::check(mbedtls_ssl_setup(&mSsl, &mConf));
 		mbedtls_ssl_set_bio(&mSsl, static_cast<void *>(this), WriteCallback, ReadCallback, NULL);
+
 	} catch (...) {
 		mbedtls_entropy_free(&mEntropy);
 		mbedtls_ctr_drbg_free(&mDrbg);
@@ -416,14 +422,18 @@ void TlsTransport::doRecv() {
 		if (state() == State::Connecting) {
 			while (true) {
 				auto ret = mbedtls_ssl_handshake(&mSsl);
+
 				if (ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
 					return;
-				} else if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
-				           ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
+				}
+
+				if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
+				    ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
 					continue;
 				}
 
-				mbedtls::check(ret);
+				mbedtls::check(ret, "Handshake failed");
+
 				PLOG_INFO << "TLS handshake finished";
 				changeState(State::Connected);
 				postHandshake();
@@ -436,18 +446,21 @@ void TlsTransport::doRecv() {
 				auto ret =
 				    mbedtls_ssl_read(&mSsl, reinterpret_cast<unsigned char *>(buffer), bufferSize);
 
+				if (ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
+					return;
+				}
+
+				if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
+				    ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
+					continue;
+				}
+
 				if (ret == 0 || ret == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY) {
 					// Closed
 					PLOG_DEBUG << "TLS connection cleanly closed";
 					break;
 				}
 
-				if (ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
-					return;
-				} else if (ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ||
-				           ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) {
-					continue;
-				}
 				mbedtls::check(ret);
 
 				auto *b = reinterpret_cast<byte *>(buffer);
@@ -458,9 +471,14 @@ void TlsTransport::doRecv() {
 		PLOG_ERROR << "TLS recv: " << e.what();
 	}
 
-	PLOG_INFO << "TLS closed";
-	changeState(State::Disconnected);
-	recv(nullptr);
+	if (state() == State::Connected) {
+		PLOG_INFO << "TLS closed";
+		changeState(State::Disconnected);
+		recv(nullptr);
+	} else {
+		PLOG_ERROR << "TLS handshake failed";
+		changeState(State::Failed);
+	}
 }
 
 int TlsTransport::WriteCallback(void *ctx, const unsigned char *buf, size_t len) {