Re: [HACKERS] Password identifiers, protocol aging and SCRAM protocol
Hi! On 03/15/2016 06:59 PM, Michael Paquier wrote: The set of patches I am proposing here does not go through those code paths, and this is likely an aggregate failure. Michael, you were right. It was incorrect installation of contrib binaries. Now all tests pass OK, both check-world and installcheck-world, Thanks. -- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS][REVIEW]: Password identifiers, protocol aging and SCRAM protocol
This is a review of "Password identifiers, protocol aging and SCRAM protocol" patches http://www.postgresql.org/message-id/CAB7nPqSMXU35g=w9x74hveqp0uvgjxvyoua4a-a3m+0wfeb...@mail.gmail.com Contents & Purpose -- There was a discussion dedicated to SCRAM: http://www.postgresql.org/message-id/55192afe.6080...@iki.fi This set of patches implements the following: - Introduce in Postgres an extensible password aging facility, by having a new concept of 1 user/multiple password verifier, one password verifier per protocol. - Give to system administrators tools to decide unsupported protocols, and have pg_upgrade use that - Introduce new password protocols for Postgres, aimed at replacing existing, say limited ones. This set of patches consists of 9 separate patches. Description of each patch is well described in initial thread email and in comments. The first set of patches 0001-0008 adds facility to store multiple password verifiers, CREATE ROLE and ALTER ROLE are extended with PASSWORD VERIFIERS, new superuser GUC parameters which specifies a list of supported password protocols in Postgres backend, added pg_auth_verifiers_sanitize function, removed password verifiers for unsupported protocols in pg_upgrade, and more features. The second set of patch 0005~0008 introduces a new protocol, SCRAM, and 0009 is SCRAM itself. Initial Run - Included in the patches are: - source code - regression tests - documentation The source code is well commented. The patches are in context diff format and were applied correctly to HEAD (there were 2 warnings, and it was fixed by author). There were several markup warnings, should be fixed by author. Regression tests pass successfully, without errors. It seems that the patches work as expected. The patch 0009 depends on all previous patches 0001-0008: first we need to apply patches 0001-0008, then 0009. Performance --- I have not tested possible performance issues yet. Conclusion -- I think introduced features are useful and I vote for commit +1. On 03/02/2016 02:55 PM, Michael Paquier wrote: On Wed, Mar 2, 2016 at 5:43 PM, Valery Popov wrote: So the markup is missing. Thanks. I am taking note of it. -- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS][REVIEW]: Password identifiers, protocol aging and SCRAM protocol
db_user_namespace causes the client's and server's user name representation to differ. Authentication checks are always done with the server's user name so authentication methods must be configured for the server's user name, not the client's. Because md5 uses the user name as salt on both the client and server, md5 cannot be used with db_user_namespace. Also in doc/src/sgml/ref/create_role.sgml is should be instead of PASSWORD VERIFIERS ( class="PARAMETER">verifier_type = 'class="PARAMETER">password' like this PASSWORD VERIFIERS ( class="PARAMETER">verifier_type = 'class="PARAMETER">password'-- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS][REVIEW]: Password identifiers, protocol aging and SCRAM protocol
Hi, Michael 23.02.2016 10:17, Michael Paquier пишет: Attached is a set of patches implementing a couple of things that have been discussed, so let's roll in. Those 4 patches are aimed at putting in-core basics for the concept I call password protocol aging, which is a way to allow multiple password protocols to be defined in Postgres, and aimed at easing administration as well as retirement of outdated protocols, which is something that is not doable now in Postgres. The second set of patch 0005~0008 introduces a new protocol, SCRAM. 9) 0009 is the SCRAM authentication itself The theme with password checking is interesting for me, and I can give review for CF for some features. I think that review of all suggested features will require a lot of time. Is it possible to make subset of patches concerning only password strength and its aging? The patches you have applied are non-independent. They should be apply consequentially one by one. Thus the patch 0009 can't be applied without git error before 0001. In this conditions all patches were successfully applied and compiled. All tests successfully passed. If you want to focus on the password protocol aging, you could just have a look at 0001~0004. OK, I will review patches 0001-0004, for starting. Below are the results of compiling and testing. I've got the last version of sources from git://git.postgresql.org/git/postgresql.git. vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git branch * master Then I've applied patches 0001-0004 with two warnings: vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 0001-Add-facility-to-store-multiple-password-verifiers.patch 0001-Add-facility-to-store-multiple-password-verifiers.patch:2547: trailing whitespace. warning: 1 line adds whitespace errors. vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 0002-Introduce-password_protocols.patch vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 0003-Add-pg_auth_verifiers_sanitize.patch 0003-Add-pg_auth_verifiers_sanitize.patch:87: indent with spaces. if (!superuser()) warning: 1 line adds whitespace errors. vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 0004-Remove-password-verifiers-for-unsupported-protocols-.patch The compilation with option ./configure --enable-debug --enable-nls --enable-cassert --enable-tap-tests --with-perl was successful. Regression tests and all TAP-tests also passed successfully. Also I've applied patches 0005-0008 into clean sources directory with no warnings. vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 0005-Move-sha1.c-to-src-common.patch vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 0006-Refactor-sendAuthRequest.patch vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 0007-Refactor-RandomSalt-to-handle-salts-of-different-len.patch vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 0008-Move-encoding-routines-to-src-common.patch The compilation with option ./configure --enable-debug --enable-nls --enable-cassert --enable-tap-tests --with-perl was successful. Regression and the TAP-tests also passed successfully. The patch 0009 depends on all previous patches 0001-0008: first we need to apply patches 0001-0008, then 0009. Then, all patches were successfully compiled. All test passed. -- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Password identifiers, protocol aging and SCRAM protocol
26.02.2016 01:10, Michael Paquier пишет: On Fri, Feb 26, 2016 at 1:38 AM, Valery Popov wrote: Hi, Michael 23.02.2016 10:17, Michael Paquier пишет: Attached is a set of patches implementing a couple of things that have been discussed, so let's roll in. Those 4 patches are aimed at putting in-core basics for the concept I call password protocol aging, which is a way to allow multiple password protocols to be defined in Postgres, and aimed at easing administration as well as retirement of outdated protocols, which is something that is not doable now in Postgres. The second set of patch 0005~0008 introduces a new protocol, SCRAM. 9) 0009 is the SCRAM authentication itself The theme with password checking is interesting for me, and I can give review for CF for some features. I think that review of all suggested features will require a lot of time. Is it possible to make subset of patches concerning only password strength and its aging? The patches you have applied are non-independent. They should be apply consequentially one by one. Thus the patch 0009 can't be applied without git error before 0001. In this conditions all patches were successfully applied and compiled. All tests successfully passed. If you want to focus on the password protocol aging, you could just have a look at 0001~0004. OK, I will review patches 0001-0004, for starting. -- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Password identifiers, protocol aging and SCRAM protocol
Hi, Michael 23.02.2016 10:17, Michael Paquier пишет: Attached is a set of patches implementing a couple of things that have been discussed, so let's roll in. Those 4 patches are aimed at putting in-core basics for the concept I call password protocol aging, which is a way to allow multiple password protocols to be defined in Postgres, and aimed at easing administration as well as retirement of outdated protocols, which is something that is not doable now in Postgres. The second set of patch 0005~0008 introduces a new protocol, SCRAM. 9) 0009 is the SCRAM authentication itself The theme with password checking is interesting for me, and I can give review for CF for some features. I think that review of all suggested features will require a lot of time. Is it possible to make subset of patches concerning only password strength and its aging? The patches you have applied are non-independent. They should be apply consequentially one by one. Thus the patch 0009 can't be applied without git error before 0001. In this conditions all patches were successfully applied and compiled. All tests successfully passed. The first 4 patches obviously are the core portion that I would like to discuss about in this CF, as they put in the base for the rest, and will surely help Postgres long-term. 0005~0008 are just refactoring patches, so they are quite simple. 0009 though is quite difficult, and needs careful review because it manipulates areas of the code where it is not necessary to be an authenticated user, so if there are bugs in it it would be possible for example to crash down Postgres just by sending authentication requests. -- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PROPOSAL] Max recursion depth in WITH Queries (Common Table Expressions)
28.10.2015 16:33, Tom Lane пишет: Valery Popov writes: Recursive queries are typically used to deal with hierarchical or tree-structured data. In some conditions when data contain relationships with cycles recursive query will loop unlimited and significantly slows the client's session. The standard way of dealing with that is to include logic in the query to limit the recursion depth, for example WITH RECURSIVE t(n) AS ( SELECT 1 UNION ALL SELECT n+1 FROM t WHERE n < 10 ) SELECT n FROM t; Yes, I agree with this thesis. But I think in some cases would be better to receive error message and stop execution than results will incomplete. -- Regards, Valery Popov Postgres Professional http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PROPOSAL] Max recursion depth in WITH Queries (Common Table Expressions)
Hi, Hackers
Recursive queries are typically used to deal with hierarchical or
tree-structured data.
In some conditions when data contain relationships with cycles recursive query
will loop
unlimited and significantly slows the client's session.
To prevent "infinite" loop I suggest the max_recursion_depth parameter,
which defines the maximum recursion level during the execution of recursive
query.
When max_recursion_depth > 0 and the recursion level of query exceeds
specified value then the execution of query interrupts with error message.
In the MS SQL Server there is MAXRECURSION hint for the same purpose.
Thanks!
--
Regards,
Valery Popov
Postgres Professional http://www.postgrespro.com
The Russian Postgres Company
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 1da7dfb..33a6009 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6048,6 +6048,23 @@ SET XML OPTION { DOCUMENT | CONTENT };
+
+ max_recursion_depth (integer)
+
+ max_recursion_depth configuration
parameter
+
+
+
+
+Sets the maximum recursion depth in WITH Queries (Common
Table Expressions).
+The default value is 0 and it means no limit for recursion depth
(infinite loop is possible).
+When max_recursion_depth > 0 and the recursion level of
query exceeds specified value
+ then execution of query interrupts with error message.
+ See for more information.
+
+
+
+
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index ab49bd7..80a63c8 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -2216,6 +2216,46 @@ SELECT n FROM t LIMIT 100;
In each case it effectively provides temporary table(s) that can
be referred to in the main command.
+
+
+
+Also it is possible to limit the number of returning rows by setting
parameter
+max_recursion_depth in the postgresql.conf file.
+See for more information.
+
+
+
+
+ Another way to set max_recursion_depth is SET
max_recursion_depth command in psql.
+
+
+postgres=# show max_recursion_depth;
+LOG: statement: show max_recursion_depth;
+ max_recursion_depth
+-
+ 0
+(1 row)
+
+postgres=# set max_recursion_depth = 5;
+LOG: statement: set max_recursion_depth = 5;
+SET
+postgres=# show max_recursion_depth;
+LOG: statement: show max_recursion_depth;
+ max_recursion_depth
+-
+ 5
+(1 row)
+
+ When max_recursion_depth > 0 and the recursion level of query
exceeds specified value
+ then execution of query interrupts with error message like this:
+
+
+ ERROR: The statement terminated. The maximum recursion depth 5 has been
exhausted before statement completion.
+
+
+
+
+
diff --git a/src/backend/executor/nodeRecursiveunion.c
b/src/backend/executor/nodeRecursiveunion.c
index 8df1639..c616250 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -110,6 +110,13 @@ ExecRecursiveUnion(RecursiveUnionState *node)
/* 2. Execute recursive term */
for (;;)
{
+ if ((with_recursive_limit > 0) && (node->ps.recursion_cnt >=
with_recursive_limit))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg("The statement terminated. The maximum
recursion depth %d has been exhausted before statement completion.",
with_recursive_limit)));
+ break;
+ }
slot = ExecProcNode(innerPlan);
if (TupIsNull(slot))
{
@@ -132,6 +139,9 @@ ExecRecursiveUnion(RecursiveUnionState *node)
innerPlan->chgParam =
bms_add_member(innerPlan->chgParam,
plan->wtParam);
+ /* go to the next recursion level */
+ node->ps.recursion_cnt++;
+
/* and continue fetching from recursive term */
continue;
}
@@ -261,6 +271,11 @@ ExecInitRecursiveUnion(RecursiveUnion *node, EState
*estate, int eflags)
build_hash_table(rustate);
}
+ /*
+* Init recursion depth counter.
+*/
+ rustate->ps.recursion_cnt=0;
+
return rustate;
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 230c5cc..c39e8ca 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -112,6 +112,9 @@ extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
extern bool synchronize_seqscans;
+/* Parameters for controlling recursive depth */
+intwith_rec
