Skip to content

[webview_flutter] Adds support to respond to recoverable SSL certificate errors #9150

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

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

bparrishMines
Copy link
Contributor

@bparrishMines bparrishMines commented Apr 25, 2025

tl;dr This adds the NavigationDelegate.onSslAuthError method for recoverable SSL certificate errors.
Fixes flutter/flutter#36925

Important Note: This is a more niche feature that should only be used for logging errors or used in a development environment. See docs for WebViewClient.onReceviedSslError. Therefore, I included multiple comments throughout that highly recommends calling cancel() and ensured that we call or set cancel/performDefaultHandling when the method is not set with unit tests.

The respective platform callback methods are:
Android: WebViewClient.onReceviedSslError
iOS/macOS: WKNavigationDelegate(_:didReceive:completionHandler:)

The Android method is only called for recoverable SSL errors while the iOS/macOS method is called for all authentication related callbacks. So for iOS/macOS, the callback is only implemented when:

  1. URLAuthenticationChallenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust
  2. SecTrustEvaluateWithError() returns false when passed URLAuthenticationChallenge.protectionSpace.serverTrust.
  3. SecTrustGetTrustResult() returns SecTrustResultType.recoverableTrustFailure when passed URLAuthenticationChallenge.protectionSpace.serverTrust.

This implementation only really provides the error description, certificate data, and the ability to proceed or cancel. Providing access to verify or manipulate a certificate seem to be outside the scope of a WebView plugin since Android and iOS/macOS have an extensive separate library for them.

Providing the encoded data for the X509Certificate should be enough to use the certificate with another library.

A few platform differences to note:

  1. Android provides a String getUrl() for the error while iOS/macOS provides a URLProtectionSpace that provides host, port, protocol, and proxyType. I didn't think I would be able to create a full Dart Uri for iOS/macOS from these reliably, so each platform provides these values separately.
  2. A certificate can have multiple errors, but only Android provides access to them. However, both platforms say that they provide access to the "primary" error, so I just provided that. A platform specific value for Android could be added later.

Side note: Android provides a WebViewClient.onReceivedClientCertRequest to read all certificate requests that don't get a recoverable error. A separate plugin method could be made to receive these.

Side Side note:
Old PR: flutter/plugins#2285
Fixes flutter/flutter#74609
Old quote about this feature from flutter/plugins#2285 (comment):

By default, Flutter plugins should expect or enforce a secure environment, therefore we can not process this PR as-is. However, I can see a use case, especially during local development when it's just may not be feasible to have a properly signed https connection.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

@bparrishMines bparrishMines changed the title Webview ssl [webview_flutter] Adds support to respond to recoverable SSL certificate errors Apr 25, 2025
@bparrishMines bparrishMines marked this pull request as ready for review April 25, 2025 22:46
@async
late List<Object?> Function(
late AuthenticationChallengeResponse Function(
Copy link
Contributor Author

@bparrishMines bparrishMines Apr 25, 2025

Choose a reason for hiding this comment

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

@stuartmorgan-g I changed this back to return an AuthenticationChallengeResponse. Alternatively, I added async constructors for AuthenticationChallengeResponse and URLCredential. Ideally this solution should still prevent flutter/flutter#162437.

If it doesn't, then that would prove that the issue isn't caused by a race condition because the object has to be added to the native InstanceManager first before the async constructor returns.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Some minor comment suggestions, but looks good to split up.

///
/// The host application must call [cancel] or, contrary to secure web
/// communication standards, [proceed] to provide the web view's response to the
/// error.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here and below we should state what the intended use case is, since currently it sounds like we have this whole feature but are telling people never to use it. How about adding one more sentence here: "[proceed] should generally only be used in test environments, as using it in production can expose users to security and privacy risks."

/// the error and continue communicating with the server.
///
/// **Warning:** When an SSL error occurs, the host application should always
/// call [cancel] rather than [proceed] because an invalid SSL certificate
Copy link
Contributor

Choose a reason for hiding this comment

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

And then here maybe change this to: "Warning: Calling [proceed] in a production environment is strongly discouraged, as an invalid SSL certificate means that the connection is not secure, so proceeding can expose users to security and privacy risks."

/// the error and continue communicating with the server.
///
/// **Warning:** When an SSL error occurs, the host application should always
/// call [cancel] rather than [proceed] because an invalid SSL certificate
Copy link
Contributor

Choose a reason for hiding this comment

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

Same wording note here.

final void Function(PlatformSslAuthError)? callback =
delegate._onSslAuthError;
if (callback == null) {
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure I'm understanding the flow here, all of these breaks are because that way we fall through to the final UrlSessionAuthChallengeDisposition.performDefaultHandling call and that will cancel?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants