public inbox for [email protected]
help / color / mirror / Atom feedKerberos delegation support in libpq and postgres_fdw
24+ messages / 8 participants
[nested] [flat]
* Kerberos delegation support in libpq and postgres_fdw
@ 2021-07-20 03:05 Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Peifeng Qiu @ 2021-07-20 03:05 UTC (permalink / raw)
To: [email protected] <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Tom Lane <[email protected]>
Hi hackers.
This is the patch to add kerberos delegation support in libpq, which
enables postgres_fdw to connect to another server and authenticate
as the same user to the current login user. This will obsolete my
previous patch which requires keytab file to be present on the fdw
server host.
After the backend accepts the gssapi context, it may also get a
proxy credential if permitted by policy. I previously made a hack
to pass the pointer of proxy credential directly into libpq. It turns
out that the correct way to do this is store/acquire using credential
cache within local process memory to prevent leak.
Because no password is needed when querying foreign table via
kerberos delegation, the "password_required" option in user
mapping must be set to false by a superuser. Other than this, it
should work with normal user.
I only tested it manually in a very simple configuration currently.
I will go on to work with TAP tests for this.
How do you feel about this patch? Any feature/security concerns
about this?
Best regards,
Peifeng Qiu
Attachments:
[text/x-patch] v1-0001-kerberos-delegation.patch (5.3K, ../../CO1PR05MB8023CC2CB575E0FAAD7DF4F8A8E29@CO1PR05MB8023.namprd05.prod.outlook.com/3-v1-0001-kerberos-delegation.patch)
download | inline diff:
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 8cc23ef7fb..235c9b32f5 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -918,6 +918,7 @@ pg_GSS_recvauth(Port *port)
int mtype;
StringInfoData buf;
gss_buffer_desc gbuf;
+ gss_cred_id_t proxy;
/*
* Use the configured keytab, if there is one. Unfortunately, Heimdal
@@ -947,6 +948,8 @@ pg_GSS_recvauth(Port *port)
*/
port->gss->ctx = GSS_C_NO_CONTEXT;
+ proxy = NULL;
+
/*
* Loop through GSSAPI message exchange. This exchange can consist of
* multiple messages sent in both directions. First message is always from
@@ -997,7 +1000,7 @@ pg_GSS_recvauth(Port *port)
&port->gss->outbuf,
&gflags,
NULL,
- NULL);
+ &proxy);
/* gbuf no longer used */
pfree(buf.data);
@@ -1009,6 +1012,9 @@ pg_GSS_recvauth(Port *port)
CHECK_FOR_INTERRUPTS();
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
if (port->gss->outbuf.length != 0)
{
/*
diff --git a/src/backend/libpq/be-gssapi-common.c b/src/backend/libpq/be-gssapi-common.c
index 38f58def25..cd243994c8 100644
--- a/src/backend/libpq/be-gssapi-common.c
+++ b/src/backend/libpq/be-gssapi-common.c
@@ -92,3 +92,40 @@ pg_GSS_error(const char *errmsg,
(errmsg_internal("%s", errmsg),
errdetail_internal("%s: %s", msg_major, msg_minor)));
}
+
+void
+pg_store_proxy_credential(gss_cred_id_t cred)
+{
+ OM_uint32 major, minor;
+ gss_OID_set mech;
+ gss_cred_usage_t usage;
+ gss_key_value_element_desc cc;
+ gss_key_value_set_desc ccset;
+
+ cc.key = "ccache";
+ cc.value = "MEMORY:";
+ ccset.count = 1;
+ ccset.elements = &cc;
+
+ /* Make the proxy credential only available to current process */
+ major = gss_store_cred_into(&minor,
+ cred,
+ GSS_C_INITIATE, /* credential only used for starting libpq connection */
+ GSS_C_NULL_OID, /* store all */
+ true, /* overwrite */
+ true, /* make default */
+ &ccset,
+ &mech,
+ &usage);
+
+
+ if (major != GSS_S_COMPLETE)
+ {
+ pg_GSS_error("gss_store_cred", major, minor);
+ }
+
+ /* quite strange that gss_store_cred doesn't work with "KRB5CCNAME=MEMORY:",
+ * we have to use gss_store_cred_into instead and set the env for later
+ * gss_acquire_cred calls. */
+ putenv("KRB5CCNAME=MEMORY:");
+}
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 316ca65db5..e27d517dea 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -497,6 +497,7 @@ secure_open_gssapi(Port *port)
bool complete_next = false;
OM_uint32 major,
minor;
+ gss_cred_id_t proxy;
/*
* Allocate subsidiary Port data for GSSAPI operations.
@@ -588,7 +589,8 @@ secure_open_gssapi(Port *port)
GSS_C_NO_CREDENTIAL, &input,
GSS_C_NO_CHANNEL_BINDINGS,
&port->gss->name, NULL, &output, NULL,
- NULL, NULL);
+ NULL, &proxy);
+
if (GSS_ERROR(major))
{
pg_GSS_error(_("could not accept GSSAPI security context"),
@@ -605,6 +607,9 @@ secure_open_gssapi(Port *port)
complete_next = true;
}
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
/* Done handling the incoming packet, reset our buffer */
PqGSSRecvLength = 0;
diff --git a/src/include/libpq/be-gssapi-common.h b/src/include/libpq/be-gssapi-common.h
index c07d7e7c5a..62d60ffbd8 100644
--- a/src/include/libpq/be-gssapi-common.h
+++ b/src/include/libpq/be-gssapi-common.h
@@ -18,9 +18,11 @@
#include <gssapi.h>
#else
#include <gssapi/gssapi.h>
+#include <gssapi/gssapi_ext.h>
#endif
extern void pg_GSS_error(const char *errmsg,
OM_uint32 maj_stat, OM_uint32 min_stat);
+extern void pg_store_proxy_credential(gss_cred_id_t cred);
#endif /* BE_GSSAPI_COMMON_H */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 3421ed4685..e0d342124e 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -62,6 +62,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
lmin_s;
gss_buffer_desc ginbuf;
gss_buffer_desc goutbuf;
+ gss_cred_id_t proxy;
/*
* On first call, there's no input token. On subsequent calls, read the
@@ -94,12 +95,16 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
ginbuf.value = NULL;
}
+ /* Check if we can aquire a proxy credential. */
+ if (!pg_GSS_have_cred_cache(&proxy))
+ proxy = GSS_C_NO_CREDENTIAL;
+
maj_stat = gss_init_sec_context(&min_stat,
- GSS_C_NO_CREDENTIAL,
+ proxy,
&conn->gctx,
conn->gtarg_nam,
GSS_C_NO_OID,
- GSS_C_MUTUAL_FLAG,
+ GSS_C_MUTUAL_FLAG | GSS_C_DELEG_FLAG,
0,
GSS_C_NO_CHANNEL_BINDINGS,
(ginbuf.value == NULL) ? GSS_C_NO_BUFFER : &ginbuf,
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index c783a53734..781af4227c 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -631,7 +631,7 @@ pqsecure_open_gss(PGconn *conn)
*/
major = gss_init_sec_context(&minor, conn->gcred, &conn->gctx,
conn->gtarg_nam, GSS_C_NO_OID,
- GSS_REQUIRED_FLAGS, 0, 0, &input, NULL,
+ GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, &input, NULL,
&output, NULL, NULL);
/* GSS Init Sec Context uses the whole packet, so clear it */
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
@ 2021-07-22 08:39 ` Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Peifeng Qiu @ 2021-07-22 08:39 UTC (permalink / raw)
To: [email protected] <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Tom Lane <[email protected]>
Hi all.
I've slightly modified the patch to support "gssencmode" and added TAP tests.
Best regards,
Peifeng Qiu
________________________________
From: Peifeng Qiu
Sent: Tuesday, July 20, 2021 11:05 AM
To: [email protected] <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Tom Lane <[email protected]>
Subject: Kerberos delegation support in libpq and postgres_fdw
Hi hackers.
This is the patch to add kerberos delegation support in libpq, which
enables postgres_fdw to connect to another server and authenticate
as the same user to the current login user. This will obsolete my
previous patch which requires keytab file to be present on the fdw
server host.
After the backend accepts the gssapi context, it may also get a
proxy credential if permitted by policy. I previously made a hack
to pass the pointer of proxy credential directly into libpq. It turns
out that the correct way to do this is store/acquire using credential
cache within local process memory to prevent leak.
Because no password is needed when querying foreign table via
kerberos delegation, the "password_required" option in user
mapping must be set to false by a superuser. Other than this, it
should work with normal user.
I only tested it manually in a very simple configuration currently.
I will go on to work with TAP tests for this.
How do you feel about this patch? Any feature/security concerns
about this?
Best regards,
Peifeng Qiu
Attachments:
[text/x-patch] v2-0001-kerberos-delegation.patch (5.9K, ../../CO1PR05MB80235F71EE030594A275AF5DA8E49@CO1PR05MB8023.namprd05.prod.outlook.com/3-v2-0001-kerberos-delegation.patch)
download | inline diff:
commit a413b020afb663b5f89722b480c1b453e4304a36
Author: Peifeng Qiu <[email protected]>
Date: Thu Jul 22 17:27:21 2021 +0900
kerberos delegation
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 4593cbc540..855f9af09c 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -247,6 +247,9 @@ InitPgFdwOptions(void)
{"sslcert", UserMappingRelationId, true},
{"sslkey", UserMappingRelationId, true},
+ /* gssencmode is also libpq option, same to above. */
+ {"gssencmode", UserMappingRelationId, true},
+
{NULL, InvalidOid, false}
};
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 8cc23ef7fb..235c9b32f5 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -918,6 +918,7 @@ pg_GSS_recvauth(Port *port)
int mtype;
StringInfoData buf;
gss_buffer_desc gbuf;
+ gss_cred_id_t proxy;
/*
* Use the configured keytab, if there is one. Unfortunately, Heimdal
@@ -947,6 +948,8 @@ pg_GSS_recvauth(Port *port)
*/
port->gss->ctx = GSS_C_NO_CONTEXT;
+ proxy = NULL;
+
/*
* Loop through GSSAPI message exchange. This exchange can consist of
* multiple messages sent in both directions. First message is always from
@@ -997,7 +1000,7 @@ pg_GSS_recvauth(Port *port)
&port->gss->outbuf,
&gflags,
NULL,
- NULL);
+ &proxy);
/* gbuf no longer used */
pfree(buf.data);
@@ -1009,6 +1012,9 @@ pg_GSS_recvauth(Port *port)
CHECK_FOR_INTERRUPTS();
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
if (port->gss->outbuf.length != 0)
{
/*
diff --git a/src/backend/libpq/be-gssapi-common.c b/src/backend/libpq/be-gssapi-common.c
index 38f58def25..cd243994c8 100644
--- a/src/backend/libpq/be-gssapi-common.c
+++ b/src/backend/libpq/be-gssapi-common.c
@@ -92,3 +92,40 @@ pg_GSS_error(const char *errmsg,
(errmsg_internal("%s", errmsg),
errdetail_internal("%s: %s", msg_major, msg_minor)));
}
+
+void
+pg_store_proxy_credential(gss_cred_id_t cred)
+{
+ OM_uint32 major, minor;
+ gss_OID_set mech;
+ gss_cred_usage_t usage;
+ gss_key_value_element_desc cc;
+ gss_key_value_set_desc ccset;
+
+ cc.key = "ccache";
+ cc.value = "MEMORY:";
+ ccset.count = 1;
+ ccset.elements = &cc;
+
+ /* Make the proxy credential only available to current process */
+ major = gss_store_cred_into(&minor,
+ cred,
+ GSS_C_INITIATE, /* credential only used for starting libpq connection */
+ GSS_C_NULL_OID, /* store all */
+ true, /* overwrite */
+ true, /* make default */
+ &ccset,
+ &mech,
+ &usage);
+
+
+ if (major != GSS_S_COMPLETE)
+ {
+ pg_GSS_error("gss_store_cred", major, minor);
+ }
+
+ /* quite strange that gss_store_cred doesn't work with "KRB5CCNAME=MEMORY:",
+ * we have to use gss_store_cred_into instead and set the env for later
+ * gss_acquire_cred calls. */
+ putenv("KRB5CCNAME=MEMORY:");
+}
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 316ca65db5..e27d517dea 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -497,6 +497,7 @@ secure_open_gssapi(Port *port)
bool complete_next = false;
OM_uint32 major,
minor;
+ gss_cred_id_t proxy;
/*
* Allocate subsidiary Port data for GSSAPI operations.
@@ -588,7 +589,8 @@ secure_open_gssapi(Port *port)
GSS_C_NO_CREDENTIAL, &input,
GSS_C_NO_CHANNEL_BINDINGS,
&port->gss->name, NULL, &output, NULL,
- NULL, NULL);
+ NULL, &proxy);
+
if (GSS_ERROR(major))
{
pg_GSS_error(_("could not accept GSSAPI security context"),
@@ -605,6 +607,9 @@ secure_open_gssapi(Port *port)
complete_next = true;
}
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
/* Done handling the incoming packet, reset our buffer */
PqGSSRecvLength = 0;
diff --git a/src/include/libpq/be-gssapi-common.h b/src/include/libpq/be-gssapi-common.h
index c07d7e7c5a..62d60ffbd8 100644
--- a/src/include/libpq/be-gssapi-common.h
+++ b/src/include/libpq/be-gssapi-common.h
@@ -18,9 +18,11 @@
#include <gssapi.h>
#else
#include <gssapi/gssapi.h>
+#include <gssapi/gssapi_ext.h>
#endif
extern void pg_GSS_error(const char *errmsg,
OM_uint32 maj_stat, OM_uint32 min_stat);
+extern void pg_store_proxy_credential(gss_cred_id_t cred);
#endif /* BE_GSSAPI_COMMON_H */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 3421ed4685..e0d342124e 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -62,6 +62,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
lmin_s;
gss_buffer_desc ginbuf;
gss_buffer_desc goutbuf;
+ gss_cred_id_t proxy;
/*
* On first call, there's no input token. On subsequent calls, read the
@@ -94,12 +95,16 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
ginbuf.value = NULL;
}
+ /* Check if we can aquire a proxy credential. */
+ if (!pg_GSS_have_cred_cache(&proxy))
+ proxy = GSS_C_NO_CREDENTIAL;
+
maj_stat = gss_init_sec_context(&min_stat,
- GSS_C_NO_CREDENTIAL,
+ proxy,
&conn->gctx,
conn->gtarg_nam,
GSS_C_NO_OID,
- GSS_C_MUTUAL_FLAG,
+ GSS_C_MUTUAL_FLAG | GSS_C_DELEG_FLAG,
0,
GSS_C_NO_CHANNEL_BINDINGS,
(ginbuf.value == NULL) ? GSS_C_NO_BUFFER : &ginbuf,
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index c783a53734..781af4227c 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -631,7 +631,7 @@ pqsecure_open_gss(PGconn *conn)
*/
major = gss_init_sec_context(&minor, conn->gcred, &conn->gctx,
conn->gtarg_nam, GSS_C_NO_OID,
- GSS_REQUIRED_FLAGS, 0, 0, &input, NULL,
+ GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, &input, NULL,
&output, NULL, NULL);
/* GSS Init Sec Context uses the whole packet, so clear it */
[text/x-patch] v2-0002-kerberos-delegation-tap-test.patch (15.5K, ../../CO1PR05MB80235F71EE030594A275AF5DA8E49@CO1PR05MB8023.namprd05.prod.outlook.com/4-v2-0002-kerberos-delegation-tap-test.patch)
download | inline diff:
commit a24714cc5507ce6027eef6f923b8d24e7e1ce208
Author: Peifeng Qiu <[email protected]>
Date: Thu Jul 22 17:27:36 2021 +0900
TAP test
diff --git a/contrib/postgres_fdw/kerberos/.gitignore b/contrib/postgres_fdw/kerberos/.gitignore
new file mode 100644
index 0000000000..871e943d50
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/.gitignore
@@ -0,0 +1,2 @@
+# Generated by test suite
+/tmp_check/
diff --git a/contrib/postgres_fdw/kerberos/Makefile b/contrib/postgres_fdw/kerberos/Makefile
new file mode 100644
index 0000000000..3efec922f0
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/Makefile
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/kerberos
+#
+# Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/kerberos/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = contrib/postgres_fdw/kerberos
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+export with_gssapi with_krb_srvnam
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
+
+clean distclean maintainer-clean:
+ rm -rf tmp_check
diff --git a/contrib/postgres_fdw/kerberos/README b/contrib/postgres_fdw/kerberos/README
new file mode 100644
index 0000000000..40b8758543
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/README
@@ -0,0 +1,45 @@
+contrib/postgres_fdw/kerberos/README
+
+Tests for Kerberos/GSSAPI functionality
+=======================================
+
+This directory contains a test suite for Kerberos/GSSAPI
+functionality. This requires a full MIT Kerberos installation,
+including server and client tools, and is therefore kept separate and
+not run by default.
+
+CAUTION: The test server run by this test is configured to listen for TCP
+connections on localhost. Any user on the same host is able to log in to the
+test server while the tests are running. Do not run this suite on a multi-user
+system where you don't trust all local users! Also, this test suite creates a
+KDC server that listens for TCP/IP connections on localhost without any real
+access control.
+
+Running the tests
+=================
+
+NOTE: You must have given the --enable-tap-tests argument to configure.
+
+Run
+ make check
+or
+ make installcheck
+You can use "make installcheck" if you previously did "make install".
+In that case, the code in the installation tree is tested. With
+"make check", a temporary installation tree is built from the current
+sources and then tested.
+
+Either way, this test initializes, starts, and stops a test Postgres
+cluster, as well as a test KDC server.
+
+Requirements
+============
+
+MIT Kerberos server and client tools are required. Heimdal is not
+supported.
+
+Debian/Ubuntu packages: krb5-admin-server krb5-kdc krb5-user
+
+RHEL/CentOS/Fedora packages: krb5-server krb5-workstation
+
+FreeBSD port: krb5 (base system has Heimdal)
diff --git a/contrib/postgres_fdw/kerberos/t/001_auth.pl b/contrib/postgres_fdw/kerberos/t/001_auth.pl
new file mode 100644
index 0000000000..79cd0bb006
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/t/001_auth.pl
@@ -0,0 +1,426 @@
+
+# Copyright (c) 2021, PostgreSQL Global Development Group
+
+# Sets up a KDC and then runs a variety of tests to make sure that the
+# GSSAPI/Kerberos authentication and encryption are working properly,
+# that the options in pg_hba.conf and pg_ident.conf are handled correctly,
+# and that the server-side pg_stat_gssapi view reports what we expect to
+# see for each test.
+#
+# Since this requires setting up a full KDC, it doesn't make much sense
+# to have multiple test scripts (since they'd have to also create their
+# own KDC and that could cause race conditions or other problems)- so
+# just add whatever other tests are needed to here.
+#
+# See the README for additional information.
+
+use strict;
+use warnings;
+use TestLib;
+use PostgresNode;
+use Test::More;
+use Time::HiRes qw(usleep);
+use Cwd 'abs_path';
+
+note "start";
+if ($ENV{with_gssapi} eq 'yes')
+{
+ plan tests => 23;
+}
+else
+{
+ plan skip_all => 'GSSAPI/Kerberos not supported by this build';
+}
+
+my ($krb5_bin_dir, $krb5_sbin_dir);
+
+if ($^O eq 'darwin')
+{
+ $krb5_bin_dir = '/usr/local/opt/krb5/bin';
+ $krb5_sbin_dir = '/usr/local/opt/krb5/sbin';
+}
+elsif ($^O eq 'freebsd')
+{
+ $krb5_bin_dir = '/usr/local/bin';
+ $krb5_sbin_dir = '/usr/local/sbin';
+}
+elsif ($^O eq 'linux')
+{
+ $krb5_sbin_dir = '/usr/sbin';
+}
+
+my $krb5_config = 'krb5-config';
+my $kinit = 'kinit';
+my $kdb5_util = 'kdb5_util';
+my $kadmin_local = 'kadmin.local';
+my $krb5kdc = 'krb5kdc';
+
+if ($krb5_bin_dir && -d $krb5_bin_dir)
+{
+ $krb5_config = $krb5_bin_dir . '/' . $krb5_config;
+ $kinit = $krb5_bin_dir . '/' . $kinit;
+}
+if ($krb5_sbin_dir && -d $krb5_sbin_dir)
+{
+ $kdb5_util = $krb5_sbin_dir . '/' . $kdb5_util;
+ $kadmin_local = $krb5_sbin_dir . '/' . $kadmin_local;
+ $krb5kdc = $krb5_sbin_dir . '/' . $krb5kdc;
+}
+
+my $host = 'auth-test-localhost.postgresql.example.com';
+my $hostaddr = '127.0.0.1';
+my $realm = 'EXAMPLE.COM';
+
+
+my $tmp_check = abs_path(${TestLib::tmp_check});
+my $krb5_conf = "$tmp_check/krb5.conf";
+my $kdc_conf = "$tmp_check/kdc.conf";
+#my $krb5_cache = "$tmp_check/krb5cc";
+my $krb5_cache = "MEMORY:";
+my $krb5_log = "${TestLib::log_path}/krb5libs.log";
+my $kdc_log = "${TestLib::log_path}/krb5kdc.log";
+my $kdc_port = get_free_port();
+my $kdc_datadir = "$tmp_check/krb5kdc";
+my $kdc_pidfile = "$tmp_check/krb5kdc.pid";
+my $keytab_client = "$tmp_check/client.keytab";
+my $keytab_server = "$tmp_check/server.keytab";
+
+my $dbname = 'testdb';
+my $testuser = 'testuser';
+my $test1 = 'test1';
+my $application = '001_auth.pl';
+
+note "setting up Kerberos";
+
+my ($stdout, $krb5_version);
+run_log [ $krb5_config, '--version' ], '>', \$stdout
+ or BAIL_OUT("could not execute krb5-config");
+BAIL_OUT("Heimdal is not supported") if $stdout =~ m/heimdal/;
+$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/
+ or BAIL_OUT("could not get Kerberos version");
+$krb5_version = $1;
+
+append_to_file(
+ $krb5_conf,
+ qq![logging]
+default = FILE:$krb5_log
+kdc = FILE:$kdc_log
+
+[libdefaults]
+default_realm = $realm
+forwardable = false
+
+[realms]
+$realm = {
+ kdc = $hostaddr:$kdc_port
+}!);
+
+append_to_file(
+ $kdc_conf,
+ qq![kdcdefaults]
+!);
+
+# For new-enough versions of krb5, use the _listen settings rather
+# than the _ports settings so that we can bind to localhost only.
+if ($krb5_version >= 1.15)
+{
+ append_to_file(
+ $kdc_conf,
+ qq!kdc_listen = $hostaddr:$kdc_port
+kdc_tcp_listen = $hostaddr:$kdc_port
+!);
+}
+else
+{
+ append_to_file(
+ $kdc_conf,
+ qq!kdc_ports = $kdc_port
+kdc_tcp_ports = $kdc_port
+!);
+}
+
+append_to_file(
+ $kdc_conf,
+ qq!
+[realms]
+$realm = {
+ database_name = $kdc_datadir/principal
+ admin_keytab = FILE:$kdc_datadir/kadm5.keytab
+ acl_file = $kdc_datadir/kadm5.acl
+ key_stash_file = $kdc_datadir/_k5.$realm
+}!);
+
+mkdir $kdc_datadir or die;
+
+# Ensure that we use test's config and cache files, not global ones.
+$ENV{'KRB5_CONFIG'} = $krb5_conf;
+$ENV{'KRB5_KDC_PROFILE'} = $kdc_conf;
+$ENV{'KRB5CCNAME'} = $krb5_cache;
+
+my $service_principal = "$ENV{with_krb_srvnam}/$host";
+
+system_or_bail $kdb5_util, 'create', '-s', '-P', 'secret0';
+
+my $test1_password = 'secret1';
+system_or_bail $kadmin_local, '-q', "addprinc -randkey $testuser";
+system_or_bail $kadmin_local, '-q', "addprinc -randkey $service_principal";
+system_or_bail $kadmin_local, '-q', "ktadd -k $keytab_client $testuser";
+system_or_bail $kadmin_local, '-q', "ktadd -k $keytab_server $service_principal";
+
+system_or_bail $krb5kdc, '-P', $kdc_pidfile;
+
+END
+{
+ kill 'INT', `cat $kdc_pidfile` if -f $kdc_pidfile;
+}
+
+note "setting up PostgreSQL instance";
+
+# server node for fdw to connect
+my $server_node = get_new_node('node');
+$server_node->init;
+$server_node->append_conf(
+ 'postgresql.conf', qq{
+listen_addresses = '$hostaddr'
+krb_server_keyfile = '$keytab_server'
+log_connections = on
+lc_messages = 'C'
+});
+$server_node->start;
+
+note "setting up user on server";
+$server_node->safe_psql('postgres', "CREATE USER $testuser;");
+# there's no direct way to get the backend pid from postgres_fdw, create a VIEW at server side to provide this.
+$server_node->safe_psql('postgres', 'CREATE VIEW my_pg_stat_gssapi AS
+ SELECT S.pid,
+ S.gss_auth AS gss_authenticated,
+ S.gss_princ AS principal,
+ S.gss_enc AS encrypted
+ FROM pg_stat_get_activity(NULL) AS S
+ WHERE S.client_port IS NOT NULL AND S.pid = pg_backend_pid();');
+$server_node->safe_psql('postgres', "GRANT ALL ON my_pg_stat_gssapi TO $testuser;");
+
+my $server_node_port = $server_node->port;
+note "setting up PostgreSQL fdw instance";
+# client node that runs postgres fdw
+my $fdw_node = get_new_node('node_fdw');
+$fdw_node->init;
+$fdw_node->append_conf(
+ 'postgresql.conf', qq{
+listen_addresses = '$hostaddr'
+krb_server_keyfile = '$keytab_server'
+log_connections = on
+lc_messages = 'C'
+});
+$fdw_node->start;
+
+$fdw_node->safe_psql('postgres', "CREATE USER $testuser;");
+$fdw_node->safe_psql('postgres', "CREATE DATABASE $dbname;");
+$fdw_node->safe_psql($dbname, "CREATE EXTENSION postgres_fdw");
+$fdw_node->safe_psql($dbname, "GRANT ALL ON FOREIGN DATA WRAPPER postgres_fdw TO $testuser");
+
+note "running tests";
+
+sub set_user_mapping_option
+{
+ my ($option, $value, $action) = @_;
+ if (!defined $action)
+ {
+ $action = "SET";
+ }
+
+ my $option_str = "$option";
+ if (defined $value)
+ {
+ $option_str = "$option '$value'";
+ }
+
+ my $q = "ALTER USER MAPPING for $testuser SERVER postgres_server OPTIONS ($action $option_str);";
+ $fdw_node->safe_psql($dbname, $q);
+}
+
+# Test connection success or failure, and if success, that query returns true.
+sub test_access
+{
+ my ($query, $expected_res, $test_name,
+ $expect_output, @expect_log_msgs)
+ = @_;
+ test_access_gssmode($query, "prefer", $expected_res, $test_name,
+ $expect_output, @expect_log_msgs)
+}
+
+sub test_access_gssmode_disable
+{
+ my ($query, $expected_res, $test_name,
+ $expect_output, @expect_log_msgs)
+ = @_;
+ test_access_gssmode($query, "disable", $expected_res, $test_name,
+ $expect_output, @expect_log_msgs)
+}
+
+sub test_access_gssmode
+{
+ my ($query, $mode, $expected_res, $test_name,
+ $expect_output, @expect_log_msgs)
+ = @_;
+
+ my %params = (sql => $query,);
+
+ # Check log in server node. Obtain log file size before we run any query.
+ # This way we can obtain logs for this psql connection only.
+ my $log_location = -s $server_node->logfile;
+
+ # Run psql in fdw node.
+ my ($ret, $stdout, $stderr) = $fdw_node->psql(
+ $dbname,
+ $query,
+ extra_params => ['-w'],
+ connstr => "user=$testuser dbname=$dbname host=$host hostaddr=$hostaddr gssencmode=$mode");
+
+
+ is($ret, $expected_res, $test_name);
+ if ($ret != $expected_res)
+ {
+ note $ret;
+ note $stdout;
+ note $stderr;
+ }
+
+ if (defined $expect_output)
+ {
+ if ($expected_res eq 0)
+ {
+ like($stdout, $expect_output, "$test_name: result matches");
+ }
+ else
+ {
+ like($stderr, $expect_output, "$test_name: result matches");
+ }
+ }
+
+ if (@expect_log_msgs)
+ {
+ # Match every message literally.
+ my @regexes = map { qr/\Q$_\E/ } @expect_log_msgs;
+ my $log_contents = TestLib::slurp_file($server_node->logfile, $log_location);
+
+ while (my $regex = shift @regexes)
+ {
+ like($log_contents, $regex, "$test_name: matches");
+ }
+ }
+}
+
+# Setup pg_hba to allow superuser trust login, and testuser gss login.
+# We need to remove the KRB5_CLIENT_KTNAME environemnt first.
+delete $ENV{'KRB5_CLIENT_KTNAME'};
+unlink($fdw_node->data_dir . '/pg_hba.conf');
+$fdw_node->append_conf('pg_hba.conf',
+ qq{local all $ENV{'USER'} trust});
+$fdw_node->append_conf('pg_hba.conf',
+ qq{host all $testuser $hostaddr/32 gss map=mymap});
+$fdw_node->append_conf('pg_ident.conf', qq{mymap /^(.*)\@$realm\$ \\1});
+$fdw_node->restart;
+
+test_access(
+ 'SELECT true FROM pg_stat_gssapi',
+ 2,
+ 'no access without credential',
+ '/No Kerberos credentials available/'
+);
+
+$ENV{'KRB5_CLIENT_KTNAME'} = $keytab_client;
+
+test_access(
+ 'SELECT true FROM pg_stat_gssapi',
+ 0,
+ 'success',
+ "/^t\$/"
+);
+
+# create foreign table using unprivileged user
+
+test_access(
+ "CREATE SERVER postgres_server FOREIGN DATA WRAPPER postgres_fdw OPTIONS (host '$host', hostaddr '$hostaddr', port '$server_node_port', dbname 'postgres')",
+ 0,
+ 'create server',
+);
+test_access(
+ "CREATE FOREIGN TABLE my_pg_stat_gssapi\(pid int, gss_authenticated boolean, principal text, encrypted boolean\) SERVER postgres_server OPTIONS(table_name 'my_pg_stat_gssapi'); ",
+ 0,
+ 'create foreign table',
+);
+test_access(
+ "CREATE USER MAPPING for $testuser SERVER postgres_server OPTIONS (user '$testuser');",
+ 0,
+ 'create user mapping',
+);
+
+test_access(
+ "SELECT gss_authenticated AND encrypted from my_pg_stat_gssapi;",
+ 3,
+ 'select failed due to password required',
+ "/password is required/"
+);
+
+set_user_mapping_option('password_required', 'false', 'ADD');
+
+test_access(
+ "SELECT gss_authenticated AND encrypted from my_pg_stat_gssapi;",
+ 0,
+ 'select fdw success, but with trust connection',
+ "/f/"
+);
+
+
+unlink($server_node->data_dir . '/pg_hba.conf');
+$server_node->append_conf('pg_hba.conf',
+ qq{host all all $hostaddr/32 gss map=mymap});
+$server_node->append_conf('pg_ident.conf', qq{mymap /^(.*)\@$realm\$ \\1});
+$server_node->restart;
+
+test_access(
+ 'SELECT gss_authenticated FROM my_pg_stat_gssapi',
+ 3,
+ 'fails without ticket, due to not forwardable',
+ '/No Kerberos credentials available/',
+);
+
+# Allow ticket forward in krb5.conf
+string_replace_file($krb5_conf, "forwardable = false", "forwardable = true");
+test_access(
+ 'SELECT gss_authenticated FROM my_pg_stat_gssapi',
+ 0,
+ 'success with ticket forward',
+ "/^t\$/"
+);
+
+test_access_gssmode_disable(
+ 'SELECT gss_authenticated,encrypted FROM pg_stat_gssapi',
+ 0,
+ 'login fdw success, encryption disabled',
+ "/^t|f\$/"
+);
+
+test_access_gssmode_disable(
+ 'SELECT gss_authenticated,encrypted FROM my_pg_stat_gssapi',
+ 0,
+ 'success, encryption enabled for fdw even disabled by login',
+ "/^t|t\$/"
+);
+
+set_user_mapping_option('gssencmode', 'disable', 'ADD');
+
+test_access(
+ 'SELECT gss_authenticated,encrypted FROM my_pg_stat_gssapi',
+ 0,
+ 'success, encryption disabled for fdw even enabled by login',
+ "/^t|f\$/"
+);
+
+test_access_gssmode_disable(
+ 'SELECT gss_authenticated,encrypted FROM my_pg_stat_gssapi',
+ 0,
+ 'success, encryption disabled both by login and fdw',
+ "/^t|f\$/"
+);
\ No newline at end of file
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 15572abbea..af4245f755 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -67,6 +67,7 @@ our @EXPORT = qw(
slurp_dir
slurp_file
append_to_file
+ string_replace_file
check_mode_recursive
chmod_recursive
check_pg_config
@@ -539,6 +540,32 @@ sub append_to_file
=pod
+=item string_replace_file(filename, find, replace)
+
+Find and replace string of a given file.
+
+=cut
+
+sub string_replace_file
+{
+ my ($filename, $find, $replace) = @_;
+ open(my $in, '<', $filename);
+ my $content;
+ while(<$in>)
+ {
+ $_ =~ s/$find/$replace/;
+ $content = $content.$_;
+ }
+ close $in;
+ open(my $out, '>', $filename);
+ print $out $content;
+ close($out);
+
+ return;
+}
+
+=pod
+
=item check_mode_recursive(dir, expected_dir_mode, expected_file_mode, ignore_list)
Check that all file/dir modes in a directory match the expected values,
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
@ 2021-09-01 08:57 ` Peter Eisentraut <[email protected]>
2021-11-03 12:41 ` Re: Kerberos delegation support in libpq and postgres_fdw Daniel Gustafsson <[email protected]>
2022-07-07 23:24 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
0 siblings, 2 replies; 24+ messages in thread
From: Peter Eisentraut @ 2021-09-01 08:57 UTC (permalink / raw)
To: Peifeng Qiu <[email protected]>; [email protected] <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Tom Lane <[email protected]>
On 22.07.21 10:39, Peifeng Qiu wrote:
> I've slightly modified the patch to support "gssencmode" and added TAP
> tests.
For the TAP tests, please put then under src/test/kerberos/, instead of
copying the whole infrastructure to contrib/postgres_fdw/. Just make a
new file, for example t/002_postgres_fdw_proxy.pl, and put your tests there.
Also, you can put code and tests in one patch, no need to separate.
I wonder if this feature would also work in dblink. Since there is no
substantial code changes in postgres_fdw itself as part of this patch, I
would suspect yes. Can you check?
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
@ 2021-11-03 12:41 ` Daniel Gustafsson <[email protected]>
2021-11-26 13:41 ` Re: Kerberos delegation support in libpq and postgres_fdw Daniel Gustafsson <[email protected]>
1 sibling, 1 reply; 24+ messages in thread
From: Daniel Gustafsson @ 2021-11-03 12:41 UTC (permalink / raw)
To: Peifeng Qiu <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected] <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Tom Lane <[email protected]>
This patch no longer applies following the Perl namespace changes, can you
please submit a rebased version? Marking the patch "Waiting on Author".
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
2021-11-03 12:41 ` Re: Kerberos delegation support in libpq and postgres_fdw Daniel Gustafsson <[email protected]>
@ 2021-11-26 13:41 ` Daniel Gustafsson <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Gustafsson @ 2021-11-26 13:41 UTC (permalink / raw)
To: Peifeng Qiu <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected] <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Tom Lane <[email protected]>
> On 3 Nov 2021, at 13:41, Daniel Gustafsson <[email protected]> wrote:
>
> This patch no longer applies following the Perl namespace changes, can you
> please submit a rebased version? Marking the patch "Waiting on Author".
As the thread has stalled, and the OP email bounces, I'm marking this patch
Returned with Feedback. Please feel free to resubmit a new entry in case
anyone picks this up.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
@ 2022-07-07 23:24 ` Jacob Champion <[email protected]>
2022-09-16 00:06 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
1 sibling, 1 reply; 24+ messages in thread
From: Jacob Champion @ 2022-07-07 23:24 UTC (permalink / raw)
To: Stephen Frost <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Magnus Hagander <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>
On 4/8/22 05:21, Stephen Frost wrote:
> Added a few more tests and updated the documentation too. Sadly, seems
> we've missed the deadline for v15 though for lack of feedback on these.
> Would really like to get some other folks commenting as these are new
> pg_hba and postgresql.conf options being added.
Sorry for the incredibly long delay; I lost track of this thread during
the email switch. I'm testing the patch with various corner cases to try
to figure out how it behaves, so this isn't a full review, but I wanted
to jump through some of the emails I missed and at least give you some
responses.
As an overall note, I think the patch progression, and adding more
explicit control over when credentials may be delegated, is very
positive, and +1 for the proposed libpq connection option elsewhere in
the thread.
On 4/7/22 21:21, Stephen Frost wrote:
>> That an admin might have a credential cache that's picked up and used
>> for connections from a regular user backend to another system strikes me
>> as an altogether concerning idea. Even so, in such a case, the admin
>> would have had to set up the user mapping with 'password required =
>> false' or it wouldn't have worked for a non-superuser anyway, so I'm not
>> sure that I'm too worried about this case.
>
> To address this, I also added a new GUC which allows an administrator to
> control what the credential cache is set to for user-authenticated
> backends, with a default of MEMORY:, which should generally be safe and
> won't cause a user backend to pick up on a file-based credential cache
> which might exist on the server somewhere. This gives the administrator
> the option to set it to more-or-less whatever they'd like though, so if
> they want to set it to a file-based credential cache, then they can do
> so (I did put some caveats about doing that into the documentation as I
> don't think it's generally a good idea to do...).
I'm not clear on how this handles the collision case. My concern was
with a case where you have more than one foreign table/server, and they
need to use separate credentials. It's not obvious to me how changing
the location of a (single, backend-global) cache mitigates that problem.
I'm also missing something about why password_required=false is
necessary (as opposed to simply setting a password in the USER MAPPING).
My current test case doesn't make use of password_required=false and it
appears to work just fine.
On 4/6/22 12:27, Stephen Frost wrote:
>> Another danger might be disclosure/compromise of middlebox secrets? Is
>> it possible for someone who has one half of the credentials to snoop on
>> a gssenc connection between the proxy Postgres and the backend
>> Postgres?
>
> A compromised middlebox would, of course, be an issue- for any kind of
> delegated credentials (which certainly goes for cleartext passwords
> being passed along, and that's currently the only thing we support..).
> One nice thing about GSSAPI is that the client and the server validate
> each other, so it wouldn't just be 'any' middle-box but would have to be
> one that was actually a trusted system in the infrastructure which has
> somehow been compromised and was still trusted.
I wasn't clear enough, sorry -- I mean that we have to prove that
defaulting allow_cred_delegation to true doesn't cause the compromise of
existing deployments.
As an example, right now I'm trying to characterize behavior with the
following pg_hba setup on the foreign server:
hostgssenc all all ... password
So in other words we're using GSS as transport encryption only, not as
an authentication provider. On the middlebox, we create a FOREIGN
SERVER/TABLE that points to this, and set up a USER MAPPING (with no
USAGE rights) that contains the necessary password. (I'm using a
plaintext password to make it more obvious what the danger is, not
suggesting that this would be good practice.)
As far as I can tell, to make this work today, a server admin has to set
up a local credential cache with the keys for some one-off principal. It
doesn't have to be an admin principal, because the point is just to
provide transport protection for the password, so it's not really
particularly scary to make it available to user backends. But this new
proposed feature lets the client override that credential cache,
substituting their own credentials, for which they have all the Kerberos
symmetric key material.
So my question is this: does substituting my credentials for the admin's
credentials let me weaken or break the transport encryption on the
backend connection, and grab the password that I'm not supposed to have
access to as a front-end client?
I honestly don't know the answer; GSSAPI is a black box that defers to
Kerberos and there's a huge number of specs that I've been slowly making
my way through. But in my tests, if I turn on credential forwarding,
Wireshark is suddenly able to use the *client's* keys to decrypt pieces
of the TGS's conversations with the *middlebox*, including session keys,
and that doesn't make me feel very good about the strength of the crypto
when the middlebox starts talking to the backend foreign server.
Maybe there's some ephemeral exchange going on that makes it too hard to
attack in practice, or some other mitigations. But Wireshark doesn't
understand how to dissect the libpq gssenc exchange, and I don't know
the specs well enough yet, so I can't really prove it either way. Do you
know more about the underlying GSS exchange, or else which specs cover
the low-level details? I'm trying to avoid writing Wireshark dissector
code, but maybe that'd be useful either way...
--Jacob
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
2022-07-07 23:24 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
@ 2022-09-16 00:06 ` Jacob Champion <[email protected]>
2022-09-19 17:05 ` Re: Kerberos delegation support in libpq and postgres_fdw Stephen Frost <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Jacob Champion @ 2022-09-16 00:06 UTC (permalink / raw)
To: Stephen Frost <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Magnus Hagander <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>
On Thu, Jul 7, 2022 at 4:24 PM Jacob Champion <[email protected]> wrote:
> So my question is this: does substituting my credentials for the admin's
> credentials let me weaken or break the transport encryption on the
> backend connection, and grab the password that I'm not supposed to have
> access to as a front-end client?
With some further research: yes, it does.
If a DBA is using a GSS encrypted tunnel to communicate to a foreign
server, accepting delegation by default means that clients will be
able to break that backend encryption at will, because the keys in use
will be under their control.
> Maybe there's some ephemeral exchange going on that makes it too hard to
> attack in practice, or some other mitigations.
There is no forward secrecy, ephemeral exchange, etc. to mitigate this [1]:
The Kerberos protocol in its basic form does not provide perfect
forward secrecy for communications. If traffic has been recorded by
an eavesdropper, then messages encrypted using the KRB_PRIV message,
or messages encrypted using application-specific encryption under
keys exchanged using Kerberos can be decrypted if the user's,
application server's, or KDC's key is subsequently discovered.
So the client can decrypt backend communications that make use of its
delegated key material. (This also means that gssencmode is a lot
weaker than I expected.)
> I'm trying to avoid writing Wireshark dissector
> code, but maybe that'd be useful either way...
I did end up filling out the existing PGSQL dissector so that it could
decrypt GSSAPI exchanges (with the use of a keytab, that is). If you'd
like to give it a try, the patch, based on Wireshark 3.7.1, is
attached. Note the GPLv2 license. It isn't correct code yet, because I
didn't understand how packet reassembly worked in Wireshark when I
started writing the code, so really large GSSAPI messages that are
split across multiple TCP packets will confuse the dissector. But it's
enough to prove the concept.
To see this in action, set up an FDW connection that uses gssencmode
(so the server in the middle will need its own Kerberos credentials).
Capture traffic starting from the kinit through the query on the
foreign table. Export the client's key material into a keytab, and set
up Wireshark to use that keytab for decryption. When credential
forwarding is *not* in use, Wireshark will be able to decrypt the
initial client connection, but it won't be able to see anything inside
the foreign server connection. When credential forwarding is in use,
Wireshark will be able to decrypt both connections.
Thanks,
--Jacob
[1] https://www.rfc-editor.org/rfc/rfc4120
From 0cf31522223a6044edae42037e45aee0fc88352f Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Wed, 14 Sep 2022 13:08:22 -0700
Subject: [PATCH] pgsql: decrypt GSS-encrypted channels and tokens if possible
License: GPLv2
Add dissection for a conversation that starts with a GSSAPI encryption
request, and pass those packets along to the gssapi dissector. This
allows parts of the conversation to be decrypted if Wireshark has been
set up with a KRB5 keytab.
Additionally, break apart the PGSQL_AUTH_GSSAPI_SSPI_DATA case into
separate GSSAPI and SSPI cases, and when we're using GSSAPI, dissect the
authentication tokens that are passed between the client and server.
This incidentally fixes an off-by-four bug in the
PGSQL_AUTH_TYPE_GSSAPI_SSPI_CONTINUE case; the offset was not being
updated correctly before.
The new code copies the dissection strategy of multipart, and adds a new
last_nongss_frame marker similar to the STARTTLS handling code's
last_nontls_frame.
---
epan/dissectors/packet-pgsql.c | 121 +++++++++++++++++++++++++++++++--
1 file changed, 116 insertions(+), 5 deletions(-)
diff --git a/epan/dissectors/packet-pgsql.c b/epan/dissectors/packet-pgsql.c
index c935eec1f8..7efe1c003f 100644
--- a/epan/dissectors/packet-pgsql.c
+++ b/epan/dissectors/packet-pgsql.c
@@ -14,6 +14,8 @@
#include <epan/packet.h>
+#include "packet-dcerpc.h"
+#include "packet-gssapi.h"
#include "packet-tls-utils.h"
#include "packet-tcp.h"
@@ -22,6 +24,7 @@ void proto_reg_handoff_pgsql(void);
static dissector_handle_t pgsql_handle;
static dissector_handle_t tls_handle;
+static dissector_handle_t gssapi_handle;
static int proto_pgsql = -1;
static int hf_frontend = -1;
@@ -80,10 +83,13 @@ static int hf_constraint_name = -1;
static int hf_file = -1;
static int hf_line = -1;
static int hf_routine = -1;
+static int hf_gssapi_length = -1;
static gint ett_pgsql = -1;
static gint ett_values = -1;
+static expert_field ei_gssapi_decryption_not_possible = EI_INIT;
+
#define PGSQL_PORT 5432
static gboolean pgsql_desegment = TRUE;
static gboolean first_message = TRUE;
@@ -92,11 +98,14 @@ typedef enum {
PGSQL_AUTH_STATE_NONE, /* No authentication seen or used */
PGSQL_AUTH_SASL_REQUESTED, /* Server sends SASL auth request with supported SASL mechanisms*/
PGSQL_AUTH_SASL_CONTINUE, /* Server and/or client send further SASL challange-response messages */
- PGSQL_AUTH_GSSAPI_SSPI_DATA, /* GSSAPI/SSPI in use */
+ PGSQL_AUTH_GSSAPI, /* GSSAPI in use */
+ PGSQL_AUTH_SSPI, /* SSPI in use */
} pgsql_auth_state_t;
typedef struct pgsql_conn_data {
gboolean ssl_requested;
+ gboolean gss_requested;
+ guint32 last_nongss_frame;
pgsql_auth_state_t auth_state; /* Current authentication state */
} pgsql_conn_data_t;
@@ -189,6 +198,22 @@ static const value_string format_vals[] = {
{ 0, NULL }
};
+static void
+dissect_gssapi_data(tvbuff_t *tvb, gint offset, guint len, packet_info *pinfo,
+ proto_tree *tree, gssapi_encrypt_info_t *encrypt)
+{
+ tvbuff_t *gssapi_tvb;
+ guint8 *data;
+
+ DISSECTOR_ASSERT(tvb_bytes_exist(tvb, offset, len));
+
+ data = (guint8 *)tvb_memdup(pinfo->pool, tvb, offset, len);
+ gssapi_tvb = tvb_new_child_real_data(tvb, data, len, len);
+
+ add_new_data_source(pinfo, gssapi_tvb, "GSSAPI Data");
+ call_dissector_with_data(gssapi_handle, gssapi_tvb, pinfo, tree, encrypt);
+}
+
static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
gint n, proto_tree *tree, packet_info *pinfo,
pgsql_conn_data_t *conv_data)
@@ -220,7 +245,12 @@ static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
proto_tree_add_item(tree, hf_sasl_auth_data, tvb, n, length-4, ENC_NA);
break;
- case PGSQL_AUTH_GSSAPI_SSPI_DATA:
+ case PGSQL_AUTH_GSSAPI:
+ proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-4, ENC_NA);
+ dissect_gssapi_data(tvb, n, length-4, pinfo, tree, NULL);
+ break;
+
+ case PGSQL_AUTH_SSPI:
proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-4, ENC_NA);
break;
@@ -356,6 +386,12 @@ static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
conv_data->ssl_requested = TRUE;
break;
+ /* GSS request */
+ case 80877104:
+ /* Next reply will be a single byte. */
+ conv_data->gss_requested = TRUE;
+ break;
+
/* Cancellation request */
case 80877102:
proto_tree_add_item(tree, hf_pid, tvb, n, 4, ENC_BIG_ENDIAN);
@@ -430,9 +466,17 @@ static void dissect_pgsql_be_msg(guchar type, guint length, tvbuff_t *tvb,
siz = (auth_type == PGSQL_AUTH_TYPE_CRYPT ? 2 : 4);
proto_tree_add_item(tree, hf_salt, tvb, n, siz, ENC_NA);
break;
+ case PGSQL_AUTH_TYPE_GSSAPI:
+ conv_data->auth_state = PGSQL_AUTH_GSSAPI;
+ break;
+ case PGSQL_AUTH_TYPE_SSPI:
+ conv_data->auth_state = PGSQL_AUTH_SSPI;
+ break;
case PGSQL_AUTH_TYPE_GSSAPI_SSPI_CONTINUE:
- conv_data->auth_state = PGSQL_AUTH_GSSAPI_SSPI_DATA;
+ n += 4;
proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-8, ENC_NA);
+ if (conv_data->auth_state == PGSQL_AUTH_GSSAPI)
+ dissect_gssapi_data(tvb, n, length-8, pinfo, tree, NULL);
break;
case PGSQL_AUTH_TYPE_SASL:
conv_data->auth_state = PGSQL_AUTH_SASL_REQUESTED;
@@ -665,6 +709,8 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
if (!conn_data) {
conn_data = wmem_new(wmem_file_scope(), pgsql_conn_data_t);
conn_data->ssl_requested = FALSE;
+ conn_data->gss_requested = FALSE;
+ conn_data->last_nongss_frame = G_MAXUINT32;
conn_data->auth_state = PGSQL_AUTH_STATE_NONE;
conversation_add_proto_data(conversation, proto_pgsql, conn_data);
}
@@ -703,7 +749,8 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
case PGSQL_AUTH_SASL_CONTINUE:
typestr = "SASLResponse message";
break;
- case PGSQL_AUTH_GSSAPI_SSPI_DATA:
+ case PGSQL_AUTH_GSSAPI: /* fallthrough */
+ case PGSQL_AUTH_SSPI:
typestr = "GSSResponse message";
break;
default:
@@ -746,6 +793,18 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
return tvb_captured_length(tvb);
}
+static void
+dissect_gssenc_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, gssapi_encrypt_info_t *encrypt)
+{
+ gint offset = 0, len;
+
+ proto_tree_add_item(tree, hf_gssapi_length, tvb, offset, 4, ENC_BIG_ENDIAN);
+ offset += 4;
+ len = tvb_reported_length_remaining(tvb, offset);
+
+ dissect_gssapi_data(tvb, offset, len, pinfo, tree, encrypt);
+}
+
/* This function is called once per TCP packet. It sets COL_PROTOCOL and
* identifies FE/BE messages by adding a ">" or "<" to COL_INFO. Then it
* arranges for each message to be dissected individually. */
@@ -782,6 +841,43 @@ dissect_pgsql(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data)
return tvb_captured_length(tvb);
}
+ if (conn_data && conn_data->gss_requested) {
+ /* Response to GSS request. */
+ switch (tvb_get_guint8(tvb, 0)) {
+ case 'G': /* Willing to perform GSS encryption */
+ /* Next packet will start using GSS encryption. */
+ conn_data->last_nongss_frame = pinfo->num;
+ break;
+ case 'N': /* Unwilling to perform GSS encryption */
+ default: /* ErrorMessage when server does not support SSL. */
+ /* TODO: maybe add expert info here? */
+ break;
+ }
+ conn_data->gss_requested = FALSE;
+ return tvb_captured_length(tvb);
+ }
+
+ if (conn_data && (pinfo->num > conn_data->last_nongss_frame)) {
+ tvbuff_t *subtvb;
+ gssapi_encrypt_info_t encrypt;
+
+ memset(&encrypt, 0, sizeof(encrypt));
+ encrypt.decrypt_gssapi_tvb = DECRYPT_GSSAPI_NORMAL;
+
+ dissect_gssenc_msg(tvb, pinfo, tree, &encrypt);
+
+ if (encrypt.gssapi_decrypted_tvb){
+ subtvb = encrypt.gssapi_decrypted_tvb;
+ tcp_dissect_pdus(subtvb, pinfo, tree, pgsql_desegment, 5,
+ pgsql_length, dissect_pgsql_msg, data);
+ } else if (encrypt.gssapi_encrypted_tvb) {
+ subtvb = encrypt.gssapi_encrypted_tvb;
+ proto_tree_add_expert(tree, pinfo, &ei_gssapi_decryption_not_possible, subtvb, 0, -1);
+ }
+
+ return tvb_captured_length(tvb);
+ }
+
tcp_dissect_pdus(tvb, pinfo, tree, pgsql_desegment, 5,
pgsql_length, dissect_pgsql_msg, data);
return tvb_captured_length(tvb);
@@ -1021,7 +1117,12 @@ proto_register_pgsql(void)
{ &hf_routine,
{ "Routine", "pgsql.routine", FT_STRINGZ, BASE_NONE, NULL, 0,
"The routine that reported an error.", HFILL }
- }
+ },
+ { &hf_gssapi_length,
+ { "Length of GSSAPI encrypted data", "pgsql.gss.length", FT_UINT32, BASE_DEC, NULL, 0,
+ "The length of the GSSAPI encrypted blob.",
+ HFILL }
+ },
};
static gint *ett[] = {
@@ -1029,9 +1130,18 @@ proto_register_pgsql(void)
&ett_values
};
+ expert_module_t* expert_pgsql;
+
+ static ei_register_info ei[] = {
+ { &ei_gssapi_decryption_not_possible, { "pgsql.gss.decryption_not_possible", PI_UNDECODED, PI_WARN, "The PGSQL dissector could not decrypt the message.", EXPFILL }},
+ };
+
proto_pgsql = proto_register_protocol("PostgreSQL", "PGSQL", "pgsql");
proto_register_field_array(proto_pgsql, hf, array_length(hf));
proto_register_subtree_array(ett, array_length(ett));
+
+ expert_pgsql = expert_register_protocol(proto_pgsql);
+ expert_register_field_array(expert_pgsql, ei, array_length(ei));
}
void
@@ -1042,6 +1152,7 @@ proto_reg_handoff_pgsql(void)
dissector_add_uint_with_preference("tcp.port", PGSQL_PORT, pgsql_handle);
tls_handle = find_dissector_add_dependency("tls", proto_pgsql);
+ gssapi_handle = find_dissector_add_dependency("gssapi", proto_pgsql);
}
/*
--
2.25.1
Attachments:
[text/plain] GPLv2-Wireshark3.7.1-pgsql-decrypt-GSS.patch.txt (10.5K, ../../CAAWbhmjwUC4kHb+yvJiTi4XQOKEqbOujErXXOx_phC9c8qN7pQ@mail.gmail.com/2-GPLv2-Wireshark3.7.1-pgsql-decrypt-GSS.patch.txt)
download | inline diff:
From 0cf31522223a6044edae42037e45aee0fc88352f Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Wed, 14 Sep 2022 13:08:22 -0700
Subject: [PATCH] pgsql: decrypt GSS-encrypted channels and tokens if possible
License: GPLv2
Add dissection for a conversation that starts with a GSSAPI encryption
request, and pass those packets along to the gssapi dissector. This
allows parts of the conversation to be decrypted if Wireshark has been
set up with a KRB5 keytab.
Additionally, break apart the PGSQL_AUTH_GSSAPI_SSPI_DATA case into
separate GSSAPI and SSPI cases, and when we're using GSSAPI, dissect the
authentication tokens that are passed between the client and server.
This incidentally fixes an off-by-four bug in the
PGSQL_AUTH_TYPE_GSSAPI_SSPI_CONTINUE case; the offset was not being
updated correctly before.
The new code copies the dissection strategy of multipart, and adds a new
last_nongss_frame marker similar to the STARTTLS handling code's
last_nontls_frame.
---
epan/dissectors/packet-pgsql.c | 121 +++++++++++++++++++++++++++++++--
1 file changed, 116 insertions(+), 5 deletions(-)
diff --git a/epan/dissectors/packet-pgsql.c b/epan/dissectors/packet-pgsql.c
index c935eec1f8..7efe1c003f 100644
--- a/epan/dissectors/packet-pgsql.c
+++ b/epan/dissectors/packet-pgsql.c
@@ -14,6 +14,8 @@
#include <epan/packet.h>
+#include "packet-dcerpc.h"
+#include "packet-gssapi.h"
#include "packet-tls-utils.h"
#include "packet-tcp.h"
@@ -22,6 +24,7 @@ void proto_reg_handoff_pgsql(void);
static dissector_handle_t pgsql_handle;
static dissector_handle_t tls_handle;
+static dissector_handle_t gssapi_handle;
static int proto_pgsql = -1;
static int hf_frontend = -1;
@@ -80,10 +83,13 @@ static int hf_constraint_name = -1;
static int hf_file = -1;
static int hf_line = -1;
static int hf_routine = -1;
+static int hf_gssapi_length = -1;
static gint ett_pgsql = -1;
static gint ett_values = -1;
+static expert_field ei_gssapi_decryption_not_possible = EI_INIT;
+
#define PGSQL_PORT 5432
static gboolean pgsql_desegment = TRUE;
static gboolean first_message = TRUE;
@@ -92,11 +98,14 @@ typedef enum {
PGSQL_AUTH_STATE_NONE, /* No authentication seen or used */
PGSQL_AUTH_SASL_REQUESTED, /* Server sends SASL auth request with supported SASL mechanisms*/
PGSQL_AUTH_SASL_CONTINUE, /* Server and/or client send further SASL challange-response messages */
- PGSQL_AUTH_GSSAPI_SSPI_DATA, /* GSSAPI/SSPI in use */
+ PGSQL_AUTH_GSSAPI, /* GSSAPI in use */
+ PGSQL_AUTH_SSPI, /* SSPI in use */
} pgsql_auth_state_t;
typedef struct pgsql_conn_data {
gboolean ssl_requested;
+ gboolean gss_requested;
+ guint32 last_nongss_frame;
pgsql_auth_state_t auth_state; /* Current authentication state */
} pgsql_conn_data_t;
@@ -189,6 +198,22 @@ static const value_string format_vals[] = {
{ 0, NULL }
};
+static void
+dissect_gssapi_data(tvbuff_t *tvb, gint offset, guint len, packet_info *pinfo,
+ proto_tree *tree, gssapi_encrypt_info_t *encrypt)
+{
+ tvbuff_t *gssapi_tvb;
+ guint8 *data;
+
+ DISSECTOR_ASSERT(tvb_bytes_exist(tvb, offset, len));
+
+ data = (guint8 *)tvb_memdup(pinfo->pool, tvb, offset, len);
+ gssapi_tvb = tvb_new_child_real_data(tvb, data, len, len);
+
+ add_new_data_source(pinfo, gssapi_tvb, "GSSAPI Data");
+ call_dissector_with_data(gssapi_handle, gssapi_tvb, pinfo, tree, encrypt);
+}
+
static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
gint n, proto_tree *tree, packet_info *pinfo,
pgsql_conn_data_t *conv_data)
@@ -220,7 +245,12 @@ static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
proto_tree_add_item(tree, hf_sasl_auth_data, tvb, n, length-4, ENC_NA);
break;
- case PGSQL_AUTH_GSSAPI_SSPI_DATA:
+ case PGSQL_AUTH_GSSAPI:
+ proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-4, ENC_NA);
+ dissect_gssapi_data(tvb, n, length-4, pinfo, tree, NULL);
+ break;
+
+ case PGSQL_AUTH_SSPI:
proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-4, ENC_NA);
break;
@@ -356,6 +386,12 @@ static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
conv_data->ssl_requested = TRUE;
break;
+ /* GSS request */
+ case 80877104:
+ /* Next reply will be a single byte. */
+ conv_data->gss_requested = TRUE;
+ break;
+
/* Cancellation request */
case 80877102:
proto_tree_add_item(tree, hf_pid, tvb, n, 4, ENC_BIG_ENDIAN);
@@ -430,9 +466,17 @@ static void dissect_pgsql_be_msg(guchar type, guint length, tvbuff_t *tvb,
siz = (auth_type == PGSQL_AUTH_TYPE_CRYPT ? 2 : 4);
proto_tree_add_item(tree, hf_salt, tvb, n, siz, ENC_NA);
break;
+ case PGSQL_AUTH_TYPE_GSSAPI:
+ conv_data->auth_state = PGSQL_AUTH_GSSAPI;
+ break;
+ case PGSQL_AUTH_TYPE_SSPI:
+ conv_data->auth_state = PGSQL_AUTH_SSPI;
+ break;
case PGSQL_AUTH_TYPE_GSSAPI_SSPI_CONTINUE:
- conv_data->auth_state = PGSQL_AUTH_GSSAPI_SSPI_DATA;
+ n += 4;
proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-8, ENC_NA);
+ if (conv_data->auth_state == PGSQL_AUTH_GSSAPI)
+ dissect_gssapi_data(tvb, n, length-8, pinfo, tree, NULL);
break;
case PGSQL_AUTH_TYPE_SASL:
conv_data->auth_state = PGSQL_AUTH_SASL_REQUESTED;
@@ -665,6 +709,8 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
if (!conn_data) {
conn_data = wmem_new(wmem_file_scope(), pgsql_conn_data_t);
conn_data->ssl_requested = FALSE;
+ conn_data->gss_requested = FALSE;
+ conn_data->last_nongss_frame = G_MAXUINT32;
conn_data->auth_state = PGSQL_AUTH_STATE_NONE;
conversation_add_proto_data(conversation, proto_pgsql, conn_data);
}
@@ -703,7 +749,8 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
case PGSQL_AUTH_SASL_CONTINUE:
typestr = "SASLResponse message";
break;
- case PGSQL_AUTH_GSSAPI_SSPI_DATA:
+ case PGSQL_AUTH_GSSAPI: /* fallthrough */
+ case PGSQL_AUTH_SSPI:
typestr = "GSSResponse message";
break;
default:
@@ -746,6 +793,18 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
return tvb_captured_length(tvb);
}
+static void
+dissect_gssenc_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, gssapi_encrypt_info_t *encrypt)
+{
+ gint offset = 0, len;
+
+ proto_tree_add_item(tree, hf_gssapi_length, tvb, offset, 4, ENC_BIG_ENDIAN);
+ offset += 4;
+ len = tvb_reported_length_remaining(tvb, offset);
+
+ dissect_gssapi_data(tvb, offset, len, pinfo, tree, encrypt);
+}
+
/* This function is called once per TCP packet. It sets COL_PROTOCOL and
* identifies FE/BE messages by adding a ">" or "<" to COL_INFO. Then it
* arranges for each message to be dissected individually. */
@@ -782,6 +841,43 @@ dissect_pgsql(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data)
return tvb_captured_length(tvb);
}
+ if (conn_data && conn_data->gss_requested) {
+ /* Response to GSS request. */
+ switch (tvb_get_guint8(tvb, 0)) {
+ case 'G': /* Willing to perform GSS encryption */
+ /* Next packet will start using GSS encryption. */
+ conn_data->last_nongss_frame = pinfo->num;
+ break;
+ case 'N': /* Unwilling to perform GSS encryption */
+ default: /* ErrorMessage when server does not support SSL. */
+ /* TODO: maybe add expert info here? */
+ break;
+ }
+ conn_data->gss_requested = FALSE;
+ return tvb_captured_length(tvb);
+ }
+
+ if (conn_data && (pinfo->num > conn_data->last_nongss_frame)) {
+ tvbuff_t *subtvb;
+ gssapi_encrypt_info_t encrypt;
+
+ memset(&encrypt, 0, sizeof(encrypt));
+ encrypt.decrypt_gssapi_tvb = DECRYPT_GSSAPI_NORMAL;
+
+ dissect_gssenc_msg(tvb, pinfo, tree, &encrypt);
+
+ if (encrypt.gssapi_decrypted_tvb){
+ subtvb = encrypt.gssapi_decrypted_tvb;
+ tcp_dissect_pdus(subtvb, pinfo, tree, pgsql_desegment, 5,
+ pgsql_length, dissect_pgsql_msg, data);
+ } else if (encrypt.gssapi_encrypted_tvb) {
+ subtvb = encrypt.gssapi_encrypted_tvb;
+ proto_tree_add_expert(tree, pinfo, &ei_gssapi_decryption_not_possible, subtvb, 0, -1);
+ }
+
+ return tvb_captured_length(tvb);
+ }
+
tcp_dissect_pdus(tvb, pinfo, tree, pgsql_desegment, 5,
pgsql_length, dissect_pgsql_msg, data);
return tvb_captured_length(tvb);
@@ -1021,7 +1117,12 @@ proto_register_pgsql(void)
{ &hf_routine,
{ "Routine", "pgsql.routine", FT_STRINGZ, BASE_NONE, NULL, 0,
"The routine that reported an error.", HFILL }
- }
+ },
+ { &hf_gssapi_length,
+ { "Length of GSSAPI encrypted data", "pgsql.gss.length", FT_UINT32, BASE_DEC, NULL, 0,
+ "The length of the GSSAPI encrypted blob.",
+ HFILL }
+ },
};
static gint *ett[] = {
@@ -1029,9 +1130,18 @@ proto_register_pgsql(void)
&ett_values
};
+ expert_module_t* expert_pgsql;
+
+ static ei_register_info ei[] = {
+ { &ei_gssapi_decryption_not_possible, { "pgsql.gss.decryption_not_possible", PI_UNDECODED, PI_WARN, "The PGSQL dissector could not decrypt the message.", EXPFILL }},
+ };
+
proto_pgsql = proto_register_protocol("PostgreSQL", "PGSQL", "pgsql");
proto_register_field_array(proto_pgsql, hf, array_length(hf));
proto_register_subtree_array(ett, array_length(ett));
+
+ expert_pgsql = expert_register_protocol(proto_pgsql);
+ expert_register_field_array(expert_pgsql, ei, array_length(ei));
}
void
@@ -1042,6 +1152,7 @@ proto_reg_handoff_pgsql(void)
dissector_add_uint_with_preference("tcp.port", PGSQL_PORT, pgsql_handle);
tls_handle = find_dissector_add_dependency("tls", proto_pgsql);
+ gssapi_handle = find_dissector_add_dependency("gssapi", proto_pgsql);
}
/*
--
2.25.1
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
2022-07-07 23:24 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
2022-09-16 00:06 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
@ 2022-09-19 17:05 ` Stephen Frost <[email protected]>
2022-09-19 21:05 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Stephen Frost @ 2022-09-19 17:05 UTC (permalink / raw)
To: Jacob Champion <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Magnus Hagander <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>
Greetings,
* Jacob Champion ([email protected]) wrote:
> On Thu, Jul 7, 2022 at 4:24 PM Jacob Champion <[email protected]> wrote:
> > So my question is this: does substituting my credentials for the admin's
> > credentials let me weaken or break the transport encryption on the
> > backend connection, and grab the password that I'm not supposed to have
> > access to as a front-end client?
>
> With some further research: yes, it does.
>
> If a DBA is using a GSS encrypted tunnel to communicate to a foreign
> server, accepting delegation by default means that clients will be
> able to break that backend encryption at will, because the keys in use
> will be under their control.
This is coming across as if it's a surprise of some kind when it
certainly isn't.. If the delegated credentials are being used to
authenticate and establish the connection from that backend to another
system then, yes, naturally that means that the keys provided are coming
from the client and the client knows them. The idea of arranging to
have an admin's credentials used to authenticate to another system where
the backend is actually controlled by a non-admin user is, in fact, the
issue in what is being outlined above as that's clearly a situation
where the user's connection is being elevated to an admin level. That's
also something that we try to avoid having happen because it's not
really a good idea, which is why we require a password today for the
connection to be established (postgres_fdw/connection.c:
Non-superuser cannot connect if the server does not request a password.
).
Consider that, in general, the user could also simply directly connect
to the other system themselves instead of having a PG backend make that
connection for them- the point in doing it from PG would be to avoid
having to pass all the data back through the client's system.
Consider SSH instead of PG. What you're pointing out, accurately, is
that if an admin were to install their keys into a user's .ssh directory
unencrypted and then the user logged into the system, they'd then be
able to SSH to another system with the admin's credentials and then
they'd need the admin's credentials to decrypt the traffic, but that if,
instead, the user brings their own credentials then they could
potentially decrypt the connection between the systems. Is that really
the issue here? Doesn't seem like that's where the concern should be in
this scenario.
> > Maybe there's some ephemeral exchange going on that makes it too hard to
> > attack in practice, or some other mitigations.
>
> There is no forward secrecy, ephemeral exchange, etc. to mitigate this [1]:
>
> The Kerberos protocol in its basic form does not provide perfect
> forward secrecy for communications. If traffic has been recorded by
> an eavesdropper, then messages encrypted using the KRB_PRIV message,
> or messages encrypted using application-specific encryption under
> keys exchanged using Kerberos can be decrypted if the user's,
> application server's, or KDC's key is subsequently discovered.
>
> So the client can decrypt backend communications that make use of its
> delegated key material. (This also means that gssencmode is a lot
> weaker than I expected.)
The backend wouldn't be able to establish the connection in the first
place without those delegated credentials.
> > I'm trying to avoid writing Wireshark dissector
> > code, but maybe that'd be useful either way...
>
> I did end up filling out the existing PGSQL dissector so that it could
> decrypt GSSAPI exchanges (with the use of a keytab, that is). If you'd
> like to give it a try, the patch, based on Wireshark 3.7.1, is
> attached. Note the GPLv2 license. It isn't correct code yet, because I
> didn't understand how packet reassembly worked in Wireshark when I
> started writing the code, so really large GSSAPI messages that are
> split across multiple TCP packets will confuse the dissector. But it's
> enough to prove the concept.
>
> To see this in action, set up an FDW connection that uses gssencmode
> (so the server in the middle will need its own Kerberos credentials).
The server in the middle should *not* be using its own Kerberos
credentials to establish the connection to the other system- that's
elevating the credentials used for that connection and is something that
should be prevented for non-superusers already (see above). We do allow
that when a superuser is involved because they are considered to
essentially have OS-level privileges and therefore could see those
credentials anyway, but that's not the case for non-superusers.
Thanks,
Stephen
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
2022-07-07 23:24 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
2022-09-16 00:06 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
2022-09-19 17:05 ` Re: Kerberos delegation support in libpq and postgres_fdw Stephen Frost <[email protected]>
@ 2022-09-19 21:05 ` Jacob Champion <[email protected]>
2022-10-12 05:32 ` Re: Kerberos delegation support in libpq and postgres_fdw Michael Paquier <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Jacob Champion @ 2022-09-19 21:05 UTC (permalink / raw)
To: Stephen Frost <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Magnus Hagander <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>
On 9/19/22 10:05, Stephen Frost wrote:
> This is coming across as if it's a surprise of some kind when it
> certainly isn't.. If the delegated credentials are being used to
> authenticate and establish the connection from that backend to another
> system then, yes, naturally that means that the keys provided are coming
> from the client and the client knows them.
I think it may be surprising to end users that credential delegation
lets them trivially break transport encryption. Like I said before, it
was a surprise to me, because the cryptosystems I'm familiar with
prevent that.
If it wasn't surprising to you, I could really have used a heads up back
when I asked you about it.
> The idea of arranging to
> have an admin's credentials used to authenticate to another system where
> the backend is actually controlled by a non-admin user is, in fact, the
> issue in what is being outlined above as that's clearly a situation
> where the user's connection is being elevated to an admin level.
Yes, controlled elevation is the goal in the scenario I'm describing.
> That's
> also something that we try to avoid having happen because it's not
> really a good idea, which is why we require a password today for the
> connection to be established (postgres_fdw/connection.c:
>
> Non-superuser cannot connect if the server does not request a password.
A password is being used in this scenario. The password is the secret
being stolen.
The rest of your email describes a scenario different from what I'm
attacking here. Here's my sample HBA line for the backend again:
hostgssenc all all ... password
I'm using password authentication with a Kerberos-encrypted channel.
It's similar to protecting password authentication with TLS and a client
cert:
hostssl all all ... password clientcert=verify-*
> Consider that, in general, the user could also simply directly connect
> to the other system themselves
No, because they don't have the password. They don't have USAGE on the
foreign table, so they can't see the password in the USER MAPPING.
With the new default introduced in this patch, they can now steal the
password by delegating their credentials and cracking the transport
encryption. This bypasses the protections that are documented for the
pg_user_mappings view.
> Consider SSH instead of PG. What you're pointing out, accurately, is
> that if an admin were to install their keys into a user's .ssh directory
> unencrypted and then the user logged into the system, they'd then be
> able to SSH to another system with the admin's credentials and then
> they'd need the admin's credentials to decrypt the traffic, but that if,
> instead, the user brings their own credentials then they could
> potentially decrypt the connection between the systems. Is that really
> the issue here?
No, it's not the issue here. This is more like setting up a restricted
shell that provides limited access to a resource on another machine
(analogous to the foreign table). The user SSHs into this restricted
shell, and then invokes an admin-blessed command whose implementation
uses some credentials (which they cannot read, analogous to the USER
MAPPING) over an encrypted channel to access the backend resource. In
this situation an admin would want to ensure that the encrypted tunnel
couldn't be weakened by the client, so that they can't learn how to
bypass the blessed command and connect to the backend directly.
Unlike SSH, we've never supported credential delegation, and now we're
introducing it. So my claim is, it's possible for someone who was
previously in a secure situation to be broken by the new default.
>> So the client can decrypt backend communications that make use of its
>> delegated key material. (This also means that gssencmode is a lot
>> weaker than I expected.)
>
> The backend wouldn't be able to establish the connection in the first
> place without those delegated credentials.
That's not true in the situation I'm describing; hopefully my comments
above help clarify.
> The server in the middle should *not* be using its own Kerberos
> credentials to establish the connection to the other system- that's
> elevating the credentials used for that connection and is something that
> should be prevented for non-superusers already (see above).
It's not prevented, because a password is being used. In my tests I'm
connecting as an unprivileged user.
You're claiming that the middlebox shouldn't be doing this. If this new
default behavior were the historical behavior, then I would have agreed.
But the cat's already out of the bag on that, right? It's safe today.
And if it's not safe today for some other reason, please share why, and
maybe I can work on a patch to try to prevent people from doing it.
Thanks,
--Jacob
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Kerberos delegation support in libpq and postgres_fdw
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Re: Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Re: Kerberos delegation support in libpq and postgres_fdw Peter Eisentraut <[email protected]>
2022-07-07 23:24 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
2022-09-16 00:06 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
2022-09-19 17:05 ` Re: Kerberos delegation support in libpq and postgres_fdw Stephen Frost <[email protected]>
2022-09-19 21:05 ` Re: Kerberos delegation support in libpq and postgres_fdw Jacob Champion <[email protected]>
@ 2022-10-12 05:32 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Michael Paquier @ 2022-10-12 05:32 UTC (permalink / raw)
To: Jacob Champion <[email protected]>; +Cc: Stephen Frost <[email protected]>; pgsql-hackers <[email protected]>; Magnus Hagander <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>
On Mon, Sep 19, 2022 at 02:05:39PM -0700, Jacob Champion wrote:
> It's not prevented, because a password is being used. In my tests I'm
> connecting as an unprivileged user.
>
> You're claiming that the middlebox shouldn't be doing this. If this new
> default behavior were the historical behavior, then I would have agreed.
> But the cat's already out of the bag on that, right? It's safe today.
> And if it's not safe today for some other reason, please share why, and
> maybe I can work on a patch to try to prevent people from doing it.
Please note that this has been marked as returned with feedback in the
current CF, as this has remained unanswered for a bit more than three
weeks.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: 003_check_guc.pl crashes if some extensions were loaded.
@ 2023-11-02 04:08 Anton A. Melnikov <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Anton A. Melnikov @ 2023-11-02 04:08 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On 02.11.2023 01:53, Michael Paquier wrote:> On Thu, Nov 02, 2023 at 12:28:05AM +0300, Anton A. Melnikov wrote:
>> Found that src/test/modules/test_misc/t/003_check_guc.pl will crash if an extension
>> that adds own GUCs was loaded into memory.
>> So it is now impossible to run a check-world with loaded extension libraries.
>
> Right. That's annoying, so let's fix it.
Thanks!
On 02.11.2023 02:29, Tom Lane wrote:
> Michael Paquier <[email protected]> writes:
>> Wouldn't it be better to add a qual as of "category <> 'Customized
>> Options'"?
>
> +1, seems like a cleaner answer.
Also agreed. That is a better variant!
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v2 02/17] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index e2f2c37f4d6..4600ee53751 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -61,8 +61,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -454,7 +453,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -484,7 +483,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -505,7 +504,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -528,7 +527,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -625,7 +624,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0003-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v7 02/16] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4e58c2c2ff4..c1542b95af8 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -427,7 +426,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -457,7 +456,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -478,7 +477,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -501,7 +500,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -598,7 +597,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0003-Rename-PruneState-snapshotConflictHorizon-to-late.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v3 02/17] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4f12413b8b1..4a2bf3dd780 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -61,8 +61,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -454,7 +453,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -484,7 +483,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -505,7 +504,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -528,7 +527,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -625,7 +624,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0003-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v3 02/17] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4f12413b8b1..4a2bf3dd780 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -61,8 +61,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -454,7 +453,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -484,7 +483,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -505,7 +504,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -528,7 +527,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -625,7 +624,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0003-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v9 02/21] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ef816c2fa9c..29c3c98b0e7 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -427,7 +426,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -453,7 +452,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -474,7 +473,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -497,7 +496,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -594,7 +593,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0003-Rename-PruneState-snapshotConflictHorizon-to-late.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v3 02/17] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4f12413b8b1..4a2bf3dd780 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -61,8 +61,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -454,7 +453,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -484,7 +483,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -505,7 +504,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -528,7 +527,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -625,7 +624,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0003-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 08cb2a6f533..7eb21b603ba 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -322,7 +321,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -451,7 +450,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -481,7 +480,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -502,7 +501,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -525,7 +524,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -622,7 +621,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0005-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v2 02/17] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index e2f2c37f4d6..4600ee53751 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -61,8 +61,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -454,7 +453,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -484,7 +483,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -505,7 +504,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -528,7 +527,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -625,7 +624,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0003-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 08cb2a6f533..7eb21b603ba 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -322,7 +321,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -451,7 +450,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -481,7 +480,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -502,7 +501,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -525,7 +524,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -622,7 +621,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0005-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v9 02/21] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ef816c2fa9c..29c3c98b0e7 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -427,7 +426,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -453,7 +452,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -474,7 +473,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -497,7 +496,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -594,7 +593,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0003-Rename-PruneState-snapshotConflictHorizon-to-late.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v2 02/17] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index e2f2c37f4d6..4600ee53751 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -61,8 +61,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -454,7 +453,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -484,7 +483,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -505,7 +504,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -528,7 +527,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -625,7 +624,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0003-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 08cb2a6f533..7eb21b603ba 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -322,7 +321,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -451,7 +450,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -481,7 +480,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -502,7 +501,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -525,7 +524,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -622,7 +621,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0005-heap_page_prune-sets-all_visible-and-frz_conflict.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v7 02/16] Pass heap_prune_chain() PruneResult output parameter
@ 2024-01-06 18:39 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-06 18:39 UTC (permalink / raw)
Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
src/backend/access/heap/pruneheap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4e58c2c2ff4..c1542b95af8 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
Buffer buffer);
static int heap_prune_chain(Buffer buffer,
OffsetNumber rootoffnum,
- int8 *htsv,
- PruneState *prstate);
+ PruneState *prstate, PruneResult *presult);
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -325,7 +324,7 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Process this item or chain of items */
presult->ndeleted += heap_prune_chain(buffer, offnum,
- presult->htsv, &prstate);
+ &prstate, presult);
}
/* Clear the offset information once we have processed the given page. */
@@ -427,7 +426,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
/*
* Prune specified line pointer or a HOT chain originating at line pointer.
*
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
*
* If the item is an index-referenced tuple (i.e. not a heap-only tuple),
* the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -457,7 +456,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
*/
static int
heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
- int8 *htsv, PruneState *prstate)
+ PruneState *prstate, PruneResult *presult)
{
int ndeleted = 0;
Page dp = (Page) BufferGetPage(buffer);
@@ -478,7 +477,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
if (ItemIdIsNormal(rootlp))
{
- Assert(htsv[rootoffnum] != -1);
+ Assert(presult->htsv[rootoffnum] != -1);
htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
if (HeapTupleHeaderIsHeapOnly(htup))
@@ -501,7 +500,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* either here or while following a chain below. Whichever path
* gets there first will mark the tuple unused.
*/
- if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+ if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
!HeapTupleHeaderIsHotUpdated(htup))
{
heap_prune_record_unused(prstate, rootoffnum);
@@ -598,7 +597,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
*/
tupdead = recent_dead = false;
- switch (htsv_get_valid_status(htsv[offnum]))
+ switch (htsv_get_valid_status(presult->htsv[offnum]))
{
case HEAPTUPLE_DEAD:
tupdead = true;
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0003-Rename-PruneState-snapshotConflictHorizon-to-late.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
end of thread, other threads:[~2024-01-06 18:39 UTC | newest]
Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 03:05 Kerberos delegation support in libpq and postgres_fdw Peifeng Qiu <[email protected]>
2021-07-22 08:39 ` Peifeng Qiu <[email protected]>
2021-09-01 08:57 ` Peter Eisentraut <[email protected]>
2021-11-03 12:41 ` Daniel Gustafsson <[email protected]>
2021-11-26 13:41 ` Daniel Gustafsson <[email protected]>
2022-07-07 23:24 ` Jacob Champion <[email protected]>
2022-09-16 00:06 ` Jacob Champion <[email protected]>
2022-09-19 17:05 ` Stephen Frost <[email protected]>
2022-09-19 21:05 ` Jacob Champion <[email protected]>
2022-10-12 05:32 ` Michael Paquier <[email protected]>
2023-11-02 04:08 Re: 003_check_guc.pl crashes if some extensions were loaded. Anton A. Melnikov <[email protected]>
2024-01-06 18:39 [PATCH v2 02/17] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v7 02/16] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v3 02/17] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v3 02/17] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v9 02/21] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v3 02/17] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v2 02/17] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v9 02/21] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v2 02/17] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[email protected]>
2024-01-06 18:39 [PATCH v7 02/16] Pass heap_prune_chain() PruneResult output parameter Melanie Plageman <[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