Add configurable minimum TLS version and IMAP parser limits#185
Conversation
`MailTLSConfiguration.makeClientConfiguration` builds on `TLSConfiguration.makeClientConfiguration()` and only overrides `certificateVerification` and `trustRoots`. It therefore inherits NIOSSL's default `minimumTLSVersion` of `.tlsv1` — TLS 1.0, which RFC 8996 deprecated in March 2021. There was no way for a caller to raise that floor. This adds `MailTLSMinimumVersion` and threads it through `IMAPServer`, `SMTPServer`, `IMAPConnection` and both SMTP TLS paths (implicit TLS and STARTTLS). The enum is deliberately SwiftMail's own type rather than NIOSSL's `TLSVersion`: SwiftMail imports NIOSSL without re-exporting it, so exposing `TLSVersion` in the public API would push NIOSSL onto every consumer. Behaviour change: the default floor moves from TLS 1.0 to TLS 1.2, the lowest version RFC 8996 still permits. Callers needing the old behaviour can pass `.tlsv1` explicitly. Happy to default to `.tlsv1` instead if you would rather keep this strictly additive. Callers who know their servers speak TLS 1.3 can now pass `.tlsv1_3` and make downgrades impossible regardless of what the server offers. Tests cover the default, each enum mapping, and that both server types carry the caller's choice. Full suite passes (325 tests).
`IMAPConnection.makeBootstrap` hardcodes the response parser to `messageAttributeLimit: .max` and `bodySizeLimit: .max`. A server controls both the length prefixes and the attribute count it sends, so a hostile or malfunctioning server can force unbounded allocation from a single response, and a caller has no way to bound it. `responseBufferLimit` does not cover this: it bounds the parser's working buffer, whereas these bound what a single response may declare. This is the class of issue that produced advisories in other IMAP clients — Ruby net-imap GHSA-j3g3-5qv5-52mj (unbounded literal size → memory exhaustion, fixed by adding a configurable `max_response_size`) and mutt CVE-2021-3657 ("reject excessively large IMAP literals"). Adds `IMAPParserLimits`, threaded through `IMAPServer` and `IMAPConnection`. Strictly additive: the defaults are `.max`/`.max`, exactly today's behaviour, so existing callers see no change. Callers who want bounds can now set them: IMAPServer( host: "imap.example.com", port: 993, parserLimits: IMAPParserLimits( bodySizeLimit: 64 * 1024 * 1024, messageAttributeLimit: 1024 ) ) `literalSizeLimit` is included for completeness — it was pinned to `IMAPDefaults.literalSizeLimit` with no way to adjust it. Bounded defaults would arguably be the better long-term choice, but that would silently break anyone fetching large attachments, so I left it to you whether to make that call separately. Tests cover defaults, partial overrides, and that IMAPServer carries the caller's choice. Full suite passes (331 tests).
odrobnik
left a comment
odrobnik
left a comment
There was a problem hiding this comment.
Requesting changes: the IMAP TLS floor is not honored for implicit TLS or spawned named/IDLE connections, spawned connections also bypass the configured parser limits, and the pinned response parser makes both advertised custom maxima effectively exclusive/off by one. SMTP's implicit and STARTTLS paths are consistently wired. Review was performed read-only; builds and tests were not run as requested.
Findings:
- HIGH
Sources/SwiftMail/IMAP/IMAPConnection.swift:47— Implicit-TLS IMAP connections never forward this value:makeConnectionBootstrapcallsmakeTLSHandlerwithoutminimumTLSVersion, so the handler silently uses its.tlsv1_2default. For example,IMAPServer(port: 993, minimumTLSVersion: .tlsv1_3)can still connect through a TLS-1.2-only endpoint, violating the caller's downgrade floor; choosing.tlsv1for a legacy TLS-1.0-only server likewise still fails because the actual floor remains 1.2. - HIGH
Sources/SwiftMail/IMAP/IMAPServer.swift:184— The configured TLS floor is passed only toprimaryConnection;makeIdleConnectionandmakeNamedConnectionomit it and construct connections with the.tlsv1_2default. A caller can successfully establish a TLS-1.3-only primary session and then haveconnection(named:)oridle(on:)negotiate TLS 1.2 through a downgraded endpoint, despite the server-level policy claiming to govern every transport. - HIGH
Sources/SwiftMail/IMAP/IMAPServer.swift:186— The configured parser limits are passed only toprimaryConnection; both spawned-connection factories omitparserLimitsand therefore use.default(.maxbody and attribute limits). A hostile server can send an oversized or attribute-heavy FETCH response on a named or dedicated IDLE connection and bypass the caller's memory-exhaustion guards entirely. - MEDIUM
Sources/SwiftMail/IMAP/IMAPParserLimits.swift:31— This property is documented as an inclusive maximum, but the pinned NIOIMAPResponseParserenforcessize < bodySizeLimit. Consequently, a caller setting a 64 MiB maximum has a valid body of exactly 64 MiB rejected withExceededMaximumBodySizeError, even though only data larger than the documented maximum should fail. - MEDIUM
Sources/SwiftMail/IMAP/IMAPParserLimits.swift:36— The pinned parser checksattributeCount < messageAttributeLimitagain when parsing the FETCH closing delimiter, making the effective maximum one less than this documented value. WithmessageAttributeLimit: 1, for example, a normal one-attributeFETCH (FLAGS (...))parses the attribute and then fails at), so callers cannot successfully receive the advertised maximum count.
Addresses the review on Cocoanetics#185. All five findings were reproduced before fixing; the two MEDIUM ones were confirmed against the pinned parser rather than taken on faith. HIGH — implicit TLS ignored minimumTLSVersion. makeConnectionBootstrap omitted the argument, so makeTLSHandler's `.tlsv1_2` default applied and IMAPServer(port: 993, minimumTLSVersion: .tlsv1_3) negotiated TLS 1.2 with a downgraded peer. Port 993 is the port every IMAPS client uses, so the floor was effectively inert for the common case. HIGH — makeIdleConnection/makeNamedConnection omitted both minimumTLSVersion and parserLimits, so a server configured with a TLS 1.3 floor and 64 MiB limits spawned IDLE and named connections with a TLS 1.2 floor and unbounded limits — on precisely the long-lived connections that sit and listen to whatever the server sends. MEDIUM ×2 — the limits are documented as maxima, but the parser enforces `size < bodySizeLimit` and `attributeCount < messageAttributeLimit`, the latter again on the FETCH closing delimiter. A 64 MiB limit rejected a 64 MiB body; messageAttributeLimit: 1 rejected a one-attribute FETCH. Rather than redefine "maximum" as "one less than this", the translation now adds one, saturating at .max where the value already means unbounded. It lives in IMAPParserLimits.makeParserOptions(bufferLimit:) so there is one place to get it right instead of one per call site — the previous arrangement, where each site spelled it out, was how only the primary connection ended up with limits at all. The root cause of all three HIGH findings is the same: security policy with default values. A default makes "forgot to pass the floor" indistinguishable from "chose TLS 1.2", and neither the compiler nor a reviewer can tell them apart. IMAPConnection's designated initializer and makeTLSHandler no longer default minimumTLSVersion or parserLimits, so omitting one is a build error. That immediately surfaced a fourth call site — the legacy useTLS convenience initializer — which neither grep nor the review had found. Tests. The reason this shipped is worth recording: MailTLSConfigurationTests asserts that makeClientConfiguration returns what it is given, and one of its cases was named "Raising the floor to TLS 1.3 makes a downgrade impossible". It was green the whole time. It could not have caught this, because the defect was not a wrong return value — it was that nobody passed the value in. A test that cannot produce the failure proves nothing about it. So the new tests assert at the level where the defect lived: - IMAPTLSFloorEnforcementTests spins up a real TLS endpoint pinned to maximumTLSVersion .tlsv1_2 and connects a real IMAPConnection to it. A TLS 1.3 floor must fail; a TLS 1.2 floor must succeed (the control — without it the first test would also pass against a closed port). Verified by reverting the fix: it fails with "an error was expected but none was thrown", and passes with it. - IMAPSecurityPolicyPropagationTests asserts on the spawned connections themselves. - IMAPParserLimitsTranslationTests covers the off-by-one and the saturation at .max. The misleading test name is corrected, and MailTLSConfigurationTests now says what it does and does not cover. 339 tests pass. Built and tested with Xcode's toolchain.
odrobnik
left a comment
odrobnik
left a comment
There was a problem hiding this comment.
Requesting changes. The five findings from the previous review are fixed at this head, but three blockers remain: bodySizeLimit is enforced per BODY attribute rather than across the single FETCH response that SwiftMail documents and may accumulate; an IDLE parser-limit violation leaves the socket and AsyncStream open while the decoder retains rejected bytes; and custom protocol-literal limits above the pinned handler's fixed 8 KiB aggregate cap fail depending on network fragmentation. Review was performed read-only; builds and tests were not run as requested.
Findings:
- HIGH
Sources/SwiftMail/IMAP/IMAPParserLimits.swift:31—bodySizeLimitis documented as the maximum body data in one response, but the pinned ResponseParser calls its size guard independently for every streaming BODY attribute and never tracks a response-wide total.FetchPartHandlerappends every streaming attribute until the FETCH finishes. WithIMAPParserLimits(bodySizeLimit: 64 * 1024 * 1024)and the default unbounded attribute count, a hostile server can return arbitrarily many 50 MiB BODY sections in one FETCH; every section passes the 64 MiB check while SwiftMail accumulates all of them into oneData, so the advertised memory-exhaustion guard remains bypassable. - HIGH
Sources/SwiftMail/IMAP/IMAPConnection+Connection.swift:59— Parser-limit errors are installed on IDLE connections, but they do not fail closed. During IDLE, an oversized BODY or over-limit attribute count makesIMAPClientHandlerfire an error;IdleHandler.errorCaughtonly fails its private promise, without finishing theAsyncStreamor closing the channel, and no task is awaiting that promise until a later DONE/checkpoint. The decoder retains the rejected response and each subsequent network read is appended before the same parse error is raised again (the post-decode buffer cap is skipped on throws), allowing a malicious peer to keep growing memory while the caller's IDLE stream hangs. - MEDIUM
Sources/SwiftMail/IMAP/IMAPParserLimits.swift:42—literalSizeLimitis exposed as a configurable maximum, but the pinnedIMAPClientHandlerconstructs its byte-to-message processor withmaximumBufferSize: IMAPDefaults.lineLengthLimit(8,192 bytes), regardless of these parser options or SwiftMail's 1 MiB response buffer limit. For example, a valid 16 KiB mailbox-name literal withliteralSizeLimit: 32 * 1024succeeds only if delivered complete enough to parse immediately; if network fragmentation leaves more than 8 KiB buffered before the literal completes, it throwsPayloadTooLargeErroreven though it is below every configured limit. Thus raised literal limits are not reliably honored.
The SwiftLint job has been failing on this branch since the first commit; I only noticed when CI mailed about it. Four of the seven violations came from the original PR commits, three from the review fixes. Neither round ran the linter locally, which the repo's CI runs with --strict. MailTLSMinimumVersion's cases are renamed tlsv1_1/tlsv1_2/tlsv1_3 -> tlsv11/tlsv12/tlsv13. identifier_name rejects the underscores, and rather than add an exception for a brand-new API, the cases now match NIOSSL's own spelling (TLSVersion.tlsv11/tlsv12/tlsv13) — which is what they map onto anyway. The API is unreleased, so the rename is free. Also: trailing comma in the parameterized test, and the TLS test server's nested types moved to file scope (nesting depth). Verified locally with swiftlint --strict: 0 violations in 262 files. 339 tests still pass.
|
@davyd15 Please be sure to comment and resolve the individual review items as you fix them (or tell your agent to do that) For a final merge all threads must be resolved and all CI be green. |
odrobnik
left a comment
odrobnik
left a comment
There was a problem hiding this comment.
Requesting changes. The TLS-floor propagation and prior inclusive-bound fixes are correct at this head, but the parser hardening still does not meet the PR's hostile-server memory-bounding goal. The three previously reported parser blockers remain, and the pipelined FETCH path also swallows parser-limit failures as an empty success while leaving the poisoned decoder and socket active. Review was performed read-only; builds and tests were not run as requested.
Findings:
- HIGH
Sources/SwiftMail/IMAP/IMAPParserLimits.swift:31—bodySizeLimitis documented as the maximum body data in one response, but the pinned ResponseParser checks each streaming BODY attribute independently and never tracks the response-wide total. FetchPartHandler appends every streaming attribute until the FETCH finishes. With a 64 MiB limit and the default unbounded attribute count, a hostile server can send arbitrarily many 50 MiB BODY sections in one FETCH; every section passes while SwiftMail accumulates them all into one Data, bypassing the advertised memory guard. - HIGH
Sources/SwiftMail/IMAP/IMAPConnection+Connection.swift:59— Parser-limit errors on an IDLE connection do not fail closed. An oversized BODY or over-limit attribute count makes IMAPClientHandler fire an error, but IdleHandler only fails its private promise; it neither finishes the AsyncStream nor closes the channel, and nothing awaits that promise until a later DONE/checkpoint. The decoder retains the rejected bytes and appends each subsequent network read before throwing again, so a malicious peer can keep growing memory while the caller's IDLE stream hangs. - MEDIUM
Sources/SwiftMail/IMAP/IMAPParserLimits.swift:42—literalSizeLimitcan be configured above 8 KiB, but the pinned IMAPClientHandler always uses an independent 8,192-byte aggregate buffer cap. A valid 16 KiB mailbox-name literal withliteralSizeLimit: 32 * 1024can parse if delivered nearly whole, yet fragmented delivery that leaves more than 8 KiB buffered throws PayloadTooLargeError despite remaining below both the configured literal limit and SwiftMail's response buffer limit. Raised literal limits are therefore network-fragmentation dependent and not reliably honored. - HIGH
Sources/SwiftMail/IMAP/IMAPConnection+Connection.swift:59— The pipelined FETCH path also mishandles the newly installed parser-limit errors. PipelinedCommandDispatcher fails all per-request promises, but collectPipelinedFetchResults catches every failure and returns an empty result; the method then removes the dispatcher and reports success without closing the still-active channel. Because the decoder retained the rejected response, later peer writes are appended and rethrow the same error, allowing continued memory growth and leaving subsequent commands on a poisoned connection.
|
@davyd15 and please excuse me inundating you with review feedback. This is a new automation that needs some more polishing. |
Addresses the four findings from the second and third reviews. They share one root: the limits advertised a bound the surrounding code did not keep. Each is reproduced by a test that fails without its fix. bodySizeLimit across the whole response (HIGH). ResponseParser checks each streaming section on its own and keeps no total, while FetchPartHandler and PipelinedFetchPartHandler append every section into one Data — so any number of 50 MiB sections passed a 64 MiB limit. The total could be counted in those two handlers, but then it would hold only for the two that exist today. The limit is a transport promise, so it is enforced on the transport: IMAPResponseLimitGuard sits directly behind the decoder, sums the byteCount each section declares, resets per message, and covers every consumer including future ones. Fail closed (HIGH ×2). The guard closes the channel on any limit violation — the peer has already proven itself, and the one thing that must not happen is to keep reading from it. That alone was not enough for IDLE: IdleHandler recorded channelInactive and errorCaught but never finished the AsyncStream, so "fail closed" would have meant socket shut, caller still hanging. It now finishes on both. The pipelined path swallowed every failure and returned an empty result, which is indistinguishable from "this message has no such section" — it now rethrows limit violations while still tolerating ordinary per-request failures, which is the point of pipelining. literalSizeLimit (MEDIUM). Not fixable here: IMAPClientHandler constructs its processor with maximumBufferSize: IMAPDefaults.lineLengthLimit, hardcoded. Above 8 KiB the outcome depends on how the network fragments the literal, and a configuration that depends on packet boundaries is not a limit. The initializer now rejects such values instead of accepting a promise it cannot keep. The error-close is deliberately narrow — only ExceededMaximumBodySizeError, ExceededMaximumMessageAttributesError and ExceededResponseBodySizeError. errorCaught sees unrelated failures too, and closing on those would change behaviour far outside this feature. Note on the tests: the first version of the fail-closed tests passed against an EmbeddedChannel that was never connected — isActive is false there to begin with, so they were green for the wrong reason. They now connect first and assert the precondition. Verified locally: 346 tests pass, swiftlint --strict reports 0 violations in 264 files.
|
Nothing to excuse — the automation earned its keep. Nine distinct findings across three rounds, and every one of them was real: I checked each against the code before touching it, and none turned out to be a false positive. Two were the kind that tests do not catch. The TLS floor was silently inert on port 993, the only port that matters for IMAPS — while a test named "Raising the floor to TLS 1.3 makes a downgrade impossible" sat there green, because it asserted that a factory returns what it is handed, and the defect was that nobody handed it the value. If it is useful signal while you tune the thing: the findings got sharper each round rather than repeating themselves, and the pipelined-FETCH one was genuinely new. The noise on this branch was mine, though: I pushed twice without answering the threads, the second time for a lint fix only. That is what turned one review into three. Threads get answered before anything lands here from now on. All findings are addressed at Two things worth your attention rather than mine: The fixes reach past the limits type.
Verified locally: 346 tests pass, |
The Swift workflow has been cancelled on this branch since ac5ba8b, and I only looked once all the review threads were closed. It was not supersession: build-macos hits its `timeout-minutes: 10` at "Test (macOS)" every time, on every head. main and other branches are green, so this is ours — mine, specifically, introduced with the TLS floor tests. Cause: `try? group.syncShutdownGracefully()` called straight from a swift-testing test blocks a cooperative-pool thread. On a core-constrained runner that violates the pool's forward-progress guarantee and deadlocks the whole run. Locally, on a machine with cores to spare, 346 tests finish in 1.6 seconds and nothing hangs — which is why it never showed up here. None of this is new. Cocoanetics#179 hit exactly this, documented it in IMAPResponseBufferLimitTests ("a 7-minute hang to the job timeout was observed"), and solved it by dispatching the blocking call to a non-cooperative GCD queue. I wrote the same trap next to the note warning about it. The fix is that same pattern, extracted to `shutDownGracefully(_:)` in a shared test-support file rather than copied a third time — so the next test that needs an event loop group finds the answer instead of the trap. The new suites also adopt `.serialized` and `.timeLimit(.minutes(1))`, matching the ones already in the repo: a test that hangs should fail in a minute and say so, not consume the job timeout and report as "cancelled", which is a lot easier to miss than a red X. Also dropped a `defer { Task { ... } }` that would have let a shutdown outlive its test. Verified locally: 346 tests pass, swiftlint --strict clean across 265 files.
|
One more, and this one was hiding in plain sight: the Swift workflow has been failing on this branch since It read as cancelled rather than failed, which is why neither of us caught it: Cause: Which is the annoying part: #179 already hit this, documented it in
Verified locally: 346 tests, |
The previous commit fixed the shutdown and missed the start. `build-macos` still hit its timeout, and the log named the culprit: "Implicit TLS refuses a TLS 1.2 peer when the floor is TLS 1.3" — started, never finished. `TLS12OnlyServer.init()` called `bind(...).wait()`. Same trap as `syncShutdownGracefully()` wearing a different hat: any `.wait()` reached from a swift-testing test blocks a cooperative-pool thread, and a core-constrained runner then loses its forward-progress guarantee. It also explains why `.timeLimit(.minutes(1))` did not save the job — a blocked thread cannot be cancelled, so the suite ran into the 10-minute job timeout instead of failing in one. The hint was there to be read: nothing else in this test suite calls `.wait()`. Not one call site. I added the only three. Now `async` throughout, with `.get()` instead of `.wait()`. Verified locally: 346 tests pass, swiftlint --strict clean. Verified on CI is the part that matters here, and I will confirm it there rather than claim it.
Third attempt at the macOS CI job, and the first one aimed at the right target. The previous two fixed blocking calls one at a time — syncShutdownGracefully(), then bind().wait() — and each time it hung somewhere else. The real problem was not any single call: it was that this test needed real sockets at all. What the last run showed: 29 of 346 tests completed, then the whole run stalled until the job timeout. Not one hanging test — the cooperative pool starving. Real sockets need a MultiThreadedEventLoopGroup and its blocking teardown, and every blocking step reachable from a swift-testing test costs a pool thread. On a core-constrained runner that is fatal, and it defeats .timeLimit too, since a blocked thread cannot be cancelled. Locally, with cores to spare, all 346 pass in 1.6 seconds — which is why I kept not seeing it. EmbeddedChannel has no threads, no sockets, and no event loop to shut down. The handshake is still real: same NIOSSLClientHandler from the same makeTLSHandler the bootstrap calls, same BoringSSL, same version negotiation — only the transport is a byte pump. 8 milliseconds instead of 80, and nothing to starve. Added a third case while there: a TLS 1.2 floor must still negotiate 1.3 with a capable peer — a floor, not an exact match. Verified by reverting the fix: "A TLS 1.3 floor refuses a peer that stops at TLS 1.2" fails. One thing stated plainly in the file: these prove the floor is enforced once it reaches the handler, not that the call sites pass it — which is what the defect actually was. Calling makeTLSHandler directly hands it the value the bootstrap forgot. That gap is closed by removing the defaults, so an omission is a build error; the compiler is a better guarantee than a test, but it is a different one, and claiming otherwise here would repeat the mistake this file documents. 347 tests pass, swiftlint --strict clean.
…y has
The in-memory handshake did not fix the macOS job either: 27 of 347 tests, then a stall. So it
was never my TLS test. I had been fixing the thing I had just written, twice, instead of
looking at what changed.
What changed: this PR added two suites that construct `IMAPServer`/`SMTPServer` —
MailTLSConfigurationTests (4) and IMAPParserLimitsTests (2) — and neither carries `.serialized`.
Every pre-existing suite here that constructs a server does. That is not a style detail:
constructing a server allocates a MultiThreadedEventLoopGroup, and SMTPServer.deinit shuts it
down with the blocking syncShutdownGracefully(). Released from a Swift Concurrency context, each
one costs a cooperative-pool thread. Six of them running in parallel with everything else, on a
core-constrained runner, starves the pool — which is why unrelated tests ("ASCII display name
behaves like description") hang too, and why nothing reproduces locally on a machine with cores
to spare.
So: `.serialized` + `.timeLimit(.minutes(1))` on both, matching the convention. `.timeLimit` on
the two guard suites as well — they construct nothing, but a hanging test should fail in a
minute and say so rather than consume the job timeout and report as "cancelled".
The convention was visible in every neighbouring file. I wrote past it.
347 tests pass, swiftlint --strict clean.
|
I need to ask for help on CI, before I burn more of your review runs guessing.
What the logs show: it is not one hanging test. Roughly 27 of 347 complete, then the run stalls for ~7 minutes until the timeout — and the tests left unfinished include ones with no relation to this PR ( It does not reproduce locally. 347 tests in 1.6s, and still 1.6s with Four attempts, none of which helped:
One observation I will offer as a lead rather than a diagnosis, since I could not confirm it: Have you seen this before on this runner? And is there a variant you would prefer: dropping the new suites and keeping only what does not construct servers, or something else entirely? I would rather ask than push a fifth guess at it. Unrelated, for completeness: Everything else stands: all findings addressed, all twelve threads resolved, SwiftLint green, Linux and Android green, 347 tests and |
|
Let's See what Fable Ultra finds... stand by... |
|
Thanks for this contribution — both changes are solid and they're being merged. The merge happens via #186, which contains this PR's commits verbatim (your authorship intact; GitHub will mark this PR merged automatically when #186 lands) plus follow-up commits that got CI green and tightened a few things found in review:
The TLS floor and parser-limit design, the propagation to IDLE/named connections, and the no-defaults principle all survive unchanged — good work, and the write-ups in your commit messages made the archaeology much easier. 🤖 Generated with Claude Code |
…9f5454 PR #185 + CI/deadlock fixes: TLS floor, parser limits, and four hang root-causes
2 participants
Two independent security hardening knobs, found while building a mail client on SwiftMail. Both were verified end-to-end against a real Hetzner IMAP/SMTP server (Dovecot, TLS 1.3).
They're separate commits and can be split or cherry-picked — happy to open two PRs instead if you prefer.
1. Minimum TLS version is not settable (
59cd258)MailTLSConfiguration.makeClientConfigurationbuilds onTLSConfiguration.makeClientConfiguration()and only overridescertificateVerificationandtrustRoots. It therefore inherits NIOSSL's defaultminimumTLSVersionof.tlsv1— TLS 1.0, deprecated by RFC 8996 since March 2021. A caller has no way to raise that floor.This adds
MailTLSMinimumVersionand threads it throughIMAPServer,SMTPServer,IMAPConnectionand both SMTP TLS paths — the STARTTLS path atSMTPServer+Connection.swift:283builds its own config and would otherwise have stayed on the old default.The enum is deliberately SwiftMail's own type rather than NIOSSL's
TLSVersion: SwiftMail imports NIOSSL without re-exporting it, so exposingTLSVersionpublicly would push NIOSSL onto every consumer.Behaviour change: the default floor moves from TLS 1.0 to TLS 1.2, the lowest version RFC 8996 still permits. Callers needing the old behaviour can pass
.tlsv1. If you'd rather keep this strictly additive, say the word and I'll default it to.tlsv1.2. Parser body/attribute limits are hardcoded to
.max(76d7a75)IMAPConnection.makeBootstrappins the response parser tomessageAttributeLimit: .maxandbodySizeLimit: .max, with no way for a caller to bound them.A server controls both the length prefixes and the attribute count it sends, so a hostile or malfunctioning server can force unbounded allocation from a single response.
responseBufferLimitdoesn't cover this — it bounds the parser's working buffer, whereas these bound what a single response may declare.This is the class of issue that produced advisories in other IMAP clients: Ruby net-imap GHSA-j3g3-5qv5-52mj (unbounded literal size → memory exhaustion, fixed by adding a configurable
max_response_size) and mutt CVE-2021-3657 ("reject excessively large IMAP literals").Adds
IMAPParserLimits. Strictly additive — defaults are.max/.max, exactly today's behaviour, so existing callers see no change.Bounded defaults would arguably be the better long-term choice, but that would silently break anyone fetching large attachments — left that call to you.
Credit where due: the recursion depth is already bounded (
StackTracker(maximumParserStackDepth: 100)), so the deeply-nested-MIME stack overflow that hit Stalwart and Dovecot (CVE-2020-12100) doesn't apply here.Testing
MailTLSConfigurationTests(default floor, each enum mapping, both server types) andIMAPParserLimitsTests(defaults, partial overrides, propagation)minimumTLSVersion: .tlsv1_3enforced, connect + login + CAPABILITY all succeedThanks for SwiftMail — it was the only actively maintained Swift IMAP client we could build on after MailCore2 went quiet.
🤖 Generated with Claude Code