-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
π Fix validateCertificate to fire pre-emission for TLS pinning
#2512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
1ce3ee2
23aa0c0
8a66e1e
a7372d3
6eb64d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import 'dart:async'; | ||
| import 'dart:convert'; | ||
| import 'dart:io'; | ||
| import 'dart:typed_data'; | ||
|
|
||
|
|
@@ -25,6 +26,30 @@ typedef ValidateCertificate = bool Function( | |
| int port, | ||
| ); | ||
|
|
||
| /// Internal sentinel thrown from [IOHttpClientAdapter]'s connection factory | ||
| /// when [IOHttpClientAdapter.validateCertificate] rejects the peer certificate. | ||
| /// [_fetch] catches it and rethrows as [DioException.badCertificate]. | ||
| class _BadCertificateException implements HandshakeException { | ||
| _BadCertificateException(this.host, this.port, this.cert); | ||
|
|
||
| final String host; | ||
| final int port; | ||
| final X509Certificate? cert; | ||
|
|
||
| @override | ||
| String get type => 'HandshakeException'; | ||
|
|
||
| @override | ||
| String get message => 'validateCertificate returned false'; | ||
|
|
||
| @override | ||
| OSError? get osError => null; | ||
|
|
||
| @override | ||
| String toString() => | ||
| 'HandshakeException: $message (host=$host, port=$port)'; | ||
| } | ||
|
|
||
| /// Creates an [IOHttpClientAdapter]. | ||
| HttpClientAdapter createAdapter() => IOHttpClientAdapter(); | ||
|
|
||
|
|
@@ -44,14 +69,42 @@ class IOHttpClientAdapter implements HttpClientAdapter { | |
|
|
||
| /// When this callback is set, [Dio] will call it every | ||
| /// time it needs a [HttpClient]. | ||
| /// | ||
| /// Note: supplying this disables the pre-emission TLS hook used by | ||
| /// [validateCertificate]. See [validateCertificate] for details. | ||
| CreateHttpClient? createHttpClient; | ||
|
|
||
| /// Allows the user to decide if the response certificate is good. | ||
| /// If this function is missing, then the certificate is allowed. | ||
| /// This method is called only if both the [SecurityContext] and | ||
| /// [badCertificateCallback] accept the certificate chain. Those | ||
| /// methods evaluate the root or intermediate certificate, while | ||
| /// [validateCertificate] evaluates the leaf certificate. | ||
| /// Allows the user to decide if the leaf certificate of the TLS connection | ||
| /// is good. If this function is missing, then the certificate is allowed. | ||
| /// | ||
| /// When [createHttpClient] is **not** set, this callback fires **before** | ||
| /// the request body is sent, immediately after the TLS handshake completes. | ||
| /// Returning `false` aborts the connection without leaking any request | ||
| /// data and surfaces as [DioException.badCertificate]. This makes the | ||
| /// callback suitable for certificate or public-key pinning. | ||
| /// | ||
| /// When [createHttpClient] **is** supplied, dio cannot install the | ||
| /// pre-emission hook on the user-built [HttpClient]; the callback then | ||
| /// runs after the response head arrives (the legacy 5.x behavior). For | ||
| /// pre-emission validation in that case, set `connectionFactory` on the | ||
| /// [HttpClient] you return. | ||
| /// | ||
| /// Validation runs once per TCP connection, not per request. Connections | ||
| /// are pooled by [HttpClient], so multiple requests to the same host | ||
| /// within `idleTimeout` will produce one approval call. | ||
| /// | ||
| /// On the pre-emission path, this callback is the **sole** gate for | ||
| /// certificate trust: system / CA validation is bypassed (the equivalent | ||
| /// of `HttpClient.badCertificateCallback: (_, _, _) => true` in the | ||
| /// existing example), so the callback receives every leaf cert and is | ||
| /// solely responsible for accepting or rejecting it. This matches what | ||
| /// users already configure manually for pinning and lets self-signed and | ||
| /// pinned-CA setups work without supplying [createHttpClient]. | ||
| /// | ||
| /// Any [SecurityContext] you set on a custom [HttpClient] is **not** | ||
| /// used by the pre-emission path. To combine mTLS or a custom trust store | ||
| /// with pinning, supply a [createHttpClient] and pin in the legacy | ||
| /// post-response path. | ||
| ValidateCertificate? validateCertificate; | ||
|
|
||
| HttpClient? _cachedHttpClient; | ||
|
|
@@ -110,6 +163,11 @@ class IOHttpClientAdapter implements HttpClientAdapter { | |
| ); | ||
| } | ||
| }); | ||
| } on _BadCertificateException catch (e) { | ||
| throw DioException.badCertificate( | ||
| requestOptions: options, | ||
| error: e.cert, | ||
| ); | ||
| } on SocketException catch (e) { | ||
| if (e.message.contains('timed out')) { | ||
| final Duration effectiveTimeout; | ||
|
|
@@ -174,7 +232,12 @@ class IOHttpClientAdapter implements HttpClientAdapter { | |
| } | ||
| final responseStream = await future; | ||
|
|
||
| if (validateCertificate != null) { | ||
| if (validateCertificate != null && createHttpClient != null) { | ||
| // Legacy post-response validation: only runs when the user supplied | ||
| // a custom [createHttpClient]. In that case we cannot install the | ||
| // pre-emission [HttpClient.connectionFactory] without clobbering the | ||
| // user's client, so the callback runs after the response head has | ||
| // arrived. See the doc-comment on [validateCertificate]. | ||
| final host = options.uri.host; | ||
| final port = options.uri.port; | ||
| final bool isCertApproved = validateCertificate!( | ||
|
|
@@ -244,7 +307,139 @@ class IOHttpClientAdapter implements HttpClientAdapter { | |
| return createHttpClient!(); | ||
| } | ||
| final client = HttpClient()..idleTimeout = const Duration(seconds: 3); | ||
| if (validateCertificate != null) { | ||
| client.connectionFactory = _pinnedConnectionFactory; | ||
| } | ||
| // ignore: deprecated_member_use, deprecated_member_use_from_same_package | ||
| return onHttpClientCreate?.call(client) ?? client; | ||
| } | ||
|
|
||
| /// Custom connection factory that performs TLS handshake and runs | ||
| /// [validateCertificate] *before* yielding the socket to [HttpClient], | ||
| /// so a rejected certificate cannot leak request bytes. | ||
| Future<ConnectionTask<Socket>> _pinnedConnectionFactory( | ||
| Uri url, | ||
| String? proxyHost, | ||
| int? proxyPort, | ||
| ) async { | ||
| // Plain HTTP β no TLS handshake to gate. | ||
| if (url.scheme != 'https') { | ||
| if (proxyHost != null) { | ||
| return Socket.startConnect(proxyHost, proxyPort!); | ||
| } | ||
| return Socket.startConnect(url.host, url.port); | ||
| } | ||
|
|
||
| // Snapshot at call time so a runtime mutation of the public field | ||
| // is honored on every new connection. | ||
| final validator = validateCertificate; | ||
|
|
||
| if (proxyHost != null) { | ||
| return _connectHttpsViaProxy(url, proxyHost, proxyPort!, validator); | ||
| } | ||
|
|
||
| final socketFuture = () async { | ||
| final ss = await SecureSocket.connect( | ||
| url.host, | ||
| url.port, | ||
| supportedProtocols: const ['http/1.1'], | ||
| // When [validateCertificate] is set, defer all certificate validation | ||
| // to the user's callback. This makes self-signed and pinned-CA setups | ||
| // work without forcing the user to also supply [createHttpClient]. | ||
| onBadCertificate: validator == null ? null : (_) => true, | ||
| ); | ||
| if (validator != null) { | ||
| _validatePeerCertificate(ss, url.host, url.port, validator); | ||
| } | ||
| return ss as Socket; | ||
| }(); | ||
| return ConnectionTask.fromSocket(socketFuture, () {}); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed differently in 8a66e1e β literal vendoring is blocked because Dart 3.0 made Switched to |
||
| } | ||
|
|
||
| Future<ConnectionTask<Socket>> _connectHttpsViaProxy( | ||
| Uri target, | ||
| String proxyHost, | ||
| int proxyPort, | ||
| ValidateCertificate? validator, | ||
| ) async { | ||
| Socket? proxySocket; | ||
| bool cancelled = false; | ||
|
|
||
| final socketFuture = () async { | ||
| proxySocket = await Socket.connect(proxyHost, proxyPort); | ||
| if (cancelled) { | ||
| proxySocket!.destroy(); | ||
| throw const SocketException('connection cancelled'); | ||
| } | ||
|
|
||
| // HTTP/1.1 CONNECT tunnel preamble. | ||
| const crlf = '\r\n'; | ||
| final preamble = StringBuffer() | ||
| ..write('CONNECT ${target.host}:${target.port} HTTP/1.1$crlf') | ||
| ..write('Host: ${target.host}:${target.port}$crlf') | ||
| ..write(crlf); | ||
| proxySocket!.write(preamble.toString()); | ||
|
|
||
| final completer = Completer<void>(); | ||
| late StreamSubscription<List<int>> subscription; | ||
| subscription = proxySocket!.listen( | ||
| (event) { | ||
| if (completer.isCompleted) { | ||
| return; | ||
| } | ||
| final response = ascii.decode(event); | ||
| final statusLine = response.split(crlf).first; | ||
| if (statusLine.contains(' 200 ')) { | ||
| completer.complete(); | ||
| } else { | ||
| completer.completeError( | ||
| SocketException( | ||
| 'Proxy CONNECT failed: $statusLine ' | ||
| '(host=${target.host}, port=${target.port})', | ||
| ), | ||
| ); | ||
| } | ||
| }, | ||
| onError: (Object e, StackTrace s) { | ||
| if (!completer.isCompleted) { | ||
| completer.completeError(e, s); | ||
| } | ||
| }, | ||
| ); | ||
| try { | ||
| await completer.future; | ||
| } finally { | ||
| await subscription.cancel(); | ||
| } | ||
|
|
||
| final ss = await SecureSocket.secure( | ||
| proxySocket!, | ||
| host: target.host, | ||
| supportedProtocols: const ['http/1.1'], | ||
| onBadCertificate: validator == null ? null : (_) => true, | ||
| ); | ||
| if (validator != null) { | ||
| _validatePeerCertificate(ss, target.host, target.port, validator); | ||
| } | ||
| return ss as Socket; | ||
| }(); | ||
|
|
||
| return ConnectionTask.fromSocket(socketFuture, () { | ||
| cancelled = true; | ||
| proxySocket?.destroy(); | ||
| }); | ||
| } | ||
|
|
||
| void _validatePeerCertificate( | ||
| SecureSocket ss, | ||
| String host, | ||
| int port, | ||
| ValidateCertificate validator, | ||
| ) { | ||
| final cert = ss.peerCertificate; | ||
| if (!validator(cert, host, port)) { | ||
| ss.destroy(); | ||
| throw _BadCertificateException(host, port, cert); | ||
| } | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert since we don't match the requirement here.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted in 8a66e1e β back to |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted in 8a66e1e β back to |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| -----BEGIN CERTIFICATE----- | ||
| MIIDJTCCAg2gAwIBAgIUaIC3vR7spGRPzpupdBHLOndap4QwDQYJKoZIhvcNAQEL | ||
| BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI2MDUxMDE0MjA1N1oXDTM2MDUw | ||
| NzE0MjA1N1owFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEF | ||
| AAOCAQ8AMIIBCgKCAQEApLUCQLEMhQ5M43KR/hpLLWnVOqSfvqFjFUGbVMWUgISl | ||
| YaHhbzlmFs+CyZrU0dd0yiJSu5WLAqY1acPch19MkVGnRIGp1Q2GMnGwQ46ai8BG | ||
| A3TG7gdZ25bHZbcqFPZKSHGE2yLZrfm3dNuWyj0QfRB9XJj6yqAOVbzuL48AOw1N | ||
| kf9f+T7UIKX1tJouHBFidVFZAS8rZkQ4RWZHW4hgF2pooQ8hPClZvVQHLQMvddrC | ||
| WkaBiRMvUjqX/VcqwcpCovU39thpAhkL9SGq1PHPsRkIyhpdgDlIbQPzqEQOYB6+ | ||
| EStxzra+Ys92z7qYEHdqoU1X6anckL70Uwg2j0TILwIDAQABo28wbTAdBgNVHQ4E | ||
| FgQU9F7u6WPX2diKewg2CH1w3u3PRyUwHwYDVR0jBBgwFoAU9F7u6WPX2diKewg2 | ||
| CH1w3u3PRyUwDwYDVR0TAQH/BAUwAwEB/zAaBgNVHREEEzARgglsb2NhbGhvc3SH | ||
| BH8AAAEwDQYJKoZIhvcNAQELBQADggEBAJw+isKRWS5t2zNNWLEkUgV8YrsIF0sn | ||
| lhwyK1dktJnBEEo99gU1iA/QjPbgD+fJHiViMooXi7ecE2hCu5tNK3QKzQgyY59B | ||
| mu23OM3YbByVwLXEw09tp2ZvSdnafKLDyb/JyvQJI+BvzdUeqLDeUwiuMxyvVDtD | ||
| Zp7Ktghbpo3itnQd8i5gQR7jRLoFmSvx0QHMotJHQ+aLsh13cuCQgLu2F1Ie2YHr | ||
| DxCAoL0FXrNALM5eOOIzdZaxnw/Z/i2jKumz1yv8UskcyMwzOxwMNmDPomXwPxQt | ||
| JKsnCy52ujrkt/qHyNb8Gl0N445gG2AeyIKNUCNXJC7lc2BWdw7Z55A= | ||
| -----END CERTIFICATE----- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCktQJAsQyFDkzj | ||
| cpH+GkstadU6pJ++oWMVQZtUxZSAhKVhoeFvOWYWz4LJmtTR13TKIlK7lYsCpjVp | ||
| w9yHX0yRUadEganVDYYycbBDjpqLwEYDdMbuB1nblsdltyoU9kpIcYTbItmt+bd0 | ||
| 25bKPRB9EH1cmPrKoA5VvO4vjwA7DU2R/1/5PtQgpfW0mi4cEWJ1UVkBLytmRDhF | ||
| ZkdbiGAXamihDyE8KVm9VActAy912sJaRoGJEy9SOpf9VyrBykKi9Tf22GkCGQv1 | ||
| IarU8c+xGQjKGl2AOUhtA/OoRA5gHr4RK3HOtr5iz3bPupgQd2qhTVfpqdyQvvRT | ||
| CDaPRMgvAgMBAAECggEACHe3Y26WwFptzS933k8Mv5mrrmkwlCMTlV47J2W3AHp0 | ||
| 3Q58/js+mMOCAPwku2InyamooN+oBSuRyU19wTgibIkK9T1lEcCxTFcqDYVN0igI | ||
| PBldHCH6D2KdVHrCzV3cyMs3epxnAcDkjK1/1LH9iy+SAXTcKvqFOUGHP0rLRva5 | ||
| 6C9byDUMbnoTA1M3RY/EleEc9Qs8m/hQqt1uyO5x7o/h5pzlVzppuVTIlUKnfmtA | ||
| pwUpLQjtnC3QHRaqLzWQCV5WUTsb1JdA1ykU6XqZ2FMcq6akOukdxi1NV6cP49qN | ||
| V6q7KiqzbHT57T+mi+kWzVxMZglNS+DUSm/18aaK2QKBgQDbD5lk/d2PX7ZGh0RD | ||
| Up4p1AnqF5PNsiGIbo2twkBYrd5NeDuosvM1qVfOKtOzaKwHtoqqUcGYMj3/vkCc | ||
| Bym+78Ud2X7VrQ+WSNERjqF5fB19Dp2PxBFIbJn2ZTATZ01JNna5E1KXNPg0Uq7q | ||
| GvpcZM6SWgZvAAXBY5faPzdTJwKBgQDAexExVBZs7bn3evhw3hR93vf8nO17Jszg | ||
| mutVsNfd1VxwgdpVb6d9qFl/7LpuAI4kiB3F+HH7GVPNcdJyCObwNKFs/M1RqDRA | ||
| HQAGLyODWW97+MNpUX7KJ8zL5i2GBM1dKlyjl4AlIepPZpd4UPAVJ4VnkZPaHHNm | ||
| 772YdKNnuQKBgQC8FQeiM2mgqQ/qPEd6+ht8VGbyy62GpZOu/SS99JOk0BuXLk0i | ||
| ygqRn0UZaaH8XzmdIbirBakPkMu0odf9XxUTr+/xcgU5Vu6UHQ8MYQb5NHxpHDxh | ||
| 7HjeUwhaCoUdk8tCufVzcEiwNLWKzxcJP4KIA7Fs7MirUzydz3HY3AAXbwKBgD8Z | ||
| 2WgPhg6N3NKKKNpvo84kA654D864yJ+1igMcp5gc82Ia3+X0ZbdnMngitneLjQ3i | ||
| 5cfaDBvikLugXfpueq8ywd0F/5WOjBqcpz5fw8ey0T6WLhHf2q4RYC1UN4ZhGqY5 | ||
| Vgd3ilYiCTugiXWzKxH0U+LBMfRmMVsEq2ZIjq1ZAoGAdppPyGZlNKzkbK29cX/F | ||
| AEjGNmEtNwUxpVZHyd9awxKY6G0WcsVFXL1AMB9uJWqFwjppg3sZHWdxr1mFGLwK | ||
| PT7TEg1KEESqQkklLzDaU6CNzFx90U/4NAP4m2WDbr08yf8c65pr/1DPvEBnQQX0 | ||
| T6d48kjMfGmnhoRhNRh+fr8= | ||
| -----END PRIVATE KEY----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about other adapters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix is intentionally scoped to
IOHttpClientAdaptersince that's the adapter with the bug. Quick rundown of the others:plugins/http2_adapterβ already does pre-emission validation correctly atconnection_manager_imp.dart:133-150. This PR bringsIOHttpClientAdapterto parity.plugins/web_adapterβ novalidateCertificatefield exists on the web adapter; TLS is browser-controlled and not introspectable from Dart. Not applicable.plugins/native_dio_adapterβ delegates tocronet_http/cupertino_http; TLS pinning is platform-native (Android NSC, iOS ATS/TrustKit), not a dio-level callback. Not applicable.Happy to add this as a one-line cross-reference comment in
io_adapter.dartif you'd like that captured in source.