TLS checking in pgstat
As I mentioned in [1]FAB21FC8-0F62-434F-AA78-6BD9336D630A@yesql.se, checking (struct Port)->ssl for NULL to determine
whether TLS is used for connection is a bit of a leaky abstraction, as that's
an OpenSSL specific struct member. This sets the requirement that all TLS
implementations use a pointer named SSL, and that the pointer is set to NULL in
case of a failed connection, which may or may not fit.
Is there a reason to not use (struct Port)->ssl_in_use flag which tracks just
what we're looking for here? This also maps against other parts of the
abstraction in be-secure.c which do just that. The attached implements this.
cheers ./daniel
[1]: FAB21FC8-0F62-434F-AA78-6BD9336D630A@yesql.se
Attachments:
ssl_reporting.patchapplication/octet-stream; name=ssl_reporting.patch; x-unix-mode=0644Download
From 2336500690c55f633cbbb075db335d671d1aa705 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Sun, 28 Jun 2020 12:58:39 +0200
Subject: [PATCH] Make pg_stat_ssl reporting backend agnostic
Inspecting Port->ssl for an indication on whether the connection is
using TLS or not is tied to the fact that the current implementation
is using a variable named ssl. Making this a requirement for all TLS
backend implementations seems restricting since there in actual var
tracking the status, ssl_in_use. Switch to inspecting this variable
instead.
---
src/backend/postmaster/pgstat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index c022597bc0..edfa774ee4 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2989,7 +2989,7 @@ pgstat_bestart(void)
MemSet(&lbeentry.st_clientaddr, 0, sizeof(lbeentry.st_clientaddr));
#ifdef USE_SSL
- if (MyProcPort && MyProcPort->ssl != NULL)
+ if (MyProcPort && MyProcPort->ssl_in_use)
{
lbeentry.st_ssl = true;
lsslstatus.ssl_bits = be_tls_get_cipher_bits(MyProcPort);
--
2.21.1 (Apple Git-122.3)
On Sun, Jun 28, 2020 at 1:39 PM Daniel Gustafsson <daniel@yesql.se> wrote:
As I mentioned in [1], checking (struct Port)->ssl for NULL to determine
whether TLS is used for connection is a bit of a leaky abstraction, as
that's
an OpenSSL specific struct member. This sets the requirement that all TLS
implementations use a pointer named SSL, and that the pointer is set to
NULL in
case of a failed connection, which may or may not fit.Is there a reason to not use (struct Port)->ssl_in_use flag which tracks
just
what we're looking for here? This also maps against other parts of the
abstraction in be-secure.c which do just that. The attached implements
this.
Yeah, this seems perfectly reasonable.
I would argue this is a bug, but given how internal it is I don't think it
has any user visible effects yet (since we don't have more than one
provider), and thus isn't worthy of a backpatch.
Pushed.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>