diff options
author | Yorhel <git@yorhel.nl> | 2021-05-25 13:29:51 +0200 |
---|---|---|
committer | Yorhel <git@yorhel.nl> | 2021-05-25 13:30:16 +0200 |
commit | be5325f64d990e486d5fae1b6c4b4ee5a548aa1b (patch) | |
tree | 34c0e2c6e847532457150fa8e403b7b42821373e | |
parent | a3581db9cc7bc2f708de828e8ca216268137cac3 (diff) |
SQL: Separate sensitive columns out of the users table
This simplifies permissions management somewhat.
-rw-r--r-- | lib/VNWeb/Auth.pm | 5 | ||||
-rw-r--r-- | lib/VNWeb/Docs/Lib.pm | 2 | ||||
-rw-r--r-- | lib/VNWeb/User/Edit.pm | 2 | ||||
-rw-r--r-- | lib/VNWeb/User/Register.pm | 7 | ||||
-rw-r--r-- | sql/data.sql | 2 | ||||
-rw-r--r-- | sql/func.sql | 22 | ||||
-rw-r--r-- | sql/perms.sql | 28 | ||||
-rw-r--r-- | sql/schema.sql | 22 | ||||
-rw-r--r-- | sql/tableattrs.sql | 1 | ||||
-rwxr-xr-x | util/devdump.pl | 7 | ||||
-rw-r--r-- | util/updates/2021-05-25-users-shadow.sql | 19 |
11 files changed, 63 insertions, 54 deletions
diff --git a/lib/VNWeb/Auth.pm b/lib/VNWeb/Auth.pm index 08ec4dad..a6c4d852 100644 --- a/lib/VNWeb/Auth.pm +++ b/lib/VNWeb/Auth.pm @@ -146,8 +146,9 @@ sub _load_session { my $user = $uid ? tuwf->dbRowi( 'SELECT ', sql_user(), ',', sql_comma(@pref_columns, map "perm_$_", @perms), ' FROM users u - WHERE id = ', \$uid, - 'AND', sql_func(user_isvalidsession => 'id', sql_fromhex($token_db), \'web') + JOIN users_shadow us ON us.id = u.id + WHERE u.id = ', \$uid, + 'AND', sql_func(user_isvalidsession => 'u.id', sql_fromhex($token_db), \'web') ) : {}; # Drop the cookie if it's not valid diff --git a/lib/VNWeb/Docs/Lib.pm b/lib/VNWeb/Docs/Lib.pm index 2f2b273c..bea36241 100644 --- a/lib/VNWeb/Docs/Lib.pm +++ b/lib/VNWeb/Docs/Lib.pm @@ -11,7 +11,7 @@ my @special_perms = qw/boardmod dbmod usermod imgmod tagmod/; sub _moderators { my $cols = sql_comma map "perm_$_", @special_perms; my $where = sql_or map "perm_$_", @special_perms; - my $l = tuwf->dbAlli("SELECT id, username, $cols FROM users WHERE $where ORDER BY id LIMIT 100"); + state $l //= tuwf->dbAlli("SELECT u.id, username, $cols FROM users u JOIN users_shadow us ON us.id = u.id WHERE $where ORDER BY u.id LIMIT 100"); xml_string sub { dl_ sub { diff --git a/lib/VNWeb/User/Edit.pm b/lib/VNWeb/User/Edit.pm index 54bce482..7406b8af 100644 --- a/lib/VNWeb/User/Edit.pm +++ b/lib/VNWeb/User/Edit.pm @@ -89,7 +89,7 @@ TUWF::get qr{/$RE{uid}/edit}, sub { $u->{prefs}{skin} ||= config->{skin_default} if $u->{prefs}; $u->{admin} = auth->permDbmod || auth->permUsermod || auth->permTagmod || auth->permBoardmod || auth->permImgmod ? - tuwf->dbRowi('SELECT ign_votes, ', sql_comma(map "perm_$_", auth->listPerms), 'FROM users WHERE id =', \$u->{id}) : undef; + tuwf->dbRowi('SELECT ign_votes, ', sql_comma(map "perm_$_", auth->listPerms), 'FROM users u JOIN users_shadow us ON us.id = u.id WHERE u.id =', \$u->{id}) : undef; $u->{password} = undef; diff --git a/lib/VNWeb/User/Register.pm b/lib/VNWeb/User/Register.pm index 89e34846..5cd28924 100644 --- a/lib/VNWeb/User/Register.pm +++ b/lib/VNWeb/User/Register.pm @@ -29,11 +29,8 @@ elm_api UserRegister => undef, { $ip =~ /:/ ? \"$ip/48" : \"$ip/30" ); - my $id = tuwf->dbVali('INSERT INTO users', { - username => $data->{username}, - mail => $data->{email}, - ip => $ip, - }, 'RETURNING id'); + my $id = tuwf->dbVali('INSERT INTO users', {username => $data->{username}, ip => $ip}, 'RETURNING id'); + tuwf->dbExeci('INSERT INTO users_shadow', {id => $id, mail => $data->{email}}); my(undef, $token) = auth->resetpass($data->{email}); my $body = sprintf diff --git a/sql/data.sql b/sql/data.sql index 627de164..52056ae7 100644 --- a/sql/data.sql +++ b/sql/data.sql @@ -1,4 +1,4 @@ -INSERT INTO users (id, username, mail, notify_dbedit) VALUES ('u1', 'multi', 'multi@vndb.org', FALSE); +INSERT INTO users (id, username, notify_dbedit) VALUES ('u1', 'multi', FALSE); SELECT setval('users_id_seq', 2); INSERT INTO stats_cache (section, count) VALUES diff --git a/sql/func.sql b/sql/func.sql index 8152fe0e..3facbcdf 100644 --- a/sql/func.sql +++ b/sql/func.sql @@ -646,13 +646,13 @@ $$ LANGUAGE SQL; CREATE OR REPLACE FUNCTION user_getscryptargs(vndbid) RETURNS bytea AS $$ SELECT CASE WHEN length(passwd) = 46 THEN substring(passwd from 1 for 14) ELSE NULL END - FROM users WHERE id = $1 + FROM users_shadow WHERE id = $1 $$ LANGUAGE SQL SECURITY DEFINER; -- Create a new web session for this user (uid, scryptpass, token) CREATE OR REPLACE FUNCTION user_login(vndbid, bytea, bytea) RETURNS boolean AS $$ - INSERT INTO sessions (uid, token, expires, type) SELECT $1, $3, NOW() + '1 month', 'web' FROM users + INSERT INTO sessions (uid, token, expires, type) SELECT $1, $3, NOW() + '1 month', 'web' FROM users_shadow WHERE length($2) = 46 AND length($3) = 20 AND id = $1 AND passwd = $2 RETURNING true @@ -676,7 +676,7 @@ $$ LANGUAGE SQL SECURITY DEFINER; -- Used for duplicate email checks and user-by-email lookup for usermods. CREATE OR REPLACE FUNCTION user_emailtoid(text) RETURNS SETOF vndbid AS $$ - SELECT id FROM users WHERE lower(mail) = lower($1) + SELECT id FROM users_shadow WHERE lower(mail) = lower($1) $$ LANGUAGE SQL SECURITY DEFINER; @@ -688,7 +688,7 @@ $$ LANGUAGE SQL SECURITY DEFINER; -- to someone's account. But alas, that'd require a separate process. CREATE OR REPLACE FUNCTION user_resetpass(text, bytea) RETURNS vndbid AS $$ INSERT INTO sessions (uid, token, expires, type) - SELECT id, $2, NOW()+'1 week', 'pass' FROM users + SELECT id, $2, NOW()+'1 week', 'pass' FROM users_shadow WHERE lower(mail) = lower($1) AND length($2) = 20 AND NOT perm_usermod RETURNING uid $$ LANGUAGE SQL SECURITY DEFINER; @@ -697,7 +697,7 @@ $$ LANGUAGE SQL SECURITY DEFINER; -- Changes the user's password and invalidates all existing sessions. args: uid, old_pass_or_reset_token, new_pass CREATE OR REPLACE FUNCTION user_setpass(vndbid, bytea, bytea) RETURNS boolean AS $$ WITH upd(id) AS ( - UPDATE users SET passwd = $3 + UPDATE users_shadow SET passwd = $3 WHERE id = $1 AND length($3) = 46 AND ( (passwd = $2 AND length($2) = 46) @@ -714,7 +714,7 @@ $$ LANGUAGE SQL SECURITY DEFINER; -- Internal function, used to verify whether user ($2 with session $3) is -- allowed to access sensitive data from user $1. CREATE OR REPLACE FUNCTION user_isauth(vndbid, vndbid, bytea) RETURNS boolean AS $$ - SELECT true FROM users + SELECT true FROM users_shadow WHERE id = $2 AND EXISTS(SELECT 1 FROM sessions WHERE uid = $2 AND token = $3 AND type = 'web') AND ($2 IS NOT DISTINCT FROM $1 OR perm_usermod) @@ -724,7 +724,7 @@ $$ LANGUAGE SQL; -- uid of user email to get, uid currently logged in, session token of currently logged in. -- Ensures that only the user itself or a useradmin can get someone's email address. CREATE OR REPLACE FUNCTION user_getmail(vndbid, vndbid, bytea) RETURNS text AS $$ - SELECT mail FROM users WHERE id = $1 AND user_isauth($1, $2, $3) + SELECT mail FROM users_shadow WHERE id = $1 AND user_isauth($1, $2, $3) $$ LANGUAGE SQL SECURITY DEFINER; @@ -742,23 +742,23 @@ CREATE OR REPLACE FUNCTION user_setmail_confirm(vndbid, bytea) RETURNS boolean A WITH u(mail) AS ( DELETE FROM sessions WHERE uid = $1 AND token = $2 AND type = 'mail' AND expires > NOW() RETURNING mail ) - UPDATE users SET mail = (SELECT mail FROM u) WHERE id = $1 AND EXISTS(SELECT 1 FROM u) RETURNING true; + UPDATE users_shadow SET mail = (SELECT mail FROM u) WHERE id = $1 AND EXISTS(SELECT 1 FROM u) RETURNING true; $$ LANGUAGE SQL SECURITY DEFINER; CREATE OR REPLACE FUNCTION user_setperm_usermod(vndbid, vndbid, bytea, boolean) RETURNS void AS $$ - UPDATE users SET perm_usermod = $4 WHERE id = $1 AND user_isauth(NULL, $2, $3) + UPDATE users_shadow SET perm_usermod = $4 WHERE id = $1 AND user_isauth(NULL, $2, $3) $$ LANGUAGE SQL SECURITY DEFINER; CREATE OR REPLACE FUNCTION user_admin_setpass(vndbid, vndbid, bytea, bytea) RETURNS void AS $$ WITH upd(id) AS ( - UPDATE users SET passwd = $4 WHERE id = $1 AND user_isauth(NULL, $2, $3) AND length($4) = 46 RETURNING id + UPDATE users_shadow SET passwd = $4 WHERE id = $1 AND user_isauth(NULL, $2, $3) AND length($4) = 46 RETURNING id ) DELETE FROM sessions WHERE uid IN(SELECT id FROM upd) $$ LANGUAGE SQL SECURITY DEFINER; CREATE OR REPLACE FUNCTION user_admin_setmail(vndbid, vndbid, bytea, text) RETURNS void AS $$ - UPDATE users SET mail = $4 WHERE id = $1 AND user_isauth(NULL, $2, $3) + UPDATE users_shadow SET mail = $4 WHERE id = $1 AND user_isauth(NULL, $2, $3) $$ LANGUAGE SQL SECURITY DEFINER; diff --git a/sql/perms.sql b/sql/perms.sql index 5ed58d40..f7572b5a 100644 --- a/sql/perms.sql +++ b/sql/perms.sql @@ -75,25 +75,8 @@ GRANT SELECT, INSERT ON traits_parents_hist TO vndb_site; GRANT SELECT, INSERT, UPDATE, DELETE ON ulist_labels TO vndb_site; GRANT SELECT, INSERT, UPDATE, DELETE ON ulist_vns TO vndb_site; GRANT SELECT, INSERT, UPDATE, DELETE ON ulist_vns_labels TO vndb_site; - --- users table is special; The 'perm_usermod', 'passwd' and 'mail' columns are --- protected and can only be accessed through the user_* functions. -GRANT SELECT ( id, username, registered, ip, ign_votes, email_confirmed, last_reports - , perm_board, perm_boardmod, perm_dbmod, perm_edit, perm_imgvote, perm_tag, perm_tagmod, perm_usermod, perm_imgmod, perm_review - , skin, customcss, notify_dbedit, notify_announce, notify_post, notify_comment - , tags_all, tags_cont, tags_ero, tags_tech, spoilers, traits_sexual, max_sexual, max_violence - , nodistract_can, nodistract_noads, nodistract_nofancy, support_can, support_enabled, uniname_can, uniname, pubskin_can, pubskin_enabled - , ulist_votes, ulist_vnlist, ulist_wish, tableopts_c - , c_vns, c_wish, c_votes, c_changes, c_imgvotes, c_tags), - INSERT ( username, mail, ip), - UPDATE ( username, ign_votes, email_confirmed, last_reports - , perm_board, perm_boardmod, perm_dbmod, perm_edit, perm_imgvote, perm_tag, perm_tagmod, perm_imgmod, perm_review - , skin, customcss, notify_dbedit, notify_announce, notify_post, notify_comment - , tags_all, tags_cont, tags_ero, tags_tech, spoilers, traits_sexual, max_sexual, max_violence - , nodistract_can, nodistract_noads, nodistract_nofancy, support_can, support_enabled, uniname_can, uniname, pubskin_can, pubskin_enabled - , ulist_votes, ulist_vnlist, ulist_wish, tableopts_c - , c_vns, c_wish, c_votes, c_changes, c_imgvotes, c_tags) ON users TO vndb_site; - +GRANT SELECT, INSERT, UPDATE ON users TO vndb_site; +GRANT SELECT (id, perm_usermod), INSERT (id, mail) ON users_shadow TO vndb_site; GRANT SELECT, INSERT, UPDATE ON vn TO vndb_site; GRANT SELECT, INSERT, DELETE ON vn_anime TO vndb_site; GRANT SELECT, INSERT ON vn_anime_hist TO vndb_site; @@ -175,11 +158,8 @@ GRANT SELECT ON traits_parents TO vndb_multi; GRANT SELECT, INSERT, UPDATE, DELETE ON ulist_labels TO vndb_multi; GRANT SELECT, INSERT, UPDATE, DELETE ON ulist_vns TO vndb_multi; GRANT SELECT, INSERT, UPDATE, DELETE ON ulist_vns_labels TO vndb_multi; - -GRANT SELECT (id, username, registered, ign_votes, email_confirmed, notify_dbedit, notify_announce, notify_post, notify_comment, c_vns, c_wish, c_votes, c_changes, c_imgvotes, c_tags, perm_imgvote, perm_imgmod, perm_tag), - UPDATE ( c_vns, c_wish, c_votes, c_changes, c_imgvotes, c_tags ) ON users TO vndb_multi; -GRANT DELETE ON users TO vndb_multi; - +GRANT SELECT, UPDATE, DELETE ON users TO vndb_multi; +GRANT SELECT (id), DELETE ON users_shadow TO vndb_multi; GRANT SELECT, UPDATE ON vn TO vndb_multi; GRANT SELECT ON vn_anime TO vndb_multi; GRANT SELECT ON vn_hist TO vndb_multi; diff --git a/sql/schema.sql b/sql/schema.sql index cceeea9e..b76f1c14 100644 --- a/sql/schema.sql +++ b/sql/schema.sql @@ -937,7 +937,7 @@ CREATE TABLE ulist_vns_labels ( CREATE TABLE users ( registered timestamptz NOT NULL DEFAULT NOW(), last_reports timestamptz, -- For mods: Most recent activity seen on the reports listing - id vndbid NOT NULL PRIMARY KEY DEFAULT vndbid('u', nextval('users_id_seq')::int) CONSTRAINT users_id_check CHECK(vndbid_type(id) = 'u'), -- [pub] + id vndbid NOT NULL PRIMARY KEY DEFAULT vndbid('u', nextval('users_id_seq')::int) CONSTRAINT users_id_check CHECK(vndbid_type(id) = 'u'), -- [pub] c_votes integer NOT NULL DEFAULT 0, c_changes integer NOT NULL DEFAULT 0, c_tags integer NOT NULL DEFAULT 0, @@ -974,15 +974,26 @@ CREATE TABLE users ( perm_imgvote boolean NOT NULL DEFAULT true, -- [pub] (public because this is used in calculating image flagging scores) perm_tag boolean NOT NULL DEFAULT true, -- [pub] (public because this is used in calculating VN tag scores) perm_tagmod boolean NOT NULL DEFAULT false, - perm_usermod boolean NOT NULL DEFAULT false, perm_imgmod boolean NOT NULL DEFAULT false, perm_review boolean NOT NULL DEFAULT true, username varchar(20) NOT NULL UNIQUE, -- [pub] uniname text NOT NULL DEFAULT '', - mail varchar(100) NOT NULL, ip inet NOT NULL DEFAULT '0.0.0.0', skin text NOT NULL DEFAULT '', customcss text NOT NULL DEFAULT '', + ulist_votes jsonb, + ulist_vnlist jsonb, + ulist_wish jsonb +); + +-- Additional fields for the 'users' table, but with some protected columns. +-- (Separated from the users table to simplify permission management) +CREATE TABLE users_shadow ( + id vndbid NOT NULL PRIMARY KEY, + -- Usermods can see other users' mail and edit their passwords, so this + -- permission is separated in this table to prevent unauthorized writes. + perm_usermod boolean NOT NULL DEFAULT false, + mail varchar(100) NOT NULL, -- A valid passwd column is 46 bytes: -- 4 bytes: N (big endian) -- 1 byte: r @@ -990,10 +1001,7 @@ CREATE TABLE users ( -- 8 bytes: salt -- 32 bytes: scrypt(passwd, global_salt + salt, N, r, p, 32) -- Anything else is invalid, account disabled. - passwd bytea NOT NULL DEFAULT '', - ulist_votes jsonb, - ulist_vnlist jsonb, - ulist_wish jsonb + passwd bytea NOT NULL DEFAULT '' ); -- vn diff --git a/sql/tableattrs.sql b/sql/tableattrs.sql index d561c77a..9cb4f061 100644 --- a/sql/tableattrs.sql +++ b/sql/tableattrs.sql @@ -86,6 +86,7 @@ ALTER TABLE ulist_vns_labels ADD CONSTRAINT ulist_vns_labels_uid_fkey ALTER TABLE ulist_vns_labels ADD CONSTRAINT ulist_vns_labels_vid_fkey FOREIGN KEY (vid) REFERENCES vn (id); ALTER TABLE ulist_vns_labels ADD CONSTRAINT ulist_vns_labels_uid_lbl_fkey FOREIGN KEY (uid,lbl) REFERENCES ulist_labels (uid,id) ON DELETE CASCADE; ALTER TABLE ulist_vns_labels ADD CONSTRAINT ulist_vns_labels_uid_vid_fkey FOREIGN KEY (uid,vid) REFERENCES ulist_vns (uid,vid) ON DELETE CASCADE; +ALTER TABLE users_shadow ADD CONSTRAINT users_shadow_id_fkey FOREIGN KEY (id) REFERENCES users (id) ON DELETE CASCADE; ALTER TABLE vn ADD CONSTRAINT vn_image_fkey FOREIGN KEY (image) REFERENCES images (id); ALTER TABLE vn ADD CONSTRAINT vn_l_wikidata_fkey FOREIGN KEY (l_wikidata)REFERENCES wikidata (id); ALTER TABLE vn_hist ADD CONSTRAINT vn_hist_chid_fkey FOREIGN KEY (chid) REFERENCES changes (id) ON DELETE CASCADE; diff --git a/util/devdump.pl b/util/devdump.pl index e90ac335..02688f8d 100755 --- a/util/devdump.pl +++ b/util/devdump.pl @@ -110,7 +110,7 @@ sub copy_entry { # A few pre-defined users # This password is 'hunter2' with the default salt my $pass = '000100000801ec4185fed438752d6b3b968e2b2cd045f70005cb7e10cafdbb694a82246bd34a065b6e977e0c3dcc'; - printf "INSERT INTO users (id, username, mail, perm_usermod, passwd, email_confirmed) VALUES ('%s', '%s', '%s', %s, decode('%s', 'hex'), true);\n", @$_, $pass for( + for( [ 'u2', 'admin', 'admin@vndb.org', 'true' ], [ 'u3', 'user1', 'user1@vndb.org', 'false'], [ 'u4', 'user2', 'user2@vndb.org', 'false'], @@ -119,7 +119,10 @@ sub copy_entry { [ 'u7', 'user5', 'user5@vndb.org', 'false'], [ 'u8', 'user6', 'user6@vndb.org', 'false'], [ 'u9', 'user7', 'user7@vndb.org', 'false'], - ); + ) { + printf "INSERT INTO users (id, username, email_confirmed) VALUES ('%s', '%s', true);\n", @{$_}[0,1]; + printf "INSERT INTO users_shadow (id, mail, perm_usermod, passwd) VALUES ('%s', '%s', %s, decode('%s', 'hex'));\n", @{$_}[0,2,3], $pass; + } print "SELECT ulist_labels_create(id) FROM users;\n"; # Tags & traits diff --git a/util/updates/2021-05-25-users-shadow.sql b/util/updates/2021-05-25-users-shadow.sql new file mode 100644 index 00000000..bf48d0af --- /dev/null +++ b/util/updates/2021-05-25-users-shadow.sql @@ -0,0 +1,19 @@ +CREATE TABLE users_shadow ( + id vndbid NOT NULL PRIMARY KEY, + perm_usermod boolean NOT NULL DEFAULT false, + mail varchar(100) NOT NULL, + passwd bytea NOT NULL DEFAULT '' +); + +BEGIN; +INSERT INTO users_shadow SELECT id, perm_usermod, mail, passwd FROM users; + +ALTER TABLE users_shadow ADD CONSTRAINT users_shadow_id_fkey FOREIGN KEY (id) REFERENCES users (id) ON DELETE CASCADE; + +ALTER TABLE users DROP COLUMN perm_usermod; +ALTER TABLE users DROP COLUMN mail; +ALTER TABLE users DROP COLUMN passwd; +COMMIT; + +\i sql/perms.sql +\i sql/func.sql |