Certificate pinning#1065
Conversation
There was a problem hiding this comment.
Pull request overview
Adds native certificate pinning support for SDK-owned TLS traffic (internal HTTPS + WSS signaling) by introducing NetworkOptions/pinning rule configuration, enforcing pins/trust stores in the SDK’s IO HttpClient and WebSocket connection path, and documenting usage.
Changes:
- Introduces certificate pinning configuration types (
NetworkOptions,CertificatePinningOptions,CertificatePinningRule) andCertificatePinningException. - Adds pinning enforcement for SDK-owned HTTP(S) requests and WSS signaling on native platforms (with explicit “unsupported” behavior on Flutter web).
- Adds unit + IO tests and README documentation for pinning modes and rule semantics.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/support/certificate_pinning_test.dart | Unit tests for SPKI pin derivation, PEM/DER helpers, host matching, and rule merging semantics. |
| test/support/certificate_pinning_io_test.dart | Native IO tests validating fail-fast behavior (no HTTP/WSS bytes sent) and trust modes (leaf/trust store/combined). |
| test/mock/websocket_mock.dart | Extends websocket mock connector to capture networkOptions and simulate connect errors. |
| test/core/certificate_pinning_test.dart | Ensures signaling path passes networkOptions through and fast-fails on pinning exceptions. |
| README.md | Documents certificate pinning scope, rule behavior, and configuration examples. |
| pubspec.yaml | Adds asn1lib and crypto dependencies. |
| pubspec.lock | Locks new direct dependency versions (asn1lib, crypto now direct). |
| lib/src/support/websocket/web.dart | Plumbs networkOptions and throws on web when pinning is enabled. |
| lib/src/support/websocket/io.dart | Uses a custom IO HttpClient for WSS when pinning is enabled; rethrows pinning exceptions. |
| lib/src/support/websocket.dart | Extends connector API to accept networkOptions. |
| lib/src/support/region_url_provider.dart | Switches region settings fetch to SDK HTTP client and threads networkOptions. |
| lib/src/support/http_client/web.dart | Web implementation of SDK HTTP client; rejects pinning as unsupported. |
| lib/src/support/http_client/io.dart | IO implementation of SDK HTTP client with pinning-aware connectionFactory. |
| lib/src/support/http_client.dart | Adds SDK-owned sdkHttpGet/sdkHttpHead helpers using the platform client. |
| lib/src/support/certificate_pinning.dart | Implements SPKI pin extraction, PEM/DER utilities, host matching, and validation logic. |
| lib/src/options.dart | Adds NetworkOptions to RoomOptions and defines pinning configuration types. |
| lib/src/exceptions.dart | Adds CertificatePinningException for clear surfacing of pinning failures. |
| lib/src/core/signal_client.dart | Passes networkOptions into websocket + validation HTTP request; special-cases pinning failures. |
| lib/src/core/room.dart | Threads networkOptions into region provider and connection preparation HEAD requests. |
| .changes/add-certificate-pinning | Changelog entry for the new feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (proxyHost != null || proxyPort != null) { | ||
| throw UnsupportedError('Certificate pinning through HTTP proxies is not supported'); | ||
| } | ||
|
|
||
| if (!_isTlsScheme(url.scheme)) { | ||
| return io.Socket.startConnect(url.host, _portFor(url)); | ||
| } |
| final server = await io.SecureServerSocket.bind( | ||
| io.InternetAddress.loopbackIPv4, | ||
| 0, | ||
| context, | ||
| ); |
|
@copilot Does the recent commits address your findings ? |
Yes —
The latest runs for that commit also show |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
lib/src/core/room.dart:239
prepareConnectionswallows all exceptions and logs a generic warning without the underlying error. With certificate pinning enabled, this can hide actionable failures (e.g.,CertificatePinningException) and makes troubleshooting harder than other nearby code paths that log the error.
await sdkHttpHead(Uri.parse(toHttpUrl(regionUrl)), networkOptions: roomOptions.networkOptions);
logger.fine('prepared connection to ${regionUrl}');
}
} else {
await sdkHttpHead(Uri.parse(toHttpUrl(url)), networkOptions: roomOptions.networkOptions);
}
} catch (e) {
logger.warning('could not prepare connection');
}
| Iterable<List<int>> certificateDerCertificates(CertificateBytes certificate) sync* { | ||
| switch (certificate.encoding) { | ||
| case CertificateBytesEncoding.pem: | ||
| yield* _certificateDerCertificatesFromPem(certificate.bytes); | ||
| case CertificateBytesEncoding.der: | ||
| yield certificate.bytes; | ||
| } | ||
| } |
|
|
||
| ### Certificate pinning | ||
|
|
||
| Certificate pinning is available for native platforms through `RoomOptions.networkOptions`. It applies to SDK-owned WSS signaling and internal HTTPS requests. It does not apply to Flutter web, WebRTC media, TURN, or application-owned token endpoints. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| e2eeOptions: e2eeOptions ?? this.e2eeOptions, | ||
| encryption: encryption ?? this.encryption, | ||
| fastPublish: fastPublish ?? this.fastPublish, | ||
| networkOptions: networkOptions ?? this.networkOptions, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔍 RoomOptions.copyWith does not propagate enableVisualizer (pre-existing)
The copyWith method at lib/src/options.dart:308-339 does not include enableVisualizer as a parameter and does not forward it to the new instance. This means any copyWith call (e.g., the E2EE backup codec disable at lib/src/core/room.dart:287) silently resets enableVisualizer to its default (false). This is a pre-existing issue not introduced by this PR, but the PR touched this method to add networkOptions, so it's worth noting. The field is deprecated, which reduces impact.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // clean up before emitting, room's EngineDisconnectedEvent handler | ||
| // drops the event while fullReconnectOnNext is still true and | ||
| // cleanUp() is what resets it | ||
| await cleanUp(); | ||
| events.emit(EngineDisconnectedEvent( | ||
| reason: DisconnectReason.disconnected, | ||
| reason: e is CertificatePinningException | ||
| ? DisconnectReason.signalingConnectionFailure | ||
| : DisconnectReason.disconnected, | ||
| )); |
There was a problem hiding this comment.
🔍 Reordered cleanUp-before-emit changes disconnect event semantics for all non-recoverable errors
The old code emitted EngineDisconnectedEvent first, then called cleanUp(). The new code reverses this order (lines 1112-1117). The comment explains this is needed because Room's EngineDisconnectedEvent handler at lib/src/core/room.dart:596-602 checks !engine.fullReconnectOnNext — if cleanUp() hasn't run yet, fullReconnectOnNext may still be true and the Room handler would silently drop the disconnect event. This fix applies to all non-recoverable errors (including the pre-existing UnexpectedConnectionState), not just CertificatePinningException. This is likely an improvement for the existing case too, but it's a behavioral change worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // while reconnecting, attemptReconnect owns disconnect handling and | ||
| // emits EngineDisconnectedEvent itself, relaying here as well would | ||
| // race it with a duplicate event. _attemptingReconnect covers the | ||
| // window where cleanUp() has already reset _isReconnecting but | ||
| // attemptReconnect has not finished its error handling yet | ||
| if (!_isReconnecting && !_attemptingReconnect) { | ||
| events.emit(EngineDisconnectedEvent( | ||
| reason: event.reason, | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔍 New reconnect guard on SignalDisconnectedEvent affects all disconnect scenarios, not just pinning
The added guard if (!_isReconnecting && !_attemptingReconnect) at line 1376 suppresses EngineDisconnectedEvent emission for signalingConnectionFailure during any reconnect attempt, not only certificate pinning failures. Previously, a signalingConnectionFailure during reconnect would always emit EngineDisconnectedEvent here, potentially racing with the engine's own reconnect error handling. The new guard prevents duplicate events. This is a general behavioral improvement but changes the event flow for all reconnect-time signaling failures. If any downstream code relied on receiving this event during reconnect, it would break.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (socketError is CertificatePinningException) { | ||
| // In reconnect mode the engine owns state and event emission, | ||
| // emitting here would race its reconnect handling. | ||
| if (!reconnect) { | ||
| _connectionState = ConnectionState.disconnected; | ||
| events.emit(SignalDisconnectedEvent(reason: DisconnectReason.signalingConnectionFailure)); | ||
| } | ||
| rethrow; | ||
| } |
There was a problem hiding this comment.
🔍 Potential duplicate EngineDisconnectedEvent on initial connect with CertificatePinningException
During initial (non-reconnect) connect, a CertificatePinningException triggers two EngineDisconnectedEvent emissions: (1) the signal client emits SignalDisconnectedEvent at lib/src/core/signal_client.dart:179, which the engine's signal listener at lib/src/core/engine.dart:1376-1380 relays as EngineDisconnectedEvent (since _isReconnecting and _attemptingReconnect are both false during initial connect), and (2) engine.connect catches the rethrown exception and emits EngineDisconnectedEvent at lib/src/core/engine.dart:271. This is the same pattern as the pre-existing signalingConnectionFailure path (the finally block at signal_client.dart:219 also emits SignalDisconnectedEvent), so it's not a regression, but Room-level listeners may see two disconnect events for a single failure.
Was this helpful? React with 👍 or 👎 to provide feedback.
## Summary Follow-up to #1065. Ensures the SDK emits exactly one `RoomDisconnectedEvent` per failed connection attempt. Previously a failed initial connect produced two `EngineDisconnectedEvent`s, and therefore two `RoomDisconnectedEvent`s and two room cleanups: 1. The signal client emits `SignalDisconnectedEvent(signalingConnectionFailure)` while `connect()` is failing (the validate path has done this since #406, and the certificate pinning path in #1065 follows the same pattern), which the engine relayed as `EngineDisconnectedEvent`. 2. `Engine.connect`'s catch then emits `EngineDisconnectedEvent(joinFailure)` for the same failure. ## Changes - `Engine.connect`'s catch is now the single emitter for initial connect failures. It picks the reason by error type: `signalingConnectionFailure` for `CertificatePinningException`, `joinFailure` otherwise. - The `signalingConnectionFailure` relay in the engine's signal listener is removed (with a comment explaining why). During reconnects the engine's reconnect handling already owns disconnect emission, so the relay only ever produced duplicates. This also removes the `_isReconnecting`/`_attemptingReconnect` guard that #1065 added to suppress the relay during reconnects, since there is no longer anything to suppress. - `SignalDisconnectedEvent` at the signal level is unchanged, only the engine-level relay is removed. ## Behavior change Apps listening for `RoomDisconnectedEvent` now receive one event per failed connect instead of two. The reasons are unchanged for the common case (`joinFailure`); certificate pinning failures surface as `signalingConnectionFailure`. Apps that specifically depended on receiving both events for a single failure would see one. ## Testing Added room-level tests through the mock e2e container asserting exactly one `RoomDisconnectedEvent` per failed initial connect, for both a certificate pinning failure (`signalingConnectionFailure`) and a generic websocket failure (`joinFailure`). Both tests fail against the previous code (two events observed) and pass with this change.
Summary
Adds native certificate pinning support for SDK-owned TLS traffic in the Flutter SDK.
UnsupportedErroratRoom.connectinstead of silently connecting without pinningCertificateBytesCertificatePinningExceptionso pinning failures can fail fast and surface clearlyDisconnectReason.signalingConnectionFailureinstead of retrying TLS handshakes against an untrusted endpointEngineDisconnectedEventwas emitted before engine cleanup in the non-recoverable reconnect branch, which caused the room to drop the event during full reconnectsRule Behavior
Rules are selected by host. Exact hosts, single-label wildcards like
*.livekit.cloud, multi-label wildcards like**.livekit.cloud, and*are supported.*.livekit.cloudmatchesproject.livekit.cloudbut nota.b.livekit.cloud;**.livekit.cloudmatches both. Emptyhostsapplies a rule to every SDK-owned TLS connection.Hosts that match no rule are connected with platform trust only and the SDK logs a warning. LiveKit Cloud region failover connects to server-provided regional hostnames like
project.region.production.livekit.cloud, which carry more labels than the project URL — use**.livekit.cloudso pinning also covers those connections.All rules that match a host are applied. Within one check type, any configured value may match. Across check types, each configured type must pass.
That means:
primaryPinsandbackupPinsare both acceptedtrustedCertificatesreplaces platform trusted roots for that matching hostSPKI pins are matched against the leaf certificate's public key only. Dart does not expose the rest of the chain, so pinning an intermediate or root CA key never matches.
Example: SPKI Pins
Use this when possible. It is friendlier to certificate renewal because the pin follows the public key rather than the full leaf certificate.
Example: Exact Leaf Certificate
Use this when the app should trust one exact leaf certificate loaded from assets. Renewing or changing the leaf certificate requires shipping updated certificate bytes unless another configured check also allows the new certificate.
pinnedLeafCertificatesis an identity check after TLS trust validation, so private or self-signed certificates also needtrustedCertificates.Example: Custom Trust Store
Use this for the asset-based Flutter pattern where the app ships a leaf, intermediate, or root certificate as trust material. When a pinning rule matches a host, the SDK owns TLS setup for that connection and
HttpOverrides.global/badCertificateCallbackare not consulted — usetrustedCertificatesfor self-signed or private PKI setups.Example: Combine Modes
Checks can be combined. This requires the TLS chain to validate against the custom trust store and the peer certificate SPKI to match one configured pin.
Testing
Local verification:
flutter analyze --no-pub flutter test --no-pub --reporter compact