Re: Logging of matching pg_hba.conf entry during auth skips trust auth, potential security issue
> We could do something like a LOG "connection: method=%s user=%s > (%s:%d)", without the "authenticated" and "identity" terms from > set_authn_id(). Just to drop an idea. That would be my inclination as well. Heck, just slap a log message right in the specific case statements that don't have actual auth as defined by set_authn_id. This assumes anyone really cares about it that much, of course. :D -- Shaun
Logging of matching pg_hba.conf entry during auth skips trust auth, potential security issue
Hi everyone, This started as a conversation on Discord. Someone asked if Postgres logs which line in pg_hba.conf matched against a certain login attempt, and I said no. That's not quite right, as enabling log_connections includes a line like this: 2023-08-15 13:26:03.159 PDT [692166] postgres@snip LOG: connection authenticated: identity="postgres" method=md5 (/etc/postgresql/15/main/pg_hba.conf:107) But I wasn't getting that output. I finally gave up and looked at the code, where I found that this particular output is only generated by the set_authn_id function. So if that function is never called, there's no message saying which line from the pg_hba.conf file matched a particular login. The switch statement that decodes port->hba->auth_method ends by simply setting status = STATUS_OK; with no supplementary output since it never calls set_authn_id. So in theory, a malicious user could add a trust line to pg_hba.conf and have unlimited unlogged access to the database. Unless you happen to notice that the "connection authenticated" line has disappeared, it would look like normal activity. Would it make sense to decouple the hba info from set_authn_id so that it is always logged even when new auth methods get added in the future? Or alternatively create a function call specifically for that output so it can be produced from the trust case statement and anywhere else that needs to tag the auth line. I personally would love to see if someone got in through a trust line, ESPECIALLY if it isn't supposed to be there. Like: 2023-08-15 13:26:03.159 PDT [692166] postgres@snip LOG: connection authenticated: identity="postgres" method=trust (/etc/postgresql/15/main/pg_hba.conf:1) Perhaps I'm being too paranoid; It just seemed to be an odd omission. Euler Taveira clued me into the initial patch which introduced the pg_hba.conf tattling: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9afffcb833d3c5e59a328a2af674fac7e7334fc1 I read through the discussion, and it doesn't seem like the security aspect of simply hiding trust auths from the log was considered. Since this is a new capability, I suppose nothing is really different from say Postgres 14 and below. Still, it never hurts to ask. Cheers! -- Shaun Thomas High Availability Architect EDB www.enterprisedb.com
Re: [PATCH] Incremental sort (was: PoC: Partial sort)
Another ping on this Incremental Sort patch. Alexander, you'd noted that you would try to get it into subsequent Commit Fests with improvements you've been considering, but I don't see it in anything but 2018-11. Have you abandoned this as a maintainer? If so, it would be nice to know so someone else can pick it up. -- Shaun M Thomas - 2ndQuadrant PostgreSQL Training, Services and Support shaun.tho...@2ndquadrant.com | www.2ndQuadrant.com
Re: Re: A separate table level option to control compression
> Setting compress_tuple_target to a higher value won't be negative because the > toast_tuple_target is used for compression anyways when compress_tuple_target > is higher than toast_tuple_target. Ack, you're right. Got my wires crossed. I must have gotten confused by my later tests that enforced main storage and pushed out the toast tuple target to prevent out-of-line storage. > I added a small discussion about negative effects of setting > compress_tuple_target > lower though, per your suggestion. Fair enough. I like the addition since it provides a very concrete reason not to abuse the parameter. :shipit: Vaya con Queso -- Shaun M Thomas - 2ndQuadrant PostgreSQL Training, Services and Support shaun.tho...@2ndquadrant.com | www.2ndQuadrant.com
Re: Re: A separate table level option to control compression
Jumping in here, please be gentle. :) Contents & Purpose == This appears to be a patch to add a new table storage option similar to `toast_tuple_target` but geared toward compression. As a result, it's been named `compress_tuple_target`, and allows modifying the threshold where inline tuples actually become compressed. If we're going to make the toast threshold configurable, it tends to make sense we'd do the same for the compression threshold. The patch includes necessary documentation to describe the storage parameter along with limitations and fallback operating modes. Several tests are also included. Verification Procedure == The patch applied clean to HEAD, which was at commit 28988a84c... by Peter Eisentraut, at the time of this review. Build succeeded without issue, as did `make check` and `make installcheck`. In addition, I also performed the following manual verification steps using table page count and `pgstattuple` page distribution for a 10-row table with a junk column in these scenarios: * A standard table with a 1000-byte junk column not using this option: 2 pages at 66% density * A table with a 1000-byte junk and `compress_tuple_target` set to 512: 1 page at 6% density; the low threshold activated compression * A table with a 8120-byte junk and `compress_tuple_target` + `toast_tuple_target` set to 8160. Further, junk column was set to `main` storage to prevent standard toast thresholds from interfering: 10 pages at 99.5% density; no compression activated despite large column * A table with a 8140-byte junk and `compress_tuple_target` + `toast_tuple_target` set to 8180. Further, junk column was set to `main` storage to prevent standard toast thresholds from interfering: 1 page at 16% density; compression threshold > 8160 ignored as documented. Additionally, neither `compress_tuple_target` or `toast_tuple_target` were saved in `pg_class`. I also performed a `pg_dump` of a table using `compress_tuple_target`, and dump faithfully preserved the option in the expected location before the DATA portion. Discussion == Generally this ended about as I expected. I suspect much of the existing code was cribbed from the implementation of `toast_tuple_target` given the similar entrypoints and the already existing hard-coded compression thresholds. I can't really speak for the discussion related to `storage.sgml`, but I based my investigation on the existing patch to `create_table.sgml`. About the only thing I would suggest there is to possibly tweak the wording. * "The compress_tuple_target ... " for example should probably read "The toast_tuple_target parameter ...". * "the (blocksize - header)" can drop "the". * "If the value is set to a value" redundant wording should be rephrased; "If the specified value is greater than toast_tuple_target, then we will substitute the current setting of toast_tuple_target instead." would work. * I'd recommend a short discussion on what negative consequences can be expected by playing with this value. As an example in my tests, setting it very high may result in extremely sparse pages that could have an adverse impact on HOT updates. Still, those are just minor nitpicks, and I don't expect that to affect the quality of the patch implementation. Good show, gents! -- Shaun M Thomas - 2ndQuadrant PostgreSQL Training, Services and Support shaun.tho...@2ndquadrant.com | www.2ndQuadrant.com
