feat(smtp): distinguish TLS handshake failure from post-TLS application error

[apowis]

When scanning with TLS (SMTPS or STARTTLS), zgrab2 previously had no way to tell whether a failed scan was caused by the TLS handshake itself failing, or by the application-layer exchange failing after TLS was successfully established. Both cases returned a generic error status, and the `tls` JSON object gave no indication of whether the handshake completed.

This PR adds that distinction via two changes:

`TLSLog.HandshakeCompletedSuccessfully bool` (`json:"handshake_completed_successfully"`)
Set to `true` in `TLSConnection.Handshake()` only when the handshake returns `nil`. All modules that call `tlsConn.GetLog()` get this field in their JSON output automatically — no per-module changes required.

The `handshake_completed_successfully` field is also registered in the shared `tls_log` SubRecord in `zgrab2_schemas/zgrab2/zgrab2.py` so zschema validation passes for all modules.

`SCAN_POST_TLS_APPLICATION_ERROR` (`"tls-application-error"`)
A new scan status for "TLS handshake succeeded but the application-layer exchange failed." The existing `SCAN_HANDSHAKE_ERROR` already covers TLS failure; this fills the gap.

The SMTP module is updated as a POC:

• Both TLS paths (SMTPS and STARTTLS) now return `SCAN_HANDSHAKE_ERROR` explicitly on handshake failure, and capture `TLSLog` even on failure (partial handshake data is preserved)
• Application-layer errors after a successful TLS handshake return `SCAN_POST_TLS_APPLICATION_ERROR`
• Pre-TLS errors (e.g. a rejected STARTTLS command) continue to return `SCAN_APPLICATION_ERROR` unchanged

How to Test

```
go test github.com/zmap/zgrab2/...
go test github.com/zmap/zgrab2/modules/smtp -v -run 'TestScanSMTPS|TestScanSTARTTLS|TestHandshake'
```

Three new SMTP scanner tests cover the three states: SMTPS handshake failure, STARTTLS handshake failure, and TLS-success with a 500 error banner (uses a real in-process TLS handshake between stdlib server and zcrypto client over `net.Pipe`).

Notes

The `handshake_completed_successfully` field is available to all other TLS-capable modules already — only the status-code handling needs updating per protocol. If this approach is accepted, IMAP, POP3, FTP, HTTP, and the remaining modules will follow in separate PRs.

[phillip-stephens]

Hello @apowis, really appreciate the work on this and the in-depth unit tests. Also good catch on the STATUS_HANDSHAKE_ERROR when we fail to establish a TLS connection.

To clarify what you think the desired status on a failed SMTP (or other protocol where the protocol occurs both before and after a TLS handshake):

• Failure with initial TCP connection - SCAN_CONNECTION_REFUSED/TIMEOUT/CLOSED
• Failure with application protocol, pre-TLS - SCAN_APPLICATION_ERROR
• Failure with TLS handshake - SCAN_HANDSHAKE_ERROR
• Failure with application protocol, post-TLS - SCAN_TLS_APPLICATION_ERROR

[apowis]

Thanks for the review @phillip-stephens!

I've addressed both points:

• Renamed SCAN_TLS_APPLICATION_ERROR → SCAN_POST_TLS_APPLICATION_ERROR as suggested
• Renamed HandshakeComplete → HandshakeCompletedSuccessfully to remove the ambiguity you flagged

I also took the opportunity to add a small feature while I was in this area: ServerHelloReceived bool on TLSLog. The motivation ties directly to your summary of the status hierarchy — when a TLS handshake fails, it's useful to know why. If ServerHelloReceived is true, the target definitively responded as a TLS server (it sent a ServerHello) even though the handshake didn't complete — which is meaningful signal for callers trying to distinguish "not a TLS server" from "TLS server, handshake failed for another reason" (cipher mismatch, cert rejection, etc.). As a bonus, zcrypto captures partial handshake data (ServerHello, certificates) before returning a handshake error, so HandshakeLog is populated in those cases too.

Happy to pull ServerHelloReceived out into a separate follow-up PR if you'd prefer to keep this one focused — just let me know.

---

Re: applying this to other protocols including HTTP

For simple protocols like IMAP, POP3, and FTP — where the module owns the TLS connection directly and calls Handshake() explicitly — extending this work is straightforward, mirroring what was done for SMTP here.

HTTP is more nuanced. The HTTP module delegates TLS entirely to lib/http/transport rather than managing the handshake itself, so the three new behaviours land differently:

• HandshakeCompletedSuccessfully — already works for free. The transport calls TLSConnection.Handshake() via the TLS wrapper and attaches conn.GetLog() to the request, so the field is populated with no further changes needed.
• SCAN_POST_TLS_APPLICATION_ERROR — achievable with a small change to modules/http/scanner.go. Because TLSLog is attached to the request object when TLS succeeds, we can detect the combination of a non-nil TLSLog and a failed http.Client.Do() and return SCAN_POST_TLS_APPLICATION_ERROR rather than a generic error.
• ServerHelloReceived on a failed TLS handshake — this is the hard case. When DialTLSContext fails, the transport returns (nil, err) and the TLSConnection carrying the partial HandshakeLog is dropped. Unlike SMTP, there is no place to recover it. Fixing this properly would require either a custom error type that carries a *TLSLog alongside the error, or a hook in the transport that preserves the partial log on a failed dial — doable, but a separate piece of work.

Happy to pick that up as a follow-on once this lands.

[phillip-stephens]

Let's pull out the ServerHelloReceived into a separate PR. I'd want to see how the http case would be handled and be sure it can be done cleanly before I'd want to get this in on smtp. We can handle all modules at once for that PR.

Also would love to get a PR to add HandshakeCompletedSuccessfully to the other TLS-using modules.

Thanks for this and just @ me when you've pulled out the ServerHelloReceived and we can get this merged in.

[apowis]

Done — I've pulled ServerHelloReceived out completely. The branch is rebased on master and pushed.

For the HandshakeCompletedSuccessfully rollout to other modules, I've broken it down into a staged plan:

1. PR A — IMAP + POP3 (one PR): identical tlsActive / appErrStatus() pattern to what was done in SMTP here. Trivial lift-and-shift.
2. PR B — FTP: GetFTPSCertificates return type changes to *ScanError; implicit-TLS path returns SCAN_HANDSHAKE_ERROR explicitly. No post-TLS application exchange, so SCAN_POST_TLS_APPLICATION_ERROR is out of scope for this module.
3. PR C — ManageSieve: STARTTLS is unconditional. TLS wrapper failure → SCAN_HANDSHAKE_ERROR; post-TLS capability-read failure → SCAN_POST_TLS_APPLICATION_ERROR.
4. PR D — MySQL: one-liner — swap TryGetScanStatus for an explicit SCAN_HANDSHAKE_ERROR at the tlsWrapper call. No post-TLS exchange.
5. PR E — Postgres: DoSSL error → SCAN_HANDSHAKE_ERROR (currently SCAN_APPLICATION_ERROR). Needs care around the connectErr.Status == SCAN_APPLICATION_ERROR retry path — that's for RequestSSL rejection, not DoSSL, so they must stay separate.
6. PR F — MSSQL: TLS lives inside TDS framing; most involved. Needs a careful read of the error paths in connection.go before touching anything.
7. PR G — HTTP: two sub-tasks. (A) client.Do failures after TLS succeeds → SCAN_POST_TLS_APPLICATION_ERROR (straightforward). (B) Surfacing a partial TLSLog when DialTLSContext fails requires threading a custom error type through the transport — that's the harder case I described previously, kept as a follow-on.

We could maybe compress some of these into one PR, such as combining A-E and leaving F and G separate. Let us know what you think.

[phillip-stephens]

That plan sounds broadly fine with a few changes.

Personally, I think we should only use the new SCAN_POST_TLS_APPLICATION_ERROR for those application protocols like SMTP that have app logic both pre- and post-handshake and keep the status quo for the modules that do a usual TCP -> TLS -> App Logic.

For something like HTTP, returning SCAN_POST_TLS_APPLICATION_ERROR implies there's a pre-TLS app error, which doesn't sit right.

Let's:

1. Retain using SCAN_APPLICATION_ERROR for all modules except those with pre and post-handshake app logic (SMTP, ManageSieve(?))
2. Document the variety of values for statuses in the README.md. (I'm good with this being a final PR after getting the modules in sync with this new status paradigm). I'll open an issue for this now to track.

3. combining A-E and leaving F and G separate - Sounds good to me

Thanks for this work, feels like an overdue alignment of our error statuses!

[phillip-stephens]

Final nit and then looks good to me

[phillip-stephens]

LGTM, thanks again!