Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nhqVT-00060O-72 for pgadmin-hackers@arkaria.postgresql.org; Fri, 22 Apr 2022 10:27:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nhqVR-0005NJ-IL for pgadmin-hackers@arkaria.postgresql.org; Fri, 22 Apr 2022 10:27:17 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nhqVR-0005NA-1e for pgadmin-hackers@lists.postgresql.org; Fri, 22 Apr 2022 10:27:17 +0000 Received: from mail-pl1-x632.google.com ([2607:f8b0:4864:20::632]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1nhqVL-0005CC-L0 for pgadmin-hackers@postgresql.org; Fri, 22 Apr 2022 10:27:15 +0000 Received: by mail-pl1-x632.google.com with SMTP id s14so9887280plk.8 for ; Fri, 22 Apr 2022 03:27:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=XGzkXeQ1J/Re+UUYtiZWeRdVOlIVS6fz1hRFo85Ojms=; b=lhEDI85F4p2pqRLWVQIFYLVEqrzk2Dki6an1e4B9dQuRPNsnX9ehFLduN7kK/9VePN Ph6gzsb3QLMAR6B0kAQc6ITA3bkIvJbN+gfURVgtGH5kxJUSNZjtvx1TpQGZejPpJKfi eczUvgycjlBv8hgRYzQzKLXAaEshqJ+cUlNemDFGGKRZypggM0lzFnPZ/tvZkX59FeUH hhhzIm2Dn4tnPeE/kXeJJOgBnamXvByTPVsd8VgoWR2VgqP6g97Yz5NNkidqm3JLl8JH VgXcgf32Wja31EgHhP1pJmfCsxqYDnhzTxFRVIMxfMpkNnuu3cUqCU6a+v/8YhCW43ak djFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=XGzkXeQ1J/Re+UUYtiZWeRdVOlIVS6fz1hRFo85Ojms=; b=zp8ooJ4RDHMTmEFkCd45GSH3w1V+HYJreD+R+rwMiV6lVXnW5IPWhjt/Z8oASgzObs fBfTHcQa9ZSw8uwXCOl630vAZLSYRwE2y2DoKmZmur+cAjrPolohGakfoQFlXrT6mYIS LLJdaXIoxsHq6Kl/BRCxdqcgW/101oJyTbvNSnS2dPp/ic6/+ypzn4IpkNuzNwbUKixF r11H8W3PsatFJpRBYmvHAEmL93QE6pGcZmGfdqNSHxYeXqhxodhZP0OfooxmQLNKEvK5 KbhVOItwnGxkdFAbGHZh91a/3tiZAhn1nKRV+5t5SA7AXulcQw2nWO30wxRiZTt/qXo1 95EA== X-Gm-Message-State: AOAM530Q+pTqVkJONU3Gor5xUq4f5PLLU6xf6rna254vmnVJWMp3wGBe ZS/5qBvYE7jkt6qfXk2mjyJv7TiwbYmkUmlTOux709ZH9X/EDjknOxTtg64Y1ieBd8clw56nv0d gvLwcObDfpyh1f4NSqmIrNOZ8rVCTrzMeAq0kI7erypNsz/xAIvBUmfObd3QR0snATaY7dLXiEV wwJLV9X3izBfOB038yU+f/DealhtO6kq8tH5bHroyi0WzJ5vlpVcCloYOl8w== X-Google-Smtp-Source: ABdhPJzCrL8ZBWi5x6p6Ok9JtK/rhctrpQ3d+SqmrfmpEl4y8CvAAg7v2YSEPKI6gAOId1qYVUHcU1U6LGY2DtAKeNU= X-Received: by 2002:a17:902:a5cc:b0:158:9a2b:480e with SMTP id t12-20020a170902a5cc00b001589a2b480emr3793847plq.19.1650623230009; Fri, 22 Apr 2022 03:27:10 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Ashesh Vashi Date: Fri, 22 Apr 2022 15:56:58 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source To: Aditya Toshniwal Cc: Dave Page , Khushboo Vashi , Akshay Joshi , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000487c1405dd3babb5" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000487c1405dd3babb5 Content-Type: text/plain; charset="UTF-8" On Fri, Apr 22, 2022 at 3:46 PM Aditya Toshniwal < aditya.toshniwal@enterprisedb.com> wrote: > > > On Fri, Apr 22, 2022 at 3:28 PM Dave Page wrote: > >> >> >> On Fri, 22 Apr 2022 at 10:49, Aditya Toshniwal < >> aditya.toshniwal@enterprisedb.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. > 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 < >>> khushboo.vashi@enterprisedb.com> wrote: >>> >>>> >>>> >>>> On Fri, Apr 22, 2022 at 2:34 PM Dave Page wrote: >>>> >>>>> >>>>> >>>>> On Fri, 22 Apr 2022 at 09:57, Khushboo Vashi < >>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Fri, Apr 22, 2022 at 2:01 PM Dave Page wrote: >>>>>> >>>>>>> Hi >>>>>>> >>>>>>> On Mon, 11 Apr 2022 at 09:20, Akshay Joshi < >>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Thanks, the patch applied. >>>>>>>> >>>>>>>> On Mon, Apr 11, 2022 at 12:00 PM Khushboo Vashi < >>>>>>>> khushboo.vashi@enterprisedb.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* >>> >>> "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* > > "Don't Complain about Heat, Plant a TREE" > --000000000000487c1405dd3babb5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, Apr 22, 2022 at 3:46 PM Aditya T= oshniwal <aditya.to= shniwal@enterprisedb.com> wrote:


On Fri, Apr 22, 2022 at 3:28 PM Dave Page <dpage@pgadmin.org> w= rote:


On Fri, 22 Apr 2022 at 10:49, Aditya Toshniwa= l <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

Generally, = secure keys like API_KEYS and all are supposed to be set in env and are rea= d by the app. Similar is the alternative encryption key.
People can run their scripts to export tho= se config vars.

On the client s= ide, yes. This is server side though. It's not uncommon on the server s= ide to include hooks to allow key retrieval from external key management sy= stems.
Even on the server side. Like the AWS= auth keys, or DB passwords. We can include hooks, not against it. Just dis= cussing.=C2=A0
Environment variab= les are good for static values.
Here - we're talking about dy= namic data (per user security key), hence - hooks are the best way forward.=

-- Ashesh

=C2=A0

On Fri, Apr 22, 2022 at 2:38 PM Khushboo Vashi <khushboo.vashi@enterprise= db.com> wrote:


On Fri, Apr 22, 2022 at 2:34 PM= Dave Page <dpage= @pgadmin.org> wrote:


On Fri, 22 Apr 2022 at 09= :57, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">


On Fri, Apr 22, 2022 at 2:01 PM Dave Page <dpage@pgadmin.org> wrote:
=
Hi

On Mon, 11 Apr 2022 at 09:20, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Thanks, the patch applied.

On Mon, Apr 11, 2022 at= 12:00 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

Please find the attached patch t= o implement the feature #7012 - Disable master password requirement when us= ing alternative auth source

When pgAdmin stores a connection password, it enc= rypts it using a key that is formed either from the master password, or fro= m 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 anythi= ng long-lived to form the encryption key from, hence it uses the master pas= sword. And if the master is disabled, there is no way to store the connecti= on password.

To resolve this, we have added an option to config.py (which defaults t= o None) for an alternate encryption key. pgAdmin would use this if a) the m= aster password is disabled AND b) there is no suitable key/password availab= le from the auth module for the user.=C2=A0If the option is set to None, pgAdmi= n works as it does now.=C2=A0


This change has just been brought to my attent= ion through other work. I think this is poorly thought out, and could easil= y 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 sho= uld allow the user to specify the path to a script or program that will ret= urn a key. In a security-conscious environment, the script might query a ce= ntralised key management system to securely retrieve the key to use. If a u= ser really wants the less secure implementation that this current patch off= ers, then a simple script as follows would offer that (but would not be rec= ommended):

=3D=3D=3D=3D
#!/bin/sh
<= div>
echo "my secret key"
=3D=3D=3D=3D

We would probably also want to allow use of a placeh= older in which the username can be passed, e.g.

MA= STER_ENCRYPTION_KEY_SCRIPT =3D '/path/to/get-key.sh %u'
<= br>
Sounds good to me.=C2=A0
D= oes this mean we are going to remove the current implementation which offer= s a hard-coded master password?

Yes, I = think that is the way to go. I don't want to add a config parameter tha= t doesn't seem like a good solution, and then remove it again in the ne= xt release.

Ok, In that case= , we need to revert the patch and also need to update the RM #7012 regardin= g our proposal.=C2=A0

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

EDB: https://www.enterprise= db.com



--
Thanks,
Aditya Toshniwal=
pgAdmin Hacker=C2=A0| Software Architect=C2=A0| = edbpostgres.com
=
&q= uot;Don't Complain about Heat, Plant a TREE"


--
Dave Page
Blog: https://pgsnake.blogspot.com
Twitter: @pgsnake<= br>
EDB: http= s://www.enterprisedb.com



--
Thanks,
Aditya Toshniwal=
pgAdmin Hacker=C2=A0| Software Architect=C2=A0| = edbpostgres.com
=
&q= uot;Don't Complain about Heat, Plant a TREE"
--000000000000487c1405dd3babb5--