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

Streaming upload and HTTP protocol info leak #1007

Closed
yutakahirano opened this issue Mar 16, 2020 · 33 comments
Closed

Streaming upload and HTTP protocol info leak #1007

yutakahirano opened this issue Mar 16, 2020 · 33 comments

Comments

@yutakahirano
Copy link
Member

At #966 we are talking about restricting the streaming upload feature only for certain HTTP versions (H1.1 and above, or H2 and above).

This may leak protocol versions, so we are wondering if Timing-Allow-Origin is needed.

On the other hand, the feature requires CORS preflight and some resource timing people talked about a possibility that CORS implies TAO.

@yoavweiss @npm1 What do you think?

cc: @sleevi @annevk @yoichio

@npm1
Copy link
Contributor

npm1 commented Mar 17, 2020

We have an issue regarding nextHopProtocol: w3c/resource-timing#221. I don't think that right now it's gated under Timing-Allow-Origin, so I see no reason to gate on this here. It's not the correct header to use here, as TAO should affect measurement, not functionality. It would be better to consider whether other security primitives (CORP, COEP) are needed here. TAO is not.

@annevk
Copy link
Member

annevk commented Mar 18, 2020

Yeah, when I read the nextHopProtocol description I also immediately thought that it would be an issue for same-origin as well. That's not to say that it's not a problem for cross-origin as well though, but that's a different issue from revealing proxies.

@yutakahirano yutakahirano changed the title Streaming upload and Timing-Allow-Origin Streaming upload and HTTP protocol info leak Mar 19, 2020
@yutakahirano
Copy link
Member Author

Thank you for the clarification! I changed the issue title.

@yoavweiss
Copy link
Collaborator

/cc @camillelamy @mikewest

@yoichio
Copy link
Contributor

yoichio commented Apr 10, 2020

I'm considering below API in fetch:

fetch('https://grpc.example.com/posts', {
  method: 'POST',
  body: createReadableStream(content),
  AllowHTTP1ForStreamingUpload: false})
.catch((error) => {
    // This error flag is also a new API.
    // This means middleboxes forced H1 (e.g. a local proxy), 
    // and the author can detect that and fall back to non-streaming upload.
  if (error.IsHTTP1Negotiated) {
   fallbackUploadWithoutStream(content);
  }
});

With that, web author knows the negotiated protocol is HTTP/1.
This can be abused as nextHopProtocol? If so, how might we refrain from it?

@sleevi
Copy link

sleevi commented Apr 10, 2020 via email

@yutakahirano
Copy link
Member Author

@sleevi Is a PerformanceResourceTiming record added for failure cases?

@sleevi
Copy link

sleevi commented Apr 10, 2020

@sleevi Is a PerformanceResourceTiming record added for failure cases?

https://w3c.github.io/resource-timing/#resources-included-in-the-performanceresourcetiming-interface

Resources for which the fetch was initiated, but was later aborted (e.g. due to a network error) MAY be included as PerformanceResourceTiming objects in the Performance Timeline and MUST contain initialized attribute values for processed substeps of the processing model.

@yutakahirano
Copy link
Member Author

Thank you.

The problematic part is, usually we know the HTTP version when the response arrives, in this case we need to fail the request before sending the request. Also we were talking about rejecting the request when the server likely to use HTTP/0.9 and HTTP/1.0: #966 (comment).

Does the following make sense to you?

  1. Same-origin case
    1. If the response to the main resource used HTTP/0.9 or HTTP/1.0:
      Fail the request, and record that HTTP version to the PerformanceResourceTiming entry of the failed request.
    2. If there is an existing HTTP/2 session, or the browser succeeded to create an HTTP/2 session: Continue processing the request.
    3. if AllowHTTP1ForStreamingUpload is true:
      Continue processing the request.
    4. if AllowHTTP1ForStreamingUpload is false:
      Fail the request, and record HTTP/1.1 to the PerformanceResourceTiming entry of the failed request.
  2. Cross-origin case
    1. If the response to the CORS preflight used HTTP/0.9 or HTTP/1.0:
      Fail the request, and record that HTTP version to the PerformanceResourceTiming entry of the failed request.
    2. If there is an existing HTTP/2 session, or the browser succeeded to create an HTTP/2 session: Continue processing the request.
    3. if AllowHTTP1ForStreamingUpload is true:
      Continue processing the request.
    4. if AllowHTTP1ForStreamingUpload is false:
      Fail the request, and record HTTP/1.1 to the PerformanceResourceTiming entry of the failed request.

@yoichio
Copy link
Contributor

yoichio commented Apr 16, 2020

I think session existence and the flag are independent to each other.
Thus the condition list should look like:

  1. If there is an HTTP/2 session,
    1. if AllowHTTP1ForStreamingUpload is true ..
    2. if AllowHTTP1ForStreamingUpload is false ..
  2. If there is no HTTP/2 session,
    1. if AllowHTTP1ForStreamingUpload is true ..
    2. if AllowHTTP1ForStreamingUpload is false ..

Also there is no difference between Same-origin and Cross-origin case. Is that true?

@sleevi
Copy link

sleevi commented Apr 16, 2020

The problematic part is, usually we know the HTTP version when the response arrives, in this case we need to fail the request before sending the request.

I’m not sure I understand? You know the transport protocol before you’ve sent the request. Either:

  • You established a TLS connection and no ALPN was negotiated or H1 (1.1/1.0/0.9) was explicitly negotiated
    • Check the flag / Fail the request
  • You established a TLS connection and ALPN negotiated H/2
    • You’re talking H/2 and are fine
  • You established a QUIC connection and thus negotiated H/3
    • You’re talking H/3 and are fine

The only time you need the response to know the version is when you’re talking <= H/1, but that’s OK, because you already know that by needing that, you’re talking H/1

@yoichio
Copy link
Contributor

yoichio commented Apr 20, 2020

Friendly ping @yutakahirano ?

@yutakahirano
Copy link
Member Author

Sorry for the delay. I think we would like to fail a request if it likely ends up using HTTP/1.0 or HTTP/0.9 even when AllowHTTP1ForStreamingUpload is true. Please see #966 (comment).

@yoichio
Copy link
Contributor

yoichio commented Apr 22, 2020

Let me re-summarize the chart:

Same-origin case:

  1. If the response to the main resource used HTTP/0.9 or HTTP/1.0:
    Fail the request.
  2. if the response used HTTP/1.1:
    1. If allowHTTP1ForStreamingUpload is true (default):
      Continue processing
    2. If alowHTTP1ForStreamingUpload is false:
      Fail the request
  3. If there is an existing HTTP/2 or /3 session, or the browser succeeded to create an HTTP/2 or /3 session:
    Continue processing

Cross-origin case:

  1. If the response to the CORS preflight resource used HTTP/0.9 or HTTP/1.0:
    Fail the request.
  2. if the response used HTTP/1.1:
    1. If allowHTTP1ForStreamingUpload is true (default):
      Continue processing
    2. If allowHTTP1ForStreamingUpload is false:
      Fail the request
  3. If there is an existing HTTP/2 or /3 session, or the browser succeeded to create an HTTP/2 or /3 session:
    Continue processing

Following this chart, fetch() upload streaming behaves different only when a web author knows the protocol with a previous response then this doesn't leak the protocol.

@annevk
Copy link
Member

annevk commented Apr 22, 2020

What is the main resource?

@yutakahirano
Copy link
Member Author

The document / worker top script from which the fetch is initiated.

@annevk
Copy link
Member

annevk commented Apr 22, 2020

What if that's a blob URL or data URL? Is this another kind of policy we need to inherit all over in (currently) buggy ways?

@yutakahirano
Copy link
Member Author

That information is used as a hint when we don't get information via ALPN. I don't think we need to provide that information for all cases - for example we can assume that the HTTP version is HTTP 1.0 when

  • No ALPN information is available,
  • The request is same-origin, and
  • We don't have the hint information (e.g., the fetch client is a blob document).

@annevk
Copy link
Member

annevk commented Apr 22, 2020

This still feels like a lot of complexity (and edge cases to test) over failing when >= H/2 could not be negotiated.

@domenic
Copy link
Member

domenic commented Apr 22, 2020

If allowHTTP1ForStreamingUpload is true (default):

Booleans should default to false, so consider flipping this to e.g. preventHTTP1ForStreamingUpload (default false)

@sleevi
Copy link

sleevi commented Apr 22, 2020

Let me re-summarize the chart:

Same-origin case:

  1. If the response to the main resource used HTTP/0.9 or HTTP/1.0:
    Fail the request.

  2. if the response used HTTP/1.1:

    1. If allowHTTP1ForStreamingUpload is true (default):
      Continue processing
    2. If alowHTTP1ForStreamingUpload is false:
      Fail the request

I’m not sure why a dependency on the previous request is being made? The new request should more or less be independent.

If it’s trying to avoid protocol leaks, they still happen in this model, because the client only sees the protocol to the intermediary and the server only sees the protocol from the intermediary, and they’re not necessarily the same. It seems like this can just be:

  • If you have 0 connections in the connection pool
    • make a new connection
  • If you have an H/2 or H/3 connection established
    • Use it
  • If the flag is not set / is false
    • Fail the request
  • If the flag is set
    • Attempt the request (which may result in a 1.1, 1.0, or 0.9 response)

I’m not sure why the distinction here on 1.0 / 0.9 responses, because it’s the request that’s sending the chunked encoding. Am I missing something basic?

@yoichio
Copy link
Contributor

yoichio commented Apr 24, 2020

If allowHTTP1ForStreamingUpload is true (default):

Booleans should default to false, so consider flipping this to e.g. preventHTTP1ForStreamingUpload (default false)

Since I noticed upload streaming over HTTP/1.1 is flaky and unreliable, allowHTTP1ForStreamingUpload 's default value should be false.

I’m not sure why the distinction here on 1.0 / 0.9 responses, because it’s the request that’s sending the chunked encoding. Am I missing something basic?

We require HTTPS. This prevents any unauthorized intermediates (i.e. not explicitly authorized by the server or the user) from breaking. From #966 (comment)

If it’s trying to avoid protocol leaks, they still happen in this model, because the client only sees the protocol to the intermediary and the server only sees the protocol from the intermediary, and they’re not necessarily the same.

I don't understand why protocol leaks with my chart given the asymmetric characteristic and yours block that. Could you explain more? @sleevi

@yutakahirano
Copy link
Member Author

I checked the ALPN numbers (for Chrome).

On Windows:

  • ALPN not used: 30%
  • HTTP/1.1 negotiated via ALPN: 22%
  • HTTP/2 negotiated via ALPN: 47%

On Android:

  • ALPN not used: 24%
  • HTTP/1.1 negotiated via ALPN: 25%
  • HTTP/2 negotiated via ALPN: 51%

@annevk
Copy link
Member

annevk commented Apr 27, 2020

What do those numbers mean? That ~50% of page loads use HTTP/2? For this feature what we need is a site attempting to use HTTP/2 and it failing for a percentage of users, right?

@yutakahirano
Copy link
Member Author

They're per TLS connection status, and hence unencrypted HTTP is excluded.

For this feature what we need is a site attempting to use HTTP/2 and it failing for a percentage of users, right?

This depends on allowHTTP1ForStreamingUpload.

I think we have four cases:

  1. HTTP/2 (and above)
  2. HTTP/1.1 that can be detected via ALPN
  3. HTTP/1.1 with no ALPN information
  4. HTTP/1.0 and below

With allowHTTP1ForStreamingUpload: false, we accept 1 and reject 2, 3 and 4.

With allowHTTP1ForStreamingUpload: true, we accept 1 and 2, but I'm not sure how to deal with 3 and 4. We can

  • Make a try ("accept")
  • Get some hint information from related response (main resource or CORS preflight)
  • Give up ("reject")

If "ALPN not used" were much smaller than "HTTP/1.1 negotiated via ALPN" we didn't need to take much care of 3 and 4 but apparently that's not the case.

@annevk
Copy link
Member

annevk commented Apr 27, 2020

My understanding of your data and above comment is that a larger number of sites would have to update their server to use this feature. Why is that considered problematic?

@yutakahirano
Copy link
Member Author

Sorry I'm not sure if I understand your point...

What do you think is the best for 3 and 4 in #1007 (comment) when allowHTTP1ForStreamingUpload is true? Does you assessment in the last comment depend on the choice?

@annevk
Copy link
Member

annevk commented Apr 27, 2020

I don't understand why we can't require H/2 as a baseline.

@yutakahirano
Copy link
Member Author

That has been discussed at #966 and we haven't had conclusion yet. We (Chrome) are planning to have a joint experiment with @wenbozhu.

I filed this issue to discuss the protocol leak, but unfortunately the discussion is converging to #966.

@annevk
Copy link
Member

annevk commented Apr 27, 2020

I see. If you don't want to reveal the protocol anyone is using, then for 3/4 you need to try (as sleevi also indicated I think).

The "main resource" or even the "CORS preflight" might well come from a different server. And proxies might also do different things depending on the type of request.

@yoichio
Copy link
Contributor

yoichio commented Apr 30, 2020

Given that this protocol leak is concerned if fetch upload streaming has an API to check which protocol is accepted and the API decision will be done on #966,
temporally we postpone this issue. Right?

@yoavweiss
Copy link
Collaborator

Belatedly catching up...

Seems to me that it would be near impossible for us to prevent the observable differences between HTTP/1.0, HTTP/1.1 and HTTP/2. They request sending and TCP/TLS connection patterns are different enough to make them highly observable without requiring a direct API.
The presence of caching proxies is also detectable using onload (e.g. by fetching resources that should be cached and observing RTTs, vs. fetching resources from the origin).

@yutakahirano
Copy link
Member Author

Thank you very much. Let me close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

9 participants
@domenic @yoavweiss @sleevi @annevk @yutakahirano @yoichio @npm1 and others