MIME-Version: 1.0
In-Reply-To: 
 <CA+OCxownxfR2eDEaXNkgSdFqat6+AQgukrzcYOyoFX0V-zs_VA@mail.gmail.com>
References: 
 <CA+OCxownxfR2eDEaXNkgSdFqat6+AQgukrzcYOyoFX0V-zs_VA@mail.gmail.com>
From: Ashesh Vashi <ashesh.vashi@enterprisedb.com>
Date: Thu, 13 Oct 2016 19:46:56 +0530
Message-ID: 
 <CAG7mmoyhJXXrLv+fgjmgd-Z5GFSaJfHTC89MWQ8LQX3Atw-04A@mail.gmail.com>
Subject: Re: RM1849: Auto-generating security keys
To: Dave Page <dpage@pgadmin.org>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>,
 Josh Berkus <josh@agliodbs.com>,
	=?UTF-8?B?RGV2cmltIEfDnE5Ew5xa?= <devrim@gunduz.org>
Content-Type: multipart/mixed; boundary=001a114acd064ed934053ebfc415
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

--001a114acd064ed934053ebfc415
Content-Type: multipart/alternative; boundary=001a114acd064ed930053ebfc413

--001a114acd064ed930053ebfc413
Content-Type: text/plain; charset=UTF-8

Hi Dave,

On Tue, Oct 11, 2016 at 9:10 PM, Dave Page <dpage@pgadmin.org> wrote:

> Hi Ashesh,
>
> Can you please review the attached patch, and apply if you're happy with
> it?
>
Overall the patch looked good to me.
But - I encounter an issue in 'web' mode, which wont happen with 'runtime'.

Steps for reproduction on existing pgAdmin 4 environment with 'web' mode.
- Apply the patch
- Start the pgAdmin4 application (stand alone application).
- Open pgAdmin home page.
- Log out (if already login).

And, you will see an exception.

I have figure out the issue with the patch.
We were setting the SECURITY_PASSWORD_SALT, after initializing the Security
object.
Hence - it could not set the SECURITY_KEY, and SECURITY_PASSWORD_SALT
properly.

I had moved the Security object initialization after fetching these
configurations from the database.
I have attached a addon patch for the same.

Now - I run into another issue.
Because - the existing password was hashed using the old
SECURITY_PASSWORD_SALT, I am no more able to login to pgAdmin 4.

I think - we need to think about different strategy for upgrading the
configuration file in the 'web' mode.
I was thinking - we can store the existing security configurations in the
database during upgrade process in 'web' mode.

I was not able to spend much time on it due to some other priorities.

--
Thanks & Regards,
Ashesh Vashi


> The purpose is to auto-generate the various security keys that are
> currently in the configuration file, and store them in the SQLite database.
> This allows us to remove the checks for config_local.py and the hard-coded
> default keys which are causing some problems with packaging:
>
> - Hard coded defaults are fine for Desktop mode, and packages generally
> aim to make that work primarily.
> - Hard coded defaults are a security risk for Server mode, hence we
> currently require the user to manually setup keys, which is currently being
> overridden by packagers for Desktop mode.
>
> This change ensures that we have unique security keys for every
> installation, whether running in desktop or server mode (generated from
> os.urandom).
>
> Thanks!
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

--001a114acd064ed930053ebfc413
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi Dave,<div><br></div><div class=3D"gmail_extra"><div cla=
ss=3D"gmail_quote">On Tue, Oct 11, 2016 at 9:10 PM, Dave Page <span dir=3D"=
ltr">&lt;<a href=3D"mailto:dpage@pgadmin.org" target=3D"_blank">dpage@pgadm=
in.org</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"=
margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Ashesh,<d=
iv><br></div><div>Can you please review the attached patch, and apply if yo=
u&#39;re happy with it?</div></blockquote><div>Overall the patch looked goo=
d to me.</div><div>But - I encounter an issue in &#39;web&#39; mode, which =
wont happen with &#39;runtime&#39;.</div><div><br></div><div>Steps for repr=
oduction on existing pgAdmin 4 environment with &#39;web&#39; mode.</div><d=
iv>- Apply the patch</div><div>- Start the pgAdmin4 application (stand alon=
e application).</div><div>- Open pgAdmin home page.</div><div>- Log out (if=
 already login).</div><div><br></div><div>And, you will see an exception.</=
div><div><br></div><div>I have figure out the issue with the patch.</div><d=
iv>We were setting the SECURITY_PASSWORD_SALT, after initializing the Secur=
ity object.</div><div>Hence - it could not set the SECURITY_KEY, and SECURI=
TY_PASSWORD_SALT properly.</div><div><br></div><div>I had moved the Securit=
y object initialization after fetching these configurations from the databa=
se.</div><div>I have attached a addon patch for the same.</div><div><br></d=
iv><div>Now - I run into another issue.</div><div>Because - the existing pa=
ssword was hashed using the old SECURITY_PASSWORD_SALT, I am no more able t=
o login to pgAdmin 4.</div><div><br></div><div>I think - we need to think a=
bout different strategy for upgrading the configuration file in the &#39;we=
b&#39; mode.</div><div>I was thinking - we can store the existing security =
configurations in the database during upgrade process in &#39;web&#39; mode=
.</div><div><br></div><div>I was not able to spend much time on it due to s=
ome other priorities.<br></div><div><br></div><div>--</div><div>Thanks &amp=
; Regards,</div><div>Ashesh Vashi</div><div><br></div><blockquote class=3D"=
gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-=
left:1ex"><div><br></div><div>The purpose is to auto-generate the various s=
ecurity keys that are currently in the configuration file, and store them i=
n the SQLite database. This allows us to remove the checks for config_local=
.py and the hard-coded default keys which are causing some problems with pa=
ckaging:</div><div><br></div><div>- Hard coded defaults are fine for Deskto=
p mode, and packages generally aim to make that work primarily.</div><div>-=
 Hard coded defaults are a security risk for Server mode, hence we currentl=
y require the user to manually setup keys, which is currently being overrid=
den by packagers for Desktop mode.</div><div><br></div><div>This change ens=
ures that we have unique security keys for every installation, whether runn=
ing in desktop or server mode (generated from os.urandom).</div><div><br></=
div><div>Thanks!</div><span class=3D"HOEnZb"><font color=3D"#888888"><br><b=
r>-- <br>Dave Page<br>Blog: <a href=3D"http://pgsnake.blogspot.com" target=
=3D"_blank">http://pgsnake.blogspot.com</a><br>Twitter: @pgsnake<br><br>Ent=
erpriseDB UK: <a href=3D"http://www.enterprisedb.com" target=3D"_blank">htt=
p://www.enterprisedb.com</a><br>The Enterprise PostgreSQL Company<br><br>
</font></span></blockquote></div><br></div></div>

--001a114acd064ed930053ebfc413--

--001a114acd064ed934053ebfc415
Content-Type: application/octet-stream;
	name="add_auto_generate_security_keys.patch"
Content-Disposition: attachment;
	filename="add_auto_generate_security_keys.patch"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_iu8fidaz1

ZGlmZiAtLWdpdCBhL3dlYi9wZ2FkbWluL19faW5pdF9fLnB5IGIvd2ViL3Bn
YWRtaW4vX19pbml0X18ucHkKaW5kZXggMjFmZTYzNi4uYjlkMWNiNCAxMDA2
NDQKLS0tIGEvd2ViL3BnYWRtaW4vX19pbml0X18ucHkKKysrIGIvd2ViL3Bn
YWRtaW4vX19pbml0X18ucHkKQEAgLTIwMSw3ICsyMDEsNyBAQCBkZWYgY3Jl
YXRlX2FwcChhcHBfbmFtZT1jb25maWcuQVBQX05BTUUpOgogCiAgICAgIyBT
ZXR1cCBGbGFzay1TZWN1cml0eQogICAgIHVzZXJfZGF0YXN0b3JlID0gU1FM
QWxjaGVteVVzZXJEYXRhc3RvcmUoZGIsIFVzZXIsIFJvbGUpCi0gICAgc2Vj
dXJpdHkgPSBTZWN1cml0eShhcHAsIHVzZXJfZGF0YXN0b3JlKQorICAgIHNl
Y3VyaXR5ID0gU2VjdXJpdHkoTm9uZSwgdXNlcl9kYXRhc3RvcmUpCiAKICAg
ICAjIFVwZ3JhZGUgdGhlIHNjaGVtYSAoaWYgcmVxdWlyZWQpCiAgICAgd2l0
aCBhcHAuYXBwX2NvbnRleHQoKToKQEAgLTIyNSw2ICsyMjUsMTQgQEAgZGVm
IGNyZWF0ZV9hcHAoYXBwX25hbWU9Y29uZmlnLkFQUF9OQU1FKToKICAgICAg
ICAgY29uZmlnLlNFQ1JFVF9LRVkgPSBLZXlzLnF1ZXJ5LmZpbHRlcl9ieShu
YW1lID0gJ1NFQ1JFVF9LRVknKS5maXJzdCgpLnZhbHVlCiAgICAgICAgIGNv
bmZpZy5TRUNVUklUWV9QQVNTV09SRF9TQUxUID0gS2V5cy5xdWVyeS5maWx0
ZXJfYnkobmFtZSA9ICdTRUNVUklUWV9QQVNTV09SRF9TQUxUJykuZmlyc3Qo
KS52YWx1ZQogCisgICAgIyBVcGRhdGUgdGhlIGFwcC5jb25maWcgd2l0aCBw
cm9wZXIgc2VjdXJpdHkga2V5ZXMgZm9yIHNpZ25pbmcgQ1NSRiBkYXRhLAor
ICAgICMgc2lnbmluZyBjb29raWVzLCBhbmQgdGhlIFNBTFQgZm9yIGhhc2hp
bmcgdGhlIHBhc3N3b3Jkcy4KKyAgICBhcHAuY29uZmlnLnVwZGF0ZShkaWN0
KENTUkZfU0VTU0lPTl9LRVk9Y29uZmlnLkNTUkZfU0VTU0lPTl9LRVkpKQor
ICAgIGFwcC5jb25maWcudXBkYXRlKGRpY3QoU0VDUkVUX0tFWT1jb25maWcu
U0VDUkVUX0tFWSkpCisgICAgYXBwLmNvbmZpZy51cGRhdGUoZGljdChTRUNV
UklUWV9QQVNTV09SRF9TQUxUPWNvbmZpZy5TRUNVUklUWV9QQVNTV09SRF9T
QUxUKSkKKworICAgIHNlY3VyaXR5LmluaXRfYXBwKGFwcCkKKwogICAgIGFw
cC5zZXNzaW9uX2ludGVyZmFjZSA9IGNyZWF0ZV9zZXNzaW9uX2ludGVyZmFj
ZShhcHApCiAKICAgICAjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIwpk
aWZmIC0tZ2l0IGEvd2ViL3NldHVwLnB5IGIvd2ViL3NldHVwLnB5CmluZGV4
IDkwNjk3ZmEuLmVhYTMyMzUgMTAwNzU1Ci0tLSBhL3dlYi9zZXR1cC5weQor
KysgYi93ZWIvc2V0dXAucHkKQEAgLTQyLDE1ICs0Miw2IEBAIGlmIGhhc2F0
dHIoX19idWlsdGluc19fLCAncmF3X2lucHV0Jyk6CiBkZWYgZG9fc2V0dXAo
YXBwKToKICAgICAiIiJDcmVhdGUgYSBuZXcgc2V0dGluZ3MgZGF0YWJhc2Ug
ZnJvbSBzY3JhdGNoIiIiCiAKLSAgICAjIEdldCBzb21lIGRlZmF1bHRzIGZv
ciB0aGUgdmFyaW91cyBrZXlzCi0gICAgd2l0aCBhcHAuYXBwX2NvbnRleHQo
KToKLSAgICAgICAgY29uZmlnLkNTUkZfU0VTU0lPTl9LRVkgPSBiYXNlNjQu
dXJsc2FmZV9iNjRlbmNvZGUob3MudXJhbmRvbSgzMikpCi0gICAgICAgIGFw
cC5qaW5qYV9lbnYuZ2xvYmFsc1snY29uZmlnJ11bJ0NTUkZfU0VTU0lPTl9L
RVknXSA9IGNvbmZpZy5DU1JGX1NFU1NJT05fS0VZCi0gICAgICAgIGNvbmZp
Zy5TRUNSRVRfS0VZID0gYmFzZTY0LnVybHNhZmVfYjY0ZW5jb2RlKG9zLnVy
YW5kb20oMzIpKQotICAgICAgICBhcHAuamluamFfZW52Lmdsb2JhbHNbJ2Nv
bmZpZyddWydTRUNSRVRfS0VZJ10gPSBjb25maWcuU0VDUkVUX0tFWQotICAg
ICAgICBjb25maWcuU0VDVVJJVFlfUEFTU1dPUkRfU0FMVCA9IGJhc2U2NC51
cmxzYWZlX2I2NGVuY29kZShvcy51cmFuZG9tKDMyKSkKLSAgICAgICAgYXBw
LmppbmphX2Vudi5nbG9iYWxzWydjb25maWcnXVsnU0VDVVJJVFlfUEFTU1dP
UkRfU0FMVCddID0gY29uZmlnLlNFQ1VSSVRZX1BBU1NXT1JEX1NBTFQKLQog
ICAgIGlmIGNvbmZpZy5TRVJWRVJfTU9ERSBpcyBGYWxzZToKICAgICAgICAg
cHJpbnQoIk5PVEU6IENvbmZpZ3VyaW5nIGF1dGhlbnRpY2F0aW9uIGZvciBE
RVNLVE9QIG1vZGUuIikKICAgICAgICAgZW1haWwgPSBjb25maWcuREVTS1RP
UF9VU0VSCkBAIC0xNTAsNyArMTQxLDcgQEAgZGVmIGRvX3NldHVwKGFwcCk6
CiAgICAgKQogCiAKLWRlZiBkb191cGdyYWRlKGFwcCwgZGF0YXN0b3JlLCBz
ZWN1cml0eSwgdmVyc2lvbik6CitkZWYgZG9fdXBncmFkZShhcHAsIGRhdGFz
dG9yZSwgdmVyc2lvbik6CiAgICAgIiIiVXBncmFkZSBhbiBleGlzdGluZyBz
ZXR0aW5ncyBkYXRhYmFzZSIiIgogICAgICMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjCiAgICAgIyBSdW4gd2hhdGV2ZXIgaXMgcmVxdWlyZWQgdG8gdXBk
YXRlIHRoZSBkYXRhYmFzZSBzY2hlbWEgdG8gdGhlIGN1cnJlbnQKQEAgLTM4
Niw2ICszNzcsMTIgQEAgQ1JFQVRFIFRBQkxFIGtleXMgKAogIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIwogaWYgX19uYW1lX18gPT0gJ19f
bWFpbl9fJzoKICAgICBhcHAgPSBGbGFzayhfX25hbWVfXykKKworICAgICMg
R2V0IHNvbWUgZGVmYXVsdHMgZm9yIHRoZSB2YXJpb3VzIGtleXMKKyAgICBj
b25maWcuQ1NSRl9TRVNTSU9OX0tFWSA9IGJhc2U2NC51cmxzYWZlX2I2NGVu
Y29kZShvcy51cmFuZG9tKDMyKSkKKyAgICBjb25maWcuU0VDUkVUX0tFWSA9
IGJhc2U2NC51cmxzYWZlX2I2NGVuY29kZShvcy51cmFuZG9tKDMyKSkKKyAg
ICBjb25maWcuU0VDVVJJVFlfUEFTU1dPUkRfU0FMVCA9IGJhc2U2NC51cmxz
YWZlX2I2NGVuY29kZShvcy51cmFuZG9tKDMyKSkKKwogICAgIGFwcC5jb25m
aWcuZnJvbV9vYmplY3QoY29uZmlnKQogCiAgICAgaWYgY29uZmlnLlRFU1RJ
TkdfTU9ERToKQEAgLTQxMSw3ICs0MDgsNiBAQCBFbnRlcmluZyB1cGdyYWRl
IG1vZGUuLi4iIiIgJSBjb25maWcuU1FMSVRFX1BBVEgpCiAKICAgICAgICAg
IyBTZXR1cCBGbGFzay1TZWN1cml0eQogICAgICAgICB1c2VyX2RhdGFzdG9y
ZSA9IFNRTEFsY2hlbXlVc2VyRGF0YXN0b3JlKGRiLCBVc2VyLCBSb2xlKQot
ICAgICAgICBzZWN1cml0eSA9IFNlY3VyaXR5KGFwcCwgdXNlcl9kYXRhc3Rv
cmUpCiAKICAgICAgICAgIyBBbHdheXMgdXNlICI8IFJFUVVJUkVEX1ZFUlNJ
T04iIGFzIHRoZSB0ZXN0IGZvciByZWFkYWJpbGl0eQogICAgICAgICB3aXRo
IGFwcC5hcHBfY29udGV4dCgpOgpAQCAtNDMzLDcgKzQyOSw3IEBAIEV4aXRp
bmcuLi4iIiIgJSAodmVyc2lvbi52YWx1ZSkpCiAgICAgICAgICAgICBwcmlu
dCgiTk9URTogVXBncmFkaW5nIGRhdGFiYXNlIHNjaGVtYSBmcm9tIHZlcnNp
b24gJWQgdG8gJWQuIiAlICgKICAgICAgICAgICAgICAgICB2ZXJzaW9uLnZh
bHVlLCBjb25maWcuU0VUVElOR1NfU0NIRU1BX1ZFUlNJT04KICAgICAgICAg
ICAgICkpCi0gICAgICAgICAgICBkb191cGdyYWRlKGFwcCwgdXNlcl9kYXRh
c3RvcmUsIHNlY3VyaXR5LCB2ZXJzaW9uKQorICAgICAgICAgICAgZG9fdXBn
cmFkZShhcHAsIHVzZXJfZGF0YXN0b3JlLCB2ZXJzaW9uKQogICAgIGVsc2U6
CiAgICAgICAgIGRpcmVjdG9yeSA9IG9zLnBhdGguZGlybmFtZShjb25maWcu
U1FMSVRFX1BBVEgpCiAgICAgICAgIGlmIG5vdCBvcy5wYXRoLmV4aXN0cyhk
aXJlY3RvcnkpOgo=

--001a114acd064ed934053ebfc415
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--001a114acd064ed934053ebfc415--