COR-170 - Verify TLS certificate hostnames on non-Apple platforms - #107
COR-170 - Verify TLS certificate hostnames on non-Apple platforms#107dbezverkhnii wants to merge 565 commits into
Conversation
If MCONNTPDisconnectOperation inherits from NSObject, MCONNTPDisconnectOperation+mco_objectWithMCObject: will call a method that doesn’t exist (-initWithMCOperation:). Set it to extend MCONNTPOperation instead.
[Windows] General Windows support
[Windows] Fix time parse issue
[Windows] Fix win time parse issue, second attempt
[Windows] Faster build
[Windows] Use our ctemplate fork
# Conflicts: # src/swift/imap/IMAPOperation.swift
Fix race condition in IMAPOperation completionBlock
Fix build issues
Loose CMake requirements to v3.12
Fix possible deadlock with cancel
Swift6 Android
Move the Android CI pipeline to the Readdle Swift Android SDK based on Swift 6.2.1 and the new swift-android build tools. Update the runner from macos-13 to macos-latest, bump the emulator to API 36, and point libetpan at the 1.9.3-readdle.9 release branch.
Use .exact() instead of .branch() to ensure reproducible builds
Swift 6.2 Android support
Update `tidy-html5` dependency without deprecated headers
DSK-658: mailcore paths are UTF-8 (fileSystemRepresentation), but the Windows layer used narrow CRT calls (fopen_s, _unlink, _mkdir, stat) that resolve paths through the ANSI codepage. Any path with characters outside the codepage — e.g. a temp dir under a non-ASCII user profile — failed to open, breaking RFC822 temp files (MCOErrorFile) and stalling Outbox. Convert UTF-8 to UTF-16 and call _wfopen_s/_wunlink/_wmkdir/_wstat64i32, falling back to the narrow call when the input is not valid UTF-8. Also route MCDataStreamDecoder and MCCertificateUtils through MCWin32.h so their fopen calls hit the wrapper.
corecrt_io.h declares POSIX-compat unlink; when io.h is included after MCWin32.h, the unlink macro rewrites that declaration into extern "C" mailcore::win32_unlink and clang-cl rejects the language linkage mismatch. Pre-include io.h (and wchar.h for _wstat64i32) so all CRT declarations of the wrapped names precede the macros, matching how direct.h and sys/stat.h are already handled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DSK-658 - Support non ascii
…path fix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DSK-658 - Bump prebuild version
DSK-658 - Add instruction how build mailcore for windows
…ndependent-from-aws COR-181 - Make MailCore2 independent from AWS
…mmand-support COR-173 - Add IMAP interrupt command support
(cherry picked from commit fad23d7)
…-starttls-certificate-check COR-186 - Ensure checking SSL Certificate in IMAP StartTLS.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9b4b1c0. Configure here.
| goto free_certs; | ||
| } | ||
|
|
||
| ERR_clear_error(); |
There was a problem hiding this comment.
TLS hostnames not actually verified
High Severity
On non-Apple platforms checkCertificate never uses hostname. X509_verify_cert only validates the chain, so any trusted certificate is accepted. Implicit TLS only sets SNI via mailstream_ssl_set_server_name, which does not check the peer name. Clients remain open to MITM with a valid cert for a different host.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9b4b1c0. Configure here.
| * pError = ErrorCertificate; | ||
| goto close; | ||
| } | ||
|
|
There was a problem hiding this comment.
SNI missing on several TLS paths
High Severity
The new setMailStreamSSLContextServerName callback is used only for implicit TLS on IMAP and SMTP. STARTTLS (IMAP, SMTP, POP) and POP/NNTP implicit TLS still connect without a server name. Hosts that require SNI can present the wrong certificate, and those sessions never receive the hostname that was added for verification.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 9b4b1c0. Configure here.
| @@ -138,7 +176,7 @@ bool mailcore::checkCertificate(mailstream * stream, String * hostname) | |||
| CertCloseStore(systemStore, 0); | |||
| #elif defined(ANDROID) || defined(__ANDROID__) | |||
| dir = opendir("/system/etc/security/cacerts"); | |||
| while (ent = readdir(dir)) { | |||
| while ((ent = readdir(dir))) { | |||
There was a problem hiding this comment.
Windows cert decode corrupts context
High Severity
d2i_X509 is called with a pointer to nextCert->pbCertEncoded. That API advances the input pointer, so it overwrites the system CERT_CONTEXT buffer pointer during store enumeration. Later certificates can fail to load or the store walk can break, so Windows TLS verification can miss trusted roots.
Reviewed by Cursor Bugbot for commit 9b4b1c0. Configure here.
15 participants
Note
Medium Risk
Large build and release-path changes (CMake, SPM, digest-gated Windows binaries) can break downstream Spark builds if prebuits or pins are missed; the diff shown is mostly infrastructure rather than runtime TLS logic.
Overview
This PR adds GitHub Actions pull-request checks for Android (
swift-android build --build-testswith Swift 6.2.1), macOS/iOS (build-scripts/test-*.shplus JUnit on failure), and a Windows prebuilt gate that runsCheck-PrebuiltPublished.ps1on the merge commit to ensure amailcore2-windows-<digest>.zipexists on thewindows-prebuiltrelease.Windows and CMake are expanded so mailcore2 can be built outside Apple platforms:
CMakeLists.txtnow targets WIN32 (OpenSSL, dispatch/BlocksRuntime, static libxml, shared libs), skipstests/unitteston Windows, and addsCMakeSettings.jsonfor Visual Studio. README and AGENTS.md document digest-based prebuilts (GitHub release assets instead of S3/SPARK_PREBUILT_KEY) and the publish/check scripts underbuild-windows-5.10.Swift Package Manager arrives via new
Package.swift(CMailCore+MailCore+ tests, Readdle dependency pins, Android-specific excludes).RSMMailCore.podspec(2.1.36) packages the same C/Swift stack for CocoaPods. Android Swift build support lands inbuild-android-swift/(NDKAndroid.mk, CMailCore header stubs, resource push script) with small tweaks to legacybuild-android/build.sh.Public macOS/iOS umbrella headers (
build-mac/OSX|iOS/MailCore.h) switch from importing the large Objective-CMC*surface to C API headers (CIndexSet,CIMAPAsyncSession, etc.), with matchingCMailCoremodule maps underbuild-mac/. Housekeeping includes.gitignoreupdates, Sourcery config forLinuxMain.swift, removal of pinned Travisosx_image, and.arcconfigfor Phabricator.Reviewed by Cursor Bugbot for commit 9b4b1c0. Bugbot is set up for automated code reviews on this repo. Configure here.