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 1nhq3y-0004WL-AN for pgadmin-hackers@arkaria.postgresql.org; Fri, 22 Apr 2022 09:58:54 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nhq3x-0003ZK-6V for pgadmin-hackers@arkaria.postgresql.org; Fri, 22 Apr 2022 09:58:53 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nhq3w-0003ZB-Sn for pgadmin-hackers@lists.postgresql.org; Fri, 22 Apr 2022 09:58:52 +0000 Received: from mail-ej1-x635.google.com ([2a00:1450:4864:20::635]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1nhq3q-0007A1-UM for pgadmin-hackers@postgresql.org; Fri, 22 Apr 2022 09:58:52 +0000 Received: by mail-ej1-x635.google.com with SMTP id g18so15311645ejc.10 for ; Fri, 22 Apr 2022 02:58:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8Y1dHsAVb2MywIf4SVrzpgNUrucLBgNfQJN88JUlxM8=; b=EugNB4/9fFBA6nf8kDwv/aZfomtHwQtIgK1QMVNAXQvTENKLwnt1JWWOpX6R23iilG 3Rzgs2/uMx6H8Gk123sFvtiou7HwXI0USJrm8jMuw5G0zwMgLLTYt6f+wKZ3X6FVHez9 FBjcKk2cKDLCFCH9zCyoXy3CyWFbKQcQY20RQrIYJTQheLic7tTSdXf5dNbg2qzUZF+5 gX7+D5N1AMEcv9qYenk5JabOhMJATDefHUWWAJH5Kn2onsuWsKJqHFHi+PwgHWgtt8CH 3O4+coVgRKXqsRlS91mbdVpj0ezxe1hqQxL1/0CZcIPeRP6pZJ30wMex3MXGSx3zD30m NM7A== 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=8Y1dHsAVb2MywIf4SVrzpgNUrucLBgNfQJN88JUlxM8=; b=bM4dUhGLeSWKZ6VI1zmVkXmP3T4DSgb36BS0NCwMXaIO23QO+dyqsAgTtWTOusSIkI k0zng+HALeEBrSdBZlMwhPxye7PVsrS/Su6POtmhXbxi5hX9qZ9ip+2kuCcb2HpF2+KT FL3HS2rrdZaQ27bOrzFjC6yG1scbSrb/bPxN9cJOeo0i7qDRygeutDH9qxF7UZZDwq5J ln6EsKoqzmjP9gc4s4MxijiRD/9jhtUYyu6Xg1NfeBM8YurfaPVeZaB1eo41+1FVT1wF b9qpZ00J56259vuybWrkLust+b09fzYIqX2A7PbmwniQ4opaaV10T3ur4ZEH82s+g+6T 0WWQ== X-Gm-Message-State: AOAM532Z7tZ4rikg1EdzG5vzMW59xi4GYGCxTioQvD+VjHOeGWJfksmm 5ku43uiDLKp/duPfLMIPk9xXdphlqrhriflqt8byEA== X-Google-Smtp-Source: ABdhPJxQULFiPweGIuBmf46XmG+PkE8SLYhDtepgZpnV9uIO71ZmSg7Acz1hIO1cnDdHD5t9EOddjRO+Qy0Ar33qTLY= X-Received: by 2002:a17:907:3f86:b0:6df:ad43:583 with SMTP id hr6-20020a1709073f8600b006dfad430583mr3436337ejc.535.1650621525729; Fri, 22 Apr 2022 02:58:45 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Fri, 22 Apr 2022 10:58:34 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch]- Feature #7012 - disable master password requirement when using alternative auth source To: Aditya Toshniwal Cc: Khushboo Vashi , Akshay Joshi , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000b33e7105dd3b452b" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000b33e7105dd3b452b Content-Type: text/plain; charset="UTF-8" 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. > > 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 --000000000000b33e7105dd3b452b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, 22 Apr 2022 at 10:49, Aditya = Toshniwal <aditya.t= oshniwal@enterprisedb.com> wrote:
Hi Dave,

Generally, secure keys like API_KEYS and all are suppo= sed to be set in env and are read by the app. Similar is the alternative en= cryption key.
People can= run their scripts to export those config vars.

On the client side, yes. This is server side though. It= 9;s not uncommon on the server side to include hooks to allow key retrieval= from external key management systems.

=C2=A0

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


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

On Fri, 2= 2 Apr 2022 at 09:57, Khushboo Vashi <khushboo.vashi@enterprisedb.com> w= rote:


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

On Mon, 11 Apr 2022 at 09:20, Akshay Joshi &l= t;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 s= tores a connection password, it encrypts it using a key that is formed eith= er from the master password, or from the pgAdmin login password for the use= r. In the case of auth methods such as OAuth, Kerberos or Webserver, pgAdmi= n 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, th= ere is no way to store the connection password.

To resolve this, we have added an op= tion 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.=C2= =A0I= f the option is set to None, pgAdmin works as it does now.=C2=A0

=

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 ha= rd-coding a master password, which is only slightly more secure than not ha= ving one in the first place, we should allow the user to specify the path t= o a script or program that will return a key. In a security-conscious envir= onment, the script might query a centralised key management system to secur= ely retrieve the key to use. If a user really wants the less secure impleme= ntation that this current patch offers, then a simple script as follows wou= ld offer that (but would not be recommended):

=3D= =3D=3D=3D
#!/bin/sh

echo "my secret= key"
=3D=3D=3D=3D

We would probabl= y also want to allow use of a placeholder in which the username can be pass= ed, e.g.

MASTER_ENCRYPTION_KEY_SCRIPT =3D '/pa= th/to/get-key.sh %u'

= Sounds good to me.=C2=A0
Does this mean we are going to remove th= e 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 c= ase, we need to revert the patch and also need to update the RM #7012 regar= ding our proposal.=C2=A0

--
=


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


--
<= /div> --000000000000b33e7105dd3b452b--