[RFC PATCH] Add config option to ignore certs in CMS payload for cert validation

[David Gstir]

The CMS SignedData structure defined in RFC 5652 allows embedding
additional X.509 certificates in the signature data. This is mainly used
to transport the signing certificates for validation, but also allows
including additional certificates that are used during certificate
chain validation.

While these certificates are treated as "untrusted", there are cases
where this can be harmful for how we use CMS in swupdate. Consider the
following real-world example:

+---------+ +-----------+ +---------------+
| Root-CA |------>| CA-prod |------>| swu-sign-cert |
+---------+ +-----------+ +---------------+
|
| +------------+ +----------------+
+------------->| CA-devel |---->| some-sign-cert |
+------------+ +----------------+

Root-CA signed multiple intermediate CAs (CA-prod and CA-devel).
Each of these intermediate CAs has a different purpose and signed
one or more leaf signing certificates. In swupdate we use
`-k ca-chain.crt` to supply the root CA and CA-prod certificate chain
as trusted certificates. Every update file will contain the
swu-sign-cert and the respective signature. When swupdate verifies
this signature, it will verify that the certificate chain ends in a
trusted certificate.

CMS however also allows to include certificates used for chain
construction in the CMS structure that is signed. This means we can
also use some-sign-cert to sign a malicious update file and include
CA-devel in the update file. According to the CMS specification these
additional certificates will be treated as untrusted, but will be used
in certificate chain construction. If a valid chain ending in a trusted
certificate can be constructed, the signature check passes and swupdate
will install the malicious update.

Add the option CONFIG_CMS_IGNORE_ADDITIONAL_CERTS to strictly ignore
any CMS-transported certificates besides the direct signing certificates
themselves during certificate verification.

As there are still legitimate use cases for delivering intermediate
certificates via the CMS SignedData such as an expired intermediate CA
in the trust store, the option is off by default.

Signed-off-by: David Gstir <da...@sigma-star.at>
---
crypto/Kconfig | 4 +++
crypto/swupdate_cms_verify_openssl.c | 47 ++++++++++++++++++++--------
2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 4f109fd0..1e2b4fe5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -80,6 +80,10 @@ config CMS_IGNORE_CERTIFICATE_PURPOSE
config CMS_SKIP_UNKNOWN_SIGNERS
bool "Ignore unverifiable signatures if known signer verifies"
depends on SIGALG_CMS
+
+config CMS_IGNORE_ADDITIONAL_CERTS
+ bool "Use only direct signer certificates from CMS signature"
+ depends on SIGALG_CMS
endmenu

menu "Encryption"
diff --git a/crypto/swupdate_cms_verify_openssl.c b/crypto/swupdate_cms_verify_openssl.c
index 849152d1..9feec799 100644
--- a/crypto/swupdate_cms_verify_openssl.c
+++ b/crypto/swupdate_cms_verify_openssl.c
@@ -16,10 +16,10 @@
#include "util.h"
#include "swupdate_crypto.h"

-#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS)
-#define VERIFY_UNKNOWN_SIGNER_FLAGS (CMS_NO_SIGNER_CERT_VERIFY)
+#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS) || defined(CONFIG_CMS_IGNORE_ADDITIONAL_CERTS)
+#define VERIFY_CMS_FLAGS (CMS_NO_SIGNER_CERT_VERIFY)
#else
-#define VERIFY_UNKNOWN_SIGNER_FLAGS (0)
+#define VERIFY_CMS_FLAGS (0)
#endif

#define MODNAME "opensslCMS"
@@ -223,11 +223,10 @@ static int check_signer_name(CMS_ContentInfo *cms, const char *name)
return ret;
}

-#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS)
-static int check_verified_signer(CMS_ContentInfo* cms, X509_STORE* store)
+#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS) || defined(CONFIG_CMS_IGNORE_ADDITIONAL_CERTS)
+static int verify_signer_certs(CMS_ContentInfo* cms, X509_STORE* store)
{
- int i, ret = 1;
-
+ int i, valid_signers, needed_signers, ret = 1;
X509_STORE_CTX *ctx = X509_STORE_CTX_new();
STACK_OF(CMS_SignerInfo) *infos = CMS_get0_SignerInfos(cms);
STACK_OF(X509)* cms_certs = CMS_get1_certs(cms);
@@ -237,7 +236,25 @@ static int check_verified_signer(CMS_ContentInfo* cms, X509_STORE* store)
return ret;
}

- for (i = 0; i < sk_CMS_SignerInfo_num(infos) && ret != 0; ++i) {
+#if defined(CONFIG_CMS_IGNORE_ADDITIONAL_CERTS)
+ /*
+ * we want all signers need to be valid, but do not use any additionaly
+ * certs from CMS signedData. This way we prevent adjacent intermediate certs
+ * to our trusted cert chain from being used and only validate actual
+ * signing certificates without using any additional certificates
+ * besides the one in the trust store.
+ */
+ needed_signers = sk_CMS_SignerInfo_num(infos);
+ cms_certs = NULL;
+#endif
+
+#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS)
+ /* we only need a single valid signer */
+ needed_signers = 1;
+#endif
+
+ valid_signers = 0;
+ for (i = 0; i < sk_CMS_SignerInfo_num(infos); ++i) {
CMS_SignerInfo *si = sk_CMS_SignerInfo_value(infos, i);
X509 *signer = NULL;

@@ -250,12 +267,17 @@ static int check_verified_signer(CMS_ContentInfo* cms, X509_STORE* store)
X509_STORE_CTX_set_default(ctx, "smime_sign");
if (X509_verify_cert(ctx) > 0) {
TRACE("Verified signature %d in signer sequence", i);
- ret = 0;
+ valid_signers++;
} else {
TRACE("Failed to verify certificate %d in signer sequence", i);
}

X509_STORE_CTX_cleanup(ctx);
+
+ if (valid_signers == needed_signers) {
+ ret = 0;
+ break;
+ }
}

X509_STORE_CTX_free(ctx);
@@ -375,16 +397,15 @@ static int openssl_cms_verify_file(void *ctx, const char *sigfile,

/* Then try to verify signature */
if (!CMS_verify(cms, NULL, dgst->certs, content_bio,
- NULL, CMS_BINARY | VERIFY_UNKNOWN_SIGNER_FLAGS)) {
+ NULL, CMS_BINARY | VERIFY_CMS_FLAGS)) {
ERR_print_errors_fp(stderr);
ERROR("Signature verification failed");
status = -EBADMSG;
goto out;
}

-#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS)
- /* Verify at least one signer authenticates */
- if (check_verified_signer(cms, dgst->certs)) {
+#if defined(CONFIG_CMS_SKIP_UNKNOWN_SIGNERS) || defined(CONFIG_CMS_IGNORE_ADDITIONAL_CERTS)
+ if (verify_signer_certs(cms, dgst->certs)) {
ERROR("Authentication of all signatures failed");
status = -EBADMSG;
goto out;
--
2.51.0