CC'd security@ ... Yeah, we definitely want to involve as many people as we can from the original change. Cookies and javascript variables are very different things, cookies are available across tabs/windows (hence why the change breaks auto-login via opening a new tab) and persist beyond the life of the page load. Variables are not available across windows, and when you refresh it's gone, unless you store it in a cookie or use the HTML5 local storage API.
I think it's safe to say that, objectively speaking, putting something in a JS variable is more secure than a cookie, but the real question is why cookies weren't deemed "secure enough", and I have a hunch that there's a specific answer to that which we'd want to find out before doing anything. It looks like Min Chen and Rohit were involved in changing this code, though I can't seem to tie it back to a specific CLOUDSTACK bug. I see references to CS-18149 and CS-19734, but I don't know what those are and the details on the commits are sparse. On Wed, May 27, 2015 at 4:00 AM, Rafael Fonseca <rsafons...@gmail.com> wrote: > Thank you Stephen, it's always nice to have some more people reviewing > this, this just got started through looking at a broken feature, not a > security issue. But if improving security will get the consensus for > actually fixing what's broken, that's what I propose :-) two for one hehe > > Rafael > On May 27, 2015 12:56 PM, "Stephen Turner" <stephen.tur...@citrix.com> > wrote: > > > I've got no specific view on your change, Rafael: I just think security > > matters should be discussed on the security list. I'm copying this email > to > > them. > > > > -- > > Stephen Turner > > > > > > -----Original Message----- > > From: Rafael Fonseca [mailto:rsafons...@gmail.com] > > Sent: 27 May 2015 11:50 > > To: dev@cloudstack.apache.org > > Subject: Re: refresh browser - logged out from ACS ? > > > > Well, if just 'discuss' an issue with a security team, it may do you no > > good as they will not have all the answers without actually > > testing/reviewing If you ask a security team if you should have a client > > handled cookie with credentials they will immediately tell you it's not > > wise to do so, but they will probably not guess that the same data is > just > > getting kept in a js variable or they would also advise against it. > > I think that it's clear that my approach improves security and restores > > functionality (just read a bit about handling sensitive data on js side > and > > have a quick look at my code in the PR) Although this does not propose to > > be a magical security solution, it DOES improve on current security and > > DOES restore a broken functionality. > > > > Feel free to move this to the security list, but ultimately all users > > should be able to view the status in the PR. > > > > > > On Wed, May 27, 2015 at 12:27 PM, Stephen Turner < > > stephen.tur...@citrix.com> > > wrote: > > > > > I know for sure it was discussed with Citrix security team before > > > changing it. Probably also on the ACS security list, but I'm not on > that > > list. > > > Anyway, even if the security concern turns out to be illusory, we > > > shouldn't change a claimed security fix without taking it back to the > > security list. > > > > > > -- > > > Stephen Turner > > > > > > > > > -----Original Message----- > > > From: Rafael Fonseca [mailto:rsafons...@gmail.com] > > > Sent: 27 May 2015 11:16 > > > To: dev@cloudstack.apache.org > > > Subject: Re: refresh browser - logged out from ACS ? > > > > > > This doesn't really do much for security, since the sessionKey is > > > still available to JS in a window variable, so this mostly just breaks > > > functionality and adds no value. > > > This probably wasn't discusses with security experts before > > > implementation, so this just breaks functionality period. > > > My approach does indeed add some security (set a httponly cookie with > > > the > > > data) and restores session persistence. > > > > > > > > > > > > On Wed, May 27, 2015 at 11:50 AM, Stephen Turner < > > > stephen.tur...@citrix.com> > > > wrote: > > > > > > > Is this being discussed on the security list? I think that's the > > > > place for it, because I wouldn't want us to restore the old > > > > behaviour without a proper audit from security experts. > > > > > > > > -- > > > > Stephen Turner > > > > > > > > > > > > -----Original Message----- > > > > From: Rafael Fonseca [mailto:rsafons...@gmail.com] > > > > Sent: 27 May 2015 10:39 > > > > To: dev@cloudstack.apache.org > > > > Subject: Re: refresh browser - logged out from ACS ? > > > > > > > > Hi guys, > > > > > > > > I had a look at this issue yesterday and created a PR to fix it, > > > > it's being discussed here > > > > https://github.com/apache/cloudstack/pull/308 > > > > Since this seems to be a security related issue I will be updating > > > > my PR soon with a secure fix :) > > > > > > > > On Wed, May 27, 2015 at 11:24 AM, Andrija Panic > > > > <andrija.pa...@gmail.com> > > > > wrote: > > > > > > > > > its not the case with i.e. 4.3.2...its is the case with 4.4.3 and > > > > > 4.5.1 at the moment... > > > > > > > > > > On 27 May 2015 at 11:20, Vadim Kimlaychuk > > > > > <vadim.kimlayc...@elion.ee> > > > > > wrote: > > > > > > > > > > > Is it possible to fix? It seems such a behaviour was always be > > > > > > like > > > > this. > > > > > > > > > > > > Vadim. > > > > > > > > > > > > -----Original Message----- > > > > > > From: Andrija Panic [mailto:andrija.pa...@gmail.com] > > > > > > Sent: Wednesday, May 27, 2015 12:17 PM > > > > > > To: dev@cloudstack.apache.org > > > > > > Subject: Re: refresh browser - logged out from ACS ? > > > > > > > > > > > > openign a new windows/tab with same address/URL also break > > things... > > > > > > > > > > > > > > > > > > On 27 May 2015 at 11:11, Stephen Turner > > > > > > <stephen.tur...@citrix.com> > > > > > wrote: > > > > > > > > > > > > > Agreed, I thought it was on opening a new window (maybe a new > > > > > > > tab > > > > > > > too?) rather than refresh. But maybe refresh broke too as a > > > > > > > side > > > > > effect. > > > > > > > > > > > > > > -- > > > > > > > Stephen Turner > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: ilya [mailto:ilya.mailing.li...@gmail.com] > > > > > > > Sent: 27 May 2015 04:28 > > > > > > > To: dev@cloudstack.apache.org > > > > > > > Subject: Re: refresh browser - logged out from ACS ? > > > > > > > > > > > > > > But it was not refresh - to best of my recollection.. > > > > > > > > > > > > > > On 5/26/15 8:27 PM, ilya wrote: > > > > > > > > I vaguely recall Rohit mentioned it was some sort of > > > > > > > > security fix that was causing this side effect due to the > > > > > > > > way sessionids were > > > > > > handled.. > > > > > > > > > > > > > > > > On 5/26/15 8:15 AM, Andrija Panic wrote: > > > > > > > >> Thx Rafael, as usuall :) > > > > > > > >> > > > > > > > >> I remember there was some thread on this topic, but cant > > > > > > > >> really find it... > > > > > > > >> > > > > > > > >> On 26 May 2015 at 17:14, Rafael Fonseca > > > > > > > >> <rsafons...@gmail.com> > > > > > wrote: > > > > > > > >> > > > > > > > >>> Hi Andrija, > > > > > > > >>> > > > > > > > >>> I noticed the same is also happening on the 4.6.0-SNAPSHOT > .. > > > > > > > >>> it's a bit annoying. > > > > > > > >>> > > > > > > > >>> I'll have a closer look later today if i can find the time > > > > > > > >>> for it > > > > > > > >>> :) > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> On Tue, May 26, 2015 at 4:11 PM, Andrija Panic > > > > > > > >>> <andrija.pa...@gmail.com> > > > > > > > >>> wrote: > > > > > > > >>> > > > > > > > >>>> Hi guys, > > > > > > > >>>> > > > > > > > >>>> just wondering - when I refresh browser/UI I get logged > > > > > > > >>>> out of ACS > > > > > > > >>>> - > > > > > > > >>> 4.4.3 > > > > > > > >>>> (testing with 4.5.1 in few minutes...). > > > > > > > >>>> > > > > > > > >>>> I remember there was some thread on this, but can't > > > > > > > >>>> really find it > > > > > > > >>> anywhere > > > > > > > >>>> This behaviour is not present in 4.3 and prior AFAIK. > > > > > > > >>>> > > > > > > > >>>> Any tips ? > > > > > > > >>>> -- > > > > > > > >>>> > > > > > > > >>>> Andrija Panić > > > > > > > >>>> > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > Andrija Panić > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > Andrija Panić > > > > > > > > > > > > > > >
