On Wed, Mar 19, 2025 at 4:21 PM Jacob Champion
wrote:
>
> On Tue, Mar 18, 2025 at 12:32 PM Peter Eisentraut
> wrote:
> > Yeah, I think option (2) is enough for now. If someone wants to enable
> > the kinds of things you describe, they can always come back and
> > implement option (1) later.
>
>
On Fri, Mar 21, 2025 at 1:28 PM Jacob Champion
wrote:
>
> Great, thank you! Looking over v10, I think I've run out of feedback
> at this point. Marked Ready for Committer.
Thanks for all the effort reviewing this patch!
--
Matheus Alcantara
On 24.03.25 21:33, Matheus Alcantara wrote:
I'm a bit confused about the refactoring patch 0001. There are some
details there that don't seem right. For example, you write that the
pfree(rconn) calls are no longer necessary, but AFAICT, it would still
be needed in dblink_get_conn(). Also, ther
On Wed, Mar 26, 2025 at 7:41 AM Peter Eisentraut wrote:
>
> On 24.03.25 21:33, Matheus Alcantara wrote:
> >> I'm a bit confused about the refactoring patch 0001. There are some
> >> details there that don't seem right. For example, you write that the
> >> pfree(rconn) calls are no longer necessa
On Mon, Mar 24, 2025 at 1:16 PM Peter Eisentraut wrote:
>
> On 21.03.25 19:24, Matheus Alcantara wrote:
> > On Fri, Mar 21, 2025 at 1:28 PM Jacob Champion
> > wrote:
> >>
> >> Great, thank you! Looking over v10, I think I've run out of feedback
> >> at this point. Marked Ready for Committer.
> >
On 21.03.25 19:24, Matheus Alcantara wrote:
On Fri, Mar 21, 2025 at 1:28 PM Jacob Champion
wrote:
Great, thank you! Looking over v10, I think I've run out of feedback
at this point. Marked Ready for Committer.
Thanks for all the effort reviewing this patch!
I have committed the 0003 patch
Great, thank you! Looking over v10, I think I've run out of feedback
at this point. Marked Ready for Committer.
--Jacob
On Thu, Mar 20, 2025 at 9:02 PM Jacob Champion
wrote:
>
> On Thu, Mar 20, 2025 at 12:54 PM Matheus Alcantara
> wrote:
> > Since the security checks are defined I'm attaching 0003 which include
> > the fix of security checks for postgres_fdw. It implements the
> > validations very similar to what
On Thu, Mar 20, 2025 at 12:54 PM Matheus Alcantara
wrote:
> Since the security checks are defined I'm attaching 0003 which include
> the fix of security checks for postgres_fdw. It implements the
> validations very similar to what are being implemented on dblink.
Comments on 0003:
> +
On Tue, Mar 18, 2025 at 12:32 PM Peter Eisentraut wrote:
> Yeah, I think option (2) is enough for now. If someone wants to enable
> the kinds of things you describe, they can always come back and
> implement option (1) later.
Sounds good to me.
--
Notes on v8:
- The following documentation pi
On 18.03.25 17:53, Jacob Champion wrote:
On Tue, Mar 18, 2025 at 9:35 AM Peter Eisentraut wrote:
So the way I understand this is that the options are:
(1) We add a libpq function like PQconnectionUsedScramKeys() in the
style of PQconnectionUsedPassword() and call that function during the
check
On Tue, Mar 18, 2025 at 9:35 AM Peter Eisentraut wrote:
> So the way I understand this is that the options are:
>
> (1) We add a libpq function like PQconnectionUsedScramKeys() in the
> style of PQconnectionUsedPassword() and call that function during the
> checks.
>
> (2) We make use_scram_passth
On 17.03.25 17:49, Jacob Champion wrote:
If we implement this, it needs to check that the keys were actually
sent during scram_exchange(). Having them set on the PGconn doesn't
mean that we used them for authentication.
We use the client key and server key on calculate_client_proof and
verify_s
On Mon, Mar 17, 2025 at 8:26 PM Jacob Champion
wrote:
> > > I was referring to the discussion upthread [1]; you'd mentioned that
> > > the only reason that get_connect_string() didn't call GetUserMapping()
> > > itself was because we needed that mapping later on for
> > > UseScramPassthrough(). Bu
On Mon, Mar 17, 2025 at 11:54 AM Matheus Alcantara
wrote:
> > If the check functions aren't going to check those because it's
> > unnecessary, then that's fine, but then the comments should be
> > adjusted.
> >
> Ok, now I get all of your points. I've misinterpreted your comments,
> sorry about th
On Mon, Mar 17, 2025 at 1:49 PM Jacob Champion
wrote:
>
> On Thu, Mar 13, 2025 at 2:38 PM Matheus Alcantara
> wrote:
> > I thought about this; The problem is that at this point, the scram keys
> > on connection options are base64 encoded (see appendSCRAMKeysInfo), so
> > we can't compare with the
On Thu, Mar 13, 2025 at 2:38 PM Matheus Alcantara
wrote:
> I thought about this; The problem is that at this point, the scram keys
> on connection options are base64 encoded (see appendSCRAMKeysInfo), so
> we can't compare with the values stored on MyProcPort.
I don't think that's necessary -- th
On Thu, Mar 13, 2025 at 4:54 PM Jacob Champion
wrote:
>
> On Thu, Mar 13, 2025 at 6:59 AM Matheus Alcantara
> wrote:
> > Fixed by wrapping on PG_CATCH/pfree/PG_RE_THROW. I didn't managed to
> > create a test that use this code path, so let me know if I'm still
> > missing something.
>
> Thanks! L
On Thu, Mar 13, 2025 at 6:59 AM Matheus Alcantara
wrote:
> Fixed by wrapping on PG_CATCH/pfree/PG_RE_THROW. I didn't managed to
> create a test that use this code path, so let me know if I'm still
> missing something.
Thanks! Looks like the regression suite has one test that takes that
path (foun
Hi, thanks for all the comments! Attached v5 with some fixes
(I'll answer comments for different emails on this since most of them is
fixed on this new v5 version)
> I think this fix may break the other usage in dblink_get_conn(), where
> rconn comes from a hash entry. Maybe dblink_connect() shou
On Mon, Mar 10, 2025 at 11:25 AM Jacob Champion
wrote:
>
> On Fri, Mar 7, 2025 at 8:22 AM Peter Eisentraut wrote:
> > Right. How about the attached? It checks as an alternative to a
> > password whether the SCRAM keys were provided. That should get us back
> > to the same level of checking?
>
On Fri, Mar 7, 2025 at 8:22 AM Peter Eisentraut wrote:
> Right. How about the attached? It checks as an alternative to a
> password whether the SCRAM keys were provided. That should get us back
> to the same level of checking?
Yes, I think so. Attached is a set of tests to illustrate, mirrorin
On 06.03.25 22:58, Jacob Champion wrote:
On Thu, Mar 6, 2025 at 12:33 PM Peter Eisentraut wrote:
AFAICT, in pgfdw_security_check(), if SCRAM has been used for the
outgoing server connection, then PQconnectionUsedPassword() is true, and
then this check should fail if no "password" parameter was
On Thu, Mar 6, 2025 at 12:33 PM Peter Eisentraut wrote:
> AFAICT, in pgfdw_security_check(), if SCRAM has been used for the
> outgoing server connection, then PQconnectionUsedPassword() is true, and
> then this check should fail if no "password" parameter was given. That
> check should be expande
On 11.02.25 00:19, Jacob Champion wrote:
These don't seem right to me. SCRAM passthrough should be considered
as_part_ of the connstr/security checks, but I think it should not
_bypass_ those checks. We have to enforce the use of the SCRAM
credentials on the remote for safety, similarly to GSS de
On Mon, Mar 3, 2025 at 9:01 AM Jacob Champion
wrote:
> I keep getting pulled away from my review of 0002
Here's a review of v3-0002:
> +dblink_connstr_check(const char *connstr, bool useScramPassthrough)
> {
> + if (useScramPassthrough)
> + {
> + if (dblink_connstr_has_scram_require_a
On Fri, Feb 21, 2025 at 6:48 AM Matheus Alcantara
wrote:
> Hi, thanks for all the reviews. Attached v3 with some fixes.
Thanks! I keep getting pulled away from my review of 0002, so I'll
just comment on 0001 to get things moving again; sorry for the delay.
> I agree, I've just declared outside o
Hi, thanks for all the reviews. Attached v3 with some fixes.
> They also check for GSS delegation. I think SCRAM passthrough should
> ideally be considered a second form of credentials delegation, from
> the perspective of those functions.
>
Changed dblink_connstr_check and dblink_security_check t
On Wed, Feb 19, 2025 at 12:02 PM Matheus Alcantara
wrote:
> > Hardcoding to scram-sha-256 would also prohibit the use of GSS or
> > standard password auth, now that I think about it. The docs currently
> > have a note about being able to choose... Should we add the other
> > permitted authenticati
On Thu, Feb 13, 2025 at 8:25 PM Jacob Champion
wrote:
> > I also agree that we should enforce the use of the SCRAM on the remote for
> > safety. To do this I think that we could set require_auth=scram-sha-256 on
> > connection options if SCRAM pass-through is being used, with this we will
> > get
On Wed, Feb 12, 2025 at 11:54 AM Matheus Alcantara
wrote:
> Currently dblink_connstr_check and dblink_security_check only check if the
> password is present on connection options, in case of not superuser.
They also check for GSS delegation. I think SCRAM passthrough should
ideally be considered
Hi, thanks for reviewing this patch!
Em seg., 10 de fev. de 2025 às 20:19, Jacob Champion
escreveu:
>
> On Wed, Jan 22, 2025 at 6:10 AM Matheus Alcantara
> wrote:
> > The attached patch enables SCRAM authentication for dblink connections when
> > using dblink_fdw without requiring a plain-text p
On Wed, Jan 22, 2025 at 6:10 AM Matheus Alcantara
wrote:
> The attached patch enables SCRAM authentication for dblink connections when
> using dblink_fdw without requiring a plain-text password on user mapping
> properties. The implementation is very similar to what was implemented on
> postgres_f
33 matches
Mail list logo
