Closed Bug 1454242 Opened 3 years ago Closed 3 years ago

Login on fails after same-site cookies set by cross site requests have been forbidden


(Core :: DOM: Security, defect, P1)




Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed
firefox61 --- fixed


(Reporter: aryx, Assigned: ckerschb)



(Keywords: regression, Whiteboard: [kanban:][domsecurity-active])


(2 files)

Bisection with Nightly on Windows confirms that the patches for bug 1452496 broke the login on

Steps to reproduce:
1. Open e.g.
2. Login with your LDAP and second factor.

Actual result:
File not found

An error occurred during a connection to

    Check the file name for capitalization or other typing errors.
    Check to see if the file was moved, renamed or deleted.

Expected result:
Testrail's own login form to login with LDAP shown.

needinfo for Miles as the op for testrail and Christoph as developer of bug 1452496 which has a pending beta uplift.
CC Julien and Ryan because of the pending uplift.
CC Andrei, Cristian and Rares in case SV might also work with testrail on Nightly.
Flags: needinfo?(miles)
Flags: needinfo?(ckerschb)
Whiteboard: [kanban:]
Assignee: server-ops-webops → nobody
Component: WebOps: IT-Managed Tools → DOM: Security
Flags: needinfo?(ckerschb)
Priority: -- → P1
Product: Infrastructure & Operations → Core
QA Contact: smani
Whiteboard: [kanban:] → [kanban:][domsecurity-active]

within Bug 1452496 we started to not allow same-site cookies being set from within a cross origin context. Unfortunately I used NS_IsSameSite() instead of simply consulting the third party util to determine whether a same-site cookie is allowed to be set. NS_IsSameSite uses the triggeringPrincipal for top-level loads [1] to determine whether samesite cookies in lax mode should be send. In this case we not only checked for sending same-site cookies, but also for setting, which is different and wrong.

In more detail, the URI wants to set samesite cookie in 'lax' mode; when setting the cookie on the top-level page we used the triggeringPrincipal which did not allow the same site cookie to be set. Simply using the thirdparty util within CanSetCookie() does the right check and causes the problem from comment 0 work correctly again.

Attachment #8968078 - Flags: review?(valentin.gosu)
Comment on attachment 8968078 [details] [diff] [review]

Review of attachment 8968078 [details] [diff] [review]:

Glad we caught it early.
Attachment #8968078 - Flags: review?(valentin.gosu) → review+
Mark, since we need to uplift that change I suppose it's wise having an automated test for that as well. Please see a description of the problem in comment 1 - thanks!
Attachment #8968114 - Flags: review?(mgoodwin)
Comment on attachment 8968114 [details] [diff] [review]

Review of attachment 8968114 [details] [diff] [review]:

Good catch!
Attachment #8968114 - Flags: review?(mgoodwin) → review+
Pushed by
Setting samesite cookie should not rely on NS_IsSameSiteForeign. r=valentin
Test samesite cookie on top-level page from cross-origin context. r=mgoodwin
Comment on attachment 8968078 [details] [diff] [review]

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1452496
[User impact if declined]: does not allow same-site cookies to be set if top-level page was triggered by cross origin page
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: Bug 1452496
[Is the change risky?]: no
[Why is the change risky/not risky?]: only affects same-site cookies
[String changes made/needed]: no
Attachment #8968078 - Flags: approval-mozilla-beta?
Comment on attachment 8968114 [details] [diff] [review]

Approval Request Comment
see comment 7
Attachment #8968114 - Flags: approval-mozilla-beta?
Comment on attachment 8968078 [details] [diff] [review]

same-site cookies followup, approved for 60.0b13
Attachment #8968078 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8968114 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(miles)
We have several admin panels using lua-resty-openidc (Balrog, Normandy, probably others) that are affected by this.
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.