Integrate b1 signed exchanges with Fetch #281

jyasskin · 2018-08-06T21:26:25Z

jyasskin · 2018-08-06T23:11:49Z

loading.bs

+the stashed response.
+
+A Service Worker for `https://distributor.example.org/` gets to handle the
+original request. A Service Worker for `https://publisher.example.org/` does not


@kinu Have y'all looked into the Service Worker side? I believe Fetch's https://fetch.spec.whatwg.org/#concept-http-fetch step 4.2

If request’s redirect mode is "follow", then set request’s service-workers mode to "none".

prevents all redirects from hitting a service worker, so even if distributor.example.org just served a 302 to publisher.example.org, publisher.example.org's SW wouldn't get to handle the navigation. Am I misreading anything?

For navigation the default redirect mode is "manual", therefore every time a request's redirected corresponding SW has a chance to see it.

Aha! Thanks!

I think both @cramforce's idea to use the preloadResponse and asking people to call fetch() with cache=="only-if-cached" will work, but to use "only-if-cached" we'll have to tell people to modify the original request or its clone, instead of creating a new request with the same URL. A new request with the same URL won't have the stashed exchange attached, and wiring those up will require an extra global map in the SW itself, which could also get confused by interleaved requests to different signed exchanges holding the same publication URL.

jyasskin · 2018-08-06T23:19:17Z

loading.bs

+    exchange trusted for longer, are constrained to modify the client's clock
+    and can't also attack its estimate of its skew.
+1. Let |message| be the [=b1 signed message=] for |signature| and |headerBytes|.
+1. Let |publicKey| be the [=certificate/public key=] of |parsedSignature|'s


@sleevi @davidben The fact that the certificate is stored un-parsed means that the spec winds up saying to parse out a public key twice, which feels like a security risk. How much can systems actually parse these structures ahead of time, vs when they're validating origin-trust?

Can you explain why it feels like a security risk? I'm not sure I fully understand the question. Extracting an SPKI from a certificate is something easy on virtually all systems - the question is whether or not it's "trusted" data yet.

I've been conditioned that any time we parse something twice, security bugs appear in the differences between the two parsers. However, if you're comfortable with this case (e.g. that nobody will actually write separate parsers), I'm also happy with it.

I presume they're going to be using the same parser - is there reason to believe they may be different?

No particular reason.

jyasskin · 2018-08-06T23:23:07Z

loading.bs

+
+In <a spec="fetch">HTTP-network-or-cache fetch</a>, after
+
+> 5.19. If |httpRequest|’s [=request/cache mode=] is neither "`no-store`" nor


Right now, I'm ignoring all of the cache modes in deciding to accept a signed exchange's content. They may be useful in controlling how to fetch the .sxg itself, and applying them to the content would probably just have the effect of forcing that to the network, which would defeat the point.

Does that sound right to y'all, or do you see any of the modes I should apply here?

This sg to me.

jyasskin · 2018-08-06T23:43:57Z

loading.bs

+    [=stateful request header=] or |exchange|'s [=exchange/response=]'s
+    [=response/header list=] includes a [=stateful response header=], return
+    "untrusted".
+1. If |signature|'s [=exchange signature/expiration time=] is more than 604800


I check the length of the validity window in cross-origin trust rather than while parsing the signature so that other uses of signatures can define their own maximum validities. e.g. maybe the Play store wants to sign that it's audited an app and have that be valid for more than a week.

jyasskin · 2018-08-06T23:45:10Z

loading.bs

+    Note: This does not check for revocation of intermediate certificates, and
+    clients SHOULD implement another mechanism for that.
+
+1. If |leaf| doesn't have enough SCTs from trusted logs return `untrusted`,


@sleevi Is this a reasonable way to phrase the requirement?

No. There's no such thing as "trusted logs", and this also isn't how clients evaluate whether a certificate complies with a given CT policy.

I think much of this algorithm can be replaced by waving into the blackbox and saying that the OCSP response, the SCTs, and the certificates are inputs, for the host, and determine the response. I suspect you may want an added requirement on the OCSP responses freshness, or that the validating client MUST support Certificate Transparency (or is it optional for UAs that don't support CT but still support SXG?)

So put this all in the "Attempt to build a trustworthy path" step? Sounds good; how's this new wording?

I don't think CT should be optional for signed exchanges even for UAs that don't use it for TLS connections. Knowing that a certificate was mis-issued has been an important part of our security analysis.

You realize this means that only Chrome will implement SXGs for the foreseeable future? Coupling to CT in this way is going to either explicitly preclude certain UAs, or encourage them to adopt patterns that they've been reticent to do for TLS, precisely because they know it will harm the CT ecosystem. We should sync up on that. I realize the intersection between CT relates to SXG, but I think we need to be careful about MUSTing certain policy bits, compared to highlighting the tradeoffs.

This is now a SHOULD with a link to a new Security Considerations section.

jyasskin · 2018-08-06T23:49:38Z

loading.bs

+1. Return "match".
+
+
+## Stream algorithms ## {#stream-algs}


@domenic This whole section could migrate to either Fetch or Streams if you think it'll be useful to other specs.

jyasskin · 2018-08-06T23:51:30Z

loading.bs

+    for [[draft-yasskin-http-origin-signed-responses]] will use distinct context
+    strings.
+1. A single 0x00 byte which serves as a separator.
+1. The concatenation of the following [=byte sequences=], which is the


@davidben How do you feel about this message algorithm? I think we can simplify it for b2, since I only made it this complicated to avoid confusion between multiple versions of the message, and that's now handled by the HTTP Exchange 1 b1 context string, but it'll inform whether we can go to Origin Trial with b1 or have to go to b2 first.

kinu · 2018-08-07T04:46:51Z

loading.bs

+request. The redirect applies all the usual processing, and then when it would
+normally check for an HTTP cache hit, it also checks whether the stashed request
+matches the redirected request and which of the stashed exchange or HTTP cache
+contents is newer. If the stashed exchange matches and is newer, the UA returns


This means that whenever UA loads things from a Signed Exchange UA also needs to check HTTP cache. Can you clarify what situation we are trying to deal with by this? If we don't cache the stashed exchange it feels it's just fine, or a site should probably set a short expiration date?

Imagine that Vashti visits https://app.com/ regularly, and app.com also publishes a signed exchange valid for a week to allow offline distribution. Now app.com discovers a vulnerability and patches it, but the vulnerable signed exchange is still out there.

If Vashti visits the site after the update and gets the new version into her cache, even stale, then even if an attacker manages to serve her the vulnerable signed exchange, she won't use it.

This isn't an absolute defense against anything, but it lowers the probability of a successful attack on frequent users of a site.

Chrome's current impl is not following this, which is the biggest concern, but that part probably needs a separate discussion anyways to proceed. (I'll file an impl bug and take a closer look if this needs to block our further milestones)

jyasskin · 2018-08-07T18:36:12Z

loading.bs

+1. Let |clockSkew| be the uncertainty in the UA's estimate of the current time
+    caused by clock skew on the client. The UA MAY set this to 0 or use a more
+    sophisticated estimate.
+1. If the UA's estimate of the current time is more than |clockSkew| before


@davidben @sleevi How many hives does this bit give you?

I'm not going to have bandwidth to review this this week, in part because I need to go get treated for hives. It quite concerns me, and I can tell it's going to take time and focus that I don't yet have to adequately review :/

SG. I've added a pointer to #141 so that anyone implementing this knows to check with security folks.

This still needs a definition of request matching in a subsequent PR, and it probably has several mistakes, which I'm hoping my reviewers will catch.

As previously agreed with Chrome's loading team.

jyasskin · 2018-08-13T22:26:23Z

Hooking into HTTP fetch gives me an easy way to inject a redirect, but it means that signed exchanges loaded from the filesystem, from blobs, etc. don't work. In the long run, I think we'll want them to work, so we'll want to either move the attachment point up to main fetch, or manually go through some of the redirect steps in the other branches of scheme fetch.

I still think this PR is the right place to start: it'll be easier to see what that extension changes with this as a baseline.

nyaxt · 2018-08-20T01:48:45Z

Given that other @jyasskin PRs build on top of this PR, I'd like to proceed with this and then revise the bits later. @kinu Are you cool with this?

kinu · 2018-08-20T06:36:30Z

Okay, let's merge it and consider separate PRs if we find any modifications are needed.

rwlbuis · 2018-08-20T12:25:52Z

loading.bs

+1. If |browserRequest|'s [=request/url=] is not [=url/equal=] to
+    |storedRequest|'s [=request/url=], return "mismatch".
+
+1. Issue: Flesh this out, along the lines of


Note that the way this is rendered in Safari looks a bit weird, I only see step 1. and 3. with a big ISSUE section in between. But I guess once Step 2 has been fleshed out, it will look ok.

jyasskin requested review from nyaxt and davidben August 6, 2018 21:26

jyasskin force-pushed the loading-spec branch from ca767ef to 0aee9e6 Compare August 6, 2018 21:30

jyasskin mentioned this pull request Aug 6, 2018

Resolve Signed Exchange monkeypatches whatwg/fetch#784

Open

jyasskin commented Aug 6, 2018

View reviewed changes

jyasskin force-pushed the loading-spec branch from 2cdb79d to c3fab22 Compare August 6, 2018 23:58

kinu reviewed Aug 7, 2018

View reviewed changes

jyasskin commented Aug 7, 2018

View reviewed changes

jyasskin force-pushed the loading-spec branch 2 times, most recently from 00207d0 to cf4eb32 Compare August 8, 2018 18:36

jyasskin added 9 commits August 8, 2018 12:29

Integrate Signed Exchanges with Fetch

e06d785

This still needs a definition of request matching in a subsequent PR, and it probably has several mistakes, which I'm hoping my reviewers will catch.

Move the Read Buffer definition to the Structures section.

a7cc9b5

Block redirects and sxg's inside sxg's.

e1cae8f

As previously agreed with Chrome's loading team.

Describe how a SW can kinda detect the signed exchange.

ddd65e2

Block redirected prefetches to preserve privacy.

496c7fa

Treat certificate validation as more of a black box.

5d7b193

Add an issue to re-review clock skew.

a0d4f05

Have TravisCI check and publish the loading spec.

135e087

Turn CT into a SHOULD, and explain its benefits.

4c9707c

jyasskin force-pushed the loading-spec branch from cf4eb32 to 4c9707c Compare August 8, 2018 19:29

This was referenced Aug 8, 2018

Update the README and explainer to match the new loading spec. #284

Merged

Define request matching in terms of draft-ietf-httpbis-variants. #285

Merged

jyasskin mentioned this pull request Aug 14, 2018

Put a fallback URL at the beginning of signed exchanges #288

Merged

nyaxt approved these changes Aug 20, 2018

View reviewed changes

kinu approved these changes Aug 20, 2018

View reviewed changes

nyaxt merged commit 51fa4e8 into WICG:master Aug 20, 2018

rwlbuis reviewed Aug 20, 2018

View reviewed changes

jyasskin deleted the loading-spec branch August 20, 2018 21:42

horo-t mentioned this pull request Mar 19, 2019

Extend link HTTP header to support subresource signed exchange loading #347

Open

jyasskin mentioned this pull request Mar 20, 2019

SXG loading and service worker integration #409

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate b1 signed exchanges with Fetch #281

Integrate b1 signed exchanges with Fetch #281

jyasskin commented Aug 6, 2018 •

edited

Loading

jyasskin Aug 6, 2018

kinu Aug 7, 2018

jyasskin Aug 7, 2018

jyasskin Aug 6, 2018

sleevi Aug 7, 2018

jyasskin Aug 7, 2018

sleevi Aug 7, 2018

jyasskin Aug 8, 2018

jyasskin Aug 6, 2018

kinu Aug 20, 2018

jyasskin Aug 6, 2018

jyasskin Aug 6, 2018

sleevi Aug 7, 2018

jyasskin Aug 7, 2018 •

edited

Loading

sleevi Aug 7, 2018

jyasskin Aug 8, 2018

jyasskin Aug 6, 2018

jyasskin Aug 6, 2018

kinu Aug 7, 2018

jyasskin Aug 7, 2018

kinu Aug 20, 2018

jyasskin Aug 7, 2018

sleevi Aug 7, 2018

jyasskin Aug 7, 2018

jyasskin commented Aug 13, 2018

nyaxt commented Aug 20, 2018

kinu commented Aug 20, 2018

rwlbuis Aug 20, 2018


		In <a spec="fetch">HTTP-network-or-cache fetch</a>, after

		> 5.19. If \|httpRequest\|’s [=request/cache mode=] is neither "`no-store`" nor

Integrate b1 signed exchanges with Fetch #281

Integrate b1 signed exchanges with Fetch #281

Conversation

jyasskin commented Aug 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyasskin Aug 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyasskin commented Aug 13, 2018

nyaxt commented Aug 20, 2018

kinu commented Aug 20, 2018

Choose a reason for hiding this comment

jyasskin commented Aug 6, 2018 •

edited

Loading

jyasskin Aug 7, 2018 •

edited

Loading