diff --git a/docs/superpowers/plans/2026-08-10-external-tls-validation.md b/docs/superpowers/plans/2026-08-10-external-tls-validation.md new file mode 100644 index 00000000..e1fb8ae9 --- /dev/null +++ b/docs/superpowers/plans/2026-08-10-external-tls-validation.md @@ -0,0 +1,144 @@ +# External TLS validation implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix #122 by restoring default TLS certificate validation for every external HTTPS request. + +**Architecture:** Delete the global `HttpClient.badCertificateCallback` assignment. Keep proxy selection and Android local proxy authentication unchanged. A source-bound test prevents the bypass from returning; existing request tests protect route selection. + +**Tech Stack:** Dart, Flutter test, Dio, GitHub Actions. + +## Global Constraints + +- Only change #122's global TLS bypass. +- Write and observe a failing test before production code. +- Invalid, self-signed, expired, and hostname-mismatched external certificates must fail normally. +- Do not claim unavailable Android, Windows, or Linux device testing as complete. +- Do not start the next issue until full cross-platform CI is green. + +--- + +### Task 1: Add a failing security regression test + +**Files:** + +- Create: `test/common/http_test.dart` +- Read: `lib/common/http.dart` + +**Interfaces:** The test reads exactly the source file that creates global `HttpClient` instances and protects the public invariant that production code cannot assign `badCertificateCallback`. + +- [ ] **Step 1: Write the test** + +```dart +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('HTTP overrides never bypass TLS certificate validation', () { + final source = File('lib/common/http.dart').readAsStringSync(); + expect(source, isNot(contains('badCertificateCallback'))); + }); +} +``` + +- [ ] **Step 2: Confirm it fails for the right reason** + +Run: + +```bash +/Volumes/SING_02/flclash/flutter-sdk-3.44.8/bin/flutter test test/common/http_test.dart --reporter expanded +``` + +Expected: FAIL because current production code assigns `client.badCertificateCallback`. + +### Task 2: Make the smallest safe production change + +**Files:** + +- Modify: `lib/common/http.dart:47-53` +- Test: `test/common/http_test.dart` + +**Interfaces:** Preserve `FlClashHttpOverrides.handleFindProxy(Uri)` and `configureLocalProxyAuthentication(HttpClient)` exactly. Remove only the certificate callback assignment. + +- [ ] **Step 1: Delete the insecure assignment** + +The resulting method must be: + +```dart + @override + HttpClient createHttpClient(SecurityContext? context) { + final client = super.createHttpClient(context); + client.findProxy = handleFindProxy; + configureLocalProxyAuthentication(client); + return client; + } +``` + +- [ ] **Step 2: Confirm the new test passes** + +Run: + +```bash +/Volumes/SING_02/flclash/flutter-sdk-3.44.8/bin/flutter test test/common/http_test.dart --reporter expanded +``` + +Expected: PASS. + +- [ ] **Step 3: Confirm unaffected request paths still pass** + +Run: + +```bash +/Volumes/SING_02/flclash/flutter-sdk-3.44.8/bin/flutter test test/common/request_test.dart test/common/local_proxy_test.dart --reporter expanded +``` + +Expected: PASS. + +### Task 3: Verify and commit exactly this repair + +**Files:** + +- Verify: `lib/common/http.dart` +- Verify: `test/common/http_test.dart` + +- [ ] **Step 1: Format and inspect** + +Run: + +```bash +/Volumes/SING_02/flclash/flutter-sdk-3.44.8/bin/dart format lib/common/http.dart test/common/http_test.dart +git diff --check +``` + +Expected: no whitespace errors. + +- [ ] **Step 2: Run analysis and the full Flutter suite** + +Run: + +```bash +/Volumes/SING_02/flclash/flutter-sdk-3.44.8/bin/flutter analyze lib/common/http.dart test/common/http_test.dart +/Volumes/SING_02/flclash/flutter-sdk-3.44.8/bin/flutter test --reporter expanded +``` + +Expected: no analyze errors and all tests pass. + +- [ ] **Step 3: Commit only the security repair** + +Run: + +```bash +git add lib/common/http.dart test/common/http_test.dart +git commit -m "fix: restore external TLS certificate validation" +``` + +Expected: one commit with the production change and its regression test. + +### Task 4: Cross-platform CI and release evidence + +**Files:** Verify the existing GitHub Actions workflows. + +- [ ] Push the isolated branch and open a draft PR referencing #122 after local verification. +- [ ] Require successful Dart tests/analyze plus Android, Windows, macOS, and Linux builds before closing #122. +- [ ] Record available normal-TLS subscription and WebDAV checks; leave unavailable device evidence explicitly pending. diff --git a/docs/superpowers/specs/2026-08-10-external-tls-validation-design.md b/docs/superpowers/specs/2026-08-10-external-tls-validation-design.md new file mode 100644 index 00000000..de0f2da0 --- /dev/null +++ b/docs/superpowers/specs/2026-08-10-external-tls-validation-design.md @@ -0,0 +1,44 @@ +# 外部 HTTPS 憑證驗證修復設計 + +## 背景 + +我方追蹤單 [#122](https://github.com/SingLinkNetwork/FlClash/issues/122) 與上游 PR #2204 指出:`FlClashHttpOverrides.createHttpClient` 目前對每個 `HttpClient` 設定 `badCertificateCallback = (_, _, _) => true`。這會接受所有外部 HTTPS 的無效憑證。 + +全域 override 會被一般 `HttpClient()` 使用;`Request` 建立的預設、proxy 與 direct Dio client 都可能受影響。因此訂閱下載、更新檢查、WebDAV 同步與圖片下載都不應保有這個全域繞過。 + +## 目標與不變事項 + +- 外部 HTTPS 一律使用 Dart 正常的 TLS 憑證與主機名稱驗證。 +- 保留現有 `findProxy`:核心啟動時外部請求仍可經本機 HTTP proxy,停用或暫停時仍直接連線。 +- 保留 Android 本機 HTTP proxy 的 Basic authentication 邏輯。 +- 不新增「忽略憑證錯誤」設定或隱藏例外。 +- 不修改使用者設定、核心 YAML 或 WebDAV 資料。 + +## 方案比較與採用方案 + +1. **完全移除 callback(採用)**:HTTP CONNECT proxy 對外 TLS 驗證的是目的網站,並非本機 proxy;本機控制器也使用 HTTP。因此不需要 TLS 例外,安全邊界最清楚。 +2. 僅允許 `localhost`/`127.0.0.1`:比現狀安全,但 callback 實際收到的是 HTTPS 目的主機,這個例外沒有實際需求,且日後可能被誤用。 +3. 讓使用者開關略過驗證:方便不正確伺服器,但會重新暴露訂閱與 WebDAV 的中間人風險,不採用。 + +## 設計 + +`FlClashHttpOverrides.createHttpClient` 只設定 `findProxy` 與 Android 本機 proxy authentication,不設定 `badCertificateCallback`。Dart 預設行為會在外部憑證無效或主機名稱不符時拒絕連線。 + +回歸測試會以 `File` 讀取此安全邊界的少量原始碼,明確斷言 production code 不再指派 `badCertificateCallback`;這比替一個未使用 helper 寫測試更能防止日後把全域 bypass 加回去。現有 request tests 則驗證 proxy/direct 選路不變。 + +## 受影響檔案 + +- `lib/common/http.dart`:移除全域 callback,保留 proxy 與 Android authentication wiring。 +- `test/common/http_test.dart`:先寫「production code 不可指派 callback」的回歸測試,並保留 proxy 路由與 Android authentication 的測試。 +- `docs/superpowers/plans/2026-08-10-external-tls-validation.md`:記錄 TDD、CI 與真機驗收步驟。 + +## 驗收 + +- `FlClashHttpOverrides` 不再含 `badCertificateCallback` 指派。 +- 現有 proxy/direct 路由與 Android proxy authentication 測試持續通過。 +- Flutter analyze、完整 Flutter tests,以及 GitHub Actions 的 Android、Windows、macOS、Linux build 都成功。 +- 發佈前以正常 TLS 訂閱與 WebDAV 端點驗證 proxy/direct 路徑;無法取得的其他平台真機不以 CI 冒充。 + +## 風險與回滾 + +自簽、過期或網域不符的外部 HTTPS 端點會改為失敗;這是安全修復的預期結果,使用者應修正服務端憑證。回滾只需回滾本次單一 commit,無資料遷移或設定變更。 diff --git a/lib/common/http.dart b/lib/common/http.dart index 9d9c1f50..c5e35dfb 100644 --- a/lib/common/http.dart +++ b/lib/common/http.dart @@ -11,8 +11,12 @@ void configureLocalProxyAuthentication(HttpClient client) { final mixedPort = globalState.container.read( patchClashConfigProvider.select((state) => state.mixedPort), ); - if (!isLocalProxyEndpoint(host, port, mixedPort) || - scheme.toLowerCase() != 'basic') { + if (!shouldAuthenticateLocalProxy( + host: host, + port: port, + scheme: scheme, + expectedPort: mixedPort, + )) { return false; } final credentials = globalState.localProxyCredentials; @@ -27,8 +31,13 @@ void configureLocalProxyAuthentication(HttpClient client) { } class FlClashHttpOverrides extends HttpOverrides { + static bool _isLoopbackHost(String host) { + return host == 'localhost' || + InternetAddress.tryParse(host)?.isLoopback == true; + } + static String handleFindProxy(Uri url) { - if ([localhost].contains(url.host)) { + if (_isLoopbackHost(url.host)) { return 'DIRECT'; } final ref = globalState.container; @@ -45,7 +54,6 @@ class FlClashHttpOverrides extends HttpOverrides { @override HttpClient createHttpClient(SecurityContext? context) { final client = super.createHttpClient(context); - client.badCertificateCallback = (_, _, _) => true; client.findProxy = handleFindProxy; configureLocalProxyAuthentication(client); return client; diff --git a/lib/common/local_proxy.dart b/lib/common/local_proxy.dart index f7c64753..04d3dd5e 100644 --- a/lib/common/local_proxy.dart +++ b/lib/common/local_proxy.dart @@ -45,3 +45,13 @@ bool shouldUseSystemProxy({required bool isAndroid, required bool requested}) { bool isLocalProxyEndpoint(String host, int port, int expectedPort) { return (host == 'localhost' || host == '127.0.0.1') && port == expectedPort; } + +bool shouldAuthenticateLocalProxy({ + required String host, + required int port, + required String scheme, + required int expectedPort, +}) { + return isLocalProxyEndpoint(host, port, expectedPort) && + scheme.toLowerCase() == 'basic'; +} diff --git a/test/common/http_test.dart b/test/common/http_test.dart new file mode 100644 index 00000000..e00be5aa --- /dev/null +++ b/test/common/http_test.dart @@ -0,0 +1,70 @@ +import 'dart:io'; + +import 'package:fl_clash/common/http.dart'; +import 'package:fl_clash/providers/providers.dart'; +import 'package:fl_clash/state.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +void main() { + late ProviderContainer container; + late HttpServer server; + + setUpAll(() async { + container = ProviderContainer(); + globalState.container = container; + final context = SecurityContext() + ..useCertificateChain('test/fixtures/tls/localhost-cert.pem') + ..usePrivateKey('test/fixtures/tls/localhost-key.pem'); + server = await HttpServer.bindSecure( + InternetAddress.loopbackIPv4, + 0, + context, + ); + server.listen((request) => request.response.close()); + }); + + setUp(() { + container.read(runTimeProvider.notifier).update((_) => null); + }); + + tearDownAll(() { + container.dispose(); + return server.close(force: true); + }); + + test('HTTP overrides reject an untrusted TLS certificate', () async { + final client = FlClashHttpOverrides().createHttpClient(null); + addTearDown(client.close); + + final request = client.getUrl( + Uri(scheme: 'https', host: '127.0.0.1', port: server.port), + ); + + await expectLater(request, throwsA(isA())); + }); + + test('HTTP overrides preserve proxy routing for active connections', () { + container.read(runTimeProvider.notifier).update((_) => 1); + final mixedPort = container.read( + patchClashConfigProvider.select((state) => state.mixedPort), + ); + + expect( + FlClashHttpOverrides.handleFindProxy(Uri.parse('https://example.com')), + 'PROXY localhost:$mixedPort', + ); + expect( + FlClashHttpOverrides.handleFindProxy(Uri.parse('https://localhost')), + 'DIRECT', + ); + expect( + FlClashHttpOverrides.handleFindProxy(Uri.parse('https://127.0.0.1')), + 'DIRECT', + ); + expect( + FlClashHttpOverrides.handleFindProxy(Uri.parse('https://[::1]')), + 'DIRECT', + ); + }); +} diff --git a/test/common/local_proxy_test.dart b/test/common/local_proxy_test.dart index a7a6329b..afc1f039 100644 --- a/test/common/local_proxy_test.dart +++ b/test/common/local_proxy_test.dart @@ -50,4 +50,52 @@ void main() { expect(isLocalProxyEndpoint('10.0.0.2', 7890, 7890), isFalse); expect(isLocalProxyEndpoint('localhost', 7891, 7890), isFalse); }); + + test('Android proxy authentication only accepts a local Basic challenge', () { + expect( + shouldAuthenticateLocalProxy( + host: 'localhost', + port: 7890, + scheme: 'Basic', + expectedPort: 7890, + ), + isTrue, + ); + expect( + shouldAuthenticateLocalProxy( + host: '127.0.0.1', + port: 7890, + scheme: 'BASIC', + expectedPort: 7890, + ), + isTrue, + ); + expect( + shouldAuthenticateLocalProxy( + host: 'example.com', + port: 7890, + scheme: 'Basic', + expectedPort: 7890, + ), + isFalse, + ); + expect( + shouldAuthenticateLocalProxy( + host: 'localhost', + port: 7891, + scheme: 'Basic', + expectedPort: 7890, + ), + isFalse, + ); + expect( + shouldAuthenticateLocalProxy( + host: 'localhost', + port: 7890, + scheme: 'Digest', + expectedPort: 7890, + ), + isFalse, + ); + }); } diff --git a/test/fixtures/tls/localhost-cert.pem b/test/fixtures/tls/localhost-cert.pem new file mode 100644 index 00000000..aeb1f835 --- /dev/null +++ b/test/fixtures/tls/localhost-cert.pem @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICyTCCAbGgAwIBAgIJAIJpo1EbkTw9MA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNV +BAMMCWxvY2FsaG9zdDAeFw0yNjA4MTAxMTQ2NTNaFw0zNjA4MDcxMTQ2NTNaMBQx +EjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC +ggEBAJwUefJ8gq2Bki6+UF6X5jSxbW6CwFjpjFZAHkbjRFPmHP9V4ELFk8lNXUFG +MRyS839PnVSFDqgYyObmuC84FxyGwerHOJE2VmFZb/31qP7k8llbeW4M+0L0gYyf +urN44CKY0qnFZPxfspKfG73jjKVn8zg73z6nUSBhmpRU2Oc0RgGmBKf9BNalieDR +T5otop/MQD+kNQrVxmzor5zHzGCg3Aa0s0uieO8b0isbGFEb87+voy5yhn+E+gLj +FG3JwUr8oFOXrKxKsYZ+3p9Vppuc1KCRNhidzCK/Ae2VglL4ONTdh6VGoNOT7YMQ +tduYmyd2TsCWo2xtY8GOE/zXwaMCAwEAAaMeMBwwGgYDVR0RBBMwEYIJbG9jYWxo +b3N0hwR/AAABMA0GCSqGSIb3DQEBCwUAA4IBAQApIxgZqLsj76bmyqwo0HLhL/9f ++FCNVXGNbU0Cij7PD70UwPIXhfl+gDeCzArjbXtEG3VRzVMa3ntw5SQmVL0Fqhsw +InEapJIGNHVD2WW9Ok7Jj47a8mh2UqvbmJZjDPeOoQ7X5VjCVwto64VnAVU09qWO +nEo1OMqrzgqNTJR5yod8dH12y3qOgaVzoxPnmDtWjaJuPeoe3NJ+mFWPTvkC2GUs +VMD0pwwQWbwmvAah7CSyQND0okgLye1o2aRU4OG6DPDDtraWKWnyFvTZN3td9VmD +U9JQjskyzMoFkgrMKT4E/cLYTuinxEzhUbDz+y0Kn+1cANwiUbnnESEjEPUr +-----END CERTIFICATE----- diff --git a/test/fixtures/tls/localhost-key.pem b/test/fixtures/tls/localhost-key.pem new file mode 100644 index 00000000..28a55187 --- /dev/null +++ b/test/fixtures/tls/localhost-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAnBR58nyCrYGSLr5QXpfmNLFtboLAWOmMVkAeRuNEU+Yc/1Xg +QsWTyU1dQUYxHJLzf0+dVIUOqBjI5ua4LzgXHIbB6sc4kTZWYVlv/fWo/uTyWVt5 +bgz7QvSBjJ+6s3jgIpjSqcVk/F+ykp8bveOMpWfzODvfPqdRIGGalFTY5zRGAaYE +p/0E1qWJ4NFPmi2in8xAP6Q1CtXGbOivnMfMYKDcBrSzS6J47xvSKxsYURvzv6+j +LnKGf4T6AuMUbcnBSvygU5esrEqxhn7en1Wmm5zUoJE2GJ3MIr8B7ZWCUvg41N2H +pUag05PtgxC125ibJ3ZOwJajbG1jwY4T/NfBowIDAQABAoIBAAoXNM8dh6FKCDZn +bXDAJ0zuz6FeaKsW5lDJMmvo04kNDoDff3Exrt1L2CSZMiGjyoRHlGdbdtgdShfL +gMrkuHHKsIzKNnkE507B5oCnujZIfPaT+GTKCBCLGDkMNpjVzr/N24jIT0AVu8jI +6pU7OeOTteDD1ioJzS/nv4OpfKBt4eNcjN61BxX/av1IjL2tZwLOdfY21aFG4nSp +88tFG3nZ+V4rsYbE4g+hrnJWR8coWFBotmhtZN+IOMcHcT8cFvycZpieDp+IJQnC +aZ8d6j5UO/6D5mmtIPO1FVEMyTca1Pv8N9WYXUWo61bZG8lEvKU1nD7Fg4McnwRH +QThqa3kCgYEAytYB2/HpJpW9d4EUbHfVWM2IQiIjgmIw10NlQXgbELwiPDzOhwAB +nGP9BD8hlAhvcR3SBluiY6rEYZ0kkrzAFK8fWruEe5xv5Kl/yXeCjAHyypw2hG8A +Kmh0goo8jOAnakEECY1BDhA4blW3c4rvL2cIcYcwEsQ4kj6B1ElgfScCgYEAxP02 +TSEHAt3Y96+zlMFsH/9945xGw7DFGAiPTirnao+u03/heoLyzSpnPk1T9WydzLji +Vh+ec9vgTBJZ4/Yy0IQ3d3CFT4nbXrkj2v/awWfJ6+Hr09NVVNKjX8fCb69ajzU4 +LogmRMJhDjSh5/bYjcV9LvMWEX7PfHJLifot3SUCgYEAvlHhurOY9dhox9n5BPHH +4KQxpwQLV4q0kzcsqip9Es3EPcs8ahWKbWiwfqyR4CmuD3BQoiCXFXek/oaJS5oJ +jR7oh6UNYULfBqo4OhyGBMLLQmsjCE5y1FfggqAcwn6M3TkWBbRhh0WW/Yg2Mxb4 +6Px/Qq77X5B4GFjITx1uiN8CgYBnzqgW7RiswgvcOpMACQ3ZruHAWFJSPY0fpSMf +pixdU2plujYwQWGCDJ5rjoR4CMz/XU6o4/C0TWJdtC36m3sbtxEjuAAIcjVQLXBf +sASvdupJi7yi2/NTx8ygZLxp7pZicUEyBD1mbqiGu+W2P7bOvwvA+gjfH1xn0DNO +erBAoQKBgB53y5jrcz7zsPFqBsshEO6IXPFnDfBKk4rat/IzLL/ZGpjjMN0xbxdN +tbCWhSHM7gBL2XHidcsEH89mrZOVS7Jug+K0SH3rbsc+slc2fD17fU1f7zs/4bsk +GxJ7FkHrqSWVmUh31Ej/PiGyBfO20hbpFoZQxZKkSn4Or+uNTnlG +-----END RSA PRIVATE KEY----- diff --git a/tool/verify_android_local_proxy_security.rb b/tool/verify_android_local_proxy_security.rb index 1885b1b7..6396a85e 100644 --- a/tool/verify_android_local_proxy_security.rb +++ b/tool/verify_android_local_proxy_security.rb @@ -42,7 +42,13 @@ http.include?('authenticateProxy') && http.include?('addProxyCredentials') && http.include?('HttpClientBasicCredentials') && - http.include?('isLocalProxyEndpoint') + http.include?('shouldAuthenticateLocalProxy') + +abort 'Dart proxy authentication helper does not restrict local Basic challenges' unless + local_proxy.include?('bool shouldAuthenticateLocalProxy') && + local_proxy.include?("host == 'localhost'") && + local_proxy.include?("host == '127.0.0.1'") && + local_proxy.include?("scheme.toLowerCase() == 'basic'") abort 'Android shared VPN state can still request the system proxy' unless state.include?('shouldUseSystemProxy') && @@ -63,9 +69,14 @@ core_patch.scan('if disableUDP || shouldUDPIgnore').length == 2 abort 'Dart local proxy regression test is missing' unless File.file?(dart_test) +abort 'Dart local proxy authentication regression test is missing' unless + File.read(dart_test).include?( + 'Android proxy authentication only accepts a local Basic challenge', + ) + abort 'Core patch does not contain the Go local proxy regression test' unless core_patch.include?('listener_security_test.go') && - core_patch.include?('TestShouldDisableDefaultUDP') + core_patch.include?('TestShouldDisableDefaultUDP') abort 'Core patch applier is missing' unless patch_applier.include?('applyClashMetaPatches')