Re: RM1849: Auto-generating security keys

Patch applied.

Fahar, can you please test this thoroughly in desktop and server modes,
with both fresh and upgraded installations?

https://redmine.postgresql.org/issues/1849

Packagers: This change means that packages are no longer forced to create a
config_local.py file, and there is no longer any need to explicitly set
SECURITY_PASSWORD_SALT, SECURITY_KEY and CSRF_SESSION_KEY in the config (in
fact, they should be removed for new installations, if you have included
them in 1.0)

Thanks.

On Wed, Oct 19, 2016 at 6:46 AM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> Hi Dave,
>
> On Sat, Oct 15, 2016 at 8:02 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>>
>> On Friday, October 14, 2016, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Thursday, October 13, 2016, Ashesh Vashi <
>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Tue, Oct 11, 2016 at 9:10 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi Ashesh,
>>>>>
>>>>> Can you please review the attached patch, and apply if you're happy
>>>>> with it?
>>>>>
>>>> Overall the patch looked good to me.
>>>> But - I encounter an issue in 'web' mode, which wont happen with
>>>> 'runtime'.
>>>>
>>>> Steps for reproduction on existing pgAdmin 4 environment with 'web'
>>>> mode.
>>>> - Apply the patch
>>>> - Start the pgAdmin4 application (stand alone application).
>>>> - Open pgAdmin home page.
>>>> - Log out (if already login).
>>>>
>>>> And, you will see an exception.
>>>>
>>>> I have figure out the issue with the patch.
>>>> We were setting the SECURITY_PASSWORD_SALT, after initializing the
>>>> Security object.
>>>> Hence - it could not set the SECURITY_KEY, and SECURITY_PASSWORD_SALT
>>>> properly.
>>>>
>>>
>>> Hmm.
>>>
>>>
>>>>
>>>> I had moved the Security object initialization after fetching these
>>>> configurations from the database.
>>>> I have attached a addon patch for the same.
>>>>
>>>
>>> OK, thanks.
>>>
>>>
>>>>
>>>> Now - I run into another issue.
>>>> Because - the existing password was hashed using the old
>>>> SECURITY_PASSWORD_SALT, I am no more able to login to pgAdmin 4.
>>>>
>>>> I think - we need to think about different strategy for upgrading the
>>>> configuration file in the 'web' mode.
>>>> I was thinking - we can store the existing security configurations in
>>>> the database during upgrade process in 'web' mode.
>>>>
>>>
>>> My concern with that is that we'll likely be storing the default config
>>> values in many cases, thus for those users, perpetuating the problem.
>>>
>>> I guess what we need to do is re-encrypt the password during the upgrade
>>> - however, that makes me think; we then have both the key and the encrypted
>>> passwords in the same database which is clearly not a good idea. Sigh...
>>> Needs more thought.
>>>
>>
>> OK, so I've been thinking about this and experimenting for a couple of
>> hours, as well as annoying the crap out of Magnus by thinking out loud in
>> his general direction, and it looks like this isn't a major problem as from
>> what I can see, SECURITY_PASSWORD_SALT is (aside from really being a key
>> not a salt) not the only salting that's done.
>>
>> It looks like it's used system-wide as the key to generate an HMAC of the
>> users password, which is then passed to passlib which salts and hashes it.
>> I did some testing, and found that two users with the same password end up
>> with different hashes in the database, so clearly there is also per-user
>> salting happening. I also created two users, then dropped the database and
>> created the same user accounts with the same passwords again, and found
>> that the resulting hashes were different in both databases - thus there is
>> something else ensuring the hashes are unique across different
>> installations/databases.
>>
>> So, I believe we can do as you suggest and migrate existing values for
>> SECURITY_PASSWORD_SALT, given that there's clearly some other per user and
>> per installation/database salting going on anyway. New installations can
>> have the random value for SECURITY_PASSWORD_SALT.
>>
> We do not need to generate the random SECURITY_PASSWORD_SALT during
> upgrade mode, which was wrong added in my addon patch.
>
> Please find the updated patch.
>
> Otherwise - looks good to me.
> Please commit the new patch (if you're ok with the change).
>
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> <http://www.enterprisedb.com/>
>
>
> *http://www.linkedin.com/in/asheshvashi*
> <http://www.linkedin.com/in/asheshvashi>
>
>>
>> I don't believe SECURITY_KEY and CSRF_SESSION_KEY are issues either, as
>> they're used for purposes that are essentially ephemeral, and thus can be
>> changed during an upgrade.
>>
>> Adding Magnus as I'd appreciate any thoughts he may have.
>>
>> Patch attached - please review (Ashesh, but others too would be
>> appreciated)!
>>
>> Thanks.
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company