On Fri, Mar 24, 2023 at 09:30:06AM -0700, Jacob Champion wrote:
> 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
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
On Thu, Mar 23, 2023 at 03:40:55PM -0700, Jacob Champion wrote:
> On Tue, Mar 21, 2023 at 11:01 PM Michael Paquier wrote:
>> contrib/sslinfo/ has ssl_client_cert_present(), that we could use in
>> the tests to make sure that the client has actually sent a
>> certificate? How about adding some of
On Tue, Mar 21, 2023 at 11:01 PM Michael Paquier wrote:
> - # Function introduced in OpenSSL 1.0.2.
> + # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of
> these.
>['X509_get_signature_nid'],
> + ['SSL_CTX_set_cert_cb'],
>
> From what I can see,
On Tue, Mar 14, 2023 at 12:14:40PM -0700, Jacob Champion wrote:
> On Mon, Mar 13, 2023 at 10:39 PM Michael Paquier wrote:
>> 0002 looks pretty simple as well, I think that's worth a look for this
>> CF.
>
> Cool. v17 just rebases the set over HEAD, then, for cfbot.
I have looked at 0002, and I
On Mon, Mar 13, 2023 at 10:39 PM Michael Paquier wrote:
> 0001 was looking fine enough seen from here, so applied it after
> tweaking a few comments. That's enough to cover most of the needs of
> this thread.
Thank you very much!
> 0002 looks pretty simple as well, I think that's worth a look
On Mon, Mar 13, 2023 at 12:38:10PM -0700, Jacob Champion wrote:
> Here's a v16:
> - updated 0001 patch message
> - all test names should have commas rather than colons now
> - new test for an empty require_auth
> - new SSPI suite (note that it doesn't run by default on Cirrus, due
> to the use of
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 for an empty require_auth
-
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
On Fri, Mar 10, 2023 at 02:32:17PM -0800, Jacob Champion wrote:
> On 3/8/23 22:35, Michael Paquier wrote:
> Works for me. I wonder if
>
>sizeof(((PGconn*) 0)->allowed_auth_methods)
>
> would make pgindent any happier? That'd let you keep the assertion local
> to auth_method_allowed, but it
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
On Mon, Mar 06, 2023 at 04:02:25PM -0800, Jacob Champion wrote:
> 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,
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
On Tue, Feb 28, 2023 at 03:38:21PM -0800, Jacob Champion wrote:
> 0001 and 0002 are the core features. 0003 is a more future-looking
> refactoring of the internals, to make it easier to handle more SASL
> mechanisms, but it's not required and contains some unexercised code.
I was refreshing my
On 2/16/23 10:57, Jacob Champion wrote:
> v14 rebases over the test and solution conflicts from 9244c11afe2.
Since we're to the final CF for PG16, here's a rough summary.
This patchset provides two related features: 1) the ability for a client
to explicitly allow or deny particular methods of
v14 rebases over the test and solution conflicts from 9244c11afe2.
Thanks,
--Jacob
1: 542d330310 ! 1: eec891c519 libpq: let client reject unexpected auth methods
@@ src/test/ssl/t/002_scram.pl: $node->connect_ok(
+qr/channel binding is required, but server did not offer an
On Tue, Jan 31, 2023 at 02:03:54PM +0300, Aleksander Alekseev wrote:
>> What is the status of this patch set? Michael had registered himself as
>> committer and then removed himself again. So I hadn't been paying much
>> attention myself. Was there anything left to discuss?
Yes, sorry about
On Tue, Jan 31, 2023 at 5:20 AM Aleksander Alekseev
wrote:
> To my knowledge there are no open questions left. I think the
> patch is as good as it will ever get.
A committer will need to decide whether they're willing to maintain
0003 or not, as mentioned with the v11 post. Which I suppose is
Hi Peter,
> > What is the status of this patch set? Michael had registered himself as
> > committer and then removed himself again. So I hadn't been paying much
> > attention myself. Was there anything left to discuss?
>
> Previously I marked the patch as RfC. Although it's been a few months
>
Hi Peter,
> > Updated in v13, thanks!
>
> What is the status of this patch set? Michael had registered himself as
> committer and then removed himself again. So I hadn't been paying much
> attention myself. Was there anything left to discuss?
Previously I marked the patch as RfC. Although
On 16.11.22 18:26, Jacob Champion wrote:
On Tue, Nov 15, 2022 at 11:07 PM Michael Paquier wrote:
I am beginning to look at the last version proposed, which has been
marked as RfC. Does this patch need a refresh in light of a9e9a9f and
0873b2d? The changes for libpq_append_conn_error() should
On Tue, Nov 15, 2022 at 11:07 PM Michael Paquier wrote:
> I am beginning to look at the last version proposed, which has been
> marked as RfC. Does this patch need a refresh in light of a9e9a9f and
> 0873b2d? The changes for libpq_append_conn_error() should be
> straight-forward.
Updated in
On Thu, Oct 20, 2022 at 11:36:34AM -0700, Jacob Champion wrote:
> Maybe I should just add a basic Assert here, to trip if someone adds a
> new SASL mechanism, and point that lucky person to this thread with a
> comment?
I am beginning to look at the last version proposed, which has been
marked as
On 11/11/22 22:57, Aleksander Alekseev wrote:
> I did a little more research and I think you are right. What happens
> according to the C standard:
Thanks for confirming! (I personally prefer -1 to a *MAX macro, because
it works regardless of the length of the type.)
--Jacob
Hi Jacob,
> > Assigning a negative number to uint32 doesn't necessarily work on all
> > platforms. I suggest using PG_UINT32_MAX.
>
> Hmm -- on which platforms is "-1 converted to unsigned" not equivalent
> to the maximum value? Are they C-compliant?
I did a little more research and I think you
On 11/11/22 05:52, Aleksander Alekseev wrote:
> I noticed that this patchset stuck a bit so I decided to take a look.
Thanks!
> Assigning a negative number to uint32 doesn't necessarily work on all
> platforms. I suggest using PG_UINT32_MAX.
Hmm -- on which platforms is "-1 converted to
Hi Jacob,
> v11 makes an attempt at this (see 0003), using the proposed string list.
I noticed that this patchset stuck a bit so I decided to take a look.
In 0001:
```
+conn->auth_required = false;
+conn->allowed_auth_methods = -1;
...
+uint32
On Wed, Oct 12, 2022 at 9:40 AM Jacob Champion wrote:
> On 10/5/22 06:33, Peter Eisentraut wrote:
> > I think it would be good to put some provisions in place here, even if
> > they are elementary. Otherwise, there will be a significant burden on
> > the person who implements the next SASL
On 10/5/22 06:33, Peter Eisentraut wrote:
> I think it would be good to put some provisions in place here, even if
> they are elementary. Otherwise, there will be a significant burden on
> the person who implements the next SASL method (i.e., you ;-) ) to
> figure that out then.
Sounds good,
On 23.09.22 02:02, Jacob Champion wrote:
On Thu, Sep 22, 2022 at 4:52 AM Peter Eisentraut
wrote:
On 22.09.22 01:37, Jacob Champion wrote:
I think this is potentially
dangerous, but it mirrors the current behavior of libpq and I'm not
sure that we should change it as part of this patch.
It
On Thu, Sep 22, 2022 at 4:52 AM Peter Eisentraut
wrote:
> On 22.09.22 01:37, Jacob Champion wrote:
> > I think this is potentially
> > dangerous, but it mirrors the current behavior of libpq and I'm not
> > sure that we should change it as part of this patch.
>
> It might be worth reviewing that
On 22.09.22 01:37, Jacob Champion wrote:
On Wed, Sep 21, 2022 at 3:36 PM Peter Eisentraut
wrote:
So let's look at the two TODO comments you have:
* TODO: how should !auth_required interact with an incomplete
* SCRAM exchange?
What specific combination of events are you
On Wed, Sep 21, 2022 at 3:36 PM Peter Eisentraut
wrote:
> So let's look at the two TODO comments you have:
>
> * TODO: how should !auth_required interact with an incomplete
> * SCRAM exchange?
>
> What specific combination of events are you thinking of here?
Let's say the
On 21.09.22 17:33, Jacob Champion wrote:
On Fri, Sep 16, 2022 at 1:29 PM Jacob Champion wrote:
I'm happy to implement proofs of concept for that, or any other ideas,
given the importance of getting this "right enough" the first time.
Just let me know.
v8 rebases over the postgres_fdw HINT
On Fri, Sep 16, 2022 at 1:29 PM Jacob Champion wrote:
> I'm happy to implement proofs of concept for that, or any other ideas,
> given the importance of getting this "right enough" the first time.
> Just let me know.
v8 rebases over the postgres_fdw HINT changes; there are no functional
On Fri, Sep 16, 2022 at 7:56 AM Peter Eisentraut
wrote:
> On 08.09.22 20:18, Jacob Champion wrote:
> After thinking about this a bit more, I think it would be best if the
> words used here match exactly with what is used in pg_hba.conf. That's
> the only thing the user cares about: reject
On 08.09.22 20:18, Jacob Champion wrote:
Sounds fair. "cleartext"? "plaintext"? "plain" (like SASL's PLAIN)?
On the SASL front: In the back of my head I'd been considering adding
a "sasl:" prefix to "scram-sha-256", so that we have a namespace for
new SASL methods. That would also give us a
On Thu, Sep 8, 2022 at 6:25 AM Peter Eisentraut
wrote:
> For example, before long someone is going to try putting "ldap" into
> require_auth. The fact that the methods in pg_hba.conf are not what
> libpq sees is not something that was really exposed to users until now.
> "none" vs. "trust" takes
I'm wondering about making the list of things you can specify in
require_auth less confusing and future proof.
For example, before long someone is going to try putting "ldap" into
require_auth. The fact that the methods in pg_hba.conf are not what
libpq sees is not something that was really
On Thu, Jun 30, 2022 at 04:26:54PM -0700, Jacob Champion wrote:
> Yeah, maybe it'd be better to tell the user the correct context for an
> otherwise-valid option ("the 'sslcert' option may only be applied to
> USER MAPPING"), and avoid the option dump entirely?
Yes, that would be nice. Now, this
On Wed, Jun 29, 2022 at 6:36 AM Peter Eisentraut
wrote:
> It's not strictly related to your patch, but maybe this hint has
> outlived its usefulness? I mean, we don't list all available tables
> when you try to reference a table that doesn't exist. And unordered on
> top of that.
Yeah, maybe
On 27.06.22 23:40, Jacob Champion wrote:
-HINT: Valid options in this context are: service, passfile, channel_binding,
connect_timeout, dbname, host, hostaddr, port, options, application_name,
keepalives, keepalives_idle, keepalives_interval, keepalives_count,
tcp_user_timeout, sslmode,
On Mon, Jun 27, 2022 at 12:05 PM Jacob Champion wrote:
> v5 adds a second patch which implements a client-certificate analogue
> to gssencmode; I've named it sslcertmode.
...and v6 fixes check-world, because I always forget about postgres_fdw.
--Jacob
diff --git
On Fri, Jun 24, 2022 at 12:17 PM Jacob Champion wrote:
> Both NOT (via ! negation) and "none" are implemented in v4.
v5 adds a second patch which implements a client-certificate analogue
to gssencmode; I've named it sslcertmode. This takes the place of the
require_auth=[!]cert setting
On Thu, Jun 23, 2022 at 10:33 AM Jacob Champion wrote:
> - I think NOT is a important case in practice, which is effectively a
> negative OR ("anything but this/these")
Both NOT (via ! negation) and "none" are implemented in v4.
Examples:
# The server must use SCRAM.
require_auth=scram-sha-256
On Wed, Jun 22, 2022 at 5:56 PM David G. Johnston
wrote:
> That just makes me want to not implement OR'ing...
>
> The existing set of individual parameters doesn't work as an API for
> try-and-fallback.
>
> Something like would be less problematic when it comes to setting multiple
> related
On Thu, Jun 9, 2022 at 4:30 PM Jacob Champion
wrote:
> On Wed, Jun 8, 2022 at 9:58 PM Michael Paquier
> wrote:
>
> > One
> > interesting case comes down to stuff like channel_binding=require
> > require_auth="md5,scram-sha-256", where I think that we should still
> > fail even if the server
On Mon, Jun 13, 2022 at 10:00 PM Michael Paquier wrote:
> > Personally I think the ability to provide a default of `!password` is
> > very compelling. Any allowlist we hardcode won't be future-proof (see
> > also my response to Laurenz upthread [1]).
>
> Hm, perhaps. You could use that as a
On Thu, Jun 09, 2022 at 04:29:38PM -0700, Jacob Champion wrote:
> On Wed, Jun 8, 2022 at 9:58 PM Michael Paquier wrote:
>> Er, this one could be a problem protocol-wise for SASL, because that
>> would mean that the authentication gets completed but that the
>> exchange has begun and is not
On Wed, Jun 8, 2022 at 9:58 PM Michael Paquier wrote:
> > - require_auth=scram-sha-256,cert means that either a SCRAM handshake
> > must be completed, or the server must request a client certificate. It
> > has a potential pitfall in that it allows a partial SCRAM handshake,
> > as long as a
On Tue, Jun 07, 2022 at 02:22:28PM -0700, Jacob Champion wrote:
> v2 rebases over latest, removes the alternate spellings of "password",
> and implements OR operations with a comma-separated list. For example:
>
> - require_auth=cert means that the server must ask for, and the client
> must
v2 rebases over latest, removes the alternate spellings of "password",
and implements OR operations with a comma-separated list. For example:
- require_auth=cert means that the server must ask for, and the client
must provide, a client certificate.
- require_auth=password,md5 means that the
On Wed, Jun 1, 2022 at 12:55 AM Michael Paquier wrote:
>
> Jacob, do you still have plans to work on this patch?
Yes, definitely. That said, the more the merrier if there are others
interested in taking a shot at it. There are a large number of
alternative implementation proposals.
Thanks,
On Sat, Mar 05, 2022 at 01:04:05AM +, Jacob Champion wrote:
> the connection string, and libpq will fail the connection if the server
> doesn't use that method.
>
> (This is not intended for PG15. I'm generally anxious about posting
> experimental work during a commitfest, but there's been
On Wed, 2022-03-23 at 21:31 +, Jacob Champion wrote:
> On Mon, 2022-03-07 at 11:44 +0100, Laurenz Albe wrote:
> > I am all for the idea, but you implemented the reverse of proposal 2.
> >
> > Wouldn't it be better to list the *rejected* authentication methods?
> > Then we could have "password"
On Mon, 2022-03-07 at 11:44 +0100, Laurenz Albe wrote:
> I am all for the idea, but you implemented the reverse of proposal 2.
(This email was caught in my spam filter; sorry for the delay.)
> Wouldn't it be better to list the *rejected* authentication methods?
> Then we could have "password" on
On Sat, 2022-03-05 at 01:04 +, Jacob Champion wrote:
> TL;DR: this patch lets you specify exactly one authentication method in
> the connection string, and libpq will fail the connection if the server
> doesn't use that method.
>
> (This is not intended for PG15. I'm generally anxious about
Andrew Dunstan writes:
> On 3/4/22 20:19, Tom Lane wrote:
>> Seems reasonable, but I bet that for very little more code you could
>> accept a comma-separated list of allowed methods; libpq already allows
>> comma-separated lists for some other connection options. That seems
>> like it'd be a
On 3/4/22 20:19, Tom Lane wrote:
> Jacob Champion writes:
>> $subject keeps coming up in threads. I think my first introduction to
>> it was after the TLS injection CVE, and then it came up again in the
>> pluggable auth thread. It's hard for me to generalize based on "sound
>> bites", but
On Fri, Mar 04, 2022 at 08:19:26PM -0500, Tom Lane wrote:
> Jacob Champion writes:
>> Here is my take on option 2, then: you get to choose exactly one method
>> that the client will accept. If you want to use client certificates,
>> use require_auth=cert. If you want to force SCRAM, use
>>
Jacob Champion writes:
> $subject keeps coming up in threads. I think my first introduction to
> it was after the TLS injection CVE, and then it came up again in the
> pluggable auth thread. It's hard for me to generalize based on "sound
> bites", but among the proposals I've seen are
> 1.
Hello,
TL;DR: this patch lets you specify exactly one authentication method in
the connection string, and libpq will fail the connection if the server
doesn't use that method.
(This is not intended for PG15. I'm generally anxious about posting
experimental work during a commitfest, but there's
62 matches
Mail list logo
