From 9991304ad444d1737e91a5eed34249711ba8a079 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Fri, 4 Sep 2026 07:45:36 +0300 Subject: [PATCH 1/5] Fix: verify TLS certificate hostnames on non-Apple platforms COR-170 On Windows, Android and Linux mailcore::checkCertificate() only validated that the server certificate chained to a trusted root; the hostname parameter was never used, so a valid certificate for any other domain was accepted for IMAP, SMTP, POP and NNTP (CWE-297). The Apple branch already evaluated SecPolicyCreateSSL(true, hostname). - MCCertificateUtils: attach the expected identity to the X509 verify parameters (X509_VERIFY_PARAM_set1_host / set1_ip_asc, no partial wildcards) so X509_verify_cert() fails with "hostname mismatch"; log X509_verify_cert_error_string() on failure; reject empty hostnames and malformed DER instead of proceeding. - checkCertificate() now fetches the chain from the stream and hands it to checkCertificateChain(cCerts, hostname, cTrustAnchors, verifyTime), whose body is the original verification unchanged. The extra trust anchors and the verify time exist for tests only; production passes NULL and 0 and keeps using the system store. - Send SNI on non-Apple platforms for every TLS path that lacked it: IMAP/SMTP/POP StartTLS and POP/NNTP direct TLS. Without SNI a server may present a default certificate for another name, which the new check would correctly reject. - CCertificateUtils: C entry point so the verifier can be unit-tested from Swift; CertificateUtilsTests cover exact/case-insensitive/ wildcard/IPv4/IPv6 matches and mismatches, wrong root, expiry, empty and malformed input, against fixtures in data/certificates generated with a private CA (leaf valid 2026-01-01..2027-12-31, verification pinned to 2026-06-15). Co-Authored-By: Claude Fable 5.1 --- Package.swift | 2 +- configure-headers.sh | 2 + src/c/utils/CCertificateUtils.cpp | 49 ++++++++ src/c/utils/CCertificateUtils.h | 36 ++++++ src/cmake/public-headers.cmake | 1 + src/core/imap/MCIMAPSession.cpp | 9 ++ src/core/nntp/MCNNTPSession.cpp | 17 +++ src/core/pop/MCPOPSession.cpp | 26 ++++ src/core/security/MCCertificateUtils.cpp | 107 +++++++++++++--- src/core/security/MCCertificateUtils.h | 11 ++ src/core/smtp/MCSMTPSession.cpp | 9 ++ src/include/MailCore/CCertificateUtils.h | 36 ++++++ src/include/MailCore/MCCertificateUtils.h | 33 +++++ unittest/CertificateUtilsTests.swift | 146 ++++++++++++++++++++++ unittest/data/certificates/ca.der | Bin 0 -> 873 bytes unittest/data/certificates/leaf.der | Bin 0 -> 959 bytes unittest/data/certificates/other.der | Bin 0 -> 865 bytes 17 files changed, 466 insertions(+), 18 deletions(-) create mode 100644 src/c/utils/CCertificateUtils.cpp create mode 100644 src/c/utils/CCertificateUtils.h create mode 100644 src/include/MailCore/CCertificateUtils.h create mode 100644 src/include/MailCore/MCCertificateUtils.h create mode 100644 unittest/CertificateUtilsTests.swift create mode 100644 unittest/data/certificates/ca.der create mode 100644 unittest/data/certificates/leaf.der create mode 100644 unittest/data/certificates/other.der diff --git a/Package.swift b/Package.swift index b562cf7f1..4c3269e0f 100644 --- a/Package.swift +++ b/Package.swift @@ -265,7 +265,7 @@ var targets: [Target] = [ "unittest.cpp", "unittest.mm" ], - sources: ["IMAPInterruptCurrentCommandTests.swift", "LibetpanHelperTests.swift", "unittest.swift"], + sources: ["CertificateUtilsTests.swift", "IMAPInterruptCurrentCommandTests.swift", "LibetpanHelperTests.swift", "unittest.swift"], resources: [ .copy("data") ] diff --git a/configure-headers.sh b/configure-headers.sh index a31b71c15..19974991c 100755 --- a/configure-headers.sh +++ b/configure-headers.sh @@ -50,6 +50,7 @@ cp core/basetypes/MCDataStreamDecoder.h ./include/MailCore cp core/basetypes/MCDefines.h ./include/MailCore cp core/basetypes/MCMD5.h ./include/MailCore cp core/basetypes/MCOperationQueueCallback.h ./include/MailCore +cp core/security/MCCertificateUtils.h ./include/MailCore cp core/basetypes/MCBase64.h ./include/MailCore cp core/basetypes/MCLock.h ./include/MailCore cp core/basetypes/MCMainThread.h ./include/MailCore @@ -252,3 +253,4 @@ cp c/imap/CIMAPSearchOperation.h ./include/MailCore cp c/utils/COperation.h ./include/MailCore cp c/utils/CAutoreleasePool.h ./include/MailCore +cp c/utils/CCertificateUtils.h ./include/MailCore diff --git a/src/c/utils/CCertificateUtils.cpp b/src/c/utils/CCertificateUtils.cpp new file mode 100644 index 000000000..8bb0a2210 --- /dev/null +++ b/src/c/utils/CCertificateUtils.cpp @@ -0,0 +1,49 @@ +// +// CCertificateUtils.cpp +// mailcore2 +// + +#include "CCertificateUtils.h" +#include "CBase+Private.h" + +#include +#include +#include + +// Converts an Array of Data into the carray of MMAPString that libetpan uses for +// certificate chains. Free with mailstream_certificate_chain_free(). +static carray * certificateChainFromArray(mailcore::Array * array) +{ + if (array == NULL) { + return NULL; + } + carray * result = carray_new(array->count() > 0 ? array->count() : 1); + for(unsigned int i = 0 ; i < array->count() ; i ++) { + mailcore::Data * der = (mailcore::Data *) array->objectAtIndex(i); + MMAPString * str = mmap_string_new_len(der->bytes(), (size_t) der->length()); + carray_add(result, str, NULL); + } + return result; +} + +bool CCertificateUtils_checkCertificateChain(CArray derCertificates, + MailCoreString hostname, + CArray derTrustAnchors, + int64_t verifyTime) +{ + carray * cCerts = certificateChainFromArray(derCertificates.instance); + carray * cTrustAnchors = certificateChainFromArray(derTrustAnchors.instance); + + bool result = false; + if (cCerts != NULL) { + result = mailcore::checkCertificateChain(cCerts, hostname.instance, cTrustAnchors, (time_t) verifyTime); + } + + if (cCerts != NULL) { + mailstream_certificate_chain_free(cCerts); + } + if (cTrustAnchors != NULL) { + mailstream_certificate_chain_free(cTrustAnchors); + } + return result; +} diff --git a/src/c/utils/CCertificateUtils.h b/src/c/utils/CCertificateUtils.h new file mode 100644 index 000000000..213662647 --- /dev/null +++ b/src/c/utils/CCertificateUtils.h @@ -0,0 +1,36 @@ +// +// CCertificateUtils.h +// mailcore2 +// +// C entry point to mailcore::checkCertificateChain(), so the certificate verification every +// session performs after the TLS handshake can be exercised from Swift unit tests (COR-170). +// + +#ifndef MAILCORE_C_CERTIFICATEUTILS_H +#define MAILCORE_C_CERTIFICATEUTILS_H + +#include "CBase.h" +#include "CArray.h" +#include "MailCoreString.h" + +#ifdef __cplusplus +extern "C" { +#endif + + /// Verifies a DER-encoded certificate chain (leaf first, CArray of CData) for `hostname`: + /// the chain must lead to a trusted root and the leaf must be issued for `hostname`, which + /// may be a DNS name or an IPv4/IPv6 literal. + /// + /// `derTrustAnchors` (CArray of CData): when its instance is non-NULL these roots are + /// trusted *instead of* the system store. `verifyTime`: Unix time to evaluate validity at, + /// 0 means now. + CMAILCORE_EXPORT bool CCertificateUtils_checkCertificateChain(CArray derCertificates, + MailCoreString hostname, + CArray derTrustAnchors, + int64_t verifyTime); + +#ifdef __cplusplus +} +#endif + +#endif /* MAILCORE_C_CERTIFICATEUTILS_H */ diff --git a/src/cmake/public-headers.cmake b/src/cmake/public-headers.cmake index 9a28f17a2..4bd260ee4 100644 --- a/src/cmake/public-headers.cmake +++ b/src/cmake/public-headers.cmake @@ -82,6 +82,7 @@ set(MAILCORE2_CORE_HEADERS core/smtp/MCSMTP.h core/smtp/MCSMTPProgressCallback.h core/smtp/MCSMTPSession.h + core/security/MCCertificateUtils.h ) set(MAILCORE2_ASYNC_HEADERS diff --git a/src/core/imap/MCIMAPSession.cpp b/src/core/imap/MCIMAPSession.cpp index 59b44d044..1be5c3fd8 100644 --- a/src/core/imap/MCIMAPSession.cpp +++ b/src/core/imap/MCIMAPSession.cpp @@ -679,7 +679,16 @@ void IMAPSession::connect(ErrorCode * pError) goto close; } +#if __APPLE__ r = mailimap_socket_starttls(mImap); +#else + // Passing callback to set the server name into SSL context + // Needed for SNI extension for TLS https://en.wikipedia.org/wiki/Server_Name_Indication + // On Apple platforms libetpan uses CFNetwork instead, that's why callback is not needed + r = mailimap_socket_starttls_with_callback(mImap, + setMailStreamSSLContextServerName, + const_cast(static_cast(MCUTF8(mHostname)))); +#endif if (hasError(r)) { MCLog("no TLS %i", r); * pError = ErrorTLSNotAvailable; diff --git a/src/core/nntp/MCNNTPSession.cpp b/src/core/nntp/MCNNTPSession.cpp index 4af470552..0c5a6cf78 100644 --- a/src/core/nntp/MCNNTPSession.cpp +++ b/src/core/nntp/MCNNTPSession.cpp @@ -25,6 +25,12 @@ using namespace mailcore; +#if !__APPLE__ +static void setMailStreamSSLContextServerName(mailstream_ssl_context * ssl_context, void * data) { + mailstream_ssl_set_server_name(ssl_context, static_cast(data)); +} +#endif + static int xover_resp_to_fields(struct newsnntp_xover_resp_item * item, struct mailimf_fields ** result); enum { @@ -282,7 +288,18 @@ void NNTPSession::connect(ErrorCode * pError) case ConnectionTypeTLS: MCLog("connect %s %u", MCUTF8(hostname()), (unsigned int) port()); +#if __APPLE__ r = newsnntp_ssl_connect(mNNTP, MCUTF8(hostname()), port()); +#else + // Passing callback to set the server name into SSL context + // Needed for SNI extension for TLS https://en.wikipedia.org/wiki/Server_Name_Indication + // On Apple platforms libetpan uses CFNetwork instead, that's why callback is not needed + r = newsnntp_ssl_connect_with_callback(mNNTP, + MCUTF8(hostname()), + port(), + setMailStreamSSLContextServerName, + const_cast(static_cast(MCUTF8(hostname())))); +#endif if (r != NEWSNNTP_NO_ERROR) { * pError = ErrorConnection; return; diff --git a/src/core/pop/MCPOPSession.cpp b/src/core/pop/MCPOPSession.cpp index 4e9b77515..b5d2b202a 100644 --- a/src/core/pop/MCPOPSession.cpp +++ b/src/core/pop/MCPOPSession.cpp @@ -13,6 +13,12 @@ using namespace mailcore; +#if !__APPLE__ +static void setMailStreamSSLContextServerName(mailstream_ssl_context * ssl_context, void * data) { + mailstream_ssl_set_server_name(ssl_context, static_cast(data)); +} +#endif + enum { STATE_DISCONNECTED, STATE_CONNECTED, @@ -223,7 +229,16 @@ void POPSession::connect(ErrorCode * pError) } MCLog("start TLS"); +#if __APPLE__ r = mailpop3_socket_starttls(mPop); +#else + // Passing callback to set the server name into SSL context + // Needed for SNI extension for TLS https://en.wikipedia.org/wiki/Server_Name_Indication + // On Apple platforms libetpan uses CFNetwork instead, that's why callback is not needed + r = mailpop3_socket_starttls_with_callback(mPop, + setMailStreamSSLContextServerName, + const_cast(static_cast(MCUTF8(hostname())))); +#endif if (r != MAILPOP3_NO_ERROR) { * pError = ErrorStartTLSNotAvailable; return; @@ -237,7 +252,18 @@ void POPSession::connect(ErrorCode * pError) case ConnectionTypeTLS: MCLog("connect %s %u", MCUTF8(hostname()), (unsigned int) port()); +#if __APPLE__ r = mailpop3_ssl_connect(mPop, MCUTF8(hostname()), port()); +#else + // Passing callback to set the server name into SSL context + // Needed for SNI extension for TLS https://en.wikipedia.org/wiki/Server_Name_Indication + // On Apple platforms libetpan uses CFNetwork instead, that's why callback is not needed + r = mailpop3_ssl_connect_with_callback(mPop, + MCUTF8(hostname()), + port(), + setMailStreamSSLContextServerName, + const_cast(static_cast(MCUTF8(hostname())))); +#endif if (r != MAILPOP3_NO_ERROR) { * pError = ErrorConnection; return; diff --git a/src/core/security/MCCertificateUtils.cpp b/src/core/security/MCCertificateUtils.cpp index f0efbad86..3a7d473f4 100644 --- a/src/core/security/MCCertificateUtils.cpp +++ b/src/core/security/MCCertificateUtils.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #endif @@ -65,6 +66,23 @@ char* X509_to_string(X509* cert) { bool mailcore::checkCertificate(mailstream * stream, String * hostname) { + carray * cCerts = mailstream_get_certificate_chain(stream); + if (cCerts == NULL) { + fprintf(stderr, "warning: No certificate chain retrieved"); + return false; + } + bool result = checkCertificateChain(cCerts, hostname, NULL, 0); + mailstream_certificate_chain_free(cCerts); + return result; +} + +bool mailcore::checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime) +{ + if (hostname == NULL || hostname->length() == 0) { + MCLog("MCCertificateUtils error: no hostname to verify the certificate against"); + return false; + } + #if __APPLE__ bool result = false; CFStringRef hostnameCFString; @@ -74,12 +92,6 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) SecTrustResultType trustResult; OSStatus status; - carray * cCerts = mailstream_get_certificate_chain(stream); - if (cCerts == NULL) { - fprintf(stderr, "warning: No certificate chain retrieved"); - goto err; - } - hostnameCFString = CFStringCreateWithCharacters(NULL, (const UniChar *) hostname->unicodeCharacters(), hostname->length()); policy = SecPolicyCreateSSL(true, hostnameCFString); @@ -90,6 +102,11 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) str = (MMAPString *) carray_get(cCerts, i); CFDataRef data = CFDataCreate(NULL, (const UInt8 *) str->str, (CFIndex) str->len); SecCertificateRef cert = SecCertificateCreateWithData(NULL, data); + if (cert == NULL) { + MCLog("MCCertificateUtils error: certificate %u is not a valid DER certificate", i); + CFRelease(data); + goto free_certs; + } CFArrayAppendValue(certificates, cert); CFRelease(data); CFRelease(cert); @@ -106,6 +123,30 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) goto free_certs; } + if (cTrustAnchors != NULL) { + // Tests: trust these roots in addition to the system store. + CFMutableArrayRef anchors = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); + for(unsigned int i = 0 ; i < carray_count(cTrustAnchors) ; i ++) { + MMAPString * str = (MMAPString *) carray_get(cTrustAnchors, i); + CFDataRef data = CFDataCreate(NULL, (const UInt8 *) str->str, (CFIndex) str->len); + SecCertificateRef anchor = SecCertificateCreateWithData(NULL, data); + if (anchor != NULL) { + CFArrayAppendValue(anchors, anchor); + CFRelease(anchor); + } + CFRelease(data); + } + SecTrustSetAnchorCertificates(trust, anchors); + SecTrustSetAnchorCertificatesOnly(trust, false); + CFRelease(anchors); + } + if (verifyTime != 0) { + // Tests: evaluate validity at a fixed point in time. + CFDateRef verifyDate = CFDateCreate(NULL, (CFAbsoluteTime) verifyTime - kCFAbsoluteTimeIntervalSince1970); + SecTrustSetVerifyDate(trust, verifyDate); + CFRelease(verifyDate); + } + status = SecTrustEvaluate(trust, &trustResult); if (status != noErr) { MC_UNLOCK(&lock); @@ -129,10 +170,8 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) CFRelease(trust); free_certs: CFRelease(certificates); - mailstream_certificate_chain_free(cCerts); CFRelease(policy); CFRelease(hostnameCFString); -err: return result; #else bool result = false; @@ -146,12 +185,6 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) #endif int status; - carray * cCerts = mailstream_get_certificate_chain(stream); - if (cCerts == NULL) { - fprintf(stderr, "warning: No certificate chain retrieved"); - goto err; - } - store = X509_STORE_new(); if (store == NULL) { goto free_certs; @@ -201,6 +234,19 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) MCLog("Error loading the system-wide CA certificates"); } + if (cTrustAnchors != NULL) { + // Tests: trust these roots in addition to the system store. + for(unsigned int i = 0 ; i < carray_count(cTrustAnchors) ; i ++) { + MMAPString * str = (MMAPString *) carray_get(cTrustAnchors, i); + const unsigned char * p = (const unsigned char *) str->str; + X509 * anchor = d2i_X509(NULL, &p, (long) str->len); + if (anchor != NULL) { + X509_STORE_add_cert(store, anchor); + X509_free(anchor); + } + } + } + certificates = sk_X509_new_null(); for(unsigned int i = 0 ; i < carray_count(cCerts) ; i ++) { MMAPString * str; @@ -213,6 +259,10 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) BIO *bio = BIO_new_mem_buf((void *) str->str, str->len); X509 *certificate = d2i_X509_bio(bio, NULL); BIO_free(bio); + if (certificate == NULL) { + MCLog("MCCertificateUtils error: certificate %u is not a valid DER certificate", i); + goto free_certs; + } if (!sk_X509_push(certificates, certificate)) { MCLog("MCCertificateUtils error: can't sk_X509_push"); goto free_certs; @@ -233,13 +283,38 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) goto free_certs; } + // COR-170: X509_verify_cert() only validates the chain. It does not check that the + // certificate was issued for the host we are connecting to (CWE-297), so a valid + // certificate for any other domain would be accepted. Attach the expected identity to + // the verification parameters so the hostname (or IP literal) is checked as part of + // X509_verify_cert(), matching SecPolicyCreateSSL(true, hostname) on Apple platforms. + { + const char * host = hostname->UTF8Characters(); + X509_VERIFY_PARAM * param = X509_STORE_CTX_get0_param(storectx); + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + // set1_ip_asc() succeeds only when host parses as an IPv4/IPv6 literal and leaves + // the parameters untouched otherwise, so fall back to a DNS name match. + if (X509_VERIFY_PARAM_set1_ip_asc(param, host) != 1) { + if (X509_VERIFY_PARAM_set1_host(param, host, 0) != 1) { + MCLog("MCCertificateUtils error: can't set expected hostname %s", host); + goto free_certs; + } + } + if (verifyTime != 0) { + // Tests: evaluate validity at a fixed point in time. + X509_VERIFY_PARAM_set_time(param, verifyTime); + } + } + ERR_clear_error(); status = X509_verify_cert(storectx); if (status == 1) { result = true; } else { - MCLog("MCCertificateUtils error: X509_verify_cert status %d", status); + int verifyError = X509_STORE_CTX_get_error(storectx); + MCLog("MCCertificateUtils error: X509_verify_cert status %d: %s (depth %d)", status, + X509_verify_cert_error_string(verifyError), X509_STORE_CTX_get_error_depth(storectx)); unsigned long errCode; while ((errCode = ERR_get_error()) != 0) { char *errMsg = ERR_error_string(errCode, NULL); @@ -248,7 +323,6 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) } free_certs: - mailstream_certificate_chain_free(cCerts); if (certificates != NULL) { sk_X509_pop_free((STACK_OF(X509) *) certificates, X509_free); } @@ -258,7 +332,6 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) if (store != NULL) { X509_STORE_free(store); } -err: return result; #endif } diff --git a/src/core/security/MCCertificateUtils.h b/src/core/security/MCCertificateUtils.h index d5b926793..9b67eb56e 100644 --- a/src/core/security/MCCertificateUtils.h +++ b/src/core/security/MCCertificateUtils.h @@ -10,13 +10,24 @@ #define MAILCORE_MCCERTIFICATEUTILS_H +#include #include #include +#ifdef __cplusplus + namespace mailcore { bool checkCertificate(mailstream * stream, String * hostname); + // The verification behind checkCertificate(), on a DER chain (carray of MMAPString, + // leaf first) instead of a stream. `cTrustAnchors` (same layout) are trusted in + // addition to the system store and `verifyTime` (Unix time, 0 = now) pins the + // validity check; both exist for unit tests, production passes NULL and 0 (COR-170). + bool checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime); + } #endif + +#endif diff --git a/src/core/smtp/MCSMTPSession.cpp b/src/core/smtp/MCSMTPSession.cpp index fa5b0cee5..3e1a999c6 100644 --- a/src/core/smtp/MCSMTPSession.cpp +++ b/src/core/smtp/MCSMTPSession.cpp @@ -330,7 +330,16 @@ void SMTPSession::connect(ErrorCode * pError) } MCLog("start TLS"); +#if __APPLE__ r = mailsmtp_socket_starttls(mSmtp); +#else + // Passing callback to set the server name into SSL context + // Needed for SNI extension for TLS https://en.wikipedia.org/wiki/Server_Name_Indication + // On Apple platforms libetpan uses CFNetwork instead, that's why callback is not needed + r = mailsmtp_socket_starttls_with_callback(mSmtp, + setMailStreamSSLContextServerName, + const_cast(static_cast(MCUTF8(mHostname)))); +#endif saveLastResponse(); if (r != MAILSMTP_NO_ERROR) { * pError = ErrorStartTLSNotAvailable; diff --git a/src/include/MailCore/CCertificateUtils.h b/src/include/MailCore/CCertificateUtils.h new file mode 100644 index 000000000..213662647 --- /dev/null +++ b/src/include/MailCore/CCertificateUtils.h @@ -0,0 +1,36 @@ +// +// CCertificateUtils.h +// mailcore2 +// +// C entry point to mailcore::checkCertificateChain(), so the certificate verification every +// session performs after the TLS handshake can be exercised from Swift unit tests (COR-170). +// + +#ifndef MAILCORE_C_CERTIFICATEUTILS_H +#define MAILCORE_C_CERTIFICATEUTILS_H + +#include "CBase.h" +#include "CArray.h" +#include "MailCoreString.h" + +#ifdef __cplusplus +extern "C" { +#endif + + /// Verifies a DER-encoded certificate chain (leaf first, CArray of CData) for `hostname`: + /// the chain must lead to a trusted root and the leaf must be issued for `hostname`, which + /// may be a DNS name or an IPv4/IPv6 literal. + /// + /// `derTrustAnchors` (CArray of CData): when its instance is non-NULL these roots are + /// trusted *instead of* the system store. `verifyTime`: Unix time to evaluate validity at, + /// 0 means now. + CMAILCORE_EXPORT bool CCertificateUtils_checkCertificateChain(CArray derCertificates, + MailCoreString hostname, + CArray derTrustAnchors, + int64_t verifyTime); + +#ifdef __cplusplus +} +#endif + +#endif /* MAILCORE_C_CERTIFICATEUTILS_H */ diff --git a/src/include/MailCore/MCCertificateUtils.h b/src/include/MailCore/MCCertificateUtils.h new file mode 100644 index 000000000..9b67eb56e --- /dev/null +++ b/src/include/MailCore/MCCertificateUtils.h @@ -0,0 +1,33 @@ +// +// MCCertificateUtils.h +// mailcore2 +// +// Created by DINH Viêt Hoà on 7/25/13. +// Copyright (c) 2013 MailCore. All rights reserved. +// + +#ifndef MAILCORE_MCCERTIFICATEUTILS_H + +#define MAILCORE_MCCERTIFICATEUTILS_H + +#include +#include +#include + +#ifdef __cplusplus + +namespace mailcore { + + bool checkCertificate(mailstream * stream, String * hostname); + + // The verification behind checkCertificate(), on a DER chain (carray of MMAPString, + // leaf first) instead of a stream. `cTrustAnchors` (same layout) are trusted in + // addition to the system store and `verifyTime` (Unix time, 0 = now) pins the + // validity check; both exist for unit tests, production passes NULL and 0 (COR-170). + bool checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime); + +} + +#endif + +#endif diff --git a/unittest/CertificateUtilsTests.swift b/unittest/CertificateUtilsTests.swift new file mode 100644 index 000000000..1346cdbbd --- /dev/null +++ b/unittest/CertificateUtilsTests.swift @@ -0,0 +1,146 @@ +// +// CertificateUtilsTests.swift +// mailcore2 +// +// Tests for mailcore::checkCertificateChain(), the verification every IMAP/SMTP/POP/NNTP +// session runs on the server certificate right after the TLS handshake (COR-170). +// + +import Foundation +import XCTest + +#if SWIFT_PACKAGE +import CMailCore +#endif + +@testable import MailCore + +/// Fixtures live in data/certificates and were generated with OpenSSL: +/// +/// - `ca.der` a private root, valid 2020-01-01 .. 2120-01-01 +/// - `leaf.der` issued by that root for `imap.example.test`, `*.wild.example.test`, +/// `10.1.2.3` and `2001:db8::1`; valid 2026-01-01 .. 2027-12-31 +/// - `other.der` an unrelated root that signed nothing here +/// +/// The root is passed to the verifier as the only trust anchor, so the tests do not depend on +/// the device's trust store, and verification is pinned to 2026-06-15 so they do not depend on +/// the clock either. The leaf's validity stays under 825 days on purpose: Apple's TLS policy +/// rejects longer-lived server certificates regardless of who issued them. +final class CertificateUtilsTests: XCTestCase { + + /// 2026-06-15T00:00:00Z, inside the leaf's validity period. + private static let verifyTime: Int64 = 1_781_481_600 + /// 2030-01-01T00:00:00Z, after the leaf expired. + private static let afterLeafExpiry: Int64 = 1_893_456_000 + + private var root = Data() + private var leaf = Data() + private var unrelatedRoot = Data() + + override func setUpWithError() throws { + try super.setUpWithError() + + #if os(Android) + let directory = Bundle.main.bundleURL.appendingPathComponent("resources/data/certificates") + #else + let directory = Bundle.module.resourceURL!.appendingPathComponent("data/certificates") + #endif + + root = try Data(contentsOf: directory.appendingPathComponent("ca.der")) + leaf = try Data(contentsOf: directory.appendingPathComponent("leaf.der")) + unrelatedRoot = try Data(contentsOf: directory.appendingPathComponent("other.der")) + } + + /// Runs the verifier the way a session does, with the given DER chain (leaf first). + private func verify(chain: [Data], + hostname: String, + anchors: [Data]? = nil, + at verifyTime: Int64 = CertificateUtilsTests.verifyTime) -> Bool { + return mailCoreAutoreleasePool { + let cChain = CArray_init() + for certificate in chain { + cChain.addObject(certificate.mailCoreData().toCObject()) + } + let cAnchors = CArray_init() + for anchor in anchors ?? [root] { + cAnchors.addObject(anchor.mailCoreData().toCObject()) + } + return CCertificateUtils_checkCertificateChain(cChain, hostname.mailCoreString(), cAnchors, verifyTime) + } + } + + // MARK: - Accepted + + func testAcceptsCertificateIssuedForHostname() { + XCTAssertTrue(verify(chain: [leaf, root], hostname: "imap.example.test")) + } + + func testAcceptsChainWithoutTheRootIncluded() { + // Servers commonly send only the leaf (and intermediates); the root comes from the store. + XCTAssertTrue(verify(chain: [leaf], hostname: "imap.example.test")) + } + + func testHostnameMatchIsCaseInsensitive() { + XCTAssertTrue(verify(chain: [leaf, root], hostname: "IMAP.Example.TEST")) + } + + func testAcceptsSingleLabelWildcardMatch() { + XCTAssertTrue(verify(chain: [leaf, root], hostname: "mail.wild.example.test")) + } + + func testAcceptsIPv4LiteralFromSubjectAlternativeName() { + XCTAssertTrue(verify(chain: [leaf, root], hostname: "10.1.2.3")) + } + + func testAcceptsIPv6LiteralFromSubjectAlternativeName() { + XCTAssertTrue(verify(chain: [leaf, root], hostname: "2001:db8::1")) + } + + // MARK: - Rejected: identity + + func testRejectsCertificateIssuedForAnotherHostname() { + // The COR-170 report: a valid, trusted certificate for a different host was accepted. + XCTAssertFalse(verify(chain: [leaf, root], hostname: "gmail-imap.l.google.com")) + } + + func testRejectsHostnameThatOnlySharesTheParentDomain() { + XCTAssertFalse(verify(chain: [leaf, root], hostname: "example.test")) + XCTAssertFalse(verify(chain: [leaf, root], hostname: "smtp.example.test")) + } + + func testRejectsWildcardSpanningSeveralLabels() { + XCTAssertFalse(verify(chain: [leaf, root], hostname: "a.b.wild.example.test")) + } + + func testRejectsWildcardParentDomainItself() { + XCTAssertFalse(verify(chain: [leaf, root], hostname: "wild.example.test")) + } + + func testRejectsIPLiteralNotInSubjectAlternativeName() { + XCTAssertFalse(verify(chain: [leaf, root], hostname: "10.1.2.4")) + XCTAssertFalse(verify(chain: [leaf, root], hostname: "2001:db8::2")) + } + + func testRejectsEmptyHostname() { + XCTAssertFalse(verify(chain: [leaf, root], hostname: "")) + } + + // MARK: - Rejected: chain + + func testRejectsChainNotLeadingToATrustedRoot() { + // Correct hostname, but the only trusted root did not issue the chain. + XCTAssertFalse(verify(chain: [leaf, root], hostname: "imap.example.test", anchors: [unrelatedRoot])) + } + + func testRejectsExpiredCertificate() { + XCTAssertFalse(verify(chain: [leaf, root], hostname: "imap.example.test", at: CertificateUtilsTests.afterLeafExpiry)) + } + + func testRejectsEmptyChain() { + XCTAssertFalse(verify(chain: [], hostname: "imap.example.test")) + } + + func testRejectsMalformedCertificate() { + XCTAssertFalse(verify(chain: [Data("not a certificate".utf8)], hostname: "imap.example.test")) + } +} diff --git a/unittest/data/certificates/ca.der b/unittest/data/certificates/ca.der new file mode 100644 index 0000000000000000000000000000000000000000..b210ad9614d5a40ab5ef70b1de4b5ae72b96d0b3 GIT binary patch literal 873 zcmXqLVoo(^V)9+U%*4pVBvO4M)IBw7>8$o8HqP<>ucD)CIT{Rj**LY@JlekVGBR?r zG8kAIiW`WsF^95n@$mU3X688O7o{qMq!yPH8_F5TfFziCL{TIZg7WiA6r3Fm6vTOr z3=9l`5DcOu_>Bxv1Sm1QiBSpJsf?@)%uS5^33+dKUt z*TF46e3mpd-*{h`*?2UN%c!gSdey5YTeCBBgN@9Vtl3fixzAH&`Q$ol5%b@2jGEpp z`?e*gZQ_pajXxs#xW=e<`cl3hnG6X#mkEbo5WBH|&w_8Mm*2f-lHYP)+U6s7+TNb; z>(z|Jj`}GqQnpCp6t(Zx)el%Qzll#S;kUbQy4NayCWBbJqI15Ne@)nu;9$0(q~)3P z>u9sbb9V~ZSb1I5ox}3GxxXt^`PB>)Pk~j7Q``f;7Jk3f5PE)R$Yi-!^Pe)BpV+$n z#EFA9e$77?aoKv!O6M=9_W8>?-q8%P{`iYCZ}OIs$f>iJoDB$=xp=qZyW6avnV16LTjM2_ZNZSyFVKEF3b=eN@HGb=yxJia)i$u6074ijg=nd@`RH?^5AQE$0*f5%(% z<;qQue<*MtII!K@(so;jX$VW~(XYoQf1Z`1&b!>8H@#AE^D~15ch%zz1LX=fiGJpq zDx@RBD_-2};`}D-k?|)`le)Lye?)IhT{uJj6e|h?466_+SSN?PF r`Sxw+>4rf0#3_l68@K;rR;t;md17j!@%QVc>9zcAyLshYq6#Dc1p`h4_s^KzzCc8szCFB_*;n@8JsUPeZ4 zRt5u0LvaI9Hs(+kE*?JL#LOJ${GwEakksOmVnaCt8IS}sk0^?SLQsBwiGs7Eft)z6 zk(q%Z5Q0GzkZW#eWDG>6mZp|b2Bx@;mNJll7%iBYn^>TiT9KGrkdvxc0<_nliBSpJ zk&LVi%uS5^3_x)%rY1&4hHHW^Be(6?0i4X_u9>T zYity6=e2$ox>YwX(%`JXww|YHdDePGaVzp-yGwntZPfeMYXs~rcC}h}{(a=uZs+qa zeJ0(`Ud^#pK{2E!UCMQWbnE)S)1i-kp7Tx9yDYn6jfcE$!@sFF`I3G9)ww4znyf5r zF5d9r;~Wm#aN{(s&h&{h7L|*YrZ_RFMo)kBu&sa7+MoGM-pXrMxrn?nE`Cz7EM?=t zAB|3LCjFXIQQUL&h}r6P{|)c;p7|}kb&!dfk%4h><35AN-3Fe(P?Qy9aWZgd!VJkK zVJ*G#%$yXI;B9B&Vq{`&7f@j2-NAqWj0T(_ql8(Q3>XafKumr{#{VoV%uK8c41_^^ zRTdrtE;bHrHbz!fc4kHcS&$$fix`Uto8J3BCp>pN*gEZKuy6DWgS8WKgbd_C(#k9n z24W2&owhsP895xdawCdmmZlK@?n_TAJ|jmlFdYJ;n2~{D_uZhhE~gdW^|cZWSFZM8 zd+Q}4Bzz-B#KbAVFY2*}f^*^Dy?@VNU@~o(7`J^@ShrRF!?mkcs@>(C8n(LaVcYw+ zl^Y~7cUOq%CD3b=-ydM^LxB%`+4d22Q+ z(W%Sl?9bYAPFXqaf9LnumfNiUw{A+$I8@>O;aBN26LqogH;>r#E!f*?YE!}eTrqV` z|MlBbg&(ubww=%2uap0prQ$@g0EbejklShBlvfqOXO-D*&Uwc6wsHP(t$PU?F85xp xFwW8WR<`SQ>C~hFi#vIDR#lz>Dn3?08UzDm4l3H9+Y$#As9qS@EaMT2#{-V6QdHc6B$_@M_bj_fGelvzn*dt>oFi?py}fBDF)D>pE|yo@ae~UG1^%p5+Vb z4{pDz?EStjJg-@@dU5`;HJN2C>kCWbdK0V+?ta@#UcL{mP4{B`W|J^6rA@rL?epe`k{c|2b|z*Q-Rfj}cyId0xv%5q zTNKvlTNdv*^lDXKH|u3v#toCN+-5Kpxp?l1y`$xkDMAI@%&(9Ae83)`cRkI>tUPF| zDHAgz1LNXkgG2*aVC2a1v52vV{N{XDZCTX3wR5FWkNDTwCs;i`?>CSKNh`BR7>G4s zSHKTaAk4`4pM}+c8AuuMfdu$L0xZl-Of1OJ08A;sXkcX6?AKW2VJdW4;+0>N$fNfX zJ9jEJKiuB(B=feMXTce@M}NiDolRLB7Hh0uv5YsMWtY*v-VRw6kwV4hZ!ZnmzqQQT z`t_~S`6h|o^;254Z2JXlSAITpU0aLU+pTO_T9wF?$3kl+uSf{dy1F3lwZfD~H#=u} zeBU{5>rU^I`ApYsl(S8iSv=qWd&SqsuWlaTdVPy+y6-HFI}@AkZMwWe!_s(#Nks_j zqy^0)!SAFNEI9PrqWlIMzna;tU+u9|ZvXIW+Ue0@`fi6qPl3|QYbhZsCrTVEOc%?` y*e+c)S8#4b{83dU4$DpL_a^>IO4>PP$-Ia1`>w~RMgQ58)n;`pYkyp2m;nIzB3(fM literal 0 HcmV?d00001 From 844837f56a1e74cd302f4ac2d742b92d9c5074f7 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Mon, 7 Sep 2026 12:54:32 +0300 Subject: [PATCH 2/5] Fix: verify TLS certificate hostnames on non-Apple platforms COR-170 --- src/CMakeLists.txt | 1 + src/c/utils/CCertificateUtils.h | 2 +- src/core/security/MCCertificateUtils.h | 2 +- src/include/MailCore/CCertificateUtils.h | 2 +- src/include/MailCore/MCCertificateUtils.h | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6b2c5a807..50a623b39 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -114,6 +114,7 @@ if(WIN32) target_link_libraries(CMailCore PRIVATE ${MAILCORE2_LIB_TARGET} + ${LIBETPAN_LIBRARY} ${DISPATCH_LIBRARY} ${DISPATCH_BLOCKS_LIBRARY} ) diff --git a/src/c/utils/CCertificateUtils.h b/src/c/utils/CCertificateUtils.h index 213662647..b61213757 100644 --- a/src/c/utils/CCertificateUtils.h +++ b/src/c/utils/CCertificateUtils.h @@ -22,7 +22,7 @@ extern "C" { /// may be a DNS name or an IPv4/IPv6 literal. /// /// `derTrustAnchors` (CArray of CData): when its instance is non-NULL these roots are - /// trusted *instead of* the system store. `verifyTime`: Unix time to evaluate validity at, + /// trusted in addition to the system store. `verifyTime`: Unix time to evaluate validity at, /// 0 means now. CMAILCORE_EXPORT bool CCertificateUtils_checkCertificateChain(CArray derCertificates, MailCoreString hostname, diff --git a/src/core/security/MCCertificateUtils.h b/src/core/security/MCCertificateUtils.h index 9b67eb56e..b542dde31 100644 --- a/src/core/security/MCCertificateUtils.h +++ b/src/core/security/MCCertificateUtils.h @@ -24,7 +24,7 @@ namespace mailcore { // leaf first) instead of a stream. `cTrustAnchors` (same layout) are trusted in // addition to the system store and `verifyTime` (Unix time, 0 = now) pins the // validity check; both exist for unit tests, production passes NULL and 0 (COR-170). - bool checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime); + MAILCORE_EXPORT bool checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime); } diff --git a/src/include/MailCore/CCertificateUtils.h b/src/include/MailCore/CCertificateUtils.h index 213662647..b61213757 100644 --- a/src/include/MailCore/CCertificateUtils.h +++ b/src/include/MailCore/CCertificateUtils.h @@ -22,7 +22,7 @@ extern "C" { /// may be a DNS name or an IPv4/IPv6 literal. /// /// `derTrustAnchors` (CArray of CData): when its instance is non-NULL these roots are - /// trusted *instead of* the system store. `verifyTime`: Unix time to evaluate validity at, + /// trusted in addition to the system store. `verifyTime`: Unix time to evaluate validity at, /// 0 means now. CMAILCORE_EXPORT bool CCertificateUtils_checkCertificateChain(CArray derCertificates, MailCoreString hostname, diff --git a/src/include/MailCore/MCCertificateUtils.h b/src/include/MailCore/MCCertificateUtils.h index 9b67eb56e..b542dde31 100644 --- a/src/include/MailCore/MCCertificateUtils.h +++ b/src/include/MailCore/MCCertificateUtils.h @@ -24,7 +24,7 @@ namespace mailcore { // leaf first) instead of a stream. `cTrustAnchors` (same layout) are trusted in // addition to the system store and `verifyTime` (Unix time, 0 = now) pins the // validity check; both exist for unit tests, production passes NULL and 0 (COR-170). - bool checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime); + MAILCORE_EXPORT bool checkCertificateChain(carray * cCerts, String * hostname, carray * cTrustAnchors, time_t verifyTime); } From db938a96b81ebf5506e316d322687237647c3191 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Mon, 7 Sep 2026 13:39:54 +0300 Subject: [PATCH 3/5] Test: run IMAPInterruptCurrentCommandTests on Android, fix trust-anchor comment COR-170 - IMAPInterruptCurrentCommandTests only needed Darwin for the POSIX socket API; import Android/Glibc instead so the suite runs on Android and Linux too. - CertificateUtilsTests: the private root is trusted in addition to the system store, not as the only anchor (Copilot review remark on PR #108). Co-Authored-By: Claude Fable 5.1 --- unittest/CertificateUtilsTests.swift | 9 ++++--- .../IMAPInterruptCurrentCommandTests.swift | 27 +++++++++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/unittest/CertificateUtilsTests.swift b/unittest/CertificateUtilsTests.swift index 1346cdbbd..60f76cb58 100644 --- a/unittest/CertificateUtilsTests.swift +++ b/unittest/CertificateUtilsTests.swift @@ -22,10 +22,11 @@ import CMailCore /// `10.1.2.3` and `2001:db8::1`; valid 2026-01-01 .. 2027-12-31 /// - `other.der` an unrelated root that signed nothing here /// -/// The root is passed to the verifier as the only trust anchor, so the tests do not depend on -/// the device's trust store, and verification is pinned to 2026-06-15 so they do not depend on -/// the clock either. The leaf's validity stays under 825 days on purpose: Apple's TLS policy -/// rejects longer-lived server certificates regardless of who issued them. +/// The private root is handed to the verifier as an extra trust anchor. It is trusted in addition +/// to the device's store, not instead of it, but nothing in that store issued these fixtures, so +/// the outcome does not depend on the device. Verification is pinned to 2026-06-15 so it does not +/// depend on the clock either. The leaf's validity stays under 825 days on purpose: Apple's TLS +/// policy rejects longer-lived server certificates regardless of who issued them. final class CertificateUtilsTests: XCTestCase { /// 2026-06-15T00:00:00Z, inside the leaf's validity period. diff --git a/unittest/IMAPInterruptCurrentCommandTests.swift b/unittest/IMAPInterruptCurrentCommandTests.swift index 427fe09c2..231ea9739 100644 --- a/unittest/IMAPInterruptCurrentCommandTests.swift +++ b/unittest/IMAPInterruptCurrentCommandTests.swift @@ -5,11 +5,17 @@ // Tests for IMAPOperation::interruptCurrentCommand(). // -// Darwin only: the tests need a POSIX listening socket, and the Android job builds the test target -// without running it. Nothing here is platform-specific beyond that socket. -#if canImport(Darwin) +// The tests need a POSIX listening socket, so they run wherever one is available: Apple platforms, +// Android and Linux. Windows would need Winsock and is left out. +#if canImport(Darwin) || canImport(Android) || canImport(Glibc) +#if canImport(Darwin) import Darwin +#elseif canImport(Android) +import Android +#elseif canImport(Glibc) +import Glibc +#endif import Dispatch import Foundation import XCTest @@ -20,6 +26,11 @@ import CMailCore @testable import MailCore +/// The C `close()`, reachable by one name on every platform above. +private func closeSocket(_ fileDescriptor: Int32) { + _ = close(fileDescriptor) +} + /// A TCP endpoint that accepts connections and then says nothing at all. A client connected to it /// sits in its first read until the socket timeout expires - exactly the state that /// `interruptCurrentCommand()` has to break, and reproducible without an IMAP server. @@ -56,7 +67,7 @@ private final class SilentTCPEndpoint { } guard bound == 0, listen(fileDescriptor, 8) == 0 else { - close(fileDescriptor) + closeSocket(fileDescriptor) throw NSError(domain: "SilentTCPEndpoint", code: Int(errno), userInfo: nil) } @@ -69,7 +80,7 @@ private final class SilentTCPEndpoint { } guard named == 0 else { - close(fileDescriptor) + closeSocket(fileDescriptor) throw NSError(domain: "SilentTCPEndpoint", code: Int(errno), userInfo: nil) } @@ -93,7 +104,7 @@ private final class SilentTCPEndpoint { let closed = isClosed if closed { lock.unlock() - Darwin.close(accepted) + closeSocket(accepted) return } acceptedSockets.append(accepted) @@ -112,9 +123,9 @@ private final class SilentTCPEndpoint { acceptedSockets = [] lock.unlock() - Darwin.close(listeningSocket) + closeSocket(listeningSocket) for accepted in sockets { - Darwin.close(accepted) + closeSocket(accepted) } } } From 2bb23b9de444b66d7794e536e54296a88ae4829e Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Mon, 7 Sep 2026 13:44:10 +0300 Subject: [PATCH 4/5] Test: install MailCore's main queue for IMAPInterruptCurrentCommandTests on Android COR-170 MailCore's Android build has no process-wide main queue and Object::getMainQueue() aborts unless the app installed one through MCOOperation.setMainQueue(); in the XCTest process the test has to do it. Seen on the first Android run: the suite crashed with SIGABRT. Co-Authored-By: Claude Fable 5.1 --- unittest/IMAPInterruptCurrentCommandTests.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/unittest/IMAPInterruptCurrentCommandTests.swift b/unittest/IMAPInterruptCurrentCommandTests.swift index 231ea9739..ced1cc7e2 100644 --- a/unittest/IMAPInterruptCurrentCommandTests.swift +++ b/unittest/IMAPInterruptCurrentCommandTests.swift @@ -132,6 +132,18 @@ private final class SilentTCPEndpoint { final class IMAPInterruptCurrentCommandTests: XCTestCase { + #if os(Android) + /// MailCore's Android build has no process-wide main queue: the app installs one through + /// MCOOperation.setMainQueue() and Object::getMainQueue() aborts when nobody did. In this + /// process the test is the app. Kept in a static so the queue outlives every operation. + private static let mailCoreMainQueue = DispatchQueue(label: "IMAPInterruptCurrentCommandTests.mailcore-main") + + override class func setUp() { + super.setUp() + MCOOperation.setMainQueue(mailCoreMainQueue) + } + #endif + /// Well above every wait below: a command left to its own devices must not be able to finish on /// its own and pass a test that is about being interrupted. private let sessionTimeout: TimeInterval = 60 From c87ff7d1531b2adb0a6fcac13ec5a1c509428099 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Mon, 7 Sep 2026 13:55:13 +0300 Subject: [PATCH 5/5] Fix: free the X509 when sk_X509_push fails in checkCertificateChain COR-170 The certificate was never pushed onto the stack, so sk_X509_pop_free() in the cleanup path could not release it. Inherited from upstream; surfaced by the Copilot review of PR #108. Only reachable on allocation failure. Co-Authored-By: Claude Fable 5.1 --- src/core/security/MCCertificateUtils.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/security/MCCertificateUtils.cpp b/src/core/security/MCCertificateUtils.cpp index 3a7d473f4..67e7695b1 100644 --- a/src/core/security/MCCertificateUtils.cpp +++ b/src/core/security/MCCertificateUtils.cpp @@ -265,6 +265,8 @@ bool mailcore::checkCertificateChain(carray * cCerts, String * hostname, carray } if (!sk_X509_push(certificates, certificate)) { MCLog("MCCertificateUtils error: can't sk_X509_push"); + // Not on the stack, so sk_X509_pop_free() below will not release it. + X509_free(certificate); goto free_certs; } }