Proposal: `SecurityInfo` in `WebRequest` #899

vkrot-cell · 2025-11-05T12:46:24Z

The proposal following discussion at #882

This change allows to get ssl_info in TrustedHeaderClient, which is used by url_loader and web_socket. The change won't impact performance of url_loader or web_socket, that does not use trusted_headers_client, which is only used in two places, and this is where ssl_info is needed for extensions api in future: extensions/browser/api/web_request/web_request_proxying_websocket.cc extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc Extensions proposal: w3c/webextensions#899 Isolated Web App API proposal: explainers-by-googlers/security-info-web-request#1 I have added low coverage reason because websocket.cc is not tested via unit tests, but with chrome/browser/net/websocket_browsertest.cc. It is out of scope of my CL to investigate why unit tests are absent for websocket.cc. Change-Id: I40f0fcb40db7a5efeaaf18276d6cc9bf6c7b48e3 Bug: 458045659 Low-Coverage-Reason: HARD_TO_TEST Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106780 Reviewed-by: Adam Rice <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Andrew Rayskiy <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541131}

This affects the override we have, however there is no use for this argument in our use-case. Chromium changes: https://chromium.googlesource.com/chromium/src/+/c62fa5cbaa4542755128cda06cfd1ea813e41103 commit c62fa5cbaa4542755128cda06cfd1ea813e41103 Author: Vlad Krot <[email protected]> Date: Thu Nov 6 03:07:35 2025 -0800 Add ssl info to TrustedHeaderClient This change allows to get ssl_info in TrustedHeaderClient, which is used by url_loader and web_socket. The change won't impact performance of url_loader or web_socket, that does not use trusted_headers_client, which is only used in two places, and this is where ssl_info is needed for extensions api in future: extensions/browser/api/web_request/web_request_proxying_websocket.cc extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc Extensions proposal: w3c/webextensions#899 Isolated Web App API proposal: explainers-by-googlers/security-info-web-request#1 I have added low coverage reason because websocket.cc is not tested via unit tests, but with chrome/browser/net/websocket_browsertest.cc. It is out of scope of my CL to investigate why unit tests are absent for websocket.cc. Change-Id: I40f0fcb40db7a5efeaaf18276d6cc9bf6c7b48e3 Bug: 458045659 Low-Coverage-Reason: HARD_TO_TEST Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106780 Reviewed-by: Adam Rice <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Andrew Rayskiy <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541131}

vkrot-cell · 2025-11-19T13:27:26Z

@Rob--W Hi, any comments on the proposal?

@devlin

The change parses SecurityInfo object according to specification: w3c/webextensions#899 @devlin has given approval to start working on this feature before the spec is finalized. Fixed: 460072357 Change-Id: I160c4b61c0d2da46eab10fae4e0794c7f692d26f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7167074 Commit-Queue: Vlad Krot <[email protected]> Reviewed-by: Dominic Battre <[email protected]> Cr-Commit-Position: refs/heads/main@{#1547169}

Rob--W · 2025-11-19T16:33:14Z

@Rob--W Hi, any comments on the proposal?

I'm still in Japan following TPAC last week. I also have not had the chance to query for API usage (which I mentioned in https://github.com/w3c/webextensions/blob/main/_minutes/2025-11-06-wecg.md ), but that does not have to block my review.

The most important part is to verify that similar names and properties have equivalent meanings across browsers. I am not available until mid December, but if this is really pressing I can ask another team member to assist in the verification (or do that myself when idle). On what timeframe do you need feedback?

vkrot-cell · 2025-11-19T16:39:11Z

@Rob--W It is fine for me to wait a week or two.

This affects the override we have, however there is no use for this argument in our use-case. Chromium changes: https://chromium.googlesource.com/chromium/src/+/c62fa5cbaa4542755128cda06cfd1ea813e41103 commit c62fa5cbaa4542755128cda06cfd1ea813e41103 Author: Vlad Krot <[email protected]> Date: Thu Nov 6 03:07:35 2025 -0800 Add ssl info to TrustedHeaderClient This change allows to get ssl_info in TrustedHeaderClient, which is used by url_loader and web_socket. The change won't impact performance of url_loader or web_socket, that does not use trusted_headers_client, which is only used in two places, and this is where ssl_info is needed for extensions api in future: extensions/browser/api/web_request/web_request_proxying_websocket.cc extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc Extensions proposal: w3c/webextensions#899 Isolated Web App API proposal: explainers-by-googlers/security-info-web-request#1 I have added low coverage reason because websocket.cc is not tested via unit tests, but with chrome/browser/net/websocket_browsertest.cc. It is out of scope of my CL to investigate why unit tests are absent for websocket.cc. Change-Id: I40f0fcb40db7a5efeaaf18276d6cc9bf6c7b48e3 Bug: 458045659 Low-Coverage-Reason: HARD_TO_TEST Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106780 Reviewed-by: Adam Rice <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Andrew Rayskiy <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541131}

@devlin

This CL adds main logic for intercepting TLS certificate info in Web Request. @devlin has given his approval to start implementation while the proposal is still not approved. The feature is behind feature flag. The presubmit.py was modified to allow --ignore-certificate-errors in tests. The flag is necessary because tests need to check code path with expired certificate. The flag network::switches::kIgnoreCertificateErrorsSPKIList does not work for this case, because it treats invalid cert as valid, producing state="secure". Proposal: w3c/webextensions#899 Regarding low coverage reason, the mentioned files are tested via api test currently chrome/browser/extensions/api/web_request/web_request_apitest.cc and I consider it out of scope of my CL to introduce unit tests from 0 to existing files. Low-Coverage-Reason: HARD_TO_TEST Bug: 458045659 Change-Id: I29af4f38591457fb1e09c641b6385c5b7c6e8f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7166297 Reviewed-by: Andrew Grieve <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Reviewed-by: Tim <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Kelvin Jiang <[email protected]> Cr-Commit-Position: refs/heads/main@{#1551464}

This affects the override we have, however there is no use for this argument in our use-case. Chromium changes: https://chromium.googlesource.com/chromium/src/+/c62fa5cbaa4542755128cda06cfd1ea813e41103 commit c62fa5cbaa4542755128cda06cfd1ea813e41103 Author: Vlad Krot <[email protected]> Date: Thu Nov 6 03:07:35 2025 -0800 Add ssl info to TrustedHeaderClient This change allows to get ssl_info in TrustedHeaderClient, which is used by url_loader and web_socket. The change won't impact performance of url_loader or web_socket, that does not use trusted_headers_client, which is only used in two places, and this is where ssl_info is needed for extensions api in future: extensions/browser/api/web_request/web_request_proxying_websocket.cc extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc Extensions proposal: w3c/webextensions#899 Isolated Web App API proposal: explainers-by-googlers/security-info-web-request#1 I have added low coverage reason because websocket.cc is not tested via unit tests, but with chrome/browser/net/websocket_browsertest.cc. It is out of scope of my CL to investigate why unit tests are absent for websocket.cc. Change-Id: I40f0fcb40db7a5efeaaf18276d6cc9bf6c7b48e3 Bug: 458045659 Low-Coverage-Reason: HARD_TO_TEST Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106780 Reviewed-by: Adam Rice <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Andrew Rayskiy <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541131}

This CL adds logic to proxy web socket to obtain ssl info from there. This follows previous CL - https://chromium-review.googlesource.com/c/chromium/src/+/7166297/1 Proposal: w3c/webextensions#899 Bug: 458045659 Change-Id: Id0773cee906ee5c145faae642b5de71f40e087f3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7165511 Reviewed-by: Finnur Thorarinsson <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1553911}

This affects the override we have, however there is no use for this argument in our use-case. Chromium changes: https://chromium.googlesource.com/chromium/src/+/c62fa5cbaa4542755128cda06cfd1ea813e41103 commit c62fa5cbaa4542755128cda06cfd1ea813e41103 Author: Vlad Krot <[email protected]> Date: Thu Nov 6 03:07:35 2025 -0800 Add ssl info to TrustedHeaderClient This change allows to get ssl_info in TrustedHeaderClient, which is used by url_loader and web_socket. The change won't impact performance of url_loader or web_socket, that does not use trusted_headers_client, which is only used in two places, and this is where ssl_info is needed for extensions api in future: extensions/browser/api/web_request/web_request_proxying_websocket.cc extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc Extensions proposal: w3c/webextensions#899 Isolated Web App API proposal: explainers-by-googlers/security-info-web-request#1 I have added low coverage reason because websocket.cc is not tested via unit tests, but with chrome/browser/net/websocket_browsertest.cc. It is out of scope of my CL to investigate why unit tests are absent for websocket.cc. Change-Id: I40f0fcb40db7a5efeaaf18276d6cc9bf6c7b48e3 Bug: 458045659 Low-Coverage-Reason: HARD_TO_TEST Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106780 Reviewed-by: Adam Rice <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Andrew Rayskiy <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541131}

vkrot-cell · 2025-12-08T14:27:15Z

Hi @Rob--W
Can you take a look this week?

Rob--W · 2025-12-08T15:11:29Z

Hi @Rob--W
Can you take a look this week?

I'll try to do so, yes. I wrote "mid December" before to have a bit of buffer, but I actually put this issue on my todo list for this week.

This affects the override we have, however there is no use for this argument in our use-case. Chromium changes: https://chromium.googlesource.com/chromium/src/+/c62fa5cbaa4542755128cda06cfd1ea813e41103 commit c62fa5cbaa4542755128cda06cfd1ea813e41103 Author: Vlad Krot <[email protected]> Date: Thu Nov 6 03:07:35 2025 -0800 Add ssl info to TrustedHeaderClient This change allows to get ssl_info in TrustedHeaderClient, which is used by url_loader and web_socket. The change won't impact performance of url_loader or web_socket, that does not use trusted_headers_client, which is only used in two places, and this is where ssl_info is needed for extensions api in future: extensions/browser/api/web_request/web_request_proxying_websocket.cc extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc Extensions proposal: w3c/webextensions#899 Isolated Web App API proposal: explainers-by-googlers/security-info-web-request#1 I have added low coverage reason because websocket.cc is not tested via unit tests, but with chrome/browser/net/websocket_browsertest.cc. It is out of scope of my CL to investigate why unit tests are absent for websocket.cc. Change-Id: I40f0fcb40db7a5efeaaf18276d6cc9bf6c7b48e3 Bug: 458045659 Low-Coverage-Reason: HARD_TO_TEST Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106780 Reviewed-by: Adam Rice <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Andrew Rayskiy <[email protected]> Commit-Queue: Vlad Krot <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541131}

Rob--W

Thanks for the patience; this feedback took longer than expected because I wanted to complete the API usage analysis first (shared in the issue at #882 (comment)).