Re: [whatwg/fetch] Prevent cross-origin sensitive header probing (PR #1434)

@annevk commented on this pull request.

I don't see where this algorithm is actually invoked. Some of these headers are added quite late in the game too (e.g., `Cookie` and `Authorization`) so I'm not sure how easy this would be to do.

> @@ -1123,6 +1123,56 @@ format of range header value can be set using <a>add a range header</a>.
 <a>implementation-defined</a> <a for=/>header value</a> for the `<code>User-Agent</code>`
 <a for=/>header</a>.
 
+<p>To
+<dfn export for="header list" id=concept-header-list-filter-include>filter include</dfn> a
+<a for=/>header list</a> <var>list</var> and <a for=/>header name</a>s <var>includeNames</var>, run these steps:

The second argument here is a list of header names? As such this is not a correct type. (Also applicable below.)

> @@ -1123,6 +1123,56 @@ format of range header value can be set using <a>add a range header</a>.
 <a>implementation-defined</a> <a for=/>header value</a> for the `<code>User-Agent</code>`
 <a for=/>header</a>.
 
+<p>To
+<dfn export for="header list" id=concept-header-list-filter-include>filter include</dfn> a
+<a for=/>header list</a> <var>list</var> and <a for=/>header name</a>s <var>includeNames</var>, run these steps:
+
+<ol>
+ <li>Let <var>filteredList</var> be an empty <a for=/>header list</a>.

Editorial: use `<li><p>` throughout like the existing algorithms.

> @@ -1123,6 +1123,56 @@ format of range header value can be set using <a>add a range header</a>.
 <a>implementation-defined</a> <a for=/>header value</a> for the `<code>User-Agent</code>`
 <a for=/>header</a>.
 
+<p>To
+<dfn export for="header list" id=concept-header-list-filter-include>filter include</dfn> a
+<a for=/>header list</a> <var>list</var> and <a for=/>header name</a>s <var>includeNames</var>, run these steps:
+
+<ol>
+ <li>Let <var>filteredList</var> be an empty <a for=/>header list</a>.
+
+ <li><a for=list>For each</a> (<var>name</var>, <var>value</var>) in <var>list</var>:
+
+  <ol>
+   <li>If <var>name</var> in <var>includeNames</var> then <a for=list>append</a> (<var>name</var>, <var>value</var>) to <var>filteredList</var>.

This could use some more precise language. Also needs a comma.

> +<a for=/>header list</a> <var>list</var>, run these steps:
+
+<ol>
+ <li>Let <var>sizeInBytes</var> be `0`.
+
+ <li><a for=list>For each</a> (<var>name</var>, <var>value</var>) in <var>list</var>:
+
+  <ol>
+   <li>Add the size of <var>name</var> to <var>sizeInBytes</var>.
+   <li>Add `2` to <var>sizeInBytes</var> (represents the `: ` after the name).
+   <li>Add the size of <var>value</var> to <var>sizeInBytes</var>.
+   <li>Add `2` to <var>sizeInBytes</var> (represents the `\r\n` after the value).
+  </ol>
+
+ <li><p>Return <var>sizeInBytes</var>.
+</ol>
 

Editorial: needs an additional newline.

> +  </ol>
+
+ <li><p>Return <var>filteredList</var>.
+</ol>
+
+<p>To
+<dfn export for="header list" id=concept-header-list-get-the-size>get the size</dfn> of a
+<a for=/>header list</a> <var>list</var>, run these steps:
+
+<ol>
+ <li>Let <var>sizeInBytes</var> be `0`.
+
+ <li><a for=list>For each</a> (<var>name</var>, <var>value</var>) in <var>list</var>:
+
+  <ol>
+   <li>Add the size of <var>name</var> to <var>sizeInBytes</var>.

You want https://infra.spec.whatwg.org/#byte-sequence-length here.

> +
+ <li><p>Return <var>filteredList</var>.
+</ol>
+
+<p>To
+<dfn export for="header list" id=concept-header-list-get-the-size>get the size</dfn> of a
+<a for=/>header list</a> <var>list</var>, run these steps:
+
+<ol>
+ <li>Let <var>sizeInBytes</var> be `0`.
+
+ <li><a for=list>For each</a> (<var>name</var>, <var>value</var>) in <var>list</var>:
+
+  <ol>
+   <li>Add the size of <var>name</var> to <var>sizeInBytes</var>.
+   <li>Add `2` to <var>sizeInBytes</var> (represents the `: ` after the name).

2 is a number, not code.

I'd probably group this with the 2 below and explain it's for HTTP/1 syntax and might not correctly represent the total size in newer versions.

> @@ -2864,6 +2914,33 @@ run these steps:
  <li><p>Return <b>allowed</b>.
 </ol>
 
+<h3 dfn export lt="is a cors requests header size over the limit" id=is-a-cors-requests-header-size-over-the-limit>
+Is a CORS request's header size over the limit?</h3>

We're not necessarily talking about a CORS request here, are we? I'd prefer not having the algorithm name in the heading if we can avoid it.

> @@ -2864,6 +2914,33 @@ run these steps:
  <li><p>Return <b>allowed</b>.
 </ol>
 
+<h3 dfn export lt="is a cors requests header size over the limit" id=is-a-cors-requests-header-size-over-the-limit>
+Is a CORS request's header size over the limit?</h3>
+
+Note: The goal of this algorithm is to prevent cross-origin requests from probing the size of sensitive <a for=/>header</a>s
+(`<a http-header><code>Authorization</code></a>` or `<a http-header><code>Cookie</code></a>`) by adding <a for=/>header</a>s
+to cross-origin requests until the total size of all HTTP request <a for=/>header</a>s exceeds the server side limit. If this
+algorithm returns true, the <a>CORS-preflight request</a> must be run. In order for this approach to succeed, servers should
+not set an HTTP request <a for=/>header</a>s size limit below 8KB.

8 kibibytes*

> +Note: The goal of this algorithm is to prevent cross-origin requests from probing the size of sensitive <a for=/>header</a>s
+(`<a http-header><code>Authorization</code></a>` or `<a http-header><code>Cookie</code></a>`) by adding <a for=/>header</a>s
+to cross-origin requests until the total size of all HTTP request <a for=/>header</a>s exceeds the server side limit. If this
+algorithm returns true, the <a>CORS-preflight request</a> must be run. In order for this approach to succeed, servers should
+not set an HTTP request <a for=/>header</a>s size limit below 8KB.
+
+<p>Run these steps to check if <dfn>the header size is over the CORS limits</dfn> on the provided <var>request</var>:
+
+<ol>
+ <li><p>If <var>request</var>'s <a for=request>origin</a> is <a>same origin</a> with <var>request</var>'s
+ <a for=request>current URL</a>'s <a for=url>origin</a>, return `false`.
+
+ <li><p>Let <var>sensitiveHeaderList</var> be the result of running <a for="header list">filter include</a> on <var>request</var>'s
+ <a for=response>header list</a> with `(<a http-header><code>Authorization</code></a>, <a http-header><code>Cookie</code></a>) as <var>includeNames</var>`.
+
+ <li><p>If the result of running <a for="header list">get the size</a> on <var>sensitiveHeaderList</var> is greater than 4KB, return `true`.

4 kibibytes*

> @@ -4628,17 +4705,19 @@ these steps:
 
   <ol>
    <li>
-    <p>If <var>makeCORSPreflight</var> is true and one of these conditions is true:
+    <p>If one of these conditions is true:

What changed here?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1434#pullrequestreview-1120030114
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1434/review/1120030114@github.com>