1503696 - Firefox 63 is dropping sessions with Spring application, 62 worked as expected.

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[kaiwpost]

Attachment: Screen Shot 2018-10-31 at 1.55.16 PM.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

A spring (JBoss) application for the State of Maryland that worked with Firefox 62 no longer works for 63. Everytime a link is clicked or a task is performed the session is dropped and the user is re-routed to the login screen. We tested changing some of the security settings but none of them resolved the problem. This could be an issue for many Spring applications out there, not just this one.


Actual results:

A spring (JBoss) application for the State of Maryland that worked with Firefox 62 no longer works for 63. Everytime a link is clicked or a task is performed the session is dropped and the user is re-routed to the login screen. We tested changing some of the security settings but none of them resolved the problem. This could be an issue for many Spring applications out there, not just this one.


Expected results:

Attaching screen shot for 63, including the view of the JSessionID cookies. The top portion of the image is before, the bottom after an action is performed. The session is lost as apparent from the JSessonID cookie. When performed using Firefox 62 this works as expected. Are there any settings we can change to get around this issue? The gorup has been testing and recommending the use of Firefox and were hoping there would be a solution for this.

Comment 1

[Matthias Versen [:Matti]]

This is difficult to debug without access to the site but there is one thing that you could do to narrow down the problem.
We have a regression finder tool that downloads Firefox builds from different dates to find the change in Firefox that caused the problem. Can you please try our tool to find the change in Firefox that caused this ?

-> https://mozilla.github.io/mozregression/

Comment 3

[Josh Matthews [:jdm]]

No, sorry, I confused this with other bugs.

Comment 4

[Peter Saint-Andre [:stpeter]]

A few questions for the original poster to determine if this is related to tracking protection...

1. What tracking protection settings were enabled in the browser? (See about:preferences#privacy)

2. What domains were loaded on the page (see Network in developer tools) that are related to the service which is broken?

3. What cookies are used or expected by the service, and what domains try to set those cookies?

Thanks!

Comment 5

[(no longer active)]

I can easily reproduce on https://mdedrs.health.maryland.gov.  The session cookie changes when you go from the login page to the forgot password page.

Comment 6

[(no longer active)]

Err, sorry, on https://mdedrs.health.maryland.gov/prod/secureLogin

Comment 7

[(no longer active)]

This is a regression from bug 1453751.  This is such a sad bug...

What happens is that the main page is served with a Set-Cookie header and the favicon is set with another Set-Cookie header, and the Set-Cookie header from the favicon seems to override the one received from the main page.

Mossop any chance you can have a look please?

Comment 8

[Dave Townsend [:mossop]]

(In reply to :Ehsan Akhgari from comment #5)
> I can easily reproduce on https://mdedrs.health.maryland.gov.  The session
> cookie changes when you go from the login page to the forgot password page.

You're right that this didn't happen before bug 1453751 landed so Firefox has definitely changed in behaviour here. I can see why too and the fix is straightforward. But I'm not convinced that this is exactly the bug because the same session cookie change happens in Chrome where apparently the webapp works.

Regardless I'll get up the fix to take us back to Firefox 62 behaviour and we can see where that takes us.

Comment 9

[(no longer active)]

(In reply to Dave Townsend [:mossop] (he/him) from comment #8)
> (In reply to :Ehsan Akhgari from comment #5)
> > I can easily reproduce on https://mdedrs.health.maryland.gov.  The session
> > cookie changes when you go from the login page to the forgot password page.
> 
> You're right that this didn't happen before bug 1453751 landed so Firefox
> has definitely changed in behaviour here. I can see why too and the fix is
> straightforward. But I'm not convinced that this is exactly the bug because
> the same session cookie change happens in Chrome where apparently the webapp
> works.

Note that there is a big difference between how Firefox and Chrome set cookies from their content/renderer processes.  In Firefox we set it asynchronously and in Chrome (the last time I checked) they set it synchronously.

But indeed the behavior in comment 5 happens in Chrome too...

Comment 10

[Dave Townsend [:mossop]]

Ok so here is what is happening with the steps in comment 5:

We load https://mdedrs.health.maryland.gov/prod/secureLogin which sets the session cookie for /prod to "foo".

We load the icon from https://mdedrs.health.maryland.gov/favicon.ico, no cookie is sent because no cookie is set for the root. Its response sets the session cookie for /prod to "bar".

We load https://mdedrs.health.maryland.gov/prod/forgot sending the session cookie "bar".

We again load the icon from https://mdedrs.health.maryland.gov/favicon.ico, no cookie is sent because no cookie is set for the root. Its response sets the session cookie for /prod to "baz".

So basically every page load will set the session cookie to something new. This didn't used to happen because pre bug 1453751 we don't even contact the webserver if the icon is in the cache. So once the icon sets the cookie to "bar" above, it sticks going forwards.

Even that seems like buggy behaviour, ideally the webapp would just set the session cookie to "foo" and have it stick. But I think that that is a bug on the server side. /favicon.ico is setting a session cookie for /prod, presumably because it doesn't receive a session cookie in the request. Short of deciding to ignore cookies from icon loads (and who knows what other sites that might break) I think Firefox is doing the right thing there.

Comment 11

[Dave Townsend [:mossop]]

Attachment: Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak

Previous to bug 1453751 favicons were loaded from the network by a <xul:image>
tag with validate="never". This caused us to always use any cached version if
possible. Bug 1453751 used a normal load type causing us to revalidate with the
server for each request. This switches the loader to using the cache if possible.

Comment 12

[(no longer active)]

(In reply to Dave Townsend [:mossop] (he/him) from comment #10)
> Ok so here is what is happening with the steps in comment 5:
> 
> We load https://mdedrs.health.maryland.gov/prod/secureLogin which sets the
> session cookie for /prod to "foo".
> 
> We load the icon from https://mdedrs.health.maryland.gov/favicon.ico, no
> cookie is sent because no cookie is set for the root. Its response sets the
> session cookie for /prod to "bar".
> 
> We load https://mdedrs.health.maryland.gov/prod/forgot sending the session
> cookie "bar".
> 
> We again load the icon from https://mdedrs.health.maryland.gov/favicon.ico,
> no cookie is sent because no cookie is set for the root. Its response sets
> the session cookie for /prod to "baz".
> 
> So basically every page load will set the session cookie to something new.
> This didn't used to happen because pre bug 1453751 we don't even contact the
> webserver if the icon is in the cache. So once the icon sets the cookie to
> "bar" above, it sticks going forwards.
> 
> Even that seems like buggy behaviour, ideally the webapp would just set the
> session cookie to "foo" and have it stick. But I think that that is a bug on
> the server side. /favicon.ico is setting a session cookie for /prod,
> presumably because it doesn't receive a session cookie in the request. Short
> of deciding to ignore cookies from icon loads (and who knows what other
> sites that might break) I think Firefox is doing the right thing there.

Aha, now I see what's happening.  The server is returning a 302 redirect respond for /favicon.ico, pointing you to https://mdedrs.health.maryland.gov/prod/secureLogin;jsessionid=gre3cLnjs-TUZDc3vYBPox6x.worker03 which is basically the login page.  What is happening here is that the server has been configured to redirect any URL that it doesn't understand to the login page, and alongside that request it also serves you a cookie to ensure that the visit to the login page includes the same value in the Cookie header as in the URL.

This interacts poorly with browsers' heuristics to try to guesswork their way and try to download /favicon.ico.  So we hit a non-existing URL, and get redirected to the login page.  That response isn't cacheable since it's a 302, so we end up requesting it over and over again when trying to validate.

So basically the bug is our heuristic hits a case that the server admin hasn't thought about.  Who's to blame, who knows.  But I bet this is why we used to have VALIDATE_NEVER before.  We just didn't have a test for it.  :-(

Comment 13

[Dave Townsend [:mossop]]

(In reply to :Ehsan Akhgari from comment #12)
> This interacts poorly with browsers' heuristics to try to guesswork their
> way and try to download /favicon.ico.  So we hit a non-existing URL, and get
> redirected to the login page.  That response isn't cacheable since it's a
> 302, so we end up requesting it over and over again when trying to validate.

FWIW, I discovered that loading /favicon.ico is actually defined in a spec now. It doesn't specify what cache mode to use so the expectation would be to validate as normal.

https://html.spec.whatwg.org/multipage/links.html#rel-icon

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/655b8b4a0e67
Cache cookies in the same way we did before Firefox 63. r=mak

Comment 15

[Andreea Pavel [:apavel]]

Backed out for failing eslint at gecko/browser/base/content/test/favicons/browser_favicon_cache.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=211235384&revision=655b8b4a0e67c347be75b454ce52a046ccc25a40

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211235384&repo=autoland&lineNumber=273

Backout: https://hg.mozilla.org/integration/autoland/rev/c78efa8bed086c825bb471494cd10d06ca3423d5

Comment 16

[(no longer active)]

(In reply to Dave Townsend [:mossop] (he/him) from comment #13)
> (In reply to :Ehsan Akhgari from comment #12)
> > This interacts poorly with browsers' heuristics to try to guesswork their
> > way and try to download /favicon.ico.  So we hit a non-existing URL, and get
> > redirected to the login page.  That response isn't cacheable since it's a
> > 302, so we end up requesting it over and over again when trying to validate.
> 
> FWIW, I discovered that loading /favicon.ico is actually defined in a spec
> now. It doesn't specify what cache mode to use so the expectation would be
> to validate as normal.
> 
> https://html.spec.whatwg.org/multipage/links.html#rel-icon

Might be worth filing a spec issue about it to get the spec changed based on the findings in this bug (CCing Anne).

Comment 17

[Anne (:annevk)]

I filed https://github.com/whatwg/html/issues/4176 to at least keep track of this.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fddfe98e6db5
Cache cookies in the same way we did before Firefox 63. r=mak

Comment 19

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/fddfe98e6db5

Comment 20

[Dave Townsend [:mossop]]

This fix is now available in the latest Firefox Nightly. Any chance you could test it to see if it completely solves the problem for you? Regardless I'm going to try to get the fix uplifted but it would be useful to know if there is more to do here.

Comment 21

[Dave Townsend [:mossop]]

Comment on attachment 9023808 [details]
Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1453751

User impact if declined: User may be logged out of certain web applications with each page load.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Simple change to how we cache favicon requests.

String changes made/needed:

Comment 22

[Julien Cristau [:jcristau]]

Comment on attachment 9023808 [details]
Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak

load favicons from cache, fixing regression in 63; approved for 64.0b12.

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5021562e6baf

Comment 24

[Pascal Chevrel:pascalc]

Comment on attachment 9023808 [details]
Bug 1503696: Cache cookies in the same way we did before Firefox 63. r=mak

We don't have plans for another dot release and 64 is 2 weeks away, so wontfix for 63.