Skip to content

Commit

Permalink
fix(plugins): More thorough validation that the issuer is a valid CA
Browse files Browse the repository at this point in the history
  • Loading branch information
@jpfr
jpfr committed Oct 2, 2024
1 parent dfe4eea commit f9d4c16
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 39 deletions.
94 changes: 64 additions & 30 deletions plugins/crypto/mbedtls/ua_pki_mbedtls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#ifdef UA_ENABLE_ENCRYPTION_MBEDTLS

#include <mbedtls/x509.h>
#include <mbedtls/oid.h>
#include <mbedtls/x509_crt.h>
#include <mbedtls/error.h>
#include <mbedtls/version.h>
Expand Down Expand Up @@ -232,31 +233,63 @@ reloadCertificates(CertInfo *ci) {

#endif

#define UA_MBEDTLS_MAX_CHAIN_LENGTH 10
#define UA_MBEDTLS_MAX_DN_LENGTH 256

/* We need to access some private fields below */
#ifndef MBEDTLS_PRIVATE
#define MBEDTLS_PRIVATE(x) x
#endif

/* Return the first matching issuer candidate AFTER prev */
/* Is the certificate a CA? */
static UA_Boolean
mbedtlsCheckCA(mbedtls_x509_crt *cert) {
/* The Basic Constraints extension must be set and the cert acts as CA */
if(!(cert->MBEDTLS_PRIVATE(ext_types) & MBEDTLS_X509_EXT_BASIC_CONSTRAINTS) ||
!cert->MBEDTLS_PRIVATE(ca_istrue))
return false;

/* The Key Usage extension must be set to cert signing and CRL issuing */
if(!(cert->MBEDTLS_PRIVATE(ext_types) & MBEDTLS_X509_EXT_KEY_USAGE) ||
mbedtls_x509_crt_check_key_usage(cert, MBEDTLS_X509_KU_KEY_CERT_SIGN) != 0 ||
mbedtls_x509_crt_check_key_usage(cert, MBEDTLS_X509_KU_CRL_SIGN) != 0)
return false;

return true;
}

static UA_Boolean
mbedtlsSameName(UA_String name, const mbedtls_x509_name *name2) {
char buf[UA_MBEDTLS_MAX_DN_LENGTH];
int len = mbedtls_x509_dn_gets(buf, UA_MBEDTLS_MAX_DN_LENGTH, name2);
if(len < 0)
return false;
UA_String nameString = {(size_t)len, (UA_Byte*)buf};
return UA_String_equal(&name, &nameString);
}

/* Return the first matching issuer candidate AFTER prev.
* This can return the cert itself if self-signed. */
static mbedtls_x509_crt *
mbedtlsFindNextIssuer(CertInfo *ci, mbedtls_x509_crt *stack,
mbedtls_x509_crt *cert, mbedtls_x509_crt *prev) {
char inbuf[512], snbuf[512];
if(mbedtls_x509_dn_gets(inbuf, 512, &cert->issuer) < 0)
char inbuf[UA_MBEDTLS_MAX_DN_LENGTH];
int nameLen = mbedtls_x509_dn_gets(inbuf, UA_MBEDTLS_MAX_DN_LENGTH, &cert->issuer);
if(nameLen < 0)
return NULL;
UA_String issuerName = {(size_t)nameLen, (UA_Byte*)inbuf};
do {
for(mbedtls_x509_crt *i = stack; i; i = i->next) {
/* Compare issuer name and subject name */
if(mbedtls_x509_dn_gets(snbuf, 512, &i->subject) < 0)
continue;
if(strncmp(inbuf, snbuf, 512) != 0)
continue;
/* Skip when the key does not match the signature */
if(!mbedtls_pk_can_do(&i->pk, cert->MBEDTLS_PRIVATE(sig_pk)))
if(prev) {
if(prev == i)
prev = NULL; /* This was the last issuer we tried to verify */
continue;
if(prev == NULL)
}
/* Compare issuer name and subject name.
* Skip when the key does not match the signature. */
if(mbedtlsSameName(issuerName, &i->subject) &&
mbedtls_pk_can_do(&i->pk, cert->MBEDTLS_PRIVATE(sig_pk)))
return i;
prev = NULL; /* This was the last issuer we tried to verify */
}
/* Switch from the stack that came with the cert to the ctx->skIssue list */
stack = (stack != &ci->certificateIssuerList) ? &ci->certificateIssuerList : NULL;
Expand All @@ -266,17 +299,16 @@ mbedtlsFindNextIssuer(CertInfo *ci, mbedtls_x509_crt *stack,

static UA_Boolean
mbedtlsCheckRevoked(CertInfo *ci, mbedtls_x509_crt *cert) {
char inbuf[512], inbuf2[512];
if(mbedtls_x509_dn_gets(inbuf, 512, &cert->issuer) < 0)
char inbuf[UA_MBEDTLS_MAX_DN_LENGTH];
int nameLen = mbedtls_x509_dn_gets(inbuf, UA_MBEDTLS_MAX_DN_LENGTH, &cert->issuer);
if(nameLen < 0)
return true;
UA_String issuerName = {(size_t)nameLen, (UA_Byte*)inbuf};
for(mbedtls_x509_crl *crl = &ci->certificateRevocationList; crl; crl = crl->next) {
/* Is the CRL for the issuer of the certificate? */
if(mbedtls_x509_dn_gets(inbuf2, 512, &crl->issuer) < 0)
return true;
if(strncmp(inbuf, inbuf2, 512) != 0)
continue;
/* Is the serial number of the certificate contained in the CRL? */
if(mbedtls_x509_crt_is_revoked(cert, crl) != 0)
/* Is the CRL for certificates from the cert issuer?
* Is the serial number of the certificate contained in the CRL? */
if(mbedtlsSameName(issuerName, &crl->issuer) &&
mbedtls_x509_crt_is_revoked(cert, crl) != 0)
return true;
}
return false;
Expand Down Expand Up @@ -305,8 +337,6 @@ mbedtlsCheckSignature(const mbedtls_x509_crt *cert, mbedtls_x509_crt *issuer) {
hash, hash_len, sig->p, sig->len) == 0);
}

#define UA_MBEDTLS_MAX_CHAIN_LENGTH 10

static UA_StatusCode
mbedtlsVerifyChain(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_x509_crt **old_issuers,
mbedtls_x509_crt *cert, int depth) {
Expand Down Expand Up @@ -337,6 +367,13 @@ mbedtlsVerifyChain(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_x509_crt **old
if(!issuer)
break;

/* Verification Step: Certificate Usage
* Can the issuer act as CA? Omit for self-signed leaf certificates. */
if((depth > 0 || issuer != cert) && !mbedtlsCheckCA(issuer)) {
ret = UA_STATUSCODE_BADCERTIFICATEISSUERUSENOTALLOWED;
continue;
}

/* Verification Step: Signature */
if(!mbedtlsCheckSignature(cert, issuer)) {
ret = UA_STATUSCODE_BADCERTIFICATEINVALID; /* Wrong issuer, try again */
Expand All @@ -346,7 +383,7 @@ mbedtlsVerifyChain(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_x509_crt **old
/* The certificate is self-signed. We have arrived at the top of the
* chain. We check whether the certificate is trusted below. This is the
* only place where we return UA_STATUSCODE_BADCERTIFICATEUNTRUSTED.
* This sinals that the chain is complete (but can be still
* This signals that the chain is complete (but can be still
* untrusted). */
if(issuer == cert || (cert->tbs.len == issuer->tbs.len &&
memcmp(cert->tbs.p, issuer->tbs.p, cert->tbs.len) == 0)) {
Expand Down Expand Up @@ -408,12 +445,9 @@ certificateVerification_verify(void *verificationContext,

/* Verification Step: Certificate Usage
* Check whether the certificate is a User certificate or a CA certificate.
* If the KU_KEY_CERT_SIGN and KU_CRL_SIGN of key_usage are set, then the
* certificate shall be condidered as CA Certificate and cannot be used to
* establish a connection. Refer the test case CTT/Security/Security
* Certificate Validation/029.js for more details */
unsigned int ca_flags = MBEDTLS_X509_KU_KEY_CERT_SIGN | MBEDTLS_X509_KU_CRL_SIGN;
if(mbedtls_x509_crt_check_key_usage(&cert, ca_flags)) {
* Refer the test case CTT/Security/Security Certificate Validation/029.js
* for more details. */
if(mbedtlsCheckCA(&cert)) {
mbedtls_x509_crt_free(&cert);
return UA_STATUSCODE_BADCERTIFICATEUSENOTALLOWED;
}
Expand Down
28 changes: 19 additions & 9 deletions plugins/crypto/openssl/ua_pki_openssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,16 @@ openSSLFindNextIssuer(CertContext *ctx, STACK_OF(X509) *stack, X509 *x509, X509
int size = sk_X509_num(stack);
for(int i = 0; i < size; i++) {
X509 *candidate = sk_X509_value(stack, i);
if(X509_check_issued(candidate, x509) != 0)
if(prev) {
if(prev == candidate)
prev = NULL; /* This was the last issuer we tried to verify */
continue;
if(prev == NULL)
}
/* This checks subject/issuer name and the key usage of the issuer.
* It does not verify the validity period and if the issuer key was
* used for the signature. We check that afterwards. */
if(X509_check_issued(candidate, x509) == 0)
return candidate;
prev = NULL;
}
/* Switch to search in the ctx->skIssue list */
stack = (stack != ctx->skIssue) ? ctx->skIssue : NULL;
Expand Down Expand Up @@ -519,6 +524,13 @@ openSSL_verifyChain(CertContext *ctx, STACK_OF(X509) *stack, X509 **old_issuers,
if(!issuer)
break;

/* Verification Step: Certificate Usage
* Can the issuer act as CA? Omit for self-signed leaf certificates. */
if((depth > 0 || issuer != cert) && !X509_check_ca(issuer)) {
ret = UA_STATUSCODE_BADCERTIFICATEISSUERUSENOTALLOWED;
continue;
}

/* Verification Step: Signature */
int opensslRet = X509_verify(cert, X509_get0_pubkey(issuer));
if(opensslRet == -1) {
Expand All @@ -531,7 +543,7 @@ openSSL_verifyChain(CertContext *ctx, STACK_OF(X509) *stack, X509 **old_issuers,
/* The certificate is self-signed. We have arrived at the top of the
* chain. We check whether the certificate is trusted below. This is the
* only place where we return UA_STATUSCODE_BADCERTIFICATEUNTRUSTED.
* This sinals that the chain is complete (but can be still
* This signals that the chain is complete (but can be still
* untrusted). */
if(cert == issuer || X509_cmp(cert, issuer) == 0) {
ret = UA_STATUSCODE_BADCERTIFICATEUNTRUSTED;
Expand Down Expand Up @@ -589,12 +601,10 @@ UA_CertificateVerification_Verify(void *verificationContext,

/* Verification Step: Certificate Usage
* Check whether the certificate is a User certificate or a CA certificate.
* If the KU_KEY_CERT_SIGN and KU_CRL_SIGN of key_usage are set, then the
* certificate shall be condidered as CA Certificate and cannot be used to
* establish a connection. Refer the test case CTT/Security/Security
* Certificate Validation/029.js for more details */
* Refer the test case CTT/Security/Security Certificate Validation/029.js
* for more details. */
X509 *leaf = sk_X509_value(stack, 0);
if(X509_check_purpose(leaf, X509_PURPOSE_CRL_SIGN, 0) && X509_check_ca(leaf)) {
if(X509_check_ca(leaf)) {
sk_X509_pop_free(stack, X509_free);
return UA_STATUSCODE_BADCERTIFICATEUSENOTALLOWED;
}
Expand Down

0 comments on commit f9d4c16

Please sign in to comment.