Hi
On Friday, October 14, 2016, Dave Page <dp...@pgadmin.org> wrote:
> Hi
>
> On Thursday, October 13, 2016, Ashesh Vashi <ashesh.va...@enterprisedb.com
> <javascript:_e(%7B%7D,'cvml','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.
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
diff --git a/web/config.py b/web/config.py
index 20714f9..8411d79 100644
--- a/web/config.py
+++ b/web/config.py
@@ -140,21 +140,13 @@ DEFAULT_SERVER_PORT = 5050
# Enable CSRF protection?
CSRF_ENABLED = True
-# Secret key for signing CSRF data. Override this in config_local.py if
-# running on a web server
-CSRF_SESSION_KEY = 'SuperSecret1'
-
-# Secret key for signing cookies. Override this in config_local.py if
-# running on a web server
-SECRET_KEY = 'SuperSecret2'
-
-# Salt used when hashing passwords. Override this in config_local.py if
-# running on a web server
-SECURITY_PASSWORD_SALT = 'SuperSecret3'
-
# Hashing algorithm used for password storage
SECURITY_PASSWORD_HASH = 'pbkdf2_sha512'
+# NOTE: CSRF_SESSION_KEY, SECRET_KEY and SECURITY_PASSWORD_SALT are no
+# longer part of the main configuration, but are stored in the
+# configuration databases 'keys' table and are auto-generated.
+
# Should HTML be minified on the fly when not in debug mode?
# Note: This is disabled by default as it will error when processing the
# docs. If the serving of docs is handled by an Apache HTTPD
diff --git a/web/pgAdmin4.py b/web/pgAdmin4.py
index 1fb34f9..f894f8b 100644
--- a/web/pgAdmin4.py
+++ b/web/pgAdmin4.py
@@ -32,18 +32,6 @@ config.SETTINGS_SCHEMA_VERSION = SCHEMA_VERSION
# Sanity checks
##########################################################################
-# Check for local settings if running in server mode
-if config.SERVER_MODE is True:
- local_config = os.path.join(os.path.dirname(os.path.realpath(__file__)),
- 'config_local.py')
- if not os.path.isfile(local_config):
- print("The configuration file %s does not exist.\n" % local_config)
- print("Before running this application, ensure that config_local.py has been created")
- print("and sets values for SECRET_KEY, SECURITY_PASSWORD_SALT and CSRF_SESSION_KEY")
- print("at bare minimum. See config.py for more information and a complete list of")
- print("settings. Exiting...")
- sys.exit(1)
-
# Check if the database exists. If it does not, create it.
if not os.path.isfile(config.SQLITE_PATH):
setupfile = os.path.join(os.path.dirname(os.path.realpath(__file__)),
diff --git a/web/pgadmin/__init__.py b/web/pgadmin/__init__.py
index d988172..79fa1c6 100644
--- a/web/pgadmin/__init__.py
+++ b/web/pgadmin/__init__.py
@@ -26,7 +26,7 @@ from pgadmin.utils.session import create_session_interface
from werkzeug.local import LocalProxy
from werkzeug.utils import find_modules
-from pgadmin.model import db, Role, Server, ServerGroup, User, Version
+from pgadmin.model import db, Role, Server, ServerGroup, User, Version, Keys
# Configuration settings
import config
@@ -127,11 +127,6 @@ def create_app(app_name=config.APP_NAME):
app.config.update(dict(PROPAGATE_EXCEPTIONS=True))
##########################################################################
- # Setup session management
- ##########################################################################
- app.session_interface = create_session_interface(app)
-
- ##########################################################################
# Setup logging and log the application startup
##########################################################################
@@ -206,7 +201,7 @@ def create_app(app_name=config.APP_NAME):
# Setup Flask-Security
user_datastore = SQLAlchemyUserDatastore(db, User, Role)
- security = Security(app, user_datastore)
+ security = Security(None, user_datastore)
# Upgrade the schema (if required)
with app.app_context():
@@ -220,9 +215,29 @@ def create_app(app_name=config.APP_NAME):
)
)
from setup import do_upgrade
- do_upgrade(app, user_datastore, security, version)
+ do_upgrade(app, user_datastore, version)
+
+ ##########################################################################
+ # Setup security
+ ##########################################################################
+ with app.app_context():
+ config.CSRF_SESSION_KEY = Keys.query.filter_by(name = 'CSRF_SESSION_KEY').first().value
+ config.SECRET_KEY = Keys.query.filter_by(name = 'SECRET_KEY').first().value
+ config.SECURITY_PASSWORD_SALT = Keys.query.filter_by(name = 'SECURITY_PASSWORD_SALT').first().value
+
+ # Update the app.config with proper security keyes for signing CSRF data,
+ # signing cookies, and the SALT for hashing the passwords.
+ app.config.update(dict(CSRF_SESSION_KEY=config.CSRF_SESSION_KEY))
+ app.config.update(dict(SECRET_KEY=config.SECRET_KEY))
+ app.config.update(dict(SECURITY_PASSWORD_SALT=config.SECURITY_PASSWORD_SALT))
+ security.init_app(app)
+
+ app.session_interface = create_session_interface(app)
+
+ ##########################################################################
# Load all available server drivers
+ ##########################################################################
driver.init_app(app)
##########################################################################
diff --git a/web/pgadmin/model/__init__.py b/web/pgadmin/model/__init__.py
index 019e9b1..9727d2b 100644
--- a/web/pgadmin/model/__init__.py
+++ b/web/pgadmin/model/__init__.py
@@ -29,7 +29,7 @@ from flask_sqlalchemy import SQLAlchemy
#
##########################################################################
-SCHEMA_VERSION = 13
+SCHEMA_VERSION = 14
##########################################################################
#
@@ -207,3 +207,10 @@ class Process(db.Model):
end_time = db.Column(db.String(), nullable=True)
exit_code = db.Column(db.Integer(), nullable=True)
acknowledge = db.Column(db.String(), nullable=True)
+
+
+class Keys(db.Model):
+ """Define the keys table."""
+ __tablename__ = 'keys'
+ name = db.Column(db.String(), nullable=False, primary_key=True)
+ value = db.Column(db.String(), nullable=False)
\ No newline at end of file
diff --git a/web/setup.py b/web/setup.py
index 64273fb..441a0f3 100755
--- a/web/setup.py
+++ b/web/setup.py
@@ -10,6 +10,7 @@
"""Perform the initial setup of the application, by creating the auth
and settings database."""
+import base64
import getpass
import os
import random
@@ -22,7 +23,7 @@ from flask_security import Security, SQLAlchemyUserDatastore
from flask_security.utils import encrypt_password
from pgadmin.model import db, Role, User, Server, \
- ServerGroup, Version
+ ServerGroup, Version, Keys
# Configuration settings
import config
@@ -40,6 +41,7 @@ if hasattr(__builtins__, 'raw_input'):
def do_setup(app):
"""Create a new settings database from scratch"""
+
if config.SERVER_MODE is False:
print("NOTE: Configuring authentication for DESKTOP mode.")
email = config.DESKTOP_USER
@@ -116,6 +118,17 @@ def do_setup(app):
name='ConfigDB', value=config.SETTINGS_SCHEMA_VERSION
)
db.session.merge(version)
+ db.session.commit()
+
+ # Create the keys
+ key = Keys(name='CSRF_SESSION_KEY', value=config.CSRF_SESSION_KEY)
+ db.session.merge(key)
+
+ key = Keys(name='SECRET_KEY', value=config.SECRET_KEY)
+ db.session.merge(key)
+
+ key = Keys(name='SECURITY_PASSWORD_SALT', value=config.SECURITY_PASSWORD_SALT)
+ db.session.merge(key)
db.session.commit()
@@ -128,7 +141,7 @@ def do_setup(app):
)
-def do_upgrade(app, datastore, security, version):
+def do_upgrade(app, datastore, version):
"""Upgrade an existing settings database"""
#######################################################################
# Run whatever is required to update the database schema to the current
@@ -329,6 +342,29 @@ ALTER TABLE SERVER
ADD COLUMN discovery_id TEXT
""")
+ if int(version.value) < 14:
+ db.engine.execute("""
+CREATE TABLE keys (
+ name TEST NOT NULL,
+ value TEXT NOT NULL,
+ PRIMARY KEY (name))
+ """)
+
+ sql = "INSERT INTO keys (name, value) VALUES ('CSRF_SESSION_KEY', '%s')" % base64.urlsafe_b64encode(os.urandom(32))
+ db.engine.execute(sql)
+
+ sql = "INSERT INTO keys (name, value) VALUES ('SECRET_KEY', '%s')" % base64.urlsafe_b64encode(os.urandom(32))
+ db.engine.execute(sql)
+
+ # If SECURITY_PASSWORD_SALT is not in the config, but we're upgrading, then it must (unless the
+ # user edited the main config - which they shouldn't have done) have been at it's default
+ # value, so we'll use that. Otherwise, use whatever we can find in the config.
+ if hasattr(config, 'SECURITY_PASSWORD_SALT'):
+ sql = "INSERT INTO keys (name, value) VALUES ('SECURITY_PASSWORD_SALT', '%s')" % config.SECURITY_PASSWORD_SALT
+ else:
+ sql = "INSERT INTO keys (name, value) VALUES ('SECURITY_PASSWORD_SALT', 'SuperSecret3')"
+ db.engine.execute(sql)
+
# Finally, update the schema version
version.value = config.SETTINGS_SCHEMA_VERSION
db.session.merge(version)
@@ -347,6 +383,12 @@ ALTER TABLE SERVER
###############################################################################
if __name__ == '__main__':
app = Flask(__name__)
+
+ # Get some defaults for the various keys
+ config.CSRF_SESSION_KEY = base64.urlsafe_b64encode(os.urandom(32))
+ config.SECRET_KEY = base64.urlsafe_b64encode(os.urandom(32))
+ config.SECURITY_PASSWORD_SALT = base64.urlsafe_b64encode(os.urandom(32))
+
app.config.from_object(config)
if config.TESTING_MODE:
@@ -364,15 +406,6 @@ if __name__ == '__main__':
'config_local.py'
)
- if not os.path.isfile(local_config):
- print("""
- The configuration file - {0} does not exist.
- Before running this application, ensure that config_local.py has been created
- and sets values for SECRET_KEY, SECURITY_PASSWORD_SALT and CSRF_SESSION_KEY
- at bare minimum. See config.py for more information and a complete list of
- settings. Exiting...""".format(local_config))
- sys.exit(1)
-
# Check if the database exists. If it does, tell the user and exit.
if os.path.isfile(config.SQLITE_PATH):
print("""
@@ -381,7 +414,6 @@ Entering upgrade mode...""" % config.SQLITE_PATH)
# Setup Flask-Security
user_datastore = SQLAlchemyUserDatastore(db, User, Role)
- security = Security(app, user_datastore)
# Always use "< REQUIRED_VERSION" as the test for readability
with app.app_context():
@@ -403,7 +435,7 @@ Exiting...""" % (version.value))
print("NOTE: Upgrading database schema from version %d to %d." % (
version.value, config.SETTINGS_SCHEMA_VERSION
))
- do_upgrade(app, user_datastore, security, version)
+ do_upgrade(app, user_datastore, version)
else:
directory = os.path.dirname(config.SQLITE_PATH)
if not os.path.exists(directory):
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
