[PATCH] ini file: make cookie name unique

[Thomas De Schampheleire]

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...