feat: allow setting SSL config #25461
Conversation
There was a problem hiding this comment.
Implementation looks good.
One thing to make sure is that system network context code path is fine without this change, before network s13n the system network context was dependency for network fetching code of OCSP/CRL/AIA data but now the code path has been modified to use per-profile network context https://bugs.chromium.org/p/chromium/issues/detail?id=829029, so it should not be an issue. The only other use cases of system network context I was able to find was the one used for device_service (for fetching geolocation data) and creating tcp connection for viz devtools, both of which are fine without this change. Might want to ensure any other code paths I might have missed.
|
@deepak1556 hm, I'm not sure what I should be searching for. Can you give me some pointers? Do you think we should add a |
Checking for the usages of these apis https://github.com/electron/electron/blob/master/shell/browser/net/system_network_context_manager.h#L72-L82 should do.
If there is no usage other than the ones I mentioned then its not required to add this, geolocation service is a google maps endpoint which the app doesn't have to control. |
|
I looked and I could only find those two as well. As a precaution, what do you think about setting up the system network context to support the same TLS versions that Chrome does (without adding a new API to edit them)? i.e. min_version = TLS1.2. |
|
yeah that would be a good thing to do |
|
The API WG approved this at the Sept 21, 2020 meeting. |
jkleinsc
left a comment
There was a problem hiding this comment.
Just a small nit - can we note in the documentation what the defaults are if you do not set this config?
|
@jkleinsc NB. these defaults are set by a mojom file inside chromium and we don't have tests that will notify us if they change. So this documentation may go out of date. |
|
Merging as WOA failure is flake unrelated to this change. |
|
Release Notes Persisted
|
Description of Change
This allows apps to configure min and max supported SSL versions. Chromium currently uses a minimum of TLS1.2 by default, whereas Electron defaults to TLS1.0 by default. This gives apps the ability to control supported TLS versions according to their needs.
Checklist
npm testpassesRelease Notes
Notes: Added
session.setSSLConfig()to allow configuring SSL.