8000 Delete function is_role_valid(), and revert changes to get_role_passw… · gurjeet/postgres@4ba609d · GitHub
[go: up one dir, main page]
Skip to content

Commit 4ba609d

Browse files
gurjeetgurjeet
committed
Delete function is_role_valid(), and revert changes to get_role_passwords()
Plus some other minor changes to code, comments, and docs.
1 parent 5e5ccb4 commit 4ba609d

File tree

8 files changed

+57
-71
lines changed

8 files changed

+57
-71
lines changed

‎README.multiple_passwords

Copy file name to clipboard
Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,30 @@
1-
Allow Users to have multiple passwords
1+
Allow Users to have Multiple Passwords
22
======================================
33

44
This patchset allows the users to have multiple passwords. This is useful for
55
cases when users are rolling over passwords, such that users/applications can
66
start using new password before the old one expires.
77

8+
Notes
9+
-----
10+
11+
is_role_valid() (backend/commands/user.c) is a new function that pulls out a
12+
small section of code from get_role_passwords() (backend/libpq.c). I don't think
13+
moving this code block to a new function gains us anything; in fact, it now
14+
forces us to call the new function in two new locations, which we didn't have to
15+
do before. It has to throw the same error messages as before, to maintain
16+
compatibility with external tools/libraries, hence it duplicates those messages
17+
as well, which is not ideal.
18+
19+
Moreover, before the patch, in case of CheckPasswordAuth(), the error (if any)
20+
would have been thrown _after_ network communication done by sendAuthRequest()
21+
call. But after the patch, the error is thrown before the network interaction,
22+
hence this changes the order of network interaction and the error message. This
23+
may have security implications, too, but I'm unable to articulate one right now.
24+
25+
So I removed the is_role_valid() function, and reverted the code removal from
26+
get_role_passwords().
27+
828
Patchset
929
--------
1030
- 0001: Move passwords to a new catalog

‎doc/src/sgml/client-auth.sgml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1281,7 +1281,7 @@ omicron bryanh guest1
12811281
<para>
12821282
<productname>PostgreSQL</productname> database passwords are
12831283
separate from operating system user passwords. The password for
1284-
each database user is stored in the <literal>pg_authid</literal> system
1284+
each database user is stored in the <literal>pg_passwords</literal> system
12851285
catalog. Passwords can be managed with the SQL commands
12861286
<xref linkend="sql-createrole"/> and
12871287
<xref linkend="sql-alterrole"/>,

‎src/backend/catalog/Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ CATALOG_HEADERS := \
9090
pg_tablespace.h \
9191
pg_authid.h \
9292
pg_auth_members.h \
93-
pg_auth_password.h \
9493
pg_shdepend.h \
9594
pg_shdescription.h \
9695
pg_ts_config.h \
@@ -119,7 +118,8 @@ CATALOG_HEADERS := \
119118
pg_publication_namespace.h \
120119
pg_publication_rel.h \
121120
pg_subscription.h \
122-
pg_subscription_rel.h
121+
pg_subscription_rel.h \
122+
pg_auth_password.h
123123

124124
GENERATED_HEADERS := $(CATALOG_HEADERS:%.h=%_d.h) schemapg.h system_fk_info.h
125125

‎src/backend/catalog/system_views.sql

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ CREATE VIEW pg_shadow AS
4646
FROM pg_authid LEFT JOIN pg_db_role_setting s
4747
ON (pg_authid.oid = setrole AND setdatabase = 0)
4848
LEFT JOIN pg_auth_password p
49-
ON p.roleid = pg_authid.oid
50-
49+
ON (p.roleid = pg_authid.oid)
5150
WHERE rolcanlogin;
5251

5352
REVOKE ALL ON pg_shadow FROM public;

‎src/backend/commands/user.c

Lines changed: 5 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ GrantRoleOptions createrole_self_grant_options;
9494
Interval *default_password_duration = NULL;
9595

9696
/* default password name */
97-
const char* default_passname = "__def__";
97+
static const char* default_password_name = "__def__";
9898

9999
/* Hook to check passwords in CreateRole() and AlterRole() */
100100
check_password_hook_type check_password_hook = NULL;
@@ -133,45 +133,7 @@ have_createrole_privilege(void)
133133
return has_createrole_privilege(GetUserId());
134134
}
135135

136-
/*
137-
* Is the role able to log in
138-
*/
139-
bool
140-
is_role_valid(const char *rolename, const char **logdetail)
141-
{
142-
HeapTuple roleTup;
143-
Datum datum;
144-
bool isnull;
145-
TimestampTz vuntil = 0;
146-
147-
/* Get role info from pg_authid */
148-
roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(rolename));
149-
if (!HeapTupleIsValid(roleTup))
150-
{
151-
*logdetail = psprintf(_("Role \"%s\" does not exist."),
152-
rolename);
153-
return false; /* no such user */
154-
}
155-
156-
datum = SysCacheGetAttr(AUTHNAME, roleTup,
157-
Anum_pg_authid_rolvaliduntil, &isnull);
158-
ReleaseSysCache(roleTup);
159-
160-
if (!isnull)
161-
vuntil = DatumGetTimestampTz(datum);
162-
163-
if (!isnull && vuntil < GetCurrentTimestamp())
164-
{
165-
*logdetail = psprintf(_("User \"%s\" has an expired password."), rolename);
166-
return false;
167-
}
168-
169-
return true;
170-
}
171-
172-
173-
static
174-
bool
136+
static bool
175137
validate_and_get_salt(char *rolename, char **salt, const char **logdetail)
176138
{
177139
char **current_secrets;
@@ -699,7 +661,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
699661
DirectFunctionCall1(namein, CStringGetDatum(passname));
700662
else
701663
new_password_record[Anum_pg_auth_password_name - 1] =
702-
DirectFunctionCall1(namein, CStringGetDatum(default_passname));
664+
DirectFunctionCall1(namein, CStringGetDatum(default_password_name));
703665

704666
/* open password table and insert it. */
705667
pg_auth_password_rel = table_open(AuthPasswordRelationId, RowExclusiveLock);
@@ -1207,7 +1169,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
12071169
DirectFunctionCall1(namein, CStringGetDatum(passname));
12081170
else
12091171
new_password_record[Anum_pg_auth_password_name - 1] =
1210-
DirectFunctionCall1(namein, CStringGetDatum(default_passname));
1172+
DirectFunctionCall1(namein, CStringGetDatum(default_password_name));
12111173
}
12121174
}
12131175
}
@@ -1248,7 +1210,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
12481210
if (dpassName)
12491211
password_tuple = SearchSysCache2(AUTHPASSWORDNAME, ObjectIdGetDatum(roleid), CStringGetDatum(passname));
12501212
else
1251-
password_tuple = SearchSysCache2(AUTHPASSWORDNAME, ObjectIdGetDatum(roleid), CStringGetDatum(default_passname));
1213+
password_tuple = SearchSysCache2(AUTHPASSWORDNAME, ObjectIdGetDatum(roleid), CStringGetDatum(default_password_name));
12521214

12531215
if (new_password_record_nulls[Anum_pg_auth_password_password - 1] == true) /* delete existing password */
12541216
{

‎src/backend/libpq/auth.c

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -590,26 +590,10 @@ ClientAuthentication(Port *port)
590590

591591
case uaMD5:
592592
case uaSCRAM:
593-
/*
594-
* check to be sure we are not past rolvaliduntil
595-
*/
596-
if (!is_role_valid(port->user_name, &logdetail)) {
597-
status = STATUS_ERROR;
598-
break;
599-
}
600-
601593
status = CheckPWChallengeAuth(port, &logdetail);
602594
break;
603595

604596
case uaPassword:
605-
/*
606-
* check to be sure we are not past rolvaliduntil
607-
*/
608-
if (!is_role_valid(port->user_name, &logdetail)) {
609-
status = STATUS_ERROR;
610-
break;
611-
}
612-
613597
status = CheckPasswordAuth(port, &logdetail);
614598
break;
615599

‎src/backend/libpq/crypt.c

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,32 +47,54 @@ get_role_passwords(const char *role, const char **logdetail, int *num)
4747
CatCList *passlist;
4848
int i, j = 0, num_valid_passwords = 0;
4949

50+
*num = 0;
51+
5052
/* Get role info from pg_authid */
5153
roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(role));
5254
if (!HeapTupleIsValid(roleTup))
5355
{
5456
*logdetail = psprintf(_("Role \"%s\" does not exist."),
5557
role);
56-
*num = 0;
5758
return NULL; /* no such user */
5859
}
5960

61+
datum = SysCacheGetAttr(AUTHNAME, roleTup,
62+
Anum_pg_authid_rolvaliduntil, &isnull);
63+
if (!isnull)
64+
vuntil = DatumGetTimestampTz(datum);
65+
66+
current = GetCurrentTimestamp();
67+
68+
/*
69+
* Check to be sure we are not past rolvaliduntil
70+
*/
71+
if (!isnull && vuntil < current)
72+
{
73+
/*
74+
* Although it is the role that has expired, not one of its passwords,
75+
* below we complain that the password has expired. This is to maintain
76+
* compatibility with external tools that may have come to depend on
77+
* this message when a role expires.
78+
*/
79+
*logdetail = psprintf(_("User \"%s\" has an expired password."),
80+
role);
81+
return NULL;
82+
}
83+
6084
datum = SysCacheGetAttr(AUTHNAME, roleTup, Anum_pg_authid_oid, &isnull);
6185
ReleaseSysCache(roleTup);
62-
/* Find any existing password that is not the one being updated to get the salt */
86+
87+
/* Find any existing password that is not the one being updated, to get the salt */
6388
passlist = SearchSysCacheList1(AUTHPASSWORDNAME, datum);
6489

6590
if (passlist->n_members == 0)
6691
{
67-
*num = 0;
6892
*logdetail = psprintf(_("User \"%s\" has no password assigned."),
6993
role);
7094
ReleaseCatCacheList(passlist);
7195
return NULL; /* user has no password */
7296
}
7397

74-
current = GetCurrentTimestamp();
75-
7698
for (i = 0; i < passlist->n_members; i++)
7799
{
78100
HeapTuple tup = &passlist->members[i]->tuple;

‎src/include/commands/user.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ typedef void (*check_password_hook_type) (const char *username, const char *shad
2929

3030
extern PGDLLIMPORT check_password_hook_type check_password_hook;
3131

32-
extern bool is_role_valid(const char *rolename, const char **logdetail);
3332
extern Oid CreateRole(ParseState *pstate, CreateRoleStmt *stmt);
3433
extern Oid AlterRole(ParseState *pstate, AlterRoleStmt *stmt);
3534
extern Oid AlterRoleSet(AlterRoleSetStmt *stmt);

0 commit comments

Comments
 (0)
0