All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/6] key: Add NULL check to l_key_get_info
@ 2016-08-08 17:25 Mat Martineau
  2016-08-08 17:25 ` [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto Mat Martineau
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:25 UTC (permalink / raw)
  To: ell

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

---
 ell/key.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ell/key.c b/ell/key.c
index 669d257..01489bb 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -378,6 +378,9 @@ bool l_key_get_info(struct l_key *key, enum l_asymmetric_cipher_type cipher,
 			enum l_checksum_type hash, size_t *bits,
 			bool *public)
 {
+	if (unlikely(!key))
+		return false;
+
 	return !kernel_query_key(key->serial, lookup_cipher(cipher),
 					lookup_checksum(hash), bits, public);
 }
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto
  2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
@ 2016-08-08 17:25 ` Mat Martineau
  2016-08-09 16:17   ` Denis Kenzior
  2016-08-08 17:25 ` [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign Mat Martineau
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:25 UTC (permalink / raw)
  To: ell

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

Keep track of the peer public key as a struct l_key (loaded in the
kernel), and use that key in encrypt and verify crypto operations.
---
 ell/tls-private.h |   3 +-
 ell/tls.c         | 127 ++++++++++++++++--------------------------------------
 2 files changed, 39 insertions(+), 91 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 1a47970..516d3db 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -164,8 +164,7 @@ struct l_tls {
 	bool cert_requested, cert_sent;
 	bool peer_authenticated;
 	struct tls_cert *peer_cert;
-	uint8_t *peer_pubkey;
-	size_t peer_pubkey_length;
+	struct l_key *peer_pubkey;
 	enum handshake_hash_type signature_hash;
 
 	/* SecurityParameters current and pending */
diff --git a/ell/tls.c b/ell/tls.c
index 18e5fae..da3832c 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -36,6 +36,7 @@
 #include "pem.h"
 #include "tls-private.h"
 #include "cipher-private.h"
+#include "key.h"
 
 void tls10_prf(const uint8_t *secret, size_t secret_len,
 		const char *label,
@@ -142,6 +143,7 @@ static void tls_reset_handshake(struct l_tls *tls)
 
 	if (tls->peer_cert) {
 		l_free(tls->peer_cert);
+		l_key_free(tls->peer_pubkey);
 
 		tls->peer_cert = NULL;
 		tls->peer_pubkey = NULL;
@@ -860,9 +862,9 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 	uint8_t buf[1024 + 32];
 	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
 	uint8_t pre_master_secret[48];
-	struct l_asymmetric_cipher *rsa_server_pubkey;
-	int key_size;
+	size_t key_size;
 	ssize_t bytes_encrypted;
+	bool is_public;
 
 	if (!tls->peer_pubkey) {
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
@@ -874,37 +876,30 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 	pre_master_secret[1] = (uint8_t) (TLS_VERSION >> 0);
 	l_getrandom(pre_master_secret + 2, 46);
 
-	/* Fill in the RSA Client Key Exchange body */
-
-	rsa_server_pubkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
-							tls->peer_pubkey,
-							tls->peer_pubkey_length,
-							true);
-	if (!rsa_server_pubkey) {
+	if (!l_key_get_info(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, &key_size,
+					&is_public)) {
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
 
 		return false;
 	}
 
-	key_size = l_asymmetric_cipher_get_key_size(rsa_server_pubkey);
+	key_size /= 8;
 
 	if (key_size + 32 > (int) sizeof(buf)) {
-		l_asymmetric_cipher_free(rsa_server_pubkey);
-
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
 
 		return false;
 	}
 
 	l_put_be16(key_size, ptr);
-	bytes_encrypted = l_asymmetric_cipher_encrypt(rsa_server_pubkey,
-							pre_master_secret,
-							ptr + 2, 48, key_size);
+	bytes_encrypted = l_key_encrypt(tls->peer_pubkey,
+					L_CIPHER_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, pre_master_secret,
+					ptr + 2, 48, key_size);
 	ptr += key_size + 2;
 
-	l_asymmetric_cipher_free(rsa_server_pubkey);
-
-	if (bytes_encrypted != key_size) {
+	if (bytes_encrypted != (ssize_t) key_size) {
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
 
 		return false;
@@ -1006,16 +1001,15 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
 static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 				tls_get_hash_t get_hash)
 {
-	struct l_asymmetric_cipher *rsa_client_pubkey;
 	size_t key_size;
-	ssize_t verify_bytes;
 	uint8_t hash[HANDSHAKE_HASH_MAX_SIZE];
 	size_t hash_len;
 	enum l_checksum_type hash_type;
 	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE * 2 + 32];
 	size_t expected_len;
-	uint8_t *digest_info;
 	unsigned int offset;
+	bool is_public;
+	bool success;
 
 	/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
 	offset = 2;
@@ -1029,23 +1023,21 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 		return false;
 	}
 
-	rsa_client_pubkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
-							tls->peer_pubkey,
-							tls->peer_pubkey_length,
-							true);
-	if (!rsa_client_pubkey) {
+	if (!l_key_get_info(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, &key_size,
+					&is_public)) {
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
 
 		return false;
 	}
 
-	key_size = l_asymmetric_cipher_get_key_size(rsa_client_pubkey);
+	key_size /= 8;
 
 	/* Only the default hash type supported */
 	if (len != offset + 2 + key_size) {
 		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
 
-		goto err_free_rsa;
+		return false;
 	}
 
 	if (tls->negotiated_version >= TLS_V12) {
@@ -1053,13 +1045,13 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 		if (in[1] != 1 /* RSA_sign */) {
 			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
 
-			goto err_free_rsa;
+			return false;
 		}
 
 		if (!get_hash(tls, in[0], hash, &hash_len, &hash_type)) {
 			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
 
-			goto err_free_rsa;
+			return false;
 		}
 
 		/*
@@ -1091,27 +1083,14 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
 		 */
 	}
 
-	digest_info = alloca(expected_len);
-
-	verify_bytes = l_asymmetric_cipher_verify(rsa_client_pubkey, in + 4,
-							digest_info, key_size,
-							expected_len);
+	success = l_key_verify(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
+				L_CHECKSUM_NONE, expected, in + 4,
+				expected_len, key_size);
 
-	l_asymmetric_cipher_free(rsa_client_pubkey);
-
-	if (verify_bytes != (ssize_t)expected_len ||
-		memcmp(digest_info, expected, expected_len)) {
+	if (!success)
 		tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
 
-		return false;
-	}
-
-	return true;
-
-err_free_rsa:
-	l_asymmetric_cipher_free(rsa_client_pubkey);
-
-	return false;
+	return success;
 }
 
 static void tls_get_handshake_hash(struct l_tls *tls,
@@ -1496,10 +1475,9 @@ decode_error:
 static void tls_handle_certificate(struct l_tls *tls,
 					const uint8_t *buf, size_t len)
 {
-	int total, cert_len, pubkey_len;
+	int total, cert_len;
 	struct tls_cert *certchain = NULL, **tail = &certchain;
 	struct tls_cert *ca_cert = NULL;
-	uint8_t *pubkey;
 
 	/* Length checks */
 
@@ -1582,26 +1560,19 @@ static void tls_handle_certificate(struct l_tls *tls,
 		goto done;
 	}
 
-	/*
-	 * Save the public key from the certificate for use in premaster
-	 * secret encryption.
-	 */
-	pubkey = tls_cert_find_pubkey(certchain, &pubkey_len);
-	if (!pubkey) {
-		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
+	/* Save the end-entity cert and free the rest of the chain */
+	tls->peer_cert = certchain;
+	tls_cert_free_certchain(certchain->issuer);
+	certchain->issuer = NULL;
+	certchain = NULL;
 
-		goto done;
-	}
+	tls->peer_pubkey = l_key_new(L_KEY_RSA, tls->peer_cert->asn1,
+					tls->peer_cert->size);
 
-	if (pubkey) {
-		/* Save the end-entity cert and free the rest of the chain */
-		tls->peer_cert = certchain;
-		tls_cert_free_certchain(certchain->issuer);
-		certchain->issuer = NULL;
-		certchain = NULL;
+	if (!tls->peer_pubkey) {
+		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
 
-		tls->peer_pubkey = pubkey;
-		tls->peer_pubkey_length = pubkey_len;
+		goto done;
 	}
 
 	if (tls->server)
@@ -2505,28 +2476,6 @@ void tls_cert_free_certchain(struct tls_cert *cert)
 	}
 }
 
-uint8_t *tls_cert_find_pubkey(struct tls_cert *cert, int *pubkey_len)
-{
-	uint8_t *key;
-	size_t len;
-
-	key = der_find_elem_by_path(cert->asn1, cert->size, ASN1_ID_BIT_STRING,
-					&len,
-					X509_CERTIFICATE_POS,
-					X509_TBSCERTIFICATE_POS,
-					X509_TBSCERT_SUBJECT_KEY_POS,
-					X509_SUBJECT_KEY_VALUE_POS, -1);
-	if (!key || len < 10)
-		return NULL;
-
-	/* Skip the BIT STRING metadata byte */
-	key += 1;
-	len -= 1;
-
-	*pubkey_len = len;
-	return key;
-}
-
 enum tls_cert_key_type tls_cert_get_pubkey_type(struct tls_cert *cert)
 {
 	uint8_t *key_type;
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign
  2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
  2016-08-08 17:25 ` [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto Mat Martineau
@ 2016-08-08 17:25 ` Mat Martineau
  2016-08-09 16:26   ` Denis Kenzior
  2016-08-08 17:25 ` [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests Mat Martineau
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:25 UTC (permalink / raw)
  To: ell

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

Store the private key in a struct l_key when auth is configured so the
key doesn't need to be reloaded for each use. The key path and password
are no longer needed in memory once the key is configured.

l_asymmetric_cipher is no longer used by TLS.
---
 ell/tls-private.h |   4 +-
 ell/tls.c         | 142 +++++++++++++++++++-----------------------------------
 ell/tls.h         |   2 +-
 3 files changed, 53 insertions(+), 95 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 516d3db..31f45b9 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -139,8 +139,8 @@ struct l_tls {
 
 	char *ca_cert_path;
 	char *cert_path;
-	char *priv_key_path;
-	char *priv_key_passphrase;
+	struct l_key *priv_key;
+	size_t priv_key_size;
 
 	/* Record layer */
 
diff --git a/ell/tls.c b/ell/tls.c
index da3832c..516d855 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -119,12 +119,6 @@ static void tls_write_random(uint8_t *buf)
 	l_getrandom(buf + 4, 28);
 }
 
-static void tls_free_key(uint8_t *key, size_t size)
-{
-	memset(key, 0, size);
-	l_free(key);
-}
-
 static void tls_drop_handshake_hash(struct l_tls *tls,
 					enum handshake_hash_type hash)
 {
@@ -916,9 +910,6 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
 				tls_get_hash_t get_hash)
 {
-	struct l_asymmetric_cipher *rsa_privkey;
-	uint8_t *privkey;
-	size_t key_size;
 	ssize_t result;
 	const struct tls_hash_algorithm *hash_type;
 	uint8_t hash[HANDSHAKE_HASH_MAX_SIZE];
@@ -927,35 +918,14 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
 	bool prepend_hash_type = false;
 	size_t expected_bytes;
 
-	if (!tls->priv_key_path) {
+	if (!tls->priv_key || !tls->priv_key_size) {
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
 				TLS_ALERT_BAD_CERT);
 
 		return -ENOKEY;
 	}
 
-	privkey = l_pem_load_private_key(tls->priv_key_path,
-						tls->priv_key_passphrase,
-						&key_size);
-	if (!privkey) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-				TLS_ALERT_BAD_CERT);
-
-		return -ENOKEY;
-	}
-
-	rsa_privkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
-						privkey, key_size, false);
-	tls_free_key(privkey, key_size);
-
-	if (!rsa_privkey) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
-
-		return -ENOKEY;
-	}
-
-	key_size = l_asymmetric_cipher_get_key_size(rsa_privkey);
-	expected_bytes = key_size + 2;
+	expected_bytes = tls->priv_key_size + 2;
 
 	if (tls->negotiated_version >= TLS_V12) {
 		hash_type = &tls_handshake_hash_data[tls->signature_hash];
@@ -981,17 +951,15 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
 			*out++ = 1;	/* RSA_sign */
 		}
 
-		l_put_be16(key_size, out);
-		result = l_asymmetric_cipher_sign(rsa_privkey, sign_input,
-							out + 2, sign_input_len,
-							key_size);
+		l_put_be16(tls->priv_key_size, out);
+		result = l_key_sign(tls->priv_key, L_CIPHER_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, sign_input, out + 2,
+					sign_input_len, tls->priv_key_size);
 
-		if (result == (ssize_t) key_size)
+		if (result == (ssize_t) tls->priv_key_size)
 			result = expected_bytes;
 	}
 
-	l_asymmetric_cipher_free(rsa_privkey);
-
 	if (result < 0)
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
 
@@ -1730,44 +1698,16 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
 						const uint8_t *buf, size_t len)
 {
 	uint8_t pre_master_secret[48], random_secret[46];
-	struct l_asymmetric_cipher *rsa_server_privkey;
-	uint8_t *privkey;
-	size_t key_size;
 	ssize_t bytes_decrypted;
 
-	if (!tls->priv_key_path) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
-				TLS_ALERT_BAD_CERT);
-
-		return;
-	}
-
-	privkey = l_pem_load_private_key(tls->priv_key_path,
-						tls->priv_key_passphrase,
-						&key_size);
-	if (!privkey) {
+	if (!tls->priv_key || !tls->priv_key_size) {
 		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
 				TLS_ALERT_BAD_CERT);
 
 		return;
 	}
 
-	rsa_server_privkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
-							privkey, key_size,
-							false);
-	tls_free_key(privkey, key_size);
-
-	if (!rsa_server_privkey) {
-		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
-
-		return;
-	}
-
-	key_size = l_asymmetric_cipher_get_key_size(rsa_server_privkey);
-
-	if (len != key_size + 2) {
-		l_asymmetric_cipher_free(rsa_server_privkey);
-
+	if (len != tls->priv_key_size + 2) {
 		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
 
 		return;
@@ -1775,19 +1715,16 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
 
 	len = l_get_be16(buf);
 
-	if (len != key_size) {
-		l_asymmetric_cipher_free(rsa_server_privkey);
-
+	if (len != tls->priv_key_size) {
 		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
 
 		return;
 	}
 
-	bytes_decrypted = l_asymmetric_cipher_decrypt(rsa_server_privkey,
-							buf + 2,
-							pre_master_secret,
-							key_size, 48);
-	l_asymmetric_cipher_free(rsa_server_privkey);
+	bytes_decrypted = l_key_decrypt(tls->priv_key, L_CIPHER_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, buf + 2,
+					pre_master_secret, tls->priv_key_size,
+					48);
 
 	/*
 	 * Assume correct premaster secret client version which according
@@ -2279,31 +2216,52 @@ LIB_EXPORT void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path)
 		tls->ca_cert_path = l_strdup(ca_cert_path);
 }
 
-LIB_EXPORT void l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
-				const char *priv_key_path,
-				const char *priv_key_passphrase)
+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)
 {
+	uint8_t *priv_key = NULL;
+	bool is_public = true;
+
 	if (tls->cert_path) {
 		l_free(tls->cert_path);
-		l_free(tls->priv_key_path);
 		tls->cert_path = NULL;
-		tls->priv_key_path = NULL;
 	}
 
-	if (cert_path) {
-		tls->cert_path = l_strdup(cert_path);
-		tls->priv_key_path = l_strdup(priv_key_path);
+	if (tls->priv_key) {
+		l_key_free(tls->priv_key);
+		tls->priv_key = NULL;
+		tls->priv_key_size = 0;
 	}
 
-	if (tls->priv_key_passphrase) {
-		memset(tls->priv_key_passphrase, 0,
-				strlen(tls->priv_key_passphrase));
-		l_free(tls->priv_key_passphrase);
-		tls->priv_key_passphrase = NULL;
+	if (priv_key_path) {
+		priv_key = l_pem_load_private_key(priv_key_path,
+							priv_key_passphrase,
+							&tls->priv_key_size);
+
+		tls->priv_key = l_key_new(L_KEY_RSA, priv_key,
+						tls->priv_key_size);
+		if (priv_key) {
+			memset(priv_key, 0, tls->priv_key_size);
+			l_free(priv_key);
+		}
+
+		if (!l_key_get_info(tls->priv_key, L_CIPHER_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, &tls->priv_key_size,
+					&is_public) || is_public) {
+			l_key_free(tls->priv_key);
+			tls->priv_key = NULL;
+			tls->priv_key_size = 0;
+			return false;
+		}
+
+		tls->priv_key_size /= 8;
 	}
 
-	if (priv_key_passphrase)
-		tls->priv_key_passphrase = l_strdup(priv_key_passphrase);
+	if (cert_path)
+		tls->cert_path = l_strdup(cert_path);
+
+	return true;
 }
 
 LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
diff --git a/ell/tls.h b/ell/tls.h
index a3f3a28..0a7c920 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -97,7 +97,7 @@ void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path);
  * one certificate of each type so they can be used depending on which
  * is compatible with the negotiated parameters.
  */
-void l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
+bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
 				const char *priv_key_path,
 				const char *priv_key_passphrase);
 
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests
  2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
  2016-08-08 17:25 ` [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto Mat Martineau
  2016-08-08 17:25 ` [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign Mat Martineau
@ 2016-08-08 17:25 ` Mat Martineau
  2016-08-08 17:42   ` Denis Kenzior
  2016-08-08 17:25 ` [PATCH v2 5/6] unit: Check return value of l_tls_set_auth_data Mat Martineau
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:25 UTC (permalink / raw)
  To: ell

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

---
 unit/cert-server-key-pkcs8.pem | 28 ++++++++++++++++++++++++++++
 unit/gencerts.sh               |  1 +
 unit/test-tls.c                | 16 ++++++++--------
 3 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 unit/cert-server-key-pkcs8.pem

diff --git a/unit/cert-server-key-pkcs8.pem b/unit/cert-server-key-pkcs8.pem
new file mode 100644
index 0000000..85dd370
--- /dev/null
+++ b/unit/cert-server-key-pkcs8.pem
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDTAJBeptooxomx
+q/SJ606rbzgSbcPj0dlzbQVhpGx2gYHGc5DHYK/4uXprSKzO6WrjK53BwF03J4/Z
+RibDkUahBaddXN7MBjvAO+CpsT7+fIZqj7Oxgv9ZTsoDk14gIf2xJs48I+FY30ay
+kwxEwtF+I/42N3i3n2OpAnOXye7qsLjwYcG8kxgMGZHuf7xissqhb5nEuqTBmvVk
+e5cI/uuo34ZX2qH5KA4VT+zUzSMl4l5bbq7zeRS7HnYeXj3eNhwqf0T4ru7ZaFZJ
+aY0OgAhT71nMttUov6AkvjaREb1aquuUHJ4em+J6RpQ8S1YkHhJkIJXg8qqyD33a
+lytfLuu1AgMBAAECggEBAMxMjHyI80xtx16XTyPHCRnmixUU4ImSYwhms8Ix8K7h
+gCUFOlZBoMCj3gtIh8GjsdGZJps1xUuk65wFV30eCZPZJI66YCKNmobrswC61HKV
+YZSF5Qfn/ZOcPO8vXgCgyMEhAKisWQGy1gILbOKa+zo2YWpfVWv9UQKQlBmQ9NqY
+VbZZIZvq3BSdyAnXd7OMqvAN27sLrxdgvdRzCf0E/dSMtkxOkI3G1Z0hNzDFXKxa
+CS75Xb9WaiS8m6gynqPio5PjVe9mps1FMUZuVyJdUgsZS/LtfRabtsc8Da44uEVv
+Z6UvL4Ax5iv8UToGM05+gTLZPHz3kk83EYANnTUJ8TUCgYEA6StzqgpFiL4sa1gU
+AhPcyaL5CmOlojmY+rqVtH+EHDnfHi2p1BkJxkOsWUbuW+3lvkG7pTVQ5TXM5R/d
+yyTF07HyQgI7VJZrelQupYGl6ItwZmEzoJoUUXTsXKySUXUSvLmBeJuOIDWtMyXu
+n0xetWRl8ZuYHCqIT5qmEOt8/CMCgYEA56l4BO65Pqr2k42RQM4ibspDcAZMoYh0
+ysJLVL7PWMXhFbPX6GJCz1+Vm6//pwFqTyzbNpob/vNYNJFfD3syCQATx6ije0MS
+jcP9AdO//fMLfMfhoBKNk589POoqFbXWSqX0UKuAJcFwtkurn/gdq+3/Dxd8pmS6
+7zyXw/kP6kcCgYEA1nzQEzULrbQyrDQDg729tgYizPnJHaeaH9qPZ9B9OHHL2rjq
+pl657RXHbwCetxXp5tAUyu52kcKhzos3vW0ARbJFRY9EAJW2HHtfxYOzmGzcYnE7
+ypqx4hSKcN2WYzQsnkqO3OFLJjn6LzjPft4DqRzH4i7dB9vgNEnwRVQ6Tr0CgYA9
+UnrWs2qN0CudO2grw2UR9rCLQt+eEsT3tx7BvBCe6yJ94DFS7k/JHQA5SgleZ92A
+P9t3RKwMfNXodGK5cl74SvDxdQ5xXnvW0v5yMV8tFd0Alth+yup3HTvUmBezz4J+
+GBfoEr3FQMNZPgacPc186W8Oy6TPVvK0yVFTKuavmQKBgEH5359VsH6c/KHq8r4Z
+0lYaAp+tQz2iramOkCytvlSj8jyPnVS4u/rVgq7oLxxz210wF28Mm3LonrdTOtI7
+uoC4tpuTq46ie6XlhYNW9G1ph+kR+WdXRlKZ2+iGe1Al1tgjvghlFyPdn+Ggj4qO
+ebDb59kVMg21BvxJuAggvwLp
+-----END PRIVATE KEY-----
diff --git a/unit/gencerts.sh b/unit/gencerts.sh
index cfa6486..be186f1 100755
--- a/unit/gencerts.sh
+++ b/unit/gencerts.sh
@@ -6,6 +6,7 @@ openssl req -x509 -new -nodes -extensions ca_ext -config ./gencerts.cnf -subj '/
 
 echo -e "\n*** Server Certificate ***"
 openssl genrsa -out cert-server-key.pem
+openssl pkcs8 -topk8 -nocrypt -in cert-server-key.pem -out cert-server-key-pkcs8.pem
 openssl req -new -extensions cert_ext -config ./gencerts.cnf -subj '/O=Foo Example Organization/CN=Foo Example Organization/emailAddress=foo(a)mail.example' -key cert-server-key.pem -out cert-server.csr
 openssl x509 -req -extensions cert_ext -extfile ./gencerts.cnf -in cert-server.csr -CA cert-ca.pem -CAkey cert-ca-key.pem -CAcreateserial -sha256 -days 10000 -out cert-server.pem
 openssl verify -CAfile cert-ca.pem cert-server.pem
diff --git a/unit/test-tls.c b/unit/test-tls.c
index 1b896aa..9ddb80c 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -242,14 +242,14 @@ struct tls_conn_test {
 
 static const struct tls_conn_test tls_conn_test_no_auth = {
 	.server_cert_path = TESTDATADIR "/cert-server.pem",
-	.server_key_path = TESTDATADIR "/cert-server-key.pem",
+	.server_key_path = TESTDATADIR "/cert-server-key-pkcs8.pem",
 	.server_expect_identity = NULL,
 	.client_expect_identity = NULL,
 };
 
 static const struct tls_conn_test tls_conn_test_server_auth = {
 	.server_cert_path = TESTDATADIR "/cert-server.pem",
-	.server_key_path = TESTDATADIR "/cert-server-key.pem",
+	.server_key_path = TESTDATADIR "/cert-server-key-pkcs8.pem",
 	.server_expect_identity = NULL,
 	.client_ca_cert_path = TESTDATADIR "/cert-ca.pem",
 	.client_expect_identity = "Foo Example Organization",
@@ -257,7 +257,7 @@ static const struct tls_conn_test tls_conn_test_server_auth = {
 
 static const struct tls_conn_test tls_conn_test_client_auth_attempt = {
 	.server_cert_path = TESTDATADIR "/cert-server.pem",
-	.server_key_path = TESTDATADIR "/cert-server-key.pem",
+	.server_key_path = TESTDATADIR "/cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = TESTDATADIR "/cert-ca.pem",
 	.server_expect_identity = NULL,
 	.client_expect_identity = NULL,
@@ -265,17 +265,17 @@ static const struct tls_conn_test tls_conn_test_client_auth_attempt = {
 
 static const struct tls_conn_test tls_conn_test_client_auth = {
 	.server_cert_path = TESTDATADIR "/cert-server.pem",
-	.server_key_path = TESTDATADIR "/cert-server-key.pem",
+	.server_key_path = TESTDATADIR "/cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = TESTDATADIR "/cert-ca.pem",
 	.server_expect_identity = "Bar Example Organization",
 	.client_cert_path = TESTDATADIR "/cert-client.pem",
-	.client_key_path = TESTDATADIR "/cert-client-key.pem",
+	.client_key_path = TESTDATADIR "/cert-client-key-pkcs8.pem",
 	.client_expect_identity = NULL,
 };
 
 static const struct tls_conn_test tls_conn_test_full_auth_attempt = {
 	.server_cert_path = TESTDATADIR "/cert-server.pem",
-	.server_key_path = TESTDATADIR "/cert-server-key.pem",
+	.server_key_path = TESTDATADIR "/cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = TESTDATADIR "/cert-ca.pem",
 	.server_expect_identity = NULL,
 	.client_ca_cert_path = TESTDATADIR "/cert-ca.pem",
@@ -284,11 +284,11 @@ static const struct tls_conn_test tls_conn_test_full_auth_attempt = {
 
 static const struct tls_conn_test tls_conn_test_full_auth = {
 	.server_cert_path = TESTDATADIR "/cert-server.pem",
-	.server_key_path = TESTDATADIR "/cert-server-key.pem",
+	.server_key_path = TESTDATADIR "/cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = TESTDATADIR "/cert-ca.pem",
 	.server_expect_identity = "Bar Example Organization",
 	.client_cert_path = TESTDATADIR "/cert-client.pem",
-	.client_key_path = TESTDATADIR "/cert-client-key.pem",
+	.client_key_path = TESTDATADIR "/cert-client-key-pkcs8.pem",
 	.client_ca_cert_path = TESTDATADIR "/cert-ca.pem",
 	.client_expect_identity = "Foo Example Organization",
 };
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] unit: Check return value of l_tls_set_auth_data
  2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
                   ` (2 preceding siblings ...)
  2016-08-08 17:25 ` [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests Mat Martineau
@ 2016-08-08 17:25 ` Mat Martineau
  2016-08-08 17:25 ` [PATCH v2 6/6] examples: " Mat Martineau
  2016-08-08 17:39 ` [PATCH v2 1/6] key: Add NULL check to l_key_get_info Denis Kenzior
  5 siblings, 0 replies; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:25 UTC (permalink / raw)
  To: ell

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

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

diff --git a/unit/test-tls.c b/unit/test-tls.c
index 9ddb80c..0d23f16 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -356,6 +356,7 @@ static void tls_test_disconnected(enum l_tls_alert_desc reason, bool remote,
 
 static void test_tls_test(const void *data)
 {
+	bool auth_ok;
 	const struct tls_conn_test *test = data;
 	struct tls_test_state s[2] = {
 		{
@@ -388,12 +389,14 @@ static void test_tls_test(const void *data)
 	assert(s[0].tls);
 	assert(s[1].tls);
 
-	l_tls_set_auth_data(s[0].tls, test->server_cert_path,
-				test->server_key_path,
-				test->server_key_passphrase);
-	l_tls_set_auth_data(s[1].tls, test->client_cert_path,
-				test->client_key_path,
-				test->client_key_passphrase);
+	auth_ok = l_tls_set_auth_data(s[0].tls, test->server_cert_path,
+					test->server_key_path,
+					test->server_key_passphrase);
+	assert(auth_ok);
+	auth_ok = l_tls_set_auth_data(s[1].tls, test->client_cert_path,
+					test->client_key_path,
+					test->client_key_passphrase);
+	assert(auth_ok);
 	l_tls_set_cacert(s[0].tls, test->server_ca_cert_path);
 	l_tls_set_cacert(s[1].tls, test->client_ca_cert_path);
 
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] examples: Check return value of l_tls_set_auth_data
  2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
                   ` (3 preceding siblings ...)
  2016-08-08 17:25 ` [PATCH v2 5/6] unit: Check return value of l_tls_set_auth_data Mat Martineau
@ 2016-08-08 17:25 ` Mat Martineau
  2016-08-08 17:39 ` [PATCH v2 1/6] key: Add NULL check to l_key_get_info Denis Kenzior
  5 siblings, 0 replies; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:25 UTC (permalink / raw)
  To: ell

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

---
 examples/https-client-test.c | 6 ++++--
 examples/https-server-test.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/examples/https-client-test.c b/examples/https-client-test.c
index 04a3b37..7a9c2a8 100644
--- a/examples/https-client-test.c
+++ b/examples/https-client-test.c
@@ -119,6 +119,7 @@ int main(int argc, char *argv[])
 	struct in_addr **addr_list;
 	struct sockaddr_in addr;
 	int fd;
+	bool auth_ok = true;
 
 	if (argc != 2 && argc != 3 && argc != 6) {
 		printf("Usage: %s <https-host-name> [<ca-cert-path> "
@@ -172,9 +173,10 @@ int main(int argc, char *argv[])
 	if (argc > 2)
 		l_tls_set_cacert(tls, argv[2]);
 	if (argc > 5)
-		l_tls_set_auth_data(tls, argv[3], argv[4], argv[5]);
+		auth_ok = l_tls_set_auth_data(tls, argv[3], argv[4], argv[5]);
 
-	l_main_run();
+	if (tls && auth_ok)
+		l_main_run();
 
 	l_io_destroy(io);
 	l_tls_free(tls);
diff --git a/examples/https-server-test.c b/examples/https-server-test.c
index 0f16ed2..f5f5d7a 100644
--- a/examples/https-server-test.c
+++ b/examples/https-server-test.c
@@ -111,6 +111,7 @@ int main(int argc, char *argv[])
 {
 	struct sockaddr_in addr;
 	int fd, listenfd;
+	bool auth_ok;
 
 	if (argc != 4 && argc != 5) {
 		printf("Usage: %s <server-cert-path> <server-key-path> "
@@ -159,10 +160,11 @@ int main(int argc, char *argv[])
 
 	tls = l_tls_new(true, https_new_data, https_tls_write,
 			https_tls_ready, https_tls_disconnected, NULL);
-	l_tls_set_auth_data(tls, argv[1], argv[2], argv[3]);
+	auth_ok = l_tls_set_auth_data(tls, argv[1], argv[2], argv[3]);
 	l_tls_set_cacert(tls, argc > 4 ? argv[4] : NULL);
 
-	l_main_run();
+	if (tls && !auth_ok)
+		l_main_run();
 
 	l_io_destroy(io);
 	l_tls_free(tls);
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/6] key: Add NULL check to l_key_get_info
  2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
                   ` (4 preceding siblings ...)
  2016-08-08 17:25 ` [PATCH v2 6/6] examples: " Mat Martineau
@ 2016-08-08 17:39 ` Denis Kenzior
  5 siblings, 0 replies; 16+ messages in thread
From: Denis Kenzior @ 2016-08-08 17:39 UTC (permalink / raw)
  To: ell

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

Hi Mat,

On 08/08/2016 12:25 PM, Mat Martineau wrote:
> ---
>   ell/key.c | 3 +++
>   1 file changed, 3 insertions(+)
>

Applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests
  2016-08-08 17:25 ` [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests Mat Martineau
@ 2016-08-08 17:42   ` Denis Kenzior
  2016-08-08 17:53     ` Mat Martineau
  0 siblings, 1 reply; 16+ messages in thread
From: Denis Kenzior @ 2016-08-08 17:42 UTC (permalink / raw)
  To: ell

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

Hi Mat,

On 08/08/2016 12:25 PM, Mat Martineau wrote:
> ---
>   unit/cert-server-key-pkcs8.pem | 28 ++++++++++++++++++++++++++++
>   unit/gencerts.sh               |  1 +
>   unit/test-tls.c                | 16 ++++++++--------
>   3 files changed, 37 insertions(+), 8 deletions(-)
>   create mode 100644 unit/cert-server-key-pkcs8.pem
>

I wonder, what's the reasoning behind this change?

Regards,
-Denis


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests
  2016-08-08 17:42   ` Denis Kenzior
@ 2016-08-08 17:53     ` Mat Martineau
  2016-08-08 19:58       ` Denis Kenzior
  0 siblings, 1 reply; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 17:53 UTC (permalink / raw)
  To: ell

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

On Mon, 8 Aug 2016, Denis Kenzior wrote:

> Hi Mat,
>
> On 08/08/2016 12:25 PM, Mat Martineau wrote:
>> ---
>>   unit/cert-server-key-pkcs8.pem | 28 ++++++++++++++++++++++++++++
>>   unit/gencerts.sh               |  1 +
>>   unit/test-tls.c                | 16 ++++++++--------
>>   3 files changed, 37 insertions(+), 8 deletions(-)
>>   create mode 100644 unit/cert-server-key-pkcs8.pem
>> 
>
> I wonder, what's the reasoning behind this change?

It's the private key format that the keyctl API knows how to parse. The 
previous private key format doesn't work.

--
Mat Martineau
Intel OTC

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests
  2016-08-08 17:53     ` Mat Martineau
@ 2016-08-08 19:58       ` Denis Kenzior
  2016-08-08 22:27         ` Mat Martineau
  0 siblings, 1 reply; 16+ messages in thread
From: Denis Kenzior @ 2016-08-08 19:58 UTC (permalink / raw)
  To: ell

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

Hi Mat,

On 08/08/2016 12:53 PM, Mat Martineau wrote:
> On Mon, 8 Aug 2016, Denis Kenzior wrote:
>
>> Hi Mat,
>>
>> On 08/08/2016 12:25 PM, Mat Martineau wrote:
>>> ---
>>>   unit/cert-server-key-pkcs8.pem | 28 ++++++++++++++++++++++++++++
>>>   unit/gencerts.sh               |  1 +
>>>   unit/test-tls.c                | 16 ++++++++--------
>>>   3 files changed, 37 insertions(+), 8 deletions(-)
>>>   create mode 100644 unit/cert-server-key-pkcs8.pem
>>>
>>
>> I wonder, what's the reasoning behind this change?
>
> It's the private key format that the keyctl API knows how to parse. The
> previous private key format doesn't work.
>

The previous format was PKCS 1.5 right?  There's no way to support that one?

Unit tests inside iwd assume PKCS1.5 at the moment.  And I'm worried 
that most existing certificates won't be PKCS#8.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests
  2016-08-08 19:58       ` Denis Kenzior
@ 2016-08-08 22:27         ` Mat Martineau
  2016-08-08 23:30           ` Denis Kenzior
  0 siblings, 1 reply; 16+ messages in thread
From: Mat Martineau @ 2016-08-08 22:27 UTC (permalink / raw)
  To: ell

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

On Mon, 8 Aug 2016, Denis Kenzior wrote:

> Hi Mat,
>
> On 08/08/2016 12:53 PM, Mat Martineau wrote:
>> On Mon, 8 Aug 2016, Denis Kenzior wrote:
>> 
>>> Hi Mat,
>>> 
>>> On 08/08/2016 12:25 PM, Mat Martineau wrote:
>>>> ---
>>>>   unit/cert-server-key-pkcs8.pem | 28 ++++++++++++++++++++++++++++
>>>>   unit/gencerts.sh               |  1 +
>>>>   unit/test-tls.c                | 16 ++++++++--------
>>>>   3 files changed, 37 insertions(+), 8 deletions(-)
>>>>   create mode 100644 unit/cert-server-key-pkcs8.pem
>>>> 
>>> 
>>> I wonder, what's the reasoning behind this change?
>> 
>> It's the private key format that the keyctl API knows how to parse. The
>> previous private key format doesn't work.
>> 
>
> The previous format was PKCS 1.5 right?  There's no way to support that one?
>
> Unit tests inside iwd assume PKCS1.5 at the moment.  And I'm worried that 
> most existing certificates won't be PKCS#8.

RSA private keys are a pending addition to the kernel key subsystem in the 
keys-next branch. Only a PKCS#8 private key parser was added to the 
asymmetric key type. Additional parsers can be added, it will try each 
registered parser until one succeeds.

The PKCS#1 key data is stored as an octet string inside the PKCS#8 format.

My impression is that the kernel uses the PKCS#8 format because it only 
accepts DER-encoded keys. PKCS#8 retains both a crypto algorithm 
identifier and a key encryption identifier when private key data is 
converted from PEM to DER (binary), so it the kernel can deduce the 
correct crypto algorithm and decrypt the key. With the OpenSSL-style 
PKCS#1 keys we used before, all the kernel can do is assume that an ASN.1 
sequence of 9 integers is, in fact, an RSA private key.

Since it's trivial to convert private key files to PKCS#8 with openssl, I 
think it makes sense to stick to PKCS#8 in the ELL key API.

Do you want me to change the filenames so that cert-client-key.pem and 
cert-server-key.pem are PKCS#8 format? That way there aren't two copies of 
the private key around. I think that would be transparent to the iwd unit 
tests.

--
Mat Martineau
Intel OTC

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests
  2016-08-08 22:27         ` Mat Martineau
@ 2016-08-08 23:30           ` Denis Kenzior
  0 siblings, 0 replies; 16+ messages in thread
From: Denis Kenzior @ 2016-08-08 23:30 UTC (permalink / raw)
  To: ell

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

Hi Mat,

 >
> RSA private keys are a pending addition to the kernel key subsystem in
> the keys-next branch. Only a PKCS#8 private key parser was added to the
> asymmetric key type. Additional parsers can be added, it will try each
> registered parser until one succeeds.
>
> The PKCS#1 key data is stored as an octet string inside the PKCS#8 format.
>
> My impression is that the kernel uses the PKCS#8 format because it only
> accepts DER-encoded keys. PKCS#8 retains both a crypto algorithm
> identifier and a key encryption identifier when private key data is
> converted from PEM to DER (binary), so it the kernel can deduce the
> correct crypto algorithm and decrypt the key. With the OpenSSL-style
> PKCS#1 keys we used before, all the kernel can do is assume that an
> ASN.1 sequence of 9 integers is, in fact, an RSA private key.
>
> Since it's trivial to convert private key files to PKCS#8 with openssl,
> I think it makes sense to stick to PKCS#8 in the ELL key API.

The problem is that the WPA-Enterprise certificate is usually generated 
by the sysadmin or network admin.  So we can't really control what we 
get.  My impression is that PKCS1 format is much more common than #8, 
and converting on the fly isn't really an option.

>
> Do you want me to change the filenames so that cert-client-key.pem and
> cert-server-key.pem are PKCS#8 format? That way there aren't two copies
> of the private key around. I think that would be transparent to the iwd
> unit tests.

No, don't do that.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto
  2016-08-08 17:25 ` [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto Mat Martineau
@ 2016-08-09 16:17   ` Denis Kenzior
  2016-08-09 18:36     ` Mat Martineau
  0 siblings, 1 reply; 16+ messages in thread
From: Denis Kenzior @ 2016-08-09 16:17 UTC (permalink / raw)
  To: ell

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

Hi Mat,

On 08/08/2016 12:25 PM, Mat Martineau wrote:
> Keep track of the peer public key as a struct l_key (loaded in the
> kernel), and use that key in encrypt and verify crypto operations.
> ---
>   ell/tls-private.h |   3 +-
>   ell/tls.c         | 127 ++++++++++++++++--------------------------------------
>   2 files changed, 39 insertions(+), 91 deletions(-)
>
> diff --git a/ell/tls-private.h b/ell/tls-private.h
> index 1a47970..516d3db 100644
> --- a/ell/tls-private.h
> +++ b/ell/tls-private.h
> @@ -164,8 +164,7 @@ struct l_tls {
>   	bool cert_requested, cert_sent;
>   	bool peer_authenticated;
>   	struct tls_cert *peer_cert;
> -	uint8_t *peer_pubkey;
> -	size_t peer_pubkey_length;
> +	struct l_key *peer_pubkey;
>   	enum handshake_hash_type signature_hash;
>
>   	/* SecurityParameters current and pending */
> diff --git a/ell/tls.c b/ell/tls.c
> index 18e5fae..da3832c 100644
> --- a/ell/tls.c
> +++ b/ell/tls.c
> @@ -36,6 +36,7 @@
>   #include "pem.h"
>   #include "tls-private.h"
>   #include "cipher-private.h"
> +#include "key.h"
>
>   void tls10_prf(const uint8_t *secret, size_t secret_len,
>   		const char *label,
> @@ -142,6 +143,7 @@ static void tls_reset_handshake(struct l_tls *tls)
>
>   	if (tls->peer_cert) {
>   		l_free(tls->peer_cert);
> +		l_key_free(tls->peer_pubkey);
>
>   		tls->peer_cert = NULL;
>   		tls->peer_pubkey = NULL;
> @@ -860,9 +862,9 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
>   	uint8_t buf[1024 + 32];
>   	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
>   	uint8_t pre_master_secret[48];
> -	struct l_asymmetric_cipher *rsa_server_pubkey;
> -	int key_size;
> +	size_t key_size;
>   	ssize_t bytes_encrypted;
> +	bool is_public;
>
>   	if (!tls->peer_pubkey) {
>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
> @@ -874,37 +876,30 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
>   	pre_master_secret[1] = (uint8_t) (TLS_VERSION >> 0);
>   	l_getrandom(pre_master_secret + 2, 46);
>
> -	/* Fill in the RSA Client Key Exchange body */
> -
> -	rsa_server_pubkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
> -							tls->peer_pubkey,
> -							tls->peer_pubkey_length,
> -							true);
> -	if (!rsa_server_pubkey) {
> +	if (!l_key_get_info(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
> +					L_CHECKSUM_NONE, &key_size,
> +					&is_public)) {

We don't seem to ever check the is_public variable anywhere.  Should the 
is_public be validated somewhere in handle_certificate?

Also, maybe we should store public_key_size and not incur the syscall 
overhead of l_key_get_info more than once?

>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>
>   		return false;
>   	}
>
> -	key_size = l_asymmetric_cipher_get_key_size(rsa_server_pubkey);
> +	key_size /= 8;
>
>   	if (key_size + 32 > (int) sizeof(buf)) {
> -		l_asymmetric_cipher_free(rsa_server_pubkey);
> -
>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>
>   		return false;
>   	}
>
>   	l_put_be16(key_size, ptr);
> -	bytes_encrypted = l_asymmetric_cipher_encrypt(rsa_server_pubkey,
> -							pre_master_secret,
> -							ptr + 2, 48, key_size);
> +	bytes_encrypted = l_key_encrypt(tls->peer_pubkey,
> +					L_CIPHER_RSA_PKCS1_V1_5,
> +					L_CHECKSUM_NONE, pre_master_secret,
> +					ptr + 2, 48, key_size);
>   	ptr += key_size + 2;
>
> -	l_asymmetric_cipher_free(rsa_server_pubkey);
> -
> -	if (bytes_encrypted != key_size) {
> +	if (bytes_encrypted != (ssize_t) key_size) {
>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>
>   		return false;
> @@ -1006,16 +1001,15 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
>   static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
>   				tls_get_hash_t get_hash)
>   {
> -	struct l_asymmetric_cipher *rsa_client_pubkey;
>   	size_t key_size;
> -	ssize_t verify_bytes;
>   	uint8_t hash[HANDSHAKE_HASH_MAX_SIZE];
>   	size_t hash_len;
>   	enum l_checksum_type hash_type;
>   	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE * 2 + 32];
>   	size_t expected_len;
> -	uint8_t *digest_info;
>   	unsigned int offset;
> +	bool is_public;
> +	bool success;
>
>   	/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
>   	offset = 2;
> @@ -1029,23 +1023,21 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
>   		return false;
>   	}
>
> -	rsa_client_pubkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
> -							tls->peer_pubkey,
> -							tls->peer_pubkey_length,
> -							true);
> -	if (!rsa_client_pubkey) {
> +	if (!l_key_get_info(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
> +					L_CHECKSUM_NONE, &key_size,
> +					&is_public)) {
>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>
>   		return false;
>   	}
>
> -	key_size = l_asymmetric_cipher_get_key_size(rsa_client_pubkey);
> +	key_size /= 8;
>
>   	/* Only the default hash type supported */
>   	if (len != offset + 2 + key_size) {
>   		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
>
> -		goto err_free_rsa;
> +		return false;
>   	}
>
>   	if (tls->negotiated_version >= TLS_V12) {
> @@ -1053,13 +1045,13 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
>   		if (in[1] != 1 /* RSA_sign */) {
>   			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
>
> -			goto err_free_rsa;
> +			return false;
>   		}
>
>   		if (!get_hash(tls, in[0], hash, &hash_len, &hash_type)) {
>   			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
>
> -			goto err_free_rsa;
> +			return false;
>   		}
>
>   		/*
> @@ -1091,27 +1083,14 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
>   		 */
>   	}
>
> -	digest_info = alloca(expected_len);
> -
> -	verify_bytes = l_asymmetric_cipher_verify(rsa_client_pubkey, in + 4,
> -							digest_info, key_size,
> -							expected_len);
> +	success = l_key_verify(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
> +				L_CHECKSUM_NONE, expected, in + 4,
> +				expected_len, key_size);
>
> -	l_asymmetric_cipher_free(rsa_client_pubkey);
> -
> -	if (verify_bytes != (ssize_t)expected_len ||
> -		memcmp(digest_info, expected, expected_len)) {
> +	if (!success)
>   		tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
>
> -		return false;
> -	}
> -
> -	return true;
> -
> -err_free_rsa:
> -	l_asymmetric_cipher_free(rsa_client_pubkey);
> -
> -	return false;
> +	return success;
>   }
>
>   static void tls_get_handshake_hash(struct l_tls *tls,
> @@ -1496,10 +1475,9 @@ decode_error:
>   static void tls_handle_certificate(struct l_tls *tls,
>   					const uint8_t *buf, size_t len)
>   {
> -	int total, cert_len, pubkey_len;
> +	int total, cert_len;
>   	struct tls_cert *certchain = NULL, **tail = &certchain;
>   	struct tls_cert *ca_cert = NULL;
> -	uint8_t *pubkey;
>
>   	/* Length checks */
>
> @@ -1582,26 +1560,19 @@ static void tls_handle_certificate(struct l_tls *tls,
>   		goto done;
>   	}
>
> -	/*
> -	 * Save the public key from the certificate for use in premaster
> -	 * secret encryption.
> -	 */
> -	pubkey = tls_cert_find_pubkey(certchain, &pubkey_len);
> -	if (!pubkey) {
> -		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
> +	/* Save the end-entity cert and free the rest of the chain */
> +	tls->peer_cert = certchain;
> +	tls_cert_free_certchain(certchain->issuer);
> +	certchain->issuer = NULL;
> +	certchain = NULL;
>
> -		goto done;
> -	}
> +	tls->peer_pubkey = l_key_new(L_KEY_RSA, tls->peer_cert->asn1,
> +					tls->peer_cert->size);
>
> -	if (pubkey) {
> -		/* Save the end-entity cert and free the rest of the chain */
> -		tls->peer_cert = certchain;
> -		tls_cert_free_certchain(certchain->issuer);
> -		certchain->issuer = NULL;
> -		certchain = NULL;
> +	if (!tls->peer_pubkey) {
> +		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
>
> -		tls->peer_pubkey = pubkey;
> -		tls->peer_pubkey_length = pubkey_len;
> +		goto done;
>   	}
>
>   	if (tls->server)
> @@ -2505,28 +2476,6 @@ void tls_cert_free_certchain(struct tls_cert *cert)
>   	}
>   }
>
> -uint8_t *tls_cert_find_pubkey(struct tls_cert *cert, int *pubkey_len)

Do me a favor and remove the declaration of this function from tls-private.h

> -{
> -	uint8_t *key;
> -	size_t len;
> -
> -	key = der_find_elem_by_path(cert->asn1, cert->size, ASN1_ID_BIT_STRING,
> -					&len,
> -					X509_CERTIFICATE_POS,
> -					X509_TBSCERTIFICATE_POS,
> -					X509_TBSCERT_SUBJECT_KEY_POS,
> -					X509_SUBJECT_KEY_VALUE_POS, -1);
> -	if (!key || len < 10)
> -		return NULL;
> -
> -	/* Skip the BIT STRING metadata byte */
> -	key += 1;
> -	len -= 1;
> -
> -	*pubkey_len = len;
> -	return key;
> -}
> -
>   enum tls_cert_key_type tls_cert_get_pubkey_type(struct tls_cert *cert)
>   {
>   	uint8_t *key_type;
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign
  2016-08-08 17:25 ` [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign Mat Martineau
@ 2016-08-09 16:26   ` Denis Kenzior
  2016-08-09 18:40     ` Mat Martineau
  0 siblings, 1 reply; 16+ messages in thread
From: Denis Kenzior @ 2016-08-09 16:26 UTC (permalink / raw)
  To: ell

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

Hi Mat,

 >
> -LIB_EXPORT void l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
> -				const char *priv_key_path,
> -				const char *priv_key_passphrase)
> +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)
>   {
> +	uint8_t *priv_key = NULL;
> +	bool is_public = true;
> +

Just a quick nitpick.  Can you move these into the if (priv_key_path) block?

>   	if (tls->cert_path) {
>   		l_free(tls->cert_path);
> -		l_free(tls->priv_key_path);
>   		tls->cert_path = NULL;
> -		tls->priv_key_path = NULL;
>   	}
>
> -	if (cert_path) {
> -		tls->cert_path = l_strdup(cert_path);
> -		tls->priv_key_path = l_strdup(priv_key_path);
> +	if (tls->priv_key) {
> +		l_key_free(tls->priv_key);
> +		tls->priv_key = NULL;
> +		tls->priv_key_size = 0;
>   	}
>
> -	if (tls->priv_key_passphrase) {
> -		memset(tls->priv_key_passphrase, 0,
> -				strlen(tls->priv_key_passphrase));
> -		l_free(tls->priv_key_passphrase);
> -		tls->priv_key_passphrase = NULL;
> +	if (priv_key_path) {
> +		priv_key = l_pem_load_private_key(priv_key_path,
> +							priv_key_passphrase,
> +							&tls->priv_key_size);
> +
> +		tls->priv_key = l_key_new(L_KEY_RSA, priv_key,
> +						tls->priv_key_size);
> +		if (priv_key) {
> +			memset(priv_key, 0, tls->priv_key_size);
> +			l_free(priv_key);
> +		}
> +
> +		if (!l_key_get_info(tls->priv_key, L_CIPHER_RSA_PKCS1_V1_5,
> +					L_CHECKSUM_NONE, &tls->priv_key_size,
> +					&is_public) || is_public) {
> +			l_key_free(tls->priv_key);
> +			tls->priv_key = NULL;
> +			tls->priv_key_size = 0;
> +			return false;
> +		}
> +
> +		tls->priv_key_size /= 8;
>   	}
>
> -	if (priv_key_passphrase)
> -		tls->priv_key_passphrase = l_strdup(priv_key_passphrase);
> +	if (cert_path)
> +		tls->cert_path = l_strdup(cert_path);
> +
> +	return true;
>   }
>
>   LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
> diff --git a/ell/tls.h b/ell/tls.h
> index a3f3a28..0a7c920 100644
> --- a/ell/tls.h
> +++ b/ell/tls.h
> @@ -97,7 +97,7 @@ void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path);
>    * one certificate of each type so they can be used depending on which
>    * is compatible with the negotiated parameters.
>    */
> -void l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
> +bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
>   				const char *priv_key_path,
>   				const char *priv_key_passphrase);
>
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto
  2016-08-09 16:17   ` Denis Kenzior
@ 2016-08-09 18:36     ` Mat Martineau
  0 siblings, 0 replies; 16+ messages in thread
From: Mat Martineau @ 2016-08-09 18:36 UTC (permalink / raw)
  To: ell

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

On Tue, 9 Aug 2016, Denis Kenzior wrote:

> Hi Mat,
>
> On 08/08/2016 12:25 PM, Mat Martineau wrote:
>> Keep track of the peer public key as a struct l_key (loaded in the
>> kernel), and use that key in encrypt and verify crypto operations.
>> ---
>>   ell/tls-private.h |   3 +-
>>   ell/tls.c         | 127 
>> ++++++++++++++++--------------------------------------
>>   2 files changed, 39 insertions(+), 91 deletions(-)
>> 
>> diff --git a/ell/tls-private.h b/ell/tls-private.h
>> index 1a47970..516d3db 100644
>> --- a/ell/tls-private.h
>> +++ b/ell/tls-private.h
>> @@ -164,8 +164,7 @@ struct l_tls {
>>   	bool cert_requested, cert_sent;
>>   	bool peer_authenticated;
>>   	struct tls_cert *peer_cert;
>> -	uint8_t *peer_pubkey;
>> -	size_t peer_pubkey_length;
>> +	struct l_key *peer_pubkey;
>>   	enum handshake_hash_type signature_hash;
>>
>>   	/* SecurityParameters current and pending */
>> diff --git a/ell/tls.c b/ell/tls.c
>> index 18e5fae..da3832c 100644
>> --- a/ell/tls.c
>> +++ b/ell/tls.c
>> @@ -36,6 +36,7 @@
>>   #include "pem.h"
>>   #include "tls-private.h"
>>   #include "cipher-private.h"
>> +#include "key.h"
>>
>>   void tls10_prf(const uint8_t *secret, size_t secret_len,
>>   		const char *label,
>> @@ -142,6 +143,7 @@ static void tls_reset_handshake(struct l_tls *tls)
>>
>>   	if (tls->peer_cert) {
>>   		l_free(tls->peer_cert);
>> +		l_key_free(tls->peer_pubkey);
>>
>>   		tls->peer_cert = NULL;
>>   		tls->peer_pubkey = NULL;
>> @@ -860,9 +862,9 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls 
>> *tls)
>>   	uint8_t buf[1024 + 32];
>>   	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
>>   	uint8_t pre_master_secret[48];
>> -	struct l_asymmetric_cipher *rsa_server_pubkey;
>> -	int key_size;
>> +	size_t key_size;
>>   	ssize_t bytes_encrypted;
>> +	bool is_public;
>>
>>   	if (!tls->peer_pubkey) {
>>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>> @@ -874,37 +876,30 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls 
>> *tls)
>>   	pre_master_secret[1] = (uint8_t) (TLS_VERSION >> 0);
>>   	l_getrandom(pre_master_secret + 2, 46);
>> 
>> -	/* Fill in the RSA Client Key Exchange body */
>> -
>> -	rsa_server_pubkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
>> -							tls->peer_pubkey,
>> - 
>> tls->peer_pubkey_length,
>> -							true);
>> -	if (!rsa_server_pubkey) {
>> +	if (!l_key_get_info(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
>> +					L_CHECKSUM_NONE, &key_size,
>> +					&is_public)) {
>
> We don't seem to ever check the is_public variable anywhere.  Should the 
> is_public be validated somewhere in handle_certificate?

No, just needed a valid bool* to pass in to l_key_get_info.

>
> Also, maybe we should store public_key_size and not incur the syscall 
> overhead of l_key_get_info more than once?

Sure, that's what the private keys do.

>
>>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>>
>>   		return false;
>>   	}
>> 
>> -	key_size = l_asymmetric_cipher_get_key_size(rsa_server_pubkey);
>> +	key_size /= 8;
>>
>>   	if (key_size + 32 > (int) sizeof(buf)) {
>> -		l_asymmetric_cipher_free(rsa_server_pubkey);
>> -
>>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>>
>>   		return false;
>>   	}
>>
>>   	l_put_be16(key_size, ptr);
>> -	bytes_encrypted = l_asymmetric_cipher_encrypt(rsa_server_pubkey,
>> -							pre_master_secret,
>> -							ptr + 2, 48, 
>> key_size);
>> +	bytes_encrypted = l_key_encrypt(tls->peer_pubkey,
>> +					L_CIPHER_RSA_PKCS1_V1_5,
>> +					L_CHECKSUM_NONE, pre_master_secret,
>> +					ptr + 2, 48, key_size);
>>   	ptr += key_size + 2;
>> 
>> -	l_asymmetric_cipher_free(rsa_server_pubkey);
>> -
>> -	if (bytes_encrypted != key_size) {
>> +	if (bytes_encrypted != (ssize_t) key_size) {
>>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>>
>>   		return false;
>> @@ -1006,16 +1001,15 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, 
>> uint8_t *out, size_t len,
>>   static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t 
>> len,
>>   				tls_get_hash_t get_hash)
>>   {
>> -	struct l_asymmetric_cipher *rsa_client_pubkey;
>>   	size_t key_size;
>> -	ssize_t verify_bytes;
>>   	uint8_t hash[HANDSHAKE_HASH_MAX_SIZE];
>>   	size_t hash_len;
>>   	enum l_checksum_type hash_type;
>>   	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE * 2 + 32];
>>   	size_t expected_len;
>> -	uint8_t *digest_info;
>>   	unsigned int offset;
>> +	bool is_public;
>> +	bool success;
>>
>>   	/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
>>   	offset = 2;
>> @@ -1029,23 +1023,21 @@ static bool tls_rsa_verify(struct l_tls *tls, const 
>> uint8_t *in, size_t len,
>>   		return false;
>>   	}
>> 
>> -	rsa_client_pubkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
>> -							tls->peer_pubkey,
>> - 
>> tls->peer_pubkey_length,
>> -							true);
>> -	if (!rsa_client_pubkey) {
>> +	if (!l_key_get_info(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
>> +					L_CHECKSUM_NONE, &key_size,
>> +					&is_public)) {
>>   		tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
>>
>>   		return false;
>>   	}
>> 
>> -	key_size = l_asymmetric_cipher_get_key_size(rsa_client_pubkey);
>> +	key_size /= 8;
>>
>>   	/* Only the default hash type supported */
>>   	if (len != offset + 2 + key_size) {
>>   		tls_disconnect(tls, TLS_ALERT_DECODE_ERROR, 0);
>> 
>> -		goto err_free_rsa;
>> +		return false;
>>   	}
>>
>>   	if (tls->negotiated_version >= TLS_V12) {
>> @@ -1053,13 +1045,13 @@ static bool tls_rsa_verify(struct l_tls *tls, const 
>> uint8_t *in, size_t len,
>>   		if (in[1] != 1 /* RSA_sign */) {
>>   			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
>> 
>> -			goto err_free_rsa;
>> +			return false;
>>   		}
>>
>>   		if (!get_hash(tls, in[0], hash, &hash_len, &hash_type)) {
>>   			tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
>> 
>> -			goto err_free_rsa;
>> +			return false;
>>   		}
>>
>>   		/*
>> @@ -1091,27 +1083,14 @@ static bool tls_rsa_verify(struct l_tls *tls, const 
>> uint8_t *in, size_t len,
>>   		 */
>>   	}
>> 
>> -	digest_info = alloca(expected_len);
>> -
>> -	verify_bytes = l_asymmetric_cipher_verify(rsa_client_pubkey, in + 4,
>> -							digest_info, 
>> key_size,
>> -							expected_len);
>> +	success = l_key_verify(tls->peer_pubkey, L_CIPHER_RSA_PKCS1_V1_5,
>> +				L_CHECKSUM_NONE, expected, in + 4,
>> +				expected_len, key_size);
>> 
>> -	l_asymmetric_cipher_free(rsa_client_pubkey);
>> -
>> -	if (verify_bytes != (ssize_t)expected_len ||
>> -		memcmp(digest_info, expected, expected_len)) {
>> +	if (!success)
>>   		tls_disconnect(tls, TLS_ALERT_DECRYPT_ERROR, 0);
>> 
>> -		return false;
>> -	}
>> -
>> -	return true;
>> -
>> -err_free_rsa:
>> -	l_asymmetric_cipher_free(rsa_client_pubkey);
>> -
>> -	return false;
>> +	return success;
>>   }
>>
>>   static void tls_get_handshake_hash(struct l_tls *tls,
>> @@ -1496,10 +1475,9 @@ decode_error:
>>   static void tls_handle_certificate(struct l_tls *tls,
>>   					const uint8_t *buf, size_t len)
>>   {
>> -	int total, cert_len, pubkey_len;
>> +	int total, cert_len;
>>   	struct tls_cert *certchain = NULL, **tail = &certchain;
>>   	struct tls_cert *ca_cert = NULL;
>> -	uint8_t *pubkey;
>>
>>   	/* Length checks */
>> 
>> @@ -1582,26 +1560,19 @@ static void tls_handle_certificate(struct l_tls 
>> *tls,
>>   		goto done;
>>   	}
>> 
>> -	/*
>> -	 * Save the public key from the certificate for use in premaster
>> -	 * secret encryption.
>> -	 */
>> -	pubkey = tls_cert_find_pubkey(certchain, &pubkey_len);
>> -	if (!pubkey) {
>> -		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
>> +	/* Save the end-entity cert and free the rest of the chain */
>> +	tls->peer_cert = certchain;
>> +	tls_cert_free_certchain(certchain->issuer);
>> +	certchain->issuer = NULL;
>> +	certchain = NULL;
>> 
>> -		goto done;
>> -	}
>> +	tls->peer_pubkey = l_key_new(L_KEY_RSA, tls->peer_cert->asn1,
>> +					tls->peer_cert->size);
>> 
>> -	if (pubkey) {
>> -		/* Save the end-entity cert and free the rest of the chain */
>> -		tls->peer_cert = certchain;
>> -		tls_cert_free_certchain(certchain->issuer);
>> -		certchain->issuer = NULL;
>> -		certchain = NULL;
>> +	if (!tls->peer_pubkey) {
>> +		tls_disconnect(tls, TLS_ALERT_UNSUPPORTED_CERT, 0);
>> 
>> -		tls->peer_pubkey = pubkey;
>> -		tls->peer_pubkey_length = pubkey_len;
>> +		goto done;
>>   	}
>>
>>   	if (tls->server)
>> @@ -2505,28 +2476,6 @@ void tls_cert_free_certchain(struct tls_cert *cert)
>>   	}
>>   }
>> 
>> -uint8_t *tls_cert_find_pubkey(struct tls_cert *cert, int *pubkey_len)
>
> Do me a favor and remove the declaration of this function from tls-private.h

Ok.

>
>> -{
>> -	uint8_t *key;
>> -	size_t len;
>> -
>> -	key = der_find_elem_by_path(cert->asn1, cert->size, 
>> ASN1_ID_BIT_STRING,
>> -					&len,
>> -					X509_CERTIFICATE_POS,
>> -					X509_TBSCERTIFICATE_POS,
>> -					X509_TBSCERT_SUBJECT_KEY_POS,
>> -					X509_SUBJECT_KEY_VALUE_POS, -1);
>> -	if (!key || len < 10)
>> -		return NULL;
>> -
>> -	/* Skip the BIT STRING metadata byte */
>> -	key += 1;
>> -	len -= 1;
>> -
>> -	*pubkey_len = len;
>> -	return key;
>> -}
>> -
>>   enum tls_cert_key_type tls_cert_get_pubkey_type(struct tls_cert *cert)
>>   {
>>   	uint8_t *key_type;
>>


--
Mat Martineau
Intel OTC

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign
  2016-08-09 16:26   ` Denis Kenzior
@ 2016-08-09 18:40     ` Mat Martineau
  0 siblings, 0 replies; 16+ messages in thread
From: Mat Martineau @ 2016-08-09 18:40 UTC (permalink / raw)
  To: ell

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


On Tue, 9 Aug 2016, Denis Kenzior wrote:

> Hi Mat,
>
>>
>> -LIB_EXPORT void l_tls_set_auth_data(struct l_tls *tls, const char 
>> *cert_path,
>> -				const char *priv_key_path,
>> -				const char *priv_key_passphrase)
>> +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)
>>   {
>> +	uint8_t *priv_key = NULL;
>> +	bool is_public = true;
>> +
>
> Just a quick nitpick.  Can you move these into the if (priv_key_path) block?

Sure. Good to know that fits with the style guidelines, I'd prefer to have 
them in the smaller scope.

>>   	if (tls->cert_path) {
>>   		l_free(tls->cert_path);
>> -		l_free(tls->priv_key_path);
>>   		tls->cert_path = NULL;
>> -		tls->priv_key_path = NULL;
>>   	}
>> 
>> -	if (cert_path) {
>> -		tls->cert_path = l_strdup(cert_path);
>> -		tls->priv_key_path = l_strdup(priv_key_path);
>> +	if (tls->priv_key) {
>> +		l_key_free(tls->priv_key);
>> +		tls->priv_key = NULL;
>> +		tls->priv_key_size = 0;
>>   	}
>> 
>> -	if (tls->priv_key_passphrase) {
>> -		memset(tls->priv_key_passphrase, 0,
>> -				strlen(tls->priv_key_passphrase));
>> -		l_free(tls->priv_key_passphrase);
>> -		tls->priv_key_passphrase = NULL;
>> +	if (priv_key_path) {
>> +		priv_key = l_pem_load_private_key(priv_key_path,
>> +							priv_key_passphrase,
>> +							&tls->priv_key_size);
>> +
>> +		tls->priv_key = l_key_new(L_KEY_RSA, priv_key,
>> +						tls->priv_key_size);
>> +		if (priv_key) {
>> +			memset(priv_key, 0, tls->priv_key_size);
>> +			l_free(priv_key);
>> +		}
>> +
>> +		if (!l_key_get_info(tls->priv_key, L_CIPHER_RSA_PKCS1_V1_5,
>> +					L_CHECKSUM_NONE, &tls->priv_key_size,
>> +					&is_public) || is_public) {
>> +			l_key_free(tls->priv_key);
>> +			tls->priv_key = NULL;
>> +			tls->priv_key_size = 0;
>> +			return false;
>> +		}
>> +
>> +		tls->priv_key_size /= 8;
>>   	}
>> 
>> -	if (priv_key_passphrase)
>> -		tls->priv_key_passphrase = l_strdup(priv_key_passphrase);
>> +	if (cert_path)
>> +		tls->cert_path = l_strdup(cert_path);
>> +
>> +	return true;
>>   }
>>
>>   LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
>> diff --git a/ell/tls.h b/ell/tls.h
>> index a3f3a28..0a7c920 100644
>> --- a/ell/tls.h
>> +++ b/ell/tls.h
>> @@ -97,7 +97,7 @@ void l_tls_set_cacert(struct l_tls *tls, const char 
>> *ca_cert_path);
>>    * one certificate of each type so they can be used depending on which
>>    * is compatible with the negotiated parameters.
>>    */
>> -void l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
>> +bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
>>   				const char *priv_key_path,
>>   				const char *priv_key_passphrase);
>> 
>> 
>
> Regards,
> -Denis
>

--
Mat Martineau
Intel OTC

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-08-09 18:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-08 17:25 [PATCH v2 1/6] key: Add NULL check to l_key_get_info Mat Martineau
2016-08-08 17:25 ` [PATCH v2 2/6] tls: Convert encrypt and verify to l_key crypto Mat Martineau
2016-08-09 16:17   ` Denis Kenzior
2016-08-09 18:36     ` Mat Martineau
2016-08-08 17:25 ` [PATCH v2 3/6] tls: Use l_key crypto for decrypt and sign Mat Martineau
2016-08-09 16:26   ` Denis Kenzior
2016-08-09 18:40     ` Mat Martineau
2016-08-08 17:25 ` [PATCH v2 4/6] unit: Generate and use PKCS8 version of server key for TLS tests Mat Martineau
2016-08-08 17:42   ` Denis Kenzior
2016-08-08 17:53     ` Mat Martineau
2016-08-08 19:58       ` Denis Kenzior
2016-08-08 22:27         ` Mat Martineau
2016-08-08 23:30           ` Denis Kenzior
2016-08-08 17:25 ` [PATCH v2 5/6] unit: Check return value of l_tls_set_auth_data Mat Martineau
2016-08-08 17:25 ` [PATCH v2 6/6] examples: " Mat Martineau
2016-08-08 17:39 ` [PATCH v2 1/6] key: Add NULL check to l_key_get_info Denis Kenzior

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.