Skip to content

fix(tls): verify certificates by default#12

Open
alanhoff wants to merge 3 commits intoendel:mainfrom
alanhoff:fix/tls-verify-certificates-by-default
Open

fix(tls): verify certificates by default#12
alanhoff wants to merge 3 commits intoendel:mainfrom
alanhoff:fix/tls-verify-certificates-by-default

Conversation

@alanhoff
Copy link

Summary

  • flip the low-level TlsConfig.skip_cert_verify default to false so client certificate and CertificateVerify validation are on by default
  • keep the fake-certificate loopback tests on an explicit opt-out path instead of relying on the insecure default
  • align the chain-validation spec note with the actual default behavior

Vulnerability

The TLS 1.3 client implementation has certificate-chain and CertificateVerify verification code, but the low-level TlsConfig still defaults skip_cert_verify to true. That means any caller that constructs TlsConfig without explicitly overriding the field silently disables server authentication by default.

Concrete examples:

  • a new client that sets only server_name, ALPN, and key material will still skip certificate validation unless it remembers to override skip_cert_verify
  • an on-path attacker can present a self-signed certificate and forged handshake messages to such a client, because the default configuration opts out of the authentication checks entirely
  • this contradicts the repo's own user-facing contract: the README already documents skip_cert_verify as defaulting to false and “testing only” when enabled

Validation

  • zig build test
  • zig build
  • zig build fuzz
  • gh issue list --repo endel/quic-zig --search "TlsConfig skip_cert_verify default false" --limit 20

References

Make the low-level TLS client config secure by default, keep the fake-certificate tests on an explicit opt-out path, and align the chain-validation spec note with the actual default.

Co-authored-by: Codex <noreply@openai.com>
Copilot AI review requested due to automatic review settings March 17, 2026 21:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the TLS 1.3 client defaults by enabling certificate and CertificateVerify validation unless callers explicitly opt out, and updates loopback tests/spec docs to reflect that safer default.

Changes:

  • Flip TlsConfig.skip_cert_verify default from true to false.
  • Update loopback handshake tests to explicitly set skip_cert_verify = true for fake/self-signed cert scenarios.
  • Update the RFC 5280 chain-validation spec note to match the new default behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/quic/tls13.zig Changes the default TLS verification behavior and adjusts/adds tests to keep loopback coverage working under the safer default.
SPEC/RFC5280_CHAIN_VALIDATION.md Updates documentation to reflect the new default and opt-out guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Codex <noreply@openai.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the TLS 1.3 client defaults by enabling certificate-chain and CertificateVerify validation unless callers explicitly opt out, aligning behavior with the documented security contract.

Changes:

  • Flip TlsConfig.skip_cert_verify default to false and tighten client-side certificate/CV failure handling.
  • Update loopback tests to explicitly opt out of verification when using fake/self-signed cert material.
  • Update the RFC 5280 chain-validation spec note to match the new default behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/quic/tls13.zig Enables cert verification by default, enforces CA bundle presence during validation, and updates tests for the new secure default.
SPEC/RFC5280_CHAIN_VALIDATION.md Updates documentation to reflect the new default for skip_cert_verify.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Update the RFC 5280 status note so it reflects the branch's fail-closed ca_bundle requirement and the remaining gap around automatic system trust loading.

Co-authored-by: Codex <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants