public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
5+ messages / 5 participants
[nested] [flat]

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Proposal: Support custom authentication methods using hooks
@ 2022-02-17 19:25  samay sharma <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread

From: samay sharma @ 2022-02-17 19:25 UTC (permalink / raw)
  To: [email protected]

Hi all,

I wanted to submit a patch to expose 2 new hooks (one for the
authentication check and another one for error reporting) in auth.c. These
will allow users to implement their own authentication methods for Postgres
or add custom logic around authentication.

A use case where this is useful are environments where you want
authentication to be centrally managed across different services. This is a
common deployment model for cloud providers where customers like to use
single sign on and authenticate across different services including
Postgres. Implementing this now is tricky as it requires syncing that
authentication method's credentials with Postgres (and that gets trickier
with TTL/expiry etc.). With these hooks, you can implement an extension to
check credentials directly using the authentication provider's APIs.

To enable this, I've proposed adding a new authentication method "custom"
which can be specified in pg_hba.conf and takes a mandatory argument
"provider" specifying which authentication provider to use. I've also moved
a couple static functions to headers so that extensions can call them.

Sample pg_hba.conf line to use a custom provider:

host    all             all             ::1/128                 custom
provider=test


As an example and a test case, I've added an extension named
test_auth_provider in src/test/modules which fetches credentials from
a pre-defined array. I've also added tap tests for the extension to test
this functionality.


One constraint in the current implementation is that we allow only one
authentication provider to be loaded at a time. In the future, we can add
more functionality to maintain an array of hooks and call the appropriate
one based on the provider name in the pg_hba line.


A couple of my tests are flaky and sometimes fail in CI. I think the reason
for that is I don't wait for pg_hba reload to be processed before checking
logs for error messages. I didn't find an immediate way to address that and
I'm looking into it but wanted to get an initial version out for
feedback on the approach taken and interfaces. Once those get finalized, I
can submit a patch to add docs as well.


Looking forward to your feedback.


Regards,

Samay


Attachments:

  [application/octet-stream] 0001-Add-support-for-custom-authentication-methods.patch (10.7K, ../../CAJxrbyxTRn5P8J-p+wHLwFahK5y56PhK28VOb55jqMO05Y-DJw@mail.gmail.com/3-0001-Add-support-for-custom-authentication-methods.patch)
  download | inline diff:
From f838dd377c30778f530ec19bc004cba16a80d8e8 Mon Sep 17 00:00:00 2001
From: Samay Sharma <[email protected]>
Date: Tue, 15 Feb 2022 22:23:29 -0800
Subject: [PATCH 1/3] Add support for custom authentication methods

Currently, PostgreSQL supports only a set of pre-defined authentication
methods. This patch adds support for 2 hooks which allow users to add
their custom authentication methods by defining a check function and an
error function. Users can then use these methods by using a new "custom"
keyword in pg_hba.conf and specifying the authentication provider they
want to use.
---
 src/backend/libpq/auth.c | 85 ++++++++++++++++++++++++++++++----------
 src/backend/libpq/hba.c  | 36 +++++++++++++++++
 src/include/libpq/auth.h | 27 +++++++++++++
 src/include/libpq/hba.h  |  4 ++
 4 files changed, 131 insertions(+), 21 deletions(-)

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index efc53f3135..2e3d02b35a 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -47,8 +47,6 @@
  *----------------------------------------------------------------
  */
 static void auth_failed(Port *port, int status, const char *logdetail);
-static char *recv_password_packet(Port *port);
-static void set_authn_id(Port *port, const char *id);
 
 
 /*----------------------------------------------------------------
@@ -206,23 +204,6 @@ static int	pg_SSPI_make_upn(char *accountname,
 static int	CheckRADIUSAuth(Port *port);
 static int	PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd);
 
-
-/*
- * Maximum accepted size of GSS and SSPI authentication tokens.
- * We also use this as a limit on ordinary password packet lengths.
- *
- * Kerberos tickets are usually quite small, but the TGTs issued by Windows
- * domain controllers include an authorization field known as the Privilege
- * Attribute Certificate (PAC), which contains the user's Windows permissions
- * (group memberships etc.). The PAC is copied into all tickets obtained on
- * the basis of this TGT (even those issued by Unix realms which the Windows
- * realm trusts), and can be several kB in size. The maximum token size
- * accepted by Windows systems is determined by the MaxAuthToken Windows
- * registry setting. Microsoft recommends that it is not set higher than
- * 65535 bytes, so that seems like a reasonable limit for us as well.
- */
-#define PG_MAX_AUTH_TOKEN_LENGTH	65535
-
 /*----------------------------------------------------------------
  * Global authentication functions
  *----------------------------------------------------------------
@@ -235,6 +216,16 @@ static int	PerformRadiusTransaction(const char *server, const char *secret, cons
  */
 ClientAuthentication_hook_type ClientAuthentication_hook = NULL;
 
+/*
+ * These hooks allow plugins to get control of the client authentication check
+ * and error reporting logic. This allows users to write extensions to
+ * implement authentication using any protocol of their choice. To acquire these
+ * hooks, plugins need to call the RegisterAuthProvider() function.
+ */
+static CustomAuthenticationCheck_hook_type CustomAuthenticationCheck_hook = NULL;
+static CustomAuthenticationError_hook_type CustomAuthenticationError_hook = NULL;
+char *custom_provider_name = NULL;
+
 /*
  * Tell the user the authentication failed, but not (much about) why.
  *
@@ -311,6 +302,12 @@ auth_failed(Port *port, int status, const char *logdetail)
 		case uaRADIUS:
 			errstr = gettext_noop("RADIUS authentication failed for user \"%s\"");
 			break;
+		case uaCustom:
+			if (CustomAuthenticationError_hook)
+				errstr = CustomAuthenticationError_hook(port);
+			else
+				errstr = gettext_noop("Custom authentication failed for user \"%s\"");
+			break;
 		default:
 			errstr = gettext_noop("authentication failed for user \"%s\": invalid authentication method");
 			break;
@@ -345,7 +342,7 @@ auth_failed(Port *port, int status, const char *logdetail)
  * lifetime of the Port, so it is safe to pass a string that is managed by an
  * external library.
  */
-static void
+void
 set_authn_id(Port *port, const char *id)
 {
 	Assert(id);
@@ -630,6 +627,10 @@ ClientAuthentication(Port *port)
 		case uaTrust:
 			status = STATUS_OK;
 			break;
+		case uaCustom:
+			if (CustomAuthenticationCheck_hook)
+				status = CustomAuthenticationCheck_hook(port);
+			break;
 	}
 
 	if ((status == STATUS_OK && port->hba->clientcert == clientCertFull)
@@ -689,7 +690,7 @@ sendAuthRequest(Port *port, AuthRequest areq, const char *extradata, int extrale
  *
  * Returns NULL if couldn't get password, else palloc'd string.
  */
-static char *
+char *
 recv_password_packet(Port *port)
 {
 	StringInfoData buf;
@@ -3343,3 +3344,45 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		}
 	}							/* while (true) */
 }
+
+/*----------------------------------------------------------------
+ * Custom authentication
+ *----------------------------------------------------------------
+ */
+
+/*
+ * RegisterAuthProvider registers a custom authentication provider to be
+ * used for authentication. Currently, we allow only one authentication
+ * provider to be registered for use at a time.
+ *
+ * This function should be called in _PG_init() by any extension looking to
+ * add a custom authentication method.
+ */
+void RegisterAuthProvider(const char *provider_name,
+		CustomAuthenticationCheck_hook_type AuthenticationCheckFunction,
+		CustomAuthenticationError_hook_type AuthenticationErrorFunction)
+{
+	if (provider_name == NULL)
+	{
+		ereport(ERROR,
+				(errmsg("cannot register authentication provider without name")));
+	}
+
+	if (AuthenticationCheckFunction == NULL)
+	{
+		ereport(ERROR,
+				(errmsg("cannot register authentication provider without a check function")));
+	}
+
+	if (custom_provider_name)
+	{
+		ereport(ERROR,
+				(errmsg("cannot register authentication provider %s", provider_name),
+				 errdetail("Only one authentication provider allowed.  Provider %s is already registered.",
+							custom_provider_name)));
+	}
+
+	custom_provider_name = pstrdup(provider_name);
+	CustomAuthenticationCheck_hook = AuthenticationCheckFunction;
+	CustomAuthenticationError_hook = AuthenticationErrorFunction;
+}
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index d84a40b726..956d7d6857 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -134,6 +134,7 @@ static const char *const UserAuthName[] =
 	"ldap",
 	"cert",
 	"radius",
+	"custom",
 	"peer"
 };
 
@@ -1399,6 +1400,8 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 #endif
 	else if (strcmp(token->string, "radius") == 0)
 		parsedline->auth_method = uaRADIUS;
+	else if (strcmp(token->string, "custom") == 0)
+		parsedline->auth_method = uaCustom;
 	else
 	{
 		ereport(elevel,
@@ -1691,6 +1694,14 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 		parsedline->clientcert = clientCertFull;
 	}
 
+	/*
+	 * Ensure that the provider name is specified for custom authentication method.
+	 */
+	if (parsedline->auth_method == uaCustom)
+	{
+		MANDATORY_AUTH_ARG(parsedline->custom_provider, "provider", "custom");
+	}
+
 	return parsedline;
 }
 
@@ -2102,6 +2113,31 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 		hbaline->radiusidentifiers = parsed_identifiers;
 		hbaline->radiusidentifiers_s = pstrdup(val);
 	}
+	else if (strcmp(name, "provider") == 0)
+	{
+		REQUIRE_AUTH_OPTION(uaCustom, "provider", "custom");
+
+		/*
+		 * Verify that the provider mentioned is same as the one loaded
+		 * via shared_preload_libraries.
+		 */
+
+		if (custom_provider_name == NULL || strcmp(val,custom_provider_name) != 0)
+		{
+			ereport(elevel,
+					(errcode(ERRCODE_CONFIG_FILE_ERROR),
+					 errmsg("cannot use authentication provider %s",val),
+					 errhint("Load authentication provider via shared_preload_libraries."),
+					 errcontext("line %d of configuration file \"%s\"",
+							line_num, HbaFileName)));
+
+			return false;
+		}
+		else
+		{
+			hbaline->custom_provider = pstrdup(val);
+		}
+	}
 	else
 	{
 		ereport(elevel,
diff --git a/src/include/libpq/auth.h b/src/include/libpq/auth.h
index 6d7ee1acb9..1d10cccc1b 100644
--- a/src/include/libpq/auth.h
+++ b/src/include/libpq/auth.h
@@ -23,9 +23,36 @@ extern char *pg_krb_realm;
 extern void ClientAuthentication(Port *port);
 extern void sendAuthRequest(Port *port, AuthRequest areq, const char *extradata,
 							int extralen);
+extern void set_authn_id(Port *port, const char *id);
+extern char *recv_password_packet(Port *port);
 
 /* Hook for plugins to get control in ClientAuthentication() */
+typedef int (*CustomAuthenticationCheck_hook_type) (Port *);
 typedef void (*ClientAuthentication_hook_type) (Port *, int);
 extern PGDLLIMPORT ClientAuthentication_hook_type ClientAuthentication_hook;
 
+/* Hook for plugins to report error messages in auth_failed() */
+typedef const char * (*CustomAuthenticationError_hook_type) (Port *);
+
+extern void RegisterAuthProvider
+		(const char *provider_name,
+		 CustomAuthenticationCheck_hook_type CustomAuthenticationCheck_hook,
+		 CustomAuthenticationError_hook_type CustomAuthenticationError_hook);
+
+/*
+ * Maximum accepted size of GSS and SSPI authentication tokens.
+ * We also use this as a limit on ordinary password packet lengths.
+ *
+ * Kerberos tickets are usually quite small, but the TGTs issued by Windows
+ * domain controllers include an authorization field known as the Privilege
+ * Attribute Certificate (PAC), which contains the user's Windows permissions
+ * (group memberships etc.). The PAC is copied into all tickets obtained on
+ * the basis of this TGT (even those issued by Unix realms which the Windows
+ * realm trusts), and can be several kB in size. The maximum token size
+ * accepted by Windows systems is determined by the MaxAuthToken Windows
+ * registry setting. Microsoft recommends that it is not set higher than
+ * 65535 bytes, so that seems like a reasonable limit for us as well.
+ */
+#define PG_MAX_AUTH_TOKEN_LENGTH	65535
+
 #endif							/* AUTH_H */
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 8d9f3821b1..c5aef6994c 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -38,6 +38,7 @@ typedef enum UserAuth
 	uaLDAP,
 	uaCert,
 	uaRADIUS,
+	uaCustom,
 	uaPeer
 #define USER_AUTH_LAST uaPeer	/* Must be last value of this enum */
 } UserAuth;
@@ -120,6 +121,7 @@ typedef struct HbaLine
 	char	   *radiusidentifiers_s;
 	List	   *radiusports;
 	char	   *radiusports_s;
+	char	   *custom_provider;
 } HbaLine;
 
 typedef struct IdentLine
@@ -144,4 +146,6 @@ extern int	check_usermap(const char *usermap_name,
 						  bool case_sensitive);
 extern bool pg_isblank(const char c);
 
+extern char *custom_provider_name;
+
 #endif							/* HBA_H */
-- 
2.34.1



  [application/octet-stream] 0002-Add-sample-extension-to-test-custom-auth-provider-ho.patch (3.6K, ../../CAJxrbyxTRn5P8J-p+wHLwFahK5y56PhK28VOb55jqMO05Y-DJw@mail.gmail.com/4-0002-Add-sample-extension-to-test-custom-auth-provider-ho.patch)
  download | inline diff:
From 46d00b74cee2e6c312ece90b9add91bb0f4ac8e8 Mon Sep 17 00:00:00 2001
From: Samay Sharma <[email protected]>
Date: Tue, 15 Feb 2022 22:28:40 -0800
Subject: [PATCH 2/3] Add sample extension to test custom auth provider hooks

This change adds a new extension to src/test/modules to
test the custom authentication provider hooks. In this
extension, we use an array to define which users to
authenticate and what passwords to use.
---
 src/test/modules/test_auth_provider/Makefile  | 16 ++++
 .../test_auth_provider/test_auth_provider.c   | 90 +++++++++++++++++++
 2 files changed, 106 insertions(+)
 create mode 100644 src/test/modules/test_auth_provider/Makefile
 create mode 100644 src/test/modules/test_auth_provider/test_auth_provider.c

diff --git a/src/test/modules/test_auth_provider/Makefile b/src/test/modules/test_auth_provider/Makefile
new file mode 100644
index 0000000000..17971a5c7a
--- /dev/null
+++ b/src/test/modules/test_auth_provider/Makefile
@@ -0,0 +1,16 @@
+# src/test/modules/test_auth_provider/Makefile
+
+MODULE_big = test_auth_provider
+OBJS = test_auth_provider.o
+PGFILEDESC = "test_auth_provider - provider to test auth hooks"
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_auth_provider
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_auth_provider/test_auth_provider.c b/src/test/modules/test_auth_provider/test_auth_provider.c
new file mode 100644
index 0000000000..477ef8b2c3
--- /dev/null
+++ b/src/test/modules/test_auth_provider/test_auth_provider.c
@@ -0,0 +1,90 @@
+/* -------------------------------------------------------------------------
+ *
+ * test_auth_provider.c
+ *			example authentication provider plugin		
+ *
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		contrib/test_auth_provider/test_auth_provider.c
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+#include "fmgr.h"
+#include "libpq/auth.h"
+#include "libpq/libpq.h"
+
+PG_MODULE_MAGIC;
+
+void _PG_init(void);
+
+static char *get_password_for_user(char *user_name);
+
+/*
+ * List of usernames / passwords to approve. Here we are not
+ * getting passwords from Postgres but from this list. In a more real-life
+ * extension, you can fetch valid credentials and authentication tokens /
+ * passwords from an external authentication provider.
+ */
+char credentials[3][3][50] = {
+	{"bob","alice","carol"},
+	{"bob123","alice123","carol123"}
+};
+
+static int TestAuthenticationCheck(Port *port)
+{
+	char *passwd;
+	int result = STATUS_ERROR;
+	char *real_pass;
+
+	sendAuthRequest(port, AUTH_REQ_PASSWORD, NULL, 0);
+
+	passwd = recv_password_packet(port);
+	if (passwd == NULL)
+		return STATUS_EOF;
+		
+	real_pass = get_password_for_user(port->user_name);
+	if (real_pass)
+	{
+		if(strcmp(passwd, real_pass) == 0)
+		{
+			result = STATUS_OK;
+		}
+		pfree(real_pass);
+	}
+
+	pfree(passwd);
+
+	return result;
+}
+
+static char *
+get_password_for_user(char *user_name)
+{
+	char *password = NULL;
+	int i;
+	for (i=0; i<3; i++)
+	{
+		if (strcmp(user_name, credentials[0][i]) == 0)
+		{
+			password = pstrdup(credentials[1][i]);
+		}
+	}
+
+	return password;
+}
+
+static const char *TestAuthenticationError(Port *port)
+{
+	char *error_message = (char *)palloc (100);
+	sprintf(error_message, "Test authentication failed for user %s", port->user_name);
+	return error_message;
+}
+
+void
+_PG_init(void)
+{
+	RegisterAuthProvider("test", TestAuthenticationCheck, TestAuthenticationError);
+}
-- 
2.34.1



  [application/octet-stream] 0003-Add-tests-for-test_auth_provider-extension.patch (6.4K, ../../CAJxrbyxTRn5P8J-p+wHLwFahK5y56PhK28VOb55jqMO05Y-DJw@mail.gmail.com/5-0003-Add-tests-for-test_auth_provider-extension.patch)
  download | inline diff:
From 65ecda4827dfc139af2cc85a050e9f4e31ad0acf Mon Sep 17 00:00:00 2001
From: Samay Sharma <[email protected]>
Date: Wed, 16 Feb 2022 12:28:36 -0800
Subject: [PATCH 3/3] Add tests for test_auth_provider extension

Add tap tests for test_auth_provider extension allow make check in
src/test/modules to run them.
---
 src/test/modules/Makefile                     |   1 +
 src/test/modules/test_auth_provider/Makefile  |   2 +
 .../test_auth_provider/t/001_custom_auth.pl   | 134 ++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 src/test/modules/test_auth_provider/t/001_custom_auth.pl

diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index dffc79b2d9..f56533ea13 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -14,6 +14,7 @@ SUBDIRS = \
 		  plsample \
 		  snapshot_too_old \
 		  spgist_name_ops \
+		  test_auth_provider \
 		  test_bloomfilter \
 		  test_ddl_deparse \
 		  test_extensions \
diff --git a/src/test/modules/test_auth_provider/Makefile b/src/test/modules/test_auth_provider/Makefile
index 17971a5c7a..7d601cf7d5 100644
--- a/src/test/modules/test_auth_provider/Makefile
+++ b/src/test/modules/test_auth_provider/Makefile
@@ -4,6 +4,8 @@ MODULE_big = test_auth_provider
 OBJS = test_auth_provider.o
 PGFILEDESC = "test_auth_provider - provider to test auth hooks"
 
+TAP_TESTS = 1
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/src/test/modules/test_auth_provider/t/001_custom_auth.pl b/src/test/modules/test_auth_provider/t/001_custom_auth.pl
new file mode 100644
index 0000000000..4eb0cdf43e
--- /dev/null
+++ b/src/test/modules/test_auth_provider/t/001_custom_auth.pl
@@ -0,0 +1,134 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+# Set of tests for testing custom authentication.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Delete pg_hba.conf from the given node, add a new entry to it
+# and then execute a reload to refresh it.
+sub reset_pg_hba
+{
+	my $node       = shift;
+	my $hba_method = shift;
+
+	unlink($node->data_dir . '/pg_hba.conf');
+	# just for testing purposes, use a continuation line
+	$node->append_conf('pg_hba.conf', "local all all\\\n $hba_method");
+	$node->reload;
+	return;
+}
+
+# Test that you get expected output after making a change to hba.conf
+# and reloading it.
+sub test_hba_reload
+{
+	my ($node,$method,$expected_res,$log_message) = @_;
+	my $status_string = 'failed';
+	$status_string = 'success' if ($expected_res eq 0);
+	my $testname = "pg_hba.conf reload $status_string for method $method";
+
+	my $log_location = -s $node->logfile;
+	
+	reset_pg_hba($node,$method);
+
+	my $log_contents = slurp_file($node->logfile, $log_location);
+
+	# Search for specific error message if it's a failure.
+	# For success, just confirm that there was no error message.
+	if ($expected_res eq 1)
+	{
+		like($log_contents,$log_message,$testname);
+	}
+	else
+	{
+		unlike($log_contents,$log_message,$testname);
+	}
+}
+
+# Test access for a single role, useful to wrap all tests into one.  Extra
+# named parameters are passed to connect_ok/fails as-is.
+sub test_role
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+	my ($node, $role, $method, $expected_res, %params) = @_;
+	my $status_string = 'failed';
+	$status_string = 'success' if ($expected_res eq 0);
+
+	my $connstr = "user=$role";
+	my $testname =
+	  "authentication $status_string for method $method, role $role";
+
+	if ($expected_res eq 0)
+	{
+		$node->connect_ok($connstr, $testname, %params);
+	}
+	else
+	{
+		# No checks of the error message, only the status code.
+		$node->connect_fails($connstr, $testname, %params);
+	}
+}
+
+# Initialize server node
+my $node = PostgreSQL::Test::Cluster->new('server');
+$node->init;
+$node->append_conf('postgresql.conf', "log_connections = on\n");
+$node->append_conf('postgresql.conf', "shared_preload_libraries = 'test_auth_provider.so'\n");
+$node->start;
+
+$node->safe_psql('postgres',"CREATE ROLE bob LOGIN;");
+$node->safe_psql('postgres',"CREATE ROLE alice LOGIN;");
+$node->safe_psql('postgres',"CREATE ROLE test LOGIN;");
+
+# Add custom auth method to pg_hba.conf
+reset_pg_hba($node, 'custom provider=test');
+
+# Test that users are able to login with correct passwords.
+$ENV{"PGPASSWORD"} = 'bob123';
+test_role($node, 'bob','custom', 0, log_like => [qr/connection authorized: user=bob/]);
+$ENV{"PGPASSWORD"} = 'alice123';
+test_role($node, 'alice','custom', 0, log_like => [qr/connection authorized: user=alice/]);
+
+# Test that bad passwords are rejected. 
+$ENV{"PGPASSWORD"} = 'badpassword';
+test_role($node, 'bob','custom', 2, log_unlike => [qr/connection authorized:/]);
+test_role($node, 'alice','custom', 2, log_unlike => [qr/connection authorized:/]);
+
+# Test that users not in authentication list are rejected.
+test_role($node, 'test','custom', 2, log_unlike => [qr/connection authorized:/]);
+
+# Tests for invalid auth options
+
+# Test that an incorrect provider name is not accepted.
+test_hba_reload($node, 'custom provider=wrong', 1, qr/cannot use authentication provider wrong/);
+
+# Test that specifying provider option with different auth method is not allowed.
+test_hba_reload($node, 'trust provider=test', 1, qr/only valid for authentication methods custom/);
+
+# Test that provider name is a mandatory option for custom auth.
+test_hba_reload($node, 'custom', 1, qr/requires argument/);
+
+# Test that correct provider name allows reload to succeed.
+test_hba_reload($node, 'custom provider=test', 0, qr/pg_hba.conf was not reloaded/);
+
+# Custom auth modules require mentioning extension in shared_preload_libraries.
+
+# Remove extension from shared_preload_libraries and try to restart.
+$node->adjust_conf('postgresql.conf','shared_preload_libraries',"''");
+command_fails(['pg_ctl', '-w', '-D', $node->data_dir, '-l', $node->logfile, 'restart'],'restart with empty shared_preload_libraries failed');
+
+# Fix shared_preload_libraries and confirm that you can now restart.
+$node->adjust_conf('postgresql.conf','shared_preload_libraries',"'test_auth_provider.so'");
+command_ok(['pg_ctl', '-w', '-D', $node->data_dir, '-l', $node->logfile,'start'],'restart with correct shared_preload_libraries succeeded');
+
+# Test that we can connect again
+$ENV{"PGPASSWORD"} = 'bob123';
+test_role($node, 'bob', 'custom', 0, log_like => [qr/connection authorized: user=bob/]);
+
+done_testing();
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
@ 2022-02-28 10:26  Peter Eisentraut <[email protected]>
  parent: samay sharma <[email protected]>
  0 siblings, 2 replies; 5+ messages in thread

From: Peter Eisentraut @ 2022-02-28 10:26 UTC (permalink / raw)
  To: samay sharma <[email protected]>; [email protected]

On 17.02.22 20:25, samay sharma wrote:
> A use case where this is useful are environments where you want 
> authentication to be centrally managed across different services. This 
> is a common deployment model for cloud providers where customers like to 
> use single sign on and authenticate across different services including 
> Postgres. Implementing this now is tricky as it requires syncing that 
> authentication method's credentials with Postgres (and that gets 
> trickier with TTL/expiry etc.). With these hooks, you can implement an 
> extension to check credentials directly using the 
> authentication provider's APIs.

We already have a variety of authentication mechanisms that support 
central management: LDAP, PAM, Kerberos, Radius.  What other mechanisms 
are people thinking about implementing using these hooks?  Maybe there 
are a bunch of them, in which case a hook system might be sensible, but 
if there are only one or two plausible ones, we could also just make 
them built in.







^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
@ 2022-03-01 21:18  Jonathan S. Katz <[email protected]>
  parent: Peter Eisentraut <[email protected]>
  1 sibling, 0 replies; 5+ messages in thread

From: Jonathan S. Katz @ 2022-03-01 21:18 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; samay sharma <[email protected]>; [email protected]

On 2/28/22 5:26 AM, Peter Eisentraut wrote:
> On 17.02.22 20:25, samay sharma wrote:
>> A use case where this is useful are environments where you want 
>> authentication to be centrally managed across different services. This 
>> is a common deployment model for cloud providers where customers like 
>> to use single sign on and authenticate across different services 
>> including Postgres. Implementing this now is tricky as it requires 
>> syncing that authentication method's credentials with Postgres (and 
>> that gets trickier with TTL/expiry etc.). With these hooks, you can 
>> implement an extension to check credentials directly using the 
>> authentication provider's APIs.
> 
> We already have a variety of authentication mechanisms that support 
> central management: LDAP, PAM, Kerberos, Radius.  What other mechanisms 
> are people thinking about implementing using these hooks?  Maybe there 
> are a bunch of them, in which case a hook system might be sensible, but 
> if there are only one or two plausible ones, we could also just make 
> them built in.

OIDC is the big one that comes to mind.

Jonathan


Attachments:

  [application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
@ 2022-03-01 21:34  Andres Freund <[email protected]>
  parent: Peter Eisentraut <[email protected]>
  1 sibling, 0 replies; 5+ messages in thread

From: Andres Freund @ 2022-03-01 21:34 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: samay sharma <[email protected]>; [email protected]

Hi,

On 2022-02-28 11:26:06 +0100, Peter Eisentraut wrote:
> We already have a variety of authentication mechanisms that support central
> management: LDAP, PAM, Kerberos, Radius.

LDAP, PAM and Radius all require cleartext passwords, so aren't a great
solution based on the concerns voiced in this thread. IME Kerberos is
operationally too complicated to really be used, unless it's already part of
the operating environment.


> What other mechanisms are people thinking about implementing using these
> hooks?

The cases I've heard about are about centralizing auth across multiple cloud
services. Including secret management in some form. E.g. allowing an
application to auth to postgres, redis and having the secret provided by
infrastructure, rather than having to hardcode it in config or such.

I can't see application developers configuring kerberos and I don't think
LDAP, PAM, Radius are a great answer either, due to the plaintext requirement
alone? LDAP is pretty clearly dying technology, PAM is fragile complicated C
stuff that's not portable across OSs. Radius is probably the most realistic,
but at least as implemented doesn't seem flexible enough (e.g. no access to
group memberships etc).

Nor does baking stuff like that in seem realistic to me, it'll presumably be
too cloud provider specific.

Greetings,

Andres Freund






^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2022-03-01 21:34 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2022-02-17 19:25 Proposal: Support custom authentication methods using hooks samay sharma <[email protected]>
2022-02-28 10:26 ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
2022-03-01 21:18   ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
2022-03-01 21:34   ` Re: Proposal: Support custom authentication methods using hooks Andres Freund <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox