Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source
Date: 2022-04-22 10:16:00
Message-ID: CAM9w-_m=mZZSO-e4DPW3pJ74dyTqy-jBtKt+2rN_dzQOmASozA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Fri, Apr 22, 2022 at 3:28 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Fri, 22 Apr 2022 at 10:49, Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> Generally, secure keys like API_KEYS and all are supposed to be set in
>> env and are read by the app. Similar is the alternative encryption key.
>> People can run their scripts to export those config vars.
>>
>
> On the client side, yes. This is server side though. It's not uncommon on
> the server side to include hooks to allow key retrieval from external key
> management systems.
>
Even on the server side. Like the AWS auth keys, or DB passwords. We can
include hooks, not against it. Just discussing.

>
>
>
>>
>> On Fri, Apr 22, 2022 at 2:38 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Fri, Apr 22, 2022 at 2:34 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Fri, 22 Apr 2022 at 09:57, Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, Apr 22, 2022 at 2:01 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Mon, 11 Apr 2022 at 09:20, Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Thanks, the patch applied.
>>>>>>>
>>>>>>> On Mon, Apr 11, 2022 at 12:00 PM Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find the attached patch to implement the feature #7012 -
>>>>>>>> Disable master password requirement when using alternative auth source
>>>>>>>>
>>>>>>>> When pgAdmin stores a connection password, it encrypts it using a
>>>>>>>> key that is formed either from the master password, or from the pgAdmin
>>>>>>>> login password for the user. In the case of auth methods such as OAuth,
>>>>>>>> Kerberos or Webserver, pgAdmin doesn't have access to anything long-lived
>>>>>>>> to form the encryption key from, hence it uses the master password. And if
>>>>>>>> the master is disabled, there is no way to store the connection password.
>>>>>>>>
>>>>>>>> To resolve this, we have added an option to config.py (which
>>>>>>>> defaults to None) for an alternate encryption key. pgAdmin would use this
>>>>>>>> if a) the master password is disabled AND b) there is no suitable
>>>>>>>> key/password available from the auth module for the user. If the
>>>>>>>> option is set to None, pgAdmin works as it does now.
>>>>>>>>
>>>>>>>
>>>>>> This change has just been brought to my attention through other work.
>>>>>> I think this is poorly thought out, and could easily be made much more
>>>>>> secure and flexible than the current design.
>>>>>>
>>>>>> Instead of effectively hard-coding a master password, which is only
>>>>>> slightly more secure than not having one in the first place, we should
>>>>>> allow the user to specify the path to a script or program that will return
>>>>>> a key. In a security-conscious environment, the script might query a
>>>>>> centralised key management system to securely retrieve the key to use. If a
>>>>>> user really wants the less secure implementation that this current patch
>>>>>> offers, then a simple script as follows would offer that (but would not be
>>>>>> recommended):
>>>>>>
>>>>>> ====
>>>>>> #!/bin/sh
>>>>>>
>>>>>> echo "my secret key"
>>>>>> ====
>>>>>>
>>>>>> We would probably also want to allow use of a placeholder in which
>>>>>> the username can be passed, e.g.
>>>>>>
>>>>>> MASTER_ENCRYPTION_KEY_SCRIPT = '/path/to/get-key.sh %u'
>>>>>>
>>>>>> Sounds good to me.
>>>>> Does this mean we are going to remove the current implementation which
>>>>> offers a hard-coded master password?
>>>>>
>>>>>>
>>>> Yes, I think that is the way to go. I don't want to add a config
>>>> parameter that doesn't seem like a good solution, and then remove it again
>>>> in the next release.
>>>>
>>>> Ok, In that case, we need to revert the patch and also need to update
>>> the RM #7012 regarding our proposal.
>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: https://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EDB: https://www.enterprisedb.com
>>>>
>>>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>> <http://edbpostgres.com>
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Dave Page
> Blog: https://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: https://www.enterprisedb.com
>
>

--
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Software Architect | *edbpostgres.com*
<http://edbpostgres.com>
"Don't Complain about Heat, Plant a TREE"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2022-04-22 10:26:58 Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source
Previous Message Dave Page 2022-04-22 09:58:34 Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source