Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-06 Thread Jacob Champion
On Fri, Mar 3, 2023 at 6:35 PM Michael Paquier wrote: > I was refreshing my mind with 0001 yesterday, and except for the two > parts where we need to worry about AUTH_REQ_OK being sent too early > and the business with gssenc, this is a rather straight-forward. It > also looks like the the partic

Re: zstd compression for pg_dump

2023-03-08 Thread Jacob Champion
On Sat, Mar 4, 2023 at 8:57 AM Justin Pryzby wrote: > pryzbyj=# CREATE TABLE t1 AS SELECT i,array_agg(j) FROM > generate_series(1,444)i,generate_series(1,9)j GROUP BY 1; > $ ./src/bin/pg_dump/pg_dump -d pryzbyj -Fc -Z zstd:long=1 |wc -c > 82023 > $ ./src/bin/pg_dump/pg_dump -d pryzbyj -Fc -Z

Re: proposal - get_extension_version function

2023-03-08 Thread Jacob Champion
On Wed, Mar 8, 2023 at 10:49 AM Tom Lane wrote: > This is a bad idea. How will you do extension upgrades, if the new .so > won't run till you apply the extension upgrade script but the old .so > malfunctions as soon as you do? Which upgrade paths allow you to have an old .so with a new version n

Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security

2023-03-08 Thread Jacob Champion
On Tue, Mar 7, 2023 at 11:04 AM Robert Haas wrote: > > On Thu, Feb 9, 2023 at 4:46 PM Jacob Champion wrote: > > On 2/6/23 08:22, Robert Haas wrote: > > > I don't think that's quite the right concept. It seems to me that the > > > client is responsi

Re: proposal - get_extension_version function

2023-03-08 Thread Jacob Champion
On Wed, Mar 8, 2023 at 11:18 AM Tom Lane wrote: > > Jacob Champion writes: > > On Wed, Mar 8, 2023 at 10:49 AM Tom Lane wrote: > >> This is a bad idea. How will you do extension upgrades, if the new .so > >> won't run till you apply the exten

Re: proposal - get_extension_version function

2023-03-08 Thread Jacob Champion
On Wed, Mar 8, 2023 at 11:11 AM Pavel Stehule wrote: > installation from rpm or deb packages Right, but I thought the safe order for a downgrade was to issue the SQL downgrade first (thus putting the system back into the post-upgrade state), and only then replacing the packages with prior version

Re: proposal - get_extension_version function

2023-03-08 Thread Jacob Champion
On Wed, Mar 8, 2023 at 11:22 AM Pavel Stehule wrote: > There is agreement - I call this check from functions. I think pg_auto_failover does this too, or at least used to. Timescale does strict compatibility checks as well. It's not entirely comparable to your implementation, though. --Jacob

Re: Can we let extensions change their dumped catalog schemas?

2023-03-08 Thread Jacob Champion
On Tue, Feb 7, 2023 at 10:16 AM Jacob Champion wrote: > Even more concretely, here's a draft patch. There's no nuance -- > setting dump_version affects all past versions of the extension, and > it can't be turned off at restore time. So extensions opting in would > ha

Re: RFC: logical publication via inheritance root?

2023-03-08 Thread Jacob Champion
On Tue, Mar 7, 2023 at 2:40 AM Aleksander Alekseev wrote: > So far I only have a couple of nitpicks, mostly regarding the code coverage > [1]: Yeah, I need to work on error cases and their coverage in general. There are more cases that I need to reject as well (marked TODO). > +Datum > +pg_get_

Re: proposal - get_extension_version function

2023-03-08 Thread Jacob Champion
On Wed, Mar 8, 2023 at 1:47 PM Tom Lane wrote: > Pavel's proposed check would break that too. There's going to be some > interval where the SQL definitions are not in sync with the .so version, > so you really want the .so to support at least two versions' worth of > SQL objects. I think we're i

Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security

2023-03-08 Thread Jacob Champion
On Wed, Mar 8, 2023 at 11:40 AM Robert Haas wrote: > On Wed, Mar 8, 2023 at 2:30 PM Jacob Champion wrote: > > I don't think I necessarily like that option better than SASL-style, > > but hopefully that clarifies it somewhat? > > Hmm, yeah, I guess that's OK. Okay,

Re: zstd compression for pg_dump

2023-03-10 Thread Jacob Champion
Hi, This'll need another rebase over the meson ICU changes. On Wed, Mar 8, 2023 at 10:59 AM Jacob Champion wrote: > I did some smoke testing against zstd's GitHub release on Windows. To > build against it, I had to construct an import library, and put that > and the DLL in

Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-10 Thread Jacob Champion
On 3/8/23 22:35, Michael Paquier wrote: > I have been reviewing 0001, finishing with the attached, and that's > nice work. My notes are below. Thanks! > pqDropServerData() is in charge of cleaning up the transient data of a > connection between different attempts. Shouldn't client_finished_auth

Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-10 Thread Jacob Champion
On Fri, Mar 10, 2023 at 3:09 PM Michael Paquier wrote: > >> + reason = libpq_gettext("server did not complete > >> authentication"), > >> -+ result = false; > >> ++ result = false; > >> + } > > > > This reindent

Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security

2023-03-10 Thread Jacob Champion
On Thu, Mar 9, 2023 at 6:17 AM Robert Haas wrote: > That seems like a circular argument. If you call the problem the > confused deputy problem then the issue must indeed be that the deputy > is confused, and needs to talk to someone else to get un-confused. But > why is the deputy necessarily conf

Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-13 Thread Jacob Champion
On Fri, Mar 10, 2023 at 3:16 PM Jacob Champion wrote: > > Could you send a new patch with all these adjustments? That would > > help a lot. > > Will do! Here's a v16: - updated 0001 patch message - all test names should have commas rather than colons now - new test fo

Re: Testing autovacuum wraparound (including failsafe)

2023-03-13 Thread Jacob Champion
On Sat, Mar 11, 2023 at 8:47 PM Peter Geoghegan wrote: > I was joking. But I did have a real point: once we have tests for the > xidStopLimit mechanism, why not take the opportunity to correct the > long standing issue with the documentation advising the use of single > user mode? Does https://co

Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-14 Thread Jacob Champion
d9ecacfa71c60cc250fa208d Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Tue, 18 Oct 2022 16:55:36 -0700 Subject: [PATCH v17 2/2] require_auth: decouple SASL and SCRAM Rather than assume that an AUTH_REQ_SASL* code refers to SCRAM-SHA-256, future-proof by separating the single allowlist into a l

Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

2023-03-14 Thread Jacob Champion
'OPENSSL_API_COMPAT', '0x10001000L', ++endif + endif + endif + ## src/include/pg_config.h.in ## @@ 2: 5de1c458b1 = 2: 11b69d0bc0 libpq: force sslmode=verify-full for system CAs From 84f67249e683d79a9a004b3f12507e77be3d9c6f Mon Sep 17 00:00:00 20

Re: Data is copied twice when specifying both child and parent table in publication

2023-03-17 Thread Jacob Champion
On Thu, Mar 16, 2023 at 11:28 PM wangw.f...@fujitsu.com wrote: > Attach the new patch set. Hi, I ran into this problem while hacking on [1], so thank you for tackling it! I have no strong opinions on the implementation itself; I just want to register a concern that the tests have not kept up wit

Re: Data is copied twice when specifying both child and parent table in publication

2023-03-20 Thread Jacob Champion
On Fri, Mar 17, 2023 at 9:45 PM Amit Kapila wrote: > > There are a bunch of moving parts and hidden subtleties here, and I fell > > into a few traps when I was working on my patch, so it'd be nice to have > > additional coverage. I'm happy to contribute effort in that area if it's > > helpful. > >

Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-23 Thread Jacob Champion
++ ++foreach my $c (@cases) { ++ $result = $node->safe_psql( ++ "trustdb", ++ "SELECT ssl_client_cert_present();", ++ connstr => "$common_connstr dbname=trustdb $c->{'opts'}" ++ ); ++ is($result, $

Re: [PoC] Let libpq reject unexpected authentication requests

2023-03-24 Thread Jacob Champion
On Thu, Mar 23, 2023 at 10:18 PM Michael Paquier wrote: > I have spent a couple of hours looking at the whole again today, > testing that with OpenSSL to make sure that everything was OK. Apart > from a few tweaks, that seemed pretty good. So, applied. Thank you! --Jacob

Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security

2023-03-24 Thread Jacob Champion
On 3/20/23 09:32, Robert Haas wrote: > I think this is the root of our disagreement. Agreed. > My understanding of the > previous discussion is that people think that the major problem here > is the wraparound-to-superuser attack. That is, in general, we expect > that when we connect to a databas

Re: WIP Incremental JSON Parser

2024-03-14 Thread Jacob Champion
I've been poking at the partial token logic. The json_errdetail() bug mentioned upthread (e.g. for an invalid input `[12zz]` and small chunk size) seems to be due to the disconnection between the "main" lex instance and the dummy_lex that's created from it. The dummy_lex contains all the informatio

Re: Support json_errdetail in FRONTEND builds

2024-03-18 Thread Jacob Champion
On Sun, Mar 17, 2024 at 4:49 PM Daniel Gustafsson wrote: > I took another look at this tonight and committed it with some mostly cosmetic > changes. Great! Thanks everyone. --Jacob

Re: sslinfo extension - add notbefore and notafter timestamps

2024-03-18 Thread Jacob Champion
On Fri, Mar 8, 2024 at 4:16 PM Cary Huang wrote: > Yes, I noticed this in the SSL test too. I am also in GTM-8, so for me the > tests would fail too due to the time zone differences from GMT. It shall be > okay to specifically set the outputs of pg_stat_ssl, > ssl_client_get_notbefore, and ssl_

Re: WIP Incremental JSON Parser

2024-03-18 Thread Jacob Champion
oken_terminator = ptok->data + ptok->len; + lex->token_terminator = dummy_lex.token_terminator; if (partial_result == JSON_SUCCESS) lex->inc_state->partial_completed = true; return partial_result; commit 3d615593d6d79178a8

Re: sslinfo extension - add notbefore and notafter timestamps

2024-03-18 Thread Jacob Champion
On Mon, Mar 18, 2024 at 1:48 PM Cary Huang wrote: > Attached is the v10 patch with the above changes. Thanks again for the review. Awesome, looks good. On my final review pass I saw one last thing that bothered me (sorry for not seeing it before). The backend version of ASN1_TIME_to_timestamptz

Re: sslinfo extension - add notbefore and notafter timestamps

2024-03-20 Thread Jacob Champion
On Wed, Mar 20, 2024 at 7:03 AM Daniel Gustafsson wrote: > The issue here is that postgres use a different epoch from the unix epoch, so > any dates calcuated based on the unix epoch need to be adjusted. Ah, thank you! I had just reproduced Cary's problem and was really confused... > I've hacked

Re: sslinfo extension - add notbefore and notafter timestamps

2024-03-20 Thread Jacob Champion
On Wed, Mar 20, 2024 at 7:50 AM Daniel Gustafsson wrote: > We are subtracting 30 years from a calculation that we know didnt overflow, so > I guess if the certificate notBefore (the notAfter cannot be that early since > we wouldn't be able to connect with it) was set to early enough? It didn't >

Re: WIP Incremental JSON Parser

2024-03-20 Thread Jacob Champion
On Tue, Mar 19, 2024 at 3:07 PM Andrew Dunstan wrote: > On Mon, Mar 18, 2024 at 3:35 PM Jacob Champion > wrote: >> With the incremental parser, I think prev_token_terminator is not >> likely to be safe to use except in very specific circumstances, since >> it could

Re: WIP Incremental JSON Parser

2024-03-20 Thread Jacob Champion
This new return path... > + if (!tok_done) > + { > + if (lex->inc_state->is_last_chunk) > + return JSON_INVALID_TOKEN; ...also needs to set the token pointers. See one approach in the attached diff, which additionally asserts that we've consumed the entire chun

Re: [PATCH] Exponential backoff for auth_delay

2024-03-20 Thread Jacob Champion
On Wed, Mar 20, 2024 at 2:15 PM Jacob Champion wrote: > I think solutions for case 1 and case 2 are necessarily at odds under > the current design, if auth_delay relies on slot exhaustion to do its > work effectively. Weakening that on purpose doesn't make much sense to >

Re: sslinfo extension - add notbefore and notafter timestamps

2024-03-22 Thread Jacob Champion
On Fri, Mar 22, 2024 at 6:15 AM Daniel Gustafsson wrote: > While staging this to commit I realized one silly thing about it warranting > another round here. The ASN.1 timediff code can diff against *any* timestamp, > not just the UNIX epoch, so we could just pass in the postgres epoch and skip >

Re: Proposal for implementing OCSP Stapling in PostgreSQL

2024-03-22 Thread Jacob Champion
On Tue, Mar 5, 2024 at 4:12 PM David Zhang wrote: > Any comments or feedback would be greatly appreciated! Hi David -- I haven't had time to get to this for the 17 release cycle, but I'm interested in this feature and I intend to review it at some point for 18. I think OCSP will be especially hel

Re: WIP Incremental JSON Parser

2024-03-25 Thread Jacob Champion
On Wed, Mar 20, 2024 at 11:56 PM Andrew Dunstan wrote: > Thanks, included that and attended to the other issues we discussed. I think > this is pretty close now. Okay, looking over the thread, there are the following open items: - extend the incremental test in order to exercise the semantic cal

Re: WIP Incremental JSON Parser

2024-03-25 Thread Jacob Champion
On Mon, Mar 25, 2024 at 4:02 PM Andrew Dunstan wrote: > Well, what's the alternative? The current parser doesn't check stack depth in > frontend code. Presumably it too will eventually just run out of memory, > possibly rather sooner as the stack frames could be more expensive than the > incre

Re: WIP Incremental JSON Parser

2024-03-25 Thread Jacob Champion
On Mon, Mar 25, 2024 at 4:12 PM Jacob Champion wrote: > Stack size should be pretty limited, at least on the platforms I'm > familiar with. So yeah, the recursive descent will segfault pretty > quickly, but it won't repalloc() an unbounded amount of heap space. > The altern

Re: WIP Incremental JSON Parser

2024-03-25 Thread Jacob Champion
On Mon, Mar 25, 2024 at 4:24 PM Andrew Dunstan wrote: > OK, so we invent a new error code and have the parser return that if the > stack depth gets too big? Yeah, that seems reasonable. I'd potentially be able to build on that via OAuth for next cycle, too, since that client needs to limit its

Re: WIP Incremental JSON Parser

2024-03-26 Thread Jacob Champion
On Mon, Mar 25, 2024 at 3:14 PM Jacob Champion wrote: > - add Assert calls in impossible error cases [2] To expand on this one, I think these parts of the code (marked with `<- here`) are impossible to reach: > switch (top) > { > case JSON_TOKEN_STRING: > if (next

Re: WIP Incremental JSON Parser

2024-03-29 Thread Jacob Champion
On Fri, Mar 29, 2024 at 9:42 AM Andrew Dunstan wrote: > Here's a new set of patches, with I think everything except the error case > Asserts attended to. There's a change to add semantic processing to the test > suite in patch 4, but I'd fold that into patch 1 when committing. Thanks! 0004 did

Re: [PoC] Federated Authn/z with OAUTHBEARER

2024-04-01 Thread Jacob Champion
On Thu, Mar 28, 2024 at 3:34 PM Daniel Gustafsson wrote: > In jsonapi.c, makeJsonLexContextCstringLen initialize a JsonLexContext with > palloc0 which would need to be ported over to use ALLOC for frontend code. Seems reasonable (but see below, too). > On > that note, the errorhandling in parse_

Re: WIP Incremental JSON Parser

2024-04-02 Thread Jacob Champion
On Mon, Apr 1, 2024 at 4:53 PM Andrew Dunstan wrote: > Anyway, here are new patches. I've rolled the new semantic test into the > first patch. Looks good! I've marked RfC. > json_lex() is not really a very hot piece of code. Sure, but I figure if someone is trying to get the performance of the

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-03 Thread Jacob Champion
On Tue, Apr 2, 2024 at 11:55 AM Daniel Gustafsson wrote: > The attached removes 1.0.2 support (meson build parts untested yet) with a few > small touch ups of related documentation. I haven't yet done the research on > where that leaves LibreSSL since we don't really define anywhere what we > sup

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-03 Thread Jacob Champion
On Wed, Apr 3, 2024 at 8:29 AM Tom Lane wrote: > I count 3 machines running 1.0.1, 18 running some flavor > of 1.0.2, and 7 running various LibreSSL versions. I don't know all the tradeoffs with buildfarm wrangling, but IMO all those 1.0.2 installations are the most problematic, so I dug in a bit

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-03 Thread Jacob Champion
On Wed, Apr 3, 2024 at 10:38 AM Tom Lane wrote: > Also, calling Photon 3 > dead because it went EOL three days ago seems over-hasty. Well, March 1, but either way I thought "dead" for the purposes of this thread meant "you can't build the very latest version of Postgres on it", not "we've forgott

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-03 Thread Jacob Champion
On Wed, Apr 3, 2024 at 11:13 AM Tom Lane wrote: > wikipedia says that RHEL7 ends ELS as of June 2026 [1]. I may have misunderstood something in here then: https://www.redhat.com/en/blog/announcing-4-years-extended-life-cycle-support-els-red-hat-enterprise-linux-7 > ELS for RHEL 7 is now av

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-04 Thread Jacob Champion
On Wed, Apr 3, 2024 at 3:27 PM Daniel Gustafsson wrote: > The patch will also need to be adjusted to work with LibreSSL, but I know > Jacob > was looking into that so ideally we should have something to review before > the weekend. v3 does that by putting back checks for symbols that aren't part

Re: WIP Incremental JSON Parser

2024-04-04 Thread Jacob Champion
On Thu, Apr 4, 2024 at 10:17 AM Jelte Fennema-Nio wrote: > Command 'test_json_parser_incremental' not found in > /home/jelte/opensource/postgres/build/tmp_install//home/jelte/.pgenv/pgsql-17beta9/bin, > ... I can't reproduce this locally... What's in the `...`? I wouldn't expect to find the test

Re: WIP Incremental JSON Parser

2024-04-04 Thread Jacob Champion
On Thu, Apr 4, 2024 at 11:12 AM Jacob Champion wrote: > What's in the `...`? I wouldn't expect to find the test binary in your > tmp_install. Oh, I wonder if this is just a build dependency thing? I typically run a bare `ninja` right before testing, because I think most of those

Re: WIP Incremental JSON Parser

2024-04-04 Thread Jacob Champion
On Thu, Apr 4, 2024 at 11:06 AM Nathan Bossart wrote: > ../postgresql/src/common/jsonapi.c: In function ‘IsValidJsonNumber’: > ../postgresql/src/common/jsonapi.c:2016:30: error: ‘dummy_lex.inc_state’ may > be used uninitialized in this function [-Werror=maybe-uninitialized] > 2016 | if (lex->in

Re: WIP Incremental JSON Parser

2024-04-04 Thread Jacob Champion
On Thu, Apr 4, 2024 at 1:42 PM Jelte Fennema-Nio wrote: > Maybe that's something worth addressing too. I expected that > install/install-quiet was a strict superset of a plain ninja > invocation. Maybe that's the intent, but I hope not, because I don't see any reason for `ninja install` to worry

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-05 Thread Jacob Champion
On Thu, Apr 4, 2024 at 6:37 PM Michael Paquier wrote: > From where did you pull the LibreSSL sources? Directly from the > OpenBSD tree? I've been building LibreSSL Portable: https://github.com/libressl/portable > Ah, right. OpenSSL_add_all_algorithms() is documented as having no > effect in 1.

Re: Security lessons from liblzma

2024-04-05 Thread Jacob Champion
On Fri, Apr 5, 2024 at 6:24 AM Robert Haas wrote: > I wonder how hard it would be to just code up our own binary to do > this. If it'd be a pain to do that, or to maintain it across SSL > versions, then it's a bad plan and we shouldn't do it. But if it's not > that much code, maybe it'd be worth c

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-05 Thread Jacob Champion
On Fri, Apr 5, 2024 at 9:59 AM Daniel Gustafsson wrote: > Attached is a WIP patch to get more eyes on it, the Meson test for 1.1.1 fails > on Windows in CI which I will investigate next. The changes for SSL_OP_NO_CLIENT_RENEGOTIATION and SSL_R_VERSION_TOO_LOW look good to me. > -Remove s

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-05 Thread Jacob Champion
On Fri, Apr 5, 2024 at 2:48 PM Daniel Gustafsson wrote: > But does that actually work? If I change the API_COMPAT to the 1.1.1 version > number and run configure against 1.0.2 it passes just fine. Am I missing some > clever trick here? Similarly, I changed my API_COMPAT to a nonsense 0x9010

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-05 Thread Jacob Champion
On Fri, Apr 5, 2024 at 3:32 PM Daniel Gustafsson wrote: > > An autoreconf run on my machine pulls in more changes (getting rid of > > the symbols we no longer check for). > > Ah yes, missed updating before formatting the patch. Done in the attached. The commit subject may still need to be reverte

Re: Security lessons from liblzma

2024-04-08 Thread Jacob Champion
On Fri, Apr 5, 2024 at 5:14 PM Michael Paquier wrote: > Saying that, my spidey sense tingles at the recent commit > 3311ea86edc7, that had the idea to introduce a 20k line output file > based on a 378 line input file full of random URLs. In my experience, > tests don't require to be that large to

Re: WIP Incremental JSON Parser

2024-04-09 Thread Jacob Champion
On Tue, Apr 9, 2024 at 4:54 AM Andrew Dunstan wrote: > On 2024-04-09 Tu 01:23, Michael Paquier wrote: > There is no direct check on test_json_parser_perf.c, either, only a > custom rule in the Makefile without specifying something for meson. > So it looks like you could do short execution check in

Re: WIP Incremental JSON Parser

2024-04-09 Thread Jacob Champion
On Tue, Apr 9, 2024 at 7:30 AM Andrew Dunstan wrote: > I think Michael's point was that if we carry the code we should test we > can run it. The other possibility would be just to remove it. I can see > arguments for both. Hm. If it's not acceptable to carry this (as a worse-is-better smoke test)

Re: WIP Incremental JSON Parser

2024-04-09 Thread Jacob Champion
On Mon, Apr 8, 2024 at 10:24 PM Michael Paquier wrote: > At the end, having a way to generate JSON blobs randomly to test this > stuff would be more appealing For the record, I'm working on an LLVM fuzzer target for the JSON parser. I think that would be a lot more useful than anything we can han

Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

2024-04-10 Thread Jacob Champion
On Wed, Apr 10, 2024 at 12:31 AM Peter Eisentraut wrote: > * src/backend/libpq/be-secure-openssl.c > > +#include > > This patch doesn't appear to add anything, so why does it need a new > include? This one was mine -- it was an indirect header dependency that was effectively removed in 1.1.0 and

Re: WIP Incremental JSON Parser

2024-02-20 Thread Jacob Champion
On Tue, Feb 20, 2024 at 2:10 PM Andrew Dunstan wrote: > Well, that didn't help a lot, but meanwhile the CFBot seems to have > decided in the last few days that it's now happy, so full steam aead! ;-) I haven't been able to track down the root cause yet, but I am able to reproduce the failure cons

Re: [PoC] Federated Authn/z with OAUTHBEARER

2024-02-20 Thread Jacob Champion
Hi all, v14 rebases over latest and fixes a warning when assertions are disabled. 0006 is temporary and hacks past a couple of issues I have not yet root caused -- one of which makes me wonder if 0001 needs to be considered alongside the recent pg_combinebackup and incremental JSON work...? --Jac

Re: WIP Incremental JSON Parser

2024-02-21 Thread Jacob Champion
ssert=true - GCC 11.4 - build path is nested a bit (~/src/postgres/worktree-inc-json/build-dev) --Jacob commit 0075a88beec160cbb408d9a1e0a11d836fb55bdf Author: Jacob Champion Date: Wed Feb 21 06:36:55 2024 -0800 WIP: mesonify diff --git a/src/test/modules/meson.build b/src/test/modules

Re: WIP Incremental JSON Parser

2024-02-21 Thread Jacob Champion
On Wed, Feb 21, 2024 at 6:50 AM Jacob Champion wrote: > On Tue, Feb 20, 2024 at 9:32 PM Andrew Dunstan wrote: > > *sigh* That's weird. I wonder why you can reproduce it and I can't. Can > > you give me details of the build? OS, compiler, path to source, build > >

Re: WIP Incremental JSON Parser

2024-02-22 Thread Jacob Champion
provides raw strings", not "all strings are now NULL". --Jacob commit 590ea7bec167058340624313d98c72976fa89d7a Author: Jacob Champion Date: Wed Feb 21 06:36:55 2024 -0800 WIP: mesonify diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index 8fbe742d38

Re: WIP Incremental JSON Parser

2024-02-26 Thread Jacob Champion
On Thu, Feb 22, 2024 at 3:43 PM Andrew Dunstan wrote: > > Are there plans to fill out the test suite more? Since we should be > > able to control all the initial conditions, it'd be good to get fairly > > comprehensive coverage of the new code. > > Well, it's tested (as we know) by the backup mani

Re: WIP Incremental JSON Parser

2024-02-26 Thread Jacob Champion
On Mon, Feb 26, 2024 at 7:08 AM Jacob Champion wrote: > As a brute force example of the latter, with the attached diff I get > test failures at chunk sizes 1, 2, 3, 4, 6, and 12. But this time with the diff. --Jacob diff --git a/src/test/modules/test_json_pa

Re: Refactor SASL exchange in preparation for OAuth Bearer

2024-02-26 Thread Jacob Champion
On Fri, Feb 23, 2024 at 2:30 AM Daniel Gustafsson wrote: > > The attached two patches are smaller refactorings to the SASL exchange and > init > codepaths which are required for the OAuthbearer work [0]. Regardless of the > future of that patchset, these refactorings are nice cleanups and can be

Re: WIP Incremental JSON Parser

2024-02-27 Thread Jacob Champion
On Mon, Feb 26, 2024 at 9:20 PM Andrew Dunstan wrote: > The good news is that the parser is doing fine - this issue was due to a > thinko on my part in the test program that got triggered by the input > file size being an exact multiple of the chunk size. I'll have a new > patch set later this wee

Re: [PoC] Federated Authn/z with OAUTHBEARER

2024-02-27 Thread Jacob Champion
On Tue, Feb 27, 2024 at 11:20 AM Jacob Champion wrote: > This is done in v17, which is also now based on the two patches pulled > out by Daniel in [1]. It looks like my patchset has been eaten by a malware scanner: 550 Message content failed content sc

Re: [PoC] Federated Authn/z with OAUTHBEARER

2024-02-29 Thread Jacob Champion
[re-adding the CC list I dropped earlier] On Wed, Feb 28, 2024 at 1:52 PM Daniel Gustafsson wrote: > > > On 28 Feb 2024, at 22:50, Andrew Dunstan wrote: > > Can you give some more details about what this python gadget would buy us? > > I note that there are a couple of CPAN modules that provide

Re: Refactor SASL exchange in preparation for OAuth Bearer

2024-02-29 Thread Jacob Champion
On Wed, Feb 28, 2024 at 2:54 PM Daniel Gustafsson wrote: > I rank that as one of my better typos actually. Fixed though. LGTM! Thanks, --Jacob

Re: [PoC] Federated Authn/z with OAUTHBEARER

2024-02-29 Thread Jacob Champion
On Wed, Feb 28, 2024 at 9:40 AM Daniel Gustafsson wrote: > +#define ALLOC(size) malloc(size) > I wonder if we should use pg_malloc_extended(size, MCXT_ALLOC_NO_OOM) instead > to self document the code. We clearly don't want feature-parity with server- > side palloc here. I know we use malloc in

Re: [PoC] Federated Authn/z with OAUTHBEARER

2024-02-29 Thread Jacob Champion
On Thu, Feb 29, 2024 at 1:08 PM Daniel Gustafsson wrote: > + /* TODO */ > + CHECK_SETOPT(actx, CURLOPT_WRITEDATA, stderr); > I might be missing something, but what this is intended for in > setup_curl_handles()? Ah, that's cruft left over from early debugging, just so that I could see what wa

Re: Experiments with Postgres and SSL

2024-03-01 Thread Jacob Champion
On Wed, Feb 28, 2024 at 4:10 AM Heikki Linnakangas wrote: > I think we'd want to *avoid* changing the major protocol version in a > way that would introduce a new roundtrip, though. I'm starting to get up to speed with this patchset. So far I'm mostly testing how it works; I have yet to take an i

Re: Experiments with Postgres and SSL

2024-03-05 Thread Jacob Champion
ccidentally disabled direct SSL connection completely and always used > negotiated mode, the tests would still pass. I'd like to see some tests > that would catch that. +1 On Mon, Mar 4, 2024 at 7:29 AM Heikki Linnakangas wrote: > On 01/03/2024 23:49, Jacob Champion wrote: > &

Re: sslinfo extension - add notbefore and notafter timestamps

2024-03-05 Thread Jacob Champion
On Mon, Mar 4, 2024 at 6:23 AM Daniel Gustafsson wrote: > > On 12 Sep 2023, at 21:40, Jacob Champion wrote: Sorry for the long delay! > >> + ssl_client_get_notbefore() returns text > >> ...> + ssl_client_get_notafter() returns text > > > > I t

Re: Experiments with Postgres and SSL

2024-03-05 Thread Jacob Champion
I keep forgetting -- attached is the diff I'm carrying to plug libpq_encryption into Meson. (The current patchset has a meson.build for it, but it's not connected.) --Jacob commit 64215f1e6b68208378b34cc0736d2f0eb1d45919 Author: Jacob Champion Date: Wed Feb 28 11:28:17 2024 -080

Re: [PATCH] Exponential backoff for auth_delay

2024-03-05 Thread Jacob Champion
On Tue, Mar 5, 2024 at 1:51 PM Nathan Bossart wrote: > I don't have a strong opinion about making this configurable, but I do > think we should consider making this a hash table so that we can set > MAX_CONN_RECORDS much higher. I'm curious why? It seems like the higher you make MAX_CONN_RECORDS,

Re: [PATCH] Exponential backoff for auth_delay

2024-03-06 Thread Jacob Champion
On Wed, Mar 6, 2024 at 8:10 AM Nathan Bossart wrote: > Assuming you are referring to the case where we run out of free slots in > acr_array, I'm not sure I see that as desirable behavior. Once you run out > of slots, all failed authentication attempts are now subject to the maximum > delay, which

Re: [PATCH] Exponential backoff for auth_delay

2024-03-06 Thread Jacob Champion
On Wed, Mar 6, 2024 at 12:34 PM Tomas Vondra wrote: > Doesn't this mean this approach (as implemented) doesn't actually work > as a protection against this sort of DoS? > > If I'm an attacker, and I can just keep opening new connections for each > auth request, am I even subject to any auth delay?

Re: [PATCH] Exponential backoff for auth_delay

2024-03-06 Thread Jacob Champion
On Wed, Mar 6, 2024 at 2:45 PM Michael Banck wrote: > In order to at least make case 2 not worse for exponential backoff, we > could maybe disable it (and just wait for auth_delay.milliseconds) once > MAX_CONN_RECORDS is full. In addition, maybe MAX_CONN_RECORDS should be > some fraction of max_co

Re: WIP Incremental JSON Parser

2024-03-07 Thread Jacob Champion
Some more observations as I make my way through the patch: In src/common/jsonapi.c, > +#define JSON_NUM_NONTERMINALS 6 Should this be 5 now? > + res = pg_parse_json_incremental(&(incstate->lex), &(incstate->sem), > + >

Re: WIP Incremental JSON Parser

2024-03-11 Thread Jacob Champion
On Sun, Mar 10, 2024 at 11:43 PM Andrew Dunstan wrote: > I haven't managed to reproduce this. But I'm including some tests for it. If I remove the newline from the end of the new tests: > @@ -25,7 +25,7 @@ for (my $size = 64; $size > 0; $size--) > foreach my $test_string (@inlinetests) > { >

Support json_errdetail in FRONTEND builds

2024-03-12 Thread Jacob Champion
Hello, Both the incremental JSON [1] and OAuth [2] patchsets would be improved by json_errdetail(), which was removed from FRONTEND builds in b44669b2ca: >The routine providing a detailed error message based on the error code >is made backend-only, the existing code being unsafe to use in

Re: Support json_errdetail in FRONTEND builds

2024-03-13 Thread Jacob Champion
On Tue, Mar 12, 2024 at 11:38 PM Michael Paquier wrote: > On Tue, Mar 12, 2024 at 08:38:43PM -0400, Andrew Dunstan wrote: > > yeah, although maybe worth a different patch. > > I've wanted that a few times, FWIW. I would do a split, mainly for > clarity. Sounds good, split into v2-0002. (That giv

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-13 Thread Jacob Champion
On Wed, Mar 13, 2024 at 12:01 PM Alvaro Herrera wrote: > On 2024-Mar-13, Jelte Fennema-Nio wrote: > > Sadly I'm having a hard time reliably reproducing this race condition > > locally. So it's hard to be sure what is happening here. Attached is a > > patch with a wild guess as to what the issue mi

Re: Remove SHA256_HMAC_B from scram-common.h

2022-12-13 Thread Jacob Champion
On Mon, Dec 12, 2022 at 8:57 PM Michael Paquier wrote: > While doing some hackery on SCRAM, I have noticed $subject giving the > attached. I guess that this is not going to cause any objections, but > feel free to comment just in case. Yeah, no objection :D That cryptohash refactoring was quite

Re: [PoC] Federated Authn/z with OAUTHBEARER

2022-12-16 Thread Jacob Champion
On Mon, Dec 12, 2022 at 9:06 PM Andrey Chudnovsky wrote: > If your concern is extension not honoring the DBA configured values: > Would a server-side logic to prefer HBA value over extension-provided > resolve this concern? Yeah. It also seals the role of the extension here as "optional". > We a

Re: [PATCH] CF app: add "Returned: Needs more interest"

2023-01-03 Thread Jacob Champion
On Tue, Jan 3, 2023 at 4:14 AM vignesh C wrote: > The patch does not apply on top of HEAD as in [1], please post a rebased > patch: Hi Vignesh -- this is a patch for the CF app, not the Postgres repo, so cfbot won't be able to apply it. Let me know if there's a better place for me to put it. Th

Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

2023-01-03 Thread Jacob Champion
On Thu, Dec 8, 2022 at 3:10 PM Jacob Champion wrote: > For now, it's worked around in v4. This should finally get the cfbot > fully green. Cirrus's switch to M1 Macs changed the Homebrew installation path, so v5 adjusts the workaround accordingly. --Jacob 1: b01812f604 ! 1:

Re: [PATCH] CF app: add "Returned: Needs more interest"

2023-01-04 Thread Jacob Champion
On Tue, Jan 3, 2023 at 8:56 PM vignesh C wrote: > I'm not sure if this should be included in commitfest as we generally > include the postgres repository patches in the commitfest. I felt we > could have the discussion in the thread and remove the entry from > commitfest. Is there a good way to r

Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

2023-01-06 Thread Jacob Champion
On Fri, Jan 6, 2023 at 2:18 AM Jelte Fennema wrote: > > Huge +1 from me. On Azure we're already using public CAs to sign certificates > for our managed postgres offerings[1][2]. Right now, our customers have to go > to the hassle of downloading a specific root cert or finding their OS default >

Re: Introduce "log_connection_stages" setting.

2023-01-06 Thread Jacob Champion
Hi, +1 for the idea! > +authenticated > +Logs the original identity that an authentication method > employs to identify a user. In most cases, the identity string equals the > PostgreSQL username, > +but some third-party authentication methods may alter the original > u

Re: RFC: logical publication via inheritance root?

2023-01-06 Thread Jacob Champion
On Fri, Dec 9, 2022 at 10:21 AM Jacob Champion wrote: > Some inheritance hierarchies won't be "partitioned" hierarchies, of > course, but the user can simply not set that replication option for > those publications. The more I noodle around with this approach, the less I

Re: RFC: logical publication via inheritance root?

2023-01-10 Thread Jacob Champion
On Mon, Jan 9, 2023 at 12:41 AM Aleksander Alekseev wrote: > I would like to point out that we shouldn't necessarily support > multiple inheritance in all the possible cases, at least not in the > first implementation. Supporting simple cases of inheritance would be > already a valuable feature ev

Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

2023-01-10 Thread Jacob Champion
On Mon, Jan 9, 2023 at 7:40 AM Andrew Dunstan wrote: > I'm confused. A client cert might not have a hostname at all, and isn't > used to verify the connecting address, but to verify the username. It > needs to have a CN/DN equal to the user name of the connection, or that > maps to that name via p

<    1   2   3   4   5   6   7   8   9   10   >