[PATCH 1/8] util: Add L_IN_SET macros

[PATCH 1/8] util: Add L_IN_SET macros

[Andrew Zaborowski]

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

This can be used with enum types or ints as in L_IN_SET(-err, EIO, EINVAL).
I'm also adding an L_IN_STRSET() for string comparisons.  This
macro is not equivalent to (x == (val1) || x == (val2) || ...)
because val2 will be evaluated even if (x == (val1)) was true.  There
seems to be no way to avoid that without adding another numbr of macros
dependent on the maximum number of arguments expected.  The "x" in the
above example is evaluated once.

We may have to add a warning about everything being cast to the type of
the "x", I'm not sure how important of a problem this is.
---
 ell/util.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/ell/util.h b/ell/util.h
index 0b3e1a8..11708c2 100644
--- a/ell/util.h
+++ b/ell/util.h
@@ -315,6 +315,26 @@ const char *l_util_get_debugfs_path(void);
        while (__result == -1L && errno == EINTR);  \
        __result; }))
 
+#define _L_IN_SET_CMP(val, type, cmp, ...) __extension__ ({		\
+		const type __v = (val);					\
+		const typeof(__v) __elems[] = {__VA_ARGS__};		\
+		unsigned int __i;					\
+		static const unsigned int __n = L_ARRAY_SIZE(__elems);	\
+		bool __r = false;					\
+		for (__i = 0; __i < __n && !__r; __i++)			\
+			__r = (cmp);					\
+		__r;							\
+	})
+
+/* Warning: evaluates all set elements even after @val has matched one */
+#define L_IN_SET(val, ...)	\
+	_L_IN_SET_CMP((val), __auto_type, __v == __elems[__i], ##__VA_ARGS__)
+
+#define L_IN_STRSET(val, ...)						\
+	_L_IN_SET_CMP((val), const char *, __v == __elems[__i] ||	\
+				(__v && __elems[__i] &&			\
+				 !strcmp(__v, __elems[__i])), ##__VA_ARGS__)
+
 /*
  * Taken from https://github.com/chmike/cst_time_memcmp, adding a volatile to
  * ensure the compiler does not try to optimize the constant time behavior.
-- 
2.27.0

[PATCH 2/8] unit: Add an L_IN_SET test

[Andrew Zaborowski]

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

---
 unit/test-util.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/unit/test-util.c b/unit/test-util.c
index 883319c..067a708 100644
--- a/unit/test-util.c
+++ b/unit/test-util.c
@@ -124,6 +124,27 @@ static void test_strlcpy(const void *test_data)
 	do_strlcpy(10, 12);
 }
 
+static void test_in_set(const void *test_data)
+{
+	char *a1 = "a";
+	const char *a2 = a1;
+
+	assert(L_IN_SET(1, 1, 2, 3));
+	assert(L_IN_SET(2U, 1, 2, 3));
+	assert(L_IN_SET(3LL, 1, 2, 3));
+	assert(!L_IN_SET(4, 1, 2, 3));
+	assert(!L_IN_SET(4));
+
+	assert(L_IN_STRSET(a1, a2, "b"));
+	assert(L_IN_STRSET(a2, a1, "b"));
+	assert(L_IN_STRSET("b", "a", "b"));
+	assert(!L_IN_STRSET("c", "a", "b"));
+	assert(L_IN_STRSET(NULL, "a", NULL));
+	assert(!L_IN_STRSET(NULL, "a", "b"));
+	assert(!L_IN_STRSET("a", NULL, NULL));
+	assert(!L_IN_STRSET("a"));
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -136,5 +157,7 @@ int main(int argc, char *argv[])
 
 	l_test_add("l_strlcpy", test_strlcpy, NULL);
 
+	l_test_add("L_IN_SET", test_in_set, NULL);
+
 	return l_test_run();
 }
-- 
2.27.0

[PATCH 3/8] pem: Move PKCS private key parsing to cert.c

[Andrew Zaborowski]

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

This code implements the parsing and building of ASN.1 structures
defined in PKCS and RFCs that say nothing about encoding them as PEM, in
fact the PKCS#8 private key structures are also used in binary file
formats.  The legacy encrypted PKCS#1 RSAPrivateKey format is probably
only used in PEM files since it depends on the non-standard PEM headers,
so leave the decoding in pem.c.
---
 ell/cert-private.h |   8 +++
 ell/cert.c         | 163 +++++++++++++++++++++++++++++++++++++++++++++
 ell/pem-private.h  |   7 --
 ell/pem.c          | 160 +-------------------------------------------
 4 files changed, 174 insertions(+), 164 deletions(-)

diff --git a/ell/cert-private.h b/ell/cert-private.h
index 929f88d..e792c4c 100644
--- a/ell/cert-private.h
+++ b/ell/cert-private.h
@@ -28,3 +28,11 @@ void certchain_link_issuer(struct l_certchain *chain, struct l_cert *ca);
 const uint8_t *cert_get_extension(struct l_cert *cert,
 					const struct asn1_oid *ext_id,
 					bool *out_critical, size_t *out_len);
+
+struct l_key *cert_key_from_pkcs8_private_key_info(const uint8_t *der,
+							size_t der_len);
+struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
+							size_t der_len,
+							const char *passphrase);
+struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
+							size_t der_len);
diff --git a/ell/cert.c b/ell/cert.c
index cfe9a8d..a102fcc 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -29,6 +29,9 @@
 #include "key.h"
 #include "queue.h"
 #include "asn1-private.h"
+#include "cipher.h"
+#include "pkcs5.h"
+#include "pkcs5-private.h"
 #include "cert.h"
 #include "cert-private.h"
 
@@ -576,3 +579,163 @@ LIB_EXPORT bool l_certchain_verify(struct l_certchain *chain,
 	l_key_free(prev_key);
 	return true;
 }
+
+struct l_key *cert_key_from_pkcs8_private_key_info(const uint8_t *der,
+							size_t der_len)
+{
+	return l_key_new(L_KEY_RSA, der, der_len);
+}
+
+struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
+							size_t der_len,
+							const char *passphrase)
+{
+	const uint8_t *key_info, *alg_id, *data;
+	uint8_t tag;
+	size_t key_info_len, alg_id_len, data_len, tmp_len;
+	struct l_cipher *alg;
+	uint8_t *decrypted;
+	int i;
+	struct l_key *pkey;
+	bool r;
+	bool is_block;
+	size_t decrypted_len;
+
+	/* Technically this is BER, not limited to DER */
+	key_info = asn1_der_find_elem(der, der_len, 0, &tag, &key_info_len);
+	if (!key_info || tag != ASN1_ID_SEQUENCE)
+		return NULL;
+
+	alg_id = asn1_der_find_elem(key_info, key_info_len, 0, &tag,
+					&alg_id_len);
+	if (!alg_id || tag != ASN1_ID_SEQUENCE)
+		return NULL;
+
+	data = asn1_der_find_elem(key_info, key_info_len, 1, &tag, &data_len);
+	if (!data || tag != ASN1_ID_OCTET_STRING || data_len < 8 ||
+			(data_len & 7) != 0)
+		return NULL;
+
+	if (asn1_der_find_elem(der, der_len, 2, &tag, &tmp_len))
+		return NULL;
+
+	alg = pkcs5_cipher_from_alg_id(alg_id, alg_id_len, passphrase,
+					&is_block);
+	if (!alg)
+		return NULL;
+
+	decrypted = l_malloc(data_len);
+
+	r = l_cipher_decrypt(alg, data, decrypted, data_len);
+	l_cipher_free(alg);
+
+	if (!r) {
+		l_free(decrypted);
+		return NULL;
+	}
+
+	decrypted_len = data_len;
+
+	if (is_block) {
+		/*
+		 * For block ciphers strip padding as defined in RFC8018
+		 * (for PKCS#5 v1) or RFC1423 / RFC5652 (for v2).
+		 */
+		pkey = NULL;
+
+		if (decrypted[data_len - 1] >= data_len ||
+				decrypted[data_len - 1] > 16)
+			goto cleanup;
+
+		for (i = 1; i < decrypted[data_len - 1]; i++)
+			if (decrypted[data_len - 1 - i] !=
+					decrypted[data_len - 1])
+				goto cleanup;
+
+		decrypted_len -= decrypted[data_len - 1];
+	}
+
+	pkey = cert_key_from_pkcs8_private_key_info(decrypted, decrypted_len);
+
+cleanup:
+	explicit_bzero(decrypted, data_len);
+	l_free(decrypted);
+	return pkey;
+}
+
+struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
+							size_t der_len)
+{
+	const uint8_t *data;
+	uint8_t tag;
+	size_t data_len;
+	const uint8_t *key_data;
+	size_t key_data_len;
+	int i;
+	uint8_t *private_key;
+	size_t private_key_len;
+	uint8_t *one_asymmetric_key;
+	uint8_t *ptr;
+	struct l_key *pkey;
+
+	static const uint8_t version0[] = {
+		ASN1_ID_INTEGER, 0x01, 0x00
+	};
+	static const uint8_t pkcs1_rsa_encryption[] = {
+		ASN1_ID_SEQUENCE, 0x0d,
+		ASN1_ID_OID, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d,
+		0x01, 0x01, 0x01,
+		ASN1_ID_NULL, 0x00,
+	};
+
+	/*
+	 * Sanity check that it's a version 0 or 1 RSAPrivateKey structure
+	 * with the 8 integers.
+	 */
+	key_data = asn1_der_find_elem(der, der_len, 0, &tag, &key_data_len);
+	if (!key_data || tag != ASN1_ID_SEQUENCE)
+		return NULL;
+
+	data = asn1_der_find_elem(key_data, key_data_len, 0, &tag,
+					&data_len);
+	if (!data || tag != ASN1_ID_INTEGER || data_len != 1 ||
+			(data[0] != 0x00 && data[0] != 0x01))
+		return NULL;
+
+	for (i = 1; i < 9; i++) {
+		data = asn1_der_find_elem(key_data, key_data_len, i, &tag,
+						&data_len);
+		if (!data || tag != ASN1_ID_INTEGER || data_len < 1)
+			return NULL;
+	}
+
+	private_key = l_malloc(10 + der_len);
+	ptr = private_key;
+	*ptr++ = ASN1_ID_OCTET_STRING;
+	asn1_write_definite_length(&ptr, der_len);
+	memcpy(ptr, der, der_len);
+	ptr += der_len;
+	private_key_len = ptr - private_key;
+
+	one_asymmetric_key = l_malloc(32 + private_key_len);
+	ptr = one_asymmetric_key;
+	*ptr++ = ASN1_ID_SEQUENCE;
+	asn1_write_definite_length(&ptr, sizeof(version0) +
+					sizeof(pkcs1_rsa_encryption) +
+					private_key_len);
+	memcpy(ptr, version0, sizeof(version0));
+	ptr += sizeof(version0);
+	memcpy(ptr, pkcs1_rsa_encryption, sizeof(pkcs1_rsa_encryption));
+	ptr += sizeof(pkcs1_rsa_encryption);
+	memcpy(ptr, private_key, private_key_len);
+	ptr += private_key_len;
+	explicit_bzero(private_key, private_key_len);
+	l_free(private_key);
+
+	pkey = cert_key_from_pkcs8_private_key_info(one_asymmetric_key,
+						ptr - one_asymmetric_key);
+	explicit_bzero(one_asymmetric_key, ptr - one_asymmetric_key);
+	l_free(one_asymmetric_key);
+
+	return pkey;
+}
diff --git a/ell/pem-private.h b/ell/pem-private.h
index 10f918f..68e3fcb 100644
--- a/ell/pem-private.h
+++ b/ell/pem-private.h
@@ -33,12 +33,5 @@ const char *pem_next(const void *buf, size_t buf_len, char **type_label,
 				size_t *base64_len,
 				const char **endp, bool strict);
 
-struct l_key *pem_key_from_pkcs8_private_key_info(const uint8_t *der,
-							size_t der_len);
-
-struct l_key *pem_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
-							size_t der_len,
-							const char *passphrase);
-
 int pem_write_certificate_chain(const struct l_certchain *cert,
 				const char *filename);
diff --git a/ell/pem.c b/ell/pem.c
index fd25016..384fb9f 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -41,7 +41,6 @@
 #include "base64.h"
 #include "utf8.h"
 #include "asn1-private.h"
-#include "pkcs5-private.h"
 #include "cipher.h"
 #include "cert-private.h"
 #include "missing.h"
@@ -646,89 +645,6 @@ cleanup:
 	return cipher;
 }
 
-struct l_key *pem_key_from_pkcs8_private_key_info(const uint8_t *der,
-							size_t der_len)
-{
-	return l_key_new(L_KEY_RSA, der, der_len);
-}
-
-struct l_key *pem_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
-							size_t der_len,
-							const char *passphrase)
-{
-	const uint8_t *key_info, *alg_id, *data;
-	uint8_t tag;
-	size_t key_info_len, alg_id_len, data_len, tmp_len;
-	struct l_cipher *alg;
-	uint8_t *decrypted;
-	int i;
-	struct l_key *pkey;
-	bool r;
-	bool is_block;
-	size_t decrypted_len;
-
-	/* Technically this is BER, not limited to DER */
-	key_info = asn1_der_find_elem(der, der_len, 0, &tag, &key_info_len);
-	if (!key_info || tag != ASN1_ID_SEQUENCE)
-		return NULL;
-
-	alg_id = asn1_der_find_elem(key_info, key_info_len, 0, &tag,
-					&alg_id_len);
-	if (!alg_id || tag != ASN1_ID_SEQUENCE)
-		return NULL;
-
-	data = asn1_der_find_elem(key_info, key_info_len, 1, &tag, &data_len);
-	if (!data || tag != ASN1_ID_OCTET_STRING || data_len < 8 ||
-			(data_len & 7) != 0)
-		return NULL;
-
-	if (asn1_der_find_elem(der, der_len, 2, &tag, &tmp_len))
-		return NULL;
-
-	alg = pkcs5_cipher_from_alg_id(alg_id, alg_id_len, passphrase,
-					&is_block);
-	if (!alg)
-		return NULL;
-
-	decrypted = l_malloc(data_len);
-
-	r = l_cipher_decrypt(alg, data, decrypted, data_len);
-	l_cipher_free(alg);
-
-	if (!r) {
-		l_free(decrypted);
-		return NULL;
-	}
-
-	decrypted_len = data_len;
-
-	if (is_block) {
-		/*
-		 * For block ciphers strip padding as defined in RFC8018
-		 * (for PKCS#5 v1) or RFC1423 / RFC5652 (for v2).
-		 */
-		pkey = NULL;
-
-		if (decrypted[data_len - 1] >= data_len ||
-				decrypted[data_len - 1] > 16)
-			goto cleanup;
-
-		for (i = 1; i < decrypted[data_len - 1]; i++)
-			if (decrypted[data_len - 1 - i] !=
-					decrypted[data_len - 1])
-				goto cleanup;
-
-		decrypted_len -= decrypted[data_len - 1];
-	}
-
-	pkey = pem_key_from_pkcs8_private_key_info(decrypted, decrypted_len);
-
-cleanup:
-	explicit_bzero(decrypted, data_len);
-	l_free(decrypted);
-	return pkey;
-}
-
 static struct l_key *pem_load_private_key(uint8_t *content,
 						size_t len,
 						char *label,
@@ -749,7 +665,7 @@ static struct l_key *pem_load_private_key(uint8_t *content,
 		if (headers)
 			goto err;
 
-		pkey = pem_key_from_pkcs8_private_key_info(content, len);
+		pkey = cert_key_from_pkcs8_private_key_info(content, len);
 		goto done;
 	}
 
@@ -771,7 +687,7 @@ static struct l_key *pem_load_private_key(uint8_t *content,
 		if (headers)
 			goto err;
 
-		pkey = pem_key_from_pkcs8_encrypted_private_key_info(content,
+		pkey = cert_key_from_pkcs8_encrypted_private_key_info(content,
 								len,
 								passphrase);
 		goto done;
@@ -784,29 +700,9 @@ static struct l_key *pem_load_private_key(uint8_t *content,
 	 * PrivateKeyInfo for the pkcs8-key-parser module.
 	 */
 	if (!strcmp(label, "RSA PRIVATE KEY")) {
-		const uint8_t *data;
-		uint8_t tag;
-		size_t data_len;
-		const uint8_t *key_data;
-		size_t key_data_len;
-		int i;
-		uint8_t *private_key;
-		size_t private_key_len;
-		uint8_t *one_asymmetric_key;
-		uint8_t *ptr;
 		const char *dekalgid;
 		const char *dekparameters;
 
-		static const uint8_t version0[] = {
-			ASN1_ID_INTEGER, 0x01, 0x00
-		};
-		static const uint8_t pkcs1_rsa_encryption[] = {
-			ASN1_ID_SEQUENCE, 0x0d,
-			ASN1_ID_OID, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d,
-			0x01, 0x01, 0x01,
-			ASN1_ID_NULL, 0x00,
-		};
-
 		/*
 		 * "openssl rsa ..." can produce encrypted PKCS#1-formatted
 		 * keys.  These are incompatible with RFC7468 parsing because
@@ -855,57 +751,7 @@ static struct l_key *pem_load_private_key(uint8_t *content,
 			len -= content[len - 1];
 		}
 
-		/*
-		 * Sanity check that it's a version 0 or 1 RSAPrivateKey
-		 * structure with the 8 integers, if it's not, make a last
-		 * ditch attempt to load it into the kernel directly.
-		 */
-		key_data = asn1_der_find_elem(content, len, 0, &tag,
-						&key_data_len);
-		if (!key_data || tag != ASN1_ID_SEQUENCE)
-			goto err;
-
-		data = asn1_der_find_elem(key_data, key_data_len, 0, &tag,
-						&data_len);
-		if (!data || tag != ASN1_ID_INTEGER || data_len != 1 ||
-				(data[0] != 0x00 && data[0] != 0x01))
-			goto err;
-
-		for (i = 1; i < 9; i++) {
-			data = asn1_der_find_elem(key_data, key_data_len,
-							i, &tag, &data_len);
-			if (!data || tag != ASN1_ID_INTEGER || data_len < 1)
-				goto err;
-		}
-
-		private_key = l_malloc(10 + len);
-		ptr = private_key;
-		*ptr++ = ASN1_ID_OCTET_STRING;
-		asn1_write_definite_length(&ptr, len);
-		memcpy(ptr, content, len);
-		ptr += len;
-		private_key_len = ptr - private_key;
-
-		one_asymmetric_key = l_malloc(32 + private_key_len);
-		ptr = one_asymmetric_key;
-		*ptr++ = ASN1_ID_SEQUENCE;
-		asn1_write_definite_length(&ptr,
-						sizeof(version0) +
-						sizeof(pkcs1_rsa_encryption) +
-						private_key_len);
-		memcpy(ptr, version0, sizeof(version0));
-		ptr += sizeof(version0);
-		memcpy(ptr, pkcs1_rsa_encryption, sizeof(pkcs1_rsa_encryption));
-		ptr += sizeof(pkcs1_rsa_encryption);
-		memcpy(ptr, private_key, private_key_len);
-		ptr += private_key_len;
-		explicit_bzero(private_key, private_key_len);
-		l_free(private_key);
-
-		pkey = pem_key_from_pkcs8_private_key_info(one_asymmetric_key,
-						ptr - one_asymmetric_key);
-		explicit_bzero(one_asymmetric_key, ptr - one_asymmetric_key);
-		l_free(one_asymmetric_key);
+		pkey = cert_key_from_pkcs1_rsa_private_key(content, len);
 		goto done;
 	}
 
-- 
2.27.0

[PATCH 4/8] pkcs5: Rename to cert-crypto

[Andrew Zaborowski]

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

After recent changes pkcs5.c contained cryptographic routines not related
to PKCS#5 because there are ciphers in other standards defined in
similar ways.  From the pkcs5_cipher_from_alg_id() client's point of view
it didn't make sense to call different functions in different files
depending on which standard the Algorithm ID was expected or where any
well known cipher should be accepted.

cert.c and cert-crypto.c will now contain the classes and utilites related
to both certificates and private keys and low-level cryptographics
routines (in cert-crypto.c) related to the public-key cryptography
standard.  PKCS (Public-Key Cryptography Standards) would be a fitting
name but use the name "cert" to not imply that the contents are limited
to the standards created by RSA.

In this patch I rename both the file and the functions.  This is
backwards incompatible and users of the functions need to be updated.
---
 Makefile.am                    |  6 ++--
 ell/{pkcs5.c => cert-crypto.c} | 56 ++++++++++++++++++----------------
 ell/cert-private.h             | 19 ++++++++++++
 ell/cert.c                     |  6 ++--
 ell/cert.h                     |  9 ++++++
 ell/ell.h                      |  1 -
 ell/ell.sym                    |  5 ++-
 ell/pkcs5-private.h            | 38 -----------------------
 ell/pkcs5.h                    | 47 ----------------------------
 ell/tls.c                      |  2 +-
 10 files changed, 65 insertions(+), 124 deletions(-)
 rename ell/{pkcs5.c => cert-crypto.c} (93%)
 delete mode 100644 ell/pkcs5-private.h
 delete mode 100644 ell/pkcs5.h

diff --git a/Makefile.am b/Makefile.am
index ad68e4a..2f9a4ce 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,7 +46,6 @@ pkginclude_HEADERS = ell/ell.h \
 			ell/tls.h \
 			ell/uuid.h \
 			ell/key.h \
-			ell/pkcs5.h \
 			ell/file.h \
 			ell/dir.h \
 			ell/net.h \
@@ -114,8 +113,6 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/tls-suites.c \
 			ell/uuid.c \
 			ell/key.c \
-			ell/pkcs5-private.h \
-			ell/pkcs5.c \
 			ell/file.c \
 			ell/dir.c \
 			ell/net-private.h \
@@ -130,8 +127,9 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/dhcp6-lease.c \
 			ell/dhcp-util.c \
 			ell/dhcp-server.c \
-			ell/cert.c \
 			ell/cert-private.h \
+			ell/cert.c \
+			ell/cert-crypto.c \
 			ell/ecc-private.h \
 			ell/ecc.h \
 			ell/ecc-external.c \
diff --git a/ell/pkcs5.c b/ell/cert-crypto.c
similarity index 93%
rename from ell/pkcs5.c
rename to ell/cert-crypto.c
index 25bf431..6eb4e14 100644
--- a/ell/pkcs5.c
+++ b/ell/cert-crypto.c
@@ -2,7 +2,7 @@
  *
  *  Embedded Linux library
  *
- *  Copyright (C) 2017  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2020  Intel Corporation. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -35,16 +35,17 @@
 #include "util.h"
 #include "utf8.h"
 #include "asn1-private.h"
-#include "pkcs5.h"
-#include "pkcs5-private.h"
 #include "private.h"
 #include "missing.h"
+#include "cert.h"
+#include "cert-private.h"
 
 /* RFC8018 section 5.1 */
-LIB_EXPORT bool l_pkcs5_pbkdf1(enum l_checksum_type type, const char *password,
-				const uint8_t *salt, size_t salt_len,
-				unsigned int iter_count,
-				uint8_t *out_dk, size_t dk_len)
+LIB_EXPORT bool l_cert_pkcs5_pbkdf1(enum l_checksum_type type,
+					const char *password,
+					const uint8_t *salt, size_t salt_len,
+					unsigned int iter_count,
+					uint8_t *out_dk, size_t dk_len)
 {
 	size_t hash_len, t_len;
 	uint8_t t[20 + salt_len + strlen(password)];
@@ -103,10 +104,11 @@ LIB_EXPORT bool l_pkcs5_pbkdf1(enum l_checksum_type type, const char *password,
 }
 
 /* RFC8018 section 5.2 */
-LIB_EXPORT bool l_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
-				const uint8_t *salt, size_t salt_len,
-				unsigned int iter_count,
-				uint8_t *out_dk, size_t dk_len)
+LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
+					const char *password,
+					const uint8_t *salt, size_t salt_len,
+					unsigned int iter_count,
+					uint8_t *out_dk, size_t dk_len)
 {
 	size_t h_len;
 	struct l_checksum *checksum;
@@ -184,9 +186,11 @@ LIB_EXPORT bool l_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
 }
 
 /* RFC7292 Appendix B */
-uint8_t *pkcs12_pbkdf(const char *password, const struct pkcs12_hash *hash,
-			const uint8_t *salt, size_t salt_len,
-			unsigned int iterations, uint8_t id, size_t key_len)
+uint8_t *cert_pkcs12_pbkdf(const char *password,
+				const struct cert_pkcs12_hash *hash,
+				const uint8_t *salt, size_t salt_len,
+				unsigned int iterations, uint8_t id,
+				size_t key_len)
 {
 	/* All lengths in bytes instead of bits */
 	size_t passwd_len = password ? 2 * strlen(password) + 2 : 0;
@@ -301,7 +305,7 @@ uint8_t *pkcs12_pbkdf(const char *password, const struct pkcs12_hash *hash,
 }
 
 /* RFC7292 Appendix A */
-static const struct pkcs12_hash pkcs12_sha1_hash = {
+static const struct cert_pkcs12_hash pkcs12_sha1_hash = {
 	.alg = L_CHECKSUM_SHA1,
 	.len = 20,
 	.u   = 20,
@@ -459,7 +463,7 @@ static const struct pkcs5_enc_alg_oid {
 	},
 };
 
-static struct l_cipher *pkcs5_cipher_from_pbes2_params(
+static struct l_cipher *cipher_from_pkcs5_pbes2_params(
 						const uint8_t *pbes2_params,
 						size_t pbes2_params_len,
 						const char *password)
@@ -597,8 +601,8 @@ static struct l_cipher *pkcs5_cipher_from_pbes2_params(
 
 	/* RFC8018 section 6.2 */
 
-	if (!l_pkcs5_pbkdf2(prf_alg, password, salt, salt_len, iter_count,
-				derived_key, key_len))
+	if (!l_cert_pkcs5_pbkdf2(prf_alg, password, salt, salt_len, iter_count,
+					derived_key, key_len))
 		return NULL;
 
 	cipher = l_cipher_new(enc_scheme->cipher_type, derived_key, key_len);
@@ -611,7 +615,7 @@ static struct l_cipher *pkcs5_cipher_from_pbes2_params(
 	return cipher;
 }
 
-static struct l_cipher *pkcs12_cipher_from_alg_id(
+static struct l_cipher *cipher_from_pkcs12_alg_id(
 				const struct pkcs12_encryption_oid *scheme,
 				const uint8_t *params, size_t params_len,
 				const char *password, bool *out_is_block)
@@ -647,7 +651,7 @@ static struct l_cipher *pkcs12_cipher_from_alg_id(
 		return NULL;
 
 	key_len = scheme->key_length;
-	key = pkcs12_pbkdf(password, &pkcs12_sha1_hash, salt, salt_len,
+	key = cert_pkcs12_pbkdf(password, &pkcs12_sha1_hash, salt, salt_len,
 				iterations, 1, key_len);
 	if (!key)
 		return NULL;
@@ -678,7 +682,7 @@ static struct l_cipher *pkcs12_cipher_from_alg_id(
 		return NULL;
 
 	if (scheme->iv_length) {
-		uint8_t *iv = pkcs12_pbkdf(password, &pkcs12_sha1_hash,
+		uint8_t *iv = cert_pkcs12_pbkdf(password, &pkcs12_sha1_hash,
 						salt, salt_len, iterations, 2,
 						scheme->iv_length);
 
@@ -699,7 +703,7 @@ static struct l_cipher *pkcs12_cipher_from_alg_id(
 	return cipher;
 }
 
-struct l_cipher *pkcs5_cipher_from_alg_id(const uint8_t *id_asn1,
+struct l_cipher *cert_cipher_from_pkcs_alg_id(const uint8_t *id_asn1,
 						size_t id_asn1_len,
 						const char *password,
 						bool *out_is_block)
@@ -727,7 +731,7 @@ struct l_cipher *pkcs5_cipher_from_alg_id(const uint8_t *id_asn1,
 		if (out_is_block)
 			*out_is_block = true;
 
-		return pkcs5_cipher_from_pbes2_params(params, params_len,
+		return cipher_from_pkcs5_pbes2_params(params, params_len,
 							password);
 	}
 
@@ -746,7 +750,7 @@ struct l_cipher *pkcs5_cipher_from_alg_id(const uint8_t *id_asn1,
 		for (i = 0; i < L_ARRAY_SIZE(pkcs12_encryption_oids); i++)
 			if (asn1_oid_eq(&pkcs12_encryption_oids[i].oid,
 					oid_len, oid))
-				return pkcs12_cipher_from_alg_id(
+				return cipher_from_pkcs12_alg_id(
 						&pkcs12_encryption_oids[i],
 						params, params_len, password,
 						out_is_block);
@@ -774,8 +778,8 @@ struct l_cipher *pkcs5_cipher_from_alg_id(const uint8_t *id_asn1,
 
 	/* RFC8018 section 6.1 */
 
-	if (!l_pkcs5_pbkdf1(pbes1_scheme->checksum_type,
-				password, salt, 8, iter_count, derived_key, 16))
+	if (!l_cert_pkcs5_pbkdf1(pbes1_scheme->checksum_type, password,
+					salt, 8, iter_count, derived_key, 16))
 		return NULL;
 
 	cipher = l_cipher_new(pbes1_scheme->cipher_type, derived_key + 0, 8);
diff --git a/ell/cert-private.h b/ell/cert-private.h
index e792c4c..3fa9c9c 100644
--- a/ell/cert-private.h
+++ b/ell/cert-private.h
@@ -36,3 +36,22 @@ struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
 							const char *passphrase);
 struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
 							size_t der_len);
+
+struct cert_pkcs12_hash {
+	enum l_checksum_type alg;
+	unsigned int len;
+	unsigned int u;
+	unsigned int v;
+	struct asn1_oid oid;
+};
+
+uint8_t *cert_pkcs12_pbkdf(const char *password,
+				const struct cert_pkcs12_hash *hash,
+				const uint8_t *salt, size_t salt_len,
+				unsigned int iterations, uint8_t id,
+				size_t key_len);
+
+struct l_cipher *cert_cipher_from_pkcs_alg_id(const uint8_t *id_asn1,
+						size_t id_asn1_len,
+						const char *password,
+						bool *out_is_block);
diff --git a/ell/cert.c b/ell/cert.c
index a102fcc..8f0a4c2 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -30,8 +30,6 @@
 #include "queue.h"
 #include "asn1-private.h"
 #include "cipher.h"
-#include "pkcs5.h"
-#include "pkcs5-private.h"
 #include "cert.h"
 #include "cert-private.h"
 
@@ -619,8 +617,8 @@ struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
 	if (asn1_der_find_elem(der, der_len, 2, &tag, &tmp_len))
 		return NULL;
 
-	alg = pkcs5_cipher_from_alg_id(alg_id, alg_id_len, passphrase,
-					&is_block);
+	alg = cert_cipher_from_pkcs_alg_id(alg_id, alg_id_len, passphrase,
+						&is_block);
 	if (!alg)
 		return NULL;
 
diff --git a/ell/cert.h b/ell/cert.h
index 9fab88e..8dbb4ab 100644
--- a/ell/cert.h
+++ b/ell/cert.h
@@ -59,6 +59,15 @@ void l_certchain_walk_from_ca(struct l_certchain *chain,
 bool l_certchain_verify(struct l_certchain *chain, struct l_queue *ca_certs,
 			const char **error);
 
+bool l_cert_pkcs5_pbkdf1(enum l_checksum_type type, const char *password,
+				const uint8_t *salt, size_t salt_len,
+				unsigned int iter_count,
+				uint8_t *out_dk, size_t dk_len);
+bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
+				const uint8_t *salt, size_t salt_len,
+				unsigned int iter_count,
+				uint8_t *out_dk, size_t dk_len);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/ell/ell.h b/ell/ell.h
index 6662ad5..22fddf7 100644
--- a/ell/ell.h
+++ b/ell/ell.h
@@ -45,7 +45,6 @@
 #include <ell/tls.h>
 #include <ell/uuid.h>
 #include <ell/key.h>
-#include <ell/pkcs5.h>
 #include <ell/file.h>
 #include <ell/dir.h>
 #include <ell/net.h>
diff --git a/ell/ell.sym b/ell/ell.sym
index c98bd8c..cdbc5e8 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -415,9 +415,6 @@ global:
 	l_pem_load_file;
 	l_pem_load_private_key;
 	l_pem_load_private_key_from_data;
-	/* pkcs5 */
-	l_pkcs5_pbkdf1;
-	l_pkcs5_pbkdf2;
 	/* getrandom */
 	l_getrandom;
 	l_getrandom_is_supported;
@@ -536,6 +533,8 @@ global:
 	l_certchain_walk_from_leaf;
 	l_certchain_walk_from_ca;
 	l_certchain_verify;
+	l_cert_pkcs5_pbkdf1;
+	l_cert_pkcs5_pbkdf2;
 	/* ecc */
 	l_ecc_curve_get;
 	l_ecc_curve_get_name;
diff --git a/ell/pkcs5-private.h b/ell/pkcs5-private.h
deleted file mode 100644
index 9b85fdd..0000000
--- a/ell/pkcs5-private.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- *
- *  Embedded Linux library
- *
- *  Copyright (C) 2017  Intel Corporation. All rights reserved.
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-struct pkcs12_hash {
-	enum l_checksum_type alg;
-	unsigned int len;
-	unsigned int u;
-	unsigned int v;
-	struct asn1_oid oid;
-};
-
-uint8_t *pkcs12_pbkdf(const char *password, const struct pkcs12_hash *hash,
-			const uint8_t *salt, size_t salt_len,
-			unsigned int iterations, uint8_t id, size_t key_len);
-
-struct l_cipher *pkcs5_cipher_from_alg_id(const uint8_t *id_asn1,
-						size_t id_asn1_len,
-						const char *password,
-						bool *out_is_block);
diff --git a/ell/pkcs5.h b/ell/pkcs5.h
deleted file mode 100644
index ff7bdfd..0000000
--- a/ell/pkcs5.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- *
- *  Embedded Linux library
- *
- *  Copyright (C) 2017  Intel Corporation. All rights reserved.
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifndef __ELL_PKCS5_H
-#define __ELL_PKCS5_H
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-#include <stdbool.h>
-#include <stdint.h>
-
-bool l_pkcs5_pbkdf1(enum l_checksum_type type, const char *password,
-			const uint8_t *salt, size_t salt_len,
-			unsigned int iter_count,
-			uint8_t *out_dk, size_t dk_len);
-
-bool l_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
-			const uint8_t *salt, size_t salt_len,
-			unsigned int iter_count,
-			uint8_t *out_dk, size_t dk_len);
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif /* __ELL_PKCS5_H */
diff --git a/ell/tls.c b/ell/tls.c
index 4eaa66d..47eac22 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -38,11 +38,11 @@
 #include "queue.h"
 #include "pem.h"
 #include "pem-private.h"
+#include "asn1-private.h"
 #include "cert.h"
 #include "cert-private.h"
 #include "tls-private.h"
 #include "key.h"
-#include "asn1-private.h"
 #include "strv.h"
 #include "missing.h"
 #include "string.h"
-- 
2.27.0

[PATCH 5/8] cert: Add l_cert_load_container_file

[Andrew Zaborowski]

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

Add a function that takes a file path and detects what X.509 certificate
and/or private key format it uses and parses it accordingly.  This is to
make it easier for clients to support multiple file formats, including
raw X.509 certificates -- without this they would have to load the
contents of the file and call l_cert_new_from_der().
l_cert_load_container can also be used instead of
l_pem_load_certificate_chain() and l_pem_load_private_key().

This function can now load binary certificate files which are not PEM,
that's why it's placed in cert.c but a bunch of utilities in pem.c are
exposed in pem-private.h for this function.. It treats PEM files as
containers that can have both private keys and certificates at the same
time, openssl can parse such files and also produces such files in some
situations and they may have some good uses.
---
 ell/cert.c        | 184 ++++++++++++++++++++++++++++++++++++++++++++++
 ell/cert.h        |   5 ++
 ell/ell.sym       |   1 +
 ell/pem-private.h |  18 +++++
 ell/pem.c         |  34 +++------
 5 files changed, 220 insertions(+), 22 deletions(-)

diff --git a/ell/cert.c b/ell/cert.c
index 8f0a4c2..b2bf568 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -30,6 +30,7 @@
 #include "queue.h"
 #include "asn1-private.h"
 #include "cipher.h"
+#include "pem-private.h"
 #include "cert.h"
 #include "cert-private.h"
 
@@ -737,3 +738,186 @@ struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
 
 	return pkey;
 }
+
+/*
+ * Look at a file, try to detect which of the few X.509 certificate and/or
+ * private key container formats it uses and load any certificates in it as
+ * a certificate chain object, and load the first private key as an l_key
+ * object.
+ *
+ * Currently supported are:
+ *  PEM X.509 certificates
+ *  PEM PKCS#8 encrypted and unenecrypted private keys
+ *  PEM legacy PKCS#1 encrypted and unenecrypted private keys
+ *  Raw X.509 certificates (.cer, .der, .crt)
+ *
+ * The raw format contains exactly one certificate, PEM and PKCS#12 files
+ * can contain any combination of certificates and private keys.
+ *
+ * Returns false on "unrecoverable" errors, and *out_certchain,
+ * *out_privkey and *out_encrypted (if provided) are not modified.  However
+ * when true is returned, *out_certchain and *out_privkey (if provided) may
+ * be set to NULL when nothing could be loaded only due to missing password,
+ * and *out_encrypted (if provided) will be set accordingly.  It will also
+ * be set on success to indicate whether the password was used.
+ * *out_certchain and/or *out_privkey will also be NULL if the container
+ * was loaded but there were no certificates or private keys in it.
+ */
+LIB_EXPORT bool l_cert_load_container_file(const char *filename,
+					const char *password,
+					struct l_certchain **out_certchain,
+					struct l_key **out_privkey,
+					bool *out_encrypted)
+{
+	struct pem_file_info file;
+	const char *ptr;
+	size_t len;
+	bool error = true;
+	bool done = false;
+	struct l_certchain *certchain = NULL;
+	struct l_key *privkey = NULL;
+	bool encrypted = false;
+
+	if (unlikely(!filename))
+		return false;
+
+	if (pem_file_open(&file, filename) < 0)
+		return false;
+
+	if (file.st.st_size < 1)
+		goto close;
+
+	/* See if we have a DER sequence tag@the start */
+	if (file.data[0] == ASN1_ID_SEQUENCE) {
+		const uint8_t *seq_data;
+		const uint8_t *elem_data;
+		size_t elem_len;
+		uint8_t tag;
+
+		if (!(seq_data = asn1_der_find_elem(file.data, file.st.st_size,
+							0, &tag, &len)))
+			goto not_der_after_all;
+
+		/*
+		 * See if the first sub-element is another sequence, then, out
+		 * of the formats that we currently support this can only be a
+		 * raw certificate.  If integer, it's going to be PKCS#12.  If
+		 * we wish to add any more formats we'll probably need to start
+		 * guessing from the filename suffix.
+		 */
+		if (!(elem_data = asn1_der_find_elem(seq_data, len,
+							0, &tag, &elem_len)))
+			goto not_der_after_all;
+
+		if (tag == ASN1_ID_SEQUENCE) {
+			if (out_certchain) {
+				struct l_cert *cert;
+
+				if (!(cert = l_cert_new_from_der(file.data,
+							file.st.st_size)))
+					goto close;
+
+				error = false;
+				certchain = certchain_new_from_leaf(cert);
+			}
+
+			goto close;
+		}
+	}
+
+not_der_after_all:
+	/*
+	 * RFC 7486 allows whitespace and possibly other data before the
+	 * PEM "encapsulation boundary" so rather than check if the start
+	 * of the data looks like PEM, we fall back to this format if the
+	 * data didn't look like anything else we knew about.
+	 */
+	ptr = (const char *) file.data;
+	len = file.st.st_size;
+	error = false;
+	while (!done && !error && len) {
+		uint8_t *der;
+		size_t der_len;
+		char *type_label;
+		char *headers;
+		const char *endp;
+
+		if (!(der = pem_load_buffer(ptr, len, &type_label, &der_len,
+						&headers, &endp)))
+			break;
+
+		len -= endp - ptr;
+		ptr = endp;
+
+		if (out_certchain && L_IN_STRSET(type_label, "CERTIFICATE")) {
+			struct l_cert *cert;
+
+			if (!(cert = l_cert_new_from_der(der, der_len))) {
+				error = true;
+				goto next;
+			}
+
+			if (!certchain)
+				certchain = certchain_new_from_leaf(cert);
+			else
+				certchain_link_issuer(certchain, cert);
+
+			goto next;
+		}
+
+		/* Only use the first private key found */
+		if (out_privkey && !privkey && L_IN_STRSET(type_label,
+							"PRIVATE KEY",
+							"ENCRYPTED PRIVATE KEY",
+							"RSA PRIVATE KEY")) {
+			privkey = pem_load_private_key(der, der_len, type_label,
+							password, headers,
+							&encrypted);
+			if (!privkey) {
+				if (certchain) {
+					l_certchain_free(certchain);
+					certchain = NULL;
+				}
+
+				if (password)
+					error = true;
+				else
+					error = !encrypted || !out_encrypted;
+
+				done = true;
+			}
+
+			continue;
+		}
+
+next:
+		explicit_bzero(der, der_len);
+		l_free(der);
+		l_free(type_label);
+		l_free(headers);
+	}
+
+close:
+	pem_file_close(&file);
+
+	if (error) {
+		if (certchain)
+			l_certchain_free(certchain);
+
+		if (privkey)
+			l_key_free(privkey);
+
+		return false;
+	}
+
+	if (out_certchain)
+		*out_certchain = certchain;
+
+	if (out_privkey)
+		*out_privkey = privkey;
+
+	if (out_encrypted)
+		*out_encrypted = encrypted;
+
+	return true;
+}
diff --git a/ell/cert.h b/ell/cert.h
index 8dbb4ab..f3910bb 100644
--- a/ell/cert.h
+++ b/ell/cert.h
@@ -59,6 +59,11 @@ void l_certchain_walk_from_ca(struct l_certchain *chain,
 bool l_certchain_verify(struct l_certchain *chain, struct l_queue *ca_certs,
 			const char **error);
 
+bool l_cert_load_container_file(const char *filename, const char *password,
+				struct l_certchain **out_certchain,
+				struct l_key **out_privkey,
+				bool *out_encrypted);
+
 bool l_cert_pkcs5_pbkdf1(enum l_checksum_type type, const char *password,
 				const uint8_t *salt, size_t salt_len,
 				unsigned int iter_count,
diff --git a/ell/ell.sym b/ell/ell.sym
index cdbc5e8..aa0c046 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -533,6 +533,7 @@ global:
 	l_certchain_walk_from_leaf;
 	l_certchain_walk_from_ca;
 	l_certchain_verify;
+	l_cert_load_container_file;
 	l_cert_pkcs5_pbkdf1;
 	l_cert_pkcs5_pbkdf2;
 	/* ecc */
diff --git a/ell/pem-private.h b/ell/pem-private.h
index 68e3fcb..c249283 100644
--- a/ell/pem-private.h
+++ b/ell/pem-private.h
@@ -26,12 +26,30 @@
 
 #define _GNU_SOURCE
 #include <sys/types.h>
+#include <sys/stat.h>
 
 struct l_certchain;
 
+struct pem_file_info {
+	int fd;
+	struct stat st;
+	uint8_t *data;
+};
+
+int pem_file_open(struct pem_file_info *info, const char *filename);
+void pem_file_close(struct pem_file_info *info);
+
 const char *pem_next(const void *buf, size_t buf_len, char **type_label,
 				size_t *base64_len,
 				const char **endp, bool strict);
 
+uint8_t *pem_load_buffer(const void *buf, size_t buf_len,
+				char **out_type_label, size_t *out_len,
+				char **out_headers, const char **out_endp);
+
+struct l_key *pem_load_private_key(uint8_t *content, size_t len, char *label,
+					const char *passphrase, char *headers,
+					bool *encrypted);
+
 int pem_write_certificate_chain(const struct l_certchain *cert,
 				const char *filename);
diff --git a/ell/pem.c b/ell/pem.c
index 384fb9f..bc2865a 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -25,8 +25,6 @@
 #endif
 
 #define _GNU_SOURCE
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/mman.h>
@@ -192,9 +190,9 @@ const char *pem_next(const void *buf, size_t buf_len, char **type_label,
 	return NULL;
 }
 
-static uint8_t *pem_load_buffer(const void *buf, size_t buf_len,
+uint8_t *pem_load_buffer(const void *buf, size_t buf_len,
 				char **out_type_label, size_t *out_len,
-				char **out_headers)
+				char **out_headers, const char **out_endp)
 {
 	size_t base64_len;
 	const char *base64;
@@ -202,7 +200,7 @@ static uint8_t *pem_load_buffer(const void *buf, size_t buf_len,
 	uint8_t *ret;
 
 	base64 = pem_next(buf, buf_len, &label, &base64_len,
-				NULL, false);
+				out_endp, false);
 	if (!base64)
 		return NULL;
 
@@ -261,16 +259,10 @@ static uint8_t *pem_load_buffer(const void *buf, size_t buf_len,
 LIB_EXPORT uint8_t *l_pem_load_buffer(const void *buf, size_t buf_len,
 					char **type_label, size_t *out_len)
 {
-	return pem_load_buffer(buf, buf_len, type_label, out_len, NULL);
+	return pem_load_buffer(buf, buf_len, type_label, out_len, NULL, NULL);
 }
 
-struct pem_file_info {
-	int fd;
-	struct stat st;
-	uint8_t *data;
-};
-
-static int pem_file_open(struct pem_file_info *info, const char *filename)
+int pem_file_open(struct pem_file_info *info, const char *filename)
 {
 	info->fd = open(filename, O_RDONLY);
 	if (info->fd < 0)
@@ -295,7 +287,7 @@ static int pem_file_open(struct pem_file_info *info, const char *filename)
 	return 0;
 }
 
-static void pem_file_close(struct pem_file_info *info)
+void pem_file_close(struct pem_file_info *info)
 {
 	munmap(info->data, info->st.st_size);
 	close(info->fd);
@@ -314,7 +306,8 @@ static uint8_t *pem_load_file(const char *filename, char **out_type_label,
 		return NULL;
 
 	result = pem_load_buffer(file.data, file.st.st_size,
-					out_type_label, out_len, out_headers);
+					out_type_label, out_len, out_headers,
+					NULL);
 	pem_file_close(&file);
 	return result;
 }
@@ -645,12 +638,9 @@ cleanup:
 	return cipher;
 }
 
-static struct l_key *pem_load_private_key(uint8_t *content,
-						size_t len,
-						char *label,
-						const char *passphrase,
-						char *headers,
-						bool *encrypted)
+struct l_key *pem_load_private_key(uint8_t *content, size_t len, char *label,
+					const char *passphrase, char *headers,
+					bool *encrypted)
 {
 	struct l_key *pkey;
 
@@ -779,7 +769,7 @@ LIB_EXPORT struct l_key *l_pem_load_private_key_from_data(const void *buf,
 	if (encrypted)
 		*encrypted = false;
 
-	content = pem_load_buffer(buf, buf_len, &label, &len, &headers);
+	content = pem_load_buffer(buf, buf_len, &label, &len, &headers, NULL);
 
 	if (!content)
 		return NULL;
-- 
2.27.0

Re: [PATCH 5/8] cert: Add l_cert_load_container_file

[Denis Kenzior]

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

Hi Andrew,

On 1/6/21 1:54 PM, Andrew Zaborowski wrote:
> Add a function that takes a file path and detects what X.509 certificate
> and/or private key format it uses and parses it accordingly.  This is to
> make it easier for clients to support multiple file formats, including
> raw X.509 certificates -- without this they would have to load the
> contents of the file and call l_cert_new_from_der().
> l_cert_load_container can also be used instead of
> l_pem_load_certificate_chain() and l_pem_load_private_key().
> 
> This function can now load binary certificate files which are not PEM,
> that's why it's placed in cert.c but a bunch of utilities in pem.c are
> exposed in pem-private.h for this function.. It treats PEM files as
> containers that can have both private keys and certificates at the same
> time, openssl can parse such files and also produces such files in some
> situations and they may have some good uses.
> ---
>   ell/cert.c        | 184 ++++++++++++++++++++++++++++++++++++++++++++++
>   ell/cert.h        |   5 ++
>   ell/ell.sym       |   1 +
>   ell/pem-private.h |  18 +++++
>   ell/pem.c         |  34 +++------
>   5 files changed, 220 insertions(+), 22 deletions(-)
> 
> diff --git a/ell/cert.c b/ell/cert.c
> index 8f0a4c2..b2bf568 100644
> --- a/ell/cert.c
> +++ b/ell/cert.c
> @@ -30,6 +30,7 @@
>   #include "queue.h"
>   #include "asn1-private.h"
>   #include "cipher.h"
> +#include "pem-private.h"
>   #include "cert.h"
>   #include "cert-private.h"
>   
> @@ -737,3 +738,186 @@ struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
>   
>   	return pkey;
>   }
> +
> +/*
> + * Look at a file, try to detect which of the few X.509 certificate and/or
> + * private key container formats it uses and load any certificates in it as
> + * a certificate chain object, and load the first private key as an l_key
> + * object.
> + *
> + * Currently supported are:
> + *  PEM X.509 certificates
> + *  PEM PKCS#8 encrypted and unenecrypted private keys
> + *  PEM legacy PKCS#1 encrypted and unenecrypted private keys

I fixed 'unenerypted' -> 'unencrypted' in these two lines

> + *  Raw X.509 certificates (.cer, .der, .crt)
> + *
> + * The raw format contains exactly one certificate, PEM and PKCS#12 files
> + * can contain any combination of certificates and private keys.
> + *
> + * Returns false on "unrecoverable" errors, and *out_certchain,
> + * *out_privkey and *out_encrypted (if provided) are not modified.  However
> + * when true is returned, *out_certchain and *out_privkey (if provided) may
> + * be set to NULL when nothing could be loaded only due to missing password,
> + * and *out_encrypted (if provided) will be set accordingly.  It will also
> + * be set on success to indicate whether the password was used.
> + * *out_certchain and/or *out_privkey will also be NULL if the container
> + * was loaded but there were no certificates or private keys in it.
> + */
> +LIB_EXPORT bool l_cert_load_container_file(const char *filename,
> +					const char *password,
> +					struct l_certchain **out_certchain,
> +					struct l_key **out_privkey,
> +					bool *out_encrypted)
> +{
> +	struct pem_file_info file;
> +	const char *ptr;
> +	size_t len;
> +	bool error = true;
> +	bool done = false;
> +	struct l_certchain *certchain = NULL;
> +	struct l_key *privkey = NULL;
> +	bool encrypted = false;
> +
> +	if (unlikely(!filename))
> +		return false;
> +
> +	if (pem_file_open(&file, filename) < 0)
> +		return false;
> +
> +	if (file.st.st_size < 1)
> +		goto close;
> +
> +	/* See if we have a DER sequence tag at the start */
> +	if (file.data[0] == ASN1_ID_SEQUENCE) {
> +		const uint8_t *seq_data;
> +		const uint8_t *elem_data;
> +		size_t elem_len;
> +		uint8_t tag;
> +
> +		if (!(seq_data = asn1_der_find_elem(file.data, file.st.st_size,
> +							0, &tag, &len)))
> +			goto not_der_after_all;
> +
> +		/*
> +		 * See if the first sub-element is another sequence, then, out
> +		 * of the formats that we currently support this can only be a
> +		 * raw certificate.  If integer, it's going to be PKCS#12.  If
> +		 * we wish to add any more formats we'll probably need to start
> +		 * guessing from the filename suffix.
> +		 */
> +		if (!(elem_data = asn1_der_find_elem(seq_data, len,
> +							0, &tag, &elem_len)))
> +			goto not_der_after_all;
> +
> +		if (tag == ASN1_ID_SEQUENCE) {
> +			if (out_certchain) {
> +				struct l_cert *cert;
> +
> +				if (!(cert = l_cert_new_from_der(file.data,
> +							file.st.st_size)))
> +					goto close;
> +
> +				error = false;
> +				certchain = certchain_new_from_leaf(cert);
> +			}
> +
> +			goto close;
> +		}
> +	}
> +
> +not_der_after_all:
> +	/*
> +	 * RFC 7486 allows whitespace and possibly other data before the
> +	 * PEM "encapsulation boundary" so rather than check if the start
> +	 * of the data looks like PEM, we fall back to this format if the
> +	 * data didn't look like anything else we knew about.
> +	 */
> +	ptr = (const char *) file.data;
> +	len = file.st.st_size;
> +	error = false;
> +	while (!done && !error && len) {
> +		uint8_t *der;
> +		size_t der_len;
> +		char *type_label;
> +		char *headers;
> +		const char *endp;
> +
> +		if (!(der = pem_load_buffer(ptr, len, &type_label, &der_len,
> +						&headers, &endp)))
> +			break;
> +
> +		len -= endp - ptr;
> +		ptr = endp;
> +
> +		if (out_certchain && L_IN_STRSET(type_label, "CERTIFICATE")) {
> +			struct l_cert *cert;
> +
> +			if (!(cert = l_cert_new_from_der(der, der_len))) {
> +				error = true;
> +				goto next;
> +			}
> +
> +			if (!certchain)
> +				certchain = certchain_new_from_leaf(cert);
> +			else
> +				certchain_link_issuer(certchain, cert);
> +
> +			goto next;
> +		}
> +
> +		/* Only use the first private key found */
> +		if (out_privkey && !privkey && L_IN_STRSET(type_label,
> +							"PRIVATE KEY",
> +							"ENCRYPTED PRIVATE KEY",
> +							"RSA PRIVATE KEY")) {
> +			privkey = pem_load_private_key(der, der_len, type_label,
> +							password, headers,
> +							&encrypted);

Remind me again why pem_load_private_key frees all the input data?  This makes 
things really asymmetrical in this function.  The logic seems fine, but I found 
it hard to follow.  Not sure if this is worth fixing though...

> +			if (!privkey) {
> +				if (certchain) {
> +					l_certchain_free(certchain);
> +					certchain = NULL;
> +				}
> +
> +				if (password)
> +					error = true;
> +				else
> +					error = !encrypted || !out_encrypted;
> +
> +				done = true;
> +			}
> +
> +			continue;
> +		}
> +
> +next:
> +		explicit_bzero(der, der_len);
> +		l_free(der);
> +		l_free(type_label);
> +		l_free(headers);
> +	}
> +
> +close:
> +	pem_file_close(&file);
> +
> +	if (error) {
> +		if (certchain)
> +			l_certchain_free(certchain);
> +
> +		if (privkey)
> +			l_key_free(privkey);
> +
> +		return false;
> +	}
> +
> +	if (out_certchain)
> +		*out_certchain = certchain;
> +
> +	if (out_privkey)
> +		*out_privkey = privkey;
> +
> +	if (out_encrypted)
> +		*out_encrypted = encrypted;
> +
> +	return true;
> +}

Applied, thanks.

Regards,
-Denis

Re: [PATCH 5/8] cert: Add l_cert_load_container_file

[Andrew Zaborowski]

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

On Fri, 8 Jan 2021 at 05:37, Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Andrew,
>
> On 1/6/21 1:54 PM, Andrew Zaborowski wrote:
> > + * Currently supported are:
> > + *  PEM X.509 certificates
> > + *  PEM PKCS#8 encrypted and unenecrypted private keys
> > + *  PEM legacy PKCS#1 encrypted and unenecrypted private keys
>
> I fixed 'unenerypted' -> 'unencrypted' in these two lines
>
> > +                     privkey = pem_load_private_key(der, der_len, type_label,
> > +                                                     password, headers,
> > +                                                     &encrypted);
>
> Remind me again why pem_load_private_key frees all the input data?

Within pem.c this avoided some duplication in the callers, but...

> This makes
> things really asymmetrical in this function.

It does, let me know if you prefer it fixed.

Best regards

Re: [PATCH 5/8] cert: Add l_cert_load_container_file

[Denis Kenzior]

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

Hi Andrew,

>> This makes
>> things really asymmetrical in this function.
> 
> It does, let me know if you prefer it fixed.
> 

That would be nice, but I'm not sure what the fix might be?

It almost feels like you should break up this function into separate parts.  One 
part for loading PEMs, one part for loading raw DERs, etc.  I mean if you look 
at the code block that handles "CERTIFICATE" PEMs, it is almost exact copy of 
what the block under 'if (tag == ASN1_ID_SEQUENCE)' does.  That might also 
contribute to fixing your asymmetry...

Regards,
-Denis

[PATCH 6/8] cert: Add PKCS#12 loading support

[Andrew Zaborowski]

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

Add ability to parse PKCS#12 files in l_cert_load_container_file().  This
has only been tested with the files produced by openssl so far.

Extend the static buffer in struct asn1_oid (asn1-private.h) so we can
handle some 11-byte OIDs used in PKCS#12 files.
---
 ell/asn1-private.h |   2 +-
 ell/cert.c         | 634 +++++++++++++++++++++++++++++++++++++++++++++
 ell/pem.c          |   4 +
 3 files changed, 639 insertions(+), 1 deletion(-)

diff --git a/ell/asn1-private.h b/ell/asn1-private.h
index 2a31241..079570b 100644
--- a/ell/asn1-private.h
+++ b/ell/asn1-private.h
@@ -37,7 +37,7 @@
 
 struct asn1_oid {
 	uint8_t asn1_len;
-	uint8_t asn1[10];
+	uint8_t asn1[11];
 };
 
 #define asn1_oid_eq(oid1, oid2_len, oid2_string) \
diff --git a/ell/cert.c b/ell/cert.c
index b2bf568..fb4e443 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -585,6 +585,11 @@ struct l_key *cert_key_from_pkcs8_private_key_info(const uint8_t *der,
 	return l_key_new(L_KEY_RSA, der, der_len);
 }
 
+/*
+ * The passphrase, if given, must have been validated as UTF-8 unless the
+ * caller knows that PKCS#12 encryption algorithms are not used.
+ * Use l_utf8_validate.
+ */
 struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
 							size_t der_len,
 							const char *passphrase)
@@ -739,6 +744,580 @@ struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
 	return pkey;
 }
 
+static const uint8_t *cert_unpack_pkcs7_content_info(const uint8_t *container,
+					size_t container_len, int pos,
+					const struct asn1_oid *expected_oid,
+					struct asn1_oid *out_oid,
+					uint8_t *out_tag, size_t *out_len)
+{
+	const uint8_t *content_info;
+	size_t content_info_len;
+	const uint8_t *type;
+	size_t type_len;
+	const uint8_t *ret;
+	uint8_t tag;
+
+	if (!(content_info = asn1_der_find_elem(container, container_len, pos,
+						&tag, &content_info_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return NULL;
+
+	if (!(type = asn1_der_find_elem(content_info, content_info_len, 0,
+					&tag, &type_len)) ||
+			tag != ASN1_ID_OID ||
+			type_len > sizeof(out_oid->asn1))
+		return NULL;
+
+	if (expected_oid && !asn1_oid_eq(expected_oid, type_len, type))
+		return NULL;
+
+	if (!(ret = asn1_der_find_elem(content_info, content_info_len,
+					ASN1_CONTEXT_EXPLICIT(0),
+					out_tag, out_len)) ||
+			ret + *out_len != content_info + content_info_len)
+		return NULL;
+
+	if (out_oid) {
+		out_oid->asn1_len = type_len;
+		memcpy(out_oid->asn1, type, type_len);
+	}
+
+	return ret;
+}
+
+/* RFC5652 Section 8 */
+static uint8_t *cert_decrypt_pkcs7_encrypted_data(const uint8_t *data,
+						size_t data_len,
+						const char *password,
+						struct asn1_oid *out_oid,
+						size_t *out_len)
+{
+	const uint8_t *version;
+	size_t version_len;
+	const uint8_t *encrypted_info;
+	size_t encrypted_info_len;
+	const uint8_t *type;
+	size_t type_len;
+	const uint8_t *alg_id;
+	size_t alg_id_len;
+	const uint8_t *encrypted;
+	size_t encrypted_len;
+	uint8_t tag;
+	struct l_cipher *alg;
+	uint8_t *plaintext;
+	int i;
+	bool ok;
+	bool is_block;
+
+	if (!(version = asn1_der_find_elem(data, data_len, 0, &tag,
+						&version_len)) ||
+			tag != ASN1_ID_INTEGER || version_len != 1 ||
+			!L_IN_SET(version[0], 0, 2))
+		return NULL;
+
+	if (!(encrypted_info = asn1_der_find_elem(data, data_len, 1, &tag,
+							&encrypted_info_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return NULL;
+
+	if (!(type = asn1_der_find_elem(encrypted_info, encrypted_info_len, 0,
+					&tag, &type_len)) ||
+			tag != ASN1_ID_OID ||
+			type_len > sizeof(out_oid->asn1))
+		return NULL;
+
+	if (!(alg_id = asn1_der_find_elem(encrypted_info, encrypted_info_len, 1,
+					&tag, &alg_id_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return NULL;
+
+	/* Not optional in our case, defined [0] IMPLICIT OCTET STRING */
+	if (!(encrypted = asn1_der_find_elem(encrypted_info, encrypted_info_len,
+						ASN1_CONTEXT_IMPLICIT(0),
+						&tag, &encrypted_len)) ||
+			tag != ASN1_ID(ASN1_CLASS_CONTEXT, 0, 0) ||
+			encrypted_len < 8)
+		return NULL;
+
+	if (!(alg = cert_cipher_from_pkcs_alg_id(alg_id, alg_id_len, password,
+							&is_block)))
+		return NULL;
+
+	plaintext = l_malloc(encrypted_len);
+	ok = l_cipher_decrypt(alg, encrypted, plaintext, encrypted_len);
+	l_cipher_free(alg);
+
+	if (!ok)
+		return NULL;
+
+	if (is_block) {
+		bool ok = true;
+
+		/* Also validate the padding */
+		if (encrypted_len < plaintext[encrypted_len - 1] ||
+				plaintext[encrypted_len - 1] > 16)
+			return NULL;
+
+		for (i = 1; i < plaintext[encrypted_len - 1]; i++)
+			if (plaintext[encrypted_len - 1 - i] !=
+					plaintext[encrypted_len - 1])
+				ok = false;
+
+		if (!ok) {
+			explicit_bzero(plaintext, encrypted_len);
+			l_free(plaintext);
+			return NULL;
+		}
+
+		encrypted_len -= plaintext[encrypted_len - 1];
+	}
+
+	if (out_oid) {
+		out_oid->asn1_len = type_len;
+		memcpy(out_oid->asn1, type, type_len);
+	}
+
+	*out_len = encrypted_len;
+	return plaintext;
+}
+
+/* RFC7292 Appendix A. */
+static const struct cert_pkcs12_hash pkcs12_mac_algs[] = {
+	{
+		L_CHECKSUM_MD5,    16, 16, 64,
+		{ 8, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0f, 0x02, 0x05 } }
+	},
+	{
+		L_CHECKSUM_SHA1,   20, 20, 64,
+		{ 5, { 0x2b, 0x0e, 0x03, 0x02, 0x1a } }
+	},
+	{
+		L_CHECKSUM_SHA224, 28, 28, 64,
+		{ 9, { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04 } }
+	},
+	{
+		L_CHECKSUM_SHA256, 32, 32, 64,
+		{ 9, { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01 } }
+	},
+	{
+		L_CHECKSUM_SHA384, 48, 48, 128,
+		{ 9, { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02 } }
+	},
+	{
+		L_CHECKSUM_SHA512, 64, 64, 128,
+		{ 9, { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03 } }
+	},
+	{
+		L_CHECKSUM_SHA512, 64, 28, 128,
+		{ 9, { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x05 } }
+	},
+	{
+		L_CHECKSUM_SHA512, 64, 32, 128,
+		{ 9, { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x06 } }
+	},
+};
+
+static const struct asn1_oid pkcs12_key_bag_oid = {
+	11, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x0c, 0x0a, 0x01, 0x01 }
+};
+
+static const struct asn1_oid pkcs12_pkcs8_shrouded_key_bag_oid = {
+	11, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x0c, 0x0a, 0x01, 0x02 }
+};
+
+static const struct asn1_oid pkcs12_cert_bag_oid = {
+	11, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x0c, 0x0a, 0x01, 0x03 }
+};
+
+static const struct asn1_oid pkcs12_safe_contents_bag_oid = {
+	11, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x0c, 0x0a, 0x01, 0x06 }
+};
+
+static const struct asn1_oid pkcs9_x509_certificate_oid = {
+	10, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x16, 0x01 }
+};
+
+/* RFC7292 Section 4.2.3 */
+static bool cert_parse_pkcs12_cert_bag(const uint8_t *data, size_t data_len,
+					struct l_certchain **out_certchain)
+{
+	const uint8_t *cert_bag;
+	size_t cert_bag_len;
+	const uint8_t *cert_id;
+	size_t cert_id_len;
+	const uint8_t *cert_value;
+	size_t cert_value_len;
+	uint8_t tag;
+	struct l_cert *cert;
+
+	if (!(cert_bag = asn1_der_find_elem(data, data_len, 0,
+						&tag, &cert_bag_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return false;
+
+	if (!(cert_id = asn1_der_find_elem(cert_bag, cert_bag_len, 0,
+						&tag, &cert_id_len)) ||
+			tag != ASN1_ID_OID)
+		return false;
+
+	if (!(cert_value = asn1_der_find_elem(cert_bag, cert_bag_len,
+						ASN1_CONTEXT_EXPLICIT(0),
+						&tag, &cert_value_len)) ||
+			tag != ASN1_ID_OCTET_STRING ||
+			cert_value + cert_value_len != data + data_len)
+		return false;
+
+	/* Skip unsupported certificate types */
+	if (!asn1_oid_eq(&pkcs9_x509_certificate_oid, cert_id_len, cert_id))
+		return true;
+
+	if (!(cert = l_cert_new_from_der(cert_value, cert_value_len)))
+		return false;
+
+	if (!*out_certchain)
+		*out_certchain = certchain_new_from_leaf(cert);
+	else
+		certchain_link_issuer(*out_certchain, cert);
+
+	return true;
+}
+
+static bool cert_parse_pkcs12_safe_contents(const uint8_t *data,
+					size_t data_len, const char *password,
+					struct l_certchain **out_certchain,
+					struct l_key **out_privkey)
+{
+	const uint8_t *safe_contents;
+	size_t safe_contents_len;
+	uint8_t tag;
+
+	if (!(safe_contents = asn1_der_find_elem(data, data_len, 0, &tag,
+							&safe_contents_len)) ||
+			tag != ASN1_ID_SEQUENCE ||
+			data + data_len != safe_contents + safe_contents_len)
+		return false;
+
+	/* RFC7292 Section 4.2 */
+	while (safe_contents_len) {
+		const uint8_t *safe_bag;
+		size_t safe_bag_len;
+		const uint8_t *bag_id;
+		size_t bag_id_len;
+		const uint8_t *bag_value;
+		int bag_value_len;
+
+		/* RFC7292 Section 4.2 */
+		if (!(safe_bag = asn1_der_find_elem(safe_contents,
+							safe_contents_len, 0,
+							&tag, &safe_bag_len)) ||
+				tag != ASN1_ID_SEQUENCE)
+			return false;
+
+		if (!(bag_id = asn1_der_find_elem(safe_bag, safe_bag_len, 0,
+							&tag, &bag_id_len)) ||
+				tag != ASN1_ID_OID)
+			return false;
+
+		/*
+		 * The bagValue is EXPLICITly tagged but we don't want to
+		 * unpack the inner TLV yet so don't use asn1_der_find_elem.
+		 */
+		safe_bag_len -= bag_id + bag_id_len - safe_bag;
+		safe_bag = bag_id + bag_id_len;
+
+		if (safe_bag_len < 4)
+			return false;
+
+		tag = *safe_bag++;
+		safe_bag_len--;
+		bag_value_len = asn1_parse_definite_length(&safe_bag,
+								&safe_bag_len);
+		bag_value = safe_bag;
+
+		if (bag_value_len < 0 || bag_value_len > (int) safe_bag_len ||
+				tag != ASN1_ID(ASN1_CLASS_CONTEXT, 1, 0))
+			return false;
+
+		/* PKCS#9 attributes ignored */
+
+		safe_contents_len -= (safe_bag + safe_bag_len - safe_contents);
+		safe_contents = safe_bag + safe_bag_len;
+
+		if (asn1_oid_eq(&pkcs12_key_bag_oid, bag_id_len, bag_id)) {
+			if (!out_privkey || *out_privkey)
+				continue;
+
+			*out_privkey =
+				cert_key_from_pkcs8_private_key_info(bag_value,
+								bag_value_len);
+			if (!*out_privkey)
+				return false;
+		} else if (asn1_oid_eq(&pkcs12_pkcs8_shrouded_key_bag_oid,
+					bag_id_len, bag_id)) {
+			if (!out_privkey || *out_privkey)
+				continue;
+
+			*out_privkey =
+				cert_key_from_pkcs8_encrypted_private_key_info(
+								bag_value,
+								bag_value_len,
+								password);
+			if (!*out_privkey)
+				return false;
+		} else if (asn1_oid_eq(&pkcs12_cert_bag_oid,
+					bag_id_len, bag_id)) {
+			if (!out_certchain)
+				continue;
+
+			if (!cert_parse_pkcs12_cert_bag(bag_value, bag_value_len,
+							out_certchain))
+				return false;
+		} else if (asn1_oid_eq(&pkcs12_safe_contents_bag_oid,
+					bag_id_len, bag_id)) {
+			/* TODO: depth check */
+			if (!(cert_parse_pkcs12_safe_contents(bag_value,
+								bag_value_len,
+								password,
+								out_certchain,
+								out_privkey)))
+				return false;
+		}
+	}
+
+	return true;
+}
+
+/* RFC5652 Section 4 */
+static const struct asn1_oid pkcs7_data_oid = {
+	9, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x01 }
+};
+
+/* RFC5652 Section 8 */
+static const struct asn1_oid pkcs7_encrypted_data_oid = {
+	9, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x06 }
+};
+
+static bool cert_parse_pkcs12_pfx(const uint8_t *ptr, size_t len,
+					const char *password,
+					struct l_certchain **out_certchain,
+					struct l_key **out_privkey)
+{
+	const uint8_t *version;
+	size_t version_len;
+	const uint8_t *auth_safe;
+	size_t auth_safe_len;
+	const uint8_t *auth_safe_seq;
+	size_t auth_safe_seq_len;
+	const uint8_t *mac_data;
+	size_t mac_data_len;
+	const uint8_t *mac;
+	size_t mac_len;
+	const uint8_t *mac_salt;
+	size_t mac_salt_len;
+	const uint8_t *iterations_data;
+	size_t iterations_len;
+	unsigned int iterations;
+	const uint8_t *digest_alg;
+	size_t digest_alg_len;
+	const uint8_t *digest;
+	size_t digest_len;
+	const uint8_t *alg_id;
+	size_t alg_id_len;
+	const struct cert_pkcs12_hash *mac_hash;
+	L_AUTO_FREE_VAR(uint8_t *, key) = NULL;
+	struct l_checksum *hmac;
+	uint8_t hmac_val[64];
+	uint8_t tag;
+	unsigned int i;
+	bool ok;
+
+	/* RFC7292 Section 4 */
+	if (!(version = asn1_der_find_elem(ptr, len, 0, &tag, &version_len)) ||
+			tag != ASN1_ID_INTEGER)
+		return false;
+
+	if (version_len != 1 || version[0] != 3)
+		return false;
+
+	/*
+	 * Since we only support the password-based integrity mode,  the
+	 * authSafe must be of PKCS#7 type "data" and not "signedData".
+	 */
+	if (!(auth_safe = cert_unpack_pkcs7_content_info(ptr, len, 1,
+							&pkcs7_data_oid, NULL,
+							&tag,
+							&auth_safe_len)) ||
+			tag != ASN1_ID_OCTET_STRING)
+		return false;
+
+	/*
+	 * openssl can generate PFX structures without macData not signed
+	 * with a public key so handle this case, otherwise the macData
+	 * would not be optional.
+	 */
+	if (auth_safe + auth_safe_len == ptr + len)
+		goto integrity_check_done;
+
+	if (!(mac_data = asn1_der_find_elem(ptr, len, 2, &tag,
+						&mac_data_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return false;
+
+	if (!(mac = asn1_der_find_elem(mac_data, mac_data_len, 0, &tag,
+					&mac_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return false;
+
+	if (!(mac_salt = asn1_der_find_elem(mac_data, mac_data_len, 1, &tag,
+						&mac_salt_len)) ||
+			tag != ASN1_ID_OCTET_STRING || mac_salt_len > 1024)
+		return false;
+
+	if (!(iterations_data = asn1_der_find_elem(mac_data, mac_data_len, 2,
+							&tag,
+							&iterations_len)) ||
+			tag != ASN1_ID_INTEGER || iterations_len > 4)
+		return false;
+
+	for (iterations = 0; iterations_len; iterations_len--)
+		iterations = (iterations << 8) | *iterations_data++;
+
+	if (iterations < 1 || iterations > 8192)
+		return false;
+
+	/* RFC2315 Section 9.4 */
+	if (!(digest_alg = asn1_der_find_elem(mac, mac_len, 0, &tag,
+						&digest_alg_len)) ||
+			tag != ASN1_ID_SEQUENCE)
+		return false;
+
+	if (!(digest = asn1_der_find_elem(mac, mac_len, 1, &tag,
+						&digest_len)) ||
+			tag != ASN1_ID_OCTET_STRING)
+		return false;
+
+	if (!(alg_id = asn1_der_find_elem(digest_alg, digest_alg_len,
+						0, &tag, &alg_id_len)) ||
+			tag != ASN1_ID_OID)
+		return false;
+
+	/* This is going to be used for both the MAC and its key derivation */
+	for (i = 0; i < L_ARRAY_SIZE(pkcs12_mac_algs); i++)
+		if (asn1_oid_eq(&pkcs12_mac_algs[i].oid, alg_id_len, alg_id)) {
+			mac_hash = &pkcs12_mac_algs[i];
+			break;
+		}
+
+	if (i == L_ARRAY_SIZE(pkcs12_mac_algs) || digest_len != mac_hash->u)
+		return false;
+
+	if (!(key = cert_pkcs12_pbkdf(password, mac_hash,
+					mac_salt, mac_salt_len,
+					iterations, 3, mac_hash->u)))
+		return false;
+
+	hmac = l_checksum_new_hmac(mac_hash->alg, key, mac_hash->u);
+	explicit_bzero(key, mac_hash->u);
+
+	if (!hmac)
+		return false;
+
+	ok = l_checksum_update(hmac, auth_safe, auth_safe_len) &&
+		l_checksum_get_digest(hmac, hmac_val, mac_hash->len) > 0;
+	l_checksum_free(hmac);
+
+	if (!ok)
+		return false;
+
+	/*
+	 * SHA-512/224 and SHA-512/256 are not supported.  We can truncate the
+	 * output for key derivation but we can't do this inside the HMAC
+	 * algorithms based on these hashes.  We skip the MAC verification
+	 * if one of these hashes is used (identified by .u != .len)
+	 */
+	if (mac_hash->u != mac_hash->len)
+		goto integrity_check_done;
+
+	if (l_secure_memcmp(hmac_val, digest, digest_len))
+		return false;
+
+integrity_check_done:
+	if (!(auth_safe_seq = asn1_der_find_elem(auth_safe, auth_safe_len, 0,
+						&tag, &auth_safe_seq_len)) ||
+			tag != ASN1_ID_SEQUENCE ||
+			auth_safe + auth_safe_len !=
+			auth_safe_seq + auth_safe_seq_len)
+		return false;
+
+	i = 0;
+	while (1) {
+		struct asn1_oid data_oid;
+		const uint8_t *data;
+		size_t data_len;
+
+		if (!(data = cert_unpack_pkcs7_content_info(auth_safe_seq,
+							auth_safe_seq_len, i++,
+							NULL, &data_oid, &tag,
+							&data_len)))
+			return false;
+
+		if (asn1_oid_eq(&pkcs7_encrypted_data_oid,
+					data_oid.asn1_len, data_oid.asn1)) {
+			uint8_t *plaintext;
+			size_t plaintext_len;
+			struct asn1_oid oid;
+
+			if (tag != ASN1_ID_SEQUENCE)
+				return false;
+
+			/*
+			 * This is same as PKCS#7 encryptedData but the ciphers
+			 * used are from PKCS#12 (broken but still the default
+			 * everywhere) and PKCS#5 (recommended).
+			 */
+			plaintext = cert_decrypt_pkcs7_encrypted_data(data,
+								data_len,
+								password, &oid,
+								&plaintext_len);
+			if (!plaintext)
+				return false;
+
+			/*
+			 * Since we only support PKCS#7 data and encryptedData
+			 * types, and there's no point re-encrypting
+			 * encryptedData, the plaintext must be a PKCS#7
+			 * "data".
+			 */
+			ok = asn1_oid_eq(&pkcs7_data_oid,
+						oid.asn1_len, oid.asn1) &&
+				cert_parse_pkcs12_safe_contents(plaintext,
+								plaintext_len,
+								password,
+								out_certchain,
+								out_privkey);
+			explicit_bzero(plaintext, plaintext_len);
+			l_free(plaintext);
+
+			if (!ok)
+				return false;
+		} else if (asn1_oid_eq(&pkcs7_data_oid,
+					data_oid.asn1_len, data_oid.asn1)) {
+			if (tag != ASN1_ID_OCTET_STRING)
+				return false;
+
+			if (!cert_parse_pkcs12_safe_contents(data, data_len,
+								password,
+								out_certchain,
+								out_privkey))
+				return false;
+		}
+		/* envelopedData support not needed */
+
+		if (data + data_len == auth_safe_seq + auth_safe_seq_len)
+			return true;
+	}
+}
+
 /*
  * Look at a file, try to detect which of the few X.509 certificate and/or
  * private key container formats it uses and load any certificates in it as
@@ -750,10 +1329,16 @@ struct l_key *cert_key_from_pkcs1_rsa_private_key(const uint8_t *der,
  *  PEM PKCS#8 encrypted and unenecrypted private keys
  *  PEM legacy PKCS#1 encrypted and unenecrypted private keys
  *  Raw X.509 certificates (.cer, .der, .crt)
+ *  PKCS#12 certificates
+ *  PKCS#12 encrypted private keys
  *
  * The raw format contains exactly one certificate, PEM and PKCS#12 files
  * can contain any combination of certificates and private keys.
  *
+ * The password must have been validated as UTF-8 (use l_utf8_validate)
+ * unless the caller knows that no PKCS#12-defined encryption algorithm
+ * or MAC is used.
+ *
  * Returns false on "unrecoverable" errors, and *out_certchain,
  * *out_privkey and *out_encrypted (if provided) are not modified.  However
  * when true is returned, *out_certchain and *out_privkey (if provided) may
@@ -823,6 +1408,32 @@ LIB_EXPORT bool l_cert_load_container_file(const char *filename,
 
 			goto close;
 		}
+
+		if (tag == ASN1_ID_INTEGER) {
+			/*
+			 * Since we don't support public key-protected PKCS#12
+			 * modes, we always require the password at least for
+			 * the integrity check.  Strictly speaking encryption
+			 * may not actually be in use.  We also don't support
+			 * files with different integrity and privacy
+			 * passwords, they must be identical if privacy is
+			 * enabled.
+			 */
+			encrypted = true;
+
+			if (!password) {
+				error = !out_encrypted;
+				done = true;
+				goto close;
+			}
+
+			error = !cert_parse_pkcs12_pfx(seq_data, len, password,
+							out_certchain ?
+							&certchain : NULL,
+							out_privkey ?
+							&privkey : NULL);
+			goto close;
+		}
 	}
 
 not_der_after_all:
@@ -890,6 +1501,29 @@ not_der_after_all:
 			continue;
 		}
 
+		/* Cisco/gnutls-type PEM-encoded PKCS#12, probably rare */
+		if (L_IN_STRSET(type_label, "PKCS12")) {
+			encrypted = true;
+
+			if (!password) {
+				if (certchain && out_privkey) {
+					l_certchain_free(certchain);
+					certchain = NULL;
+				}
+
+				error = !out_encrypted;
+				done = true;
+				goto next;
+			}
+
+			error = !cert_parse_pkcs12_pfx(der, der_len, password,
+							out_certchain ?
+							&certchain : NULL,
+							out_privkey ?
+							&privkey : NULL);
+			goto next;
+		}
+
 next:
 		explicit_bzero(der, der_len);
 		l_free(der);
diff --git a/ell/pem.c b/ell/pem.c
index bc2865a..67d2af1 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -791,6 +791,10 @@ LIB_EXPORT struct l_key *l_pem_load_private_key_from_data(const void *buf,
  * success case and on error when NULL is returned.  This can be used to
  * check if a passphrase is required without prior information.
  *
+ * The passphrase, if given, must have been validated as UTF-8 unless the
+ * caller knows that PKCS#12 encryption algorithms are not used.
+ * Use l_utf8_validate.
+ *
  * Returns: An l_key object to be freed with an l_key_free* function,
  * or NULL.
  **/
-- 
2.27.0

Re: [PATCH 6/8] cert: Add PKCS#12 loading support

[Denis Kenzior]

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

Hi Andrew,

<snip>

> +/* RFC5652 Section 8 */
> +static uint8_t *cert_decrypt_pkcs7_encrypted_data(const uint8_t *data,
> +						size_t data_len,
> +						const char *password,
> +						struct asn1_oid *out_oid,
> +						size_t *out_len)
> +{
> +	const uint8_t *version;
> +	size_t version_len;
> +	const uint8_t *encrypted_info;
> +	size_t encrypted_info_len;
> +	const uint8_t *type;
> +	size_t type_len;
> +	const uint8_t *alg_id;
> +	size_t alg_id_len;
> +	const uint8_t *encrypted;
> +	size_t encrypted_len;
> +	uint8_t tag;
> +	struct l_cipher *alg;
> +	uint8_t *plaintext;
> +	int i;
> +	bool ok;
> +	bool is_block;
> +
> +	if (!(version = asn1_der_find_elem(data, data_len, 0, &tag,
> +						&version_len)) ||
> +			tag != ASN1_ID_INTEGER || version_len != 1 ||
> +			!L_IN_SET(version[0], 0, 2))
> +		return NULL;
> +
> +	if (!(encrypted_info = asn1_der_find_elem(data, data_len, 1, &tag,
> +							&encrypted_info_len)) ||
> +			tag != ASN1_ID_SEQUENCE)
> +		return NULL;
> +
> +	if (!(type = asn1_der_find_elem(encrypted_info, encrypted_info_len, 0,
> +					&tag, &type_len)) ||
> +			tag != ASN1_ID_OID ||
> +			type_len > sizeof(out_oid->asn1))
> +		return NULL;
> +
> +	if (!(alg_id = asn1_der_find_elem(encrypted_info, encrypted_info_len, 1,
> +					&tag, &alg_id_len)) ||
> +			tag != ASN1_ID_SEQUENCE)
> +		return NULL;
> +
> +	/* Not optional in our case, defined [0] IMPLICIT OCTET STRING */
> +	if (!(encrypted = asn1_der_find_elem(encrypted_info, encrypted_info_len,
> +						ASN1_CONTEXT_IMPLICIT(0),
> +						&tag, &encrypted_len)) ||
> +			tag != ASN1_ID(ASN1_CLASS_CONTEXT, 0, 0) ||
> +			encrypted_len < 8)
> +		return NULL;
> +
> +	if (!(alg = cert_cipher_from_pkcs_alg_id(alg_id, alg_id_len, password,
> +							&is_block)))
> +		return NULL;
> +
> +	plaintext = l_malloc(encrypted_len);
> +	ok = l_cipher_decrypt(alg, encrypted, plaintext, encrypted_len);
> +	l_cipher_free(alg);
> +
> +	if (!ok)
> +		return NULL;
> +

is plaintext leaked here?

> +	if (is_block) {
> +		bool ok = true;
> +
> +		/* Also validate the padding */
> +		if (encrypted_len < plaintext[encrypted_len - 1] ||
> +				plaintext[encrypted_len - 1] > 16)
> +			return NULL;
> +

same question as above?

> +		for (i = 1; i < plaintext[encrypted_len - 1]; i++)
> +			if (plaintext[encrypted_len - 1 - i] !=
> +					plaintext[encrypted_len - 1])
> +				ok = false;
> +
> +		if (!ok) {
> +			explicit_bzero(plaintext, encrypted_len);
> +			l_free(plaintext);
> +			return NULL;
> +		}
> +
> +		encrypted_len -= plaintext[encrypted_len - 1];
> +	}
> +
> +	if (out_oid) {
> +		out_oid->asn1_len = type_len;
> +		memcpy(out_oid->asn1, type, type_len);
> +	}
> +
> +	*out_len = encrypted_len;
> +	return plaintext;
> +}
> +

<snip>

> +static bool cert_parse_pkcs12_pfx(const uint8_t *ptr, size_t len,
> +					const char *password,
> +					struct l_certchain **out_certchain,
> +					struct l_key **out_privkey)
> +{
> +	const uint8_t *version;
> +	size_t version_len;
> +	const uint8_t *auth_safe;
> +	size_t auth_safe_len;
> +	const uint8_t *auth_safe_seq;
> +	size_t auth_safe_seq_len;
> +	const uint8_t *mac_data;
> +	size_t mac_data_len;
> +	const uint8_t *mac;
> +	size_t mac_len;
> +	const uint8_t *mac_salt;
> +	size_t mac_salt_len;
> +	const uint8_t *iterations_data;
> +	size_t iterations_len;
> +	unsigned int iterations;
> +	const uint8_t *digest_alg;
> +	size_t digest_alg_len;
> +	const uint8_t *digest;
> +	size_t digest_len;
> +	const uint8_t *alg_id;
> +	size_t alg_id_len;
> +	const struct cert_pkcs12_hash *mac_hash;
> +	L_AUTO_FREE_VAR(uint8_t *, key) = NULL;
> +	struct l_checksum *hmac;
> +	uint8_t hmac_val[64];
> +	uint8_t tag;
> +	unsigned int i;
> +	bool ok;

This function is unusually long.  It makes me wonder if breaking it up into 
multiple pieces would make it more readable...?

> +
> +	/* RFC7292 Section 4 */
> +	if (!(version = asn1_der_find_elem(ptr, len, 0, &tag, &version_len)) ||
> +			tag != ASN1_ID_INTEGER)
> +		return false;
> +
> +	if (version_len != 1 || version[0] != 3)
> +		return false;
> +
> +	/*
> +	 * Since we only support the password-based integrity mode,  the
> +	 * authSafe must be of PKCS#7 type "data" and not "signedData".
> +	 */
> +	if (!(auth_safe = cert_unpack_pkcs7_content_info(ptr, len, 1,
> +							&pkcs7_data_oid, NULL,
> +							&tag,
> +							&auth_safe_len)) ||
> +			tag != ASN1_ID_OCTET_STRING)
> +		return false;
> +
> +	/*
> +	 * openssl can generate PFX structures without macData not signed
> +	 * with a public key so handle this case, otherwise the macData
> +	 * would not be optional.
> +	 */
> +	if (auth_safe + auth_safe_len == ptr + len)
> +		goto integrity_check_done;
> +
> +	if (!(mac_data = asn1_der_find_elem(ptr, len, 2, &tag,
> +						&mac_data_len)) ||
> +			tag != ASN1_ID_SEQUENCE)
> +		return false;
> +
> +	if (!(mac = asn1_der_find_elem(mac_data, mac_data_len, 0, &tag,
> +					&mac_len)) ||
> +			tag != ASN1_ID_SEQUENCE)
> +		return false;
> +
> +	if (!(mac_salt = asn1_der_find_elem(mac_data, mac_data_len, 1, &tag,
> +						&mac_salt_len)) ||
> +			tag != ASN1_ID_OCTET_STRING || mac_salt_len > 1024)
> +		return false;
> +
> +	if (!(iterations_data = asn1_der_find_elem(mac_data, mac_data_len, 2,
> +							&tag,
> +							&iterations_len)) ||
> +			tag != ASN1_ID_INTEGER || iterations_len > 4)
> +		return false;
> +
> +	for (iterations = 0; iterations_len; iterations_len--)
> +		iterations = (iterations << 8) | *iterations_data++;
> +
> +	if (iterations < 1 || iterations > 8192)
> +		return false;
> +
> +	/* RFC2315 Section 9.4 */
> +	if (!(digest_alg = asn1_der_find_elem(mac, mac_len, 0, &tag,
> +						&digest_alg_len)) ||
> +			tag != ASN1_ID_SEQUENCE)
> +		return false;
> +
> +	if (!(digest = asn1_der_find_elem(mac, mac_len, 1, &tag,
> +						&digest_len)) ||
> +			tag != ASN1_ID_OCTET_STRING)
> +		return false;
> +
> +	if (!(alg_id = asn1_der_find_elem(digest_alg, digest_alg_len,
> +						0, &tag, &alg_id_len)) ||
> +			tag != ASN1_ID_OID)
> +		return false;
> +
> +	/* This is going to be used for both the MAC and its key derivation */
> +	for (i = 0; i < L_ARRAY_SIZE(pkcs12_mac_algs); i++)
> +		if (asn1_oid_eq(&pkcs12_mac_algs[i].oid, alg_id_len, alg_id)) {
> +			mac_hash = &pkcs12_mac_algs[i];
> +			break;
> +		}
> +
> +	if (i == L_ARRAY_SIZE(pkcs12_mac_algs) || digest_len != mac_hash->u)
> +		return false;
> +
> +	if (!(key = cert_pkcs12_pbkdf(password, mac_hash,
> +					mac_salt, mac_salt_len,
> +					iterations, 3, mac_hash->u)))
> +		return false;
> +
> +	hmac = l_checksum_new_hmac(mac_hash->alg, key, mac_hash->u);
> +	explicit_bzero(key, mac_hash->u);
> +
> +	if (!hmac)
> +		return false;
> +
> +	ok = l_checksum_update(hmac, auth_safe, auth_safe_len) &&
> +		l_checksum_get_digest(hmac, hmac_val, mac_hash->len) > 0;
> +	l_checksum_free(hmac);
> +
> +	if (!ok)
> +		return false;
> +
> +	/*
> +	 * SHA-512/224 and SHA-512/256 are not supported.  We can truncate the
> +	 * output for key derivation but we can't do this inside the HMAC
> +	 * algorithms based on these hashes.  We skip the MAC verification
> +	 * if one of these hashes is used (identified by .u != .len)
> +	 */
> +	if (mac_hash->u != mac_hash->len)
> +		goto integrity_check_done;
> +
> +	if (l_secure_memcmp(hmac_val, digest, digest_len))
> +		return false;
> +
> +integrity_check_done:
> +	if (!(auth_safe_seq = asn1_der_find_elem(auth_safe, auth_safe_len, 0,
> +						&tag, &auth_safe_seq_len)) ||
> +			tag != ASN1_ID_SEQUENCE ||
> +			auth_safe + auth_safe_len !=
> +			auth_safe_seq + auth_safe_seq_len)
> +		return false;
> +
> +	i = 0;
> +	while (1) {
> +		struct asn1_oid data_oid;
> +		const uint8_t *data;
> +		size_t data_len;
> +
> +		if (!(data = cert_unpack_pkcs7_content_info(auth_safe_seq,
> +							auth_safe_seq_len, i++,
> +							NULL, &data_oid, &tag,
> +							&data_len)))
> +			return false;
> +
> +		if (asn1_oid_eq(&pkcs7_encrypted_data_oid,
> +					data_oid.asn1_len, data_oid.asn1)) {
> +			uint8_t *plaintext;
> +			size_t plaintext_len;
> +			struct asn1_oid oid;
> +
> +			if (tag != ASN1_ID_SEQUENCE)
> +				return false;
> +
> +			/*
> +			 * This is same as PKCS#7 encryptedData but the ciphers
> +			 * used are from PKCS#12 (broken but still the default
> +			 * everywhere) and PKCS#5 (recommended).
> +			 */
> +			plaintext = cert_decrypt_pkcs7_encrypted_data(data,
> +								data_len,
> +								password, &oid,
> +								&plaintext_len);
> +			if (!plaintext)
> +				return false;
> +
> +			/*
> +			 * Since we only support PKCS#7 data and encryptedData
> +			 * types, and there's no point re-encrypting
> +			 * encryptedData, the plaintext must be a PKCS#7
> +			 * "data".
> +			 */
> +			ok = asn1_oid_eq(&pkcs7_data_oid,
> +						oid.asn1_len, oid.asn1) &&
> +				cert_parse_pkcs12_safe_contents(plaintext,
> +								plaintext_len,
> +								password,
> +								out_certchain,
> +								out_privkey);
> +			explicit_bzero(plaintext, plaintext_len);
> +			l_free(plaintext);
> +
> +			if (!ok)
> +				return false;
> +		} else if (asn1_oid_eq(&pkcs7_data_oid,
> +					data_oid.asn1_len, data_oid.asn1)) {
> +			if (tag != ASN1_ID_OCTET_STRING)
> +				return false;
> +
> +			if (!cert_parse_pkcs12_safe_contents(data, data_len,
> +								password,
> +								out_certchain,
> +								out_privkey))

While this function is not public API, it still feels like side-effecting the 
out parameters here is something that makes the logic harder to follow...  Are 
you sure you don't want to clean these up on failure?

> +				return false;
> +		}
> +		/* envelopedData support not needed */
> +
> +		if (data + data_len == auth_safe_seq + auth_safe_seq_len)
> +			return true;
> +	}
> +}
> +
>   /*
>    * Look at a file, try to detect which of the few X.509 certificate and/or
>    * private key container formats it uses and load any certificates in it as

<snip>

Regards,
-Denis

Re: [PATCH 6/8] cert: Add PKCS#12 loading support

[Andrew Zaborowski]

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

On Fri, 8 Jan 2021 at 05:59, Denis Kenzior <denkenz@gmail.com> wrote:
> > +     plaintext = l_malloc(encrypted_len);
> > +     ok = l_cipher_decrypt(alg, encrypted, plaintext, encrypted_len);
> > +     l_cipher_free(alg);
> > +
> > +     if (!ok)
> > +             return NULL;
> > +
>
> is plaintext leaked here?

l_cipher_decrypt has failed so the read() either failed or didn't happen.

>
> > +     if (is_block) {
> > +             bool ok = true;
> > +
> > +             /* Also validate the padding */
> > +             if (encrypted_len < plaintext[encrypted_len - 1] ||
> > +                             plaintext[encrypted_len - 1] > 16)
> > +                     return NULL;
> > +
>
> same question as above?

I think it is here, will fix.

>
> > +             for (i = 1; i < plaintext[encrypted_len - 1]; i++)
> > +                     if (plaintext[encrypted_len - 1 - i] !=
> > +                                     plaintext[encrypted_len - 1])
> > +                             ok = false;
> > +
> > +             if (!ok) {
> > +                     explicit_bzero(plaintext, encrypted_len);
> > +                     l_free(plaintext);
> > +                     return NULL;
> > +             }
> > +
> > +             encrypted_len -= plaintext[encrypted_len - 1];
> > +     }
> > +
> > +     if (out_oid) {
> > +             out_oid->asn1_len = type_len;
> > +             memcpy(out_oid->asn1, type, type_len);
> > +     }
> > +
> > +     *out_len = encrypted_len;
> > +     return plaintext;
> > +}
> > +
>
> <snip>
>
> > +static bool cert_parse_pkcs12_pfx(const uint8_t *ptr, size_t len,
> > +                                     const char *password,
> > +                                     struct l_certchain **out_certchain,
> > +                                     struct l_key **out_privkey)
> > +{
> > +     const uint8_t *version;
> > +     size_t version_len;
> > +     const uint8_t *auth_safe;
> > +     size_t auth_safe_len;
> > +     const uint8_t *auth_safe_seq;
> > +     size_t auth_safe_seq_len;
> > +     const uint8_t *mac_data;
> > +     size_t mac_data_len;
> > +     const uint8_t *mac;
> > +     size_t mac_len;
> > +     const uint8_t *mac_salt;
> > +     size_t mac_salt_len;
> > +     const uint8_t *iterations_data;
> > +     size_t iterations_len;
> > +     unsigned int iterations;
> > +     const uint8_t *digest_alg;
> > +     size_t digest_alg_len;
> > +     const uint8_t *digest;
> > +     size_t digest_len;
> > +     const uint8_t *alg_id;
> > +     size_t alg_id_len;
> > +     const struct cert_pkcs12_hash *mac_hash;
> > +     L_AUTO_FREE_VAR(uint8_t *, key) = NULL;
> > +     struct l_checksum *hmac;
> > +     uint8_t hmac_val[64];
> > +     uint8_t tag;
> > +     unsigned int i;
> > +     bool ok;
>
> This function is unusually long.  It makes me wonder if breaking it up into
> multiple pieces would make it more readable...?

Ok.

>
> > +
> > +     /* RFC7292 Section 4 */
> > +     if (!(version = asn1_der_find_elem(ptr, len, 0, &tag, &version_len)) ||
> > +                     tag != ASN1_ID_INTEGER)
> > +             return false;
> > +
> > +     if (version_len != 1 || version[0] != 3)
> > +             return false;
> > +
> > +     /*
> > +      * Since we only support the password-based integrity mode,  the
> > +      * authSafe must be of PKCS#7 type "data" and not "signedData".
> > +      */
> > +     if (!(auth_safe = cert_unpack_pkcs7_content_info(ptr, len, 1,
> > +                                                     &pkcs7_data_oid, NULL,
> > +                                                     &tag,
> > +                                                     &auth_safe_len)) ||
> > +                     tag != ASN1_ID_OCTET_STRING)
> > +             return false;
> > +
> > +     /*
> > +      * openssl can generate PFX structures without macData not signed
> > +      * with a public key so handle this case, otherwise the macData
> > +      * would not be optional.
> > +      */
> > +     if (auth_safe + auth_safe_len == ptr + len)
> > +             goto integrity_check_done;
> > +
> > +     if (!(mac_data = asn1_der_find_elem(ptr, len, 2, &tag,
> > +                                             &mac_data_len)) ||
> > +                     tag != ASN1_ID_SEQUENCE)
> > +             return false;
> > +
> > +     if (!(mac = asn1_der_find_elem(mac_data, mac_data_len, 0, &tag,
> > +                                     &mac_len)) ||
> > +                     tag != ASN1_ID_SEQUENCE)
> > +             return false;
> > +
> > +     if (!(mac_salt = asn1_der_find_elem(mac_data, mac_data_len, 1, &tag,
> > +                                             &mac_salt_len)) ||
> > +                     tag != ASN1_ID_OCTET_STRING || mac_salt_len > 1024)
> > +             return false;
> > +
> > +     if (!(iterations_data = asn1_der_find_elem(mac_data, mac_data_len, 2,
> > +                                                     &tag,
> > +                                                     &iterations_len)) ||
> > +                     tag != ASN1_ID_INTEGER || iterations_len > 4)
> > +             return false;
> > +
> > +     for (iterations = 0; iterations_len; iterations_len--)
> > +             iterations = (iterations << 8) | *iterations_data++;
> > +
> > +     if (iterations < 1 || iterations > 8192)
> > +             return false;
> > +
> > +     /* RFC2315 Section 9.4 */
> > +     if (!(digest_alg = asn1_der_find_elem(mac, mac_len, 0, &tag,
> > +                                             &digest_alg_len)) ||
> > +                     tag != ASN1_ID_SEQUENCE)
> > +             return false;
> > +
> > +     if (!(digest = asn1_der_find_elem(mac, mac_len, 1, &tag,
> > +                                             &digest_len)) ||
> > +                     tag != ASN1_ID_OCTET_STRING)
> > +             return false;
> > +
> > +     if (!(alg_id = asn1_der_find_elem(digest_alg, digest_alg_len,
> > +                                             0, &tag, &alg_id_len)) ||
> > +                     tag != ASN1_ID_OID)
> > +             return false;
> > +
> > +     /* This is going to be used for both the MAC and its key derivation */
> > +     for (i = 0; i < L_ARRAY_SIZE(pkcs12_mac_algs); i++)
> > +             if (asn1_oid_eq(&pkcs12_mac_algs[i].oid, alg_id_len, alg_id)) {
> > +                     mac_hash = &pkcs12_mac_algs[i];
> > +                     break;
> > +             }
> > +
> > +     if (i == L_ARRAY_SIZE(pkcs12_mac_algs) || digest_len != mac_hash->u)
> > +             return false;
> > +
> > +     if (!(key = cert_pkcs12_pbkdf(password, mac_hash,
> > +                                     mac_salt, mac_salt_len,
> > +                                     iterations, 3, mac_hash->u)))
> > +             return false;
> > +
> > +     hmac = l_checksum_new_hmac(mac_hash->alg, key, mac_hash->u);
> > +     explicit_bzero(key, mac_hash->u);
> > +
> > +     if (!hmac)
> > +             return false;
> > +
> > +     ok = l_checksum_update(hmac, auth_safe, auth_safe_len) &&
> > +             l_checksum_get_digest(hmac, hmac_val, mac_hash->len) > 0;
> > +     l_checksum_free(hmac);
> > +
> > +     if (!ok)
> > +             return false;
> > +
> > +     /*
> > +      * SHA-512/224 and SHA-512/256 are not supported.  We can truncate the
> > +      * output for key derivation but we can't do this inside the HMAC
> > +      * algorithms based on these hashes.  We skip the MAC verification
> > +      * if one of these hashes is used (identified by .u != .len)
> > +      */
> > +     if (mac_hash->u != mac_hash->len)
> > +             goto integrity_check_done;
> > +
> > +     if (l_secure_memcmp(hmac_val, digest, digest_len))
> > +             return false;
> > +
> > +integrity_check_done:
> > +     if (!(auth_safe_seq = asn1_der_find_elem(auth_safe, auth_safe_len, 0,
> > +                                             &tag, &auth_safe_seq_len)) ||
> > +                     tag != ASN1_ID_SEQUENCE ||
> > +                     auth_safe + auth_safe_len !=
> > +                     auth_safe_seq + auth_safe_seq_len)
> > +             return false;
> > +
> > +     i = 0;
> > +     while (1) {
> > +             struct asn1_oid data_oid;
> > +             const uint8_t *data;
> > +             size_t data_len;
> > +
> > +             if (!(data = cert_unpack_pkcs7_content_info(auth_safe_seq,
> > +                                                     auth_safe_seq_len, i++,
> > +                                                     NULL, &data_oid, &tag,
> > +                                                     &data_len)))
> > +                     return false;
> > +
> > +             if (asn1_oid_eq(&pkcs7_encrypted_data_oid,
> > +                                     data_oid.asn1_len, data_oid.asn1)) {
> > +                     uint8_t *plaintext;
> > +                     size_t plaintext_len;
> > +                     struct asn1_oid oid;
> > +
> > +                     if (tag != ASN1_ID_SEQUENCE)
> > +                             return false;
> > +
> > +                     /*
> > +                      * This is same as PKCS#7 encryptedData but the ciphers
> > +                      * used are from PKCS#12 (broken but still the default
> > +                      * everywhere) and PKCS#5 (recommended).
> > +                      */
> > +                     plaintext = cert_decrypt_pkcs7_encrypted_data(data,
> > +                                                             data_len,
> > +                                                             password, &oid,
> > +                                                             &plaintext_len);
> > +                     if (!plaintext)
> > +                             return false;
> > +
> > +                     /*
> > +                      * Since we only support PKCS#7 data and encryptedData
> > +                      * types, and there's no point re-encrypting
> > +                      * encryptedData, the plaintext must be a PKCS#7
> > +                      * "data".
> > +                      */
> > +                     ok = asn1_oid_eq(&pkcs7_data_oid,
> > +                                             oid.asn1_len, oid.asn1) &&
> > +                             cert_parse_pkcs12_safe_contents(plaintext,
> > +                                                             plaintext_len,
> > +                                                             password,
> > +                                                             out_certchain,
> > +                                                             out_privkey);
> > +                     explicit_bzero(plaintext, plaintext_len);
> > +                     l_free(plaintext);
> > +
> > +                     if (!ok)
> > +                             return false;
> > +             } else if (asn1_oid_eq(&pkcs7_data_oid,
> > +                                     data_oid.asn1_len, data_oid.asn1)) {
> > +                     if (tag != ASN1_ID_OCTET_STRING)
> > +                             return false;
> > +
> > +                     if (!cert_parse_pkcs12_safe_contents(data, data_len,
> > +                                                             password,
> > +                                                             out_certchain,
> > +                                                             out_privkey))
>
> While this function is not public API, it still feels like side-effecting the
> out parameters here is something that makes the logic harder to follow...

I changed l_cert_load_container_file to write
out_certchain/out_privkey at the end of the function (so that I didn't
have to do it here), but I can do the same thing here and inside
cert_parse_pkcs12_safe_contents for consistency.

> Are
> you sure you don't want to clean these up on failure?

In this version they were being cleaned up in l_cert_load_container_file.

Best regards

Re: [PATCH 6/8] cert: Add PKCS#12 loading support

[Denis Kenzior]

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

Hi Andrew,

On 1/8/21 7:04 PM, Andrew Zaborowski wrote:
> On Fri, 8 Jan 2021 at 05:59, Denis Kenzior <denkenz@gmail.com> wrote:
>>> +     plaintext = l_malloc(encrypted_len);
>>> +     ok = l_cipher_decrypt(alg, encrypted, plaintext, encrypted_len);
>>> +     l_cipher_free(alg);
>>> +
>>> +     if (!ok)
>>> +             return NULL;
>>> +
>>
>> is plaintext leaked here?
> 
> l_cipher_decrypt has failed so the read() either failed or didn't happen.
> 

By my reading you l_malloc plaintext and never free it, no?

Regards,
-Denis

Re: [PATCH 6/8] cert: Add PKCS#12 loading support

[Andrew Zaborowski]

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

On Sat, 9 Jan 2021 at 03:30, Denis Kenzior <denkenz@gmail.com> wrote:
> On 1/8/21 7:04 PM, Andrew Zaborowski wrote:
> > On Fri, 8 Jan 2021 at 05:59, Denis Kenzior <denkenz@gmail.com> wrote:
> >>> +     plaintext = l_malloc(encrypted_len);
> >>> +     ok = l_cipher_decrypt(alg, encrypted, plaintext, encrypted_len);
> >>> +     l_cipher_free(alg);
> >>> +
> >>> +     if (!ok)
> >>> +             return NULL;
> >>> +
> >>
> >> is plaintext leaked here?
> >
> > l_cipher_decrypt has failed so the read() either failed or didn't happen.
> >
>
> By my reading you l_malloc plaintext and never free it, no?

Oops, yes, I missed your point and didn't see it in the code either.

Best regards

[PATCH 7/8] unit: Update tests after l_pkcs5_* renaming

[Andrew Zaborowski]

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

---
 unit/test-pbkdf2.c | 6 +++---
 unit/test-tls.c    | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/unit/test-pbkdf2.c b/unit/test-pbkdf2.c
index 37d87ed..3f272ce 100644
--- a/unit/test-pbkdf2.c
+++ b/unit/test-pbkdf2.c
@@ -51,9 +51,9 @@ static void pbkdf2_test(const void *data)
 
 	key_len = test->key_len ? : (strlen(test->key) / 2);
 
-	result = l_pkcs5_pbkdf2(L_CHECKSUM_SHA1, test->password,
-				(const uint8_t *) test->salt, salt_len,
-				test->count, output, key_len);
+	result = l_cert_pkcs5_pbkdf2(L_CHECKSUM_SHA1, test->password,
+					(const uint8_t *) test->salt, salt_len,
+					test->count, output, key_len);
 
 	assert(result == true);
 
diff --git a/unit/test-tls.c b/unit/test-tls.c
index 9503f6b..7937962 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -31,6 +31,7 @@
 #include <ell/ell.h>
 
 #include "ell/tls-private.h"
+#include "ell/asn1-private.h"
 #include "ell/cert-private.h"
 
 static void test_tls10_prf(const void *data)
-- 
2.27.0

[PATCH 8/8] unit: Add l_cert_load_container_file tests

[Andrew Zaborowski]

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

Most of the tests are abour PEM files so place them together in
test-pem.ca.
---
 unit/test-pem.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/unit/test-pem.c b/unit/test-pem.c
index f875adc..64dfb81 100644
--- a/unit/test-pem.c
+++ b/unit/test-pem.c
@@ -337,6 +337,66 @@ static void test_encrypted_pkey(const void *data)
 	l_key_free(pkey2);
 }
 
+static bool test_cert_count(struct l_cert *cert, void *user_data)
+{
+	int *count = user_data;
+
+	(*count)++;
+	return false;
+}
+
+struct test_load_file_params {
+	const char *path;
+	bool expect_cert;
+	bool expect_certchain;
+	bool expect_privkey;
+	bool expect_encrypted;
+};
+
+#define TEST_LOAD_PARAMS(fn, cert, certchain, privkey, encrypted)	\
+	(&(struct test_load_file_params) {				\
+		CERTDIR fn, (cert), (certchain), (privkey), (encrypted) })
+
+static void test_load_file(const void *data)
+{
+	const struct test_load_file_params *params = data;
+	struct l_certchain *certchain;
+	struct l_key *privkey;
+	bool encrypted;
+
+	assert(l_cert_load_container_file(params->path, NULL, &certchain,
+						&privkey, &encrypted));
+	assert(encrypted == params->expect_encrypted);
+
+	if (encrypted) {
+		assert(!certchain && !privkey);
+
+		assert(l_cert_load_container_file(params->path, "abc",
+							&certchain, &privkey,
+							&encrypted));
+		assert(encrypted);
+	}
+
+	assert(!!certchain == params->expect_cert);
+	assert(!!privkey == params->expect_privkey);
+
+	if (certchain) {
+		int count = 0;
+
+		l_certchain_walk_from_leaf(certchain, test_cert_count, &count);
+		assert(count == (params->expect_certchain ? 3 : 1));
+
+		if (params->expect_certchain)
+			assert(l_certchain_verify(certchain, NULL, NULL));
+	}
+
+	if (certchain)
+		l_certchain_free(certchain);
+
+	if (privkey)
+		l_key_free(privkey);
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -409,6 +469,56 @@ int main(int argc, char *argv[])
 				CERTDIR "cert-client-key-pkcs1-aes256.pem");
 	}
 
+	l_test_add("detect-format/PEM PKCS#1 unencrypted private key",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-client-key-pkcs1.pem",
+						false, false, true, false));
+	l_test_add("detect-format/PEM PKCS#1 encrypted private key",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-client-key-pkcs1-des.pem",
+						false, false, true, true));
+	l_test_add("detect-format/PEM PKCS#8 unencrypted private key",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-client-key-pkcs8.pem",
+						false, false, true, false));
+	l_test_add("detect-format/PEM PKCS#8 encrypted private key",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-client-key-pkcs8-sha1-des.pem",
+						false, false, true, true));
+	l_test_add("detect-format/PEM X.509 certificate",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-client.pem",
+						true, false, false, false));
+	l_test_add("detect-format/DER X.509 certificate",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-client.crt",
+						true, false, false, false));
+	l_test_add("detect-format/PEM combined",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-entity-combined.pem",
+						true, true, true, true));
+	l_test_add("detect-format/DER PKCS#12 combined",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-entity-pkcs12-nomac.p12",
+						true, false, true, true));
+
+	l_test_add("pkcs#12/Combined RC2-based ciphers + SHA1",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-entity-pkcs12-rc2-sha1.p12",
+						true, true, true, true));
+	l_test_add("pkcs#12/Combined DES-based ciphers + SHA256",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-entity-pkcs12-des-sha256.p12",
+						true, true, true, true));
+	l_test_add("pkcs#12/Combined RC4-based ciphers + SHA384",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-entity-pkcs12-rc4-sha384.p12",
+						true, true, true, true));
+	l_test_add("pkcs#12/Combined PKCS#5 ciphers + SHA512",
+			test_load_file,
+			TEST_LOAD_PARAMS("cert-entity-pkcs12-pkcs5-sha512.p12",
+						true, true, true, true));
+
 done:
 	return l_test_run();
 }
-- 
2.27.0

Re: [PATCH 1/8] util: Add L_IN_SET macros

[Denis Kenzior]

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

Hi Andrew,

On 1/6/21 1:54 PM, Andrew Zaborowski wrote:
> This can be used with enum types or ints as in L_IN_SET(-err, EIO, EINVAL).
> I'm also adding an L_IN_STRSET() for string comparisons.  This
> macro is not equivalent to (x == (val1) || x == (val2) || ...)
> because val2 will be evaluated even if (x == (val1)) was true.  There
> seems to be no way to avoid that without adding another numbr of macros
> dependent on the maximum number of arguments expected.  The "x" in the
> above example is evaluated once.
> 
> We may have to add a warning about everything being cast to the type of
> the "x", I'm not sure how important of a problem this is.
> ---
>   ell/util.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 

Patches 1-4 and 7 applied thanks.

Regards,
-Denis