Bug 210347

Summary: Add capability to opt in specific domains into SameSite=strict bounce tracking protection
Product: WebKit Reporter: John Wilander <wilander>
Component: WebKit Misc.Assignee: John Wilander <wilander>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, hi, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 210362    
Attachments:
Description Flags
Patch none

Description John Wilander 2020-04-10 11:03:23 PDT 
We should add the capability to opt in specific domains into SameSite=strict bounce tracking protection. This way we can apply the new restriction to domains known to engage in bounce tracking while also getting frequent user interaction.
Comment 1 Radar WebKit Bug Importer 2020-04-10 11:04:34 PDT 
<rdar://problem/61593323>
Comment 2 John Wilander 2020-04-10 11:39:54 PDT 
Created attachment 396110 [details]
Patch
Comment 3 Brent Fulgham 2020-04-10 13:11:48 PDT 
Comment on attachment 396110 [details]
Patch

Looks good! r=me
Comment 4 John Wilander 2020-04-10 14:54:05 PDT 
Comment on attachment 396110 [details]
Patch

Thanks, Brent!
Comment 5 EWS 2020-04-10 14:57:19 PDT 
Committed r259906: <https://trac.webkit.org/changeset/259906>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396110 [details].
Comment 6 Devin Rousso 2020-04-10 15:04:29 PDT 
Comment on attachment 396110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396110&action=review

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1632
> +                    RELEASE_LOG_INFO_IF(debugLoggingEnabled(), ITPDebug, "Did set %" PUBLIC_LOG_STRING " as making a top frame redirect to %" PUBLIC_LOG_STRING ".", sourceDomain.string().utf8().data(), targetDomain.string().utf8().data());

Do you want these messages to also be sent to Web Inspector?  If so, I'd rewrite this as:
```
    if (UNLIKELY(debugLoggingEnabled()) {
        RELEASE_LOG_INFO(ITPDebug, "Did set %" PUBLIC_LOG_STRING " as making a top frame redirect to %" PUBLIC_LOG_STRING ".", sourceDomain.string().utf8().data(), targetDomain.string().utf8().data());
        debugBroadcastConsoleMessage(MessageSource::ITPDebug, MessageLevel::Info, makeString("[ITP] Did set '"_s, sourceDomain.string(), "' as making a top frame redirect to '"_s, targetDomain.string(), "'."_s));
    }
```

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp:479
> +                    RELEASE_LOG_INFO_IF(debugLoggingEnabled(), ITPDebug, "Did set %" PUBLIC_LOG_STRING " as making a top frame redirect to %" PUBLIC_LOG_STRING ".", sourceDomain.string().utf8().data(), targetDomain.string().utf8().data());

Ditto
Comment 7 Devin Rousso 2020-04-10 15:29:03 PDT 
Comment on attachment 396110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396110&action=review

>> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1632
>> +                    RELEASE_LOG_INFO_IF(debugLoggingEnabled(), ITPDebug, "Did set %" PUBLIC_LOG_STRING " as making a top frame redirect to %" PUBLIC_LOG_STRING ".", sourceDomain.string().utf8().data(), targetDomain.string().utf8().data());
> 
> Do you want these messages to also be sent to Web Inspector?  If so, I'd rewrite this as:
> ```
>     if (UNLIKELY(debugLoggingEnabled()) {
>         RELEASE_LOG_INFO(ITPDebug, "Did set %" PUBLIC_LOG_STRING " as making a top frame redirect to %" PUBLIC_LOG_STRING ".", sourceDomain.string().utf8().data(), targetDomain.string().utf8().data());
>         debugBroadcastConsoleMessage(MessageSource::ITPDebug, MessageLevel::Info, makeString("[ITP] Did set '"_s, sourceDomain.string(), "' as making a top frame redirect to '"_s, targetDomain.string(), "'."_s));
>     }
> ```

<https://webkit.org/b/210362> Web Inspector: add `broadcastConsoleMessage` calls for new ITPDebug logs after 259275