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

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Ashesh Vashi <[email protected]>
To: Aditya Toshniwal <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Khushboo Vashi <[email protected]>
Cc: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source
Date: Fri, 22 Apr 2022 15:56:58 +0530
Message-ID: <CAG7mmoyaLYkydjR4jrsLJPJEdX2OndBFfum0SjC=4SXXQdoJBA@mail.gmail.com> (raw)
In-Reply-To: <CAM9w-_m=mZZSO-e4DPW3pJ74dyTqy-jBtKt+2rN_dzQOmASozA@mail.gmail.com>
References: <CAFOhELcrv+Rm0bBmXt1-c8NOpsaxinKi=QQTn491dbcUo2xjZA@mail.gmail.com>
	<CANxoLDf_X=dbuhgoyiHNDNEXJB+bMJCDWME5HyYwthbkR1eyKg@mail.gmail.com>
	<CA+OCxow_44OfaR1Nq4WucPaR+9fdknu8wMQpN3MNT4SqRX5XGg@mail.gmail.com>
	<CAFOhELe-f4W-U1+bAhkEBBu2nRNBk6BzYqX9COPE3mi1cQzvJA@mail.gmail.com>
	<CA+OCxozC4Sz9FWKCw7r=8qMjP7cMHOPqw1LHM-6MK5jNk=aWVw@mail.gmail.com>
	<CAFOhELdWm2CwwoiAm61x90YQpGY07kPY591UrP2WUR6ebC-eCQ@mail.gmail.com>
	<CAM9w-_m+KjWGXZv2LQj5uu_vc8zn_zyD4phjL3Kn_8GHaCkoYA@mail.gmail.com>
	<CA+OCxowYF2Jaso1=ceRQXwvuZTwSScGu4x6Mp5a6aNZ+W-U0Zw@mail.gmail.com>
	<CAM9w-_m=mZZSO-e4DPW3pJ74dyTqy-jBtKt+2rN_dzQOmASozA@mail.gmail.com>

On Fri, Apr 22, 2022 at 3:46 PM Aditya Toshniwal <
[email protected]> wrote:

>
>
> On Fri, Apr 22, 2022 at 3:28 PM Dave Page <[email protected]> wrote:
>
>>
>>
>> On Fri, 22 Apr 2022 at 10:49, Aditya Toshniwal <
>> [email protected]> 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.
>
Environment variables are good for static values.
Here - we're talking about dynamic data (per user security key), hence -
hooks are the best way forward.

-- Ashesh

>
>>
>>
>>>
>>> On Fri, Apr 22, 2022 at 2:38 PM Khushboo Vashi <
>>> [email protected]> wrote:
>>>
>>>>
>>>>
>>>> On Fri, Apr 22, 2022 at 2:34 PM Dave Page <[email protected]> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, 22 Apr 2022 at 09:57, Khushboo Vashi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, Apr 22, 2022 at 2:01 PM Dave Page <[email protected]> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Mon, 11 Apr 2022 at 09:20, Akshay Joshi <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Thanks, the patch applied.
>>>>>>>>
>>>>>>>> On Mon, Apr 11, 2022 at 12:00 PM Khushboo Vashi <
>>>>>>>> [email protected]> 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"
>

view thread (14+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source
  In-Reply-To: <CAG7mmoyaLYkydjR4jrsLJPJEdX2OndBFfum0SjC=4SXXQdoJBA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox