changeset 478:3af7ca761f6a

Purge password reset role The risk of SQL-injections and thus privilege escalation via the metamorphic user was estimated not high enough to justify the extra role. Thus, bring database back in line with rev. ffdb507d5b42 and re-enable password reset.
author Tom Gottfried <tom@intevation.de>
date Thu, 23 Aug 2018 16:41:44 +0200
parents 00b52d653039
children fce16726f51d
files example_conf.toml pkg/controllers/pwreset.go schema/Dockerfile schema/auth.sql schema/gemma.sql schema/install-db.sh schema/manage_users.sql schema/manage_users_tests.sql schema/roles.sql schema/run_tests.sh schema/std_login_roles.sql schema/tap_tests_data.sql
diffstat 12 files changed, 26 insertions(+), 96 deletions(-) [+]
line wrap: on
line diff
--- a/example_conf.toml	Thu Aug 23 16:18:07 2018 +0200
+++ b/example_conf.toml	Thu Aug 23 16:41:44 2018 +0200
@@ -1,6 +1,4 @@
 dbhost = "gemma_db"
-service-user = "gemma_service"
-service-password = "pw2Reset4"
 host = "0.0.0.0"
 sessions = "/tmp/gemma_session.data"
 web = "./web"
--- a/pkg/controllers/pwreset.go	Thu Aug 23 16:18:07 2018 +0200
+++ b/pkg/controllers/pwreset.go	Thu Aug 23 16:41:44 2018 +0200
@@ -20,29 +20,29 @@
 )
 
 const (
-	insertRequestSQL = `INSERT INTO pw_reset.password_reset_requests
+	insertRequestSQL = `INSERT INTO sys_admin.password_reset_requests
     (hash, username) VALUES ($1, $2)`
 
-	countRequestsSQL = `SELECT count(*) FROM pw_reset.password_reset_requests`
+	countRequestsSQL = `SELECT count(*) FROM sys_admin.password_reset_requests`
 
-	countRequestsUserSQL = `SELECT count(*) FROM pw_reset.password_reset_requests
+	countRequestsUserSQL = `SELECT count(*) FROM sys_admin.password_reset_requests
     WHERE username = $1`
 
-	deleteRequestSQL = `DELETE FROM pw_reset.password_reset_requests
+	deleteRequestSQL = `DELETE FROM sys_admin.password_reset_requests
     WHERE hash = $1`
 
 	findRequestSQL = `SELECT lu.email_address, lu.username
-    FROM pw_reset.password_reset_requests prr
-    JOIN pw_reset.list_users lu on prr.username = lu.username
+    FROM sys_admin.password_reset_requests prr
+    JOIN users.list_users lu on prr.username = lu.username
     WHERE prr.hash = $1`
 
-	cleanupRequestsSQL = `DELETE FROM pw_reset.password_reset_requests
+	cleanupRequestsSQL = `DELETE FROM sys_admin.password_reset_requests
     WHERE issued < $1`
 
 	userExistsSQL = `SELECT email_address
-    FROM pw_reset.list_users WHERE username = $1`
+    FROM users.list_users WHERE username = $1`
 
-	updatePasswordSQL = `UPDATE pw_reset.list_users
+	updatePasswordSQL = `UPDATE users.list_users
     SET pw = $1 WHERE username = $2`
 )
 
@@ -55,7 +55,7 @@
 	cleanupPause               = 15 * time.Minute
 )
 
-const pwResetRole = "pw_reset"
+const pwResetRole = "sys_admin"
 
 var (
 	passwordResetRequestMailTmpl = template.Must(
--- a/schema/Dockerfile	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/Dockerfile	Thu Aug 23 16:41:44 2018 +0200
@@ -31,7 +31,7 @@
 COPY *.sql *.sh ./
 COPY demo-data ./demo-data/
 RUN $PGBIN/pg_ctl start -wo "--config_file=$PGCONF" && \
-    ./install-db.sh --demo --servicepw "pw2Reset4" --metapw "geo2Serv" && \
+    ./install-db.sh --demo --metapw "geo2Serv" && \
     $PGBIN/pg_ctl stop -m smart
 
 # Set the default command to run when starting the container
--- a/schema/auth.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/auth.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -31,15 +31,7 @@
 GRANT USAGE ON SCHEMA sys_admin TO sys_admin;
 GRANT SELECT ON ALL TABLES IN SCHEMA sys_admin TO sys_admin;
 GRANT UPDATE ON sys_admin.system_config TO sys_admin;
-
-
---
--- Special privileges for pw_reset
---
-GRANT USAGE ON SCHEMA pw_reset TO pw_reset;
-GRANT SELECT (username, email_address) ON pw_reset.list_users TO pw_reset;
-GRANT UPDATE (pw) ON pw_reset.list_users TO pw_reset;
-GRANT INSERT, SELECT, DELETE ON pw_reset.password_reset_requests TO pw_reset;
+GRANT INSERT, DELETE ON sys_admin.password_reset_requests TO sys_admin;
 
 --
 -- Privileges assigned directly to metamorph
--- a/schema/gemma.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/gemma.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -56,6 +56,14 @@
         config_val varchar
     )
 
+    CREATE TABLE password_reset_requests (
+        hash varchar(32) PRIMARY KEY,
+        issued timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
+        username varchar NOT NULL
+            REFERENCES internal.user_profiles(username)
+                ON DELETE CASCADE ON UPDATE CASCADE
+    )
+
     CREATE TABLE external_services (
         local_name varchar PRIMARY KEY,
         remote_url varchar NOT NULL,
@@ -182,18 +190,6 @@
     country char(2) NOT NULL REFERENCES users.responsibility_areas;
 
 
--- Namespace intended to be the only one that pw_reset can access
-CREATE SCHEMA pw_reset
-    CREATE TABLE password_reset_requests (
-        hash varchar(32) PRIMARY KEY,
-        issued timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
-        username varchar NOT NULL
-            REFERENCES internal.user_profiles(username)
-                ON DELETE CASCADE ON UPDATE CASCADE
-    )
-;
-
-
 -- Namespace for waterway data that can change in a running system
 CREATE SCHEMA waterway
 
--- a/schema/install-db.sh	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/install-db.sh	Thu Aug 23 16:41:44 2018 +0200
@@ -17,8 +17,6 @@
   -D, --demo       also install demo accounts and data
       --adminpw    set the password to use for the "sysadmin" account.
                    Default is a random password.
-      --servicepw  set the password to use for the "gemma_service" account.
-                   Default is a random password.
       --metapw     set the password to use for the "meta_login" account.
                    Default is a random password.
       --drop       drop database and all roles
@@ -47,13 +45,12 @@
 demo=0
 drop=0
 adminpw=`genpw 15`
-servicepw=`genpw 15`
 metapw=`genpw 15`
 
 # Parse options:
 
 OPTS=`getopt \
-      -l help,demo,db:,port:,drop,adminpw:,servicepw:,metapw: \
+      -l help,demo,db:,port:,drop,adminpw:,metapw: \
       -o Dd:p: -n "$ME" -- "$@"`
 [ $? -eq 0 ] || { usage ; exit 1 ; }
 
@@ -73,10 +70,6 @@
       adminpw="$2"
       shift 2
       ;;
-    --servicepw)
-      servicepw="$2"
-      shift 2
-      ;;
     --metapw)
       metapw="$2"
       shift 2
@@ -114,7 +107,7 @@
 
   # setup initial login roles with given passwords:
   psql -qt -p "$port" -d "$db" \
-       -v adminpw="$adminpw" -v servicepw="$servicepw" -v metapw="$metapw" \
+       -v adminpw="$adminpw" -v metapw="$metapw" \
        -f "$BASEDIR/std_login_roles.sql"
 
   if [[ $demo -eq 1 ]] ; then
@@ -133,7 +126,7 @@
   if [[ $a == "yes" ]] ; then
     dropdb -p "$port" "$db"
     psql -p $port -A -t -c '\du' | awk -F '|' -v port=$port \
-        '$1 "." $3 ~ /waterway_user|waterway_admin|sys_admin|pw_reset|metamorph/ \
+        '$1 "." $3 ~ /waterway_user|waterway_admin|sys_admin|metamorph/ \
 	    { system("dropuser -p " port " \"" $1 "\"") }'
   else
     echo "No harm done."
--- a/schema/manage_users.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/manage_users.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -41,7 +41,6 @@
                 AND p.country = (
                     SELECT country FROM internal.user_profiles
                         WHERE username = current_user)
-            OR pg_has_role('pw_reset', 'MEMBER')
             OR pg_has_role('sys_admin', 'MEMBER');
 
 
@@ -101,14 +100,13 @@
     EXECUTE PROCEDURE internal.update_metamorph();
 
 
--- Prevent roles other than sys_admin and pw_reset to update any user but
+-- Prevent roles other than sys_admin to update any user but
 -- themselves (affects waterway_admin)
 CREATE OR REPLACE FUNCTION internal.authorize_update_user() RETURNS trigger
 AS $$
 BEGIN
     IF OLD.username <> current_user
-        AND NOT (pg_has_role('sys_admin', 'MEMBER')
-            OR pg_has_role('pw_reset', 'MEMBER'))
+        AND NOT pg_has_role('sys_admin', 'MEMBER')
     THEN
         RETURN NULL;
     ELSE
@@ -202,9 +200,6 @@
     EXECUTE PROCEDURE internal.delete_user();
 
 
-CREATE OR REPLACE VIEW pw_reset.list_users AS
-    SELECT username, pw, email_address FROM users.list_users;
-
 -- To set a role from a hex-encoded user name (which is save from SQL injections).
 CREATE OR REPLACE FUNCTION public.setrole(role text) RETURNS void
 AS $$
--- a/schema/manage_users_tests.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/manage_users_tests.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -314,38 +314,3 @@
     $$,
     55006, NULL,
     'Current user cannot be deleted');
-
-
---
--- Password reset
---
-
--- Workaround broken relocatability of pgtap (otherwise we could
--- put pgtap in its own schema and GRANT USAGE to PUBLIC on it)
-RESET SESSION AUTHORIZATION;
-GRANT USAGE ON SCHEMA public TO pw_reset;
-
-SET SESSION AUTHORIZATION test_pw_reset;
-
-SELECT isnt_empty($$
-    SELECT username, email_address FROM pw_reset.list_users
-    $$,
-    'Special role can see users with their email addresses');
-
-SELECT results_eq($$
-    UPDATE pw_reset.list_users
-        SET pw = 'user_at2!' WHERE username = 'test_user_at'
-        RETURNING email_address
-    $$,
-    $$
-    SELECT email_address FROM pw_reset.list_users
-        WHERE username = 'test_user_at'
-    $$,
-    'Special role can update password');
-
-SELECT throws_ok($$
-    UPDATE pw_reset.list_users
-        SET username = 'test_rename', email_address = 'test'
-    $$,
-    42501, NULL,
-    'Special role cannot update arbitrary user attributes');
--- a/schema/roles.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/roles.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -9,9 +9,6 @@
 -- Special roles
 --
 
--- A role that is intended to be used for password reset only
-CREATE ROLE pw_reset;
-
 -- A role that is intended to be used for backend- or
 -- GeoServer-connections on which SET ROLE has to be used to
 -- gain privileges of a specific role
--- a/schema/run_tests.sh	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/run_tests.sh	Thu Aug 23 16:41:44 2018 +0200
@@ -16,7 +16,7 @@
     -c 'SET client_min_messages TO WARNING' \
     -c "DROP ROLE IF EXISTS $TEST_ROLES" \
     -f tap_tests_data.sql \
-    -c 'SELECT plan(46)' \
+    -c 'SELECT plan(43)' \
     -f auth_tests.sql \
     -f manage_users_tests.sql \
     -c 'SELECT * FROM finish()'
--- a/schema/std_login_roles.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/std_login_roles.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -30,15 +30,11 @@
 -- Functional Users
 --
 
--- Used by the back end (gemma)
-CREATE ROLE gemma_service IN ROLE pw_reset LOGIN PASSWORD :'servicepw';
-
 -- Used by GeoServer and backend
 CREATE ROLE meta_login IN ROLE metamorph LOGIN PASSWORD :'metapw';
 
 -- Emit messages to the client if everything went ok
 SELECT 'Default admin user ''sysadmin'' created with password ' || :'adminpw';
-SELECT 'Backend user ''gemma_service'' created with password ' || :'servicepw';
 SELECT 'Backend user ''meta_login'' created with password ' || :'metapw';
 
 COMMIT;
--- a/schema/tap_tests_data.sql	Thu Aug 23 16:18:07 2018 +0200
+++ b/schema/tap_tests_data.sql	Thu Aug 23 16:41:44 2018 +0200
@@ -17,8 +17,6 @@
 INSERT INTO users.list_users VALUES (
     'sys_admin', 'test_sys_admin1', 'sys_admin1$', 'AT', NULL, 'zzz');
 
-CREATE ROLE test_pw_reset IN ROLE pw_reset LOGIN PASSWORD 'ppp';
-
 INSERT INTO limiting_factors VALUES ('depth'), ('width');
 
 INSERT INTO waterway.gauges (