Update spec to current explainer #131

johnivdel · 2021-04-14T19:35:24Z

This updates the draft spec to include all changes which were landed to the explainer since it was last updated.

This includes:

all naming changes on Remove "impression"/"conversion" from API surfaces #103 and Change "attributeon" naming to "attributiondestination" #121
removal of hexadecimal encoding for data fields
clarification on the value represented by a/attributiondestination

Algorithm definitions and variables are updated to reflect the new naming.

This updates the draft spec to include all changes which were landed to the explainer since it was last updated. This includes: - all naming changes on #103 and #121 - removal of hexadecimal encoding for data fields Algorithm definitions and variables are updated to reflect the new naming.

johnivdel · 2021-04-15T17:11:26Z

@jyasskin could you take a look at this? Thanks!

cc @csharrison

jyasskin

I haven't gotten all the way through the spec, but here's a bunch of comments to start on.

jyasskin · 2021-04-15T23:02:34Z

index.bs

-Title: Conversion Measurement
-Shortname: conversion-measurement
+Title: Attribution Reporting
+Shortname: attribution-reporting


I'm curious if this causes any problems for the various spec-scraping tools. Probably not, since I think this spec isn't referenced from anywhere, and I doubt there's anything you could do in this spec to mitigate any problems.

Hopefully one change is enough :)

jyasskin · 2021-04-15T23:03:51Z

index.bs

 Level: 1
 Status: CG-DRAFT
 Group: wicg
 Repository: WICG/conversion-measurement-api
 URL: https://wicg.github.io/conversion-measurement-api
 Editor: Charlie Harrison, Google Inc. https://google.com, [email protected]
-Abstract: A new API to measure and attribute cross-site conversions.
+Abstract: A new API to measure and attribute cross-site events.


I expect "attribute" to be used with "to something" at the end. Is there something you could say about where these events are being attributed to?

Clarified, this is about associating them to each other. Does this still seem vague to you?

index.bs

jyasskin · 2021-04-20T23:58:39Z

index.bs


-    : [=conversion/conversion source=]
+    : [=attribution trigger/trigger origin=]
    :: |environment|'s [=environment/top-level origin=].


Why is this the top-level origin instead of just the origin? Can attribution triggers happen in cross-origin iframes? Have you discussed somewhere the security implications of writing a different origin into the attribution trigger, and what it means for specs that need to trust or distrust that value?

Attribution triggers can happen in cross-origin iframes. The API scopes all events to the top-level site, and then performs attribution in the top-level site namespace. One example would be, an ad was shown for shoes.example, and then shoes.example loads an iframe to ad-tech.example which handles triggering attribution on that site.

We have not discussed the security implications to my knowledge.

Speaking from Chrome's implementation, the browser uses a trusted origin for this struct member. Essentially, the browser knows the top-level browsing context and it's origin, and can look that up from the browsing context which created the attribution trigger in a trusted way. So this value isn't something inherently controlled from within the browsing context in practice.

I was worried about a hostile iframe being able to create somehow-hostile attributions to the surrounding page, but I'm not sure what attacks that would actually be useful for. It helps that the user has to click in order to make the attribution source happen. Maybe hostile triggers are more worrying, since they might cause payment from the trigger to the source? In any case, this sort of attribution (no pun intended) of one origin's actions to a different origin is worth talking about in the security considerations.

Yes that would cause issues. Sources are also problematic: a hostile frame logging additional sources could cause credits to be assigned incorrectly, or cause the browser to delete already existing sources in some cases.

The main mitigation we have is requiring a permissions policy to invoke the API inside of iframes, which is disabled by default for cross-origin iframes.

I will make a note to address this in Security Considerations when the Permissions Policy is added to this draft spec.

index.bs

Co-authored-by: Jeffrey Yasskin <[email protected]>

johnivdel

Thanks for the great comments Jeffrey!

johnivdel · 2021-04-21T16:54:44Z

index.bs


-    : [=conversion/conversion source=]
+    : [=attribution trigger/trigger origin=]
    :: |environment|'s [=environment/top-level origin=].


Attribution triggers can happen in cross-origin iframes. The API scopes all events to the top-level site, and then performs attribution in the top-level site namespace. One example would be, an ad was shown for shoes.example, and then shoes.example loads an iframe to ad-tech.example which handles triggering attribution on that site.

We have not discussed the security implications to my knowledge.

Speaking from Chrome's implementation, the browser uses a trusted origin for this struct member. Essentially, the browser knows the top-level browsing context and it's origin, and can look that up from the browsing context which created the attribution trigger in a trusted way. So this value isn't something inherently controlled from within the browsing context in practice.

johnivdel · 2021-04-21T19:57:15Z

index.bs

 Level: 1
 Status: CG-DRAFT
 Group: wicg
 Repository: WICG/conversion-measurement-api
 URL: https://wicg.github.io/conversion-measurement-api
 Editor: Charlie Harrison, Google Inc. https://google.com, [email protected]
-Abstract: A new API to measure and attribute cross-site conversions.
+Abstract: A new API to measure and attribute cross-site events.


Clarified, this is about associating them to each other. Does this still seem vague to you?

index.bs

johnivdel · 2021-04-23T02:18:51Z

index.bs

 :: An [=url/origin=].
-: <dfn>impression data</dfn>
+: <dfn>event id</dfn>
 :: A [=string=].


Fixed. I noted this is a 64-bit integer, but I couldn't find any precedence in the html spec for this.

johnivdel · 2021-04-23T02:21:03Z

index.bs

+    [CEReactions, Reflect] attribute DOMString attributiondestination;
+    [CEReactions, Reflect] attribute DOMString attributionsourceeventid;
+    [CEReactions, Reflect] attribute DOMString attributionreportto;
+    [CEReactions, Reflect] attribute unsigned long long attributionexpiry;


unsigned long long is not a type defined in https://html.spec.whatwg.org/#reflecting-content-attributes-in-idl-attributes

This either needs to be an unsigned long or a string. I can address this in a separate PR.

index.bs

johnivdel · 2021-04-23T02:38:50Z

index.bs

-Title: Conversion Measurement
-Shortname: conversion-measurement
+Title: Attribution Reporting
+Shortname: attribution-reporting


Hopefully one change is enough :)

index.bs

jyasskin · 2021-04-28T00:45:15Z

index.bs

+To <dfn>parse attribution data</dfn> given a [=string=] |input| modulo an unsigned long long
+|maxData| perform the  following steps. They return an unsigned long long:


It's uncommon to use the WebIDL types inside of specs, but we don't have a uniform policy in Infra for what to do instead: whatwg/infra#87. Here I would probably use "integer" (which matches https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#non-negative-integers) for input and maybe "non-negative integer less than 2⁶⁴" for return values instead of "unsigned long long", and then explicitly say what to do when input is out of range, or use Assert: input is between X and Y inclusive. when some other part of the spec was responsible for ensuring it.

index.bs

jyasskin · 2021-04-28T23:47:18Z

index.bs

+    1. If |reportingUrl| is failure or null, return null.
+    1. Set |reportingOrigin| to |reportingUrl|'s [=url/origin=].
+1. If |anchor| has an <{a/attributionexpiry}> attribute, let |expiry| be that 
+    attribute's value in milliseconds.


Use HTML's integer parsing rules here too.

index.bs

jyasskin · 2021-04-28T23:55:53Z

index.bs


-    : [=conversion/conversion source=]
+    : [=attribution trigger/trigger origin=]
    :: |environment|'s [=environment/top-level origin=].


I was worried about a hostile iframe being able to create somehow-hostile attributions to the surrounding page, but I'm not sure what attacks that would actually be useful for. It helps that the user has to click in order to make the attribution source happen. Maybe hostile triggers are more worrying, since they might cause payment from the trigger to the source? In any case, this sort of attribution (no pun intended) of one origin's actions to a different origin is worth talking about in the security considerations.

Co-authored-by: Jeffrey Yasskin <[email protected]>

johnivdel self-assigned this Apr 14, 2021

johnivdel added 5 commits April 15, 2021 12:55

Update index.bs

ddaa6da

Update index.bs

c65bddd

Update index.bs

9c67f15

Update index.bs

3f78562

Update index.bs

8c8a1a0

jyasskin suggested changes Apr 20, 2021

View reviewed changes

johnivdel mentioned this pull request Apr 21, 2021

Unspecified [Reflect] extended attribute used for <a> attributes #133

Closed

johnivdel and others added 2 commits April 22, 2021 22:35

Respond to review feedback

7b5c196

Apply suggestions from code review

9102197

Co-authored-by: Jeffrey Yasskin <[email protected]>

johnivdel commented Apr 23, 2021

View reviewed changes

jyasskin approved these changes Apr 28, 2021

View reviewed changes

johnivdel and others added 3 commits April 28, 2021 22:34

Apply suggestions from code review

b13e153

Co-authored-by: Jeffrey Yasskin <[email protected]>

Update index.bs

02b650c

Update index.bs

32fbcc1

johnivdel merged commit 19f75d5 into main Apr 30, 2021

apasel422 deleted the update-spec-to-explainer branch May 24, 2022 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update spec to current explainer #131

Update spec to current explainer #131

johnivdel commented Apr 14, 2021 •

edited by pr-preview bot

Loading

johnivdel commented Apr 15, 2021

jyasskin left a comment

jyasskin Apr 15, 2021

johnivdel Apr 23, 2021

jyasskin Apr 15, 2021

johnivdel Apr 21, 2021

jyasskin Apr 20, 2021

johnivdel Apr 21, 2021

jyasskin Apr 28, 2021

johnivdel Apr 29, 2021

johnivdel left a comment

johnivdel Apr 21, 2021

johnivdel Apr 21, 2021

johnivdel Apr 23, 2021

johnivdel Apr 23, 2021

johnivdel Apr 23, 2021

jyasskin Apr 28, 2021

jyasskin Apr 28, 2021

johnivdel Apr 30, 2021

jyasskin Apr 28, 2021

		To <dfn>parse attribution data</dfn> given a [=string=] \|input\| modulo an unsigned long long
		\|maxData\| perform the following steps. They return an unsigned long long:

Update spec to current explainer #131

Update spec to current explainer #131

Conversation

johnivdel commented Apr 14, 2021 • edited by pr-preview bot Loading

johnivdel commented Apr 15, 2021

jyasskin left a 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

johnivdel left a 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

johnivdel commented Apr 14, 2021 •

edited by pr-preview bot

Loading