Re: pg_dump: sortDumpableObjectsByTypeName() doesn't always do that

2018-08-06 Thread Jacob Champion
On Mon, Aug 6, 2018 at 12:13 PM Tom Lane wrote: > Do you mean "incorrect results", or just "unstable results"? > If the former, what's incorrect about it? Incorrect, as in "the results are not sorted by type name." Here's an example ordering that we saw -- but note that you won't be able to

Re: pg_dump: sortDumpableObjectsByTypeName() doesn't always do that

2018-08-06 Thread Jacob Champion
On Mon, Aug 6, 2018 at 12:23 PM Jacob Champion wrote: > since the > root cause is that we're not defining a valid ordering, quicksort may > or may not behave consistently for test purposes. To expand on this, consider three objects, of the same type, compared with the current comparator

Re: pg_dump: sortDumpableObjectsByTypeName() doesn't always do that

2018-08-06 Thread Jacob Champion
On Mon, Aug 6, 2018 at 12:45 PM Tom Lane wrote: > Ah, gotcha. But whether the behavior is sane or not, it'd be reproducible > for any specific input dataset on any specific platform (unless you've got > a quicksort that actually uses randomized pivots; but ours doesn't, and > I think that

Re: pg_dump: sortDumpableObjectsByTypeName() doesn't always do that

2018-08-07 Thread Jacob Champion
On Tue, Aug 7, 2018 at 10:24 AM Tom Lane wrote: > I don't see any reason to insist on a test case before pushing this > fix, so I did that. (As I expected, the fix doesn't change any existing > regression test results.) Thanks! We have made some progress towards a repro but we're having

pg_dump: sortDumpableObjectsByTypeName() doesn't always do that

2018-08-06 Thread Jacob Champion
Hi all, We recently ran into an issue in pg_dump that caused the initial sort-by-name pass to return incorrect results. It doesn't seem to affect overall correctness, since the later toposort pass takes care of dependencies, but it does occasionally cause a spurious diff in dump output before and

Re: Proposal: http2 wire format

2018-03-25 Thread Jacob Champion
On Sun, Mar 25, 2018 at 8:11 PM, Craig Ringer wrote: > As others have noted, you'll want to find a way to handle this in the least > SSL-implementation-specific manner possible. IMO if it can't work with > OpenSSL, Windows's SSL implementation and OS X's SSL framework it's

Re: libpq debug log

2018-11-12 Thread Jacob Champion
On Tue, Oct 30, 2018 at 2:39 AM Iwata, Aya wrote: > I create a first libpq trace log patch. Couple additional thoughts from a read-through of the patch: - PQtrace() and the new trace-logging machinery overlap in some places but not others -- and if both are set, PQtrace() will take precedence.

Re: Obtaining a more consistent view definition when a UNION subquery contains undecorated constants

2018-10-02 Thread Jacob Champion
On Thu, Sep 27, 2018 at 3:38 PM Tom Lane wrote: > Jimmy Yih writes: > > Looking at the internal code (mostly get_from_clause_item() function), we > > saw that when a subquery is used, there is no tuple descriptor passed to > > get_query_def() function. Because of this, get_target_list() uses the

Re: libpq debug log

2019-02-14 Thread Jacob Champion
On Thu, Feb 14, 2019 at 10:17 AM Andres Freund wrote: > On 2018-11-28 23:20:03 +0100, Peter Eisentraut wrote: > > This does not excite me. It seems mostly redundant with using tcpdump. > > I think the one counter-argument to this is that using tcpdump in > real-world scenarios has become quite

Re: Proposal: Make use of C99 designated initialisers for nulls/values arrays

2019-10-04 Thread Jacob Champion
On Fri, Oct 4, 2019 at 7:51 AM Tom Lane wrote: > I concur with Joe here. The reason why some of the existing > memset's use "false" is for symmetry with other places where we use > "memset(p, true, n)" to set an array of bools to all-true. That > coding is unfortunately a bit dubious --- it

Re: Support for NSS as a libpq TLS backend

2020-12-03 Thread Jacob Champion
On Nov 17, 2020, at 7:00 AM, Daniel Gustafsson wrote: > > Nice, thanks for the fix! I've incorporated your patch into the attached v20 > which also fixes client side error reporting to be more readable. I was testing handshake failure modes and noticed that some FATAL messages are being sent

Re: [PATCH] Support negative indexes in split_part

2020-11-10 Thread Jacob Champion
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:not tested Patch looks good to me. Seems like a useful feature, and I agree

Re: Support for NSS as a libpq TLS backend

2020-11-10 Thread Jacob Champion
On Nov 6, 2020, at 3:11 PM, Daniel Gustafsson wrote: > > The attached switches to SSL_ConfigServerSessionIDCacheWithOpt > with which one can explicitly make the cache non-shared, which in turn backs > the mutexes with NSPR locks rather than the missing sem_init. Can you test > this version and

Re: Support for NSS as a libpq TLS backend

2020-11-11 Thread Jacob Champion
On Nov 11, 2020, at 10:17 AM, Jacob Champion wrote: > > (Two failures left on my machine.) False alarm -- the stderr debugging I'd added in to track down the assertion tripped up the "no stderr" tests. Zero failing tests now. --Jacob

Re: Support for NSS as a libpq TLS backend

2020-11-11 Thread Jacob Champion
On Nov 10, 2020, at 2:28 PM, Daniel Gustafsson wrote: > > Digging through the archives from when this landed in curl, the assertion > failure was never fully identified back then but happened spuriously. Which > version of NSPR is this happening with? This is NSPR 4.29, with debugging enabled.

Re: Support for NSS as a libpq TLS backend

2020-11-13 Thread Jacob Champion
On Nov 11, 2020, at 10:57 AM, Jacob Champion wrote: > > False alarm -- the stderr debugging I'd added in to track down the > assertion tripped up the "no stderr" tests. Zero failing tests now. I took a look at the OpenSSL interop problems you mentioned upthread. I don't see

Re: Zedstore - compressed in-core columnar storage

2020-11-13 Thread Jacob Champion
On Nov 12, 2020, at 2:40 PM, Tomas Vondra wrote: > > Hi, > > Thanks for the updated patch. It's a quite massive amount of code - I I > don't think we had many 2MB patches in the past, so this is by no means > a full review. Thanks for taking a look! You're not kidding about the patch size.

Re: Zedstore - compressed in-core columnar storage

2020-11-17 Thread Jacob Champion
On Nov 13, 2020, at 2:00 PM, Tomas Vondra wrote: > > Fedora 32, nothing special. I'm not sure if I ran the tests with pglz or > lz4, maybe there's some dependence on that, but it does fail for me > quite reliably with this: > > ./configure --enable-debug --enable-cassert --enable-tap-tests >

Re: Support for NSS as a libpq TLS backend

2020-11-16 Thread Jacob Champion
On Nov 13, 2020, at 4:14 AM, Daniel Gustafsson wrote: >> On 12 Nov 2020, at 23:12, Jacob Champion wrote: >> >> I'm not completely sure why this is exposed so easily with an OpenSSL >> server -- I'm guessing the implementation slices up its packets >> different

Re: Support for NSS as a libpq TLS backend

2020-11-06 Thread Jacob Champion
On Nov 4, 2020, at 5:09 AM, Daniel Gustafsson wrote: > (sorry for slow response). You are absolutely right, the has_password flag > must be tracked per connection in PGconn. The attached v17 implements this as > well a frontend bugfix which caused dropped connections and some smaller > fixups

Re: Allow matching whole DN from a client certificate

2021-01-20 Thread Jacob Champion
On Mon, 2021-01-18 at 11:23 +0100, Daniel Gustafsson wrote: > + /* use commas instead of slashes */ > + X509_NAME_print_ex(bio, x509name, 0, XN_FLAG_SEP_COMMA_PLUS); > I don't have strong opinions on whether we shold use slashes or commas, but I > think it needs to be

Re: Allow matching whole DN from a client certificate

2021-01-20 Thread Jacob Champion
On Wed, 2021-01-20 at 19:07 +, Jacob Champion wrote: > I think you'll want to be careful to specify the format as much as > possible, both to make sure that other backend TLS implementations can > actually use the same escaping system and to ensure that user regexes > don't su

Re: Fixing cache pollution in the Kerberos test suite

2021-01-25 Thread Jacob Champion
On Mon, 2021-01-25 at 13:49 -0500, Tom Lane wrote: > Yeah, changing global state is just awful. However, I don't > actually see any change here (RHEL8): Interesting. I'm running Ubuntu 20.04: $ klist klist: No credentials cache found (filename: /tmp/krb5cc_1000) $ make check ... $ klist

Fixing cache pollution in the Kerberos test suite

2021-01-25 Thread Jacob Champion
to use it via the KRB5CCNAME envvar. This prevents the global cache pollution. WDYT? --Jacob From c9532e72a762abeef0996ad8df5da9bbdedbccad Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 25 Jan 2021 09:32:44 -0800 Subject: [PATCH] test/kerberos: use a local credentials cache Previously

Re: Fixing cache pollution in the Kerberos test suite

2021-01-25 Thread Jacob Champion
On Mon, 2021-01-25 at 14:04 -0500, Tom Lane wrote: > Jacob Champion writes: > > On Mon, 2021-01-25 at 13:49 -0500, Tom Lane wrote: > > > Also, why are you only setting the ENV variable within narrow parts > > > of the test script? I'd be inclined to enforce it t

Re: Fixing cache pollution in the Kerberos test suite

2021-01-25 Thread Jacob Champion
On Mon, 2021-01-25 at 14:36 -0500, Tom Lane wrote: > However, this doesn't seem to explain why the test script isn't > causing a global state change. Whether the state is held in a > file or the sssd daemon shouldn't matter, it seems like. > > Also, it looks like the test causes /tmp/krb5cc_ to

Re: Support for NSS as a libpq TLS backend

2021-01-21 Thread Jacob Champion
On Thu, 2021-01-21 at 14:21 +0900, Michael Paquier wrote: > Also, what's the minimum version of NSS that would be supported? It > would be good to define an acceptable older version, to keep that > documented and to track that perhaps with some configure checks (?), > similarly to what is done

Re: Support for NSS as a libpq TLS backend

2021-01-21 Thread Jacob Champion
On Mon, 2020-07-20 at 15:35 +0200, Daniel Gustafsson wrote: > With this, I have one failing test ("intermediate client certificate is > provided by client") which I've left failing since I believe the case should > be > supported by NSS. The issue is most likely that I havent figured out the >

Re: Support for NSS as a libpq TLS backend

2021-01-19 Thread Jacob Champion
On Tue, 2021-01-19 at 21:21 +0100, Daniel Gustafsson wrote: > There is something iffy with these certs (the test fails > on mismatching ciphers and/or signature algorithms) that I haven't been able > to > pin down, but to get more eyes on this I'm posting the patch with the test > enabled.

Re: Support for NSS as a libpq TLS backend

2021-01-13 Thread Jacob Champion
On Tue, 2020-10-27 at 21:07 +0100, Daniel Gustafsson wrote: > > On 20 Oct 2020, at 21:15, Andres Freund wrote: > > > > > +static SECStatus > > > +pg_cert_auth_handler(void *arg, PRFileDesc * fd, PRBool checksig, PRBool > > > isServer) > > > +{ > > > + SECStatus status; > > > + Port

Re: Proposal: Save user's original authenticated identity for logging

2021-01-29 Thread Jacob Champion
On Fri, 2021-01-29 at 17:30 -0500, Tom Lane wrote: > What happens if ALTER USER RENAME is done while the session is still > alive? IMO the authenticated identity should be write-once. Especially since one of my goals is to have greater auditability into events as they've actually happened. So

Re: Support for NSS as a libpq TLS backend

2021-01-29 Thread Jacob Champion
On Fri, 2021-01-29 at 13:57 +0100, Daniel Gustafsson wrote: > > On 21 Jan 2021, at 06:21, Michael Paquier wrote: > > I really need to study more the choide of the options chosen for > > NSS_InitContext()... But based on the docs I can read on the matter I > > think that saving nsscontext in

Re: Proposal: Save user's original authenticated identity for logging

2021-01-29 Thread Jacob Champion
On Fri, 2021-01-29 at 17:01 -0500, Stephen Frost wrote: > > - for LDAP, the bind DN is discarded entirely; > > We don't support pg_ident.conf-style entries for LDAP, meaning that the > user provided has to match what we check, so I'm not sure what would be > improved with this change..? For

Re: Proposal: Save user's original authenticated identity for logging

2021-01-29 Thread Jacob Champion
On Fri, 2021-01-29 at 18:40 -0500, Tom Lane wrote: > Ah. So basically, this comes into play when you consider that some > outside-the-database entity is your "real" authenticated identity. > That seems reasonable when using Kerberos or the like, though it's > not real meaningful for traditional

Re: Allow matching whole DN from a client certificate

2021-01-28 Thread Jacob Champion
On Wed, 2021-01-27 at 15:42 -0500, Andrew Dunstan wrote: > I'm not sure where we want to go with the present patch. Do we want to > go with what we have and document the limitations more, or try to do > something more elaborate? If so, exactly what? After sleeping on it: I think that if you

Re: Support for NSS as a libpq TLS backend

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 21:49 +0100, Daniel Gustafsson wrote: > > Embedded NULLs are now handled in a similar manner to the OpenSSL side, > > though because this failure happens during the certificate > > authentication callback, it results in a TLS alert rather than simply > > closing the

Re: Support for NSS as a libpq TLS backend

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 21:49 +0100, Daniel Gustafsson wrote: > > On 29 Jan 2021, at 19:46, Jacob Champion wrote: > > I think the bad news is that the static approach will need support for > > ENABLE_THREAD_SAFETY. > > I did some more reading today and noticed th

Re: Proposal: Save user's original authenticated identity for logging

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 18:40 -0500, Stephen Frost wrote: > * Jacob Champion (pchamp...@vmware.com) wrote: > > My goal is to get this one single point of reference, for all of the > > auth backends. The LDAP mapping conversation is separate. > > Presumably this wo

Re: Support for NSS as a libpq TLS backend

2021-01-27 Thread Jacob Champion
On Wed, 2021-01-27 at 16:39 +0900, Michael Paquier wrote: > My apologies for chiming in. I was looking at your patch set here, > and while reviewing the strong random and cryptohash parts I have > found a couple of mistakes in the ./configure part. I think that the > switch from --with-openssl

Re: Allow matching whole DN from a client certificate

2021-01-26 Thread Jacob Champion
On Tue, 2021-01-26 at 13:49 +0100, Daniel Gustafsson wrote: > Reading more on this it seems we would essentially have to go with RFC 4514, > as > it's the closest to a standard which seems to exist. Having done a lot > research on this topic, do you have a gut feeling on direction? Yeah, if

Re: Allow matching whole DN from a client certificate

2021-01-27 Thread Jacob Champion
On Tue, 2021-01-26 at 18:43 +, Jacob Champion wrote: > On Tue, 2021-01-26 at 13:49 +0100, Daniel Gustafsson wrote: > > The OpenSSL X509_NAME_cmp function use RFC 5280 section 7.1 and RFC 4517 > > section 4.2.15 (which in turn reference RFC 4514 for the DN string format).

Re: Support for NSS as a libpq TLS backend

2021-01-28 Thread Jacob Champion
On Thu, 2021-01-21 at 20:16 +, Jacob Champion wrote: > I think we're missing a counterpart to this piece of the OpenSSL > implementation, in be_tls_init(): Never mind. Using SSL_SetTrustAnchor is something we could potentially do if we wanted to further limit the CAs that are actuall

Proposal: Save user's original authenticated identity for logging

2021-01-28 Thread Jacob Champion
et From 3f6e87a744be339748fc707cd071896e81e0323c Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Fri, 22 Jan 2021 14:03:14 -0800 Subject: [PATCH] WIP: log authenticated identity from multiple auth backends This is stored into port->authn_id according to the auth method: ldap: the final bind DN gss: the u

Re: Support for NSS as a libpq TLS backend

2021-06-15 Thread Jacob Champion
On Wed, 2021-06-16 at 00:08 +0200, Daniel Gustafsson wrote: > > On 15 Jun 2021, at 00:15, Jacob Champion wrote: > > Attached is a quick patch; does it work on your machine? > > It does, thanks! I've included it in the attached v37 along with a few tiny > non-functional im

Re: Support for NSS as a libpq TLS backend

2021-06-14 Thread Jacob Champion
o I don't know if this latest rebase introduced it or not.) Attached is a quick patch; does it work on your machine? --Jacob From 7a7b8904ef22212190bb988fab1ef696fe1a59de Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 14 Jun 2021 15:04:26 -0700 Subject: [PATCH] test/ssl: fix NSS server-side

Re: Support for NSS as a libpq TLS backend

2021-06-16 Thread Jacob Champion
On Wed, 2021-06-16 at 15:31 +0200, Daniel Gustafsson wrote: > > On 16 Jun 2021, at 01:50, Jacob Champion wrote: > > I've been tracking down reference leaks in the client. These open > > references prevent NSS from shutting down cleanly, which then makes it > > impossib

Re: Table AM modifications to accept column projection lists

2021-06-11 Thread Jacob Champion
On Sat, 2021-06-05 at 09:47 -0700, Zhihong Yu wrote: > On Fri, Jun 4, 2021 at 4:14 PM Jacob Champion wrote: > > Agreed. I'm going to double-check with Deep that the new calls > > to table_tuple_fetch_row_version() should be projecting the full row, > > then post an updated

Re: [PoC] Federated Authn/z with OAUTHBEARER

2021-06-22 Thread Jacob Champion
On Fri, 2021-06-18 at 11:31 +0300, Heikki Linnakangas wrote: > On 08/06/2021 19:37, Jacob Champion wrote: > > We've been working on ways to expand the list of third-party auth > > methods that Postgres provides. Some example use cases might be "I want > > to let anyone

Re: [PoC] Federated Authn/z with OAUTHBEARER

2021-06-22 Thread Jacob Champion
On Fri, 2021-06-18 at 13:07 +0900, Michael Paquier wrote: > On Tue, Jun 08, 2021 at 04:37:46PM +0000, Jacob Champion wrote: > > 1. Prep > > > > 0001 decouples the SASL code from the SCRAM implementation. > > 0002 makes it possible to use common/jsonapi from th

[PATCH] Pull general SASL framework out of SCRAM

2021-06-22 Thread Jacob Champion
isn't implemented to spec. --Jacob [1] https://www.postgresql.org/message-id/d1b467a78e0e36ed85a09adf979d04cf124a9d4b.ca...@vmware.com [2] https://datatracker.ietf.org/doc/html/rfc4422 From a6a65b66cc3dc5da7219378dbadb090ff10fd42b Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Tue, 13 Apr 2021 10:25:48 -0700 Subj

[PATCH] Make jsonapi usable from libpq

2021-06-22 Thread Jacob Champion
b9ff41a4640ec69491b393d54 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 3 May 2021 11:15:15 -0700 Subject: [PATCH 2/7] src/common: remove logging from jsonapi for shlib The can't-happen code in jsonapi was pulling in logging code, which for libpq is not included. --- src/common/Make

Re: Granting control of SUSET gucs to non-superusers

2021-05-13 Thread Jacob Champion
On Thu, 2021-05-13 at 11:42 -0700, Mark Dilger wrote: > The distinction that Theme+Security would make is that capabilities > can be categorized by the area of the system: > -- planner > -- replication > -- logging > ... > but also by the security implications of what is being done: > --

Re: Table AM modifications to accept column projection lists

2021-06-04 Thread Jacob Champion
On Tue, 2021-06-01 at 15:38 +0300, Aleksander Alekseev wrote: > I came across this patch and noticed that it rotted a little, > especially after removing inheritance_planner() in 86dc9005. I > managed to resolve the conflicts on current `master` (eb89cb43), see > the attached patch. The code

Re: SSL SNI

2021-06-03 Thread Jacob Champion
On Wed, 2021-04-07 at 15:32 +0200, Peter Eisentraut wrote: > Committed like that. (Default to on, but it's easy to change if there > are any further thoughts.) Hi Peter, It looks like this code needs some guards for a NULL conn->pghost. For example when running psql 'dbname=postgres

Re: [PATCH] Make jsonapi usable from libpq

2021-07-07 Thread Jacob Champion
On Wed, 2021-07-07 at 01:42 -0400, Tom Lane wrote: > Michael Paquier writes: > > It seems to me that this does not address yet the problems with the > > palloc/pstrdup in jsonapi.c though, which would need to rely on > > malloc() if we finish to use this code in libpq. I am not sure yet > > that

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-07 Thread Jacob Champion
On Wed, 2021-07-07 at 14:08 +0900, Michael Paquier wrote: > On Tue, Jul 06, 2021 at 06:20:49PM +0000, Jacob Champion wrote: > > On Mon, 2021-07-05 at 17:17 +0900, Michael Paquier wrote: > > > > > Hmm. Does the RFCs tell us anything about that? > > > > Just

Re: badly calculated width of emoji in psql

2021-07-07 Thread Jacob Champion
On Mon, 2021-04-05 at 14:07 +0900, Kyotaro Horiguchi wrote: > At Fri, 2 Apr 2021 11:51:26 +0200, Pavel Stehule > wrote in > > with this patch, the formatting is correct > > I think the hardest point of this issue is that we don't have a > reasonable authoritative source that determines

Re: Preventing abort() and exit() calls in libpq

2021-07-02 Thread Jacob Champion
On Fri, 2021-07-02 at 18:20 -0400, Tom Lane wrote: > What configure options are you using? Just `./configure --enable-coverage`, nothing else. I distclean'd right before for good measure. > Does "nm -u" report "exit" being referenced from any *.o in libpq, > or from any *_shlib.o in src/port/ or

Re: Preventing abort() and exit() calls in libpq

2021-07-02 Thread Jacob Champion
On Fri, 2021-07-02 at 18:45 -0400, Tom Lane wrote: > Jacob Champion writes: > > On Fri, 2021-07-02 at 18:20 -0400, Tom Lane wrote: > > > What configure options are you using? > > Just `./configure --enable-coverage`, nothing else. I distclean'd right > > befo

Re: Preventing abort() and exit() calls in libpq

2021-07-02 Thread Jacob Champion
On Wed, 2021-06-30 at 10:42 -0400, Tom Lane wrote: > Alvaro Herrera writes: > > On 2021-Jun-30, Tom Lane wrote: > > > You mentioned __gcov_exit, but I'm not sure if we need an > > > exception for that. I see it referenced by the individual .o > > > files, but the completed .so has no such

Re: Preventing abort() and exit() calls in libpq

2021-06-30 Thread Jacob Champion
On Wed, 2021-06-30 at 18:56 -0400, Tom Lane wrote: > Jacob Champion writes: > > On Wed, 2021-06-30 at 18:29 -0400, Tom Lane wrote: > > > Looks like we'd have to make use of a dummy stamp-file, more or less > > > as attached. Any objections? > > Spitballing --

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-06-30 Thread Jacob Champion
On Sat, 2021-06-26 at 09:47 +0900, Michael Paquier wrote: > On Fri, Jun 25, 2021 at 11:40:33PM +0000, Jacob Champion wrote: > > I can definitely move it (into, say, auth-sasl.c?). I'll probably do > > that in a second commit, though, since keeping it in place during the >

Re: Preventing abort() and exit() calls in libpq

2021-06-30 Thread Jacob Champion
On Wed, 2021-06-30 at 18:29 -0400, Tom Lane wrote: > I wrote: > > Peter Eisentraut writes: > > > Could we set this rule up a little bit differently so that it is only > > > run when the library is built. > > > Right now, make world on a built tree makes 17 calls to this "nm" line, > > > and

Re: Table AM modifications to accept column projection lists

2021-06-30 Thread Jacob Champion
Hi all, Ashwin, Deep, and I were discussing this patch today. We agree that it's fairly difficult to review in its current state, and the lack of a concrete implementation of the new API isn't helping. (A big chunk of the context for the patch exists in the zedstore megathread, which isn't

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Jacob Champion
On Sat, 2021-06-26 at 09:36 +0900, Michael Paquier wrote: > On Fri, Jun 25, 2021 at 08:58:46PM +0000, Jacob Champion wrote: > > On Thu, 2021-06-24 at 14:56 +0900, Michael Paquier wrote: > > > Looking more closely at that, I actually find a bit crazy the > > > require

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Jacob Champion
On Sun, 2021-06-27 at 10:43 +0900, Michael Paquier wrote: > On Sat, Jun 26, 2021 at 01:43:50PM -0400, Tom Lane wrote: > > BTW, so far as the original topic of this thread is concerned, > > it sounds like Jacob's ultimate goal is to put some functionality > > into libpq that requires JSON parsing.

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Jacob Champion
On Tue, 2021-06-29 at 14:50 -0400, Tom Lane wrote: > Jacob Champion writes: > > What would you think about a src/port of asprintf()? Maybe libpq > > doesn't change quickly enough to worry about it, but having developers > > revisit stack allocation for strings every time

Re: [PATCH] test/ssl: rework the sslfiles Makefile target

2021-06-29 Thread Jacob Champion
On Thu, 2021-03-04 at 00:03 +, Jacob Champion wrote: > Hello all, > > Andrew pointed out elsewhere [1] that it's pretty difficult to add new > certificates to the test/ssl suite without blowing away the current > state and starting over. I needed new cases for the NSS backend

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-06-25 Thread Jacob Champion
On Wed, 2021-06-23 at 16:38 +0900, Michael Paquier wrote: > On Tue, Jun 22, 2021 at 10:37:29PM +0000, Jacob Champion wrote: > > Currently, the SASL logic is tightly coupled to the SCRAM > > implementation. This patch untangles the two, by introducing callback > > structs

Re: [PATCH] Make jsonapi usable from libpq

2021-06-25 Thread Jacob Champion
On Thu, 2021-06-24 at 14:56 +0900, Michael Paquier wrote: > Looking more closely at that, I actually find a bit crazy the > requirement for any logging within jsonapi.c just to cope with the > fact that json_errdetail() and report_parse_error() just want to track > down if the caller is giving

Re: Dependency to logging in jsonapi.c

2021-06-30 Thread Jacob Champion
On Wed, 2021-06-30 at 11:03 -0400, Tom Lane wrote: > It does not look to me like json_errdetail can sensibly be used in > frontend, since it returns palloc'd strings in some paths and > constants in others. There'd be no way to avoid a memory leak > in a frontend usage. So I think the dependency

Re: Remove unused code from the KnownAssignedTransactionIdes submodule

2021-07-01 Thread Jacob Champion
On Tue, 2021-06-08 at 17:32 +0800, Quan Zongliang wrote: > Hi, > > In the KnownAssignedTransactionIdes sub-module, two lines of unused code > were found in a previous change. Huh. Looks like this code died as part of 2fc7af5e966? CC'ing Thomas just in case we're missing something, but I'll

Re: Preventing abort() and exit() calls in libpq

2021-07-01 Thread Jacob Champion
On Thu, 2021-07-01 at 14:14 -0400, Tom Lane wrote: > Peter Eisentraut writes: > > Somewhere in the $(shlib) rule would seem most appropriate. But I don't > > understand the rest: What ifeq, and why .DELETE_ON_ERROR? > > The variant of this I'd been thinking of was > > $(shlib): $(OBJS) |

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-12 Thread Jacob Champion
On Sun, 2021-07-11 at 13:16 +0900, Michael Paquier wrote: > On Fri, Jul 09, 2021 at 11:31:48PM +0000, Jacob Champion wrote: > > On Thu, 2021-07-08 at 16:27 +0900, Michael Paquier wrote: > > > + * outputlen: The length (0 or higher) of the client resp

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-06 Thread Jacob Champion
On Mon, 2021-07-05 at 17:17 +0900, Michael Paquier wrote: > On Wed, Jun 30, 2021 at 10:30:12PM +0000, Jacob Champion wrote: > > Done in v3, with a second patch for the code motion. > > I have gone through that, tweaking the documentation you have added as > that's the meat of th

Re: PROXY protocol support

2021-07-08 Thread Jacob Champion
Hi Magnus, I'm only just starting to page this back into my head, so this is by no means a full review of the v7 changes -- just stuff I've noticed over the last day or so of poking around. On Tue, 2021-06-29 at 11:48 +0200, Magnus Hagander wrote: > On Thu, Mar 11, 2021 at 12:05 AM Ja

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-09 Thread Jacob Champion
On Thu, 2021-07-08 at 16:27 +0900, Michael Paquier wrote: > I agree that this looks like an improvement in terms of the > expectations behind a SASL mechanism, so I have done the attached to > strengthen a bit all those checks. However, I don't really see a > point in back-patching any of that,

Re: SSL/TLS instead of SSL in docs

2021-06-30 Thread Jacob Champion
On Wed, 2021-06-30 at 20:20 +0200, Peter Eisentraut wrote: > I note that popular places such as the Apache and nginx SSL/TLS modules > just use SSL in their documentation, and they are probably under much > more scrutiny in this area. For Apache, that's not strictly true [1, 2]. mod_ssl and its

Re: Allow matching whole DN from a client certificate

2021-02-08 Thread Jacob Champion
On Sat, 2021-01-30 at 16:18 -0500, Andrew Dunstan wrote: > Here's a version of the patch that does it that way. For fun I have > modified the certificate so it has two OU fields in the DN. > diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml > [...] > + Common Name

Re: Proposal: Save user's original authenticated identity for logging

2021-02-08 Thread Jacob Champion
On Tue, 2021-02-02 at 22:22 +, Jacob Champion wrote: > Given the feedback above, I'll continue to flesh out the PoC patch, > focusing on 1) storing the identity in a single place for all auth > methods and 2) exposing it consistently in the logs as part of > log_connections. Atta

Re: Proposal: Save user's original authenticated identity for logging

2021-02-02 Thread Jacob Champion
On Thu, 2021-01-28 at 18:22 +, Jacob Champion wrote: > = Proposal = > > I propose that every auth method should store the string it uses to > identify a user -- what I'll call an "authenticated identity" -- into > one central location in Port, after authentication

Re: Proposal: Save user's original authenticated identity for logging

2021-02-01 Thread Jacob Champion
On Sun, 2021-01-31 at 12:27 +0100, Magnus Hagander wrote: > > (There's also the fact that I think pg_ident mapping for LDAP would be > > just as useful as it is for GSS or certs. That's for a different > > conversation.) > > Specifically for search+bind, I would assume? Even for the simple bind

Re: Proposal: Save user's original authenticated identity for logging

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 11:49 -0500, Stephen Frost wrote: > * Magnus Hagander (mag...@hagander.net) wrote: > > But yes, I think the enforced cleartext password proxying is at the > > core of the problem. LDAP also encourages the idea of centralized > > password-reuse, which is not exactly a great

Re: Proposal: Save user's original authenticated identity for logging

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 18:44 +0100, Magnus Hagander wrote: > What people would *really* want I think is "alow auto-creation of new > roles, and then look up which other roles they should be members of > using ldap" (or "using this script over here" for a more flexible > approach). Which is of

Re: Proposal: Save user's original authenticated identity for logging

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 17:01 -0500, Stephen Frost wrote: > * Jacob Champion (pchamp...@vmware.com) wrote: > > And I'm not holding > > my breath for LDAP servers to start implementing federated identity, > > though that would be nice. > > Not sure exactly what you'

Re: Proposal: Save user's original authenticated identity for logging

2021-02-01 Thread Jacob Champion
On Mon, 2021-02-01 at 18:01 -0500, Stephen Frost wrote: > Ok.. but what's 'go' mean here? We already have views and such for GSS > and SSL, is the idea to add another view for LDAP and add in columns > that are returned by pg_stat_get_activity() which are then pulled out by > that view? Or did

Re: Proposal: Save user's original authenticated identity for logging

2021-03-24 Thread Jacob Champion
ore authentication could signal a serious bug -- but I don't feel too strongly about it. v10 attached, which reverts to v8 test behavior, with minor updates to the commit message and test comment. --Jacob From 0b8764ac61ef37809a79ded2596c5fcd2caf25bb Mon Sep 17 00:00:00 2001 From: Jacob Champion

Re: Support for NSS as a libpq TLS backend

2021-03-24 Thread Jacob Champion
On Wed, 2021-03-24 at 09:28 +0900, Michael Paquier wrote: > On Wed, Mar 24, 2021 at 12:05:35AM +0000, Jacob Champion wrote: > > I can work around it temporarily for the > > tests, but this will be a problem if any libpq clients load up multiple > > independent databases

Re: Support for NSS as a libpq TLS backend

2021-03-24 Thread Jacob Champion
On Wed, 2021-03-24 at 13:00 -0400, Stephen Frost wrote: > * Jacob Champion (pchamp...@vmware.com) wrote: > > Right, but to clarify -- I was asking if *NSS* supports loading and > > using separate certificate databases as part of its API. It seems like > > the internals ma

Re: Support for NSS as a libpq TLS backend

2021-03-26 Thread Jacob Champion
On Fri, 2021-03-26 at 15:33 -0400, Stephen Frost wrote: > * Jacob Champion (pchamp...@vmware.com) wrote: > > Databases that are opened *after* the first one are given their own > > separate slots. [...] > > This is more-or-less what we would want though, rig

Re: Proposal: Save user's original authenticated identity for logging

2021-03-26 Thread Jacob Champion
below, because authentication +* has already succeeded and we want the log file to reflect that. */ if (!port->peer_dn) { From c21a83e71d2829accf004f5b8437a6eb115b0860 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 8 Fe

Re: Proposal: Save user's original authenticated identity for logging

2021-03-25 Thread Jacob Champion
$common_connstr, @@ -132,7 +102,7 @@ test_connect_fails( qr/channel binding required, but server authenticated client without channel binding/, "Cert authentication and channel_binding=require"); -$log = $node->rotate_logfile(); +my $log = $node->rotate_logfile(); $node-

Re: DETAIL for wrong scram password

2021-03-25 Thread Jacob Champion
On Thu, 2021-03-25 at 16:41 +0900, Michael Paquier wrote: > On top of what's > proposed, would it make sense to have a second logdetail for the case > of a mock authentication? We don't log that yet, so I guess that it > could be useful for audit purposes? It looks like the code paths that lead

Re: Support for NSS as a libpq TLS backend

2021-03-25 Thread Jacob Champion
On Fri, 2021-03-26 at 00:22 +0100, Daniel Gustafsson wrote: > > On 23 Mar 2021, at 20:04, Stephen Frost wrote: > > > > Eh, poor wording on my part. You're right, the question, reworded > > again, was "Would someone want to get the context returned by > > NSS_InitContext?". If we think there's

Re: Proposal: Save user's original authenticated identity for logging

2021-03-29 Thread Jacob Champion
On Mon, 2021-03-29 at 16:50 +0900, Michael Paquier wrote: > On Fri, Mar 26, 2021 at 10:41:03PM +0000, Jacob Champion wrote: > > For a few of the bugs I was tracking down, it was imperative. The tests > > aren't isolated enough (or at all) to keep one from affecting

Re: Proposal: Save user's original authenticated identity for logging

2021-03-30 Thread Jacob Champion
On Tue, 2021-03-30 at 09:55 +0900, Michael Paquier wrote: > On Mon, Mar 29, 2021 at 11:53:03PM +0000, Jacob Champion wrote: > > It's not a matter of the tests being stable, but of the tests needing > > to change and evolve as the implementation changes. A big part of that &g

Re: Proposal: Save user's original authenticated identity for logging

2021-04-01 Thread Jacob Champion
100644 --- a/src/test/ssl/t/002_scram.pl +++ b/src/test/ssl/t/002_scram.pl @@ -97,20 +97,10 @@ $node->connect_fails( "Cert authentication and channel_binding=require"); # Certificate verification at the connection level should still work fine. -# Truncate once the logs, to ensure

Re: Proposal: Save user's original authenticated identity for logging

2021-04-05 Thread Jacob Champion
ntradiction with the comments a couple of lines > above anyway. What do you mean by this? I took another look at the comment and it seems to match the implementation. v21 attached, which is just a rebase of my original v19. --Jacob [1] https://www.postgresql.org/message-id/8c08c6402051b5348d599

Re: Proposal: Save user's original authenticated identity for logging

2021-04-02 Thread Jacob Champion
0003 is basically doing already. Rebased on top of your patch as v19, attached. (v17 disappeared into the ether somewhere, I think. :D) Now that it's easy to add log_like to existing tests, I fleshed out the LDAP tests with a few more cases. They don't add code coverage, but they pin the desired

Re: Support for NSS as a libpq TLS backend

2021-03-31 Thread Jacob Champion
On Fri, 2021-03-26 at 18:05 -0400, Stephen Frost wrote: > * Jacob Champion (pchamp...@vmware.com) wrote: > > Yeah. I was hoping to avoid implementing our own locks and refcounts, > > but it seems like it's going to be required. > > Yeah, afraid so. I think it gets worse,

  1   2   3   4   5   6   7   8   9   >