All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/7] settings: Fix a format string
@ 2018-11-01  2:12 Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 2/7] Add format string checking to a few declarations Andrew Zaborowski
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

The %d conersion specifier is for integers.
---
 ell/settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ell/settings.c b/ell/settings.c
index c6cb0ff..e3e8303 100644
--- a/ell/settings.c
+++ b/ell/settings.c
@@ -1068,7 +1068,7 @@ LIB_EXPORT bool l_settings_set_double(struct l_settings *settings,
 {
 	L_AUTO_FREE_VAR(char *, buf);
 
-	buf = l_strdup_printf("%d", in);
+	buf = l_strdup_printf("%f", in);
 
 	return l_settings_set_value(settings, group_name, key, buf);
 }
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 2/7] Add format string checking to a few declarations
  2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
@ 2018-11-01  2:12 ` Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 3/7] tls: Add debug logging API Andrew Zaborowski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

Add the gcc attribute(format(printf)) to function declarations where
this may be useful.
---
 ell/dbus.h   | 3 ++-
 ell/string.h | 3 ++-
 ell/util.h   | 6 ++++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ell/dbus.h b/ell/dbus.h
index a7c08d2..d3af5d7 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -118,7 +118,8 @@ struct l_dbus_message *l_dbus_message_new_error_valist(
 struct l_dbus_message *l_dbus_message_new_error(
 					struct l_dbus_message *method_call,
 					const char *name,
-					const char *format, ...);
+					const char *format, ...)
+					__attribute__((format(printf, 3, 4)));
 
 struct l_dbus_message *l_dbus_message_ref(struct l_dbus_message *message);
 void l_dbus_message_unref(struct l_dbus_message *message);
diff --git a/ell/string.h b/ell/string.h
index c947c14..c1948ec 100644
--- a/ell/string.h
+++ b/ell/string.h
@@ -42,7 +42,8 @@ struct l_string *l_string_append_fixed(struct l_string *dest, const char *src,
 
 void l_string_append_vprintf(struct l_string *dest,
 					const char *format, va_list args);
-void l_string_append_printf(struct l_string *dest, const char *format, ...);
+void l_string_append_printf(struct l_string *dest, const char *format, ...)
+					__attribute__((format(printf, 2, 3)));
 
 struct l_string *l_string_truncate(struct l_string *string, size_t new_size);
 
diff --git a/ell/util.h b/ell/util.h
index 381721f..9fd775e 100644
--- a/ell/util.h
+++ b/ell/util.h
@@ -258,7 +258,8 @@ static inline void auto_free(void *a)
 
 char *l_strdup(const char *str);
 char *l_strndup(const char *str, size_t max);
-char *l_strdup_printf(const char *format, ...);
+char *l_strdup_printf(const char *format, ...)
+			__attribute__((format(printf, 1, 2)));
 char *l_strdup_vprintf(const char *format, va_list args);
 
 size_t l_strlcpy(char* dst, const char *src, size_t len);
@@ -280,7 +281,8 @@ void l_util_hexdumpv(bool in, const struct iovec *iov, size_t n_iov,
 					l_util_hexdump_func_t function,
 					void *user_data);
 void l_util_debug(l_util_hexdump_func_t function, void *user_data,
-						const char *format, ...);
+						const char *format, ...)
+			__attribute__((format(printf, 3, 4)));
 
 const char *l_util_get_debugfs_path(void);
 
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 3/7] tls: Add debug logging API
  2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 2/7] Add format string checking to a few declarations Andrew Zaborowski
@ 2018-11-01  2:12 ` Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 4/7] pem: Add l_pem_load_certificate_list utility Andrew Zaborowski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 43147 bytes --]

Add a debug logging API similar to that in dbus.h and genl.h.  A lot of
messages are added to report on the negotiation progress (Handshake, Alert
and ChangeCipherSpec messages) and all the possible error situations.
Especially configuration errors -- the most common errors -- should be
clear when looking at the logs.

No Application Data messages (plaintext) or raw TLS record contents
(encrypted) are logged because this can be easily done by the user
code and could add a flood of messages that would make it actually
harder to solve TLS issues.
---
 ell/tls-private.h |  24 +++
 ell/tls-record.c  |  66 ++++---
 ell/tls.c         | 427 +++++++++++++++++++++++++++++++++++-----------
 ell/tls.h         |   6 +
 4 files changed, 399 insertions(+), 124 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 8cf5be9..4f680a0 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -48,6 +48,7 @@ struct tls_hash_algorithm {
 	uint8_t tls_id;
 	enum l_checksum_type l_id;
 	size_t length;
+	const char *name;
 };
 
 typedef bool (*tls_get_hash_t)(struct l_tls *tls, uint8_t tls_id,
@@ -89,6 +90,7 @@ struct tls_cipher_suite {
 
 struct tls_compression_method {
 	int id;
+	const char *name;
 };
 
 enum tls_handshake_state {
@@ -136,6 +138,9 @@ struct l_tls {
 	l_tls_ready_cb_t ready_handle;
 	l_tls_disconnect_cb_t disconnected;
 	void *user_data;
+	l_tls_debug_cb_t debug_handler;
+	l_tls_destroy_cb_t debug_destroy;
+	void *debug_data;
 
 	char *ca_cert_path;
 	char *cert_path;
@@ -254,3 +259,22 @@ void tls_prf_get_bytes(struct l_tls *tls,
 				const char *label,
 				const void *seed, size_t seed_len,
 				uint8_t *buf, size_t len);
+
+#define TLS_DEBUG(fmt, args...)	\
+	l_util_debug(tls->debug_handler, tls->debug_data, "%s:%i " fmt,	\
+			__func__, __LINE__, ## args)
+#define TLS_SET_STATE(new_state)	\
+	do {	\
+		TLS_DEBUG("New state %s",	\
+				tls_handshake_state_to_str(new_state));	\
+		tls->state = new_state;	\
+	} while (0)
+#define TLS_DISCONNECT(desc, local_desc, fmt, args...)	\
+	do {	\
+		TLS_DEBUG("Disconnect desc=%s local-desc=%s reason=" fmt,\
+				l_tls_alert_to_str(desc),	\
+				l_tls_alert_to_str(local_desc), ## args);\
+		tls_disconnect(tls, desc, local_desc);	\
+	} while (0)
+
+const char *tls_handshake_state_to_str(enum tls_handshake_state state);
diff --git a/ell/tls-record.c b/ell/tls-record.c
index 0ad03ed..8038049 100644
--- a/ell/tls-record.c
+++ b/ell/tls-record.c
@@ -208,13 +208,9 @@ void tls_tx_record(struct l_tls *tls, enum tls_content_type type,
 static bool tls_handle_plaintext(struct l_tls *tls, const uint8_t *plaintext,
 					int len, uint8_t type, uint16_t version)
 {
-	int header_len, need_len;
-	int chunk_len;
-
-	uint32_t hs_len;
-
 	if (len > (1 << 14)) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Plaintext message too long: %i", len);
 		return false;
 	}
 
@@ -246,30 +242,39 @@ static bool tls_handle_plaintext(struct l_tls *tls, const uint8_t *plaintext,
 		break;
 
 	default:
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Unknown content type %i", type);
 		return false;
 	}
 
 	if (tls->message_buf_len && type != tls->message_content_type) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Message fragment type %i doesn't match "
+				"previous type %i", type,
+				tls->message_content_type);
 		return false;
 	}
 
 	tls->message_content_type = type;
 
 	while (1) {
+		int header_len, need_len;
+		int chunk_len;
+
 		/* Do we have a full header in tls->message_buf? */
 		header_len = (type == TLS_CT_ALERT) ? 2 : 4;
 		need_len = header_len;
 
 		if (tls->message_buf_len >= header_len) {
 			if (type == TLS_CT_HANDSHAKE) {
-				hs_len = (tls->message_buf[1] << 16) |
+				uint32_t hs_len = (tls->message_buf[1] << 16) |
 					(tls->message_buf[2] << 8) |
 					(tls->message_buf[3] << 0);
 				if (hs_len > (1 << 14)) {
-					tls_disconnect(tls,
-						TLS_ALERT_DECODE_ERROR, 0);
+					TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR,
+							0, "Handshake message "
+							"too long: %i",
+							(int) hs_len);
 					return false;
 				}
 
@@ -333,19 +338,22 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 	fragment_len = l_get_be16(tls->record_buf + 3);
 
 	if (fragment_len > (1 << 14) + 2048) {
-		tls_disconnect(tls, TLS_ALERT_RECORD_OVERFLOW, 0);
+		TLS_DISCONNECT(TLS_ALERT_RECORD_OVERFLOW, 0,
+				"Record fragment too long: %u", fragment_len);
 		return false;
 	}
 
 	if ((tls->negotiated_version && tls->negotiated_version != version) ||
 			(!tls->negotiated_version &&
 			 tls->record_buf[1] != 0x03 /* Appending E.1 */)) {
-		tls_disconnect(tls, TLS_ALERT_PROTOCOL_VERSION, 0);
+		TLS_DISCONNECT(TLS_ALERT_PROTOCOL_VERSION, 0,
+				"Record version mismatch: %02x", version);
 		return false;
 	}
 
 	if (fragment_len < tls->mac_length[0]) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Record fragment too short: %u", fragment_len);
 		return false;
 	}
 
@@ -366,7 +374,8 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 		else if (!l_cipher_decrypt(tls->cipher[0], tls->record_buf + 5,
 						compressed + 13,
 						cipher_output_len)) {
-			tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"Decrypting record fragment failed");
 			return false;
 		}
 
@@ -376,7 +385,8 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 
 		if (memcmp(mac_buf, compressed + 13 + compressed_len,
 							tls->mac_length[0])) {
-			tls_disconnect(tls, TLS_ALERT_BAD_RECORD_MAC, 0);
+			TLS_DISCONNECT(TLS_ALERT_BAD_RECORD_MAC, 0,
+					"Record fragment MAC mismatch");
 			return false;
 		}
 
@@ -390,7 +400,9 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 			i = tls->record_iv_length[0];
 
 		if (fragment_len <= tls->mac_length[0] + i) {
-			tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+					"Record fragment too short: %u",
+					fragment_len);
 			return false;
 		}
 
@@ -402,7 +414,11 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 			 * should be returned here but that was declared
 			 * unsafe in the TLS 1.1 spec.
 			 */
-			tls_disconnect(tls, TLS_ALERT_BAD_RECORD_MAC, 0);
+			TLS_DISCONNECT(TLS_ALERT_BAD_RECORD_MAC, 0,
+					"Fragment data len %i not a multiple "
+					"of block length %zi",
+					cipher_output_len,
+					tls->block_length[0]);
 			return false;
 		}
 
@@ -410,22 +426,23 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 			if (!l_cipher_set_iv(tls->cipher[0],
 						tls->record_buf + 5,
 						tls->record_iv_length[0])) {
-				tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-						0);
+				TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+						"Setting fragment IV failed");
 				return false;
 			}
 		} else if (tls->negotiated_version >= TLS_V11)
 			if (!l_cipher_decrypt(tls->cipher[0],
 						tls->record_buf + 5, iv,
 						tls->record_iv_length[0])) {
-				tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-						0);
+				TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+						"Setting fragment IV failed");
 				return false;
 			}
 
 		if (!l_cipher_decrypt(tls->cipher[0], tls->record_buf + 5 + i,
 					compressed + 13, cipher_output_len)) {
-			tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"Fragment decryption failed");
 			return false;
 		}
 
@@ -469,7 +486,8 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 		if ((tls->mac_length[0] && memcmp(mac_buf, compressed + 13 +
 					compressed_len, tls->mac_length[0])) ||
 				error) {
-			tls_disconnect(tls, TLS_ALERT_BAD_RECORD_MAC, 0);
+			TLS_DISCONNECT(TLS_ALERT_BAD_RECORD_MAC, 0,
+					"Record fragment MAC mismatch");
 			return false;
 		}
 
diff --git a/ell/tls.c b/ell/tls.c
index 3528ceb..4b37785 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -26,6 +26,7 @@
 #include <time.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <stdio.h>
 
 #include "util.h"
 #include "private.h"
@@ -185,7 +186,7 @@ static void tls_reset_handshake(struct l_tls *tls)
 	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
 		tls_drop_handshake_hash(tls, hash);
 
-	tls->state = TLS_HANDSHAKE_WAIT_HELLO;
+	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);
 	tls->cert_requested = 0;
 	tls->cert_sent = 0;
 }
@@ -197,11 +198,13 @@ static void tls_cleanup_handshake(struct l_tls *tls)
 	memset(tls->pending.master_secret, 0, 48);
 }
 
-static bool tls_change_cipher_spec(struct l_tls *tls, bool txrx)
+static bool tls_change_cipher_spec(struct l_tls *tls, bool txrx,
+					const char **error)
 {
 	struct tls_bulk_encryption_algorithm *enc;
 	struct tls_mac_algorithm *mac;
 	int key_offset;
+	char error_buf[50];
 
 	if (tls->cipher[txrx]) {
 		l_cipher_free(tls->cipher[txrx]);
@@ -240,8 +243,17 @@ static bool tls_change_cipher_spec(struct l_tls *tls, bool txrx)
 		/* Wipe out the now unneeded part of the key block */
 		memset(tls->pending.key_block + key_offset, 0, mac->mac_length);
 
-		if (!tls->mac[txrx])
+		if (!tls->mac[txrx]) {
+			if (error) {
+				*error = error_buf;
+				snprintf(error_buf, sizeof(error_buf),
+						"Can't create %s's %s HMAC",
+						tls->cipher_suite[txrx]->name,
+						txrx ? "Tx" : "Rx");
+			}
+
 			return false;
+		}
 
 		tls->mac_length[txrx] = mac->mac_length;
 
@@ -262,8 +274,17 @@ static bool tls_change_cipher_spec(struct l_tls *tls, bool txrx)
 		/* Wipe out the now unneeded part of the key block */
 		memset(tls->pending.key_block + key_offset, 0, enc->key_length);
 
-		if (!tls->cipher[txrx])
+		if (!tls->cipher[txrx]) {
+			if (error) {
+				*error = error_buf;
+				snprintf(error_buf, sizeof(error_buf),
+						"Can't create %s's %s cipher",
+						tls->cipher_suite[txrx]->name,
+						txrx ? "Tx" : "Rx");
+			}
+
 			return false;
+		}
 
 		tls->cipher_type[txrx] = enc->cipher_type;
 		if (enc->cipher_type == TLS_CIPHER_BLOCK) {
@@ -303,7 +324,7 @@ static void tls_reset_cipher_spec(struct l_tls *tls, bool txrx)
 
 	tls->pending.cipher_suite = NULL;
 
-	tls_change_cipher_spec(tls, txrx);
+	tls_change_cipher_spec(tls, txrx, NULL);
 }
 
 static bool tls_send_rsa_client_key_xchg(struct l_tls *tls);
@@ -440,9 +461,9 @@ static struct tls_cipher_suite *tls_find_cipher_suite(const uint8_t *id)
 }
 
 static struct tls_compression_method tls_compression_pref[] = {
-	/* CompressionMethod.null */
 	{
 		0,
+		"CompressionMethod.null",
 	},
 };
 
@@ -459,9 +480,9 @@ static struct tls_compression_method *tls_find_compression_method(
 }
 
 static const struct tls_hash_algorithm tls_handshake_hash_data[] = {
-	[HANDSHAKE_HASH_SHA256]	= { 4, L_CHECKSUM_SHA256, 32 },
-	[HANDSHAKE_HASH_MD5]	= { 1, L_CHECKSUM_MD5, 16 },
-	[HANDSHAKE_HASH_SHA1]	= { 2, L_CHECKSUM_SHA1, 20 },
+	[HANDSHAKE_HASH_SHA256]	= { 4, L_CHECKSUM_SHA256, 32, "SHA256" },
+	[HANDSHAKE_HASH_MD5]	= { 1, L_CHECKSUM_MD5, 16, "MD5" },
+	[HANDSHAKE_HASH_SHA1]	= { 2, L_CHECKSUM_SHA1, 20, "SHA1" },
 };
 
 static bool tls_init_handshake_hash(struct l_tls *tls)
@@ -469,14 +490,20 @@ static bool tls_init_handshake_hash(struct l_tls *tls)
 	enum handshake_hash_type hash;
 
 	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++) {
-		if (tls->handshake_hash[hash])
+		if (tls->handshake_hash[hash]) {
+			TLS_DEBUG("Handshake hash %s already exists",
+					tls_handshake_hash_data[hash].name);
 			goto err;
+		}
 
 		tls->handshake_hash[hash] = l_checksum_new(
 					tls_handshake_hash_data[hash].l_id);
 
-		if (!tls->handshake_hash[hash])
+		if (!tls->handshake_hash[hash]) {
+			TLS_DEBUG("Can't create %s hash",
+					tls_handshake_hash_data[hash].name);
 			goto err;
+		}
 	}
 
 	return true;
@@ -500,6 +527,31 @@ enum tls_handshake_type {
 	TLS_FINISHED		= 20,
 };
 
+#define SWITCH_ENUM_TO_STR(val) \
+	case (val):		\
+		return L_STRINGIFY(val);
+
+static const char *tls_handshake_type_to_str(enum tls_handshake_type type)
+{
+	static char buf[40];
+
+	switch (type) {
+	SWITCH_ENUM_TO_STR(TLS_HELLO_REQUEST)
+	SWITCH_ENUM_TO_STR(TLS_CLIENT_HELLO)
+	SWITCH_ENUM_TO_STR(TLS_SERVER_HELLO)
+	SWITCH_ENUM_TO_STR(TLS_CERTIFICATE)
+	SWITCH_ENUM_TO_STR(TLS_SERVER_KEY_EXCHANGE)
+	SWITCH_ENUM_TO_STR(TLS_CERTIFICATE_REQUEST)
+	SWITCH_ENUM_TO_STR(TLS_SERVER_HELLO_DONE)
+	SWITCH_ENUM_TO_STR(TLS_CERTIFICATE_VERIFY)
+	SWITCH_ENUM_TO_STR(TLS_CLIENT_KEY_EXCHANGE)
+	SWITCH_ENUM_TO_STR(TLS_FINISHED)
+	}
+
+	snprintf(buf, sizeof(buf), "tls_handshake_type(%i)", type);
+	return buf;
+}
+
 static const uint8_t pkcs1_digest_info_md5_start[] = {
 	0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d,
 	0x02, 0x05, 0x05, 0x00, 0x04, 0x10,
@@ -564,6 +616,9 @@ static void tls_send_alert(struct l_tls *tls, bool fatal,
 {
 	uint8_t buf[2];
 
+	TLS_DEBUG("Sending a %s Alert: %s", fatal ? "Fatal" : "Warning",
+			l_tls_alert_to_str(alert_desc));
+
 	buf[0] = fatal ? 2 : 1;
 	buf[1] = alert_desc;
 
@@ -601,6 +656,10 @@ static void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf,
 {
 	int i;
 
+	TLS_DEBUG("Sending a %s of %zi bytes",
+			tls_handshake_type_to_str(type),
+			length - TLS_HANDSHAKE_HEADER_SIZE);
+
 	/* Fill in the handshake header */
 
 	buf[0] = type;
@@ -693,15 +752,18 @@ static bool tls_send_certificate(struct l_tls *tls)
 		cert = NULL;
 
 	if (tls->server && !cert) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-						TLS_ALERT_BAD_CERT);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
+				"Loading server certificate %s failed",
+				tls->cert_path);
 		return false;
 	}
 
 	if (cert && !tls_cert_find_certchain(cert, tls->ca_cert_path)) {
 		if (tls->server) {
-			tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-					TLS_ALERT_UNKNOWN_CA);
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR,
+					TLS_ALERT_UNKNOWN_CA,
+					"Can't find certificate chain local "
+					"CA cert %s", tls->ca_cert_path);
 
 			return false;
 		} else
@@ -711,8 +773,10 @@ static bool tls_send_certificate(struct l_tls *tls)
 	/* TODO: might want check this earlier and exclude the cipher suite */
 	if (cert && !tls->pending.cipher_suite->key_xchg->
 			validate_cert_key_type(cert)) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-				TLS_ALERT_CERT_UNKNOWN);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_CERT_UNKNOWN,
+				"Local certificate has key type incompatible "
+				"with pending cipher suite %s",
+				tls->pending.cipher_suite->name);
 
 		return false;
 	}
@@ -898,7 +962,8 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 	ssize_t bytes_encrypted;
 
 	if (!tls->peer_pubkey) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Peer public key not received");
 
 		return false;
 	}
@@ -908,7 +973,9 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 	l_getrandom(pre_master_secret + 2, 46);
 
 	if (tls->peer_pubkey_size + 32 > (int) sizeof(buf)) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Peer public key too big: %zi",
+				tls->peer_pubkey_size);
 
 		return false;
 	}
@@ -921,7 +988,9 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 	ptr += tls->peer_pubkey_size + 2;
 
 	if (bytes_encrypted != (ssize_t) tls->peer_pubkey_size) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Encrypting PreMasterSecret failed: %s",
+				strerror(-bytes_encrypted));
 
 		return false;
 	}
@@ -946,8 +1015,8 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
 	size_t expected_bytes;
 
 	if (!tls->priv_key || !tls->priv_key_size) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-				TLS_ALERT_BAD_CERT);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
+				"No private key loaded");
 
 		return -ENOKEY;
 	}
@@ -988,7 +1057,9 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
 	}
 
 	if (result < 0)
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Signing the hash failed: %s",
+				strerror(-result));
 
 	return result;
 }
@@ -1011,14 +1082,18 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 
 	if (len < offset + 2 ||
 			(size_t) l_get_be16(in + offset) + offset + 2 != len) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0, "Signature msg too "
+				"short (%zi) or signature length doesn't match",
+				len);
 
 		return false;
 	}
 
 	/* Only the default hash type supported */
 	if (len != offset + 2 + tls->peer_pubkey_size) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Signature length %zi not equal %zi", len,
+				offset + 2 + tls->peer_pubkey_size);
 
 		return false;
 	}
@@ -1026,13 +1101,16 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 	if (tls->negotiated_version >= TLS_V12) {
 		/* Only RSA supported */
 		if (in[1] != 1 /* RSA_sign */) {
-			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Unknown signature algorithm %i",
+					in[1]);
 
 			return false;
 		}
 
 		if (!get_hash(tls, in[0], hash, &hash_len, &hash_type)) {
-			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Unknown hash type %i", in[0]);
 
 			return false;
 		}
@@ -1071,7 +1149,10 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 				expected_len, tls->peer_pubkey_size);
 
 	if (!success)
-		tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+				"Peer signature verification failed");
+	else
+		TLS_DEBUG("Peer signature verified");
 
 	return success;
 }
@@ -1190,7 +1271,9 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
 	size_t seed_len;
 
 	if (len != (size_t) tls->cipher_suite[0]->verify_data_length) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"TLS_FINISHED length not %i",
+				tls->cipher_suite[0]->verify_data_length);
 
 		return false;
 	}
@@ -1214,7 +1297,8 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
 				tls->cipher_suite[0]->verify_data_length);
 
 	if (memcmp(received, expected, len)) {
-		tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+				"TLS_FINISHED contents don't match");
 
 		return false;
 	}
@@ -1306,7 +1390,9 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	tls->client_version = l_get_be16(buf);
 
 	if (tls->client_version < TLS_MIN_VERSION) {
-		tls_disconnect(tls, TLS_ALERT_PROTOCOL_VERSION, 0);
+		TLS_DISCONNECT(TLS_ALERT_PROTOCOL_VERSION, 0,
+				"Client version too low: %02x",
+				tls->client_version);
 		return;
 	}
 
@@ -1319,6 +1405,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
 			if (i != HANDSHAKE_HASH_SHA1 && i != HANDSHAKE_HASH_MD5)
 				tls_drop_handshake_hash(tls, i);
 
+	TLS_DEBUG("Negotiated TLS 1.%i", (tls->negotiated_version & 0xff) - 1);
+
 	/* Select a cipher suite according to client's preference list */
 	while (cipher_suites_size) {
 		/*
@@ -1338,15 +1426,21 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	}
 
 	if (!cipher_suites_size) {
-		tls_disconnect(tls, TLS_ALERT_HANDSHAKE_FAIL, 0);
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"No common cipher suites");
 		return;
 	}
 
+	TLS_DEBUG("Negotiated %s", tls->pending.cipher_suite->name);
+
 	/* Select a compression method */
 
 	/* CompressionMethod.null must be present in the vector */
-	if (!memchr(compression_methods, 0, compression_methods_size))
-		goto decode_error;
+	if (!memchr(compression_methods, 0, compression_methods_size)) {
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"No common compression methods");
+		return;
+	}
 
 	while (compression_methods_size) {
 		tls->pending.compression_method =
@@ -1359,6 +1453,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
 		compression_methods_size--;
 	}
 
+	TLS_DEBUG("Negotiated %s", tls->pending.compression_method->name);
+
 	tls_send_server_hello(tls);
 
 	if (tls->pending.cipher_suite->key_xchg->certificate_check &&
@@ -1376,14 +1472,15 @@ static void tls_handle_client_hello(struct l_tls *tls,
 
 	if (tls->pending.cipher_suite->key_xchg->certificate_check &&
 			tls->ca_cert_path)
-		tls->state = TLS_HANDSHAKE_WAIT_CERTIFICATE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CERTIFICATE);
 	else
-		tls->state = TLS_HANDSHAKE_WAIT_KEY_EXCHANGE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE);
 
 	return;
 
 decode_error:
-	tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"ClientHello decode error");
 }
 
 static void tls_handle_server_hello(struct l_tls *tls,
@@ -1410,7 +1507,8 @@ static void tls_handle_server_hello(struct l_tls *tls,
 	len -= session_id_size + 2 + 1;
 
 	if (len != 0) { /* We know we haven't solicited any extensions */
-		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_EXTENSION, 0);
+		TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_EXTENSION, 0,
+				"ServerHello contains extensions");
 		return;
 	}
 
@@ -1418,9 +1516,11 @@ static void tls_handle_server_hello(struct l_tls *tls,
 
 	if (tls->negotiated_version < TLS_MIN_VERSION ||
 			tls->negotiated_version > TLS_VERSION) {
-		tls_disconnect(tls, tls->negotiated_version < TLS_MIN_VERSION ?
+		TLS_DISCONNECT(tls->negotiated_version < TLS_MIN_VERSION ?
 				TLS_ALERT_PROTOCOL_VERSION :
-				TLS_ALERT_ILLEGAL_PARAM, 0);
+				TLS_ALERT_ILLEGAL_PARAM, 0,
+				"Unsupported version %02x",
+				tls->negotiated_version);
 		return;
 	}
 
@@ -1430,29 +1530,40 @@ static void tls_handle_server_hello(struct l_tls *tls,
 			if (i != HANDSHAKE_HASH_SHA1 && i != HANDSHAKE_HASH_MD5)
 				tls_drop_handshake_hash(tls, i);
 
+	TLS_DEBUG("Negotiated TLS 1.%i", (tls->negotiated_version & 0xff) - 1);
+
 	/* Set the new cipher suite and compression method structs */
 	tls->pending.cipher_suite = tls_find_cipher_suite(cipher_suite_id);
 	if (!tls->pending.cipher_suite) {
-		tls_disconnect(tls, TLS_ALERT_HANDSHAKE_FAIL, 0);
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Unknown cipher suite %04x",
+				l_get_be16(cipher_suite_id));
 		return;
 	}
 
+	TLS_DEBUG("Negotiated %s", tls->pending.cipher_suite->name);
+
 	tls->pending.compression_method =
 		tls_find_compression_method(compression_method_id);
 	if (!tls->pending.compression_method) {
-		tls_disconnect(tls, TLS_ALERT_HANDSHAKE_FAIL, 0);
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Unknown compression method %i",
+				compression_method_id);
 		return;
 	}
 
+	TLS_DEBUG("Negotiated %s", tls->pending.compression_method->name);
+
 	if (tls->pending.cipher_suite->key_xchg->certificate_check)
-		tls->state = TLS_HANDSHAKE_WAIT_CERTIFICATE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CERTIFICATE);
 	else
-		tls->state = TLS_HANDSHAKE_WAIT_KEY_EXCHANGE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE);
 
 	return;
 
 decode_error:
-	tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"ServerHello decode error");
 }
 
 static void tls_handle_certificate(struct l_tls *tls,
@@ -1473,8 +1584,12 @@ static void tls_handle_certificate(struct l_tls *tls,
 	if (total + 3 != len)
 		goto decode_error;
 
-	if (tls_cert_from_certificate_list(buf, total, &certchain) < 0)
-		goto decode_error;
+	if (tls_cert_from_certificate_list(buf, total, &certchain) < 0) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Error decoding peer certificate chain");
+
+		goto done;
+	}
 
 	/*
 	 * "Note that a client MAY send no certificates if it does not have any
@@ -1486,12 +1601,13 @@ static void tls_handle_certificate(struct l_tls *tls,
 	 */
 	if (!certchain) {
 		if (!tls->server) {
-			tls_disconnect(tls, TLS_ALERT_HANDSHAKE_FAIL, 0);
+			TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+					"Server sent no certificate chain");
 
 			goto done;
 		}
 
-		tls->state = TLS_HANDSHAKE_WAIT_KEY_EXCHANGE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE);
 
 		goto done;
 	}
@@ -1504,15 +1620,18 @@ static void tls_handle_certificate(struct l_tls *tls,
 	if (tls->ca_cert_path) {
 		ca_cert = tls_cert_load_file(tls->ca_cert_path);
 		if (!ca_cert) {
-			tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-					TLS_ALERT_BAD_CERT);
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR,
+					TLS_ALERT_BAD_CERT,
+					"Can't load %s", tls->ca_cert_path);
 
 			goto done;
 		}
 	}
 
 	if (!tls_cert_verify_certchain(certchain, ca_cert)) {
-		tls_disconnect(tls, TLS_ALERT_BAD_CERT, 0);
+		TLS_DISCONNECT(TLS_ALERT_BAD_CERT, 0,
+				"Peer certchain verification failed against "
+				"CA cert %s", tls->ca_cert_path);
 
 		goto done;
 	}
@@ -1525,7 +1644,10 @@ static void tls_handle_certificate(struct l_tls *tls,
 	 */
 	if (!tls->pending.cipher_suite->key_xchg->
 			validate_cert_key_type(certchain)) {
-		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
+		TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_CERT, 0,
+				"Peer certificate key type incompatible with "
+				"pending cipher suite %s",
+				tls->pending.cipher_suite->name);
 
 		goto done;
 	}
@@ -1540,7 +1662,8 @@ static void tls_handle_certificate(struct l_tls *tls,
 					tls->peer_cert->size);
 
 	if (!tls->peer_pubkey) {
-		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
+		TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_CERT, 0,
+				"Error loading peer public key to kernel");
 
 		goto done;
 	}
@@ -1548,7 +1671,8 @@ static void tls_handle_certificate(struct l_tls *tls,
 	if (!l_key_get_info(tls->peer_pubkey, L_KEY_RSA_PKCS1_V1_5,
 					L_CHECKSUM_NONE, &tls->peer_pubkey_size,
 					&dummy)) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Can't l_key_get_info for peer public key");
 
 		goto done;
 	}
@@ -1556,14 +1680,15 @@ static void tls_handle_certificate(struct l_tls *tls,
 	tls->peer_pubkey_size /= 8;
 
 	if (tls->server)
-		tls->state = TLS_HANDSHAKE_WAIT_KEY_EXCHANGE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE);
 	else
-		tls->state = TLS_HANDSHAKE_WAIT_HELLO_DONE;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
 
 	goto done;
 
 decode_error:
-	tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"TLS_CERTIFICATE decode error");
 
 done:
 	if (ca_cert)
@@ -1649,7 +1774,8 @@ static void tls_handle_certificate_request(struct l_tls *tls,
 		else if ((int) first_supported != -1)
 			tls->signature_hash = first_supported;
 		else {
-			tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_CERT, 0,
+					"No supported signature hash type");
 
 			return;
 		}
@@ -1672,14 +1798,18 @@ static void tls_handle_certificate_request(struct l_tls *tls,
 	return;
 
 decode_error:
-	tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"CertificateRequest decode error");
 }
 
 static void tls_handle_server_hello_done(struct l_tls *tls,
 						const uint8_t *buf, size_t len)
 {
+	const char *error;
+
 	if (len) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"ServerHello not empty");
 		return;
 	}
 
@@ -1696,14 +1826,15 @@ static void tls_handle_server_hello_done(struct l_tls *tls,
 
 	tls_send_change_cipher_spec(tls);
 
-	if (!tls_change_cipher_spec(tls, 1)) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+	if (!tls_change_cipher_spec(tls, 1, &error)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"change_cipher_spec: %s", error);
 		return;
 	}
 
 	tls_send_finished(tls);
 
-	tls->state = TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC;
+	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 }
 
 static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
@@ -1713,14 +1844,16 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
 	ssize_t bytes_decrypted;
 
 	if (!tls->priv_key || !tls->priv_key_size) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-				TLS_ALERT_BAD_CERT);
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
+				"No private key");
 
 		return;
 	}
 
 	if (len != tls->priv_key_size + 2) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"ClientKeyExchange len %zi not %zi", len,
+				tls->priv_key_size + 2);
 
 		return;
 	}
@@ -1728,7 +1861,9 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
 	len = l_get_be16(buf);
 
 	if (len != tls->priv_key_size) {
-		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"EncryptedPreMasterSecret len %zi not %zi",
+				len, tls->priv_key_size);
 
 		return;
 	}
@@ -1753,9 +1888,13 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
 	pre_master_secret[0] = tls->client_version >> 8;
 	pre_master_secret[1] = tls->client_version >> 0;
 
-	if (bytes_decrypted != 48)
+	if (bytes_decrypted != 48) {
 		memcpy(pre_master_secret + 2, random_secret, 46);
 
+		TLS_DEBUG("Error decrypting PreMasterSecret: %s",
+				strerror(-bytes_decrypted));
+	}
+
 	tls_generate_master_secret(tls, pre_master_secret, 48);
 	memset(pre_master_secret, 0, 48);
 	memset(random_secret, 0, 46);
@@ -1819,7 +1958,7 @@ static void tls_handle_certificate_verify(struct l_tls *tls,
 	 */
 	tls->peer_authenticated = true;
 
-	tls->state = TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC;
+	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 }
 
 static void tls_finished(struct l_tls *tls)
@@ -1829,7 +1968,7 @@ static void tls_finished(struct l_tls *tls)
 	/* Free up the resources used in the handshake */
 	tls_reset_handshake(tls);
 
-	tls->state = TLS_HANDSHAKE_DONE;
+	TLS_SET_STATE(TLS_HANDSHAKE_DONE);
 	tls->ready = true;
 
 	tls->ready_handle(peer_identity, tls->user_data);
@@ -1843,15 +1982,20 @@ static void tls_finished(struct l_tls *tls)
 static void tls_handle_handshake(struct l_tls *tls, int type,
 					const uint8_t *buf, size_t len)
 {
+	TLS_DEBUG("Handling a %s of %zi bytes",
+			tls_handshake_type_to_str(type), len);
+
 	switch (type) {
 	case TLS_HELLO_REQUEST:
 		if (tls->server) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in server mode");
 			break;
 		}
 
 		if (len != 0) {
-			tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+					"HelloRequest not empty");
 			break;
 		}
 
@@ -1866,13 +2010,16 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 	case TLS_CLIENT_HELLO:
 		if (!tls->server) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in client mode");
 			break;
 		}
 
 		if (tls->state != TLS_HANDSHAKE_WAIT_HELLO &&
 				tls->state != TLS_HANDSHAKE_DONE) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1882,12 +2029,15 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 	case TLS_SERVER_HELLO:
 		if (tls->server) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in server mode");
 			break;
 		}
 
 		if (tls->state != TLS_HANDSHAKE_WAIT_HELLO) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1897,7 +2047,9 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 	case TLS_CERTIFICATE:
 		if (tls->state != TLS_HANDSHAKE_WAIT_CERTIFICATE) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1914,7 +2066,10 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 				tls->cert_requested ||
 				!tls->pending.cipher_suite->key_xchg->
 				certificate_check) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in current state "
+					"or certificate check not supported "
+					"in pending cipher suite");
 			break;
 		}
 
@@ -1924,7 +2079,9 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 	case TLS_SERVER_HELLO_DONE:
 		if (tls->state != TLS_HANDSHAKE_WAIT_HELLO_DONE) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1934,7 +2091,9 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 	case TLS_CERTIFICATE_VERIFY:
 		if (tls->state != TLS_HANDSHAKE_WAIT_CERTIFICATE_VERIFY) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1944,12 +2103,15 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 	case TLS_CLIENT_KEY_EXCHANGE:
 		if (!tls->server) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in client mode");
 			break;
 		}
 
 		if (tls->state != TLS_HANDSHAKE_WAIT_KEY_EXCHANGE) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1964,15 +2126,17 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 		 * this isn't 100% clear.
 		 */
 		if (tls->peer_pubkey)
-			tls->state = TLS_HANDSHAKE_WAIT_CERTIFICATE_VERIFY;
+			TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CERTIFICATE_VERIFY);
 		else
-			tls->state = TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC;
+			TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 
 		break;
 
 	case TLS_FINISHED:
 		if (tls->state != TLS_HANDSHAKE_WAIT_FINISHED) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 			break;
 		}
 
@@ -1980,10 +2144,13 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 			break;
 
 		if (tls->server) {
+			const char *error;
+
 			tls_send_change_cipher_spec(tls);
-			if (!tls_change_cipher_spec(tls, 1)) {
-				tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-						0);
+			if (!tls_change_cipher_spec(tls, 1, &error)) {
+				TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+						"change_cipher_spec: %s",
+						error);
 				break;
 			}
 			tls_send_finished(tls);
@@ -2014,7 +2181,8 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 		break;
 
 	default:
-		tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+		TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+				"Invalid message");
 	}
 }
 
@@ -2048,7 +2216,7 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
 		tls_send_client_hello(tls);
 	}
 
-	tls->state = TLS_HANDSHAKE_WAIT_HELLO;
+	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);
 
 	return tls;
 }
@@ -2078,6 +2246,9 @@ LIB_EXPORT void l_tls_free(struct l_tls *tls)
 	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
 		tls_drop_handshake_hash(tls, hash);
 
+	if (tls->debug_destroy)
+		tls->debug_destroy(tls->debug_data);
+
 	l_free(tls);
 }
 
@@ -2094,35 +2265,42 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 			int len, enum tls_content_type type, uint16_t version)
 {
 	enum handshake_hash_type hash;
+	const char *error;
 
 	switch (type) {
 	case TLS_CT_CHANGE_CIPHER_SPEC:
 		if (len != 1 || message[0] != 0x01) {
-			tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+					"ChangeCipherSpec msg decode error");
 
 			return false;
 		}
 
 		if (tls->state != TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"ChangeCipherSpec invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
 
 			return false;
 		}
 
-		if (!tls_change_cipher_spec(tls, 0)) {
-			tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
+		if (!tls_change_cipher_spec(tls, 0, &error)) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"change_cipher_spec: %s", error);
 
 			return false;
 		}
 
-		tls->state = TLS_HANDSHAKE_WAIT_FINISHED;
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_FINISHED);
 
 		return true;
 
 	case TLS_CT_ALERT:
 		/* Verify AlertLevel */
 		if (message[0] != 0x01 && message[0] != 0x02) {
-			tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
+			TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+					"Received bad AlertLevel %i",
+					message[0]);
 
 			return false;
 		}
@@ -2136,7 +2314,10 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 		 * a fatal alert, then disconnect, so we do that
 		 * regardless of the alert level.
 		 */
-		tls_disconnect(tls, TLS_ALERT_CLOSE_NOTIFY, message[1]);
+		TLS_DISCONNECT(TLS_ALERT_CLOSE_NOTIFY, message[1],
+				"Peer sent a %s Alert: %s",
+				message[0] == 0x02 ? "Fatal" : "Warning",
+				l_tls_alert_to_str(message[1]));
 
 		return false;
 
@@ -2196,7 +2377,9 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 
 	case TLS_CT_APPLICATION_DATA:
 		if (!tls->ready) {
-			tls_disconnect(tls, TLS_ALERT_UNEXPECTED_MESSAGE, 0);
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Application data message before "
+					"handshake finished");
 
 			return false;
 		}
@@ -2214,11 +2397,13 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 
 LIB_EXPORT void l_tls_close(struct l_tls *tls)
 {
-	tls_disconnect(tls, TLS_ALERT_CLOSE_NOTIFY, 0);
+	TLS_DISCONNECT(TLS_ALERT_CLOSE_NOTIFY, 0, "Closing session");
 }
 
 LIB_EXPORT void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path)
 {
+	TLS_DEBUG("ca-cert-path=%s", ca_cert_path);
+
 	if (tls->ca_cert_path) {
 		l_free(tls->ca_cert_path);
 		tls->ca_cert_path = NULL;
@@ -2232,6 +2417,9 @@ LIB_EXPORT bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
 					const char *priv_key_path,
 					const char *priv_key_passphrase)
 {
+	TLS_DEBUG("cert-path=%s priv-key-path=%s priv-key-passphrase=%p",
+			cert_path, priv_key_path, priv_key_passphrase);
+
 	if (tls->cert_path) {
 		l_free(tls->cert_path);
 		tls->cert_path = NULL;
@@ -2251,6 +2439,9 @@ LIB_EXPORT bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
 							priv_key_passphrase,
 							NULL,
 							&tls->priv_key_size);
+		TLS_DEBUG("l_pem_load_private_key returned %p, size %zi",
+				priv_key, tls->priv_key_size);
+
 		if (!priv_key)
 			return false;
 
@@ -2262,6 +2453,7 @@ LIB_EXPORT bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
 		if (!l_key_get_info(tls->priv_key, L_KEY_RSA_PKCS1_V1_5,
 					L_CHECKSUM_NONE, &tls->priv_key_size,
 					&is_public) || is_public) {
+			TLS_DEBUG("Not a private key or l_key_get_info failed");
 			l_key_free(tls->priv_key);
 			tls->priv_key = NULL;
 			tls->priv_key_size = 0;
@@ -2335,6 +2527,25 @@ LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
 	return NULL;
 }
 
+const char *tls_handshake_state_to_str(enum tls_handshake_state state)
+{
+	static char buf[40];
+
+	switch (state) {
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_HELLO)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_CERTIFICATE)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_HELLO_DONE)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_CERTIFICATE_VERIFY)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_FINISHED)
+	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_DONE)
+	}
+
+	snprintf(buf, sizeof(buf), "tls_handshake_state(%i)", state);
+	return buf;
+}
+
 /* X509 Certificates and Certificate Chains */
 
 #define X509_CERTIFICATE_POS			0
@@ -2581,3 +2792,19 @@ enum tls_cert_key_type tls_cert_get_pubkey_type(struct tls_cert *cert)
 
 	return pkcs1_encryption_oids[i].key_type;
 }
+
+LIB_EXPORT bool l_tls_set_debug(struct l_tls *tls, l_tls_debug_cb_t function,
+				void *user_data, l_tls_destroy_cb_t destroy)
+{
+	if (unlikely(!tls))
+		return false;
+
+	if (tls->debug_destroy)
+		tls->debug_destroy(tls->debug_data);
+
+	tls->debug_handler = function;
+	tls->debug_destroy = destroy;
+	tls->debug_data = user_data;
+
+	return true;
+}
diff --git a/ell/tls.h b/ell/tls.h
index 9ab53fa..893fd63 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -60,6 +60,9 @@ typedef void (*l_tls_write_cb_t)(const uint8_t *data, size_t len,
 typedef void (*l_tls_ready_cb_t)(const char *peer_identity, void *user_data);
 typedef void (*l_tls_disconnect_cb_t)(enum l_tls_alert_desc reason,
 					bool remote, void *user_data);
+typedef void (*l_tls_debug_cb_t)(const char *str, void *user_data);
+typedef void (*l_tls_destroy_cb_t)(void *user_data);
+
 
 /*
  * app_data_handler gets called with newly received decrypted data.
@@ -110,6 +113,9 @@ bool l_tls_prf_get_bytes(struct l_tls *tls,
 				size_t hash_len, bool use_master_secret,
 				const char *label, uint8_t *buf, size_t len);
 
+bool l_tls_set_debug(struct l_tls *tls, l_tls_debug_cb_t function,
+			void *user_data, l_tls_destroy_cb_t destroy);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 4/7] pem: Add l_pem_load_certificate_list utility
  2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 2/7] Add format string checking to a few declarations Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 3/7] tls: Add debug logging API Andrew Zaborowski
@ 2018-11-01  2:12 ` Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 5/7] tls: Support loading multiple root CAs Andrew Zaborowski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 4415 bytes --]

l_pem_load_certificate_list loads all the certificates in a file with
multiple concatenated PEM structures.  This could also be done
previously by calling l_pem_load_file with consecutive index values
until it returned an error, this is a faster way to load all of the
certificates.
---
 ell/pem.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 ell/pem.h |  8 ++++++
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/ell/pem.c b/ell/pem.c
index e24dd7d..1c5c986 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -39,6 +39,7 @@
 #include "asn1-private.h"
 #include "pkcs5-private.h"
 #include "cipher.h"
+#include "queue.h"
 
 #define PEM_START_BOUNDARY	"-----BEGIN "
 #define PEM_END_BOUNDARY	"-----END "
@@ -119,7 +120,8 @@ static bool is_end_boundary(const uint8_t *buf, size_t buf_len,
 }
 
 static uint8_t *pem_load_buffer(const uint8_t *buf, size_t buf_len, int index,
-				char **type_label, size_t *len)
+				char **type_label, size_t *len,
+				size_t *consumed)
 {
 	const uint8_t *base64_data = NULL, *label = NULL, *eol;
 	uint8_t *data;
@@ -152,6 +154,9 @@ static uint8_t *pem_load_buffer(const uint8_t *buf, size_t buf_len, int index,
 				*type_label = l_strndup((const char *) label,
 							label_len);
 
+				if (consumed)
+					*consumed = eol + 1 - buf;
+
 				return data;
 			}
 
@@ -178,7 +183,7 @@ LIB_EXPORT uint8_t *l_pem_load_buffer(const uint8_t *buf, size_t buf_len,
 					int index, char **type_label,
 					size_t *out_len)
 {
-	return pem_load_buffer(buf, buf_len, index, type_label, out_len);
+	return pem_load_buffer(buf, buf_len, index, type_label, out_len, NULL);
 }
 
 LIB_EXPORT uint8_t *l_pem_load_file(const char *filename, int index,
@@ -205,7 +210,7 @@ LIB_EXPORT uint8_t *l_pem_load_file(const char *filename, int index,
 		return NULL;
 	}
 
-	result = pem_load_buffer(data, st.st_size, index, type_label, len);
+	result = pem_load_buffer(data, st.st_size, index, type_label, len, NULL);
 
 	munmap(data, st.st_size);
 	close(fd);
@@ -233,6 +238,78 @@ LIB_EXPORT uint8_t *l_pem_load_certificate(const char *filename, size_t *len)
 	return content;
 }
 
+static void pem_element_free(void *data)
+{
+	struct l_pem_list_element *element = data;
+
+	l_free(element->content);
+	l_free(element);
+}
+
+LIB_EXPORT struct l_queue *l_pem_load_certificate_list(const char *filename)
+{
+	int fd;
+	struct stat st;
+	uint8_t *data;
+	size_t start = 0;
+	struct l_queue *result = NULL;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	if (fstat(fd, &st) < 0) {
+		close(fd);
+
+		return NULL;
+	}
+
+	data = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (data == MAP_FAILED) {
+		close(fd);
+
+		return NULL;
+	}
+
+	while (1) {
+		uint8_t *certificate;
+		size_t len, consumed;
+		char *label;
+		struct l_pem_list_element *element;
+
+		certificate = pem_load_buffer(data + start, st.st_size - start, 0,
+						&label, &len, &consumed);
+		if (!certificate)
+			goto done;
+
+		if (strcmp(label, "CERTIFICATE")) {
+			l_free(certificate);
+			break;
+		}
+
+		l_free(label);
+		element = l_malloc(sizeof(struct l_pem_list_element));
+		element->content = certificate;
+		element->len = len;
+
+		if (!result)
+			result = l_queue_new();
+
+		l_queue_push_tail(result, element);
+		start += consumed;
+	}
+
+	/* Error occured */
+	l_queue_destroy(result, pem_element_free);
+	result = NULL;
+
+done:
+	munmap(data, st.st_size);
+	close(fd);
+
+	return result;
+}
+
 /**
  * l_pem_load_private_key
  * @filename: path string to the PEM file to load
diff --git a/ell/pem.h b/ell/pem.h
index 93282a3..9cead5c 100644
--- a/ell/pem.h
+++ b/ell/pem.h
@@ -27,12 +27,20 @@
 extern "C" {
 #endif
 
+struct l_queue;
+
+struct l_pem_list_element {
+	uint8_t *content;
+	size_t len;
+};
+
 uint8_t *l_pem_load_buffer(const uint8_t *buf, size_t buf_len,
 				int index, char **type_label, size_t *out_len);
 uint8_t *l_pem_load_file(const char *filename, int index,
 				char **type_label, size_t *len);
 
 uint8_t *l_pem_load_certificate(const char *filename, size_t *len);
+struct l_queue *l_pem_load_certificate_list(const char *filename);
 
 uint8_t *l_pem_load_private_key(const char *filename, const char *passphrase,
 				bool *encrypted, size_t *len);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 5/7] tls: Support loading multiple root CAs
  2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
                   ` (2 preceding siblings ...)
  2018-11-01  2:12 ` [PATCH 4/7] pem: Add l_pem_load_certificate_list utility Andrew Zaborowski
@ 2018-11-01  2:12 ` Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 6/7] tools: Update tls_cert_verify_certchain parameters Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 7/7] unit: " Andrew Zaborowski
  5 siblings, 0 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 9043 bytes --]

If a file with multiple concatenated certificates is supplied to
set_auth_data, load all of them and verify received certificate chains
aginst any of them instead of just the first one.  This is also what
wpa_supplicant does for files supplied with the ca_cert= setting and
it's actually useful.  Some enterprise CAs have multiple root
certificates and provision clients with a file that contains all of them
concatenated and it does happen that they switch from a certificate
chain using one root to a new chain with a different root for their wifi
in which case our network config files break while wpa_supplicant
configs keep working.

Also simplify tls_cert_verify_certchain slightly.
---
 ell/tls-private.h |   3 +-
 ell/tls.c         | 137 ++++++++++++++++++++++++++++++++--------------
 ell/tls.h         |   2 +-
 3 files changed, 100 insertions(+), 42 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 4f680a0..95b8c65 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -240,6 +240,7 @@ enum tls_cert_key_type {
 };
 
 struct tls_cert *tls_cert_load_file(const char *filename);
+struct l_queue *tls_cert_list_load_file(const char *filename);
 int tls_cert_from_certificate_list(const void *data, size_t len,
 					struct tls_cert **out_certchain);
 
@@ -247,7 +248,7 @@ bool tls_cert_find_certchain(struct tls_cert *cert,
 				const char *cacert_filename);
 
 bool tls_cert_verify_certchain(struct tls_cert *certchain,
-				struct tls_cert *ca_cert);
+				struct l_queue *ca_certs);
 
 void tls_cert_free_certchain(struct tls_cert *cert);
 
diff --git a/ell/tls.c b/ell/tls.c
index 4b37785..fbbc1bf 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -38,6 +38,7 @@
 #include "tls-private.h"
 #include "key.h"
 #include "asn1-private.h"
+#include "queue.h"
 
 void tls10_prf(const void *secret, size_t secret_len,
 		const char *label,
@@ -1571,7 +1572,7 @@ static void tls_handle_certificate(struct l_tls *tls,
 {
 	size_t total;
 	struct tls_cert *certchain = NULL;
-	struct tls_cert *ca_cert = NULL;
+	struct l_queue *ca_certs = NULL;
 	bool dummy;
 
 	if (len < 3)
@@ -1614,12 +1615,12 @@ static void tls_handle_certificate(struct l_tls *tls,
 
 	/*
 	 * Validate the certificate chain's consistency and validate it
-	 * against our CA if we have any.
+	 * against our CAs if we have any.
 	 */
 
 	if (tls->ca_cert_path) {
-		ca_cert = tls_cert_load_file(tls->ca_cert_path);
-		if (!ca_cert) {
+		ca_certs = tls_cert_list_load_file(tls->ca_cert_path);
+		if (!ca_certs) {
 			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR,
 					TLS_ALERT_BAD_CERT,
 					"Can't load %s", tls->ca_cert_path);
@@ -1628,10 +1629,10 @@ static void tls_handle_certificate(struct l_tls *tls,
 		}
 	}
 
-	if (!tls_cert_verify_certchain(certchain, ca_cert)) {
+	if (!tls_cert_verify_certchain(certchain, ca_certs)) {
 		TLS_DISCONNECT(TLS_ALERT_BAD_CERT, 0,
 				"Peer certchain verification failed against "
-				"CA cert %s", tls->ca_cert_path);
+				"CA cert(s) in %s", tls->ca_cert_path);
 
 		goto done;
 	}
@@ -1691,8 +1692,8 @@ decode_error:
 			"TLS_CERTIFICATE decode error");
 
 done:
-	if (ca_cert)
-		l_free(ca_cert);
+	if (ca_certs)
+		l_queue_destroy(ca_certs, l_free);
 
 	tls_cert_free_certchain(certchain);
 }
@@ -2592,6 +2593,44 @@ struct tls_cert *tls_cert_load_file(const char *filename)
 	return cert;
 }
 
+struct l_queue *tls_cert_list_load_file(const char *filename)
+{
+	struct l_queue *pem_list;
+	struct l_queue *cert_list;
+	bool error = false;
+
+	pem_list = l_pem_load_certificate_list(filename);
+	if (!pem_list)
+		return NULL;
+
+	cert_list = l_queue_new();
+
+	while (!l_queue_isempty(pem_list)) {
+		struct tls_cert *cert;
+		struct l_pem_list_element *elem = l_queue_pop_head(pem_list);
+		uint8_t *der = elem->content;
+
+		if (!elem->len || der[0] != ASN1_ID_SEQUENCE)
+			error = true;
+
+		cert = l_malloc(sizeof(struct tls_cert) + elem->len);
+		cert->size = elem->len;
+		cert->issuer = NULL;
+		memcpy(cert->asn1, der, cert->size);
+		l_queue_push_tail(cert_list, cert);
+		l_free(elem->content);
+		l_free(elem);
+	}
+
+	l_queue_destroy(pem_list, NULL);
+
+	if (!error)
+		return cert_list;
+
+	l_queue_destroy(cert_list, l_free);
+	return NULL;
+}
+
 int tls_cert_from_certificate_list(const void *data, size_t len,
 					struct tls_cert **out_certchain)
 {
@@ -2653,9 +2692,18 @@ static void tls_key_cleanup(struct l_key **p)
 	l_key_free_norevoke(*p);
 }
 
+static bool tls_cert_link(struct tls_cert *cert, struct l_keyring *ring)
+{
+	L_AUTO_CLEANUP_VAR(struct l_key *, key, tls_key_cleanup) = NULL;
+
+	key = l_key_new(L_KEY_RSA, cert->asn1, cert->size);
+
+	return key && l_keyring_link(ring, key);
+}
+
 static bool tls_cert_verify_with_keyring(struct tls_cert *cert,
 						struct l_keyring *ring,
-						struct tls_cert *root,
+						struct l_queue *roots,
 						struct l_keyring *trusted)
 {
 	if (!cert)
@@ -2668,19 +2716,30 @@ static bool tls_cert_verify_with_keyring(struct tls_cert *cert,
 	 * specifies the root certificate authority MAY be omitted from
 	 * the chain, under the assumption that the remote end must
 	 * already possess it in order to validate it in any case."
+	 *
+	 * This is an optimization to skip adding the root cert if
+	 * it's already in the trusted keyring.  It also happens to
+	 * work around a kernel issue preventing self-signed certificates
+	 * missing the AKID extension from being linked.
+	 *
+	 * If 'roots' were supplied we assume the keyring is already
+	 * restricted.
 	 */
-	if (!cert->issuer && root && cert->size == root->size &&
-			!memcmp(cert->asn1, root->asn1, root->size))
-		return true;
+	if (!cert->issuer && roots) {
+		const struct l_queue_entry *entry;
 
-	if (tls_cert_verify_with_keyring(cert->issuer, ring, root, trusted)) {
-		L_AUTO_CLEANUP_VAR(struct l_key *, key, tls_key_cleanup);
+		for (entry = l_queue_get_entries(roots); entry;
+				entry = entry->next) {
+			const struct tls_cert *root = entry->data;
 
-		key = l_key_new(L_KEY_RSA, cert->asn1, cert->size);
-		if (!key)
-			return false;
+			if (cert->size == root->size && !memcmp(cert->asn1,
+						root->asn1, root->size))
+				return true;
+		}
+	}
 
-		if (!l_keyring_link(ring, key))
+	if (tls_cert_verify_with_keyring(cert->issuer, ring, roots, trusted)) {
+		if (!tls_cert_link(cert, ring))
 			return false;
 
 		if (trusted || cert->issuer)
@@ -2711,46 +2770,44 @@ static void tls_keyring_cleanup(struct l_keyring **p)
 }
 
 bool tls_cert_verify_certchain(struct tls_cert *certchain,
-				struct tls_cert *ca_cert)
+				struct l_queue *ca_certs)
 {
 	L_AUTO_CLEANUP_VAR(struct l_keyring *, ca_ring, tls_keyring_cleanup);
 	L_AUTO_CLEANUP_VAR(struct l_keyring *, verify_ring,
 				tls_keyring_cleanup);
 
 	ca_ring = NULL;
-	verify_ring = NULL;
-
-	if (ca_cert) {
-		L_AUTO_CLEANUP_VAR(struct l_key *, ca_key, tls_key_cleanup);
-		ca_key = NULL;
-
-		ca_ring = l_keyring_new();
-		if (!ca_ring)
-			return false;
-
-		ca_key = l_key_new(L_KEY_RSA, ca_cert->asn1, ca_cert->size);
-		if (!ca_key || !l_keyring_link(ca_ring, ca_key))
-			return false;
-	}
 
 	verify_ring = l_keyring_new();
 	if (!verify_ring)
 		return false;
 
 	/*
-	 * If a CA cert was supplied, restrict verify_ring now so
-	 * everything else in certchain is validated against the CA.
+	 * If CA cert(s) were supplied, restrict verify_ring now so
+	 * everything else in certchain is validated against the CA(s).
 	 * Otherwise, verify_ring will be restricted after the root of
 	 * certchain is added to verify_ring by
 	 * tls_cert_verify_with_keyring().
 	 */
-	if (ca_ring && !l_keyring_restrict(verify_ring,
-						L_KEYRING_RESTRICT_ASYM_CHAIN,
-						ca_ring)) {
-		return false;
+	if (ca_certs) {
+		const struct l_queue_entry *entry;
+
+		ca_ring = l_keyring_new();
+		if (!ca_ring)
+			return false;
+
+		for (entry = l_queue_get_entries(ca_certs); entry;
+				entry = entry->next)
+			if (!tls_cert_link(entry->data, ca_ring))
+				return false;
+
+		if (!l_keyring_restrict(verify_ring,
+					L_KEYRING_RESTRICT_ASYM_CHAIN,
+					ca_ring))
+			return false;
 	}
 
-	return tls_cert_verify_with_keyring(certchain, verify_ring, ca_cert,
+	return tls_cert_verify_with_keyring(certchain, verify_ring, ca_certs,
 						ca_ring);
 }
 
diff --git a/ell/tls.h b/ell/tls.h
index 893fd63..8eec360 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -86,7 +86,7 @@ void l_tls_write(struct l_tls *tls, const uint8_t *data, size_t len);
 /* Submit TLS payload from underlying transport to be decrypted */
 void l_tls_handle_rx(struct l_tls *tls, const uint8_t *data, size_t len);
 
-/* If peer is to be authenticated, supply the CA certificate */
+/* If peer is to be authenticated, supply the CA certificate(s) */
 void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path);
 
 /*
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 6/7] tools: Update tls_cert_verify_certchain parameters
  2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
                   ` (3 preceding siblings ...)
  2018-11-01  2:12 ` [PATCH 5/7] tls: Support loading multiple root CAs Andrew Zaborowski
@ 2018-11-01  2:12 ` Andrew Zaborowski
  2018-11-01  2:12 ` [PATCH 7/7] unit: " Andrew Zaborowski
  5 siblings, 0 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

---
 tools/certchain-verify.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/certchain-verify.c b/tools/certchain-verify.c
index 1800f68..9bd46b1 100644
--- a/tools/certchain-verify.c
+++ b/tools/certchain-verify.c
@@ -87,7 +87,7 @@ static void usage(const char *bin)
 	printf("%s - TLS certificate chain verification utility\n\n", bin);
 
 	printf("Usage: %s [options] <ca_cert file> <raw certificates file>\n"
-		"  <ca_cert file> - local CA Certificate to validate against\n"
+		"  <ca_cert file> - local CA Certificate(s) to validate against\n"
 		"  <raw certificates file> - Certificates obtained from PCAP\n"
 		"  --help\n\n", bin);
 }
@@ -96,7 +96,7 @@ int main(int argc, char *argv[])
 {
 	int status = EXIT_FAILURE;
 	struct tls_cert *certchain;
-	struct tls_cert *ca_cert;
+	struct l_queue *ca_certs;
 	int err;
 
 	if (argc != 3) {
@@ -116,13 +116,13 @@ int main(int argc, char *argv[])
 		goto done;
 	}
 
-	ca_cert = tls_cert_load_file(argv[1]);
-	if (!ca_cert) {
-		fprintf(stderr, "Unable to load CA certifiate\n");
+	ca_certs = tls_cert_list_load_file(argv[1]);
+	if (!ca_certs) {
+		fprintf(stderr, "Unable to load CA certifiates\n");
 		goto free_certchain;
 	}
 
-	if (!tls_cert_verify_certchain(certchain, ca_cert)) {
+	if (!tls_cert_verify_certchain(certchain, ca_certs)) {
 		fprintf(stderr, "Verification failed\n");
 		goto free_cacert;
 	}
@@ -131,7 +131,7 @@ int main(int argc, char *argv[])
 	status = EXIT_SUCCESS;
 
 free_cacert:
-	l_free(ca_cert);
+	l_queue_destroy(ca_certs, l_free);
 free_certchain:
 	tls_cert_free_certchain(certchain);
 done:
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 7/7] unit: Update tls_cert_verify_certchain parameters
  2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
                   ` (4 preceding siblings ...)
  2018-11-01  2:12 ` [PATCH 6/7] tools: Update tls_cert_verify_certchain parameters Andrew Zaborowski
@ 2018-11-01  2:12 ` Andrew Zaborowski
  5 siblings, 0 replies; 7+ messages in thread
From: Andrew Zaborowski @ 2018-11-01  2:12 UTC (permalink / raw)
  To: ell

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

---
 unit/test-tls.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/unit/test-tls.c b/unit/test-tls.c
index d969ee7..75f36f0 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -214,16 +214,16 @@ static void test_tls12_prf(const void *data)
 static void test_certificates(const void *data)
 {
 	struct tls_cert *cert;
-	struct tls_cert *cacert;
-	struct tls_cert *wrongca;
+	struct l_queue *cacert;
+	struct l_queue *wrongca;
 
 	cert = tls_cert_load_file(CERTDIR "cert-server.pem");
 	assert(cert);
 
-	cacert = tls_cert_load_file(CERTDIR "cert-ca.pem");
+	cacert = tls_cert_list_load_file(CERTDIR "cert-ca.pem");
 	assert(cacert);
 
-	wrongca = tls_cert_load_file(CERTDIR "cert-intca.pem");
+	wrongca = tls_cert_list_load_file(CERTDIR "cert-intca.pem");
 	assert(wrongca);
 
 	assert(!tls_cert_verify_certchain(cert, wrongca));
@@ -233,8 +233,8 @@ static void test_certificates(const void *data)
 	assert(tls_cert_verify_certchain(cert, NULL));
 
 	l_free(cert);
-	l_free(cacert);
-	l_free(wrongca);
+	l_queue_destroy(cacert, l_free);
+	l_queue_destroy(wrongca, l_free);
 }
 
 struct tls_conn_test {
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-01  2:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01  2:12 [PATCH 1/7] settings: Fix a format string Andrew Zaborowski
2018-11-01  2:12 ` [PATCH 2/7] Add format string checking to a few declarations Andrew Zaborowski
2018-11-01  2:12 ` [PATCH 3/7] tls: Add debug logging API Andrew Zaborowski
2018-11-01  2:12 ` [PATCH 4/7] pem: Add l_pem_load_certificate_list utility Andrew Zaborowski
2018-11-01  2:12 ` [PATCH 5/7] tls: Support loading multiple root CAs Andrew Zaborowski
2018-11-01  2:12 ` [PATCH 6/7] tools: Update tls_cert_verify_certchain parameters Andrew Zaborowski
2018-11-01  2:12 ` [PATCH 7/7] unit: " Andrew Zaborowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.