Parametrization minimum password lenght
The goal about this patch is to parameterize the minimum password lenght on
users database and apply it on the general code.
The patch is applicable to the master branch.
We already tested it: it build and works as expected and nothing is found
broken,
Settings in postgresql.conf parametrization like following:
shared_preload_libraries = 'passwordcheck'
min_password_lenght = 12
example:
postgres=# create user prova with password 'eftghaki';
ERROR: password is too short
postgres=# create user prova with password 'eftghaki1234';
CREATE ROLE
In attach the file patch.
Best regards
Emanuele Musella & Maurizio Boriani
Attachments:
min_password_length_v1.patchapplication/octet-stream; name=min_password_length_v1.patchDownload
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 9d8c58ded0..572cf8e7a8 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -30,7 +30,7 @@ PG_MODULE_MAGIC;
static check_password_hook_type prev_check_password_hook = NULL;
/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+extern int min_password_lenght;
extern void _PG_init(void);
@@ -95,7 +95,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_lenght)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index acbcb8bb81..5dc74b2c2d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -160,6 +160,10 @@ char *GUC_check_errmsg_string;
char *GUC_check_errdetail_string;
char *GUC_check_errhint_string;
+/* min_password_lenght minimum password length */
+
+int min_password_lenght;
+
static void do_serialize(char **destptr, Size *maxbytes, const char *fmt,...) pg_attribute_printf(3, 4);
static void set_config_sourcefile(const char *name, char *sourcefile,
@@ -3653,6 +3657,17 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"min_password_lenght", PGC_USERSET, QUERY_TUNING_GEQO,
+ gettext_noop("Minimum password length."),
+ NULL,
+ GUC_EXPLAIN
+ },
+ &min_password_lenght,
+ 8, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
Hi,
On 11/12/24 14:41, Emanuele Musella wrote:
The goal about this patch is to parameterize the minimum password lenght
on users database and apply it on the general code.
The patch is applicable to the master branch.
We already tested it: it build and works as expected and nothing is
found broken,Settings in postgresql.conf parametrization like following:
shared_preload_libraries = 'passwordcheck'
min_password_lenght = 12example:
postgres=# create user prova with password 'eftghaki';
ERROR: password is too short
postgres=# create user prova with password 'eftghaki1234';
CREATE ROLEIn attach the file patch.
Thanks for the patch, seems like a useful feature. Please add the patch
to the next commitfest (2025-01) at https://commitfest.postgresql.org/
A couple comments:
1) The proper spelling is "length" (not "lenght").
2) The GUC should be added to the "passwordcheck" extension, not to the
core GUC file. See how auto_explain defines options in _PG_init() using
DefineCustomIntVariable.
3) It might be a good idea to add a test to passwordcheck.sql.
regards
--
Tomas Vondra
Hi Tomas,
we have done the fixes that you have suggested us in the last mail.
1) Modified passwordcheck.sql with a_nice_long_password and also
passwordcheck.out
2) we added the documentation in the doc/src/sgml/passwordcheck.sgml.
3) we have done make check successfully.
If you have other suggestions to share with us will be welcommed.
Thank you
Best regards
Il giorno mar 12 nov 2024 alle ore 16:49 Tomas Vondra <tomas@vondra.me> ha
scritto:
Show quoted text
On 11/12/24 16:36, Emanuele Musella wrote:
Hi Tomas,
thank you for you feedback.
We have implemented you suggestion in the patch. I attached you the
latest version.Please always reply to the mailing list, not just directly to individual
people. Otherwise others won't see your new messages, it won't get into
mailing list archives, etc.We are waiting for your last feedback to upload the patch on Commitfest.
Not sure I follow. You can add the patch to the commitfest on your own,
and there's no need to wait for my feedback before doing so.A couple quick comments for the patch, though:
1) You modified just the SQL script for the test, not the expected
output in expected/passwordcheck.out. This means running "make check"
for this extension will fail.2) It's not clear to me why you removed the first ALTER TABLE with
a_nice_long_password. Seems wrong.3) The test doesn't change the new GUC at all, so how would this show
the setting actually affects anything? And you only test the "positive"
case, maybe it'd be good to change the GUC and then test that it rejects
a password.4) The new GUC needs to be added to doc/src/sgml/passwordcheck.sgml.
regards
--
Tomas Vondra
Attachments:
min_password_length_v4.patchapplication/octet-stream; name=min_password_length_v4.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..67ad8aee1f 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -6,6 +6,8 @@
* Copyright (c) 2009-2024, PostgreSQL Global Development Group
*
* Author: Laurenz Albe <laurenz.albe@wien.gv.at>
+ * Author: Maurizio Boriani <maurizio@boriani.cloud>
+ * Author: Emanuele Musella <emamuse86@gmail.com>
*
* IDENTIFICATION
* contrib/passwordcheck/passwordcheck.c
@@ -23,14 +25,16 @@
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "commands/explain.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* min_password_length minimum password length */
+int min_password_length;
/*
* check_password
@@ -93,7 +97,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -145,4 +149,17 @@ _PG_init(void)
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
+
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ "8 is default.",
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_MS,
+ NULL,
+ NULL,
+ NULL);
}
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..c151685c74 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,12 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..e56323cdc9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -22,6 +22,12 @@
<filename>postgresql.conf</filename>, then restart the server.
</para>
+ <para>
+ In <filename>postgresql.conf</filename> you may set the minimum password length
+ by setting passwordcheck.min_password_length = INT.
+ The default minimum password length if not setted passwordcheck.min_password_length is 8 chars.
+ </para>
+
<para>
You can adapt this module to your needs by changing the source code.
For example, you can use
Import Notes
Reply to msg id not found: 9bd48f6d-dec0-48be-8db3-c3b29be588e7@vondra.me
Hi all,
we have fixed the following warning from the CFbot:
[07:14:29.637] passwordcheck.c:37:5: error: no previous extern declaration
for non-static variable 'min_password_length'
[-Werror,-Wmissing-variable-declarations]
In attach the fixed patch.
Best regards
Il giorno mer 13 nov 2024 alle ore 17:54 Emanuele Musella <
emamuse86@gmail.com> ha scritto:
Show quoted text
Hi Tomas,
we have done the fixes that you have suggested us in the last mail.
1) Modified passwordcheck.sql with a_nice_long_password and also
passwordcheck.out2) we added the documentation in the doc/src/sgml/passwordcheck.sgml.
3) we have done make check successfully.
If you have other suggestions to share with us will be welcommed.
Thank you
Best regards
Il giorno mar 12 nov 2024 alle ore 16:49 Tomas Vondra <tomas@vondra.me>
ha scritto:On 11/12/24 16:36, Emanuele Musella wrote:
Hi Tomas,
thank you for you feedback.
We have implemented you suggestion in the patch. I attached you the
latest version.Please always reply to the mailing list, not just directly to individual
people. Otherwise others won't see your new messages, it won't get into
mailing list archives, etc.We are waiting for your last feedback to upload the patch on Commitfest.
Not sure I follow. You can add the patch to the commitfest on your own,
and there's no need to wait for my feedback before doing so.A couple quick comments for the patch, though:
1) You modified just the SQL script for the test, not the expected
output in expected/passwordcheck.out. This means running "make check"
for this extension will fail.2) It's not clear to me why you removed the first ALTER TABLE with
a_nice_long_password. Seems wrong.3) The test doesn't change the new GUC at all, so how would this show
the setting actually affects anything? And you only test the "positive"
case, maybe it'd be good to change the GUC and then test that it rejects
a password.4) The new GUC needs to be added to doc/src/sgml/passwordcheck.sgml.
regards
--
Tomas Vondra
Attachments:
min_password_length_v5.patchapplication/octet-stream; name=min_password_length_v5.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..948f30b748 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -6,6 +6,8 @@
* Copyright (c) 2009-2024, PostgreSQL Global Development Group
*
* Author: Laurenz Albe <laurenz.albe@wien.gv.at>
+ * Author: Maurizio Boriani <maurizio@boriani.cloud>
+ * Author: Emanuele Musella <emamuse86@gmail.com>
*
* IDENTIFICATION
* contrib/passwordcheck/passwordcheck.c
@@ -23,14 +25,16 @@
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "commands/explain.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* min_password_length minimum password length */
+static int min_password_length;
/*
* check_password
@@ -93,7 +97,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -145,4 +149,17 @@ _PG_init(void)
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
+
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ "8 is default.",
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_MS,
+ NULL,
+ NULL,
+ NULL);
}
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..c151685c74 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,12 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..e56323cdc9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -22,6 +22,12 @@
<filename>postgresql.conf</filename>, then restart the server.
</para>
+ <para>
+ In <filename>postgresql.conf</filename> you may set the minimum password length
+ by setting passwordcheck.min_password_length = INT.
+ The default minimum password length if not setted passwordcheck.min_password_length is 8 chars.
+ </para>
+
<para>
You can adapt this module to your needs by changing the source code.
For example, you can use
Hi,
On Mon, Nov 18, 2024 at 10:01:41AM +0100, Emanuele Musella wrote:
Hi all,
we have fixed the following warning from the CFbot:
[07:14:29.637] passwordcheck.c:37:5: error: no previous extern declaration
for non-static variable 'min_password_length'
[-Werror,-Wmissing-variable-declarations]In attach the fixed patch.
Thanks for the patch, that looks like a useful feature!
A few random comments:
=== 1
+ * Author: Maurizio Boriani <maurizio@boriani.cloud>
+ * Author: Emanuele Musella <emamuse86@gmail.com>I'm not sure we do that usually. For example, in cb3384a0cb, Fabien Coelho has
not been mentioned as an author in contrib/isn/isn.c.
=== 2
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "commands/explain.h"
+#include "utils/guc.h"
The "includes" should be in the order based on their ASCII value (see 7e735035f2).
=== 3
+/* min_password_length minimum password length */
I think "/* Minimum password length */ is "enough" for the comment.
Also what about adding "/* GUC variables */" on top of it?
=== 4 (Nit)
+static int min_password_length;
What about "min_pwd_len" to make it consistent with pwd_has_letter and pwd_has_nonletter
for example?
=== 5
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
+
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",What about moving the DefineCustomIntVariable before the hooks? (that would be
consistent with auto_explain.c, auth_delay.c, autoprewarm.c and pg_stat_statements.c
to name a few).
=== 6
+ "8 is default.",
I don't think that's needed (that would be visible from pg_settings.extra_desc
while it already provides the information through the boot_val field).
=== 7
+ GUC_UNIT_MS,
Not the right unit, that should be GUC_UNIT_BYTE because...
=== 8
postgres=# SET passwordcheck.min_password_length = 4;
SET
postgres=# create user test with password 'ab1';
ERROR: password is too short
But with multibyte in play:
postgres=# create user test with password '�b1';
CREATE ROLE.
That's because "strlen(password);" returns the number of bytes.
We could make it based on the characters instead by using "pg_mbstrlen()"
but that would break the behavior as compared to now. So, I think we
just need to make it clear that's bytes instead.
=== 9
I think that a call to MarkGUCPrefixReserved() is missing (see auto_explain.c
for example).
=== 10
+ <para>
+ In <filename>postgresql.conf</filename> you may set the minimum password length
+ by setting passwordcheck.min_password_length = INT.I think that would make sense to add a "Configuration Parameters" section and
follow the format done in auto-explain or pg_prewarm for example.
=== 11
+ The default minimum password length if not setted passwordcheck.min_password_length is 8 chars.
s/chars/bytes/ (as per === 8 above)
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hello Bertrand,
thank you for your feedbacks.
We have done every fixes required.
In attach the last version.
Best regards
Il giorno lun 18 nov 2024 alle ore 11:39 Bertrand Drouvot <
bertranddrouvot.pg@gmail.com> ha scritto:
Show quoted text
Hi,
On Mon, Nov 18, 2024 at 10:01:41AM +0100, Emanuele Musella wrote:
Hi all,
we have fixed the following warning from the CFbot:
[07:14:29.637] passwordcheck.c:37:5: error: no previous extern
declaration
for non-static variable 'min_password_length'
[-Werror,-Wmissing-variable-declarations]In attach the fixed patch.
Thanks for the patch, that looks like a useful feature!
A few random comments:
=== 1
+ * Author: Maurizio Boriani <maurizio@boriani.cloud> + * Author: Emanuele Musella <emamuse86@gmail.com>I'm not sure we do that usually. For example, in cb3384a0cb, Fabien Coelho
has
not been mentioned as an author in contrib/isn/isn.c.=== 2
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "commands/explain.h"
+#include "utils/guc.h"The "includes" should be in the order based on their ASCII value (see
7e735035f2).=== 3
+/* min_password_length minimum password length */
I think "/* Minimum password length */ is "enough" for the comment.
Also what about adding "/* GUC variables */" on top of it?
=== 4 (Nit)
+static int min_password_length;
What about "min_pwd_len" to make it consistent with pwd_has_letter and
pwd_has_nonletter
for example?=== 5
prev_check_password_hook = check_password_hook; check_password_hook = check_password; + + /* Define custom GUC variables. */ + DefineCustomIntVariable("passwordcheck.min_password_length",What about moving the DefineCustomIntVariable before the hooks? (that
would be
consistent with auto_explain.c, auth_delay.c, autoprewarm.c and
pg_stat_statements.c
to name a few).=== 6
+ "8 is default.",
I don't think that's needed (that would be visible from
pg_settings.extra_desc
while it already provides the information through the boot_val field).=== 7
+ GUC_UNIT_MS,
Not the right unit, that should be GUC_UNIT_BYTE because...
=== 8
postgres=# SET passwordcheck.min_password_length = 4;
SET
postgres=# create user test with password 'ab1';
ERROR: password is too shortBut with multibyte in play:
postgres=# create user test with password 'äb1';
CREATE ROLE.That's because "strlen(password);" returns the number of bytes.
We could make it based on the characters instead by using "pg_mbstrlen()"
but that would break the behavior as compared to now. So, I think we
just need to make it clear that's bytes instead.=== 9
I think that a call to MarkGUCPrefixReserved() is missing (see
auto_explain.c
for example).=== 10
+ <para> + In <filename>postgresql.conf</filename> you may set the minimum password length + by setting passwordcheck.min_password_length = INT.I think that would make sense to add a "Configuration Parameters" section
and
follow the format done in auto-explain or pg_prewarm for example.=== 11
+ The default minimum password length if not setted
passwordcheck.min_password_length is 8 chars.s/chars/bytes/ (as per === 8 above)
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
min_password_length_v6.patchapplication/octet-stream; name=min_password_length_v6.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..a49b913e46 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,13 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
+ERROR: password is too short
+-- error: too short
+ALTER USER regress_passwordcheck_user1 PASSWORD 'äbcdefghijk';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f4f90274b7 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -6,6 +6,8 @@
* Copyright (c) 2009-2024, PostgreSQL Global Development Group
*
* Author: Laurenz Albe <laurenz.albe@wien.gv.at>
+ * Author: Maurizio Boriani <maurizio@boriani.cloud>
+ * Author: Emanuele Musella <emamuse86@gmail.com>
*
* IDENTIFICATION
* contrib/passwordcheck/passwordcheck.c
@@ -20,17 +22,19 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
/*
* check_password
@@ -84,7 +88,7 @@ check_password(const char *username,
* For unencrypted passwords we can perform better checks
*/
const char *password = shadow_pass;
- int pwdlen = strlen(password);
+ int pwdlen = pg_mbstrlen(password);
int i;
bool pwd_has_letter,
pwd_has_nonletter;
@@ -93,7 +97,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +146,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..4b0cea82d4 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,15 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
+
+-- error: too short
+ALTER USER regress_passwordcheck_user1 PASSWORD 'äbcdefghijk';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..f847ff1860 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,39 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8 bytes.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+<programlisting>
+# postgresql.conf
+session_preload_libraries = 'passwordcheck'
+passwordcheck.min_password_length = 12
+
+</programlisting>
+
+ </sect2>
+
</sect1>
We notice some errors on CFBot results.
In attached the errors fixed
Il giorno lun 18 nov 2024 alle ore 16:59 Emanuele Musella <
emamuse86@gmail.com> ha scritto:
Show quoted text
Hello Bertrand,
thank you for your feedbacks.
We have done every fixes required.
In attach the last version.
Best regards
Il giorno lun 18 nov 2024 alle ore 11:39 Bertrand Drouvot <
bertranddrouvot.pg@gmail.com> ha scritto:Hi,
On Mon, Nov 18, 2024 at 10:01:41AM +0100, Emanuele Musella wrote:
Hi all,
we have fixed the following warning from the CFbot:
[07:14:29.637] passwordcheck.c:37:5: error: no previous extern
declaration
for non-static variable 'min_password_length'
[-Werror,-Wmissing-variable-declarations]In attach the fixed patch.
Thanks for the patch, that looks like a useful feature!
A few random comments:
=== 1
+ * Author: Maurizio Boriani <maurizio@boriani.cloud> + * Author: Emanuele Musella <emamuse86@gmail.com>I'm not sure we do that usually. For example, in cb3384a0cb, Fabien
Coelho has
not been mentioned as an author in contrib/isn/isn.c.=== 2
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "commands/explain.h"
+#include "utils/guc.h"The "includes" should be in the order based on their ASCII value (see
7e735035f2).=== 3
+/* min_password_length minimum password length */
I think "/* Minimum password length */ is "enough" for the comment.
Also what about adding "/* GUC variables */" on top of it?
=== 4 (Nit)
+static int min_password_length;
What about "min_pwd_len" to make it consistent with pwd_has_letter and
pwd_has_nonletter
for example?=== 5
prev_check_password_hook = check_password_hook; check_password_hook = check_password; + + /* Define custom GUC variables. */ + DefineCustomIntVariable("passwordcheck.min_password_length",What about moving the DefineCustomIntVariable before the hooks? (that
would be
consistent with auto_explain.c, auth_delay.c, autoprewarm.c and
pg_stat_statements.c
to name a few).=== 6
+ "8 is default.",
I don't think that's needed (that would be visible from
pg_settings.extra_desc
while it already provides the information through the boot_val field).=== 7
+ GUC_UNIT_MS,
Not the right unit, that should be GUC_UNIT_BYTE because...
=== 8
postgres=# SET passwordcheck.min_password_length = 4;
SET
postgres=# create user test with password 'ab1';
ERROR: password is too shortBut with multibyte in play:
postgres=# create user test with password 'äb1';
CREATE ROLE.That's because "strlen(password);" returns the number of bytes.
We could make it based on the characters instead by using "pg_mbstrlen()"
but that would break the behavior as compared to now. So, I think we
just need to make it clear that's bytes instead.=== 9
I think that a call to MarkGUCPrefixReserved() is missing (see
auto_explain.c
for example).=== 10
+ <para> + In <filename>postgresql.conf</filename> you may set the minimum password length + by setting passwordcheck.min_password_length = INT.I think that would make sense to add a "Configuration Parameters" section
and
follow the format done in auto-explain or pg_prewarm for example.=== 11
+ The default minimum password length if not setted
passwordcheck.min_password_length is 8 chars.s/chars/bytes/ (as per === 8 above)
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
min_password_length_v7.patchapplication/octet-stream; name=min_password_length_v7.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..a49b913e46 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,13 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
+ERROR: password is too short
+-- error: too short
+ALTER USER regress_passwordcheck_user1 PASSWORD 'äbcdefghijk';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..488190eccb 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -6,6 +6,8 @@
* Copyright (c) 2009-2024, PostgreSQL Global Development Group
*
* Author: Laurenz Albe <laurenz.albe@wien.gv.at>
+ * Author: Maurizio Boriani <maurizio@boriani.cloud>
+ * Author: Emanuele Musella <emamuse86@gmail.com>
*
* IDENTIFICATION
* contrib/passwordcheck/passwordcheck.c
@@ -20,17 +22,20 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
/*
* check_password
@@ -84,7 +89,7 @@ check_password(const char *username,
* For unencrypted passwords we can perform better checks
*/
const char *password = shadow_pass;
- int pwdlen = strlen(password);
+ int pwdlen = pg_mbstrlen(password);
int i;
bool pwd_has_letter,
pwd_has_nonletter;
@@ -93,7 +98,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +147,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..4b0cea82d4 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,15 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
+
+-- error: too short
+ALTER USER regress_passwordcheck_user1 PASSWORD 'äbcdefghijk';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..f847ff1860 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,39 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8 bytes.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+<programlisting>
+# postgresql.conf
+session_preload_libraries = 'passwordcheck'
+passwordcheck.min_password_length = 12
+
+</programlisting>
+
+ </sect2>
+
</sect1>
Hi,
On Mon, Nov 18, 2024 at 05:21:18PM +0100, Emanuele Musella wrote:
We notice some errors on CFBot results.
FWIW, you can run "cfbot like" tests on your own repo (see [1]https://github.com/postgres/postgres/blob/master/src/tools/ci/README).
In attached the errors fixed
Thanks for the updated version!
A few random comments:
=== 1
trailing whitespace:
$ git apply min_password_length_v7.patch
min_password_length_v7.patch:130: trailing whitespace.
There is a configuration parameter that control the behavior
warning: 1 line adds whitespace errors.
=== 2
+ * Author: Maurizio Boriani <maurizio@boriani.cloud>
+ * Author: Emanuele Musella <emamuse86@gmail.com>Same comment as in [2]/messages/by-id/ZzsZZY3YrO6hinnT@ip-10-97-1-34.eu-west-3.compute.internal.
=== 3
- int pwdlen = strlen(password);
+ int pwdlen = pg_mbstrlen(password);Sorry if I was not clear in [2]/messages/by-id/ZzsZZY3YrO6hinnT@ip-10-97-1-34.eu-west-3.compute.internal, but I meant to say to keep using strlen() to be
consistent with the current behavior.
=== 4
+ GUC_UNIT_BYTE,
this is correct if strlen() is used (see above comment).
=== 5
+ 0, INT_MAX,
INT_MAX seems too large and 0 too low. Maybe we should not allow less than it
was before the patch (8). For the max, maybe something like PG_MAX_AUTH_TOKEN_LENGTH?
(see the comment in src/backend/libpq/auth.c)
=== 6
+ There is a configuration parameter that control the behavior
+ <filename>passwordcheck</filename>s/behavior/behavior of/?
=== 7
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8 bytes.What about? "is the minimum password length in bytes. The default is 8."
=== 7
+
+<programlisting>
+# postgresql.conf
+session_preload_libraries = 'passwordcheck'
+passwordcheck.min_password_length = 12
+
+</programlisting>What about a sentence before? Something like for auto_explain means "In ordinary
usage, these parameters are set in postgresql.conf,............"
[1]: https://github.com/postgres/postgres/blob/master/src/tools/ci/README
[2]: /messages/by-id/ZzsZZY3YrO6hinnT@ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Tue, Nov 12, 2024 at 02:48:28PM +0100, Tomas Vondra wrote:
Thanks for the patch, seems like a useful feature. Please add the patch
to the next commitfest (2025-01) at https://commitfest.postgresql.org/
FYI, I have a large set of such things in my own repo with a clone of
passwordcheck:
https://github.com/michaelpq/pg_plugins/tree/main/passwordcheck_extra
Not all of them may be useful, still the minimal length is one of
them:
- passwordcheck_extra.special_chars, to define a list of special characters
with the password needing at least one. Default is "!@#$%^&*()_+{}|<>?=".
- passwordcheck_extra.restrict_lower, to enforce the use of at least one
lower-case character.
- passwordcheck_extra.restrict_upper, to enforce the use of at least one
upper-case character.
- passwordcheck_extra.restrict_numbers, to enforce the use of at least
one number.
- passwordcheck_extra.restrict_special, to enforce the use of at least
one special character listed in \"passwordcheck_extra.special_chars\".
- passwordcheck_extra.minimum_length, minimum length of password allowed.
Default is 8, which likely sucks.
- passwordcheck_extra.maximum_length, maximum length of password allowed.
Default is 15, which definitely sucks, but it is useful for tests.
I would not mind if you just copy-paste some of this stuff, and the
code is released under the PostgreSQL license. Feel free to not worry
about the authorship part as well, I'm fine even if my name is
discarded.
--
Michael
On Wed, Nov 20, 2024 at 07:45:40AM +0900, Michael Paquier wrote:
On Tue, Nov 12, 2024 at 02:48:28PM +0100, Tomas Vondra wrote:
Thanks for the patch, seems like a useful feature. Please add the patch
to the next commitfest (2025-01) at https://commitfest.postgresql.org/FYI, I have a large set of such things in my own repo with a clone of
passwordcheck:
https://github.com/michaelpq/pg_plugins/tree/main/passwordcheck_extra
Here is some previous discussion about passwordcheck that may be of
interest:
/messages/by-id/D960CB61B694CF459DCFB4B0128514C203937F49@exadv11.host.magwien.gv.at
/messages/by-id/AC785D69-41EC-4D0A-AC37-1F9FF55C9E34@amazon.com
--
nathan
Thank you Bertrand for your feedbacks. We are looking for CFbot part so we
can compile it like CFbot.
For now we have fixed all points.
Thank you
Il giorno mar 19 nov 2024 alle ore 20:28 Bertrand Drouvot <
bertranddrouvot.pg@gmail.com> ha scritto:
Show quoted text
Hi,
On Mon, Nov 18, 2024 at 05:21:18PM +0100, Emanuele Musella wrote:
We notice some errors on CFBot results.
FWIW, you can run "cfbot like" tests on your own repo (see [1]).
In attached the errors fixed
Thanks for the updated version!
A few random comments:
=== 1
trailing whitespace:
$ git apply min_password_length_v7.patch
min_password_length_v7.patch:130: trailing whitespace.
There is a configuration parameter that control the behavior
warning: 1 line adds whitespace errors.=== 2
+ * Author: Maurizio Boriani <maurizio@boriani.cloud> + * Author: Emanuele Musella <emamuse86@gmail.com>Same comment as in [2].
=== 3
- int pwdlen = strlen(password); + int pwdlen = pg_mbstrlen(password);Sorry if I was not clear in [2], but I meant to say to keep using strlen()
to be
consistent with the current behavior.=== 4
+ GUC_UNIT_BYTE,
this is correct if strlen() is used (see above comment).
=== 5
+ 0, INT_MAX,
INT_MAX seems too large and 0 too low. Maybe we should not allow less than
it
was before the patch (8). For the max, maybe something like
PG_MAX_AUTH_TOKEN_LENGTH?
(see the comment in src/backend/libpq/auth.c)=== 6
+ There is a configuration parameter that control the behavior + <filename>passwordcheck</filename>s/behavior/behavior of/?
=== 7
+ <varname>passwordcheck.min_password_length</varname> is the minimum length + of accepted password on database users. + If not setted the default is 8 bytes.What about? "is the minimum password length in bytes. The default is 8."
=== 7
+ +<programlisting> +# postgresql.conf +session_preload_libraries = 'passwordcheck' +passwordcheck.min_password_length = 12 + +</programlisting>What about a sentence before? Something like for auto_explain means "In
ordinary
usage, these parameters are set in postgresql.conf,............"[1]: https://github.com/postgres/postgres/blob/master/src/tools/ci/README
[2]:
/messages/by-id/ZzsZZY3YrO6hinnT@ip-10-97-1-34.eu-west-3.compute.internalRegards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
min_password_length_v8.patchapplication/octet-stream; name=min_password_length_v8.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f9a2c2db33 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -20,17 +20,23 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
+
+/* Max password length */
+#define PG_MAX_PASSWORD_LENGTH 128
/*
* check_password
@@ -93,7 +99,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +148,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 8, PG_MAX_PASSWORD_LENGTH,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
deleted file mode 100644
index 1fbd6b0e96..0000000000
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ /dev/null
@@ -1,23 +0,0 @@
-LOAD 'passwordcheck';
-
-CREATE USER regress_passwordcheck_user1;
-
--- ok
-ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-
--- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
-
--- error: contains user name
-ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
-
--- error: contains only letters
-ALTER USER regress_passwordcheck_user1 PASSWORD 'alessnicelongpassword';
-
--- encrypted ok (password is "secret")
-ALTER USER regress_passwordcheck_user1 PASSWORD 'md592350e12ac34e52dd598f90893bb3ae7';
-
--- error: password is user name
-ALTER USER regress_passwordcheck_user1 PASSWORD 'md507a112732ed9f2087fa90b192d44e358';
-
-DROP USER regress_passwordcheck_user1;
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..160553b8e9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,30 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior of
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
</sect1>
Sorry here the right attachment
Il giorno lun 25 nov 2024 alle ore 11:15 Emanuele Musella <
emamuse86@gmail.com> ha scritto:
Show quoted text
Thank you Bertrand for your feedbacks. We are looking for CFbot part so we
can compile it like CFbot.For now we have fixed all points.
Thank you
Il giorno mar 19 nov 2024 alle ore 20:28 Bertrand Drouvot <
bertranddrouvot.pg@gmail.com> ha scritto:Hi,
On Mon, Nov 18, 2024 at 05:21:18PM +0100, Emanuele Musella wrote:
We notice some errors on CFBot results.
FWIW, you can run "cfbot like" tests on your own repo (see [1]).
In attached the errors fixed
Thanks for the updated version!
A few random comments:
=== 1
trailing whitespace:
$ git apply min_password_length_v7.patch
min_password_length_v7.patch:130: trailing whitespace.
There is a configuration parameter that control the behavior
warning: 1 line adds whitespace errors.=== 2
+ * Author: Maurizio Boriani <maurizio@boriani.cloud> + * Author: Emanuele Musella <emamuse86@gmail.com>Same comment as in [2].
=== 3
- int pwdlen = strlen(password); + int pwdlen = pg_mbstrlen(password);Sorry if I was not clear in [2], but I meant to say to keep using
strlen() to be
consistent with the current behavior.=== 4
+ GUC_UNIT_BYTE,
this is correct if strlen() is used (see above comment).
=== 5
+ 0, INT_MAX,
INT_MAX seems too large and 0 too low. Maybe we should not allow less
than it
was before the patch (8). For the max, maybe something like
PG_MAX_AUTH_TOKEN_LENGTH?
(see the comment in src/backend/libpq/auth.c)=== 6
+ There is a configuration parameter that control the behavior + <filename>passwordcheck</filename>s/behavior/behavior of/?
=== 7
+ <varname>passwordcheck.min_password_length</varname> is the minimum length + of accepted password on database users. + If not setted the default is 8 bytes.What about? "is the minimum password length in bytes. The default is 8."
=== 7
+ +<programlisting> +# postgresql.conf +session_preload_libraries = 'passwordcheck' +passwordcheck.min_password_length = 12 + +</programlisting>What about a sentence before? Something like for auto_explain means "In
ordinary
usage, these parameters are set in postgresql.conf,............"[1]: https://github.com/postgres/postgres/blob/master/src/tools/ci/README
[2]:
/messages/by-id/ZzsZZY3YrO6hinnT@ip-10-97-1-34.eu-west-3.compute.internalRegards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
min_password_length_v9.patchapplication/octet-stream; name=min_password_length_v9.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f9a2c2db33 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -20,17 +20,23 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
+
+/* Max password length */
+#define PG_MAX_PASSWORD_LENGTH 128
/*
* check_password
@@ -93,7 +99,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +148,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 8, PG_MAX_PASSWORD_LENGTH,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..c151685c74 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,12 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..160553b8e9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,30 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior of
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
</sect1>
Here is what I have staged for commit.
--
nathan
Attachments:
v10-0001-Add-passwordcheck.min_password_length.patchtext/plain; charset=us-asciiDownload
From 537260d0492ad3bda2c2fcbfd00c6b0511f5e1b5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 18 Dec 2024 14:49:20 -0600
Subject: [PATCH v10 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 3 ++
.../expected/passwordcheck_1.out | 3 ++
contrib/passwordcheck/passwordcheck.c | 22 +++++++++--
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..ad1ac400ba 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,9 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..e97c2c74d7 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,9 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+-- ok
+SET passwordcheck.min_password_length = 5;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..a0103806d0 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -26,11 +27,11 @@
PG_MODULE_MAGIC;
-/* Saved hook value in case of unload */
+/* Original hook */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,7 +94,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +143,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..21ad8d452b 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..7ea3241046 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
Hi,
On Wed, Dec 18, 2024 at 02:56:24PM -0600, Nathan Bossart wrote:
Here is what I have staged for commit.
Thanks!
A few comments:
=== 1
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
Now that the minimum password length is not "hardcoded" anymore, I wonder if it
wouldn't be better to provide more details here (pwdlen and min_password_length).
Suggestion in on_top_of_0001.txt attached.
=== 2
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,Since password must contain both letters and nonletters, 0 seems too low. I
wonder if 2 is not a better value (done in on_top_of_0001.txt attached).
Also, it seems to me that INT_MAX is too large (as mentioned in [1]/messages/by-id/Zzzmw4IAvrypmFO4@ip-10-97-1-34.eu-west-3.compute.internal), but that's
probably a nit.
[1]: /messages/by-id/Zzzmw4IAvrypmFO4@ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
on_top_of_0001.txttext/plain; charset=us-asciiDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index ad1ac400ba..5d7a9496c9 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -5,7 +5,7 @@ CREATE USER regress_passwordcheck_user1;
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
-ERROR: password is too short
+ERROR: password is too short: 7 (< 8)
-- ok
SET passwordcheck.min_password_length = 6;
ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index a0103806d0..5027fa35c8 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -97,7 +97,7 @@ check_password(const char *username,
if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -149,7 +149,7 @@ _PG_init(void)
NULL,
&min_password_length,
8,
- 0, INT_MAX,
+ 2, INT_MAX,
PGC_SUSET,
GUC_UNIT_BYTE,
NULL, NULL, NULL);
Hi Nathan,
thank you.
It seems to me you are working on an older version of patch.
In attached the latest one for your reference.
Thanks for your support
Best regards
Emanuele Musella
Il giorno mer 18 dic 2024 alle ore 21:56 Nathan Bossart <
nathandbossart@gmail.com> ha scritto:
Show quoted text
Here is what I have staged for commit.
--
nathan
Attachments:
min_password_length_v9.patchapplication/octet-stream; name=min_password_length_v9.patchDownload
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f9a2c2db33 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -20,17 +20,23 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
+
+/* Max password length */
+#define PG_MAX_PASSWORD_LENGTH 128
/*
* check_password
@@ -93,7 +99,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +148,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 8, PG_MAX_PASSWORD_LENGTH,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..c151685c74 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,12 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..160553b8e9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,30 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior of
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
</sect1>
On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));Now that the minimum password length is not "hardcoded" anymore, I wonder if it
wouldn't be better to provide more details here (pwdlen and min_password_length).Suggestion in on_top_of_0001.txt attached.
Yeah, good point.
+ /* Define custom GUC variables. */ + DefineCustomIntVariable("passwordcheck.min_password_length", + "Minimum allowed password length.", + NULL, + &min_password_length, + 8, + 0, INT_MAX,Since password must contain both letters and nonletters, 0 seems too low. I
wonder if 2 is not a better value (done in on_top_of_0001.txt attached).Also, it seems to me that INT_MAX is too large (as mentioned in [1]), but that's
probably a nit.
I don't think we need to restrict the allowed values here. It's true that
the lower bound will be effectively 2 due to the other checks, but that's
not the responsibility of this one to enforce. Those other checks will
likely become configurable at some point, too.
- errmsg("password is too short"))); + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
I think we should explicitly point to the parameter and note its current
value.
--
nathan
On Thu, Dec 19, 2024 at 11:22:02AM +0100, Emanuele Musella wrote:
It seems to me you are working on an older version of patch.
In attached the latest one for your reference.
I used the v9 patch as the basis for v10. There are a few small edits,
such as removing the upper bound for the parameter and expanding the
documentation, but the feature itself remains intact.
--
nathan
On Thu, Dec 19, 2024 at 09:36:17AM -0600, Nathan Bossart wrote:
On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
- errmsg("password is too short"))); + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));I think we should explicitly point to the parameter and note its current
value.
Like so.
--
nathan
Attachments:
v11-0001-Add-passwordcheck.min_password_length.patchtext/plain; charset=us-asciiDownload
From ce67f3ff5fca47b443195f5bd1932429294bdbfb Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 18 Dec 2024 14:49:20 -0600
Subject: [PATCH v11 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 4 ++
.../expected/passwordcheck_1.out | 4 ++
contrib/passwordcheck/passwordcheck.c | 26 ++++++++++---
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..83472c76d2 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..fb12ec45cc 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..d79ecb00b6 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -26,11 +27,11 @@
PG_MODULE_MAGIC;
-/* Saved hook value in case of unload */
+/* Original hook */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,10 +94,12 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short"),
+ errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+ min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -142,6 +145,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..21ad8d452b 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..7ea3241046 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
On Thu, Dec 19, 2024 at 09:57:31AM -0600, Nathan Bossart wrote:
On Thu, Dec 19, 2024 at 09:36:17AM -0600, Nathan Bossart wrote:
On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
- errmsg("password is too short"))); + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));I think we should explicitly point to the parameter and note its current
value.Like so.
LGTM.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Thu, 19 Dec 2024 at 09:57, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Thu, Dec 19, 2024 at 09:36:17AM -0600, Nathan Bossart wrote:
On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
- errmsg("password is too short"))); + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));I think we should explicitly point to the parameter and note its current
value.Like so.
It seems that the v11 cannot apply on master since 7f97b4734, and rebase is needed.
$ git am ~/v11-0001-Add-passwordcheck.min_password_length.patch
Applying: Add passwordcheck.min_password_length.
error: patch failed: contrib/passwordcheck/passwordcheck.c:26
error: contrib/passwordcheck/passwordcheck.c: patch does not apply
Patch failed at 0001 Add passwordcheck.min_password_length.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
The comment of prev_check_password_hook has been changed which case the apply failed.
--
Regrads,
Japin Li
On Wed, Dec 25, 2024 at 01:15:53PM +0800, Japin Li wrote:
It seems that the v11 cannot apply on master since 7f97b4734, and rebase is needed.
$ git am ~/v11-0001-Add-passwordcheck.min_password_length.patch
Applying: Add passwordcheck.min_password_length.
error: patch failed: contrib/passwordcheck/passwordcheck.c:26
error: contrib/passwordcheck/passwordcheck.c: patch does not apply
Patch failed at 0001 Add passwordcheck.min_password_length.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".The comment of prev_check_password_hook has been changed which case the apply failed.
Here is a rebased version of the patch. Barring objections, I am planning
to commit this soon.
--
nathan
Attachments:
v12-0001-Add-passwordcheck.min_password_length.patchtext/plain; charset=us-asciiDownload
From fd7d382fc463fc47ee1c18695d78a2203723f46b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 6 Jan 2025 13:29:58 -0600
Subject: [PATCH v12 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot, Japin Li
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 4 ++
.../expected/passwordcheck_1.out | 4 ++
contrib/passwordcheck/passwordcheck.c | 24 ++++++++++--
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe008..83472c76d27 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a495..fb12ec45cc4 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 96ada5b98c6..3db42a5b99d 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -29,8 +30,8 @@ PG_MODULE_MAGIC;
/* Saved hook value */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,10 +94,12 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short"),
+ errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+ min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -142,6 +145,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c26..21ad8d452b5 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f4892272..7ea32410463 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)