[Connection-Allowlist] Enforce for PaymentRequest API: service worker registration [chromium/src : main]
Xiaochen Zhou (Gerrit)
Xiaochen Zhou added 1 comment![]( "Open in Gerrit")
+shivanisha@ for code review, thank you.
Related details
- Shivani Sharma
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shivani Sharma (Gerrit)
Shivani Sharma added 1 comment![]( "Open in Gerrit")
GlobalRenderFrameHostId requesting_frame_id,Does it not go through ServiceWorkerRegisterJob()? PTAL at https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_register_job.cc;drc=0bedfcc1f8e786bfd905e2d16909da4504df025f;l=82
for how that path creates a new creator_NRID so that even if the creator dies, its CA can be checked. If it doesn't use that path which would have been ideal, we could do something similar or error out if the Frame I'd is valid but the RFH is not.
Related details
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaochen Zhou (Gerrit)
Xiaochen Zhou added 2 comments![]( "Open in Gerrit")
+Payment API owner smcgruer@ for a question, please see the comment below. Thank you.
GlobalRenderFrameHostId requesting_frame_id,Does it not go through ServiceWorkerRegisterJob()? PTAL at https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_register_job.cc;drc=0bedfcc1f8e786bfd905e2d16909da4504df025f;l=82
for how that path creates a new creator_NRID so that even if the creator dies, its CA can be checked. If it doesn't use that path which would have been ideal, we could do something similar or error out if the Frame I'd is valid but the RFH is not.
Does it not go through ServiceWorkerRegisterJob()?
Yes, it does. The ServiceWorkerRegisterJob will receive the policies from `GetPoliciesForRegistration`.
For cases that the RFH has gone at the time of this function call, we have two approaches:
- **Fail-open:** allow the registration to continue with the fallback policies (same as the one constructed by the pre-existing code). The SW registration has no CA to check since the CA is stored in the RFH's policies. A request can go through even though it would have been blocked by CA. But this keeps the pre-existing behavior.
- **Fail-closed:** abort the registration if the RFH has gone.
Fail-closed avoids the risk of data exfiltration of the other option. For reference, for FedCM API, we also agreed to fail-closed: if RFH is gone at the time the CA needs to be retrieved from the RFH, the FedCM API's network requests will not be made.
@smcg...@chromium.org for opinions from Payment API owners, thank you.
Note: neither will affect the behavior of Extension's usage of `ServiceWorkerContextWrapper::RegisterServiceWorker`. Because it passes an invalid frame ID and it will not check whether RFH is alive or not, so it always falls back to the pre-existing behavior. Extension is the only other client of this function.
Related details
- Shivani Sharma
- Stephen McGruer
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen McGruer (Gerrit)
Stephen McGruer added 2 comments![]( "Open in Gerrit")
content::GlobalRenderFrameHostId requesting_frame_id,This shouldn't be needed in this path, since needs_installation_ is false and thus the code will never attempt to install a service worker.
GlobalRenderFrameHostId requesting_frame_id,Xiaochen ZhouDoes it not go through ServiceWorkerRegisterJob()? PTAL at https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_register_job.cc;drc=0bedfcc1f8e786bfd905e2d16909da4504df025f;l=82
for how that path creates a new creator_NRID so that even if the creator dies, its CA can be checked. If it doesn't use that path which would have been ideal, we could do something similar or error out if the Frame I'd is valid but the RFH is not.
Does it not go through ServiceWorkerRegisterJob()?
Yes, it does. The ServiceWorkerRegisterJob will receive the policies from `GetPoliciesForRegistration`.
For cases that the RFH has gone at the time of this function call, we have two approaches:
- **Fail-open:** allow the registration to continue with the fallback policies (same as the one constructed by the pre-existing code). The SW registration has no CA to check since the CA is stored in the RFH's policies. A request can go through even though it would have been blocked by CA. But this keeps the pre-existing behavior.
- **Fail-closed:** abort the registration if the RFH has gone.
Fail-closed avoids the risk of data exfiltration of the other option. For reference, for FedCM API, we also agreed to fail-closed: if RFH is gone at the time the CA needs to be retrieved from the RFH, the FedCM API's network requests will not be made.
@smcg...@chromium.org for opinions from Payment API owners, thank you.
Note: neither will affect the behavior of Extension's usage of `ServiceWorkerContextWrapper::RegisterServiceWorker`. Because it passes an invalid frame ID and it will not check whether RFH is alive or not, so it always falls back to the pre-existing behavior. Extension is the only other client of this function.
IIUC, for PaymentRequest this scenario would mean that:
1. Website created new PaymentRequest([{supportedMethods: 'https://paymentapp.example', ..}], ...);
2. `https://paymentapp.example` does not yet have a Payment Handler service worker installed
3. Website calls PaymentRequest.show(), which triggers install via ServiceWorkerPaymentApp::InvokePaymentApp
4. Sometime in-between the website calling that and the service worker actually being installed, the website goes away (e.g. via the tab being closed)
5. We hit this point, and have to decide if we continue to install the service worker or not.
Is that accurate?
Assuming so, I think we're fine to fail closed here. The payment app will just be installed when the next website that wants to invoke it creates a PaymentRequest and calls show(). I don't think there's any harm to cancelling the install (and if there was, then that would be a bug anyway since anything could interrupt it, e.g. a power cut 😄).
Related details
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |