tls: document Prefer mode plaintext behavior

[mtmk]

Documents TLS Prefer/Auto mode behavior and adds guidance for TLS-terminating proxy setups (nats-io/nats-server#7927).

Unlike most other NATS clients (Go, Rust, Java, Python), the .NET client attempts a TLS upgrade when the server advertises tls_available without tls_required. This can cause connection failures behind a TLS-terminating proxy. The workaround is TlsMode.Disable.

We are not changing the default behavior to match other clients because existing configurations may rely on the automatic TLS upgrade. Switching to plaintext silently would be a security regression for those users.

• XML doc remarks on NatsOpts.TlsOpts, NatsTlsOpts.Mode, TlsMode.Prefer, TlsMode.Auto
• TLS proxy section in docfx security page
• Real nats-server tests with tls {} + allow_non_tls
• Go client cross-check via GoHarness
• Fake-server tests for Prefer, Disable, Require, Auto modes

[synadia-claude-reviewer]

Claude finished @mtmk's task in 3m 22s —— View job

---

PR Review: tls: document MITM downgrade risk in Prefer mode

• Read changed files and gather context
• Review NatsTlsOpts.cs documentation changes
• Review TlsPreferMitmTest.cs test implementation
• Check for correctness, security, and protocol issues
• Post final review

---

Summary

Good PR. The documentation accurately describes the MITM downgrade risk, uses proper <see cref="..."/> cross-references, and points users to the correct mitigations. The test suite proves the by-design behavior. Two minor robustness issues in the tests worth addressing.

Documentation (NatsTlsOpts.cs)

No issues. The remarks are accurate, appropriately scoped to Prefer, Auto, and NatsTlsOpts.Mode, and consistently direct users to tls:// or TlsMode.Require as the secure alternative.

Tests (TlsPreferMitmTest.cs)

1. serverTask not awaited in Require_mode_throws_when_server_does_not_support_tls

After the ThrowAsync assertion, serverTask is cancelled and abandoned without await:

await act.Should().ThrowAsync<NatsException>();
cts.Cancel();
listener.Stop();
// serverTask is never awaited here

If serverTask faults (e.g., a socket error before the cancellation), the exception becomes unobserved, which xunit may surface as a test failure unrelated to the actual assertion. The fix is to await serverTask (swallowing OperationCanceledException) after cts.Cancel(). Fix this →

2. listener.Stop() not called on failure in Prefer_mode_sends_credentials_in_plaintext_when_info_has_no_tls_flags

If serverTask throws or nats.ConnectAsync() throws before await serverTask, listener.Stop() is skipped. The using var cts will be disposed, but the TcpListener remains open until GC. Not a test correctness issue, but wrapping in try/finally would be cleaner:

try
{
    await nats.ConnectAsync();
    await serverTask;
}
finally
{
    listener.Stop();
}

3. Encoding comment duplication

The // ISO 8859-1 (Latin-1): 1-byte encoding... comment is copy-pasted identically in both Require_mode_throws... and Prefer_mode_sends.... Minor nit; could be a shared constant or just accepted.

Protocol correctness

The INFO JSON in the MITM server includes "tls_required":false,"tls_available":false — both fields are explicitly set. This is correct: the client checks these flags to decide whether to upgrade. The PING/PONG handshake used to detect the completed CONNECT is consistent with how the real NATS protocol works.

[github-actions]

🚀 Deployed on https://69cfde3731c2b6b82bf65748--teal-klepon-7e92d0.netlify.app

[philpennock]

Did not review tests.

If it is not too late to rename Implicit, it would be good to do so, as "implicit" might be taken as "ambiently available without our seeing it", via things like stunnel or equivalent cryptographic privacy (tailscale etc). I'd name it OnConnect and make clear that it is a stronger form of Require. I'd also mention explicitly below the table that only Require and (OnConnect or Implicit) provide the full protection which TLS can offer.

Looks good as is, the feedback above is non-blocking.

[mtmk]

Did not review tests.

If it is not too late to rename Implicit, it would be good to do so, as "implicit" might be taken as "ambiently available without our seeing it", via things like stunnel or equivalent cryptographic privacy (tailscale etc). I'd name it OnConnect and make clear that it is a stronger form of Require. I'd also mention explicitly below the table that only Require and (OnConnect or Implicit) provide the full protection which TLS can offer.

Looks good as is, the feedback above is non-blocking.

@philpennock thanks for the review! good point. We can deprecate Implicit. OnConncet would clash with .NET event naming convention though. Perhaps TlsFirst matching server? other options: Immediate, Preemptive, Eager. Any preference, suggestion?

Will also add the note about full TLS protection.

[philpennock]

I retract my previous suggestion to avoid "Implicit". That's the RFC 8314 terminology so we should stick with it for consistency even if it is a little unfortunate.

[mtmk]

test failures for 'Check_JetStream_cluster_related_fields' are not related to this PR. We can investigate after merge to main.