Skip to content

Commit

Permalink
fix(plugins): Certificate verification must not depend on an absolute…
Browse files Browse the repository at this point in the history 
… memory location of the CertificateGroup
  • Loading branch information
@jpfr
jpfr committed Nov 29, 2024
1 parent 482c78c commit 073e81b
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 21 deletions.
18 changes: 7 additions & 11 deletions plugins/crypto/mbedtls/certificategroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
17 changes: 7 additions & 10 deletions plugins/crypto/openssl/certificategroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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. */
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 073e81b

Please sign in to comment.