1652244 - Like and Re-blog buttons do not work/missing on Wordpress

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect, P1)

Description

[bull500]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

Login to Wordpress account
Visit https://mozillagfx.wordpress.com/2020/07/10/moz-gfx-newsletter-54/
Click 'Like'

Actual results:

Click on 'Like' opens a popup window but the change is not reflected.
The Re-blog button is missing

Expected results:

Click on 'like' button should reflect change on the webpage.
Reblog button should not be missing

Setting network.cookie.cookieBehavior to 4 fixes the issue

Comment 1

[bull500]

This might be due to the dFPI again

Comment 2

[Gary Chen [:xeonchen]]

(In reply to bull500 from comment #1)

This might be due to the dFPI again

Thanks for reporting this!

Comment 3

[Gary Chen [:xeonchen]]

In this case, https://widgets.wp.com is automatically granted storage access on https://mozillagfx.wordpress.com, but https://public-api.wordpress.com also needs storage access on https://mozillagfx.wordpress.com.

Comment 4

[Steven Englehardt [:englehardt]]

Since we're visiting mozillagfx.wordpress.com I wouldn't expect public-api.wordpress.com to have partitioned storage. We partition by site (scheme+eTLD+1) and wordpress.com isn't on the public suffix this. I suspect we're partitioning some resources from public-api.wordpress.com because they're embedded in cross-origin iframes.

Here are two frames loaded from public-api.wordpress.com:

1. An iframe embedded directly in the main frame: https://public-api.wordpress.com/connect/?googleplus-sign-in=https%3A%2F%2Fmozillagfx.wordpress.com&color_scheme=light

2. An iframe embedded with the following frame ancestors and loaded from https://public-api.wordpress.com/wp-admin/rest-proxy/#https://widgets.wp.com:

   • https://widgets.wp.com/likes/master.html?ver=20190321#ver=20190321
   • https://mozillagfx.wordpress.com/2020/07/10/moz-gfx-newsletter-54/

(1) has unpartitioned storage access, which is expected. (2) has partitioned storage, which is unexpected.

Here's a simpler test page to verify: https://senglehardt.com/test/dfpi/nested_embed.html. Note that this is not the case when FPI is enabled (presumably because we also double key the main page's state).

Gary, would you mind to take a look?

Also flagging Anne because I'm not sure if we should add anything related to this in the spec.

Comment 5

[Anne (:annevk)]

I'm not sure I fully follow as I only see a single frame with a "Like" button. (I agree with the partitioning story you sketch and that hopefully matches how it will eventually get standardized. Still quite a long way to go there though.)

Comment 6

[Gary Chen [:xeonchen]]

(In reply to Steven Englehardt [:englehardt] from comment #4)

Since we're visiting mozillagfx.wordpress.com I wouldn't expect public-api.wordpress.com to have partitioned storage. We partition by site (scheme+eTLD+1) and wordpress.com isn't on the public suffix this. I suspect we're partitioning some resources from public-api.wordpress.com because they're embedded in cross-origin iframes.

We checked if an iframe is third-party not only using eTLD+1 but also this, and according to this, public-api.wordpress.com is considered as 3rd party intentionally because it's included in a third-party iframe.

Comment 7

[Gary Chen [:xeonchen]]

Steven, in this case, A (wordpress.com) -> B (wp.com) -> C (wordpress.com), so A and C should be considered as same origin regardless the origin of B, right?

Comment 8

[Dimi Lee [ooo 16.04 - 21.04］]

Hi baku,
I talked offline with gary, and also from comment 4, I think the idea is that nested same-origin iframe should still be considered as first-party.

I'll write a summaty for all our third-party APIs...

1. ThirdPartyUtil::IsThirdPartyWindow

• not consider top-level

2. ThirdPartyUtil::IsThirdPartyChannel

• not consider top-level

3. nsContentUtils::IsThirdPartyWindowOrChannel(nsPIDOMWindowInner* aWindow, nsIChannel* aChannel, nsIURI* aURI) when passing Window

• not consider top-level

4. nsContentUtils::IsThirdPartyWindowOrChannel(nsPIDOMWindowInner* aWindow, nsIChannel* aChannel, nsIURI* aURI) when passing Channel

• consider top-level

5. AntiTrackingUtils::IsThirdPartyWindow

• not consider top-level, this API is actually the same as calling nsContentUtils::IsThirdPartyWindowOrChannel with window

6. AntiTrackingUtils::IsThirdPartyChannel

• consider top-level, this API was added to support Fission because nsContentUtils::IsThirdPartyWindowOrChannel is not fission-compatible

As we talked yesterday, right now, the behavior of IsThirdPartyWindow and IsThirdPartyChannel is not sync (channel considers top-level while window doesn't). However, I guess there will be some cases requring 3rd-party API considering top-level (like this one?).
So I'll suggest we do the following:

1. Do not use nsContentUtils::IsThirdPartyWindowOrChannel anymore because it is not fission-compatible (and also the function itself is confusing). We should replace places where we use this API with the other 3rd-party APIs
2. Update AntiTrackingUtils::IsThirdPartyWindow to consider top-level.
   so for cases we need to consider top-level, use AntiTrackingUtils::IsThirdPartyXXX, for cases we don't want to consider top-level, use ThirdPartyUtil::IsThirdPartyXXX.

Does that make sense?

Comment 9

[Dimi Lee [ooo 16.04 - 21.04］]

also needinfo tim to see if tim has any comment

Comment 10

[Tim Huang[:timhuang]]

I think it totally makes sense to be consistent in terms of the behavior. There are too many APIs that we are using to check the third-partyness right now: this is confusing and error-prone. I think we need a well-written comment to describe the behavior since it is hard to tell if it is considering the top-level from the API name.

However, it is still unclear to me about in which case we need to consider top-level or not. So, in the long run, I think we need a clear definition of what it means for considering top-level in terms of antitracking and a general idea about where and why we need to consider the top-level. I believe it would be helpful to clarify things here.

Comment 11

[Andrea Marchesini [:baku]]

I agree with Tim about the number of APIs. I'm working to get rid of nsContentUtils::IsThirdPartyWindowOrChannel, AntiTrackingUtils::IsThirdPartyWindow and AntiTrackingUtils::IsThirdPartyChannel. We should have only one 3rd-party implementation.

Note that cookieService has a completely independent implementation because of the "confusion" related to the window/channel chain. CookieService uses ThirdPartyUtils directly to compute how to treat the sameSite attribute. This also should go away, but it cannot, with the Dimi's proposal.

The 3rd-partyness and cross-origin have a lot in common, but they have differences, and we should spend a bit of time formalizing them. Anne can definitely help here.

Talking about windows, in gecko we consider 3rd-party an iframe with a different base-domain (eTLD+1) than the parent's ones (exploring the whole chain). This means that if the iframe chain is: A.com (first-party) -> B.com -> A.com, A2 is 3rd-party vs B.com, B.com is 3rd-party vs A1.com and, because of this, A2 is 3rd-party vs A1.

This is what is implemented in the ThirdPartyUtils and it's fission-compatible. We don't do the same for nsIChannels, but I think it's a bug and it should be fixed. (Dimi and I had a quick chat yesterday about it).

In the anti-tracking project, often we use a different 3rd-party check: we use a window-TO-top-level comparison. This means that A2 and A1 are first-party when considering just the iframe and the top-level window. Maybe we should use this check here too.

What Dimi proposes is to use the window-vs-top-level comparison everywhere (am I right?). This could work for the anti-tracking project, but not for the rest of gecko. CookieService is an example but there are more.

To me the next steps here are:

1. unify the window and channel 3rd-party implementations. Considering the whole chain.
2. define if we want 3rd-party or 3rd-party-vs-top-level check for the anti-tracking project. To me we should use 3rd-party-vs-top-level here (without making it the default 3rd-party check).

Anne, thoughts?

Comment 12

[Dimi Lee [ooo 16.04 - 21.04］]

(In reply to Andrea Marchesini [:baku] from comment #11)

What Dimi proposes is to use the window-vs-top-level comparison everywhere (am I right?). This could work for the anti-tracking project, but not for the rest of gecko. CookieService is an example but there are more.

Not exactly, what I proposed is:

1. Make sure AntiTrackingUtils::IsThirdPartyWindow and AntiTrackingUtils::IsThirdPartyChannel consider top-level, and use this for anti-tracking project.

2. Make sure ThirdPartyUtil::IsThirdPartyWindow and ThirdPartyUtil::IsThirdPartyChannel consider the whole chain, use this API in places we don't care about top-level, for example, as you mentioned, CookieService.

3. Get rid of nsContentUtils::IsThirdPartyWindowOrChannel, replace them with either AntiTrackingUtils::IsThirdPartyXXX or ThirdPartyUtil::IsThirdPartyXXX depends on the expected behavior

Basically I think the idea is the same as yours.

To me the next steps here are:

1. unify the window and channel 3rd-party implementations. Considering the whole chain.
2. define if we want 3rd-party or 3rd-party-vs-top-level check for the anti-tracking project. To me we should use 3rd-party-vs-top-level here (without making it the default 3rd-party check).

Agree!
One question, if there is any case in Anti-tracking project we don't care about top-level?

Comment 13

[Anne (:annevk)]

As discussed yesterday, ignoring the checks needed for SameSite cookies, we basically want two concepts of third party:

• Third party origin (current origin is not same origin with the top-level origin)
• Third party site (current site is not same site with the top-level site)

And for dFPI we pretty much only need the latter. Note that this also means that the current checks for the latter need to start taking scheme into account. If http://example.com/ embeds https://example.com/, then the latter is both a third-party origin and a third-party site.

(And to address comment 7, yes, A and B are same origin and have direct script access.)

Comment 14

[Dimi Lee [ooo 16.04 - 21.04］]

Attachment: Bug 1652244 - P1. AntiTrackingUtils::IsThirdPartyWindow and AntiTrackingUtils::IsThirdPartyChannel should consider top-level r=timhuang

Comment 15

[Dimi Lee [ooo 16.04 - 21.04］]

Attachment: Bug 1652244 - P2. Use TYPE_DOCUMENT to ensure IsThirdPartyContextToTopWindow is false when loading a top-level r=timhuang

Depends on D86714

Comment 16

[Dimi Lee [ooo 16.04 - 21.04］]

Attachment: Bug 1652244 - P3. Fix test_different_domain_in_hierarchy.html test failure r=timhuang

This test tests whether cookies can be set in the following frames:
top-level: http://example.org/tests/netwerk/test/mochitests/file_domain_hierarchy_inner.html
1st-level: http://example.com/tests/netwerk/test/mochitests/file_domain_hierarchy_inner_inner.html
2nd-level: http://example.org/tests/netwerk/test/mochitests/file_domain_hierarchy_inner_inner_inner.html

Before this patch, the 2nd-level iframes can't set a cookie.
After this patch, the 2nd-level is considered first-party and can set a cookie.

Depends on D86715

Comment 17

[Dimi Lee [ooo 16.04 - 21.04］]

Attachment: Bug 1652244 - P4. Fix test_third_party.html test failure r=timhuang

This testcase has:
top-level: mochi.test
1st-level: host (depend on the value in the test data)
2nd-level: mochi.test

This test run tests in the 2nd-level iframe, which is first-party with
respect to the top-level according to the new change.

The test is re-written to run tests in both 1st-level iframe and
2nd-level iframes.

Depends on D86716

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47b142630cf9
P1. AntiTrackingUtils::IsThirdPartyWindow and AntiTrackingUtils::IsThirdPartyChannel should consider top-level r=timhuang
https://hg.mozilla.org/integration/autoland/rev/fadf02e459b6
P2. Use TYPE_DOCUMENT to ensure IsThirdPartyContextToTopWindow is false when loading a top-level r=timhuang
https://hg.mozilla.org/integration/autoland/rev/0fa31c531db2
P3. Fix test_different_domain_in_hierarchy.html test failure r=timhuang
https://hg.mozilla.org/integration/autoland/rev/1e2a698cb187
P4. Fix test_third_party.html test failure r=timhuang

Comment 19

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/47b142630cf9
https://hg.mozilla.org/mozilla-central/rev/fadf02e459b6
https://hg.mozilla.org/mozilla-central/rev/0fa31c531db2
https://hg.mozilla.org/mozilla-central/rev/1e2a698cb187