Document use of ldapurl with LDAP simple bind

2024-05-24 Thread Jacob Champion
Hi all, Our documentation implies that the ldapurl setting in pg_hba is used for search+bind mode only. It was pointed out to me recently that this is not true, and if you're dealing with simple bind on a non-standard scheme or port, then ldapurl makes the HBA easier to read: ... ldap

Re: pg_parse_json() should not leak token copies on failure

2024-05-24 Thread Jacob Champion
On Tue, Apr 30, 2024 at 2:29 PM Jacob Champion wrote: > Attached is a draft patch to illustrate what I mean, but it's > incomplete: it only solves the problem for scalar values. (Attached is a v2 of that patch; in solving a frontend leak I should probably not introduce a backend se

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

2024-05-23 Thread Jacob Champion
On Thu, May 23, 2024 at 11:12 AM Tom Lane wrote: > If a reader doesn't recognize a particular > chunk code, it can still tell whether the chunk is "critical" or not, > and thereby decide if it must give up or can proceed while ignoring > that chunk.) Would it be good to expand on that idea of

Re: libpq compression (part 3)

2024-05-21 Thread Jacob Champion
On Tue, May 21, 2024 at 12:08 PM Jacob Burroughs wrote: > I think I thought I was writing about something else when I wrote that > :sigh:. I think what I really should have written was a version of > the part below, which is that we use streaming decompression, only > decompress 8kb at a time,

Re: libpq compression (part 3)

2024-05-21 Thread Jacob Champion
On Tue, May 21, 2024 at 9:14 AM Jacob Burroughs wrote: > On Tue, May 21, 2024 at 10:43 AM Jelte Fennema-Nio wrote: > > To help get everyone on the same page I wanted to list all the > > security concerns in one place: > > > > 1. Triggering excessive CPU usage before authentication, by asking for

Re: libpq compression (part 3)

2024-05-21 Thread Jacob Champion
On Tue, May 21, 2024 at 8:23 AM Jacob Burroughs wrote: > As currently implemented, the compression only applies to > CopyData/DataRow/Query messages, none of which should be involved in > authentication, unless I've really missed something in my > understanding. Right, but Robert has argued that

Re: libpq compression (part 3)

2024-05-20 Thread Jacob Champion
On Mon, May 20, 2024 at 11:37 AM Robert Haas wrote: > But if that's a practical > attack, preventing compression prior to the authentication exchange > probably isn't good enough I mean... you said it, not me. I'm trying not to rain on the parade too much, because compression is clearly very

Re: libpq compression (part 3)

2024-05-20 Thread Jacob Champion
On Mon, May 20, 2024 at 11:05 AM Andrey M. Borodin wrote: > > So, the data would be compressed first, with framing around that, and > > then transport encryption would happen afterwards. I don't see how > > that would leak your password, but I have a feeling that might be a > > sign that I'm

Re: libpq compression (part 3)

2024-05-20 Thread Jacob Champion
On Mon, May 20, 2024 at 10:01 AM Robert Haas wrote: > I really hope that you can't poke big enough holes to kill the feature > entirely, though. Because that sounds sad. Even if there are holes, I don't think the situation's going to be bad enough to tank everything; otherwise no one would be

Re: libpq compression (part 3)

2024-05-20 Thread Jacob Champion
On Mon, May 20, 2024 at 8:29 AM Robert Haas wrote: > It does occur to me that if some compression algorithm has a buffer > overrun bug, restricting its use until after authentication might > reduce the score of the resulting CVE, because now you have to be able > to authenticate to make an

Re: libpq compression (part 3)

2024-05-20 Thread Jacob Champion
On Fri, May 17, 2024 at 4:03 PM Jelte Fennema-Nio wrote: > > On Fri, 17 May 2024 at 23:10, Jacob Champion > wrote: > > We're talking about a transport-level option, though -- I thought the > > proposal enabled compression before authentication completed? Or has > >

Re: libpq compression (part 3)

2024-05-17 Thread Jacob Champion
On Fri, May 17, 2024 at 1:53 PM Jacob Burroughs wrote: > New proposal, predicated on the assumption that if you enable > compression you are ok with the client picking whatever level they > want. At least with the currently enabled algorithms I don't think > any of them are so insane that they

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-17 Thread Jacob Champion
On Fri, May 17, 2024 at 7:40 AM Robert Haas wrote: > > On Fri, May 17, 2024 at 10:31 AM Tom Lane wrote: > > To my mind, the point of the time-boxed commitfests is to provide > > a structure wherein people will (hopefully) pay some actual attention > > to other peoples' patches. Conversely, the

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-16 Thread Jacob Champion
On Thu, May 16, 2024 at 2:04 PM Tom Lane wrote: > Oh, that's an independent pet peeve of mine. Usually, if I'm > looking over the CF list for a patch to review, I skip over ones > that already show an assigned reviewer, because I don't want to > step on that person's toes. But it seems very

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-16 Thread Jacob Champion
On Thu, May 16, 2024 at 2:29 PM Joe Conway wrote: > If no one, including the author (new or otherwise) is interested in > shepherding a particular patch, what chance does it have of ever getting > committed? That's a very different thing from what I think will actually happen, which is - new

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-16 Thread Jacob Champion
On Thu, May 16, 2024 at 2:06 PM Joe Conway wrote: > Maybe the word "care" was a poor choice, but forcing authors to think > about and decide if they have the "time to shepherd a patch" for the > *next CF* is exactly the point. If they don't, why clutter the CF with it. Because the community

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-16 Thread Jacob Champion
On Thu, May 16, 2024 at 1:31 PM Joe Conway wrote: > Maybe we should just make it a policy that *nothing* gets moved forward > from commitfest-to-commitfest and therefore the author needs to care > enough to register for the next one? I think that's going to severely disadvantage anyone who

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-16 Thread Jacob Champion
On Thu, May 16, 2024 at 1:46 PM Melanie Plageman wrote: > I should probably simply > withdraw and re-register them. My justification was that I'll lose > them if I don't keep them in the commitfest app. But, I could just, > you know, save them somewhere myself instead of polluting the >

Re: commitfest.postgresql.org is no longer fit for purpose

2024-05-16 Thread Jacob Champion
On Thu, May 16, 2024 at 12:14 PM David G. Johnston wrote: > Those likely never get out of the new WIP slot discussed above. Your patch > tracker basically. And there should be less angst in moving something in the > bimonthly into WIP rather than dropping it outright. There is discussion to

Re: Direct SSL connection with ALPN and HBA rules

2024-05-15 Thread Jacob Champion
On Wed, May 15, 2024 at 6:33 AM Heikki Linnakangas wrote: > Ok, yeah, I can see that now. Here's a new version to address that. I > merged ENC_SSL_NEGOTIATED_SSL and ENC_SSL_DIRECT_SSL to a single method, > ENC_SSL. The places that need to distinguish between them now check > conn-sslnegotiation.

Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence

2024-05-15 Thread Jacob Champion
On Tue, May 14, 2024 at 11:15 PM Peter Eisentraut wrote: > > On 15.05.24 02:00, Michael Paquier wrote: > > Is that a recent regression? I have some blurry memories from > > working on these areas that changing src/common/ reflected on the > > compiled pieces effectively at some point. > > One

Re: Direct SSL connection with ALPN and HBA rules

2024-05-14 Thread Jacob Champion
On Mon, Apr 29, 2024 at 11:04 AM Jacob Champion wrote: > On Fri, Apr 26, 2024 at 3:51 PM Heikki Linnakangas wrote: > > Unfortunately the error message you got in the client with that was > > horrible (I modified the server to not accept the 'postgresql' protocol): > > >

Re: Direct SSL connection with ALPN and HBA rules

2024-05-13 Thread Jacob Champion
[this should probably belong to a different thread, but I'm not sure what to title it] On Mon, May 13, 2024 at 4:08 AM Heikki Linnakangas wrote: > I think these options should be designed from the user's point of view, > so that the user can specify the risks they're willing to accept, and > the

Re: Direct SSL connection with ALPN and HBA rules

2024-05-13 Thread Jacob Champion
On Mon, May 13, 2024 at 9:13 AM Robert Haas wrote: > I find this idea to be a massive improvement over the status quo, +1 > and > I didn't spot any major problems when I read through the patch, > either. Definitely not a major problem, but I think select_next_encryption_method() has gone

On the use of channel binding without server certificates (was: Direct SSL connection with ALPN and HBA rules)

2024-05-13 Thread Jacob Champion
[soapbox thread, so I've changed the Subject] On Mon, May 13, 2024 at 4:08 AM Heikki Linnakangas wrote: > "channel_binding=require sslmode=require" also protects from MITM attacks. This isn't true in the same way that "standard" TLS protects against MITM. I know you know that, but for the

Re: Direct SSL connection with ALPN and HBA rules

2024-05-13 Thread Jacob Champion
(There's, uh, a lot to respond to above and I'm trying to figure out how best to type up all of it.) On Mon, May 13, 2024 at 9:13 AM Robert Haas wrote: > However, > I disagree with Jacob's assertion that sslmode=require has no security > benefits over sslmode=prefer. For the record, I didn't

Re: [PATCH] json_lex_string: don't overread on bad UTF8

2024-05-09 Thread Jacob Champion
On Wed, May 8, 2024 at 9:27 PM Michael Paquier wrote: > This is a bit mitigated by the fact that d6607016c738 is recent, but > this is incorrect since v13 so backpatched down to that. Thank you! --Jacob

Re: [PATCH] json_lex_string: don't overread on bad UTF8

2024-05-08 Thread Jacob Champion
On Tue, May 7, 2024 at 10:31 PM Michael Paquier wrote: > But looking closer, I can see that in the JSON_INVALID_TOKEN case, > when !tok_done, we set token_terminator to point to the end of the > token, and that would include an incomplete byte sequence like in your > case. :/ Ah, I see what

Re: [PATCH] json_lex_string: don't overread on bad UTF8

2024-05-07 Thread Jacob Champion
On Mon, May 6, 2024 at 8:43 PM Michael Paquier wrote: > On Fri, May 03, 2024 at 07:05:38AM -0700, Jacob Champion wrote: > > We could port something like that to src/common. IMO that'd be more > > suited for an actual conversion routine, though, as opposed to a > > parser t

[PATCH] pg_stat_activity: make slow/hanging authentication more visible

2024-05-06 Thread Jacob Champion
Hi all, Recently I dealt with a server where PAM had hung a connection indefinitely, suppressing our authentication timeout and preventing a clean shutdown. Worse, the xmin that was pinned by the opening transaction cascaded to replicas and started messing things up downstream. The DBAs didn't

Re: [PATCH] json_lex_string: don't overread on bad UTF8

2024-05-03 Thread Jacob Champion
On Fri, May 3, 2024 at 4:54 AM Peter Eisentraut wrote: > > On 30.04.24 19:39, Jacob Champion wrote: > > Tangentially: Should we maybe rethink pieces of the json_lex_string > > error handling? For example, do we really want to echo an incomplete > > multibyte sequence once

Re: [PATCH] json_lex_string: don't overread on bad UTF8

2024-05-02 Thread Jacob Champion
On Wed, May 1, 2024 at 8:40 PM Michael Paquier wrote: > > On Thu, May 02, 2024 at 11:23:13AM +0900, Michael Paquier wrote: > > About the fact that we may finish by printing unfinished UTF-8 > > sequences, I'd be curious to hear your thoughts. Now, the information > > provided about the partial

Re: [PATCH] json_lex_string: don't overread on bad UTF8

2024-05-01 Thread Jacob Champion
On Tue, Apr 30, 2024 at 11:09 PM Michael Paquier wrote: > Not sure to like much the fact that this advances token_terminator > first. Wouldn't it be better to calculate pg_encoding_mblen() first, > then save token_terminator? I feel a bit uneasy about saving a value > in token_terminator past

Re: TLS certificate alternate trust paths issue in libpq - certificate chain validation failing

2024-05-01 Thread Jacob Champion
On Wed, May 1, 2024 at 11:57 AM Thomas Spear wrote: > It does fail to validate for case 4 after all. I must have had a copy/paste > error during past tests. Okay, good. Glad it's behaving as expected! > So then it sounds like putting the MS root in root.crt (as we have done to > fix this) is

Re: TLS certificate alternate trust paths issue in libpq - certificate chain validation failing

2024-05-01 Thread Jacob Champion
On Wed, May 1, 2024 at 8:17 AM Thomas Spear wrote: > Circling back to my original question, why is there a difference in behavior? > > What I believe should be happening isn't what's happening: > 1. If ~/.postgresql/root.crt contains the MS root, and I don't specify > sslrootcert= -- successful

Re: TLS certificate alternate trust paths issue in libpq - certificate chain validation failing

2024-05-01 Thread Jacob Champion
On Wed, May 1, 2024 at 6:48 AM Thomas Spear wrote: > I dumped out the certificates presented by the server using openssl, and the > chain that gets output includes "Microsoft Azure RSA TLS Issuing CA 08". > On https://www.microsoft.com/pkiops/docs/repository.htm the page says that > that cert

Re: TLS certificate alternate trust paths issue in libpq - certificate chain validation failing

2024-04-30 Thread Jacob Champion
On Tue, Apr 30, 2024 at 2:41 PM Thomas Spear wrote: > The full details can be found at github.com/pgjdbc/pgjdbc/discussions/3236 - > in summary, both jdbc-postgres and the psql cli seem to be affected by an > issue validating the certificate chain up to a publicly trusted root > certificate

pg_parse_json() should not leak token copies on failure

2024-04-30 Thread Jacob Champion
Hi, When a client of our JSON parser registers semantic action callbacks, the parser will allocate copies of the lexed tokens to pass into those callbacks. The intent is for the callbacks to always take ownership of the copied memory, and if they don't want it then they can pfree() it. However,

[PATCH] json_lex_string: don't overread on bad UTF8

2024-04-30 Thread Jacob Champion
Hi all, When json_lex_string() hits certain types of invalid input, it calls pg_encoding_mblen_bounded(), which assumes that its input is null-terminated and calls strnlen(). But the JSON lexer is constructed with an explicit string length, and we don't ensure that the string is null-terminated

Re: Direct SSL connection with ALPN and HBA rules

2024-04-29 Thread Jacob Champion
On Mon, Apr 29, 2024 at 12:32 PM Jacob Champion wrote: > > On Mon, Apr 29, 2024 at 12:06 PM Heikki Linnakangas wrote: > > On 29/04/2024 21:43, Jacob Champion wrote: > > > But if you're in that situation, what does the use of directonly give > > > you over `sslne

Re: Direct SSL connection with ALPN and HBA rules

2024-04-29 Thread Jacob Champion
On Mon, Apr 29, 2024 at 12:06 PM Heikki Linnakangas wrote: > On 29/04/2024 21:43, Jacob Champion wrote: > > But if you're in that situation, what does the use of directonly give > > you over `sslnegotiation=direct`? You already know that servers > > support direct, so

Re: Direct SSL connection with ALPN and HBA rules

2024-04-29 Thread Jacob Champion
On Mon, Apr 29, 2024 at 11:43 AM Heikki Linnakangas wrote: > Note that if the client does not request ALPN at all, the callback is > not called, and the connection is accepted. Old clients still work > because they do not request ALPN. Ugh, sorry for the noise -- I couldn't figure out why all my

Re: Direct SSL connection with ALPN and HBA rules

2024-04-29 Thread Jacob Champion
On Mon, Apr 29, 2024 at 1:38 AM Heikki Linnakangas wrote: > Sure, we'd try to fix them ASAP. But we've had the SSLRequest > negotiation since time immemorial. If a new vulnerability is found, it's > unlikely that we'd need to disable the SSLRequest negotiation altogether > to fix it. We'd be in

Re: Direct SSL connection with ALPN and HBA rules

2024-04-29 Thread Jacob Champion
On Fri, Apr 26, 2024 at 3:51 PM Heikki Linnakangas wrote: > I finally understood what you mean. So if the client supports ALPN, but > the list of protocols that it provides does not include 'postgresql', > the server should reject the connection with 'no_applicaton_protocol' > alert. Right. (And

Re: New committers: Melanie Plageman, Richard Guo

2024-04-26 Thread Jacob Champion
On Fri, Apr 26, 2024 at 4:54 AM Jonathan S. Katz wrote: > > The Core Team would like to extend our congratulations to Melanie > Plageman and Richard Guo, who have accepted invitations to become our > newest PostgreSQL committers. Congratulations!! --Jacob

Re: Direct SSL connection with ALPN and HBA rules

2024-04-25 Thread Jacob Champion
On Thu, Apr 25, 2024 at 2:50 PM Heikki Linnakangas wrote: > > I think that comes down to the debate upthread, and whether you think > > it's a performance tweak or a security feature. My take on it is, > > `direct` mode is performance, and `requiredirect` is security. > > Agreed, although the the

Re: Direct SSL connection with ALPN and HBA rules

2024-04-25 Thread Jacob Champion
On Thu, Apr 25, 2024 at 10:35 AM Robert Haas wrote: > Maybe I'm missing something here, but why doesn't sslnegotiation > override sslmode completely? Or alternatively, why not remove > sslnegotiation entirely and just have more sslmode values? I mean > maybe this shouldn't happen categorically,

Re: Direct SSL connection with ALPN and HBA rules

2024-04-25 Thread Jacob Champion
On Thu, Apr 25, 2024 at 9:17 AM Robert Haas wrote: > > It is difficult to imagine a world in which we have both requiredirect > and forcedirect and people are not confused. Yeah... Any thoughts on a better scheme? require_auth was meant to lock down overly general authentication; maybe a

Re: Direct SSL connection with ALPN and HBA rules

2024-04-25 Thread Jacob Champion
On Tue, Apr 23, 2024 at 2:20 PM Heikki Linnakangas wrote: > > Attached patch tries to fix and clarify those. s/negotiatied/negotiated/ in the attached patch, but other than that this seems like a definite improvement. Thanks! > (Note that the client will only attempt GSSAPI encryption if it can

Re: Experiments with Postgres and SSL

2024-04-24 Thread Jacob Champion
On Wed, Apr 24, 2024 at 1:57 PM Peter Eisentraut wrote: > I'm concerned that there appears to be some confusion over whether ALPN > is a performance feature or a security feature. RFC 7301 appears to be > pretty clear that it's for performance, not for security. It was also designed to give

Re: Row pattern recognition

2024-04-24 Thread Jacob Champion
On Tue, Apr 23, 2024 at 8:13 PM Tatsuo Ishii wrote: > SELECT v.a, count(*) OVER w > FROM (VALUES ('A'),('B'),('B'),('C')) AS v (a) > WINDOW w AS ( > ORDER BY v.a > ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING > PATTERN (B+) > DEFINE B AS a = 'B' > ) > a | count > ---+--- > A |

Re: Direct SSL connection with ALPN and HBA rules

2024-04-23 Thread Jacob Champion
On Tue, Apr 23, 2024 at 10:43 AM Robert Haas wrote: > I've not followed this thread closely enough to understand the comment > about requiredirect maybe not actually requiring direct, but if that > were true it seems like it might be concerning. It may be my misunderstanding. This seems to imply

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

2024-04-23 Thread Jacob Champion
On Mon, Apr 22, 2024 at 2:20 PM Jelte Fennema-Nio wrote: > 1. I strongly believe minor protocol version bumps after the initial > 3.1 one can be made painless for clients/poolers (so the ones to > 3.2/3.3/etc). Similar to how TLS 1.3 can be safely introduced, and not > having to worry about

Re: Direct SSL connection with ALPN and HBA rules

2024-04-23 Thread Jacob Champion
On Mon, Apr 22, 2024 at 10:42 PM Michael Paquier wrote: > > On Mon, Apr 22, 2024 at 10:47:51AM +0300, Heikki Linnakangas wrote: > > On 22/04/2024 10:19, Michael Paquier wrote: > >> As a whole, I can get behind a unique GUC that forces the use of > >> direct. Or, we could extend the existing

Re: Direct SSL connection with ALPN and HBA rules

2024-04-23 Thread Jacob Champion
On Fri, Apr 19, 2024 at 2:43 PM Heikki Linnakangas wrote: > > On 19/04/2024 19:48, Jacob Champion wrote: > > On Fri, Apr 19, 2024 at 6:56 AM Heikki Linnakangas wrote: > >> With direct SSL negotiation, we always require ALPN. > > > > (As an aside: I h

Re: Direct SSL connection with ALPN and HBA rules

2024-04-19 Thread Jacob Champion
On Fri, Apr 19, 2024 at 6:56 AM Heikki Linnakangas wrote: > On 19/04/2024 08:06, Michael Paquier wrote: > > Since 91044ae4baea (require ALPN for direct SSL connections) and > > d39a49c1e459 (direct hanshake), direct SSL connections are supported > > (yeah!), still the thread where this has been

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

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

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

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

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

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

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

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

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

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

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: 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

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 depend

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

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

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

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

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

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 >

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: [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

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: 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-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-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 alternativ

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 >

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

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

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: [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: 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

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: 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: 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

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: WIP Incremental JSON Parser

2024-03-18 Thread Jacob Champion
ex->token_terminator = dummy_lex.token_terminator; if (partial_result == JSON_SUCCESS) lex->inc_state->partial_completed = true; return partial_result; commit 3d615593d6d79178a8b8a208172414806d40cc03 Author: Jacob Champion Date: Mon Mar 11

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

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: 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

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

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

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

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) > { >

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), > +

  1   2   3   4   5   6   7   8   9   >