diff --git a/plugins/crypto/mbedtls/ua_pki_mbedtls.c b/plugins/crypto/mbedtls/ua_pki_mbedtls.c index 5af9923488d..ad4a62ceb92 100644 --- a/plugins/crypto/mbedtls/ua_pki_mbedtls.c +++ b/plugins/crypto/mbedtls/ua_pki_mbedtls.c @@ -8,7 +8,6 @@ #include #include -#include #ifdef UA_ENABLE_ENCRYPTION_MBEDTLS @@ -22,6 +21,9 @@ #include #include +#define UA_MBEDTLS_MAX_CHAIN_LENGTH 10 +#define UA_MBEDTLS_MAX_DN_LENGTH 256 + /* Find binary substring. Taken and adjusted from * http://tungchingkai.blogspot.com/2011/07/binary-strstr.html */ @@ -83,6 +85,8 @@ static UA_ByteString copyDataFormatAware(const UA_ByteString *data) } typedef struct { + UA_CertificateVerification *cv; + /* If the folders are defined, we use them to reload the certificates during * runtime */ UA_String trustListFolder; @@ -229,17 +233,11 @@ reloadCertificates(const UA_CertificateVerification *cv, CertInfo *ci) { } } - if(internalErrorFlag) { - retval = UA_STATUSCODE_BADINTERNALERROR; - } - return retval; + return (internalErrorFlag) ? UA_STATUSCODE_BADINTERNALERROR : retval; } #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 @@ -272,6 +270,13 @@ mbedtlsSameName(UA_String name, const mbedtls_x509_name *name2) { return UA_String_equal(&name, &nameString); } +static UA_Boolean +mbedtlsSameBuf(mbedtls_x509_buf *a, mbedtls_x509_buf *b) { + if(a->len != b->len) + return false; + return (memcmp(a->p, b->p, a->len) == 0); +} + /* Return the first matching issuer candidate AFTER prev. * This can return the cert itself if self-signed. */ static mbedtls_x509_crt * @@ -295,27 +300,47 @@ mbedtlsFindNextIssuer(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_pk_can_do(&i->pk, cert->MBEDTLS_PRIVATE(sig_pk))) return i; } - /* Switch from the stack that came with the cert to the ctx->skIssue list */ - stack = (stack != &ci->certificateIssuerList) ? &ci->certificateIssuerList : NULL; + + /* Switch from the stack that came with the cert to the issuer list and + * then to the trust list. */ + if(stack == &ci->certificateTrustList) + stack = NULL; + else if(stack == &ci->certificateIssuerList) + stack = &ci->certificateTrustList; + else + stack = &ci->certificateIssuerList; } while(stack); return NULL; } -static UA_Boolean +static UA_StatusCode mbedtlsCheckRevoked(CertInfo *ci, 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); if(nameLen < 0) - return true; + return UA_STATUSCODE_BADINTERNALERROR; UA_String issuerName = {(size_t)nameLen, (UA_Byte*)inbuf}; + + if(ci->certificateRevocationList.raw.len == 0) { + UA_LOG_WARNING(ci->cv->logging, UA_LOGCATEGORY_SECURITYPOLICY, + "Zero revocation lists have been loaded. " + "This seems intentional - omitting the check."); + return UA_STATUSCODE_GOOD; + } + + /* Loop over the crl and match the Issuer Name */ + UA_StatusCode res = UA_STATUSCODE_BADCERTIFICATEREVOCATIONUNKNOWN; for(mbedtls_x509_crl *crl = &ci->certificateRevocationList; crl; crl = crl->next) { /* 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; + if(mbedtlsSameName(issuerName, &crl->issuer)) { + if(mbedtls_x509_crt_is_revoked(cert, crl) != 0) + return UA_STATUSCODE_BADCERTIFICATEREVOKED; + res = UA_STATUSCODE_GOOD; /* There was at least one crl that did not revoke (so far) */ + } } - return false; + return res; } /* Verify that the public key of the issuer was used to sign the certificate */ @@ -354,11 +379,6 @@ mbedtlsVerifyChain(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_x509_crt **old return (depth == 0) ? UA_STATUSCODE_BADCERTIFICATETIMEINVALID : UA_STATUSCODE_BADCERTIFICATEISSUERTIMEINVALID; - /* Verification Step: Revocation Check */ - if(mbedtlsCheckRevoked(ci, cert)) - return (depth == 0) ? UA_STATUSCODE_BADCERTIFICATEREVOKED : - UA_STATUSCODE_BADCERTIFICATEISSUERREVOKED; - /* Return the most specific error code. BADCERTIFICATECHAININCOMPLETE is * returned only if all possible chains are incomplete. */ mbedtls_x509_crt *issuer = NULL; @@ -388,16 +408,28 @@ mbedtlsVerifyChain(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_x509_crt **old * chain. We check whether the certificate is trusted below. This is the * only place where we return UA_STATUSCODE_BADCERTIFICATEUNTRUSTED. * 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)) { + * untrusted). + * + * Break here as we have reached the end of the chain. Omit the + * Revocation Check for self-signed certificates. */ + if(issuer == cert || mbedtlsSameBuf(&cert->tbs, &issuer->tbs)) { ret = UA_STATUSCODE_BADCERTIFICATEUNTRUSTED; - continue; + break; } - /* Detect (endless) loops of issuers. The last one can be skipped by the - * check for self-signed just before. */ - for(int i = 0; i < depth - 1; i++) { + /* Verification Step: Revocation Check */ + ret = mbedtlsCheckRevoked(ci, cert); + if(depth > 0) { + if(ret == UA_STATUSCODE_BADCERTIFICATEREVOKED) + ret = UA_STATUSCODE_BADCERTIFICATEISSUERREVOKED; + if(ret == UA_STATUSCODE_BADCERTIFICATEREVOCATIONUNKNOWN) + ret = UA_STATUSCODE_BADCERTIFICATEISSUERREVOCATIONUNKNOWN; + } + if(ret != UA_STATUSCODE_GOOD) + continue; + + /* Detect (endless) loops of issuers */ + for(int i = 0; i < depth; i++) { if(old_issuers[i] == issuer) return UA_STATUSCODE_BADCERTIFICATECHAININCOMPLETE; } @@ -412,8 +444,7 @@ mbedtlsVerifyChain(CertInfo *ci, mbedtls_x509_crt *stack, mbedtls_x509_crt **old * certificate "on the way down". Can we trust this certificate? */ if(ret == UA_STATUSCODE_BADCERTIFICATEUNTRUSTED) { for(mbedtls_x509_crt *t = &ci->certificateTrustList; t; t = t->next) { - if(cert->tbs.len == t->tbs.len && - memcmp(cert->tbs.p, t->tbs.p, cert->tbs.len) == 0) + if(mbedtlsSameBuf(&cert->tbs, &t->tbs)) return UA_STATUSCODE_GOOD; } } @@ -619,6 +650,7 @@ UA_CertificateVerification_Trustlist(UA_CertificateVerification *cv, if(!ci) return UA_STATUSCODE_BADOUTOFMEMORY; memset(ci, 0, sizeof(CertInfo)); + ci->cv = cv; mbedtls_x509_crt_init(&ci->certificateTrustList); mbedtls_x509_crl_init(&ci->certificateRevocationList); mbedtls_x509_crt_init(&ci->certificateIssuerList); @@ -700,6 +732,7 @@ UA_CertificateVerification_CertFolders(UA_CertificateVerification *cv, if(!ci) return UA_STATUSCODE_BADOUTOFMEMORY; memset(ci, 0, sizeof(CertInfo)); + ci->cv = cv; mbedtls_x509_crt_init(&ci->certificateTrustList); mbedtls_x509_crl_init(&ci->certificateRevocationList); mbedtls_x509_crt_init(&ci->certificateIssuerList); diff --git a/plugins/crypto/openssl/ua_pki_openssl.c b/plugins/crypto/openssl/ua_pki_openssl.c index 1ad873604d2..5cfaa455b33 100644 --- a/plugins/crypto/openssl/ua_pki_openssl.c +++ b/plugins/crypto/openssl/ua_pki_openssl.c @@ -8,7 +8,6 @@ #include #include -#include #include "securitypolicy_openssl_common.h" @@ -478,8 +477,14 @@ openSSLFindNextIssuer(CertContext *ctx, STACK_OF(X509) *stack, X509 *x509, X509 if(X509_check_issued(candidate, x509) == 0) return candidate; } - /* Switch to search in the ctx->skIssue list */ - stack = (stack != ctx->skIssue) ? ctx->skIssue : NULL; + /* Switch from the stack that came with the cert to the issuer list and + * then to the trust list. */ + if(stack == ctx->skTrusted) + stack = NULL; + else if(stack == ctx->skIssue) + stack = ctx->skTrusted; + else + stack = ctx->skIssue; } while(stack); return NULL; } @@ -504,11 +509,21 @@ openSSLCheckCA(X509 *cert) { return true; } -static UA_Boolean +static UA_StatusCode openSSLCheckRevoked(CertContext *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->skCrls); + + if(size == 0) { + UA_LOG_WARNING(ctx->cv->logging, UA_LOGCATEGORY_SECURITYPOLICY, + "Zero revocation lists have been loaded. " + "This seems intentional - omitting the check."); + return UA_STATUSCODE_GOOD; + } + + /* Loop over the crl and match the Issuer Name */ + UA_StatusCode res = UA_STATUSCODE_BADCERTIFICATEREVOCATIONUNKNOWN; for(int i = 0; i < size; i++) { /* The crl contains a list of serial numbers from the same issuer */ X509_CRL *crl = sk_X509_CRL_value(ctx->skCrls, i); @@ -519,10 +534,11 @@ openSSLCheckRevoked(CertContext *ctx, X509 *cert) { for(int j = 0; j < rsize; j++) { X509_REVOKED *r = sk_X509_REVOKED_value(rs, j); if(ASN1_INTEGER_cmp(sn, X509_REVOKED_get0_serialNumber(r)) == 0) - return true; + return UA_STATUSCODE_BADCERTIFICATEREVOKED; } + res = UA_STATUSCODE_GOOD; /* There was at least one crl that did not revoke (so far) */ } - return false; + return res; } #define UA_OPENSSL_MAX_CHAIN_LENGTH 10 @@ -541,11 +557,6 @@ openSSL_verifyChain(CertContext *ctx, STACK_OF(X509) *stack, X509 **old_issuers, return (depth == 0) ? UA_STATUSCODE_BADCERTIFICATETIMEINVALID : UA_STATUSCODE_BADCERTIFICATEISSUERTIMEINVALID; - /* Verification Step: Revocation Check */ - if(openSSLCheckRevoked(ctx, cert)) - return (depth == 0) ? UA_STATUSCODE_BADCERTIFICATEREVOKED : - UA_STATUSCODE_BADCERTIFICATEISSUERREVOKED; - /* Return the most specific error code. BADCERTIFICATECHAININCOMPLETE is * returned only if all possible chains are incomplete. */ X509 *issuer = NULL; @@ -577,12 +588,26 @@ openSSL_verifyChain(CertContext *ctx, STACK_OF(X509) *stack, X509 **old_issuers, * chain. We check whether the certificate is trusted below. This is the * only place where we return UA_STATUSCODE_BADCERTIFICATEUNTRUSTED. * This signals that the chain is complete (but can be still - * untrusted). */ + * untrusted). + * + * Break here as we have reached the end of the chain. Omit the + * Revocation Check for self-signed certificates. */ if(cert == issuer || X509_cmp(cert, issuer) == 0) { ret = UA_STATUSCODE_BADCERTIFICATEUNTRUSTED; - continue; + break; } + /* Verification Step: Revocation Check */ + ret = openSSLCheckRevoked(ctx, cert); + if(depth > 0) { + if(ret == UA_STATUSCODE_BADCERTIFICATEREVOKED) + ret = UA_STATUSCODE_BADCERTIFICATEISSUERREVOKED; + if(ret == UA_STATUSCODE_BADCERTIFICATEREVOCATIONUNKNOWN) + ret = UA_STATUSCODE_BADCERTIFICATEISSUERREVOCATIONUNKNOWN; + } + if(ret != UA_STATUSCODE_GOOD) + continue; + /* Detect (endless) loops of issuers. The last one can be skipped by the * check for self-signed just before. */ for(int i = 0; i < depth; i++) {