PKCS7: decoder full stack support

Signed-off-by: Stephan Mueller <smueller@chronox.de>

Author: smuellerDD

TODO

@@ -15,8 +15,6 @@
 
 - ARMv8: enable ARMv8 intrinsics in the kernel (SHAKE2x and by extension SLH-DSA)
 
-- PKCS7 parser: eliminate the malloc for X509 cert data structures and signer info data structures (this is, however, a challenge as we have an unknown amount of them)
-
 - lc_x509_helper: remove duplication between asn1/tests/asn1_test_helper.c and apps/src/lc_x509_generator_file_helper.c
 
 - RISCV64: update internal/src/cpufeatures_riscv_auxv.c once kernels report ZBB correctly

apps/src/lc_pkcs7_generator.c

@@ -164,41 +164,41 @@ static int pkcs7_gen_file(struct pkcs7_generator_opts *opts,
 static int pkcs7_enc_dump(struct pkcs7_generator_opts *opts,
 			  const uint8_t *pkcs7_data, size_t pkcs7_datalen)
 {
-	struct lc_pkcs7_message ppkcs7 = { 0 };
+	LC_PKCS7_MSG_ON_STACK(ppkcs7, 1, 4);
 	int ret;
 
 	if (!opts->print_pkcs7 && !opts->checker)
 		return 0;
 
-	CKINT(lc_pkcs7_message_parse(&ppkcs7, pkcs7_data, pkcs7_datalen));
+	CKINT(lc_pkcs7_message_parse(ppkcs7, pkcs7_data, pkcs7_datalen));
 
 	if (opts->data) {
-		CKINT(lc_pkcs7_set_data(&ppkcs7, opts->data, opts->datalen, 0));
+		CKINT(lc_pkcs7_set_data(ppkcs7, opts->data, opts->datalen, 0));
 	}
 
 	CKINT_LOG(lc_pkcs7_verify(
-			  &ppkcs7,
+			  ppkcs7,
 			  opts->use_trust_store ? &opts->trust_store : NULL,
 			  opts->verify_rules_set ? &opts->verify_rules : NULL),
 		  "Verification of PKCS#7 message failed\n");
 
 	if (opts->checker)
-		CKINT(apply_checks_pkcs7(&ppkcs7, &opts->checker_opts));
+		CKINT(apply_checks_pkcs7(ppkcs7, &opts->checker_opts));
 
 	if (opts->print_pkcs7) {
-		CKINT(print_pkcs7_data(&ppkcs7));
+		CKINT(print_pkcs7_data(ppkcs7));
 	}
 
 out:
-	lc_pkcs7_message_clear(&ppkcs7);
+	lc_pkcs7_message_clear(ppkcs7);
 	return ret;
 }
 
 static int pkcs7_dump_file(struct pkcs7_generator_opts *opts)
 {
-	struct lc_pkcs7_message ppkcs7 = { 0 };
 	uint8_t *pkcs7_data = NULL;
 	size_t pkcs7_datalen = 0;
+	LC_PKCS7_MSG_ON_STACK(ppkcs7, 1, 4);
 	int ret;
 
 	if (!opts->pkcs7_msg && !opts->checker)
@@ -207,13 +207,13 @@ static int pkcs7_dump_file(struct pkcs7_generator_opts *opts)
 	CKINT_LOG(get_data(opts->pkcs7_msg, &pkcs7_data, &pkcs7_datalen),
 		  "Loading of file %s failed\n", opts->pkcs7_msg);
 
-	CKINT_LOG(lc_pkcs7_message_parse(&ppkcs7, pkcs7_data, pkcs7_datalen),
+	CKINT_LOG(lc_pkcs7_message_parse(ppkcs7, pkcs7_data, pkcs7_datalen),
 		  "Parsing of input file %s failed\n", opts->pkcs7_msg);
 
 	if (opts->data) {
-		CKINT(lc_pkcs7_set_data(&ppkcs7, opts->data, opts->datalen, 0));
+		CKINT(lc_pkcs7_set_data(ppkcs7, opts->data, opts->datalen, 0));
 		CKINT_LOG(lc_pkcs7_verify(
-				  &ppkcs7,
+				  ppkcs7,
 				  opts->use_trust_store ? &opts->trust_store :
 							  NULL,
 				  opts->verify_rules_set ? &opts->verify_rules :
@@ -222,13 +222,13 @@ static int pkcs7_dump_file(struct pkcs7_generator_opts *opts)
 	}
 
 	if (opts->checker)
-		CKINT(apply_checks_pkcs7(&ppkcs7, &opts->checker_opts));
+		CKINT(apply_checks_pkcs7(ppkcs7, &opts->checker_opts));
 
 	if (opts->print_pkcs7)
-		CKINT(print_pkcs7_data(&ppkcs7));
+		CKINT(print_pkcs7_data(ppkcs7));
 
 out:
-	lc_pkcs7_message_clear(&ppkcs7);
+	lc_pkcs7_message_clear(ppkcs7);
 	release_data(pkcs7_data, pkcs7_datalen);
 	return ret;
 }
@@ -549,7 +549,7 @@ int main(int argc, char *argv[])
 	struct pkcs7_generator_opts parsed_opts = { 0 };
 	struct x509_checker_options *checker_opts = &parsed_opts.checker_opts;
 	struct lc_verify_rules *verify_rules = &parsed_opts.verify_rules;
-	LC_PKCS7_MSG_ON_STACK(pkcs7_msg, 2);
+	LC_PKCS7_MSG_ON_STACK(pkcs7_msg, 1, 4);
 	int ret = 0, opt_index = 0;
 
 	static const char *opts_short = "ho:i:";

apps/src/lc_x509_generator_print.c

@@ -271,7 +271,7 @@ int print_x509_cert(const struct lc_x509_certificate *x509)
 int print_pkcs7_data(const struct lc_pkcs7_message *pkcs7_msg)
 {
 	struct lc_x509_certificate *cert = pkcs7_msg->certs;
-	struct lc_pkcs7_signed_info *sinfos = pkcs7_msg->list_head_signed_infos;
+	struct lc_pkcs7_signed_info *sinfos = pkcs7_msg->list_head_sinfo;
 	const char *hash_name;
 	int ret = 0;
 

asn1/api/lc_pkcs7_common.h

@@ -87,27 +87,32 @@ struct lc_pkcs7_message {
 	/*
 	 * Signed information
 	 */
-	struct lc_pkcs7_signed_info *curr_signed_infos;
-	struct lc_pkcs7_signed_info *list_head_signed_infos;
-	struct lc_pkcs7_signed_info **list_tail_signed_infos;
+	struct lc_pkcs7_signed_info *curr_sinfo;
+	struct lc_pkcs7_signed_info *list_head_sinfo;
+	struct lc_pkcs7_signed_info **list_tail_sinfo;
+	uint8_t avail_preallocated_sinfo;
+	uint8_t consumed_preallocated_sinfo;
+	struct lc_pkcs7_signed_info *preallocated_sinfo;
+
 	uint8_t version; /* Version of cert (1 -> PKCS#7 or CMS; 3 -> CMS) */
 
 	/* Content Data (or NULL) */
 	enum OID data_type; /* Type of Data */
 	size_t data_len; /* Length of Data */
 	const uint8_t *data; /* Content Data (or 0) */
 
-	uint8_t avail_preallocated_sinfo;
-	uint8_t consumed_preallocated_sinfo;
-	struct lc_pkcs7_signed_info *preallocated_sinfo;
+	uint8_t avail_preallocated_x509;
+	uint8_t consumed_preallocated_x509;
+	struct lc_x509_certificate *preallocated_x509;
 
 	unsigned int have_authattrs : 1; /* T if have authattrs */
 	unsigned int embed_data : 1; /* Embed data into message */
 };
 
-#define LC_PKCS7_MSG_SIZE(num_sinfo)                                           \
+#define LC_PKCS7_MSG_SIZE(num_sinfo, num_x509)                                 \
 	sizeof(struct lc_pkcs7_message) +                                      \
-	num_sinfo * sizeof(struct lc_pkcs7_signed_info)
+	num_sinfo * sizeof(struct lc_pkcs7_signed_info) +                      \
+	num_x509 * sizeof(struct lc_x509_certificate)
 
 /// \endcond
 
@@ -117,22 +122,39 @@ struct lc_pkcs7_message {
  * @brief Allocate memory for struct lc_pkcs7_message holding given number of
  *	  preallocated sinfo members
  *
+ * This allocation allows the PKCS7 parsing to avoid allocate memory and keep
+ * all operations on stack. In case more signers than \p num_sinfo or more
+ * X.509 certificates than \p num_x509 are parsed from the PKCS7 message,
+ * then first all pre-allocated structures are used and then new ones are
+ * allocated.
+ *
+ * When not using this macro, which is perfectly legal, an simply allocating
+ * \p struct lc_pkcs7_message on stack, then for all parsed signers and
+ * X.509 certificates, a new memory entry is allocated.
+ *
  * @param [in] name Name of stack variable
  * @param [in] num_sinfo Number of preallocated sinfo members (must be less than
  *			 256)
+ * @param [in] num_x509 Number of preallocated X.509 certificate structures
+ *			(must be less than 256)
  */
-#define LC_PKCS7_MSG_ON_STACK(name, num_sinfo)                                 \
+#define LC_PKCS7_MSG_ON_STACK(name, num_sinfo, num_x509)                       \
 	_Pragma("GCC diagnostic push") _Pragma(                                \
 		"GCC diagnostic ignored \"-Wdeclaration-after-statement\"")    \
 		_Pragma("GCC diagnostic ignored \"-Wcast-align\"")             \
-		LC_ALIGNED_BUFFER(name##_ctx_buf, LC_PKCS7_MSG_SIZE(num_sinfo),\
-				  8);                                          \
+		LC_ALIGNED_BUFFER(name##_ctx_buf,                              \
+				  LC_PKCS7_MSG_SIZE(num_sinfo, num_x509), 8);  \
 	struct lc_pkcs7_message *name =                                        \
 		(struct lc_pkcs7_message *)name##_ctx_buf;                     \
 	(name)->avail_preallocated_sinfo = num_sinfo;                          \
 	(name)->preallocated_sinfo =                                           \
 		(struct lc_pkcs7_signed_info *)((uint8_t *)(name) +            \
 		 sizeof(struct lc_pkcs7_message));                             \
+	(name)->avail_preallocated_x509 = num_x509;                            \
+	(name)->preallocated_x509 =                                            \
+		(struct lc_x509_certificate *)((uint8_t *)(name) +             \
+		 sizeof(struct lc_pkcs7_message) +                             \
+		 num_sinfo * sizeof(struct lc_pkcs7_signed_info));             \
 	_Pragma("GCC diagnostic pop")
 
 #ifdef __cplusplus

asn1/api/lc_x509_common.h

@@ -303,6 +303,7 @@ struct lc_x509_certificate {
 	unsigned int
 		unsupported_sig : 1; /* T if signature uses unsupported crypto */
 	unsigned int blacklisted : 1;
+	unsigned int preallocated : 1;
 };
 
 /// \endcond

asn1/src/pkcs7_generator.c

@@ -1050,13 +1050,13 @@ static inline int pkcs7_initialize_ctx(struct pkcs7_generate_context *ctx,
 	int ret = 0;
 
 	CKNULL(pkcs7->certs, -EINVAL);
-	CKNULL(pkcs7->list_head_signed_infos, -EINVAL);
+	CKNULL(pkcs7->list_head_sinfo, -EINVAL);
 
 	ctx->pkcs7 = pkcs7;
 	ctx->current_x509 = pkcs7->certs;
-	ctx->current_sinfo = pkcs7->list_head_signed_infos;
+	ctx->current_sinfo = pkcs7->list_head_sinfo;
 
-	for (sinfo = pkcs7->list_head_signed_infos; sinfo;
+	for (sinfo = pkcs7->list_head_sinfo; sinfo;
 	     sinfo = sinfo->next) {
 		const struct lc_hash *hash;
 

asn1/src/pkcs7_internal.h

@@ -59,6 +59,10 @@ int pkcs7_sinfo_get(struct lc_pkcs7_signed_info **sinfo,
 		    struct lc_pkcs7_message *pkcs7);
 void pkcs7_sinfo_free(struct lc_pkcs7_message *pkcs7);
 
+int pkcs7_x509_get(struct lc_x509_certificate **x509,
+		   struct lc_pkcs7_message *pkcs7);
+void pkcs7_x509_free(struct lc_x509_certificate *x509);
+
 #ifdef __cplusplus
 }
 #endif

asn1/src/pkcs7_memory.c

@@ -26,9 +26,9 @@ void pkcs7_sinfo_free(struct lc_pkcs7_message *pkcs7)
 	struct lc_pkcs7_signed_info *sinfo;
 	uint8_t idx = 0;
 
-	while (pkcs7->list_head_signed_infos) {
-		sinfo = pkcs7->list_head_signed_infos;
-		pkcs7->list_head_signed_infos = sinfo->next;
+	while (pkcs7->list_head_sinfo) {
+		sinfo = pkcs7->list_head_sinfo;
+		pkcs7->list_head_sinfo = sinfo->next;
 		public_key_signature_clear(&sinfo->sig);
 		if (idx < pkcs7->consumed_preallocated_sinfo) {
 			idx++;
@@ -37,8 +37,8 @@ void pkcs7_sinfo_free(struct lc_pkcs7_message *pkcs7)
 		}
 	}
 
-	if (pkcs7->curr_signed_infos) {
-		sinfo = pkcs7->curr_signed_infos;
+	if (pkcs7->curr_sinfo) {
+		sinfo = pkcs7->curr_sinfo;
 		public_key_signature_clear(&sinfo->sig);
 
 		if (idx >= pkcs7->consumed_preallocated_sinfo)
@@ -48,14 +48,14 @@ void pkcs7_sinfo_free(struct lc_pkcs7_message *pkcs7)
 
 int pkcs7_sinfo_add(struct lc_pkcs7_message *pkcs7)
 {
-	if (!pkcs7->list_head_signed_infos) {
-		pkcs7->list_head_signed_infos = pkcs7->curr_signed_infos;
+	if (!pkcs7->list_head_sinfo) {
+		pkcs7->list_head_sinfo = pkcs7->curr_sinfo;
 	} else {
-		*pkcs7->list_tail_signed_infos = pkcs7->curr_signed_infos;
+		*pkcs7->list_tail_sinfo = pkcs7->curr_sinfo;
 	}
 
-	pkcs7->list_tail_signed_infos = &pkcs7->curr_signed_infos->next;
-	pkcs7->curr_signed_infos = NULL;
+	pkcs7->list_tail_sinfo = &pkcs7->curr_sinfo->next;
+	pkcs7->curr_sinfo = NULL;
 
 	return 0;
 }
@@ -67,20 +67,55 @@ int pkcs7_sinfo_get(struct lc_pkcs7_signed_info **sinfo,
 
 	CKNULL(sinfo, -EINVAL);
 
-	if (!pkcs7->curr_signed_infos) {
+	if (!pkcs7->curr_sinfo) {
 		if (pkcs7->consumed_preallocated_sinfo <
 		    pkcs7->avail_preallocated_sinfo) {
-			pkcs7->curr_signed_infos = pkcs7->preallocated_sinfo;
+			pkcs7->curr_sinfo = pkcs7->preallocated_sinfo;
 			pkcs7->consumed_preallocated_sinfo++;
 			pkcs7->preallocated_sinfo++;
 		} else {
 			CKINT(lc_alloc_aligned(
-				(void **)&pkcs7->curr_signed_infos, 8,
+				(void **)&pkcs7->curr_sinfo, 8,
 				sizeof(struct lc_pkcs7_signed_info)));
 		}
 	}
 
-	*sinfo = pkcs7->curr_signed_infos;
+	*sinfo = pkcs7->curr_sinfo;
+
+out:
+	return ret;
+}
+
+void pkcs7_x509_free(struct lc_x509_certificate *x509)
+{
+	if (x509->preallocated) {
+		lc_x509_cert_clear(x509);
+	} else {
+		lc_x509_cert_clear(x509);
+		lc_free(x509);
+	}
+}
+
+int pkcs7_x509_get(struct lc_x509_certificate **x509,
+		   struct lc_pkcs7_message *pkcs7)
+{
+	struct lc_x509_certificate *tmp_x509;
+	int ret = 0;
+
+	CKNULL(x509, -EINVAL);
+
+	if (pkcs7->consumed_preallocated_x509 <
+	    pkcs7->avail_preallocated_x509) {
+		tmp_x509 = pkcs7->preallocated_x509;
+		pkcs7->consumed_preallocated_x509++;
+		pkcs7->preallocated_x509++;
+		tmp_x509->preallocated = 1;
+	} else {
+		CKINT(lc_alloc_aligned((void **)&tmp_x509, 8,
+				       sizeof(struct lc_x509_certificate)));
+	}
+
+	*x509 = tmp_x509;
 
 out:
 	return ret;

asn1/src/pkcs7_parser.c

@@ -94,7 +94,7 @@ static int pkcs7_check_authattrs(struct lc_pkcs7_message *msg)
 	struct lc_pkcs7_signed_info *sinfo;
 	unsigned int want = 0;
 
-	sinfo = msg->list_head_signed_infos;
+	sinfo = msg->list_head_sinfo;
 	if (!sinfo)
 		goto inconsistent;
 
@@ -376,6 +376,7 @@ int pkcs7_extract_cert(void *context, size_t hdrlen, unsigned char tag,
 		       const uint8_t *value, size_t vlen)
 {
 	struct pkcs7_parse_context *ctx = context;
+	struct lc_pkcs7_message *pkcs7 = ctx->msg;
 	struct lc_x509_certificate *x509;
 	struct lc_asymmetric_key_id *id;
 	int ret;
@@ -397,8 +398,7 @@ int pkcs7_extract_cert(void *context, size_t hdrlen, unsigned char tag,
 	if (((uint8_t *)value)[1] == 0x80)
 		vlen += 2; /* Indefinite length - there should be an EOC */
 
-	CKINT(lc_alloc_aligned((void **)&x509, 8,
-			       sizeof(struct lc_x509_certificate)));
+	CKINT(pkcs7_x509_get(&x509, pkcs7));
 
 	CKINT(lc_x509_cert_decode(x509, value, vlen));
 
@@ -781,8 +781,7 @@ LC_INTERFACE_FUNCTION(void, lc_pkcs7_message_clear,
 		while (pkcs7->certs) {
 			cert = pkcs7->certs;
 			pkcs7->certs = cert->next;
-			lc_x509_cert_clear(cert);
-			lc_free(cert);
+			pkcs7_x509_free(cert);
 		}
 		while (pkcs7->crl) {
 			cert = pkcs7->crl;
@@ -805,10 +804,7 @@ LC_INTERFACE_FUNCTION(int, lc_pkcs7_message_parse,
 	CKNULL(pkcs7, -EINVAL);
 	CKNULL(data, -EINVAL);
 
-	lc_memset_secure(pkcs7, 0, sizeof(struct lc_pkcs7_message));
-
 	ctx.msg = pkcs7;
-
 	ctx.data = data;
 	ctx.ppcerts = &ctx.certs;
 
@@ -822,8 +818,7 @@ LC_INTERFACE_FUNCTION(int, lc_pkcs7_message_parse,
 		struct lc_x509_certificate *cert = ctx.certs;
 
 		ctx.certs = cert->next;
-		lc_x509_cert_clear(cert);
-		lc_free(cert);
+		pkcs7_x509_free(cert);
 	}
 
 	if (ret)