Skip to content
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

If the FetchEvent returns the integrity value #71

Closed
xzhan96 opened this issue Jun 16, 2017 · 8 comments
Closed

If the FetchEvent returns the integrity value #71

xzhan96 opened this issue Jun 16, 2017 · 8 comments

Comments

@xzhan96
Copy link

xzhan96 commented Jun 16, 2017

I am working a chromium fetch test issue, the test is to set a script.integrity, and then determine whether this integrity value and FetchEvent returned integrity value are equal.
My question is that if the spec require FetchEvent return this value? I didn't find that here https://www.w3.org/TR/SRI/.
Thanks a lot for your kindly answer!

Some test code:

function script_integrity_test(frame, url, integrity, expected_integrity) {                                                                                                         
   var actual_url = url + (++url_count);                                                                                                                                           
   expected_results[actual_url] = {                                                                                                                                                
       url: actual_url,                                                                                                                                                            
       mode: 'no-cors',                                                                                                                                                            
       credentials: 'include',                                                                                                                                                     
       redirect: 'follow',                                                                                                                                                         
       integrity: expected_integrity,                                                                                                                                              
       message: 'Script load (url:' + actual_url + ')'                                                                                                                             
     };                                                                                                                                                                            
   return frame.contentWindow.load_script_with_integrity(actual_url, integrity);                                                                                                     
} 
---------------------------
function load_script_with_integrity(url, integrity) {
 var script = document.createElement('script');
 script.src = url;
 script.integrity = integrity;
 document.body.appendChild(script);
}
----------------------------
assert_equals(result.integrity, expected.integrity)
@xzhan96 xzhan96 changed the title Will the FetchEvent return the integrity value If the FetchEvent returns the integrity value Jun 20, 2017
@makotoshimazu
Copy link

We should also consider the foreign fetch case. If we think service workers are similar to servers, I guess requests reaching to the foreign fetch handler on the service worker shouldn't have the integrity value.

@mfalken
Copy link
Member

mfalken commented Jun 20, 2017

This question may be better for the service worker spec repository @wanderview @jungkees @jakearchibald.

The question is that if you have something like <script src='a.js' integrity='foo'>, and a service worker intercepts the request, in the fetch event should request.integrity = 'foo' ? Does the answer change if it's a foreign fetch interception?

I'm not familiar with integrity and its security considerations so wanted to see what others think.

@wanderview
Copy link
Member

I think the spec currently exposes this information. The FetchEvent just wraps the internal request with a DOM Request object. Since the internal request has the integrity property the outer Request.integrity reflects that value. I don't see anything that tries to hide the integrity value.

@xzhan96
Copy link
Author

xzhan96 commented Jun 21, 2017

@wanderview Thanks a lot for you clarification, understand that. :)
@makotoshimazu, @mattto do you have more comments?

@xzhan96 xzhan96 closed this as completed Jun 21, 2017
@xzhan96 xzhan96 reopened this Jun 21, 2017
@makotoshimazu
Copy link

I don't have any objections for it. I think chromium can also implement it.

@xzhan96
Copy link
Author

xzhan96 commented Jun 26, 2017

yeah, I mean based on that, web-platform-test: fetch-request-resources.https.html is correct and should be passed, so let's do the further discussion in crrev.com/2941883003.

@makotoshimazu
Copy link

I have one question:
When passing the integrity attribute to the service worker, where should the integrity value be evaluated?
I think there are two places to do that in the flow of requests like: Page --[1]--> SW --[2]--> Network

If SW does like e.respondWith(fetch(e.request)), I guess checking the integrity value happens on [1] and [2], and if the SW creates another request internally, it happens only on [1]. Is this right?
// I'm also not familiar with integrity, so sorry if it's obvious thing.

@jakearchibald
Copy link

jakearchibald commented Jun 26, 2017

In the case of e.respondWith(fetch(e.request)), main fetch is invoked (at least) twice. Once for the page making the fetch, and another for the service worker making the fetch.

Integrity checks are performed at step 19:

  1. If response is not a network error and request’s integrity metadata is not the empty string, run these substeps:
    1. Wait for response’s body.
    2. If response does not have a termination reason and response does not match request’s integrity metadata, set response and internalResponse to a network error.

Following Page --[1]--> SW --[2]--> Network:

[2] arrives at 19.2 first, as [1] hasn't received a response yet. If the integrity check fails, a network error is returned, so [1] won't recheck integrity.

If the integrity check succeeds, then [1] will recheck integrity when it lands on 19.2. I guess a conforming implementation could carefully optimise here and avoid duplicating this work.

In the case of e.respondWith(fetch(e.request.url)), the integrity information is lost from [2]'s call to main fetch, so yeah, the check will only be performed when [1] hits 19.2.

@xzhan96 xzhan96 closed this as completed Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants