From 073e81ba90986064f5f8e9f6fadd06cbb61c0753 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 29 Nov 2024 18:02:16 +0100 Subject: [PATCH] fix(plugins): Certificate verification must not depend on an absolute memory location of the CertificateGroup --- plugins/crypto/mbedtls/certificategroup.c | 18 +++++++----------- plugins/crypto/openssl/certificategroup.c | 17 +++++++---------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/plugins/crypto/mbedtls/certificategroup.c b/plugins/crypto/mbedtls/certificategroup.c index d17e7efa473..04960828350 100644 --- a/plugins/crypto/mbedtls/certificategroup.c +++ b/plugins/crypto/mbedtls/certificategroup.c @@ -56,8 +56,6 @@ typedef struct { mbedtls_x509_crt issuerCertificates; mbedtls_x509_crl trustedCrls; mbedtls_x509_crl issuerCrls; - - UA_CertificateGroup *cg; } MemoryCertStore; static UA_Boolean mbedtlsCheckCA(mbedtls_x509_crt *cert); @@ -415,7 +413,7 @@ mbedtlsFindNextIssuer(MemoryCertStore *ctx, mbedtls_x509_crt *stack, } static UA_StatusCode -mbedtlsCheckRevoked(MemoryCertStore *ctx, mbedtls_x509_crt *cert) { +mbedtlsCheckRevoked(UA_CertificateGroup *cg, MemoryCertStore *ctx, mbedtls_x509_crt *cert) { /* Parse the Issuer Name */ char inbuf[UA_MBEDTLS_MAX_DN_LENGTH]; int nameLen = mbedtls_x509_dn_gets(inbuf, UA_MBEDTLS_MAX_DN_LENGTH, &cert->issuer); @@ -424,7 +422,7 @@ mbedtlsCheckRevoked(MemoryCertStore *ctx, mbedtls_x509_crt *cert) { UA_String issuerName = {(size_t)nameLen, (UA_Byte*)inbuf}; if(ctx->trustedCrls.raw.len == 0 && ctx->issuerCrls.raw.len == 0) { - UA_LOG_WARNING(ctx->cg->logging, UA_LOGCATEGORY_SECURITYPOLICY, + UA_LOG_WARNING(cg->logging, UA_LOGCATEGORY_SECURITYPOLICY, "Zero revocation lists have been loaded. " "This seems intentional - omitting the check."); return UA_STATUSCODE_GOOD; @@ -478,9 +476,8 @@ mbedtlsCheckSignature(const mbedtls_x509_crt *cert, mbedtls_x509_crt *issuer) { } static UA_StatusCode -mbedtlsVerifyChain(MemoryCertStore *ctx, mbedtls_x509_crt *stack, - mbedtls_x509_crt **old_issuers, - mbedtls_x509_crt *cert, int depth) { +mbedtlsVerifyChain(UA_CertificateGroup *cg, MemoryCertStore *ctx, mbedtls_x509_crt *stack, + mbedtls_x509_crt **old_issuers, mbedtls_x509_crt *cert, int depth) { /* Maxiumum chain length */ if(depth == UA_MBEDTLS_MAX_CHAIN_LENGTH) return UA_STATUSCODE_BADCERTIFICATECHAININCOMPLETE; @@ -530,7 +527,7 @@ mbedtlsVerifyChain(MemoryCertStore *ctx, mbedtls_x509_crt *stack, } /* Verification Step: Revocation Check */ - ret = mbedtlsCheckRevoked(ctx, cert); + ret = mbedtlsCheckRevoked(cg, ctx, cert); if(depth > 0) { if(ret == UA_STATUSCODE_BADCERTIFICATEREVOKED) ret = UA_STATUSCODE_BADCERTIFICATEISSUERREVOKED; @@ -549,7 +546,7 @@ mbedtlsVerifyChain(MemoryCertStore *ctx, mbedtls_x509_crt *stack, /* We have found the issuer certificate used for the signature. Recurse * to the next certificate in the chain (verify the current issuer). */ - ret = mbedtlsVerifyChain(ctx, stack, old_issuers, issuer, depth + 1); + ret = mbedtlsVerifyChain(cg, ctx, stack, old_issuers, issuer, depth + 1); } /* The chain is complete, but we haven't yet identified a trusted @@ -609,7 +606,7 @@ verifyCertificate(UA_CertificateGroup *certGroup, const UA_ByteString *certifica /* Verification Step: Build Certificate Chain * We perform the checks for each certificate inside. */ mbedtls_x509_crt *old_issuers[UA_MBEDTLS_MAX_CHAIN_LENGTH]; - UA_StatusCode ret = mbedtlsVerifyChain(context, &cert, old_issuers, &cert, 0); + UA_StatusCode ret = mbedtlsVerifyChain(certGroup, context, &cert, old_issuers, &cert, 0); mbedtls_x509_crt_free(&cert); return ret; } @@ -667,7 +664,6 @@ UA_CertificateGroup_Memorystore(UA_CertificateGroup *certGroup, retval = UA_STATUSCODE_BADOUTOFMEMORY; goto cleanup; } - context->cg = certGroup; certGroup->context = context; /* Default values */ context->maxTrustListSize = 65535; diff --git a/plugins/crypto/openssl/certificategroup.c b/plugins/crypto/openssl/certificategroup.c index a374a53b426..59026673025 100644 --- a/plugins/crypto/openssl/certificategroup.c +++ b/plugins/crypto/openssl/certificategroup.c @@ -53,8 +53,6 @@ struct MemoryCertStore { STACK_OF(X509) *trustedCertificates; STACK_OF(X509) *issuerCertificates; STACK_OF(X509_CRL) *crls; - - UA_CertificateGroup *cg; }; static UA_Boolean @@ -503,13 +501,13 @@ openSSLCheckCA(X509 *cert) { } static UA_StatusCode -openSSLCheckRevoked(MemoryCertStore *ctx, X509 *cert) { +openSSLCheckRevoked(UA_CertificateGroup *cg, MemoryCertStore *ctx, X509 *cert) { const ASN1_INTEGER *sn = X509_get0_serialNumber(cert); const X509_NAME *in = X509_get_issuer_name(cert); int size = sk_X509_CRL_num(ctx->crls); if(size == 0) { - UA_LOG_WARNING(ctx->cg->logging, UA_LOGCATEGORY_SECURITYPOLICY, + UA_LOG_WARNING(cg->logging, UA_LOGCATEGORY_SECURITYPOLICY, "Zero revocation lists have been loaded. " "This seems intentional - omitting the check."); return UA_STATUSCODE_GOOD; @@ -537,8 +535,8 @@ openSSLCheckRevoked(MemoryCertStore *ctx, X509 *cert) { #define UA_OPENSSL_MAX_CHAIN_LENGTH 10 static UA_StatusCode -openSSL_verifyChain(MemoryCertStore *ctx, STACK_OF(X509) *stack, X509 **old_issuers, - X509 *cert, int depth) { +openSSL_verifyChain(UA_CertificateGroup *cg, MemoryCertStore *ctx, STACK_OF(X509) *stack, + X509 **old_issuers, X509 *cert, int depth) { /* Maxiumum chain length */ if(depth == UA_OPENSSL_MAX_CHAIN_LENGTH) return UA_STATUSCODE_BADCERTIFICATECHAININCOMPLETE; @@ -591,7 +589,7 @@ openSSL_verifyChain(MemoryCertStore *ctx, STACK_OF(X509) *stack, X509 **old_issu } /* Verification Step: Revocation Check */ - ret = openSSLCheckRevoked(ctx, cert); + ret = openSSLCheckRevoked(cg, ctx, cert); if(depth > 0) { if(ret == UA_STATUSCODE_BADCERTIFICATEREVOKED) ret = UA_STATUSCODE_BADCERTIFICATEISSUERREVOKED; @@ -611,7 +609,7 @@ openSSL_verifyChain(MemoryCertStore *ctx, STACK_OF(X509) *stack, X509 **old_issu /* We have found the issuer certificate used for the signature. Recurse * to the next certificate in the chain (verify the current issuer). */ - ret = openSSL_verifyChain(ctx, stack, old_issuers, issuer, depth + 1); + ret = openSSL_verifyChain(cg, ctx, stack, old_issuers, issuer, depth + 1); } /* Is the certificate in the trust list? If yes, then we are done. */ @@ -669,7 +667,7 @@ verifyCertificate(UA_CertificateGroup *certGroup, const UA_ByteString *certifica /* Verification Step: Build Certificate Chain * We perform the checks for each certificate inside. */ X509 *old_issuers[UA_OPENSSL_MAX_CHAIN_LENGTH]; - ret = openSSL_verifyChain(context, stack, old_issuers, leaf, 0); + ret = openSSL_verifyChain(certGroup, context, stack, old_issuers, leaf, 0); sk_X509_pop_free(stack, X509_free); return ret; } @@ -727,7 +725,6 @@ UA_CertificateGroup_Memorystore(UA_CertificateGroup *certGroup, retval = UA_STATUSCODE_BADOUTOFMEMORY; goto cleanup; } - context->cg = certGroup; certGroup->context = context; /* Default values */ context->maxTrustListSize = 65535;