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

COEP:credentialless and the HTTP cache. #1253

Open
ArthurSonzogni opened this issue Jun 15, 2021 · 29 comments
Open

COEP:credentialless and the HTTP cache. #1253

ArthurSonzogni opened this issue Jun 15, 2021 · 29 comments

Comments

@ArthurSonzogni
Copy link
Member

ArthurSonzogni commented Jun 15, 2021

topic:coep-credentialless

The request's includeCredentials isn't part of the HTTP cache key.

It means if:

  • a.com requests c.com with credentials,
  • b.com requests c.com without credentials

Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not respected, and a.com influences directly b.com. The partitioned HTTP cache will fixe one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource without credentials and get a response with credentials. That would be a security issue. (tentative WPT)

One obvious solution would be to make Chrome to adhere to the current fetch specification. To take the "includeCredentials" flags into the HTTP cache key. Not sure if there are alternatives and what would be the potentials drawback to this. Happy to get your opinions. +CC @whatwg/cross-origin-isolation

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 15, 2021
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will fixe one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug:whatwg/fetch#1253
Bug:1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 15, 2021
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug:whatwg/fetch#1253
Bug:1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 15, 2021
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug:whatwg/fetch#1253
Bug:1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
@ArthurSonzogni
Copy link
Member Author

ArthurSonzogni commented Jun 15, 2021

I took a look at the specification, chromium and Firefox.

Spec:

To determine the network partition key,
==============================
given an environment settings object settings, run these steps:

1. Let topLevelOrigin be settings’s top-level origin.
2. If topLevelOrigin is null, then set topLevelOrigin to settings’s top-level creation URL’s origin.
3. Assert: topLevelOrigin is an origin.
4. Let topLevelSite be the result of obtaining a site, given topLevelOrigin.
5 .Let secondKey be null or an implementation-defined value.
6. Return (topLevelSite, secondKey).

To determine the network partition key
=============================,
given request, run these steps:
1. If request’s reserved client is non-null, then return the result of determining the network partition key given request’s reserved client.
2. If request’s client is non-null, then return the result of determining the network partition key given request’s client.

Return null.

Firefox:

// Assembles a cache-key from the given pieces of information and |mLoadFlags|
void nsHttpChannel::AssembleCacheKey(const char* spec, uint32_t postID,
                                     nsACString& cacheKey) {
  cacheKey.Truncate();

  if (mLoadFlags & LOAD_ANONYMOUS) {
    cacheKey.AssignLiteral("anon&");
  }

  if (postID) {
    char buf[32];
    SprintfLiteral(buf, "id=%x&", postID);
    cacheKey.Append(buf);
  }

  if (!cacheKey.IsEmpty()) {
    cacheKey.AppendLiteral("uri=");
  }

  // Strip any trailing #ref from the URL before using it as the key
  const char* p = strchr(spec, '#');
  if (p) {
    cacheKey.Append(spec, p - spec);
  } else {
    cacheKey.Append(spec);
  }
}

Chrome:

// static
// Generate a key that can be used inside the cache.
std::string HttpCache::GenerateCacheKey(const HttpRequestInfo* request) {
  std::string isolation_key;

  if (IsSplitCacheEnabled()) {
    // Prepend the key with |kDoubleKeyPrefix| = "_dk_" to mark it as
    // double-keyed (and makes it an invalid url so that it doesn't get
    // confused with a single-keyed entry). Separate the origin and url
    // with invalid whitespace character |kDoubleKeySeparator|.
    DCHECK(request->network_isolation_key.IsFullyPopulated());
    std::string subframe_document_resource_prefix =
        request->is_subframe_document_resource ? kSubframeDocumentResourcePrefix
                                               : "";
    isolation_key = base::StrCat(
        {kDoubleKeyPrefix, subframe_document_resource_prefix,
         request->network_isolation_key.ToString(), kDoubleKeySeparator});
  }

  // Strip out the reference, username, and password sections of the URL and
  // concatenate with the network isolation key if we are splitting the cache.
  std::string url = isolation_key + HttpUtil::SpecForRequest(request->url);

  // No valid URL can begin with numerals, so we should not have to worry
  // about collisions with normal URLs.
  if (request->upload_data_stream &&
      request->upload_data_stream->identifier()) {
    url.insert(0,
               base::StringPrintf("%" PRId64 "/",
                                  request->upload_data_stream->identifier()));
  }

  return url;
}

Two observations:

  1. Both Chrome and Firefox are keying by form ID. The specification doesn't. If you are happy, I can make a PR to 'officialize' the behavior of both implementers.
  2. Firefox is keying the cache by whether the request was made with credentials:
  if (mLoadFlags & LOAD_ANONYMOUS) {
    cacheKey.AssignLiteral("anon&");
  }

It means Firefox is already implementing the behavior I need. It means I can both fix the issue AND align Chrome and Firefox at the same time. Wonderful! If you are happy, I can make a PR to update the specification and Chrome.
My current Chrome prototype and Firefox are alike two peas in a pod:

  if (request->load_flags & LOAD_DO_NOT_SAVE_COOKIES) {
     [...]
  }

+CC @kinu FYI (who warned me |includeCredentials| might not be part of the HTTP cache key on Chrome, thanks!)

@yutakahirano, @annevk, @mikewest: I need this, so I would be happy to work on it. If you have some comments, please let me know.

@annevk
Copy link
Member

annevk commented Jun 17, 2021

I'm not familiar with "form ID". It seems like that is special cache logic for requests that have a body?

As for credentials, see #307. I'm not opposed, but @sleevi might be.

Feedback from @youennf and @whatwg/http would also be welcomehelpful here.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 17, 2021
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug: whatwg/fetch#1253
Bug: 1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961290
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893398}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 17, 2021
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug: whatwg/fetch#1253
Bug: 1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961290
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893398}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Jun 17, 2021
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug: whatwg/fetch#1253
Bug: 1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961290
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893398}
@MattMenke2
Copy link
Contributor

[@morlovich]

Keying cache entries on credentials mode certainly makes sense to me, at least. I believe we've historically depended on/assumed/hoped folks would use vary: cookies if they cared about credentials, but there isn't any way to cover auth, short of using no-store (no-cache is funky, and doesn't do anything about isolation here, since forward/backward navigations can use those resources, regardless of credentials).

@morlovich
Copy link

I think the risk of any partitioning is some risk of confusion from old copy being around and being picked up, and unlike with party split this is likely to be the same app? OTOH I think content authors find Vary: confusing, too.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 27, 2021
…TP cache., a=testonly

Automatic update from web-platform-tests
[Credentialless]: Add tests about the HTTP cache.

The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug: whatwg/fetch#1253
Bug: 1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961290
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893398}

--

wpt-commits: 9a43105fe8e0f4e56f0ff0f2637f72282521b9e6
wpt-pr: 29379
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 16, 2021
…TP cache., a=testonly

Automatic update from web-platform-tests
[Credentialless]: Add tests about the HTTP cache.

The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug: whatwg/fetch#1253
Bug: 1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961290
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893398}

--

wpt-commits: 9a43105fe8e0f4e56f0ff0f2637f72282521b9e6
wpt-pr: 29379
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 2, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are Firefox's behavior.
Chrome fail the test. Later, I would like to make Chrome to converge
toward Firefox and run a performance experiment.

whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Bug:1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 3, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are Firefox's behavior.
Chrome fail the test. Later, I would like to make Chrome to converge
toward Firefox and run a performance experiment.

whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Bug:1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 3, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are Firefox's behavior.
Chrome fail the test. Later, I would like to make Chrome to converge
toward Firefox and run a performance experiment.

whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

See:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3222766321
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ FAIL  │ Fail  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ FAIL  │ FAIL  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ FAIL  │ FAIL  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ FAIL  │ FAIL  │ PASS   │
└────────────────────────────────────────────┴───────┴───────┴────────┘

Bug:1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 3, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are Firefox's behavior.
Chrome fail the test. Later, I would like to make Chrome to converge
toward Firefox and run a performance experiment.

whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

See:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3222766321

Once:
#29612 (comment)
resolved, I am expecting:
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ FAIL  │ Fail  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ FAIL  │ FAIL  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ FAIL  │ FAIL  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ FAIL  │ FAIL  │ PASS   │
└────────────────────────────────────────────┴───────┴───────┴────────┘

Bug:1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
@ArthurSonzogni
Copy link
Member Author

ArthurSonzogni commented Aug 3, 2021

Let connection be the result of obtaining a connection, given networkPartitionKey, request’s current URL’s origin,~~ includeCredentials, and forceNewConnection.~~

(e.g. the HTTP cache key already depends on the boolean includeCredentials)
It means this is not a change to the specification, this is just making Chrome to follow the current spec.
edit: This still require a minor spec update. What I talked about was about the connection, not the cache key.

@MattMenke2
Copy link
Contributor

The fact that a site may get an older copy of the resource depending on the credential bit does seem confusing - maybe it would make more sense to just not use the cached resource if the includeCredentials bit doesn't match? An implementation that does this would be conformant, but if some do that and some key on the bit, developers could be confused as well.

@sleevi
Copy link

sleevi commented Aug 3, 2021

As @annevk surmised, I do think this is dangerous, because this is not compatible with intermediary caches, because “credentialless” is not a network-observable explicit property.

This creates the risk of browsers introducing special (“magic”) behaviour that varies from how other HTTP caches perform. That was the discussion with @mnot in #307

I’m not opposed to finding a way we can specify the behaviour desired in a way that intermediary caches can respect, but I do believe there is real danger here to interoperability if we start keying browser caches in ways that intermediaries cannot respect, and expecting there to be a security difference. Am I wrong for thinking that?

@annevk
Copy link
Member

annevk commented Aug 3, 2021

Isn't that already the case given the network partition key? (We could convey that to intermediary caches perhaps, but that would itself be a privacy leak of sorts in certain situations.)

@MattMenke2
Copy link
Contributor

MattMenke2 commented Aug 3, 2021

There seem to be a little different from NPK to me, at least.

NPK is currently best-effort privacy, and the model assumes all sites are the attackers (including intermediate caches, or at least those outside control/configuration of the user). For strong privacy guarantees, more than just not caching actually needed (e.g., willful IP blindness, or a heavily shared proxy).

That seems different from a design aimed at cross-site security, where sites are attacking other sites, which presumably don't want to be attacked, and can take (much better than best-effort) steps against it. The threat model is very different, and having a site be secure in some contexts but not in others (in a way the site might not know about) seems both a more serious issue here, and can lead to sites unknowingly being broken in the case of intermediary proxies.

@sleevi
Copy link

sleevi commented Aug 3, 2021

Correct. NPKs are, seemingly, a best-effort privacy goal. If the intermediary cache is near the server (e.g. a CDN), then we’re saying we’re OK with the origin learning that someone is interested in a given URL. We’re also assuming that the timing difference of (edge cached) vs (edge cache empty, has to backhaul to origin) is an acceptable privacy leak/something the origin controls.

If the intermediary cache is near the client (e.g. a proxy), then NIKs are less likely to provide a hard privacy boundary, because competing origins can collude to prime the proxy cache state, because the proxy has no knowledge of NPKs. NPKs add some timing jitter, but we ultimately are saying the privacy leak is due to explicit local configuration.

This is, for brevity sake, ignoring the complexity of intermediary caches for HTTPS protected resources, which, due to TLS, are more likely to be closer to the origin than the client, at least in today’s networks.

However, @ArthurSonzogni mentioning there are security concerns is why I wanted to draw attention to the complexity. Specifically, whether or not we can meet the guarantee mentioned:

With COEP:credentialless, we obviously don't want to request a resource without credentials and get a response with credentials. That would be a security issue.

The only way we can guarantee that, with today’s specifications at least, is to make that flag network observable so that any intermediaries can know and take appropriate precautions.

@ArthurSonzogni
Copy link
Member Author

ArthurSonzogni commented Aug 3, 2021

The fact that a site may get an older copy of the resource depending on the credential bit does seem confusing - maybe it would make more sense to just not use the cached resource if the includeCredentials bit doesn't match?

Firefox and the spec behaved this way for many years, this might mean this isn't a big deal? It has stood the test of time.
Firefox implemented this 13 years ago. I believe all of this started from this patch approximately:
mozilla/gecko-dev@bd5e926

However, your suggestion looks like a good idea to try if the current specification causes developers confusion.
My preference would still be to align Chrome with the spec (+Firefox) first for interoperability, and later think about updating the spec and dropping entries with mismatching includeCredentials parameters if we can find an agreement about this here.

I think the real question is whether COEP:credentialless security model still hold despite intermediary caches. We can fix Chrome's cache by adopting the spec, website can fix their own cache. However there are problems if there are caches that neither Chrome or the website control.
We want to avoids crossOriginIsolated website from loading cross-origin private data without an explicit opt-in from the server (CORS or CORP).

  • CrossOriginIsolation requires HTTPS, so the only intermediary caches that matter are the ones owning the website SSL private key. So a potential leak can only happens within website's control.
  • If the intermediary cache leaks private data toward anonymous requests, then I would argue this already represent a security problem for the website, independently of crossOriginIsolation. If the intermediary cache keys responses using IP address, then the leak happens in between two users from the same local network, otherwise globally.

I think I convinced myself intermediary cache wasn't an issue, but it probably worth discussing it more deeply with the security team in case I am missing something.

@sleevi
Copy link

sleevi commented Aug 3, 2021

Firefox and the spec behaved this way for many years, this might mean this isn't a big deal? It has stood the test of time.

We (Chrome) have disagreed here about whether the spec behaviour is the right behaviour, as discussed on the other bug.

  • CrossOriginIsolation requires HTTPS, so the only intermediary caches that matter are the ones owning the website SSL private key. So a potential leak can only happens within website's control.

This is not a correct assumption of the threat model. We see users regularly behind locally-configured proxies, whether for access purposes (e.g. sharing a metered connection), virus inspection, or for DLP purposes. Here, the intermediary cache works by TLS interception. We can't just assume it's origin-only, which I was trying to get at with my previous message

  • If the intermediary cache leaks private data toward anonymous requests, then I would argue this already represent a security problem for the website, independently of crossOriginIsolation. If the intermediary cache keys responses using IP address, then the leak happens in between two users from the same local network, otherwise globally.

Yes, it does. That's the problem with assuming we can have separation without expressing that. This is the whole situation for origin servers omitting, for example, Vary: Cookie. Many existing caches have already implicitly assumed this behavior, but the point is to not create new situations where there's a bunch of implicit/undocumented behavior.

That's not to say we can't solve this, but that it requires care to make sure we're interoperating with the specifications, and adjusting them if needed.

@ArthurSonzogni
Copy link
Member Author

Thanks! I understand now.
So the problem isn't proxy controlled by the website, but proxies controlled by the user (or their company).

User's at risk would be the ones who have installed a local proxy and override Chrome to trust their certificates. There isn't much we can do against security vulnerabilities caused by local proxy, but the least we can do it to send them headers to do the separation correctly, even if we can't ensure they will.

@annevk
Copy link
Member

annevk commented Aug 5, 2021

I don't agree that the NPK is purely for privacy. It mitigates a number of XS-Leaks attacks. I suppose you are saying those are not mitigated (as much) if the user has a local caching proxy, which seems believable. It would be great to see more research in that area.

If anything this reads more like an argument to me that NPK should be exposed (somehow) for users with local caching proxies. (And NPK should be extended to cater for this use case.)

@camillelamy
Copy link
Member

Coming back to the issue of the security impact of this, we are unfortunately in a position where our security against cross-site data leaks is best-effort . Spectre attacks are possible in non-crossOriginIsolated contexts, there are just less efficient. This situation is not going to change unless we get rid of all timers in all non-crossOriginIsolated contexts or all CPUs vulnerabilities are addressed. None of which is going to happen.

So then we need to evaluate whether the risk of a proxy caching credentialed resources fetched through HTTPS and then serving those back to the COEP credentialless document is higher than an attacker page just requesting the resource in a non-crossOriginIsolated context and performing a not very efficient Spectre attack on it.

To do a back-of-the-envelope computation, timer resolution is 20x greater in Chrome in crossoriginIsolated contexts vs non-crossOriginIsolated contexts (5 microseconds vs 100 microseconds). My understanding is that Spectre attacks efficiency are roughly proportional to timer resolution. So you can exfiltrate data 20x faster in crossOriginIsolated contexts vs non-crossOriginIsolated contexts. If we look then at COEP credentialless, unless more than 1 in 20 credentialed resources are in fact cached in local proxy that interposes on HTTPS connections, it is going to be faster to simply perform the attack in a non-crossOriginIsolated context.

@sleevi
Copy link

sleevi commented Aug 5, 2021

I don't agree that the NPK is purely for privacy. It mitigates a number of XS-Leaks attacks.

@annevk To the extent it relates to XS-Leaks, I'm just wanting to confirm: does the full NPK matter to that mitigation, or would it be fully sufficient that Vary: Cookie, Authorization would mitigate these (namely, ensuring credentialless vs credentialed requests are separately cached)?

My understanding here is that we don't need the full NPK from a security perspective. From a privacy perspective, yes, I do believe NPK fails to achieve privacy goals in the presence of any intermediary, whether it's close to the client or close to the server. We simply don't care as much about those close to the server, simply because we expect we're disclosing to the server that we are or have previously accessed it, so the fact that it learns that we're accessing it isn't that interesting.

@annevk
Copy link
Member

annevk commented Aug 5, 2021

The full NPK matters as it's about whether a resource is in the cache, see https://xsleaks.dev/docs/attacks/cache-probing/ for some high-level details and pointers.

@sleevi
Copy link

sleevi commented Aug 5, 2021

@annevk Right, I've read that document, and that's why I specifically asked about Cookie/Authorization. Do you have a pointer for something you see is relevant for the NPK from a security/XS-Leaks side? Maybe I'm missing something in particular here (as I'm expected credentialed requests are already handled by CORS/CORP/COOP/etc, so the only distinction here is uncredentialed requests)

@annevk
Copy link
Member

annevk commented Aug 6, 2021

End user visits A, which loads unique subresources from B (e.g., images representing pages the user likes). Attacker C checks which subresources from B are in the end user's cache. NPK is relevant here as without it A and C would see the same. These subresources from B don't have to be guarded by credentials.

@sleevi
Copy link

sleevi commented Aug 6, 2021

@annevk What I’m struggling with in your example is understanding why B isn’t guarded by the credentials flag. If the thinking is that this is some static CDN, where only A is guarded by credentials, then isn’t this “working as intended” from an HTTP semantics point of view?

From an HTTP PoV, B is explicitly saying this is OK to coalesce between users of B, right?

Where I’m going with this is trying to figure out how much NPK is trying to invent new semantics for the HTTP layer, versus being an opportunistic protection at the client. If you’d like it to be stronger, yes, we would have to define things that have semantic meaning.

@annevk
Copy link
Member

annevk commented Aug 6, 2021

@sleevi I don't understand what you mean by "HTTP semantics point of view". All I'm saying is that if C can tell it's cached it's a security problem that NPK addresses, except in the face of local caching proxies (no concrete research on this, but seems likely). And that if we care about local caching proxies for credentials we should also care about them for NPK.

@sleevi
Copy link

sleevi commented Aug 6, 2021

If the fact that two different users of B are supposed to be idempotent of eachother, B should express that (e.g. the resources should not be credentialless). I think we’re in agreement that there’s a probing attack here, but that doesn’t seem related to NPK, at its core. For example, if B was credentialed with Vary: Cookie, then the NPK wouldn’t matter at all: even under the same-meat-user case, A’s cookie jar for B would be different than C’s cookie jar for B, leading to different cookies, leading to different cache entries for B-in-A and B-in-C.

@annevk
Copy link
Member

annevk commented Aug 6, 2021

Why are we talking about two different users? I was considering a single user. (Also, in Chrome and Firefox (by default) A's and C's cookie jar is still the same, no?)

@sleevi
Copy link

sleevi commented Aug 6, 2021

@annevk Because it’s the same thing to the intermediary: whether it’s User 1 with B-by-A and B-by-C, or User 1 vs User 2. My point is the intermediary doesn’t need NPK for that: the origin is implicitly saying “You can share this cache entry between User 1 and User 2”.

I can totally understand that, just like services that only support HTTP, it may be that we’re saying the defaults are insecure, and the browser as the user’s agent needs to change, but my point is that the scenario you described (at least, as I tried to reflect, where A is credentialed, B is not, and C is the attacker) is already broken without NPK.

@sleevi
Copy link

sleevi commented Aug 6, 2021

@annevk Let me try framing it differently then:

  • I understand you to be asking “Is this broken because NPK”, and I’m trying to explain, “No, because it’s already broken today for multiple users without NPK”.
  • If you’re asking “Is this broken with NPK”, then the answer is “Yes, but it’s broken without NPK”.
  • If you’re asking “Can we do something for NPK to fix this”, then I’m saying “Yes, we need the origin server to change, or we need to define new behavior for intermediaries.”
  • If the question is “Is it better to change origin servers or intermediaries”, as a practical matter, I believe changing the origin server is more likely to succeed, because that’s something the origin server can control. Intermediary update rates are abysmal.

That’s not to say we can’t say “Why not both”, but to be clear, it seems the threat model you’re describing is an origin explicitly declaring its resources public and cacheable, and then being uncomfortable when they’re shared between different contexts (whether different meat people or different NPKs).

@annevk
Copy link
Member

annevk commented Aug 6, 2021

Fair, with multiple users there is more of a chance for an attacker to hit a false positive if there is an intermediary cache involved.

I don't think the origin server is aware of the potential issues and also, they shouldn't have to be. See all the threads about SRI caching where people assume this kind of setup to be fine. There is no way we will convince the long tail of origin servers to do the right thing. Also, they might become the attacker and start using this once other state is gone.

@sleevi
Copy link

sleevi commented Aug 6, 2021

Right, but there's also no way we'll convince the long tail of intermediaries to opt in to a new system, so either way, there's no perfect solution. We're just trying to figure out the way to have the most impact for the most users. As I mention, I think on a practical matter, that means providing clear guidance for high-value websites to take action (independent of intermediaries), while also working long-term to provide solutions/guidance for intermediaries to update to. I believe that's what @mnot was mentioning in 2018 with this message

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 9, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are Firefox's behavior.
Chrome fail the test. Later, I would like to make Chrome to converge
toward Firefox and run a performance experiment.

whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

See:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3222766321

Once:
#29612 (comment)
resolved, I am expecting:
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ FAIL  │ Fail  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ FAIL  │ FAIL  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ FAIL  │ FAIL  │ PASS   │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ FAIL  │ FAIL  │ PASS   │
└────────────────────────────────────────────┴───────┴───────┴────────┘

Bug:1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 9, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘

Bug:1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 9, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
```
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘
```

Bug: 1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066251
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909780}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 9, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
```
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘
```

Bug: 1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066251
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909780}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Aug 9, 2021
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
```
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘
```

Bug: 1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066251
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909780}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 14, 2021
…tials., a=testonly

Automatic update from web-platform-tests
WPT: Tentative test HTTP cache vs credentials.

Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
```
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘
```

Bug: 1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066251
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909780}

--

wpt-commits: 7d357243857d2ae0383120c22d43cd1b1b6fc371
wpt-pr: 29867
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Aug 20, 2021
…tials., a=testonly

Automatic update from web-platform-tests
WPT: Tentative test HTTP cache vs credentials.

Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
```
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘
```

Bug: 1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066251
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909780}

--

wpt-commits: 7d357243857d2ae0383120c22d43cd1b1b6fc371
wpt-pr: 29867
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Sep 3, 2021
whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Add the feature "SplitCacheByIncludeCredentials".
When enabled, it makes the HTTP cache to differentiate the requests sent
anonymously from the ones sent with credentials.

This avoids anonymous requests from getting responses requested with
credentials.

This aligns Chrome toward Firefox. See Firefox implementations:
https://github.com/mozilla/gecko-dev/blob/b31b78eea683b0eb341c676adb422cd129909fe9/netwerk/protocol/http/nsHttpChannel.cpp#L4117

Goal after this patch is to start a finch experiment to check for
potential performance regressions.

VIRTUAL_OWNERS: This adds 13 tests from http-cache WPT directory to be
run with the feature enabled. Tests are often "any.js" tests, causing
46 run.

Bug: 1221529
Change-Id: I0f1c969eb2c3401e9548c5d1e6ec63570166d7e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010379
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918101}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The request's includeCredentials isn't part of the HTTP cache key.

It means if:
- a.com requests c.com with credentials,
- b.com requests c.com without credentials
Then both a.com and b.com will get a response requested with credentials.

This seems problematic in general. The request's credential mode is not
respected, and a.com influences directly b.com. The partitioned HTTP
cache will solve one of the two problem.

With COEP:credentialless, we obviously don't want to request a resource
without credentials and get a response with credentials. That would be a
security issue.

Here is a WPT test about it.

Bug: whatwg/fetch#1253
Bug: 1218023
Change-Id: I888dc020a8ae770816d0fbc42e8803df3ba66392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961290
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893398}
NOKEYCHECK=True
GitOrigin-RevId: 7893be59819d8c03560740e49377071f6d0aac25
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Add a test. Check whether the HTTP cache discriminate the credentialled
requests from the anonymous ones.

The expectations used are the ones from the specification. That's also
Firefox's behavior.
Chrome fails the test. Safari fail similarly + do not support
SharedWorker.
At some point, we would like to make Chrome to converge with the spec,
or update with the specification.

whatwg/fetch issue:
whatwg/fetch#307
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Test results:
https://github.com/web-platform-tests/wpt/pull/29867/checks?check_run_id=3279839968
```
┌────────────────────────────────────────────┬───────┬───────┬────────┐
│Test                                        │ Chrome│ Safari│ Firefox│
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.html              │ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.serviceworker.html│ 1/3   │ 1/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.sharedworker.html │ 1/3   │ 0/3   │ 3/3    │
├────────────────────────────────────────────┼───────┼───────┼────────┤
│credentials.tentative.any.worker.html       │ 1/3   │ 1/3   │ 3/3    │
└────────────────────────────────────────────┴───────┴───────┴────────┘
```

Bug: 1221529
Change-Id: I0537108f473f37f42eef7b4fa1079cd88d987b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066251
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909780}
NOKEYCHECK=True
GitOrigin-RevId: de5fcb86dc21f385e370d07dc9720c8e7a868388
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Add the feature "SplitCacheByIncludeCredentials".
When enabled, it makes the HTTP cache to differentiate the requests sent
anonymously from the ones sent with credentials.

This avoids anonymous requests from getting responses requested with
credentials.

This aligns Chrome toward Firefox. See Firefox implementations:
https://github.com/mozilla/gecko-dev/blob/b31b78eea683b0eb341c676adb422cd129909fe9/netwerk/protocol/http/nsHttpChannel.cpp#L4117

Goal after this patch is to start a finch experiment to check for
potential performance regressions.

VIRTUAL_OWNERS: This adds 13 tests from http-cache WPT directory to be
run with the feature enabled. Tests are often "any.js" tests, causing
46 run.

Bug: 1221529
Change-Id: I0f1c969eb2c3401e9548c5d1e6ec63570166d7e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010379
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918101}
NOKEYCHECK=True
GitOrigin-RevId: 54013f2a5d6c06a909cbc7318895c2ccf4039921
@letitz
Copy link

letitz commented Jan 12, 2023

If I understand the discussion correctly, this is a bit of a rehash of #307, but specific to COEP: credentialless. Maybe there is a solution to this particular issue that does not solve #307, however.

In this issue, the problematic scenario is the following:

  1. https://evil.com/not-coi fetches https://bank.com/sensitive.json in a non-cross-origin-isolated context, with credentials
  2. https://bank.com responds with some sensitive data, forgets to set a Vary: Cookie header
  3. the browser caches the response
  4. https://evil.com/coi fetches the same resource in a cross-origin-isolated credentialless context
  5. the cache entry is used, which means COEP: credentialless is bypassed
  6. evil.com can perform a high-banddwidth spectre attack and leak the sensitive data

Now, if I understand @sleevi's point correctly, the issue with the proposed fix (aligning spec and chromium with gecko) is that it fails to address other caches' handling the same request.

Suppose the user loads b.com through a proxy (be it a CDN or a local MITM). Then instead of step 3 we have:

3a. the proxy caches the response
3b. the browser caches the response with includeCredentials: true

And instead of step 5. we have:

5a. the browser cache entry is not used, as the includeCredentials bit on the request is false
5b. the proxy cannot tell that credentials are not included and should matter, responds with the cached entry

Resulting in the same security issue.

Finally, the concern is that we would fix this issue with a Fetch spec change, then announce "mission accomplished" and forget to find a solution to the proxy problem.

Is my understanding correct?

@annevk
Copy link
Member

annevk commented Jan 12, 2023

Yeah that is correct. The proxy doesn't get sufficient context. We could expose more context, especially now we have the Sec-Fetch-* series of headers, but proxies adopting them would be another matter.

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

No branches or pull requests

7 participants