Skip to content

fix(ssl): complete Apple TLS server and peer verification - #899

Open
ithewei wants to merge 12 commits into
masterfrom
fix/appletls-server-verification
Open

ithewei wants to merge 12 commits into
masterfrom
fix/appletls-server-verification

Conversation

@ithewei

@ithewei ithewei commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • load unencrypted RSA PKCS#1/PKCS#8 identities and PEM certificate chains for Apple TLS servers and mTLS clients
  • explicitly verify peer chains, hostnames, system roots, and exclusive custom CA file/directory roots
  • preserve libhv fd/event-loop ownership while fixing nonblocking handshake direction and containing Secure Transport deprecation warnings
  • add strict bounded PEM/DER parsing, Apple TLS regression tests, deterministic fixtures, and Chinese TLS documentation

Compatibility

  • public hssl API is unchanged
  • local Apple identities require macOS 10.12+ or iOS 11.2+
  • client-only Apple TLS remains availability-safe for older deployment targets
  • EC and encrypted private keys remain unsupported by this backend

Verification

  • make clean && ./configure && make libhv && make unittest
  • bin/appletls_pem_test
  • bin/appletls_test
  • CMake shared/static library build
  • strict Apple TLS compile with -Wall -Wextra -Werror
  • iOS 9.0 and iOS 11.2 syntax/availability checks
  • Clang static analyzer on ssl/appletls.c and ssl/appletls_pem.c
  • bits-code-guard review: no unresolved P0-P2 findings

Notes

Network.framework was evaluated but cannot adopt libhv-owned connected file descriptors, so Secure Transport remains the compatibility backend and its deprecation warning is suppressed only inside ssl/appletls.c.

ithewei and others added 11 commits October 1, 2026 06:31
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 09:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The documentation overstates cross-backend verification guarantees, and the new Apple-specific tests are not executed in CI.

Review effort: Balanced
Findings: 1 Medium severity · 3 Low severity

Open (4)
What changed in this PR

Completes Apple Secure Transport support for identities, peer verification, nonblocking handshakes, and regression coverage.

Changes:

  • Adds bounded PEM/DER parsing and Apple TLS identity/trust handling.
  • Adds Apple TLS handshake, mTLS, hostname, and parser tests with fixtures.
  • Improves SNI failure handling and adds Chinese TLS documentation.
File Description
ssl/​appletls.c Implements Apple identities, verification, and handshake fixes.
ssl/​appletls_pem.c Adds bounded PEM/DER parsing.
ssl/​appletls_pem.h Declares internal parser API and limits.
event/​tls.c Handles SNI setup failures.
http/​client/​HttpClient.cpp Propagates synchronous SNI setup failures.
Makefile Builds Apple TLS tests on Darwin.
scripts/​unittest.sh Conditionally runs Apple TLS tests.
unittest/​appletls_test.c Tests handshakes, verification, and mTLS.
unittest/​appletls_pem_test.c Tests parser formats and limits.
unittest/​fixtures/​appletls/​generate.sh Regenerates TLS fixtures.
unittest/​fixtures/​appletls/​README.md Documents fixture usage.
unittest/​fixtures/​appletls/​root.crt Test root CA.
unittest/​fixtures/​appletls/​root.der DER root CA fixture.
unittest/​fixtures/​appletls/​intermediate.crt Intermediate CA fixture.
unittest/​fixtures/​appletls/​server.crt Server certificate fixture.
unittest/​fixtures/​appletls/​server-chain.pem Server certificate chain.
unittest/​fixtures/​appletls/​server-pkcs1.key PKCS#1 server key.
unittest/​fixtures/​appletls/​server-pkcs8.key PKCS#8 server key.
unittest/​fixtures/​appletls/​client.crt mTLS client certificate.
unittest/​fixtures/​appletls/​client.key mTLS client key.
unittest/​fixtures/​appletls/​client-chain.pem Client certificate chain.
unittest/​fixtures/​appletls/​wrong-root.crt Untrusted CA fixture.
unittest/​fixtures/​appletls/​wrong-server.key Mismatched key fixture.
unittest/​fixtures/​appletls/​multi-ca.pem Multiple-CA fixture.
unittest/​fixtures/​appletls/​ec-sec1.key Unsupported SEC1 EC key.
unittest/​fixtures/​appletls/​ec-pkcs8.key Unsupported PKCS#8 EC key.
unittest/​fixtures/​appletls/​encrypted-key.pem Unsupported encrypted key.
unittest/​fixtures/​appletls/​malformed-base64.pem Invalid Base64 fixture.
unittest/​fixtures/​appletls/​malformed-pkcs8.pem Invalid PKCS#8 fixture.
unittest/​fixtures/​appletls/​ca-dir/​root.crt CA-directory certificate.
unittest/​fixtures/​appletls/​ca-dir/​README.txt Non-certificate directory noise.
unittest/​fixtures/​appletls/​empty-ca-dir/​README.txt Empty CA-source fixture.
docs/​cn/​TcpServer.md Documents server TLS and mTLS.
docs/​cn/​TcpClient.md Documents client verification and mTLS.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/unittest.sh
Comment thread docs/cn/TcpClient.md Outdated
Comment thread docs/cn/TcpServer.md Outdated
Comment thread docs/cn/TcpServer.md Outdated
Co-authored-by: TRAE CLI <traecli@bytedance.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 09:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Optional client-certificate requests incorrectly abort, and Apple tests fail to link when another TLS backend is configured on macOS.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Allow session resumption without optional client certificate

ssl/​appletls.c:1287

This makes a client without a local identity abort whenever a server merely requests an optional client certificate. A client is allowed to resume without sending one; a server configured to require mTLS will then reject it itself. The previous implementation resumed here, and retaining that behavior preserves connections to servers using optional client authentication.

Comment thread Makefile

ifeq ($(shell uname -s),Darwin)
appletls_test: prepare
$(CC) -g -Wall -Wextra -O0 -std=c99 -DAPPLETLS_TESTING \
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