[PATCH] ini file: make cookie name unique

[Mads Kiilerich]

On 03/06/2015 05:30 PM, Thomas De Schampheleire wrote:
> On Fri, Mar 6, 2015 at 5:26 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 03/04/2015 09:57 PM, Thomas De Schampheleire wrote:
>>> # HG changeset patch
>>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>>> # Date 1425502595 -3600
>>> #      Wed Mar 04 21:56:35 2015 +0100
>>> # Node ID d88fe779ac6cf324062c6a4bd8b5071c8de32c3f
>>> # Parent  fc311d8c3997063a8c6020f4e8d32ca77be339e5
>>> ini file: make cookie name unique
>>>
>>> When several instances of Kallithea are running on the same machine, the
>>> same browser cannot be logged into both instances at the same time without
>>> conflicts. The login session are saved into the same cookie; logging into
>>> one instance closes the session on the second instance and vice-versa.
>>>
>>> This is caused because the cookie name is simply 'kallithea', combined
>>> with
>>> the fact that the cookie specification (RFC6265) states that there is no
>>> isolation of cookies based on port. This means that the browser sends all
>>> cookies from a given domain with all services (Kallithea instances)
>>> running
>>> on that domain, irrespective of port.
>>>
>>> The services thus need to handle any such issue themselves, for example by
>>> using unique cookie names and only interacting with one's own cookie.
>>>
>>> This commit uses the paster-provided 'app_instance_secret' to make the
>>> cookie name unique. We cannot/should not use the app_instance_uuid,
>>> because
>>> this is already used as beaker session secret; exposing it to the cookie
>>> is
>>> insecure. On the other hand, app_instance_secret is not used at all yet so
>>> can safely be used.
>>
>> I don't think it is ok to use app_instance_secret. It is a "secret".
>> Exposing it will probably at some point end up having security implications.
>>
>>> Regarding other ways to make the cookie name unique:
>>> - the port number itself would be sufficiently unique; however it is not
>>>     known at installation time which port the user will use. Depending on
>>> the
>>>     user to make the cookie name unique is not realistic.
>>
>> It is only a problem for developers and admins that happens to run multiple
>> instances. I don't think the problem is that big and it might be realistic
>> to ask the admins to make them unique if they care.
>>
>>> - any other random number would be fine, but it's unclear (to me) how to
>>>     generate such a number through the 'paster make-config' method.
>>
>> Neither do I ... but it seems like that is the only good solution.
>>
>>> - the name of the config file is not sufficiently unique, as the same
>>>     machine could host two Kallithea instances from two different
>>> installation
>>>     directories with the same config file names.
>>
>> I guess that also would require defining a custom template keyword?
>>
>> It could perhaps use the full %(here) path? Or would that be too long or
>> contain invalid characters? Some would probably also consider that an
>> unfortunate information leak... Then perhaps a hash? But that would require
>> a custom template keyword too ...
>>
>>> diff --git a/kallithea/config/deployment.ini_tmpl
>>> b/kallithea/config/deployment.ini_tmpl
>>> --- a/kallithea/config/deployment.ini_tmpl
>>> +++ b/kallithea/config/deployment.ini_tmpl
>>> @@ -345,7 +345,7 @@
>>>    ## file based cookies (default) ##
>>>    #beaker.session.type = file
>>>    -beaker.session.key = kallithea
>>> +beaker.session.key = kallithea-${app_instance_secret}
>>
>> A simple improvement would be to document that this actually is a cookie
>> name (if I understand you correctly) and when it should be customized.
>>
> I haven't tested it yet, but I thank that Andrew's patch wrapping
> around beaker's sessionmiddleware can be used to append the port to
> the cookie name on the fly, thus not needing tweaks in the config
> file...

Ok - I will wait for your testing and feedback on that.

/Mads