Hi Dave, On Sat, Oct 15, 2016 at 8:02 AM, Dave Page <dp...@pgadmin.org> wrote:
> Hi > > > On Friday, October 14, 2016, Dave Page <dp...@pgadmin.org> wrote: > >> Hi >> >> On Thursday, October 13, 2016, Ashesh Vashi < >> ashesh.va...@enterprisedb.com> wrote: >> >>> Hi Dave, >>> >>> On Tue, Oct 11, 2016 at 9:10 PM, Dave Page <dp...@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. >>> >> >> Hmm. >> >> >>> >>> I had moved the Security object initialization after fetching these >>> configurations from the database. >>> I have attached a addon patch for the same. >>> >> >> OK, thanks. >> >> >>> >>> 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. >>> >> >> My concern with that is that we'll likely be storing the default config >> values in many cases, thus for those users, perpetuating the problem. >> >> I guess what we need to do is re-encrypt the password during the upgrade >> - however, that makes me think; we then have both the key and the encrypted >> passwords in the same database which is clearly not a good idea. Sigh... >> Needs more thought. >> > > OK, so I've been thinking about this and experimenting for a couple of > hours, as well as annoying the crap out of Magnus by thinking out loud in > his general direction, and it looks like this isn't a major problem as from > what I can see, SECURITY_PASSWORD_SALT is (aside from really being a key > not a salt) not the only salting that's done. > > It looks like it's used system-wide as the key to generate an HMAC of the > users password, which is then passed to passlib which salts and hashes it. > I did some testing, and found that two users with the same password end up > with different hashes in the database, so clearly there is also per-user > salting happening. I also created two users, then dropped the database and > created the same user accounts with the same passwords again, and found > that the resulting hashes were different in both databases - thus there is > something else ensuring the hashes are unique across different > installations/databases. > > So, I believe we can do as you suggest and migrate existing values for > SECURITY_PASSWORD_SALT, given that there's clearly some other per user and > per installation/database salting going on anyway. New installations can > have the random value for SECURITY_PASSWORD_SALT. > We do not need to generate the random SECURITY_PASSWORD_SALT during upgrade mode, which was wrong added in my addon patch. Please find the updated patch. Otherwise - looks good to me. Please commit the new patch (if you're ok with the change). -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company <http://www.enterprisedb.com/> *http://www.linkedin.com/in/asheshvashi* <http://www.linkedin.com/in/asheshvashi> > > I don't believe SECURITY_KEY and CSRF_SESSION_KEY are issues either, as > they're used for purposes that are essentially ephemeral, and thus can be > changed during an upgrade. > > Adding Magnus as I'd appreciate any thoughts he may have. > > Patch attached - please review (Ashesh, but others too would be > appreciated)! > > Thanks. > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > >
auto_generate_security_keys_v3.patch
Description: Binary data
-- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
