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

From: Yogesh Mahajan <yogesh(dot)mahajan(at)enterprisedb(dot)com>
To: Dave Page <dave(dot)page(at)enterprisedb(dot)com>
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>, Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Subject: Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source
Date: 2023-05-03 09:44:22
Message-ID: CAMa=N=NAs-gucwZazxurGWsGwQOpni2nvG8ARpr=QSpDhUQB7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave/Team,

As per the new design, pgAdmin should add a config to specify a path for
script/program to retrieve an encryption key & use it to encrypt the
passwords.
The script/program will be at an application level and not a user level.
This feature will be applicable only in case of server mode as we are going
to use OS level secret storage for the same in Desktop mode.

Thanks,
Yogesh Mahajan
EnterpriseDB

On Fri, Apr 22, 2022 at 4:01 PM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

>
> On Fri, Apr 22, 2022 at 3:57 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Fri, 22 Apr 2022 at 11:16, Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> 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.
>>>
>>
>> If you're using an AWS auth key on a server, then you're acting as a
>> client for AWS - and DB passwords are a great example of why using a hook
>> is a good thing; it's a very common request from users to have a secure way
>> to retrieve credentials from an external service. Not to mention that a DB
>> password is needed on the client side of a connection, not on the server
>> side. On the server side, the database would query LDAP/Kerberos/whatever.
>>
>> A better example would be querying a key management service to unlock an
>> encrypted disk or something like the service Bruce wrote for managing
>> pgcrypto keys.
>>
>
> Got it. Thanks.
>
>>
>>
>>
>>>
>>>>
>>>>
>>>>>
>>>>> 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"
>>>
>>
>>
>> --
>> 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 Aditya Toshniwal 2023-05-03 09:48:30 [pgadmin-org/pgadmin4] 4fbfcd: Fix multiple object breadcrumbs bugs. #2078
Previous Message Akshay Joshi 2023-05-03 07:07:55 [pgadmin-org/pgadmin4] cba42e: Allow user to set the minimum value to 1 from pref...