Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andrey Chudnovsky (achudnovs...@gmail.com) wrote: > The thread link is here: > https://www.postgresql.org/message-id/flat/CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw%40mail.gmail.com#f94c36969a68a07c087fa9af0f5401e1 Thanks for pointing out the updates on that thread, I've responded over there with my thoughts. Thanks again, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, Want to resurface the OAUTH support topic in the context of the concerns raised here. > How about- if we just added OAUTH support directly into libpq and the > backend, would that work with Azure's OIDC provider? If not, why not? > If it does, then what's the justification for trying to allow custom > backend-only authentication methods? We've explored this option, and prepared a patch which has bare-bone OAUTHBEARER mechanism implementation on both server- and libpq- sides. Based on existing SASL exchange support used for SCRAM. Most of the work has been done by @Jacob Champion and was referenced in this thread before. We validated the approach would work with Azure AD and other OIDC providers with token validation logic delegated to provider-specific extension. The thread link is here: https://www.postgresql.org/message-id/flat/CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw%40mail.gmail.com#f94c36969a68a07c087fa9af0f5401e1 With the client- and server- side support, the proposed patch would allow: - Identity providers to publish PG extensions to validate their specific token format. - Client ecosystem to build generic OAUTH flows using OIDC provider metadata defined in RFCs - Cloud providers to support a combination of Identity providers they work with. Looking forward to bring the discussion to that thread, and decide if we can proceed with the OAUTH support. Thanks, Andrey. On Wed, Jan 25, 2023 at 10:55 AM Stephen Frost wrote: > > Greetings, > > * Andres Freund (and...@anarazel.de) wrote: > > On 2022-08-03 17:21:58 -0400, Stephen Frost wrote: > > > * Andres Freund (and...@anarazel.de) wrote: > > > > On 2022-08-03 16:28:08 -0400, Stephen Frost wrote: > > > > > Again, server-side only is not interesting and not a direction that > > > > > makes sense to go in because it doesn't provide any way to have > > > > > trust established in both directions, which is what all modern > > > > > authentication methods do (certificates, kerberos, scram) and is what > > > > > we > > > > > should expect from anything new in this space. > > > > > > > > As explained repeatedly before, that's plainly not true. The patch > > > > allows > > > > e.g. to use the exact scram flow we already have, with the code we have > > > > for > > > > that (e.g. using a different source of secret). In fact the example > > > > extension > > > > does so (starting in v3, from 2022-03-15): > > > > > > Sure, thanks to the bespoke code in libpq for supporting SCRAM. I don't > > > think it makes sense to move the server-side code for that into an > > > extension as it just makes work for a bunch of people. Having a way to > > > have the authenticator token for scram exist elsewhere doesn't strike me > > > as unreasonable but that's not the same thing and is certainly more > > > constrained in terms of what it's doing. > > > > The question is: Why is providing that ability not a good step, even if > > somewhat constrainted? One argument would be that a later "protocol level > > extensibility" effort would require significant changes to the API - but I > > don't think that'd be the case. And even if, this isn't a large > > extensibility > > surface, so API evolution wouldn't be a problem. > > I'm quite confused by this as I feel like I've said multiple times that > having a way to have the SCRAM authenticator token come from somewhere > else seems reasonable. If that's not what you're referring to above by > 'that ability' then it'd really help if you could clarify what you mean > there. > > > > Further, I outlined exactly how extensability in this area could be > > > achieved: use some good third party library that provides multiple SASL > > > methods then just add support for that library to *PG*, on both the > > > libpq and the server sides. > > > > > What I don't think makes sense is adding extensibility to the server > > > side, especially if it's through a 3rd party library, but then not > > > adding support for that to libpq. I don't follow what the rationale is > > > for explicitly excluding libpq from this discussion. > > > > The rationale is trivial: Breaking down projects into manageable, separately > > useful, steps is a very sensible way of doing development. Even if we get > > protocol extensibility, we're still going to need an extension API like it's > > provided by the patchset. After protocol extensibility the authentication > > extensions would then have more functions it could call to control the auth > > flow with the client. > > > > > To be clear- I'm not explicitly saying that we can only add > > > extensibility with SCRAM, I'm just saying that whatever we're doing here > > > to add other actual authentication methods we should be ensuring is done > > > on both sides with a way for each side to authenticate the other side, > > > as all of the modern authentication methods we have already do. > > > > But *why* do these need to be tied together? > > I'm also confused by this. Surely you'd need
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-08-03 17:21:58 -0400, Stephen Frost wrote: > > * Andres Freund (and...@anarazel.de) wrote: > > > On 2022-08-03 16:28:08 -0400, Stephen Frost wrote: > > > > Again, server-side only is not interesting and not a direction that > > > > makes sense to go in because it doesn't provide any way to have > > > > trust established in both directions, which is what all modern > > > > authentication methods do (certificates, kerberos, scram) and is what we > > > > should expect from anything new in this space. > > > > > > As explained repeatedly before, that's plainly not true. The patch allows > > > e.g. to use the exact scram flow we already have, with the code we have > > > for > > > that (e.g. using a different source of secret). In fact the example > > > extension > > > does so (starting in v3, from 2022-03-15): > > > > Sure, thanks to the bespoke code in libpq for supporting SCRAM. I don't > > think it makes sense to move the server-side code for that into an > > extension as it just makes work for a bunch of people. Having a way to > > have the authenticator token for scram exist elsewhere doesn't strike me > > as unreasonable but that's not the same thing and is certainly more > > constrained in terms of what it's doing. > > The question is: Why is providing that ability not a good step, even if > somewhat constrainted? One argument would be that a later "protocol level > extensibility" effort would require significant changes to the API - but I > don't think that'd be the case. And even if, this isn't a large extensibility > surface, so API evolution wouldn't be a problem. I'm quite confused by this as I feel like I've said multiple times that having a way to have the SCRAM authenticator token come from somewhere else seems reasonable. If that's not what you're referring to above by 'that ability' then it'd really help if you could clarify what you mean there. > > Further, I outlined exactly how extensability in this area could be > > achieved: use some good third party library that provides multiple SASL > > methods then just add support for that library to *PG*, on both the > > libpq and the server sides. > > > What I don't think makes sense is adding extensibility to the server > > side, especially if it's through a 3rd party library, but then not > > adding support for that to libpq. I don't follow what the rationale is > > for explicitly excluding libpq from this discussion. > > The rationale is trivial: Breaking down projects into manageable, separately > useful, steps is a very sensible way of doing development. Even if we get > protocol extensibility, we're still going to need an extension API like it's > provided by the patchset. After protocol extensibility the authentication > extensions would then have more functions it could call to control the auth > flow with the client. > > > To be clear- I'm not explicitly saying that we can only add > > extensibility with SCRAM, I'm just saying that whatever we're doing here > > to add other actual authentication methods we should be ensuring is done > > on both sides with a way for each side to authenticate the other side, > > as all of the modern authentication methods we have already do. > > But *why* do these need to be tied together? I'm also confused by this. Surely you'd need a way to *test* such a new authentication library being added to the core code and that naturally requires having it be implemented on both sides, so I don't see why you wouldn't just do that from the get go. I don't see how it makes sense to talk about such a project as if it could possibly be server-side only. The example being discussed- adding SASL support using a third party library such as libsasl2, would certainly be something that you'd add on both sides and then test if you got it right and make sure that it worked. Recent examples also bear this out- I can't imagine anyone would have gotten behind the idea of adding GSSAPI encryption to just the server side, even if you could somehow argue that it would have been a separately useful step because maybe someone out there happened to have implemented it in their own client. The same is true of Kerberos credential delegation- yeah, we could implement that just on the server, but if libpq isn't doing it then we couldn't test it as part of the core project and it's certainly not some huge amount of additional work to implement it in libpq too, where it then immediately becomes available to a large number of downstream interfaces like psycopg2 and various others. The few non-libpq-based interfaces typically follow what we do in core on the client side anyway, such as the JDBC project. If the idea is that we should just add hooks to allow someone to reimplement SCRAM but external to the core server and only on the server side then what's going to be on the client side to interface with that? How would one add support for whatever that new
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-08-03 17:21:58 -0400, Stephen Frost wrote: > * Andres Freund (and...@anarazel.de) wrote: > > On 2022-08-03 16:28:08 -0400, Stephen Frost wrote: > > > Again, server-side only is not interesting and not a direction that > > > makes sense to go in because it doesn't provide any way to have > > > trust established in both directions, which is what all modern > > > authentication methods do (certificates, kerberos, scram) and is what we > > > should expect from anything new in this space. > > > > As explained repeatedly before, that's plainly not true. The patch allows > > e.g. to use the exact scram flow we already have, with the code we have for > > that (e.g. using a different source of secret). In fact the example > > extension > > does so (starting in v3, from 2022-03-15): > > Sure, thanks to the bespoke code in libpq for supporting SCRAM. I don't > think it makes sense to move the server-side code for that into an > extension as it just makes work for a bunch of people. Having a way to > have the authenticator token for scram exist elsewhere doesn't strike me > as unreasonable but that's not the same thing and is certainly more > constrained in terms of what it's doing. The question is: Why is providing that ability not a good step, even if somewhat constrainted? One argument would be that a later "protocol level extensibility" effort would require significant changes to the API - but I don't think that'd be the case. And even if, this isn't a large extensibility surface, so API evolution wouldn't be a problem. > Further, I outlined exactly how extensability in this area could be > achieved: use some good third party library that provides multiple SASL > methods then just add support for that library to *PG*, on both the > libpq and the server sides. > What I don't think makes sense is adding extensibility to the server > side, especially if it's through a 3rd party library, but then not > adding support for that to libpq. I don't follow what the rationale is > for explicitly excluding libpq from this discussion. The rationale is trivial: Breaking down projects into manageable, separately useful, steps is a very sensible way of doing development. Even if we get protocol extensibility, we're still going to need an extension API like it's provided by the patchset. After protocol extensibility the authentication extensions would then have more functions it could call to control the auth flow with the client. > To be clear- I'm not explicitly saying that we can only add > extensibility with SCRAM, I'm just saying that whatever we're doing here > to add other actual authentication methods we should be ensuring is done > on both sides with a way for each side to authenticate the other side, > as all of the modern authentication methods we have already do. But *why* do these need to be tied together? > > > If anything, the other auth methods should be ripped out entirely > > > (password, > > > md5, ldap, etc), but certainly not used as a basis for new work or a place > > > to try and add new features, as they're all well known to have serious > > > vulnerabilities. > > > > I don't think we'd help users if we ripped out all those methods without a > > replacement, but relegating them to contrib/ and thus requiring that they > > explicitly get configured in the server seems like a decent step. But imo > > that's a separate discussion. > > We have a replacement for password and md5 and it's SCRAM. For my 2c, I > don't see the value in continuing to have those in any form at this > point. I concede that I may not get consensus on that but I don't > really see how moving them to contrib would actually be helpful. I'm much more on board with ripping out password and md5 than ldap, radius, pam et al. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-08-03 16:28:08 -0400, Stephen Frost wrote: > > Again, server-side only is not interesting and not a direction that > > makes sense to go in because it doesn't provide any way to have > > trust established in both directions, which is what all modern > > authentication methods do (certificates, kerberos, scram) and is what we > > should expect from anything new in this space. > > As explained repeatedly before, that's plainly not true. The patch allows > e.g. to use the exact scram flow we already have, with the code we have for > that (e.g. using a different source of secret). In fact the example extension > does so (starting in v3, from 2022-03-15): Sure, thanks to the bespoke code in libpq for supporting SCRAM. I don't think it makes sense to move the server-side code for that into an extension as it just makes work for a bunch of people. Having a way to have the authenticator token for scram exist elsewhere doesn't strike me as unreasonable but that's not the same thing and is certainly more constrained in terms of what it's doing. > If you're ideologically opposed to allowing extensibility in this specific > area: Say so, instead of repeating this bogus argument over and over. Considering how much I pushed for and supported the effort to include SCRAM, I hardly would say that I'm against making improvements in this area. Further, I outlined exactly how extensability in this area could be achieved: use some good third party library that provides multiple SASL methods then just add support for that library to *PG*, on both the libpq and the server sides. What I don't think makes sense is adding extensibility to the server side, especially if it's through a 3rd party library, but then not adding support for that to libpq. I don't follow what the rationale is for explicitly excluding libpq from this discussion. To be clear- I'm not explicitly saying that we can only add extensibility with SCRAM, I'm just saying that whatever we're doing here to add other actual authentication methods we should be ensuring is done on both sides with a way for each side to authenticate the other side, as all of the modern authentication methods we have already do. If we got SASL added and it included support for SCRAM and Kerberos through that and was a common enough library that we felt comfortable ripping out our own implementations in favor of what that library provides, sure, that'd be something to consider too (though I tend to doubt we'd get so lucky as to have one that worked with the existing SASL/SCRAM stuff we have today since we had to do some odd things there with the username, as I recall). > > If anything, the other auth methods should be ripped out entirely (password, > > md5, ldap, etc), but certainly not used as a basis for new work or a place > > to try and add new features, as they're all well known to have serious > > vulnerabilities. > > I don't think we'd help users if we ripped out all those methods without a > replacement, but relegating them to contrib/ and thus requiring that they > explicitly get configured in the server seems like a decent step. But imo > that's a separate discussion. We have a replacement for password and md5 and it's SCRAM. For my 2c, I don't see the value in continuing to have those in any form at this point. I concede that I may not get consensus on that but I don't really see how moving them to contrib would actually be helpful. > > I also don't agree that this makes sense as an extension as we don't > > have any way for extensions to make changes in libpq or psql, again > > leading to the issue that it either can't be exercised or we create some > > dependency on an external SASL library for libpq but object to having > > that same dependency on the server side, which doesn't seem sensible to > > me. Requiring admins to jump through hoops to install an extension > > where we have such a dependency on a library anyway doesn't make sense > > either. > > This argument doesn't make a whole lot of sense to me - as explained above you > can use the existing scram flow for plenty usecases. I'd be a bit more > convinced if you'd argue that the extension API should just allow providing a > different source of secrets for the existing scram flow (I'd argue that that's > not the best point for extensibility, but that'd be more a question of taste). As I think I said before, I don't particularly object to the idea of having an alternative backing store for pg_authid (though note that I'm actively working on extending that further to allow multiple authenticators to be able to be configured for a role, so whatever that backing store is would need to be adjusted if that ends up getting committed into core). That's quite a different thing from my perspective. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-08-03 16:28:08 -0400, Stephen Frost wrote: > Again, server-side only is not interesting and not a direction that > makes sense to go in because it doesn't provide any way to have > trust established in both directions, which is what all modern > authentication methods do (certificates, kerberos, scram) and is what we > should expect from anything new in this space. As explained repeatedly before, that's plainly not true. The patch allows e.g. to use the exact scram flow we already have, with the code we have for that (e.g. using a different source of secret). In fact the example extension does so (starting in v3, from 2022-03-15): Check 0002 from https://www.postgresql.org/message-id/CAJxrbyxgFzfqby%2BVRCkeAhJnwVZE50%2BZLPx0JT2TDg9LbZtkCg%40mail.gmail.com If you're ideologically opposed to allowing extensibility in this specific area: Say so, instead of repeating this bogus argument over and over. > If anything, the other auth methods should be ripped out entirely (password, > md5, ldap, etc), but certainly not used as a basis for new work or a place > to try and add new features, as they're all well known to have serious > vulnerabilities. I don't think we'd help users if we ripped out all those methods without a replacement, but relegating them to contrib/ and thus requiring that they explicitly get configured in the server seems like a decent step. But imo that's a separate discussion. > I also don't agree that this makes sense as an extension as we don't > have any way for extensions to make changes in libpq or psql, again > leading to the issue that it either can't be exercised or we create some > dependency on an external SASL library for libpq but object to having > that same dependency on the server side, which doesn't seem sensible to > me. Requiring admins to jump through hoops to install an extension > where we have such a dependency on a library anyway doesn't make sense > either. This argument doesn't make a whole lot of sense to me - as explained above you can use the existing scram flow for plenty usecases. I'd be a bit more convinced if you'd argue that the extension API should just allow providing a different source of secrets for the existing scram flow (I'd argue that that's not the best point for extensibility, but that'd be more a question of taste). Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, * samay sharma (smilingsa...@gmail.com) wrote: > On Tue, Aug 2, 2022 at 2:48 PM Jacob Champion > wrote: > > [dev hat] That said, I plan to do some additional dev work on top of > > this over the next couple of months. The ideal case would be to provide > > a server-only extension that provides additional features to existing > > clients in the wild (i.e. no client-side changes). > > > > I'm also interested in plugging an existing 3rd-party SASL library into > > the client, which would help extensibility on that side. > > That would be interesting. Again, server-side only is not interesting and not a direction that makes sense to go in because it doesn't provide any way to have trust established in both directions, which is what all modern authentication methods do (certificates, kerberos, scram) and is what we should expect from anything new in this space. If anything, the other auth methods should be ripped out entirely (password, md5, ldap, etc), but certainly not used as a basis for new work or a place to try and add new features, as they're all well known to have serious vulnerabilities. As it specifically relates to SASL- if there's work to properly support SASL for psql/libpq while linking to an external library which supports, say, Kerberos through SASL, then sure, having that could work out well and I wouldn't object to it, but I don't agree that we should have dedicated SASL code which links to an external library on the server side without any way to exercise it through libpq/psql, or vice-versa. I also don't agree that this makes sense as an extension as we don't have any way for extensions to make changes in libpq or psql, again leading to the issue that it either can't be exercised or we create some dependency on an external SASL library for libpq but object to having that same dependency on the server side, which doesn't seem sensible to me. Requiring admins to jump through hoops to install an extension where we have such a dependency on a library anyway doesn't make sense either. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Hi Jacob, On Tue, Aug 2, 2022 at 2:48 PM Jacob Champion wrote: > [CFM hat] Okay, either way you look at it, this patchset needs author > work before any further review can be done. Peter has given some > additional feedback on next steps, Stephen has asked for clarification > on the goals of the patchset, etc. I feel pretty confident in marking > this Returned with Feedback. > That makes sense. I just got back to working on this patch and am aiming to get this ready for the next commitfest. I plan to address the feedback by Peter and Jeff and come up with a use case to help clarify the goals to better answer Stephen's questions. > > [dev hat] That said, I plan to do some additional dev work on top of > this over the next couple of months. The ideal case would be to provide > a server-only extension that provides additional features to existing > clients in the wild (i.e. no client-side changes). > > I'm also interested in plugging an existing 3rd-party SASL library into > the client, which would help extensibility on that side. > That would be interesting. Regards, Samay > > --Jacob > >
Re: Proposal: Support custom authentication methods using hooks
[CFM hat] Okay, either way you look at it, this patchset needs author work before any further review can be done. Peter has given some additional feedback on next steps, Stephen has asked for clarification on the goals of the patchset, etc. I feel pretty confident in marking this Returned with Feedback. [dev hat] That said, I plan to do some additional dev work on top of this over the next couple of months. The ideal case would be to provide a server-only extension that provides additional features to existing clients in the wild (i.e. no client-side changes). I'm also interested in plugging an existing 3rd-party SASL library into the client, which would help extensibility on that side. --Jacob
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-03-18 15:23:21 -0400, Stephen Frost wrote: > > * Andres Freund (and...@anarazel.de) wrote: > > > On 2022-03-18 00:45:49 -0400, Stephen Frost wrote: > > > > > I also don’t think that I agree that it’s acceptable to only have the > > > > > > ability to extend the authentication on the server side as that > > > > > > implies a > > > > > > whole bunch of client-side work that goes above and beyond just > > > > > > implementing an authentication system if it’s not possible to > > > > > > leverage > > > > > > libpq (which nearly all languages out there use..). Not addressing > > > > > > that > > > > > > side of it also makes me concerned that whatever we do here won’t be > > > > > > right/enough/etc. > > > > > > > > > > You're skipping over my point of everything that can be made to use > > > > > SASL-SCRAM-256 just working with the existing libpq? Again? > > > > > > > The plan is to make scram pluggable on the client side? That isn’t > > > > what’s > > > > been proposed here that I’ve seen. > > > > > > Libpq and many other drivers already speaks SASL-SCRAM-256. If you're > > > implementing an authentication method that wants to store secrets outside > > > of > > > postgres (centrally in another postgres instance, ldap, mysql or whatnot) > > > you > > > don't need to make scram pluggable client-side. From the perspective of > > > the > > > client it'd look *exactly* like the normal auth flow with the server. > > > > Then the only way to get support for something like, say, OAUTH, is to > > modify the core code. > > It wasn't the goal of the patch to add oauth support. I think it's a good sign > that the patch allowed Jacob to move the server side code in an extension, but > it's a benefit not a requirement. I disagree that it's actually at all useful when considering this change, specifically because it doesn't move the actual goalposts at all when it comes to adding support for new authentication methods for PG overall as the core code still has to be modified through the regular process. I get that the idea of this patch isn't to add oauth, but I don't think it's sensible to claim that the changes to the oauth patch to use these hooks on just the server side while still having a bunch of code in libpq for oauth makes this set of hooks make sense. I definitely don't think we should ever even consider adding support for something on the libpq side which require a 'custom' auth module on the server side that isn't included in core. Putting it into contrib would just ultimately make it really painful for users to deal with without any benefit that I can see either as we'd still have to maintain it and update it through the regular PG process. Trying hard to give the benefit of the doubt here, maybe you could argue that by having the module in contrib and therefore not loaded unless requested that the system might be overall 'more secure', but given that we already require admins to specify what the auth method is with pg_hba and that hasn't been a source of a lot of CVEs around somehow getting the wrong code in there, I just don't see it as a sensible justfication. > I think we might be able to build ontop of this to make things even more > extensible and it's worth having the discussion whether the proposal can be > made more extensible with a reasonable amount of complexity. But that's > different from faulting the patch for not achieving something it didn't set > out to do. That it wasn't clear just what the idea of this patch was strikes me as another point against it, really. The thread talked about custom authentication methods and the modification to pg_hba clearly made it out to be a top-level alternative to SCRAM and the other methods even though it sounds like that wasn't the intent, or maybe was but was only part of it (but then, what's the other part..? That's still unclear to me even after reading these latest emails..). > > That strikes me as entirely defeating the point of having this be > > extensible, since you still have to modify the core code to get support > > for it. > > Since it wasn't the goal to add oauth... But.. a patch was specifically proposed on this thread to use this to add oauth, with encouragment from the folks working on this patch, and that was then used, even just above, as a reason why this was a useful change to make, but I don't see what about it makes it such. > > I don't get why such an exercise would have been done if the goal is to > > just allow what's in rolpassword to be pulled from somewhere else or why > > we would be talking about different auth methods in the first place if > > that's the goal here. > > It's called that way because auth.c calling it that. See > > void > ClientAuthentication(Port *port) > ... > switch (port->hba->auth_method) > > even though the different auth_methods use a smaller number of ways of > exchanges of secrets than we have "auth
Re: Proposal: Support custom authentication methods using hooks
On 15.03.22 20:27, samay sharma wrote: This patch-set adds the following: * Allow multiple custom auth providers to be registered (Addressing feedback from Aleksander and Andrew) * Modify the test extension to use SCRAM to exchange secrets (Based on Andres's suggestion) * Add support for custom auth options to configure provider's behavior (by exposing a new hook) (Required by OAUTHBEARER) * Allow custom auth methods to use usermaps. (Required by OAUTHBEARER) Some feedback on this specific patch set: Custom authentication methods should be able to register their own name other than "custom". You ought to refactor things so that existing methods such as ldap and pam go through your extension interface. So the whole thing should be more like a lookup table or list with some built-in entries that modules can dynamically add on to. Then you also don't need a test module, since the existing authentication methods would already test the interfaces.
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-03-18 15:23:21 -0400, Stephen Frost wrote: > * Andres Freund (and...@anarazel.de) wrote: > > On 2022-03-18 00:45:49 -0400, Stephen Frost wrote: > > > > I also don’t think that I agree that it’s acceptable to only have the > > > > > ability to extend the authentication on the server side as that > > > > > implies a > > > > > whole bunch of client-side work that goes above and beyond just > > > > > implementing an authentication system if it’s not possible to leverage > > > > > libpq (which nearly all languages out there use..). Not addressing > > > > > that > > > > > side of it also makes me concerned that whatever we do here won’t be > > > > > right/enough/etc. > > > > > > > > You're skipping over my point of everything that can be made to use > > > > SASL-SCRAM-256 just working with the existing libpq? Again? > > > > > The plan is to make scram pluggable on the client side? That isn’t what’s > > > been proposed here that I’ve seen. > > > > Libpq and many other drivers already speaks SASL-SCRAM-256. If you're > > implementing an authentication method that wants to store secrets outside of > > postgres (centrally in another postgres instance, ldap, mysql or whatnot) > > you > > don't need to make scram pluggable client-side. From the perspective of the > > client it'd look *exactly* like the normal auth flow with the server. > > Then the only way to get support for something like, say, OAUTH, is to > modify the core code. It wasn't the goal of the patch to add oauth support. I think it's a good sign that the patch allowed Jacob to move the server side code in an extension, but it's a benefit not a requirement. I think we might be able to build ontop of this to make things even more extensible and it's worth having the discussion whether the proposal can be made more extensible with a reasonable amount of complexity. But that's different from faulting the patch for not achieving something it didn't set out to do. > That strikes me as entirely defeating the point of having this be > extensible, since you still have to modify the core code to get support > for it. Since it wasn't the goal to add oauth... > I don't get why such an exercise would have been done if the goal is to > just allow what's in rolpassword to be pulled from somewhere else or why > we would be talking about different auth methods in the first place if > that's the goal here. It's called that way because auth.c calling it that. See void ClientAuthentication(Port *port) ... switch (port->hba->auth_method) even though the different auth_methods use a smaller number of ways of exchanges of secrets than we have "auth methods". So it doesn't at all seem insane to name something allowing different authentication methods in the sense of auth.c "custom authentication methods". > That the patch was also brought up in the context of wanting to add support > for another auth method also doesn't seem to support the argument that this > is just about changing where the value in rolpassword comes from. My point isn't that it's *just* the changing the value of rollpassword, but that the authentication exchange with the client just uses the existing secret exchanges, because they're sufficient for plenty use cases. > > As I said in > > https://www.postgresql.org/message-id/20220318032520.t2bcwrnterg6lq6g%40alap3.anarazel.de > > when describing the above, that's not enough to implement every desireable > > authentication method - but there's real use-cases where using the existing > > SASL-SCRAM-256 is sufficient. > > That this apparently isn't actually about adding new authentication > protocols or methods sure strikes me as odd when folks were adjusting > proposed patches specificially to use this for new authentication > methods and the subject is "Proposal: Support custom authentication > methods using hooks" See above. Note also it was called "Support custom authentication methods" not "Support custom authentication protocols". > If what's happening is still actually SCRAM then > I also don't get why we would change pg_hba.conf to say 'custom' instead > of scram-sha-256 w/ some option to say how to go get the actual token, > or why there isn't anything done to deal with passwords being changed. Normally make extension APIs reasonably generic. If you want to argue against that here, I think that's a debate worth having (as I explicitly said upthread!), but we should be having it about that then. It does seem good to me that the proposed API could implement stuff like peer, bsd, PAM, etc without a problem. And there's plenty other things that don't require different authentication protocols on the pg protocol level - using os specific extensions to safely get the remote user of tcp connections would be cool for example. > Basically, I don't agree with more-or-less anything that the patch is > doing if that's the goal. If the goal is to actually change where the > token in rolpassword comes from for
Re: Proposal: Support custom authentication methods using hooks
Greetings,
* Andres Freund (and...@anarazel.de) wrote:
> On 2022-03-18 00:45:49 -0400, Stephen Frost wrote:
> > > I also don’t think that I agree that it’s acceptable to only have the
> > > > ability to extend the authentication on the server side as that implies
> > > > a
> > > > whole bunch of client-side work that goes above and beyond just
> > > > implementing an authentication system if it’s not possible to leverage
> > > > libpq (which nearly all languages out there use..). Not addressing that
> > > > side of it also makes me concerned that whatever we do here won’t be
> > > > right/enough/etc.
> > >
> > > You're skipping over my point of everything that can be made to use
> > > SASL-SCRAM-256 just working with the existing libpq? Again?
>
> > The plan is to make scram pluggable on the client side? That isn’t what’s
> > been proposed here that I’ve seen.
>
> Libpq and many other drivers already speaks SASL-SCRAM-256. If you're
> implementing an authentication method that wants to store secrets outside of
> postgres (centrally in another postgres instance, ldap, mysql or whatnot) you
> don't need to make scram pluggable client-side. From the perspective of the
> client it'd look *exactly* like the normal auth flow with the server.
Then the only way to get support for something like, say, OAUTH, is to
modify the core code.
That strikes me as entirely defeating the point of having this be
extensible, since you still have to modify the core code to get support
for it.
> What's proposed here is not really about adding new authentication protocols
> between FE/BE (although some *limited* forms of that might be possible). It's
> primarily about using the existing FE/BE authentication exchanges
> (AUTH_REQ_{PASSWORD,SASL*,...}) to validate against something other than the
> builtin pg_authid.rolpassword. Because drivers already know those
> authentication exchanges, it doesn't need to learn new tricks.
The OAUTH patch was specifically changed to try and use this and yet it
still had to modify the core code on the libpq side (at least, maybe
other things or maybe not depending on later changes to this patch).
I don't get why such an exercise would have been done if the goal is to
just allow what's in rolpassword to be pulled from somewhere else or why
we would be talking about different auth methods in the first place if
that's the goal here. That the patch was also brought up in the context
of wanting to add support for another auth method also doesn't seem to
support the argument that this is just about changing where the value in
rolpassword comes from.
> As I said in
> https://www.postgresql.org/message-id/20220318032520.t2bcwrnterg6lq6g%40alap3.anarazel.de
> when describing the above, that's not enough to implement every desireable
> authentication method - but there's real use-cases where using the existing
> SASL-SCRAM-256 is sufficient.
That this apparently isn't actually about adding new authentication
protocols or methods sure strikes me as odd when folks were adjusting
proposed patches specificially to use this for new authentication
methods and the subject is "Proposal: Support custom authentication
methods using hooks". If what's happening is still actually SCRAM then
I also don't get why we would change pg_hba.conf to say 'custom' instead
of scram-sha-256 w/ some option to say how to go get the actual token,
or why there isn't anything done to deal with passwords being changed.
Basically, I don't agree with more-or-less anything that the patch is
doing if that's the goal. If the goal is to actually change where the
token in rolpassword comes from for existing authentication methods and
specifically isn't to actually try to use this to support new
authenitcation methods, then I'd suggest laying it out that way and
having it be an option for scram-sha-256 or whatever other methods
(though that seems like the only one that should really be changed in
this way, if you want my 2c, as the rest that would work this way are
basically broken, that being md5) and then make sure to have a way to
handle password changes too.
> > I also wasn’t aware that we felt that it’s acceptable to just ignore other
> > committers.
>
> I'm not talking about going ahead and committing. Just not continuing
> discussing this topci and spending my time more fruitfully on something else.
I'm still a -1 on this patch. Maybe there's a way to get a shared
storage for rolpassword and maybe that could even be extensible if we
want it to be (though I'd be more inclined to just build it in...) and
if that's actually the goal but this doesn't seem like the right
approach to be taking. I also don't think that it's a good idea to have
a synchronous call-out during authentication as that can get really
painful and if you're cacheing it to deal with the risks of that, well,
seems like you'd be better off just using rolpassword (and perhaps with
Joshua's patches to allow more than one of those to exist at a time).
Re: Proposal: Support custom authentication methods using hooks
Hi,
On 2022-03-18 00:45:49 -0400, Stephen Frost wrote:
> > I also don’t think that I agree that it’s acceptable to only have the
> > > ability to extend the authentication on the server side as that implies a
> > > whole bunch of client-side work that goes above and beyond just
> > > implementing an authentication system if it’s not possible to leverage
> > > libpq (which nearly all languages out there use..). Not addressing that
> > > side of it also makes me concerned that whatever we do here won’t be
> > > right/enough/etc.
> >
> > You're skipping over my point of everything that can be made to use
> > SASL-SCRAM-256 just working with the existing libpq? Again?
> The plan is to make scram pluggable on the client side? That isn’t what’s
> been proposed here that I’ve seen.
Libpq and many other drivers already speaks SASL-SCRAM-256. If you're
implementing an authentication method that wants to store secrets outside of
postgres (centrally in another postgres instance, ldap, mysql or whatnot) you
don't need to make scram pluggable client-side. From the perspective of the
client it'd look *exactly* like the normal auth flow with the server.
What's proposed here is not really about adding new authentication protocols
between FE/BE (although some *limited* forms of that might be possible). It's
primarily about using the existing FE/BE authentication exchanges
(AUTH_REQ_{PASSWORD,SASL*,...}) to validate against something other than the
builtin pg_authid.rolpassword. Because drivers already know those
authentication exchanges, it doesn't need to learn new tricks.
As I said in
https://www.postgresql.org/message-id/20220318032520.t2bcwrnterg6lq6g%40alap3.anarazel.de
when describing the above, that's not enough to implement every desireable
authentication method - but there's real use-cases where using the existing
SASL-SCRAM-256 is sufficient.
> I also wasn’t aware that we felt that it’s acceptable to just ignore other
> committers.
I'm not talking about going ahead and committing. Just not continuing
discussing this topci and spending my time more fruitfully on something else.
Greetings,
Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, On Fri, Mar 18, 2022 at 00:24 Andres Freund wrote: > Hi, > > On 2022-03-18 00:03:37 -0400, Stephen Frost wrote: > > On Thu, Mar 17, 2022 at 23:25 Andres Freund wrote: > > > It's imo a separate project to make the client side extensible. There's > > > plenty > > > of authentication methods that don't need any further client side > support > > > than > > > the existing SASL (or password if OK for some use case) messages, which > > > most > > > clients (including libpq) already know. > > > > > > Of course the fact that libpq "only" speaks SCRAM-SHA-256 is a limit > > > (unless > > > you have server side access to clear text passwords, but brrr). But > there's > > > plenty that can be done with that. Proxying auth via a central postgres > > > server > > > with the secrets, sharing secrets with other data stores also > understanding > > > SCRAM-SHA-256, ... > > > > > > There definitely *also* are important authentication methods that > can't be > > > implemented without further client side support. Some of those could > > > plausibly > > > be tackled on the protocol / libpq level in a way that they could be > used > > > by > > > multiple authentication methods. Other authentication methods > definitely > > > need > > > dedicated code in the client (although the protocol likely can be > fairly > > > generic). > > > > > > Given that there's benefit from the server side extensibility alone, I > > > don't > > > see much benefit in tying the server side extensibility to the client > side > > > extensibility. > > > > > > How are we going to reasonably test this then? > > The test module in the patch is a good starting point. Without a complete, independent, “this is how it will really be used” on both the server and client side set of tests I’m not sure that it is. > I also don’t think that I agree that it’s acceptable to only have the > > ability to extend the authentication on the server side as that implies a > > whole bunch of client-side work that goes above and beyond just > > implementing an authentication system if it’s not possible to leverage > > libpq (which nearly all languages out there use..). Not addressing that > > side of it also makes me concerned that whatever we do here won’t be > > right/enough/etc. > > You're skipping over my point of everything that can be made to use > SASL-SCRAM-256 just working with the existing libpq? Again? The plan is to make scram pluggable on the client side? That isn’t what’s been proposed here that I’ve seen. Or is the idea that we are going to have built-in support for some subset of “custom” things, but only on the client side because it’s hard to do custom things there, but they’re custom and have to be loaded through shared preload libraries on the server side? If you want to argue that somehow that communicating via SASL-SCRAM-256 > from a > plugin is not a sufficient use-case, or that the API should be shaped more > specifically to just use SASL-SCRAM-256, fine. Argue it. Otherwise I don't > know what to do except ignore you. One thrust of my argument is that if we are going to support custom authentication methods then we need to consider both sides of that, not just the server side. Another is- it’s highly unlikely that the next authentication method will actually be able to be implemented using these custom hooks, based on the history we have seen of pluggable authentication systems, and therefore I don’t agree that they make sense or will be what we need in the future, which seems to be a large thrust of the argument here. I’m also concerned about the risks that this presents to the project in that there will be arguments about where the fault lies between the hooks and the core code, as this is just inherently security-related bits, yet that doesn’t seem to be addressed. Either way though, it strikes me as likely to be leaving our users in a bad position either way. > I also wasn’t aware that we felt that it’s acceptable to just ignore other committers. Thanks, Stephen >
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-03-18 00:03:37 -0400, Stephen Frost wrote: > On Thu, Mar 17, 2022 at 23:25 Andres Freund wrote: > > It's imo a separate project to make the client side extensible. There's > > plenty > > of authentication methods that don't need any further client side support > > than > > the existing SASL (or password if OK for some use case) messages, which > > most > > clients (including libpq) already know. > > > > Of course the fact that libpq "only" speaks SCRAM-SHA-256 is a limit > > (unless > > you have server side access to clear text passwords, but brrr). But there's > > plenty that can be done with that. Proxying auth via a central postgres > > server > > with the secrets, sharing secrets with other data stores also understanding > > SCRAM-SHA-256, ... > > > > There definitely *also* are important authentication methods that can't be > > implemented without further client side support. Some of those could > > plausibly > > be tackled on the protocol / libpq level in a way that they could be used > > by > > multiple authentication methods. Other authentication methods definitely > > need > > dedicated code in the client (although the protocol likely can be fairly > > generic). > > > > Given that there's benefit from the server side extensibility alone, I > > don't > > see much benefit in tying the server side extensibility to the client side > > extensibility. > > > How are we going to reasonably test this then? The test module in the patch is a good starting point. > I also don’t think that I agree that it’s acceptable to only have the > ability to extend the authentication on the server side as that implies a > whole bunch of client-side work that goes above and beyond just > implementing an authentication system if it’s not possible to leverage > libpq (which nearly all languages out there use..). Not addressing that > side of it also makes me concerned that whatever we do here won’t be > right/enough/etc. You're skipping over my point of everything that can be made to use SASL-SCRAM-256 just working with the existing libpq? Again? If you want to argue that somehow that communicating via SASL-SCRAM-256 from a plugin is not a sufficient use-case, or that the API should be shaped more specifically to just use SASL-SCRAM-256, fine. Argue it. Otherwise I don't know what to do except ignore you. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, On Thu, Mar 17, 2022 at 23:25 Andres Freund wrote: > On 2022-03-17 22:13:27 -0400, Stephen Frost wrote: > > This isn’t the first time I asked about this on this thread, yet the > > question about why this is only being discussed as backend changes, and > > with the only goal seeming to be to have a backend loadable module, > without > > anything on the client side for something that’s clearly both a server > and > > client side concern, seems to just be ignored, which make me feel like my > > comments and the concerns that I raise aren’t being appreciated. > > It's imo a separate project to make the client side extensible. There's > plenty > of authentication methods that don't need any further client side support > than > the existing SASL (or password if OK for some use case) messages, which > most > clients (including libpq) already know. > > Of course the fact that libpq "only" speaks SCRAM-SHA-256 is a limit > (unless > you have server side access to clear text passwords, but brrr). But there's > plenty that can be done with that. Proxying auth via a central postgres > server > with the secrets, sharing secrets with other data stores also understanding > SCRAM-SHA-256, ... > > There definitely *also* are important authentication methods that can't be > implemented without further client side support. Some of those could > plausibly > be tackled on the protocol / libpq level in a way that they could be used > by > multiple authentication methods. Other authentication methods definitely > need > dedicated code in the client (although the protocol likely can be fairly > generic). > > Given that there's benefit from the server side extensibility alone, I > don't > see much benefit in tying the server side extensibility to the client side > extensibility. How are we going to reasonably test this then? I also don’t think that I agree that it’s acceptable to only have the ability to extend the authentication on the server side as that implies a whole bunch of client-side work that goes above and beyond just implementing an authentication system if it’s not possible to leverage libpq (which nearly all languages out there use..). Not addressing that side of it also makes me concerned that whatever we do here won’t be right/enough/etc. This is not an area, in my view, where flexibility for the sake of it is good. Misunderstandings between the client and the server or between how the core code and the hooks interact seem very likely and could easily lead to insecure setups and a good chance for CVEs. Much like encryption, authentication isn’t easy to get right. We should be working to implement standard that experts, through RFCs and similar well-defined protocols, have defined in the core code with lots of eyes looking at it. So, very much a -1 from me for trying to extend the backend in this way. I see a lot of risk and not much, if any, benefit. I’d much rather see us add support directly in the core code, on the client and sever side, for OAUTH and other well defined authentication methods, and we even have an active patch for that could make progress on that with a bit of review. Thanks, Stephen >
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-03-17 22:13:27 -0400, Stephen Frost wrote: > This isn’t the first time I asked about this on this thread, yet the > question about why this is only being discussed as backend changes, and > with the only goal seeming to be to have a backend loadable module, without > anything on the client side for something that’s clearly both a server and > client side concern, seems to just be ignored, which make me feel like my > comments and the concerns that I raise aren’t being appreciated. It's imo a separate project to make the client side extensible. There's plenty of authentication methods that don't need any further client side support than the existing SASL (or password if OK for some use case) messages, which most clients (including libpq) already know. Of course the fact that libpq "only" speaks SCRAM-SHA-256 is a limit (unless you have server side access to clear text passwords, but brrr). But there's plenty that can be done with that. Proxying auth via a central postgres server with the secrets, sharing secrets with other data stores also understanding SCRAM-SHA-256, ... There definitely *also* are important authentication methods that can't be implemented without further client side support. Some of those could plausibly be tackled on the protocol / libpq level in a way that they could be used by multiple authentication methods. Other authentication methods definitely need dedicated code in the client (although the protocol likely can be fairly generic). Given that there's benefit from the server side extensibility alone, I don't see much benefit in tying the server side extensibility to the client side extensibility. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, On Thu, Mar 17, 2022 at 17:02 Andres Freund wrote: > On 2022-03-16 18:50:23 -0400, Stephen Frost wrote: > > First, let's be clear- we aren't actually talking about custom or > > pluggable authentication here, at least when it comes to PG as a > > project. For it to actually be pluggable, it needs to be supported on > > both the client side and the server side, not just the server side. > > > > That this keeps getting swept under the carpet makes me feel like this > > isn't actually an argument about the best way to move the PG project > > forward but rather has another aim. > > This is insulting and unjustified. IMO completely inappropriate for the > list / > community. I've also brought this up privately, but I thought it important > to > state so publically. I am concerned. I don’t intend to insult you or anyone else on this thread. I’m sorry. This isn’t the first time I asked about this on this thread, yet the question about why this is only being discussed as backend changes, and with the only goal seeming to be to have a backend loadable module, without anything on the client side for something that’s clearly both a server and client side concern, seems to just be ignored, which make me feel like my comments and the concerns that I raise aren’t being appreciated. I had drafted a response to your private email to me but hadn’t wanted to send it without going over it again after taking time to be sure I had cooled down and was being level-headed in my response. I am sorry. I am still concerned. Thanks, Stephen >
Re: Proposal: Support custom authentication methods using hooks
On 2022-03-16 18:50:23 -0400, Stephen Frost wrote: > First, let's be clear- we aren't actually talking about custom or > pluggable authentication here, at least when it comes to PG as a > project. For it to actually be pluggable, it needs to be supported on > both the client side and the server side, not just the server side. > > That this keeps getting swept under the carpet makes me feel like this > isn't actually an argument about the best way to move the PG project > forward but rather has another aim. This is insulting and unjustified. IMO completely inappropriate for the list / community. I've also brought this up privately, but I thought it important to state so publically.
Re: Proposal: Support custom authentication methods using hooks
Greetings,
* Andres Freund (and...@anarazel.de) wrote:
> On 2022-03-17 14:03:31 -0400, Stephen Frost wrote:
> > * Andres Freund (and...@anarazel.de) wrote:
> > > On 2022-03-17 12:10:51 +0100, Peter Eisentraut wrote:
> > > > Looking at the existing authentication methods
> > > >
> > > > # METHOD can be "trust", "reject", "md5", "password", "scram-sha-256",
> > > > # "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert".
> > > >
> > > > how many of these could have been implemented using a plugin mechanism
> > > > that
> > > > was designed before the new method was considered? Probably not many.
> > >
> > > trust, reject, md5, password, ident, peer, pam, ldap, radius look
> > > trivially
> > > possible. I think sspi is doable as well, but I don't know it well enough
> > > to
> > > be confident. gss without transport encryption could have as well, I
> > > think. Even scram-sha-256 looks possible, although that'd have been a
> > > good bit
> > > harder. Where do you see the problems?
> >
> > I agree with Peter and don't feel like the above actually answered the
> > question in any fashion. The question wasn't "which auth methods could
> > be implemented using these new set of hooks?" but rather- which could
> > have been implemented using a plugin mechanism that was designed
> > *before* the new method was considered? The answer to that is
> > 'basically none'.
>
> I fail to see how you come to that conclusion.
...
> > There even existed a plugin mechanism (PAM) before many of them, and they
> > weren't able implemented using it.
>
> That's nonsensical. PAM is a platform specific API to start with - so it
> doesn't make sense to implement additional postgres authentication mechanism
> with it. It also wasn't added to postgres as a base mechanism for extensible
> authentication. And it's fundamentally restricted in the kind of secret
> exchanges that can be implemented with it.
Exactly because of this. The folks who came up with PAM didn't forsee
the direction that authentication methods were going to go in and I
don't see any particular reason why we're smarter than they were.
> > That's entirely the point- while this might be an interesting way to
> > redesign what we have now, should we feel that's useful to do (I don't and
> > think it would be an actively bad thing for the project to try and do...)
> > there's no reason to believe that it'll actually be useful for the *next*
> > auth method that comes along.
>
> What concrete limitation do you see in the API? It basically gives you the
> same powers as writing something in auth.c directly. And due to AUTH_REQ_SASL*
> we have fairly extensible ways of exchanging authentication data.
I'm not one to predict what the next authentication method to come down
the road is but I doubt that an API we come up with today will actually
work to perfectly implement it. GSSAPI was supposed to be an extensible
authentication method too.
Also- how is this going to work client-side in libpq?
> > > Only stuff tying into transport encryption is clearly not doable via the
> > > proposed API, but that's hardly the fault of an authentication API?
> >
> > This is absolutely the wrong way to look at it. The authentication
> > methods that are coming along today are designed to tie into the
> > transport encryption because that's *needed* to avoid MITM attacks. I'd
> > much rather we generally avoid including ones that don't support that
> > and we certainly shouldn't be trying to build a generic system which
> > explicitly doesn't support it.
>
> So you're saying that any authentication API needs to come together with a
> runtime extendable secure transport mechanism? That feels like raising the bar
> ridiculously into the sky. If we want to add more transport mechanisms - and
> I'm not sure we do - that's a very sizable project on its own. And
> independent.
No, that's not what I'm saying. Authentication mechanisms should have a
way to tie themselves to the secure transport layer is what I was
getting at, which you seemed to be saying wasn't possible with this API.
However, it's certainly a relevant point that something like encrypted
GSSAPI still wouldn't be able to be implemented with this. I don't know
that it's required for a new auth method to have that, but not being
able to do it with the proposed API is another point against this
approach. That is, even the existing methods that we've got today
wouldn't all be able to work with this.
> Note that you *can* combine existing secure transport mechanisms with the
> proposed API. You can use Port->{ssl,gss,peer_cn,...} to query information and
> do further openssl etc calls.
If it's possible then that's good, though I do have concerns about how
this plays into support for different transport layers or even libraries
once we get support for an alternative to openssl.
Thanks,
Stephen
signature.asc
Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Thu, 2022-03-17 at 14:03 -0400, Stephen Frost wrote: > * Andres Freund (and...@anarazel.de) wrote: > > > > trust, reject, md5, password, ident, peer, pam, ldap, radius look trivially > > possible. I think sspi is doable as well, but I don't know it well enough to > > be confident. gss without transport encryption could have as well, I > > think. Even scram-sha-256 looks possible, although that'd have been a good > > bit > > harder. Where do you see the problems? > > I agree with Peter and don't feel like the above actually answered the > question in any fashion. The question wasn't "which auth methods could > be implemented using these new set of hooks?" but rather- which could > have been implemented using a plugin mechanism that was designed > *before* the new method was considered? The answer to that is > 'basically none'. There even existed a plugin mechanism (PAM) before > many of them, and they weren't able implemented using it. That's > entirely the point- while this might be an interesting way to redesign > what we have now, should we feel that's useful to do (I don't and think > it would be an actively bad thing for the project to try and do...) > there's no reason to believe that it'll actually be useful for the > *next* auth method that comes along. We already have the beginnings of a SASL framework; part of the work we did with the OAUTHBEARER experiment was to generalize more pieces of it. More generalization can still be done. Samay has shown that one SASL mechanism can be ported, I'm in the process of porting another, and we already know that the system can handle a non-SASL (password- based) mechanism. So that's a pretty good start. > > Only stuff tying into transport encryption is clearly not doable via the > > proposed API, but that's hardly the fault of an authentication API? > > This is absolutely the wrong way to look at it. The authentication > methods that are coming along today are designed to tie into the > transport encryption because that's *needed* to avoid MITM attacks. I'd > much rather we generally avoid including ones that don't support that > and we certainly shouldn't be trying to build a generic system which > explicitly doesn't support it. Many SASL mechanisms support channel binding, which the server already has helpers for, and which SCRAM is already using. At least one SASL mechanism (GSSAPI) supports the negotiation of transport confidentiality. These are answerable questions. I don't think it's fair to ask Samay to have perfect answers while simultaneously implementing a bunch of different requests in code and going through the brainstorming process alongside us all. (It would be fair if Samay were claiming the design were finished, but that's not what I've seen so far.) --Jacob
Re: Proposal: Support custom authentication methods using hooks
Hi,
On 2022-03-17 14:03:31 -0400, Stephen Frost wrote:
> * Andres Freund (and...@anarazel.de) wrote:
> > On 2022-03-17 12:10:51 +0100, Peter Eisentraut wrote:
> > > Looking at the existing authentication methods
> > >
> > > # METHOD can be "trust", "reject", "md5", "password", "scram-sha-256",
> > > # "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert".
> > >
> > > how many of these could have been implemented using a plugin mechanism
> > > that
> > > was designed before the new method was considered? Probably not many.
> >
> > trust, reject, md5, password, ident, peer, pam, ldap, radius look trivially
> > possible. I think sspi is doable as well, but I don't know it well enough to
> > be confident. gss without transport encryption could have as well, I
> > think. Even scram-sha-256 looks possible, although that'd have been a good
> > bit
> > harder. Where do you see the problems?
>
> I agree with Peter and don't feel like the above actually answered the
> question in any fashion. The question wasn't "which auth methods could
> be implemented using these new set of hooks?" but rather- which could
> have been implemented using a plugin mechanism that was designed
> *before* the new method was considered? The answer to that is
> 'basically none'.
I fail to see how you come to that conclusion.
> There even existed a plugin mechanism (PAM) before many of them, and they
> weren't able implemented using it.
That's nonsensical. PAM is a platform specific API to start with - so it
doesn't make sense to implement additional postgres authentication mechanism
with it. It also wasn't added to postgres as a base mechanism for extensible
authentication. And it's fundamentally restricted in the kind of secret
exchanges that can be implemented with it.
> That's entirely the point- while this might be an interesting way to
> redesign what we have now, should we feel that's useful to do (I don't and
> think it would be an actively bad thing for the project to try and do...)
> there's no reason to believe that it'll actually be useful for the *next*
> auth method that comes along.
What concrete limitation do you see in the API? It basically gives you the
same powers as writing something in auth.c directly. And due to AUTH_REQ_SASL*
we have fairly extensible ways of exchanging authentication data.
> > Only stuff tying into transport encryption is clearly not doable via the
> > proposed API, but that's hardly the fault of an authentication API?
>
> This is absolutely the wrong way to look at it. The authentication
> methods that are coming along today are designed to tie into the
> transport encryption because that's *needed* to avoid MITM attacks. I'd
> much rather we generally avoid including ones that don't support that
> and we certainly shouldn't be trying to build a generic system which
> explicitly doesn't support it.
So you're saying that any authentication API needs to come together with a
runtime extendable secure transport mechanism? That feels like raising the bar
ridiculously into the sky. If we want to add more transport mechanisms - and
I'm not sure we do - that's a very sizable project on its own. And
independent.
Note that you *can* combine existing secure transport mechanisms with the
proposed API. You can use Port->{ssl,gss,peer_cn,...} to query information and
do further openssl etc calls.
Greetings,
Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-03-17 12:10:51 +0100, Peter Eisentraut wrote: > > Looking at the existing authentication methods > > > > # METHOD can be "trust", "reject", "md5", "password", "scram-sha-256", > > # "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert". > > > > how many of these could have been implemented using a plugin mechanism that > > was designed before the new method was considered? Probably not many. > > trust, reject, md5, password, ident, peer, pam, ldap, radius look trivially > possible. I think sspi is doable as well, but I don't know it well enough to > be confident. gss without transport encryption could have as well, I > think. Even scram-sha-256 looks possible, although that'd have been a good bit > harder. Where do you see the problems? I agree with Peter and don't feel like the above actually answered the question in any fashion. The question wasn't "which auth methods could be implemented using these new set of hooks?" but rather- which could have been implemented using a plugin mechanism that was designed *before* the new method was considered? The answer to that is 'basically none'. There even existed a plugin mechanism (PAM) before many of them, and they weren't able implemented using it. That's entirely the point- while this might be an interesting way to redesign what we have now, should we feel that's useful to do (I don't and think it would be an actively bad thing for the project to try and do...) there's no reason to believe that it'll actually be useful for the *next* auth method that comes along. > Only stuff tying into transport encryption is clearly not doable via the > proposed API, but that's hardly the fault of an authentication API? This is absolutely the wrong way to look at it. The authentication methods that are coming along today are designed to tie into the transport encryption because that's *needed* to avoid MITM attacks. I'd much rather we generally avoid including ones that don't support that and we certainly shouldn't be trying to build a generic system which explicitly doesn't support it. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-03-17 12:10:51 +0100, Peter Eisentraut wrote: > Looking at the existing authentication methods > > # METHOD can be "trust", "reject", "md5", "password", "scram-sha-256", > # "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert". > > how many of these could have been implemented using a plugin mechanism that > was designed before the new method was considered? Probably not many. trust, reject, md5, password, ident, peer, pam, ldap, radius look trivially possible. I think sspi is doable as well, but I don't know it well enough to be confident. gss without transport encryption could have as well, I think. Even scram-sha-256 looks possible, although that'd have been a good bit harder. Where do you see the problems? Only stuff tying into transport encryption is clearly not doable via the proposed API, but that's hardly the fault of an authentication API? Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
On 16.03.22 21:27, samay sharma wrote: The only goal of this patch wasn't to enable support for Azure AD. That's just one client. Users might have a need to add or change auth methods in the future and providing that extensibility so we don't need to have core changes for each one of them would be useful. I know there isn't alignment on this yet, but if we'd like to move certain auth methods out of core into extensions, then this might provide a good framework for that. Looking at the existing authentication methods # METHOD can be "trust", "reject", "md5", "password", "scram-sha-256", # "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert". how many of these could have been implemented using a plugin mechanism that was designed before the new method was considered? Probably not many. So I am fundamentally confused how this patch set can make such an ambitious claim. Maybe the scope needs to be clarified first. What kinds of authentication methods do you want to plug in? What kinds of methods are out of scope? What are examples of each one?
Re: Proposal: Support custom authentication methods using hooks
Greetings, * samay sharma (smilingsa...@gmail.com) wrote: > On Wed, Mar 16, 2022 at 8:02 AM Stephen Frost wrote: > > How about- if we just added OAUTH support directly into libpq and the > > backend, would that work with Azure's OIDC provider? If not, why not? > > Overall, Azure AD implements OIDC, so this could be doable. However, we'll > have to explore further to see if the current implementation would satisfy > all the requirements. Great, would be useful to know what's discovered here and if OIDC doesn't do what you're looking for, how you're intending to work on improving it to make it so that it does, in an open and transparent manner through the RFC process. > > If it does, then what's the justification for trying to allow custom > > backend-only authentication methods? > > The only goal of this patch wasn't to enable support for Azure AD. That's > just one client. Users might have a need to add or change auth methods in > the future and providing that extensibility so we don't need to have core > changes for each one of them would be useful. I know there isn't alignment > on this yet, but if we'd like to move certain auth methods out of core into > extensions, then this might provide a good framework for that. Hand-waving at this and saying that other people want it is not justification for it- who else, exactly, has come along and asked for it? I also disagree that we actually want to move authentication methods out of core- that was an argument brought up in this thread as justification for why these hooks should exist but I don't see anyone other than the folks that want the hooks actually saying that's something they want. There are some folks who agree that we should drop support for certain authentication methods but that's not at all the same thing (and no, moving them to contrib isn't somehow deprecating or removing them from what we have to support or making it so that we don't have to deal with them). Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Wed, 2022-03-16 at 11:02 -0400, Stephen Frost wrote: > > That we're having to extend this quite a bit to work for the proposed > > OAUTH patches and that it still doesn't do anything for the client > > side > > (note that the OAUTHBEARER patches are still patching libpq to add > > support directly and surely will still be even with this latest patch > > set ...) makes me seriously doubt that this is the right direction to > > be > > going in. > > I don't follow this concern. I assume you're referring to the last two > bullet points, which I repeat below: > > * Add support for custom auth options to configure provider's > behavior: > > Even without OAUTH I think this would have been requested. > Configuration of some kind is going to be necessary. Without custom > options, I guess the provider would need to define its own config file? > Not the end of the world, but not ideal. Yes, configuration of some kind will be needed and getting that configuration correct is going to be essential to being able to have a secure authentication system. > > How about- if we just added OAUTH support directly into libpq and the > > backend, would that work with Azure's OIDC provider? If not, why > > not? > > If it does, then what's the justification for trying to allow custom > > backend-only authentication methods? > > I understand the point, but it also has a flip side: if custom auth > works, why do we need to block waiting for a complete core OAUTH > feature? First, let's be clear- we aren't actually talking about custom or pluggable authentication here, at least when it comes to PG as a project. For it to actually be pluggable, it needs to be supported on both the client side and the server side, not just the server side. That this keeps getting swept under the carpet makes me feel like this isn't actually an argument about the best way to move the PG project forward but rather has another aim. I don't think we should even consider accepting a patch to make this something that works only on the server side as, in such a case, we wouldn't even be able to have an extension of our own that leverages it or tests it without bespoke code for that purpose. I certainly don't think we should add code to libpq for some auth method that isn't even available in a default install of PG, as would happen if we accepted both this patch and the patch to add OAUTH support to libpq. What exactly is the thinking on how this would move forward? We'd commit this custom support that requires an external extension to actually work and then add hacked-in code to libpq in order for folks to be able to use OAUTH? What happens when, and not if, something changes in that extension that requires a change on the client side..? Are we going to be dealing with CVEs for the libpq side of this? > Authentication seems like a good candidate for allowing custom methods. It's not and this is clear from looking at history. We have seen this time and time again- PAM, SASL, RADIUS could be included, EAP, etc. You'll note that we already support some of these, yet you'd like to now add some set of hooks in addition to these pluggable options that already exist because, presumably, they don't actually solve what you're looking for. That's actually rather typical when it comes to authentication methods- everyone has this idea that it can be pluggable or a few hooks to allow a custom method will work for the next great thing, but that's not what actually happens. > New ones are always coming along, being used in new ways, updating to > newer crypto, or falling out of favor. We've accumulated quite a few. I agree that we need to get rid of some of the ones that we've got, but even so, at least the ones we have are maintained and updated to the extent possible for us to fix issues with them. The idea that externally maintained custom auth methods would be taken care of in such a way is quite far fetched in my view. I also disagree that they're coming along so quickly and so fast that it's actually difficult for us to include or support, we've covered quite a few, after all, as you seem to agree with. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Tue, 2022-03-15 at 12:27 -0700, samay sharma wrote: > This patch-set adds the following: > > * Allow multiple custom auth providers to be registered (Addressing > feedback from Aleksander and Andrew) > * Modify the test extension to use SCRAM to exchange secrets (Based > on Andres's suggestion) > * Add support for custom auth options to configure provider's > behavior (by exposing a new hook) (Required by OAUTHBEARER) > * Allow custom auth methods to use usermaps. (Required by > OAUTHBEARER) Review: * I retract my previous comment that "...it seems to only be useful with plaintext password authentication over an SSL connection"[0]. - it can be used with SCRAM, as you've shown, which could be useful for storing the credentials elsewhere - it can be used with one-time auth tokens, or short-lived auth tokens, obtained from some other service - it can be used with SASL to negotiate with special clients that support some other auth protocol (this is not shown, but having custom auth would make it interesting to experiment in this area) * There's a typo in the top comment for the test module, where you say that it lives in "contrib", but it actually lives in "src/test/modules". * Don't specify ".so" in shared_preload_libraries (in the test module). * Needs docs. * I'm wondering whether provider initialization/lookup might have a performance impact. Does it make sense to initialize the CustomAuthProvider once when parsing the hba.conf, and then cache it in the HbaLine or somewhere? * When specifying the provider in pg_hba.conf, I first made a mistake and specified it as "test_auth_provider" (which is the module name) rather than "test" (which is the provider name). Maybe the message could be slightly reworded in this case, like "no authentication provider registered with name %s"? As is, the emphasis seems to be on failing to load the library, which confused me at first. * Should be a check like "if (!process_shared_preload_libraries_in_progress) ereport(...)" in RegisterAuthProvider. * Could use some fuzz testing on the hba line parsing. For instance, I was able to specify "provider" twice. And if I specify "allow" first, then "provider" second, it fails (this is probably fine to require provider to be first, but needs to be documented/commented somewhere). * If you are intending for your custom provider to be open source, it would be helpful to show the code (list, github link, whatever), even if rough. That would ensure that it's really solving at least one real use case and we aren't forgetting something. * In general I like this patch. Trying to catch up on the rest of the discussion. Regards, Jeff Davis [0] https://www.postgresql.org/message-id/bfc55e8045453659df26cd60035bfbb4b9530052.ca...@j-davis.com
Re: Proposal: Support custom authentication methods using hooks
Hi, On Wed, Mar 16, 2022 at 8:02 AM Stephen Frost wrote: > Greetings, > > * samay sharma (smilingsa...@gmail.com) wrote: > > On Fri, Mar 4, 2022 at 11:15 AM Jacob Champion > wrote: > > > On Thu, 2022-03-03 at 11:12 +0100, Peter Eisentraut wrote: > > > > At the moment, it is not possible to judge whether the hook interface > > > > you have chosen is appropriate. > > > > > > > > I suggest you actually implement the Azure provider, then make the > hook > > > > interface, and then show us both and we can see what to do with it. > > > > > > To add a data point here, I've rebased my OAUTHBEARER experiment [1] on > > > top of this patchset. (That should work with Azure's OIDC provider, and > > > if it doesn't, I'd like to know why.) > > > > Firstly, thanks for doing this. It helps to have another data point and > the > > feedback you provided is very valuable. I've looked to address it with > the > > patchset attached to this email. > > > > This patch-set adds the following: > > > > * Allow multiple custom auth providers to be registered (Addressing > > feedback from Aleksander and Andrew) > > * Modify the test extension to use SCRAM to exchange secrets (Based on > > Andres's suggestion) > > * Add support for custom auth options to configure provider's behavior > (by > > exposing a new hook) (Required by OAUTHBEARER) > > * Allow custom auth methods to use usermaps. (Required by OAUTHBEARER) > > That we're having to extend this quite a bit to work for the proposed > OAUTH patches and that it still doesn't do anything for the client side > (note that the OAUTHBEARER patches are still patching libpq to add > support directly and surely will still be even with this latest patch > set ...) makes me seriously doubt that this is the right direction to be > going in. > I think I should have clarified this in my previous email. There is nothing specific to OAUTHBEARER in these changes. Having auth options and using usermaps is something which any auth method could require. I had not added them yet, but I'm pretty sure this would have been requested. > > > > After the port, here are the changes I still needed to carry in the > > > backend to get the tests passing: > > > > > > - I needed to add custom HBA options to configure the provider. > > > > Could you try to rebase your patch to use the options hook and let me > know > > if it satisfies your requirements? > > > > Please let me know if there's any other feedback. > > How about- if we just added OAUTH support directly into libpq and the > backend, would that work with Azure's OIDC provider? If not, why not? > Overall, Azure AD implements OIDC, so this could be doable. However, we'll have to explore further to see if the current implementation would satisfy all the requirements. > If it does, then what's the justification for trying to allow custom > backend-only authentication methods? > The only goal of this patch wasn't to enable support for Azure AD. That's just one client. Users might have a need to add or change auth methods in the future and providing that extensibility so we don't need to have core changes for each one of them would be useful. I know there isn't alignment on this yet, but if we'd like to move certain auth methods out of core into extensions, then this might provide a good framework for that. Regards, Samay > > Thanks, > > Stephen >
Re: Proposal: Support custom authentication methods using hooks
On Wed, 2022-03-16 at 11:02 -0400, Stephen Frost wrote: > That we're having to extend this quite a bit to work for the proposed > OAUTH patches and that it still doesn't do anything for the client > side > (note that the OAUTHBEARER patches are still patching libpq to add > support directly and surely will still be even with this latest patch > set ...) makes me seriously doubt that this is the right direction to > be > going in. I don't follow this concern. I assume you're referring to the last two bullet points, which I repeat below: * Add support for custom auth options to configure provider's behavior: Even without OAUTH I think this would have been requested. Configuration of some kind is going to be necessary. Without custom options, I guess the provider would need to define its own config file? Not the end of the world, but not ideal. * Allow custom auth methods to use usermaps: This is really just appending the "custom" method to a list of other methods that are allowed to specify a usermap in pg_hba.conf. Again, I don't see anything specific to OAUTH, and this would likely have been requested regardless. > How about- if we just added OAUTH support directly into libpq and the > backend, would that work with Azure's OIDC provider? If not, why > not? > If it does, then what's the justification for trying to allow custom > backend-only authentication methods? I understand the point, but it also has a flip side: if custom auth works, why do we need to block waiting for a complete core OAUTH feature? Authentication seems like a good candidate for allowing custom methods. New ones are always coming along, being used in new ways, updating to newer crypto, or falling out of favor. We've accumulated quite a few. Regards, Jeff Davis
Re: Proposal: Support custom authentication methods using hooks
Greetings, * samay sharma (smilingsa...@gmail.com) wrote: > On Fri, Mar 4, 2022 at 11:15 AM Jacob Champion wrote: > > On Thu, 2022-03-03 at 11:12 +0100, Peter Eisentraut wrote: > > > At the moment, it is not possible to judge whether the hook interface > > > you have chosen is appropriate. > > > > > > I suggest you actually implement the Azure provider, then make the hook > > > interface, and then show us both and we can see what to do with it. > > > > To add a data point here, I've rebased my OAUTHBEARER experiment [1] on > > top of this patchset. (That should work with Azure's OIDC provider, and > > if it doesn't, I'd like to know why.) > > Firstly, thanks for doing this. It helps to have another data point and the > feedback you provided is very valuable. I've looked to address it with the > patchset attached to this email. > > This patch-set adds the following: > > * Allow multiple custom auth providers to be registered (Addressing > feedback from Aleksander and Andrew) > * Modify the test extension to use SCRAM to exchange secrets (Based on > Andres's suggestion) > * Add support for custom auth options to configure provider's behavior (by > exposing a new hook) (Required by OAUTHBEARER) > * Allow custom auth methods to use usermaps. (Required by OAUTHBEARER) That we're having to extend this quite a bit to work for the proposed OAUTH patches and that it still doesn't do anything for the client side (note that the OAUTHBEARER patches are still patching libpq to add support directly and surely will still be even with this latest patch set ...) makes me seriously doubt that this is the right direction to be going in. > > After the port, here are the changes I still needed to carry in the > > backend to get the tests passing: > > > > - I needed to add custom HBA options to configure the provider. > > Could you try to rebase your patch to use the options hook and let me know > if it satisfies your requirements? > > Please let me know if there's any other feedback. How about- if we just added OAUTH support directly into libpq and the backend, would that work with Azure's OIDC provider? If not, why not? If it does, then what's the justification for trying to allow custom backend-only authentication methods? Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Hi, On Fri, Mar 4, 2022 at 11:15 AM Jacob Champion wrote: > On Thu, 2022-03-03 at 11:12 +0100, Peter Eisentraut wrote: > > At the moment, it is not possible to judge whether the hook interface > > you have chosen is appropriate. > > > > I suggest you actually implement the Azure provider, then make the hook > > interface, and then show us both and we can see what to do with it. > > To add a data point here, I've rebased my OAUTHBEARER experiment [1] on > top of this patchset. (That should work with Azure's OIDC provider, and > if it doesn't, I'd like to know why.) > Firstly, thanks for doing this. It helps to have another data point and the feedback you provided is very valuable. I've looked to address it with the patchset attached to this email. This patch-set adds the following: * Allow multiple custom auth providers to be registered (Addressing feedback from Aleksander and Andrew) * Modify the test extension to use SCRAM to exchange secrets (Based on Andres's suggestion) * Add support for custom auth options to configure provider's behavior (by exposing a new hook) (Required by OAUTHBEARER) * Allow custom auth methods to use usermaps. (Required by OAUTHBEARER) > After the port, here are the changes I still needed to carry in the > backend to get the tests passing: > > - I needed to add custom HBA options to configure the provider. > Could you try to rebase your patch to use the options hook and let me know if it satisfies your requirements? Please let me know if there's any other feedback. Regards, Samay > - I needed to declare usermap support so that my provider could > actually use check_usermap(). - I had to modify the SASL mechanism registration to allow a custom > maximum message length, but I think that's not the job of Samay's > proposal to fix; it's just a needed improvement to CheckSASLAuth(). > > Obviously, the libpq frontend still needs to understand how to speak > the new SASL mechanism. There are third-party SASL implementations that > are plugin-based, which could potentially ease the pain here, at the > expense of a major dependency and a very new distribution model. > > --Jacob > > [1] > https://www.postgresql.org/message-id/flat/d1b467a78e0e36ed85a09adf979d04cf124a9d4b.camel%40vmware.com > v3-0001-Add-support-for-custom-authentication-methods.patch Description: Binary data v3-0004-Add-support-for-map-and-custom-auth-options.patch Description: Binary data v3-0002-Add-sample-extension-to-test-custom-auth-provider.patch Description: Binary data v3-0003-Add-tests-for-test_auth_provider-extension.patch Description: Binary data
Re: Proposal: Support custom authentication methods using hooks
On Tue, Mar 8, 2022 at 9:28 PM Stephen Frost wrote: > > Greetings, > > * Jeff Davis (pg...@j-davis.com) wrote: > > On Wed, 2022-03-02 at 10:54 -0500, Stephen Frost wrote: > > > It's our decision what we want to support and maintain in the code > > > base > > > and what we don't. > > > > That might be an argument in favor of custom auth methods, because we > > could move built-in methods that we don't like into a contrib module > > that implements it as a custom auth method. > > Feel like I already answered this but just to be clear- I don't view > that as actually addressing the issue since we'd still be maintaining > and distributing insecure auth methods. +1. And contrib, in particular, is already a mix of very important, stable ad useful things, and things that are just pure testing or examples that nobody in their right mind should use. Putting something security related there seems like a terrible idea on it's own, independent from shipping things that are known insecure. (yes, I know sepgsql it there. Which certainly doesn't help tell people if it's something that could be relied on or not) -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Wed, 2022-03-02 at 10:54 -0500, Stephen Frost wrote: > > It's our decision what we want to support and maintain in the code > > base > > and what we don't. > > That might be an argument in favor of custom auth methods, because we > could move built-in methods that we don't like into a contrib module > that implements it as a custom auth method. Feel like I already answered this but just to be clear- I don't view that as actually addressing the issue since we'd still be maintaining and distributing insecure auth methods. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks,Re: Proposal: Support custom authentication methods using hooks
On Fri, Mar 4, 2022 at 6:03 PM Tatsuo Ishii wrote: > > >> I still don't understand why using plaintex password authentication > >> over SSL connection is considered insecure. Actually we have been > >> stating opposite in the manual: > >> https://www.postgresql.org/docs/14/auth-password.html > >> > >> "If the connection is protected by SSL encryption then password can be > >> used safely, though." > > > > If you aren't doing client verification (i.e., cert in pg_hba) and are > > not doing verify-full on the client side then a man-in-the-middle > > attack on TLS is trivial, and the plaintext password will be > > sniffable. > > So the plaintext password is safe if used with hostssl + verify-full > (server side) and sslmode = verify-full (client side), right? > That would be safe-in-transit so long as everything was configured properly and all certificates were protected. Unfortunately PG doesn't make this incredibly easy to implement, it allows only 1 client root cert, the client side doesn't understand system certificate stores or PKI, etc. Further, if someone gains access to the password hashes there is still a pass-the-hash vulnerability, though.
Re: Proposal: Support custom authentication methods using hooks
On Wed, 2022-03-02 at 10:54 -0500, Stephen Frost wrote: > It's our decision what we want to support and maintain in the code > base > and what we don't. That might be an argument in favor of custom auth methods, because we could move built-in methods that we don't like into a contrib module that implements it as a custom auth method. Regards, Jeff Davis
Re: Proposal: Support custom authentication methods using hooks,Re: Proposal: Support custom authentication methods using hooks
>> I still don't understand why using plaintex password authentication >> over SSL connection is considered insecure. Actually we have been >> stating opposite in the manual: >> https://www.postgresql.org/docs/14/auth-password.html >> >> "If the connection is protected by SSL encryption then password can be >> used safely, though." > > If you aren't doing client verification (i.e., cert in pg_hba) and are > not doing verify-full on the client side then a man-in-the-middle > attack on TLS is trivial, and the plaintext password will be > sniffable. So the plaintext password is safe if used with hostssl + verify-full (server side) and sslmode = verify-full (client side), right? Best reagards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: Proposal: Support custom authentication methods using hooks
On Thu, 2022-03-03 at 11:12 +0100, Peter Eisentraut wrote: > At the moment, it is not possible to judge whether the hook interface > you have chosen is appropriate. > > I suggest you actually implement the Azure provider, then make the hook > interface, and then show us both and we can see what to do with it. To add a data point here, I've rebased my OAUTHBEARER experiment [1] on top of this patchset. (That should work with Azure's OIDC provider, and if it doesn't, I'd like to know why.) After the port, here are the changes I still needed to carry in the backend to get the tests passing: - I needed to add custom HBA options to configure the provider. - I needed to declare usermap support so that my provider could actually use check_usermap(). - I had to modify the SASL mechanism registration to allow a custom maximum message length, but I think that's not the job of Samay's proposal to fix; it's just a needed improvement to CheckSASLAuth(). Obviously, the libpq frontend still needs to understand how to speak the new SASL mechanism. There are third-party SASL implementations that are plugin-based, which could potentially ease the pain here, at the expense of a major dependency and a very new distribution model. --Jacob [1] https://www.postgresql.org/message-id/flat/d1b467a78e0e36ed85a09adf979d04cf124a9d4b.camel%40vmware.com
Re: Proposal: Support custom authentication methods using hooks,Re: Proposal: Support custom authentication methods using hooks
On Thu, Mar 3, 2022 at 11:50 PM Tatsuo Ishii wrote: > > >> So, dropping plaintext password authentication support from libpq will > >> make it impossible for users to use the former method. > > > > Yes, just like dropping support for md5 would make it impossible for > > users to have their passwords be hashed with md5, which is an altogether > > good thing considering how easy it is to brute-force md5 these days. > > I still don't understand why using plaintex password authentication > over SSL connection is considered insecure. Actually we have been > stating opposite in the manual: > https://www.postgresql.org/docs/14/auth-password.html > > "If the connection is protected by SSL encryption then password can be > used safely, though." If you aren't doing client verification (i.e., cert in pg_hba) and are not doing verify-full on the client side then a man-in-the-middle attack on TLS is trivial, and the plaintext password will be sniffable. The documentation should be updated to say under no circumstances is this safe.
Re: Proposal: Support custom authentication methods using hooks,Re: Proposal: Support custom authentication methods using hooks
>> So, dropping plaintext password authentication support from libpq will >> make it impossible for users to use the former method. > > Yes, just like dropping support for md5 would make it impossible for > users to have their passwords be hashed with md5, which is an altogether > good thing considering how easy it is to brute-force md5 these days. I still don't understand why using plaintex password authentication over SSL connection is considered insecure. Actually we have been stating opposite in the manual: https://www.postgresql.org/docs/14/auth-password.html "If the connection is protected by SSL encryption then password can be used safely, though." Best reagards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: Proposal: Support custom authentication methods using hooks
On Thu, Mar 3, 2022 at 12:38:32PM -0500, Jonathan Katz wrote: > On 3/3/22 12:23 PM, Bruce Momjian wrote: > > On Thu, Mar 3, 2022 at 10:45:42AM +0100, Peter Eisentraut wrote: > > > On 02.03.22 16:45, Jonathan S. Katz wrote: > > > > By that argument, we should have kept "password" (plain) as an > > > > authentication method. > > > > > > For comparison, the time between adding md5 and removing password was 16 > > > years. It has been 5 years since scram was added. > > > > Uh, when did we remove "password". I still see it mentioned in > > pg_hba.conf. Am I missing something? > > I may have explained this wrong. The protocol still supports "plain" but we > removed the ability to store passwords in plaintext: > > "Remove the ability to store unencrypted passwords on the server > > "The password_encryption server parameter no longer supports off or plain. > The UNENCRYPTED option is no longer supported in CREATE/ALTER USER ... > PASSWORD. Similarly, the --unencrypted option has been removed from > createuser. Unencrypted passwords migrated from older versions will be > stored encrypted in this release. The default setting for > password_encryption is still md5." OK, that does make sense. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Peter Eisentraut (peter.eisentr...@enterprisedb.com) wrote: > On 02.03.22 21:49, samay sharma wrote: > >I think we are discussing two topics in this thread which in my opinion > >are orthogonal. > > > >(a) Should we make authentication methods pluggable by exposing these > >hooks? - These will allow users to add plugins of their own to support > >whatever auth method they like. One immediate use case (and what prompted > >me to start looking at this) is Azure Active Directory integration which > >is a common request from Azure customers. We could also, over time, move > >some of our existing auth methods into extensions if we don’t want to > >maintain them in core. > > I don't think people are necessarily opposed to that. I'm not thrilled with it, at least. It's not clear that just backend hooks would be enough either- certainly a number of our existing mechanisms require support in libpq and those are generally the ones that are more secure too (GSSAPI, Certs), so how would that work with something that's 'pluggable'? Are we going to have libpq loading in libraries too? > At the moment, it is not possible to judge whether the hook interface you > have chosen is appropriate. Agreed. > I suggest you actually implement the Azure provider, then make the hook > interface, and then show us both and we can see what to do with it. Better- implement a standard that's also supported by Azure and not something proprietary that can't be evaluated or which hasn't been reviewed by security experts. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On 3/3/22 12:23 PM, Bruce Momjian wrote: On Thu, Mar 3, 2022 at 10:45:42AM +0100, Peter Eisentraut wrote: On 02.03.22 16:45, Jonathan S. Katz wrote: By that argument, we should have kept "password" (plain) as an authentication method. For comparison, the time between adding md5 and removing password was 16 years. It has been 5 years since scram was added. Uh, when did we remove "password". I still see it mentioned in pg_hba.conf. Am I missing something? I may have explained this wrong. The protocol still supports "plain" but we removed the ability to store passwords in plaintext: "Remove the ability to store unencrypted passwords on the server "The password_encryption server parameter no longer supports off or plain. The UNENCRYPTED option is no longer supported in CREATE/ALTER USER ... PASSWORD. Similarly, the --unencrypted option has been removed from createuser. Unencrypted passwords migrated from older versions will be stored encrypted in this release. The default setting for password_encryption is still md5." Jonathan [1] https://www.postgresql.org/docs/release/10.0/ OpenPGP_signature Description: OpenPGP digital signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Tatsuo Ishii (is...@sraoss.co.jp) wrote: > > Yes, really, it's a known-broken system which suffers from such an old > > and well known attack that it's been given a name: pass-the-hash. As > > was discussed on this thread even, just the fact that it's not trivial > > to break on the wire doesn't make it not-broken, particularly when we > > use the username (which is rather commonly the same one used across > > multiple systems..) as the salt. Worse, md5 isn't exactly the pinnacle > > I am not a big fan of md5 auth but saying that md5 auth uses username > as the salt is oversimplified. The md5 hashed password shored in > pg_shadow is created as md5(password + username). But the md5 hashed > password flying over wire is using a random salt like md5(md5(password > + username) + random_salt). Err, no, it's not oversimplified at all- we do, in fact, as you say above, use the username as the salt for what gets stored in pg_authid (pg_shadow is just a view). That's absolutely a problem because servers can be compromised, backups can be compromised, and when it comes to PG servers you don't even need to actually bother cracking the password once you've gained access to an md5 value in pg_authid anyway. Yes, we do use a challenge/response over the wire but that doesn't absolve us of the fact that the hashes we store in pg_authid with the md5 method is subject to pass-the-hash and brute-force attacks against it. If anything, the challenge/response over the wire is less useful considering the common usage of TLS these days. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Thu, Mar 3, 2022 at 10:45:42AM +0100, Peter Eisentraut wrote: > On 02.03.22 16:45, Jonathan S. Katz wrote: > > By that argument, we should have kept "password" (plain) as an > > authentication method. > > For comparison, the time between adding md5 and removing password was 16 > years. It has been 5 years since scram was added. Uh, when did we remove "password". I still see it mentioned in pg_hba.conf. Am I missing something? -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: Proposal: Support custom authentication methods using hooks,Re: Proposal: Support custom authentication methods using hooks
Greetings, * Tatsuo Ishii (is...@sraoss.co.jp) wrote: > > On 2/25/22 12:39 PM, Tom Lane wrote: > >> Jeff Davis writes: > >>> On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: > ... and, since we can't readily enforce that the client only sends > those cleartext passwords over suitably-encrypted connections, this > could easily be a net negative for security. Not sure that I think > it's a good idea. > >> > >>> I don't understand your point. Can't you just use "hostssl" rather > >>> than > >>> "host"? > >> My point is that sending cleartext passwords over the wire is an > >> insecure-by-definition protocol that we shouldn't be encouraging > >> more use of. > > > > This is my general feeling as well. We just spent a bunch of effort > > adding, refining, and making SCRAM the default method. I think doing > > anything that would drive more use of sending plaintext passwords, > > even over TLS, is counter to that. > > There's at least one use case to use plaintext passwords. Pgpool-II > accepts plaintext passwords from frontend (from frontend's point of > view, it looks as if the frontend speaks with PostgreSQL server which > requests the plaintext password authentication), then negotiates with > backends regarding authentication method they demand. Suppose we have > 2 PostgreSQL clusters and they require md5 auth. They send different > password encryption salt and Pgpool-II deal with each server using the > salt and password. So Pgpool-II needs plaintext password. Same thing > can be said to SCRAM-SHA-256 authentication because it's kind of > challenge/response based authentication. Passing around plaintext passwords isn't good, regardless of what's doing it. That some things do that today is a problem, not something that should be held up as an example use-case that we should be retaining support for. > Actually it is possible for Pgpool-II to not use plaintext passwords > reading from frontend. In this case passwords are stored in a file and > Pgpool-II reads passwords from the file. But this is annoying for > users because they have to sync the passwords stored in PostgreSQL > with the passwords stored in the file. Users authenticating to an application and then independently having applications authenticate to the database server is actually rather common. I agree that proxying credentials is generally better and I'm hoping to commit support for exactly that during this CF for GSSAPI (aka Kerberos), where it's cleanly supported. Proxying using plaintext passwords isn't good though. > So, dropping plaintext password authentication support from libpq will > make it impossible for users to use the former method. Yes, just like dropping support for md5 would make it impossible for users to have their passwords be hashed with md5, which is an altogether good thing considering how easy it is to brute-force md5 these days. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Thu, Mar 3, 2022 at 4:45 AM Peter Eisentraut wrote: > > On 02.03.22 16:45, Jonathan S. Katz wrote: > > By that argument, we should have kept "password" (plain) as an > > authentication method. > > For comparison, the time between adding md5 and removing password was 16 > years. It has been 5 years since scram was added. It's been 7 years since this thread: https://www.postgresql.org/message-id/54dbcbcf.9000...@vmware.com As Jonathan and Stephen and others have said, anyone who wishes to continue using MD5 or other plaintext methods can keep doing that for 5 more years with a supported version of PG. There is no excuse to leave well known, flawed mechanisms in PG16.
Re: Proposal: Support custom authentication methods using hooks
On 02.03.22 21:49, samay sharma wrote: I think we are discussing two topics in this thread which in my opinion are orthogonal. (a) Should we make authentication methods pluggable by exposing these hooks? - These will allow users to add plugins of their own to support whatever auth method they like. One immediate use case (and what prompted me to start looking at this) is Azure Active Directory integration which is a common request from Azure customers. We could also, over time, move some of our existing auth methods into extensions if we don’t want to maintain them in core. I don't think people are necessarily opposed to that. At the moment, it is not possible to judge whether the hook interface you have chosen is appropriate. I suggest you actually implement the Azure provider, then make the hook interface, and then show us both and we can see what to do with it. One thing that has been requested, and I would support that, is that a plugged-in authentication method should look like a built-in one. So for example it should be able to register a real name, instead of "custom". I think a fair bit of refactoring work might be appropriate in order to make the authentication code more modular.
Re: Proposal: Support custom authentication methods using hooks
On 02.03.22 15:16, Jonathan S. Katz wrote: What are the reasons they are still purposely using it? The ones I have seen/heard are: - Using an older driver - On a pre-v10 PG - Unaware of SCRAM Another reason is that SCRAM presents subtle operational issues in distributed systems. As someone who is involved with products such as pgbouncer and bdr, I am aware that there are still unresolved problems and ongoing research in that area. Maybe they can all be solved eventually, even if it is concluding "you can't do that anymore" in certain cases, but it's not all solved yet, and falling back to the best-method-before-this-one is a useful workaround. I'm thinking there might be room for an authentication method between plain and scram that is less complicated and allows distributed systems to be set up more easily. I don't know what that would be, but I don't think we should prohibit the consideration of "anything less than SCRAM". I notice that a lot of internet services are promoting "application passwords" nowadays. I don't know the implementation details of that, but it appears that the overall idea is to have instead of one high-value password have many frequently generated medium-value passwords. We also have a recent proposal to store multiple passwords per user. (Obviously that could apply to SCRAM and not-SCRAM equally.) That's the kind of direction I would like to explore.
Re: Proposal: Support custom authentication methods using hooks
On 02.03.22 16:45, Jonathan S. Katz wrote: By that argument, we should have kept "password" (plain) as an authentication method. For comparison, the time between adding md5 and removing password was 16 years. It has been 5 years since scram was added.
Re: Proposal: Support custom authentication methods using hooks
On 02.03.22 21:26, Stephen Frost wrote: Part of the point, for my part anyway, of dropping support for plaintext transmission would be to remove support for that from libpq, otherwise a compromised server could still potentially convince a client to provide a plaintext password be sent to it. I think there should be a generalized feature for client-side selecting or filtering of authentication methods. As long as there exists more than one method, there will be tradeoffs and users might want to avoid one or the other. I don't think removing a method outright is the right solution for that.
Re: Proposal: Support custom authentication methods using hooks
On Wed, Mar 02, 2022 at 11:15:28AM -0500, Stephen Frost wrote: > With... which? We removed recovery.conf without any warning between > major releases, yet it was used by every single PG file-based backup and > restore solution out there and by every single organization that had > ever done a restore of PG since it was introduced in 8.0. There was much more to the changes around recovery.conf, with the integration of its parameters as GUCs so it had a lot of benefits, and that's why it has baked for 3~4 years as far as I recall. There is SCRAM as a replacement of MD5 already in core, but as Jonathan said upthread the upgrade from an instance that still has MD5 hashes may finish by being tricky for some users because this does not concern only pg_upgrade (this could fail if it detects MD5 hashes in pg_authid), and we don't really know how to solve the pg_dump bit, do we? And it seems to me that this is not a matter of just blocking the load of MD5 hashes in the backend in the case of logical dumps. > Passing > around cleartext passwords with the LDAP authentication method is > clearly bad from a security perspective and it's a bunch of code to > support that, along with it being quite complicated to configure and get > set up (arguably harder than Kerberos, if you want my 2c). If you want > to say that it's valuable for us to continue to maintain that code > because it's good and useful and might even be the only option for some > people, fine, though I disagree, but I don't think my argument that we > shouldn't keep it just because *someone* is using it is any different > from our general project policy about features. MD5 is still used at auth method by many people, as is LDAP. They have their design flaws (backend LDAP, MD5 hashes in old backups), but those code areas are pretty mature as well, so a simple removal could hurt the user base of PG quite a lot IMO. -- Michael signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
> Yes, really, it's a known-broken system which suffers from such an old > and well known attack that it's been given a name: pass-the-hash. As > was discussed on this thread even, just the fact that it's not trivial > to break on the wire doesn't make it not-broken, particularly when we > use the username (which is rather commonly the same one used across > multiple systems..) as the salt. Worse, md5 isn't exactly the pinnacle I am not a big fan of md5 auth but saying that md5 auth uses username as the salt is oversimplified. The md5 hashed password shored in pg_shadow is created as md5(password + username). But the md5 hashed password flying over wire is using a random salt like md5(md5(password + username) + random_salt). Best reagards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: Proposal: Support custom authentication methods using hooks,Re: Proposal: Support custom authentication methods using hooks
> On 2/25/22 12:39 PM, Tom Lane wrote: >> Jeff Davis writes: >>> On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: ... and, since we can't readily enforce that the client only sends those cleartext passwords over suitably-encrypted connections, this could easily be a net negative for security. Not sure that I think it's a good idea. >> >>> I don't understand your point. Can't you just use "hostssl" rather >>> than >>> "host"? >> My point is that sending cleartext passwords over the wire is an >> insecure-by-definition protocol that we shouldn't be encouraging >> more use of. > > This is my general feeling as well. We just spent a bunch of effort > adding, refining, and making SCRAM the default method. I think doing > anything that would drive more use of sending plaintext passwords, > even over TLS, is counter to that. There's at least one use case to use plaintext passwords. Pgpool-II accepts plaintext passwords from frontend (from frontend's point of view, it looks as if the frontend speaks with PostgreSQL server which requests the plaintext password authentication), then negotiates with backends regarding authentication method they demand. Suppose we have 2 PostgreSQL clusters and they require md5 auth. They send different password encryption salt and Pgpool-II deal with each server using the salt and password. So Pgpool-II needs plaintext password. Same thing can be said to SCRAM-SHA-256 authentication because it's kind of challenge/response based authentication. Actually it is possible for Pgpool-II to not use plaintext passwords reading from frontend. In this case passwords are stored in a file and Pgpool-II reads passwords from the file. But this is annoying for users because they have to sync the passwords stored in PostgreSQL with the passwords stored in the file. So, dropping plaintext password authentication support from libpq will make it impossible for users to use the former method. Best reagards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-03-02 15:26:32 -0500, Stephen Frost wrote: > Part of the point, for my part anyway, of dropping support for plaintext > transmission would be to remove support for that from libpq, otherwise a > compromised server could still potentially convince a client to provide > a plaintext password be sent to it. IMO that's an argument for an opt-in option to permit plaintext, not an argument for removal of the code alltogether. Even that will need a long transition time, because it's effectively a form of an ABI break. Upgrading libpq will suddenly cause applications to stop working. So adding an opt-out option to disable plaintext is the next step... I don't think it makes sense to discuss this topic as part of this thread really. It seems wholly independent of making authentication pluggable. > I also just generally disagree with the idea that it makes sense for > these things to be in contrib. We should be dropping them because > they're insecure- moving them to contrib doesn't change the issue that > we're distributing authentication solutions that send (either through an > encrypted tunnel, or not, neither is good) that pass plaintext passwords > around. Shrug. I don't think it's a good idea to just leave people hanging without a replacement. It's OK to make it a bit harder and require explicit configuration, but dropping support for reasonable configurations IMO is something we should be very hesitant doing. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Hi, On Wed, Mar 2, 2022 at 12:32 AM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > On 01.03.22 22:34, Andres Freund wrote: > > The cases I've heard about are about centralizing auth across multiple > cloud > > services. Including secret management in some form. E.g. allowing an > > application to auth to postgres, redis and having the secret provided by > > infrastructure, rather than having to hardcode it in config or such. > > > > I can't see application developers configuring kerberos and I don't think > > LDAP, PAM, Radius are a great answer either, due to the plaintext > requirement > > alone? LDAP is pretty clearly dying technology, PAM is fragile > complicated C > > stuff that's not portable across OSs. Radius is probably the most > realistic, > > but at least as implemented doesn't seem flexible enough (e.g. no access > to > > group memberships etc). > > > > Nor does baking stuff like that in seem realistic to me, it'll > presumably be > > too cloud provider specific. > > Let's gather some more information on this. PostgreSQL should support > the authentication that many people want to use out of the box. I don't > think it would be good to be at a point where all the built-in methods > are outdated and if you want to use the good stuff you have to hunt for > plugins. The number of different cloud APIs is effectively small. I > expect that there are a lot of similarities, like they probably all need > support for http calls, they might need support for caching lookups, > etc. OIDC was mentioned elsewhere. That's a standard. Is that > compatible with any cloud providers? Would that be enough for many users? > I think we are discussing two topics in this thread which in my opinion are orthogonal. (a) Should we make authentication methods pluggable by exposing these hooks? - These will allow users to add plugins of their own to support whatever auth method they like. One immediate use case (and what prompted me to start looking at this) is Azure Active Directory integration which is a common request from Azure customers. We could also, over time, move some of our existing auth methods into extensions if we don’t want to maintain them in core. Please note that these hooks do not restrict auth providers to use only plaintext auth methods. We could do SCRAM with secrets which are stored outside of Postgres using this auth plugin too. I can modify the test_auth_provider sample extension to do something like that as Andres suggested. I don't see much point in that unless we deprecate *all* the > auth methods that transmit a cleartext password. (b) Should we allow plaintext auth methods (and should we deprecate a few currently supported auth methods which use plaintext exchange)? - I think this is also a very important discussion but has many factors to consider independent of the hooks. Whatever decision is made here, we can impose that in auth.c later for plugins. For eg. allow plugins to only do plaintext stuff with SSL enabled (by checking if Port->ssl_in_use), or if we remove AUTH_REQ_PASSWORD, then plugins any way can’t use plaintext exchange with the client. So, overall, if we are open to allowing plugins for auth methods, I can proceed to modify the test_auth_provider extension to use SCRAM and allow registering multiple providers for this specific change. Regards, Samay
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-03-02 09:32:26 +0100, Peter Eisentraut wrote: > > On 01.03.22 22:34, Andres Freund wrote: > > > The cases I've heard about are about centralizing auth across multiple > > > cloud > > > services. Including secret management in some form. E.g. allowing an > > > application to auth to postgres, redis and having the secret provided by > > > infrastructure, rather than having to hardcode it in config or such. > > > > > > I can't see application developers configuring kerberos and I don't think > > > LDAP, PAM, Radius are a great answer either, due to the plaintext > > > requirement > > > alone? LDAP is pretty clearly dying technology, PAM is fragile > > > complicated C > > > stuff that's not portable across OSs. Radius is probably the most > > > realistic, > > > but at least as implemented doesn't seem flexible enough (e.g. no access > > > to > > > group memberships etc). > > > > > > Nor does baking stuff like that in seem realistic to me, it'll presumably > > > be > > > too cloud provider specific. > > > > Let's gather some more information on this. PostgreSQL should support the > > authentication that many people want to use out of the box. I don't think > > it would be good to be at a point where all the built-in methods are > > outdated and if you want to use the good stuff you have to hunt for plugins. > > The number of different cloud APIs is effectively small. I expect that > > there are a lot of similarities, like they probably all need support for > > http calls, they might need support for caching lookups, etc. OIDC was > > mentioned elsewhere. That's a standard. Is that compatible with any cloud > > providers? Would that be enough for many users? > > I'm not opposed to putting something into the source tree eventually, if we > can figure out an overlapping set of capabilities that's useful enough. But I > don't see that as a reason to not make auth extensible? If anything, the > contrary - it's going to be much easier to figure out what this should look > like if it can be iteratively worked on with unmodified postgres. > > Even if we were to put something in core, it seems that contrib/ would be a > better place than auth.c for something like it. > > I think we should consider moving pam, ldap, radius to contrib > eventually. That way we'd not need to entirely remove them, but a "default" > postgres wouldn't have support for auth methods requiring plaintext secret > transmission. Part of the point, for my part anyway, of dropping support for plaintext transmission would be to remove support for that from libpq, otherwise a compromised server could still potentially convince a client to provide a plaintext password be sent to it. I also just generally disagree with the idea that it makes sense for these things to be in contrib. We should be dropping them because they're insecure- moving them to contrib doesn't change the issue that we're distributing authentication solutions that send (either through an encrypted tunnel, or not, neither is good) that pass plaintext passwords around. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-03-02 09:32:26 +0100, Peter Eisentraut wrote: > On 01.03.22 22:34, Andres Freund wrote: > > The cases I've heard about are about centralizing auth across multiple cloud > > services. Including secret management in some form. E.g. allowing an > > application to auth to postgres, redis and having the secret provided by > > infrastructure, rather than having to hardcode it in config or such. > > > > I can't see application developers configuring kerberos and I don't think > > LDAP, PAM, Radius are a great answer either, due to the plaintext > > requirement > > alone? LDAP is pretty clearly dying technology, PAM is fragile complicated C > > stuff that's not portable across OSs. Radius is probably the most realistic, > > but at least as implemented doesn't seem flexible enough (e.g. no access to > > group memberships etc). > > > > Nor does baking stuff like that in seem realistic to me, it'll presumably be > > too cloud provider specific. > > Let's gather some more information on this. PostgreSQL should support the > authentication that many people want to use out of the box. I don't think > it would be good to be at a point where all the built-in methods are > outdated and if you want to use the good stuff you have to hunt for plugins. > The number of different cloud APIs is effectively small. I expect that > there are a lot of similarities, like they probably all need support for > http calls, they might need support for caching lookups, etc. OIDC was > mentioned elsewhere. That's a standard. Is that compatible with any cloud > providers? Would that be enough for many users? I'm not opposed to putting something into the source tree eventually, if we can figure out an overlapping set of capabilities that's useful enough. But I don't see that as a reason to not make auth extensible? If anything, the contrary - it's going to be much easier to figure out what this should look like if it can be iteratively worked on with unmodified postgres. Even if we were to put something in core, it seems that contrib/ would be a better place than auth.c for something like it. I think we should consider moving pam, ldap, radius to contrib eventually. That way we'd not need to entirely remove them, but a "default" postgres wouldn't have support for auth methods requiring plaintext secret transmission. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Wed, Mar 2, 2022 at 10:54:27AM -0500, Stephen Frost wrote: > > It's our decision what we want to support and maintain in the code base > > and what we don't. Folks often ask for things that we don't or won't > > support and this isn't any different from that. We also remove things > > on a rather regular basis even when they're being used- generally > > because we have something better, as we do here. I disagree that an > > argument of 'some people use it so we can't remove it' holds any weight > > here. > > I disagree. With... which? We removed recovery.conf without any warning between major releases, yet it was used by every single PG file-based backup and restore solution out there and by every single organization that had ever done a restore of PG since it was introduced in 8.0. Passing around cleartext passwords with the LDAP authentication method is clearly bad from a security perspective and it's a bunch of code to support that, along with it being quite complicated to configure and get set up (arguably harder than Kerberos, if you want my 2c). If you want to say that it's valuable for us to continue to maintain that code because it's good and useful and might even be the only option for some people, fine, though I disagree, but I don't think my argument that we shouldn't keep it just because *someone* is using it is any different from our general project policy about features. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Wed, Mar 2, 2022 at 10:54:27AM -0500, Stephen Frost wrote: > It's our decision what we want to support and maintain in the code base > and what we don't. Folks often ask for things that we don't or won't > support and this isn't any different from that. We also remove things > on a rather regular basis even when they're being used- generally > because we have something better, as we do here. I disagree that an > argument of 'some people use it so we can't remove it' holds any weight > here. I disagree. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Wed, Mar 2, 2022 at 10:29:45AM -0500, Stephen Frost wrote: > > We don't require SSL to be used with them..? Further, as already > > discussed on this thread, SSL only helps with on-the-wire, doesn't > > address the risk of a compromised server. LDAP, in particular, is > > terrible in this regard because it's a centralized password system, > > meaning that one compromised server will lead to an attacker gaining > > full access to the victim's account throughout the enterprise. > > Yes, but the site chose LDAP, and I don't think it is our place to tell > them what to use. It's our decision what we want to support and maintain in the code base and what we don't. Folks often ask for things that we don't or won't support and this isn't any different from that. We also remove things on a rather regular basis even when they're being used- generally because we have something better, as we do here. I disagree that an argument of 'some people use it so we can't remove it' holds any weight here. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Wed, Mar 2, 2022 at 10:29:45AM -0500, Stephen Frost wrote: > We don't require SSL to be used with them..? Further, as already > discussed on this thread, SSL only helps with on-the-wire, doesn't > address the risk of a compromised server. LDAP, in particular, is > terrible in this regard because it's a centralized password system, > meaning that one compromised server will lead to an attacker gaining > full access to the victim's account throughout the enterprise. Yes, but the site chose LDAP, and I don't think it is our place to tell them what to use. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: Proposal: Support custom authentication methods using hooks
On 3/2/22 10:30 AM, Stephen Frost wrote: Greetings, * Peter Eisentraut (peter.eisentr...@enterprisedb.com) wrote: On 02.03.22 15:16, Jonathan S. Katz wrote: I find that a lot of people are still purposely using md5. Removing it now or in a year would be quite a disruption. What are the reasons they are still purposely using it? The ones I have seen/heard are: - Using an older driver - On a pre-v10 PG - Unaware of SCRAM I'm not really sure, but it seems like they are content with what they have and don't want to bother with the new fancy stuff. By that argument, we should have kept "password" (plain) as an authentication method. The specific use-cases I've presented are all solvable issues. The biggest challenging with existing users is the upgrade process, which is why I'd rather we begin a deprecation process and see if there are any ways we can make the md5 => SCRAM transition easier. There were lots and lots of folks who were comfortable with recovery.conf, yet we removed that without any qualms from one major version to the next. md5 will have had 5 years of overlap with scram. I do agree with Stephen in principle here. I encountered upgrade challenges in this an challenge with updating automation to handle this change. What I'm proposing above is to start the process of deprecating it as an auth method, which also allows to continue the education efforts to upgrae. Does that make sense? I'm not in favor of starting a process that will result in removal of the md5 method at this time. I am. +1 for starting this process. It may still take a few more years, but we should help our users to move away from an auth method with known issues. Thanks, Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: Proposal: Support custom authentication methods using hooks
On Wed, Mar 2, 2022 at 10:29 AM Stephen Frost wrote: > > Greetings, > > * Bruce Momjian (br...@momjian.us) wrote: > > On Wed, Mar 2, 2022 at 10:09:31AM -0500, Stephen Frost wrote: > > > I'm not sure that it's quite so simple. Perhaps we should also drop > > > LDAP and I don't really think PAM was ever terribly good for us to have, > > > but at least PAM and RADIUS could possibly be used with OTP solutions > > > (and maybe LDAP? Not sure, don't think I've seen that but perhaps..), > > > rendering sniffing of what's transmitted less valuable. We don't > > > support that for 'password' itself or for 'md5' in any serious way > > > though. > > > > I thought all the plain-password methods were already using SSL > > (hopefully with certificate authentication) and they were therefore > > safe. Why would we remove something like LDAP if that is what the site > > is already using? > > We don't require SSL to be used with them..? Further, as already > discussed on this thread, SSL only helps with on-the-wire, doesn't > address the risk of a compromised server. LDAP, in particular, is > terrible in this regard because it's a centralized password system, > meaning that one compromised server will lead to an attacker gaining > full access to the victim's account throughout the enterprise. I want to add support for the deprecation move, and I think/hope that my multi-password patchset can help make the transition less painful. I also want to throw out another existing issue with MD5, namely that the salt as the role is fundamentally problematic, even outside of trivial pass-the-hash attacks one could build a rainbow table today that uses 'postgres' as the salt, and get admin credentials that can be used for password stuffing attacks against other enterprise assets. This means PG is actively enabling lateral movement through enterprise systems if MD5 passwords are used.
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Peter Eisentraut (peter.eisentr...@enterprisedb.com) wrote: > On 02.03.22 15:16, Jonathan S. Katz wrote: > >>I find that a lot of people are still purposely using md5. Removing it > >>now or in a year would be quite a disruption. > > > >What are the reasons they are still purposely using it? The ones I have > >seen/heard are: > > > >- Using an older driver > >- On a pre-v10 PG > >- Unaware of SCRAM > > I'm not really sure, but it seems like they are content with what they have > and don't want to bother with the new fancy stuff. There were lots and lots of folks who were comfortable with recovery.conf, yet we removed that without any qualms from one major version to the next. md5 will have had 5 years of overlap with scram. > >What I'm proposing above is to start the process of deprecating it as an > >auth method, which also allows to continue the education efforts to > >upgrae. Does that make sense? > > I'm not in favor of starting a process that will result in removal of the > md5 method at this time. I am. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Wed, Mar 2, 2022 at 10:09:31AM -0500, Stephen Frost wrote: > > I'm not sure that it's quite so simple. Perhaps we should also drop > > LDAP and I don't really think PAM was ever terribly good for us to have, > > but at least PAM and RADIUS could possibly be used with OTP solutions > > (and maybe LDAP? Not sure, don't think I've seen that but perhaps..), > > rendering sniffing of what's transmitted less valuable. We don't > > support that for 'password' itself or for 'md5' in any serious way > > though. > > I thought all the plain-password methods were already using SSL > (hopefully with certificate authentication) and they were therefore > safe. Why would we remove something like LDAP if that is what the site > is already using? We don't require SSL to be used with them..? Further, as already discussed on this thread, SSL only helps with on-the-wire, doesn't address the risk of a compromised server. LDAP, in particular, is terrible in this regard because it's a centralized password system, meaning that one compromised server will lead to an attacker gaining full access to the victim's account throughout the enterprise. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On 02.03.22 15:16, Jonathan S. Katz wrote: I find that a lot of people are still purposely using md5. Removing it now or in a year would be quite a disruption. What are the reasons they are still purposely using it? The ones I have seen/heard are: - Using an older driver - On a pre-v10 PG - Unaware of SCRAM I'm not really sure, but it seems like they are content with what they have and don't want to bother with the new fancy stuff. What I'm proposing above is to start the process of deprecating it as an auth method, which also allows to continue the education efforts to upgrae. Does that make sense? I'm not in favor of starting a process that will result in removal of the md5 method at this time.
Re: Proposal: Support custom authentication methods using hooks
On Wed, Mar 2, 2022 at 10:09:31AM -0500, Stephen Frost wrote: > I'm not sure that it's quite so simple. Perhaps we should also drop > LDAP and I don't really think PAM was ever terribly good for us to have, > but at least PAM and RADIUS could possibly be used with OTP solutions > (and maybe LDAP? Not sure, don't think I've seen that but perhaps..), > rendering sniffing of what's transmitted less valuable. We don't > support that for 'password' itself or for 'md5' in any serious way > though. I thought all the plain-password methods were already using SSL (hopefully with certificate authentication) and they were therefore safe. Why would we remove something like LDAP if that is what the site is already using? -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Bruce Momjian (br...@momjian.us) wrote: > >> What is the logic to removing md5 but keeping 'password'? > > > I don't think we should keep 'password'. > > I don't see much point in that unless we deprecate *all* the > auth methods that transmit a cleartext password. I'm not sure that it's quite so simple. Perhaps we should also drop LDAP and I don't really think PAM was ever terribly good for us to have, but at least PAM and RADIUS could possibly be used with OTP solutions (and maybe LDAP? Not sure, don't think I've seen that but perhaps..), rendering sniffing of what's transmitted less valuable. We don't support that for 'password' itself or for 'md5' in any serious way though. We really should drop ident already though. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Stephen Frost writes: > * Bruce Momjian (br...@momjian.us) wrote: >> What is the logic to removing md5 but keeping 'password'? > I don't think we should keep 'password'. I don't see much point in that unless we deprecate *all* the auth methods that transmit a cleartext password. regards, tom lane
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Tue, Mar 1, 2022 at 08:31:19AM -0500, Stephen Frost wrote: > > > The last time I played with this area is the recent error handling > > > improvement with cryptohashes but MD5 has actually helped here in > > > detecting the problem as a patched OpenSSL would complain if trying to > > > use MD5 as hash function when FIPS is enabled. > > > > Having to continue to deal with md5 as an algorithm when it's known to > > be notably less secure and so much so that organizations essentially ban > > its use for exactly what we're using it for, in fact, another reason to > > Really? I thought it was publicly-visible MD5 hashes that were the > biggest problem. Our 32-bit salt during the connection is a problem, of > course. Neither are good. Not sure that we really need to spend a lot of effort trying to figure out which issue is the biggest problem. > > remove it, not a reason to keep it. Better code coverage testing of > > error paths is the answer to making sure that our error handling behaves > > properly. > > What is the logic to removing md5 but keeping 'password'? I don't think we should keep 'password'. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Tue, Mar 1, 2022 at 08:31:19AM -0500, Stephen Frost wrote: > > The last time I played with this area is the recent error handling > > improvement with cryptohashes but MD5 has actually helped here in > > detecting the problem as a patched OpenSSL would complain if trying to > > use MD5 as hash function when FIPS is enabled. > > Having to continue to deal with md5 as an algorithm when it's known to > be notably less secure and so much so that organizations essentially ban > its use for exactly what we're using it for, in fact, another reason to Really? I thought it was publicly-visible MD5 hashes that were the biggest problem. Our 32-bit salt during the connection is a problem, of course. > remove it, not a reason to keep it. Better code coverage testing of > error paths is the answer to making sure that our error handling behaves > properly. What is the logic to removing md5 but keeping 'password'? -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: Proposal: Support custom authentication methods using hooks
On 3/2/22 3:24 AM, Peter Eisentraut wrote: On 01.03.22 22:17, Jonathan S. Katz wrote: If you're moving to a newer version of PostgreSQL, you likely have to update your connection drivers anyway (rebuilt against new libpq, supporting any changes in the protocol, etc). I would prefer more data to support that argument, but this is generally what you need to do. However, we may need to step towards it. We took one step last release with defaulting to SCRAM. Perhaps this release we add a warning for anything using md5 auth that "this will be removed in a future release." (or specifically v16). We should also indicate in the docs that md5 is deprecated and will be removed. I find that a lot of people are still purposely using md5. Removing it now or in a year would be quite a disruption. What are the reasons they are still purposely using it? The ones I have seen/heard are: - Using an older driver - On a pre-v10 PG - Unaware of SCRAM What I'm proposing above is to start the process of deprecating it as an auth method, which also allows to continue the education efforts to upgrae. Does that make sense? It's also worth considering that keeping the code equipped to handle different kinds of password hashing would help it stay in shape if we ever need to add support for the next SHA after 256 or whatever. I think it's fine to keep the hashing code. The end goal is to remove the md5 authentication mechanism. Thanks, Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Peter Eisentraut (peter.eisentr...@enterprisedb.com) wrote: > On 01.03.22 22:34, Andres Freund wrote: > >The cases I've heard about are about centralizing auth across multiple cloud > >services. Including secret management in some form. E.g. allowing an > >application to auth to postgres, redis and having the secret provided by > >infrastructure, rather than having to hardcode it in config or such. > > > >I can't see application developers configuring kerberos and I don't think > >LDAP, PAM, Radius are a great answer either, due to the plaintext requirement > >alone? LDAP is pretty clearly dying technology, PAM is fragile complicated C > >stuff that's not portable across OSs. Radius is probably the most realistic, > >but at least as implemented doesn't seem flexible enough (e.g. no access to > >group memberships etc). > > > >Nor does baking stuff like that in seem realistic to me, it'll presumably be > >too cloud provider specific. > > Let's gather some more information on this. PostgreSQL should support the > authentication that many people want to use out of the box. I don't think > it would be good to be at a point where all the built-in methods are > outdated and if you want to use the good stuff you have to hunt for plugins. > The number of different cloud APIs is effectively small. I expect that > there are a lot of similarities, like they probably all need support for > http calls, they might need support for caching lookups, etc. OIDC was > mentioned elsewhere. That's a standard. Is that compatible with any cloud > providers? Would that be enough for many users? I tend to agree with this, and to that end I'd argue that we should probably be working to drop support for things like PAM, with appropriate code replacing any useful capabilities that folks were using it for (which would also mean it'd work across all the OS's we support, which would be nice..). Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-02-28 11:26:06 +0100, Peter Eisentraut wrote: > > We already have a variety of authentication mechanisms that support central > > management: LDAP, PAM, Kerberos, Radius. > > LDAP, PAM and Radius all require cleartext passwords, so aren't a great > solution based on the concerns voiced in this thread. IME Kerberos is > operationally too complicated to really be used, unless it's already part of > the operating environment. ... which is very, very, very often is, wrt Kerberos. > > What other mechanisms are people thinking about implementing using these > > hooks? > > The cases I've heard about are about centralizing auth across multiple cloud > services. Including secret management in some form. E.g. allowing an > application to auth to postgres, redis and having the secret provided by > infrastructure, rather than having to hardcode it in config or such. This sounds a lot like OAUTH or similar, which might be useful to consider adding if it can be done reasonably. > I can't see application developers configuring kerberos and I don't think > LDAP, PAM, Radius are a great answer either, due to the plaintext requirement > alone? LDAP is pretty clearly dying technology, PAM is fragile complicated C > stuff that's not portable across OSs. Radius is probably the most realistic, > but at least as implemented doesn't seem flexible enough (e.g. no access to > group memberships etc). Great thing about Kerberos is that, in general, application developers don't really need to do much to configure it. > Nor does baking stuff like that in seem realistic to me, it'll presumably be > too cloud provider specific. I don't quite buy an argument that hinges on the idea of centralized auth across multiple cloud services, as suggested above, while also being too cloud provider specific to be something that could be baked in, as said here. Maybe I'm misunderstanding. Also a bit concerned that adding hooks on presumptions about what some cloud provider needs isn't a good plan. In general though, I'm certainly more supportive of the idea of adding support for authentication mechanisms that are standardized and work across multiple cloud providers and services in general, than about adding things which are specific to one provider. I don't particularly care for the idea of adding hooks and then having cloud providers go off and develop their own proprietary authentication system that aren't standardized and which don't have public review, which seems like it's the use-case for adding them. I'm not dead-set against it, but it just doesn't strike me as a good area to add hooks for folks to use. Better would be baked in code that follows a well defined and reviewed RFC that anyone can look at and make sure follows the standard. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On 01.03.22 22:34, Andres Freund wrote: The cases I've heard about are about centralizing auth across multiple cloud services. Including secret management in some form. E.g. allowing an application to auth to postgres, redis and having the secret provided by infrastructure, rather than having to hardcode it in config or such. I can't see application developers configuring kerberos and I don't think LDAP, PAM, Radius are a great answer either, due to the plaintext requirement alone? LDAP is pretty clearly dying technology, PAM is fragile complicated C stuff that's not portable across OSs. Radius is probably the most realistic, but at least as implemented doesn't seem flexible enough (e.g. no access to group memberships etc). Nor does baking stuff like that in seem realistic to me, it'll presumably be too cloud provider specific. Let's gather some more information on this. PostgreSQL should support the authentication that many people want to use out of the box. I don't think it would be good to be at a point where all the built-in methods are outdated and if you want to use the good stuff you have to hunt for plugins. The number of different cloud APIs is effectively small. I expect that there are a lot of similarities, like they probably all need support for http calls, they might need support for caching lookups, etc. OIDC was mentioned elsewhere. That's a standard. Is that compatible with any cloud providers? Would that be enough for many users?
Re: Proposal: Support custom authentication methods using hooks
On 01.03.22 22:17, Jonathan S. Katz wrote: If you're moving to a newer version of PostgreSQL, you likely have to update your connection drivers anyway (rebuilt against new libpq, supporting any changes in the protocol, etc). I would prefer more data to support that argument, but this is generally what you need to do. However, we may need to step towards it. We took one step last release with defaulting to SCRAM. Perhaps this release we add a warning for anything using md5 auth that "this will be removed in a future release." (or specifically v16). We should also indicate in the docs that md5 is deprecated and will be removed. I find that a lot of people are still purposely using md5. Removing it now or in a year would be quite a disruption. It's also worth considering that keeping the code equipped to handle different kinds of password hashing would help it stay in shape if we ever need to add support for the next SHA after 256 or whatever.
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-02-25 13:40:54 -0500, Jonathan S. Katz wrote: > On 2/25/22 12:39 PM, Tom Lane wrote: > > My point is that sending cleartext passwords over the wire is an > > insecure-by-definition protocol that we shouldn't be encouraging > > more use of. > > This is my general feeling as well. We just spent a bunch of effort adding, > refining, and making SCRAM the default method. I think doing anything that > would drive more use of sending plaintext passwords, even over TLS, is > counter to that. I want to again emphasize that, as proposed, a custom auth method can use SCRAM if relevant for it, with a small amount of code. So the whole plaintext discussion seems independent. Samay, what do you think about updating the test plugin to do SCRAM instead of plaintext, just to highlight that fact? Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-02-28 11:26:06 +0100, Peter Eisentraut wrote: > We already have a variety of authentication mechanisms that support central > management: LDAP, PAM, Kerberos, Radius. LDAP, PAM and Radius all require cleartext passwords, so aren't a great solution based on the concerns voiced in this thread. IME Kerberos is operationally too complicated to really be used, unless it's already part of the operating environment. > What other mechanisms are people thinking about implementing using these > hooks? The cases I've heard about are about centralizing auth across multiple cloud services. Including secret management in some form. E.g. allowing an application to auth to postgres, redis and having the secret provided by infrastructure, rather than having to hardcode it in config or such. I can't see application developers configuring kerberos and I don't think LDAP, PAM, Radius are a great answer either, due to the plaintext requirement alone? LDAP is pretty clearly dying technology, PAM is fragile complicated C stuff that's not portable across OSs. Radius is probably the most realistic, but at least as implemented doesn't seem flexible enough (e.g. no access to group memberships etc). Nor does baking stuff like that in seem realistic to me, it'll presumably be too cloud provider specific. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
On 2/28/22 5:26 AM, Peter Eisentraut wrote: On 17.02.22 20:25, samay sharma wrote: A use case where this is useful are environments where you want authentication to be centrally managed across different services. This is a common deployment model for cloud providers where customers like to use single sign on and authenticate across different services including Postgres. Implementing this now is tricky as it requires syncing that authentication method's credentials with Postgres (and that gets trickier with TTL/expiry etc.). With these hooks, you can implement an extension to check credentials directly using the authentication provider's APIs. We already have a variety of authentication mechanisms that support central management: LDAP, PAM, Kerberos, Radius. What other mechanisms are people thinking about implementing using these hooks? Maybe there are a bunch of them, in which case a hook system might be sensible, but if there are only one or two plausible ones, we could also just make them built in. OIDC is the big one that comes to mind. Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: Proposal: Support custom authentication methods using hooks
On 3/1/22 8:31 AM, Stephen Frost wrote: Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: On Mon, Feb 28, 2022 at 04:42:55PM -0500, Stephen Frost wrote: Keeping it around will just push out the point at which everyone will finally be done with it, as there's really only two groups: those who have already moved to scram, and those who won't move until they want to upgrade to a release that doesn't have md5. FWIW, I am not sure if we are at this point yet. An extra reason to remove it would be that it is a support burden, but I don't have seen in recent memory any problems related to it that required any deep changes in the way to use it, and its code paths are independent. Ongoing reports that there's a known vulnerability aren't great to have to deal with. We can at least point people to scram but that's not great. The last time I played with this area is the recent error handling improvement with cryptohashes but MD5 has actually helped here in detecting the problem as a patched OpenSSL would complain if trying to use MD5 as hash function when FIPS is enabled. Having to continue to deal with md5 as an algorithm when it's known to be notably less secure and so much so that organizations essentially ban its use for exactly what we're using it for, in fact, another reason to remove it, not a reason to keep it. Better code coverage testing of error paths is the answer to making sure that our error handling behaves properly. So, I generally agree with Stephen and his arguments for dropping the md5 mechanism. If you're moving to a newer version of PostgreSQL, you likely have to update your connection drivers anyway (rebuilt against new libpq, supporting any changes in the protocol, etc). I would prefer more data to support that argument, but this is generally what you need to do. However, we may need to step towards it. We took one step last release with defaulting to SCRAM. Perhaps this release we add a warning for anything using md5 auth that "this will be removed in a future release." (or specifically v16). We should also indicate in the docs that md5 is deprecated and will be removed. We also need to think about upgrading: what happens if I try to upgrade and I still have user passwords stored in a md5 hash? What if all my password-based users have a md5 hash, does pg_upgrade fail? As much as I want md5 gone, I think v16 is the earliest we can do it, but we can lay the groundwork for it now. Thanks, Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: Proposal: Support custom authentication methods using hooks
On Mon, Feb 28, 2022 at 03:46:34PM -0500, Stephen Frost wrote: > md5 should be removed. +1 -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Mon, Feb 28, 2022 at 04:42:55PM -0500, Stephen Frost wrote: > > Keeping it around will just push out the point at which everyone will > > finally be done with it, as there's really only two groups: those who > > have already moved to scram, and those who won't move until they want to > > upgrade to a release that doesn't have md5. > > FWIW, I am not sure if we are at this point yet. An extra reason to > remove it would be that it is a support burden, but I don't have seen > in recent memory any problems related to it that required any deep > changes in the way to use it, and its code paths are independent. Ongoing reports that there's a known vulnerability aren't great to have to deal with. We can at least point people to scram but that's not great. > The last time I played with this area is the recent error handling > improvement with cryptohashes but MD5 has actually helped here in > detecting the problem as a patched OpenSSL would complain if trying to > use MD5 as hash function when FIPS is enabled. Having to continue to deal with md5 as an algorithm when it's known to be notably less secure and so much so that organizations essentially ban its use for exactly what we're using it for, in fact, another reason to remove it, not a reason to keep it. Better code coverage testing of error paths is the answer to making sure that our error handling behaves properly. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On Mon, Feb 28, 2022 at 04:42:55PM -0500, Stephen Frost wrote: > Keeping it around will just push out the point at which everyone will > finally be done with it, as there's really only two groups: those who > have already moved to scram, and those who won't move until they want to > upgrade to a release that doesn't have md5. FWIW, I am not sure if we are at this point yet. An extra reason to remove it would be that it is a support burden, but I don't have seen in recent memory any problems related to it that required any deep changes in the way to use it, and its code paths are independent. The last time I played with this area is the recent error handling improvement with cryptohashes but MD5 has actually helped here in detecting the problem as a patched OpenSSL would complain if trying to use MD5 as hash function when FIPS is enabled. -- Michael signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > md5 should be removed. > > Really? I've always thought that the arguments against it were > overblown for our use-case. At any rate, it's likely to be > years before we could practically do that, since it's the best > that older client libraries can manage. Yes, really, it's a known-broken system which suffers from such an old and well known attack that it's been given a name: pass-the-hash. As was discussed on this thread even, just the fact that it's not trivial to break on the wire doesn't make it not-broken, particularly when we use the username (which is rather commonly the same one used across multiple systems..) as the salt. Worse, md5 isn't exactly the pinnacle of hashing techniques around these days. The wiki page goes over it in some detail regarding LM/NTLM which suffers the same problem (and also uses a challenge-response for the over-the-network bits): https://en.wikipedia.org/wiki/Pass_the_hash Further, a whole bunch of effort was put in to get scram support added to the different libraries and language bindings and such, specifically to allow us to get to a point where we can drop md5. Even after it's removed, folks will have 5 years before the release that removes it is the oldest supported release. I don't think we'll somehow get agreement to remove it for v15, so it'll be 5 major versions of overlap (11->15) by the time v16 comes out, and a total of 10 years of support for scram before md5 is gone. That's plenty, it's time to move on. Keeping it around will just push out the point at which everyone will finally be done with it, as there's really only two groups: those who have already moved to scram, and those who won't move until they want to upgrade to a release that doesn't have md5. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Stephen Frost writes: > md5 should be removed. Really? I've always thought that the arguments against it were overblown for our use-case. At any rate, it's likely to be years before we could practically do that, since it's the best that older client libraries can manage. regards, tom lane
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Fri, 2022-02-25 at 14:10 -0500, Tom Lane wrote: > > I also have in mind here that there has been discussion of giving > > libpq a feature to refuse, on the client side, to send cleartext > > passwords. > > I am generally in favor of that idea, but I'm not sure that will > completely resolve the issue. For instance, should it also refuse MD5? md5 should be removed. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
Greetings, * Jonathan S. Katz (jk...@postgresql.org) wrote: > On 2/25/22 12:39 PM, Tom Lane wrote: > >Jeff Davis writes: > >>On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: > >>>... and, since we can't readily enforce that the client only sends > >>>those cleartext passwords over suitably-encrypted connections, this > >>>could easily be a net negative for security. Not sure that I think > >>>it's a good idea. > > > >>I don't understand your point. Can't you just use "hostssl" rather than > >>"host"? > > > >My point is that sending cleartext passwords over the wire is an > >insecure-by-definition protocol that we shouldn't be encouraging > >more use of. > > This is my general feeling as well. We just spent a bunch of effort adding, > refining, and making SCRAM the default method. I think doing anything that > would drive more use of sending plaintext passwords, even over TLS, is > counter to that. Agreed. > I do understand arguments for (e.g. systems that require checking password > complexity), but I wonder if it's better for us to delegate that to an > external auth system. Regardless, I can get behind Andres' point to "check > Port->ssl_in_use before sendAuthRequest(AUTH_REQ_PASSWORD)". Password complexity is only needed to be checked at the time of password change though, which is not on every login, and should be after a confirmed mutual authentication between the client and the server. That's a very different situation. > I'm generally in favor of being able to support additional authentication > methods, the first one coming to mind is supporting OIDC. Having a pluggable > auth infrastructure could possibly make such efforts easier. I'm definitely > intrigued. I'm not thrilled with the idea, for my part. Thanks, Stephen signature.asc Description: PGP signature
Re: Proposal: Support custom authentication methods using hooks
On 17.02.22 20:25, samay sharma wrote: A use case where this is useful are environments where you want authentication to be centrally managed across different services. This is a common deployment model for cloud providers where customers like to use single sign on and authenticate across different services including Postgres. Implementing this now is tricky as it requires syncing that authentication method's credentials with Postgres (and that gets trickier with TTL/expiry etc.). With these hooks, you can implement an extension to check credentials directly using the authentication provider's APIs. We already have a variety of authentication mechanisms that support central management: LDAP, PAM, Kerberos, Radius. What other mechanisms are people thinking about implementing using these hooks? Maybe there are a bunch of them, in which case a hook system might be sensible, but if there are only one or two plausible ones, we could also just make them built in.
Re: Proposal: Support custom authentication methods using hooks
On 28.02.22 00:17, Jeff Davis wrote: I also have in mind here that there has been discussion of giving libpq a feature to refuse, on the client side, to send cleartext passwords. I am generally in favor of that idea, but I'm not sure that will completely resolve the issue. For instance, should it also refuse MD5? Presumably that feature could be more generally "refuse these authentication mechanisms" rather than only one hardcoded one.
Re: Proposal: Support custom authentication methods using hooks
On Fri, 2022-02-25 at 14:10 -0500, Tom Lane wrote: > I'm happy to add support for custom auth methods if they can use > a protocol that's safer than cleartext-password. But if that's the > only feasible option, then we're just encouraging people to use > insecure methods. FWIW, I'd like to be able to use a token in the password field, and then authenticate that token. So I didn't intend to send an actual password in plaintext. Maybe we should have a new "token" connection parameter so that libpq can allow sending a token plaintext but refuse to send a password in plaintext? > I also have in mind here that there has been discussion of giving > libpq a feature to refuse, on the client side, to send cleartext > passwords. I am generally in favor of that idea, but I'm not sure that will completely resolve the issue. For instance, should it also refuse MD5? Regards, Jeff Davis
Re: Proposal: Support custom authentication methods using hooks
Hi,
On 2022-02-25 14:10:39 -0500, Tom Lane wrote:
> I'm happy to add support for custom auth methods if they can use
> a protocol that's safer than cleartext-password. But if that's the
> only feasible option, then we're just encouraging people to use
> insecure methods.
It looks like scram can be used without much trouble. All the necessary
infrastructure to implement it without duplication appears to be public. The
plugin would need to get a secret from whereever and call
CheckSASLAuth(_be_scram_mech, port, shadow_pass, logdetail);
or if it needs to do something more complicated than CheckSASLAuth(), it can
use AUTH_REQ_SASL{,_FIN CONT} itself.
Of course whether it's viable depends on what the custom auth method wants to
do. But it's not a restriction of the authentication plugin model.
Greetings,
Andres Freund
Re: Proposal: Support custom authentication methods using hooks
On Fri, 2022-02-25 at 13:40 -0500, Jonathan S. Katz wrote: > I'm generally in favor of being able to support additional > authentication methods, the first one coming to mind is supporting OIDC. > Having a pluggable auth infrastructure could possibly make such efforts > easier. I'm definitely intrigued. I'm hoping to dust off my OAuth patch and see if it can be ported on top of this proposal. --Jacob
Re: Proposal: Support custom authentication methods using hooks
Jeff Davis writes: > On Fri, 2022-02-25 at 12:39 -0500, Tom Lane wrote: >> My point is that sending cleartext passwords over the wire is an >> insecure-by-definition protocol that we shouldn't be encouraging >> more use of. > We can require custom auth entries in pg_hba.conf to also specify > local, hostssl or hostgssenc. That's just a band-aid, though. It does nothing for the other reasons not to want cleartext passwords, notably that you might be giving your password to a compromised server. I'm happy to add support for custom auth methods if they can use a protocol that's safer than cleartext-password. But if that's the only feasible option, then we're just encouraging people to use insecure methods. I also have in mind here that there has been discussion of giving libpq a feature to refuse, on the client side, to send cleartext passwords. (I don't recall right now if that's been mentioned publicly or only among the security team, but it's definitely under consideration.) So I think this proposal needs more thought. A server-side hook alone is not a useful improvement IMO; it's closer to being an attractive nuisance. regards, tom lane
Re: Proposal: Support custom authentication methods using hooks
On Fri, 2022-02-25 at 12:39 -0500, Tom Lane wrote: > My point is that sending cleartext passwords over the wire is an > insecure-by-definition protocol that we shouldn't be encouraging > more use of. We can require custom auth entries in pg_hba.conf to also specify local, hostssl or hostgssenc. It might annoy people who have a network secured at some other layer, or who have the client on the same machine as the host. We could allow plain "host" if someone specifies "customplain" explicitly. Regards, Jeff Davis
Re: Proposal: Support custom authentication methods using hooks
On 2/25/22 12:39 PM, Tom Lane wrote: Jeff Davis writes: On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: ... and, since we can't readily enforce that the client only sends those cleartext passwords over suitably-encrypted connections, this could easily be a net negative for security. Not sure that I think it's a good idea. I don't understand your point. Can't you just use "hostssl" rather than "host"? My point is that sending cleartext passwords over the wire is an insecure-by-definition protocol that we shouldn't be encouraging more use of. This is my general feeling as well. We just spent a bunch of effort adding, refining, and making SCRAM the default method. I think doing anything that would drive more use of sending plaintext passwords, even over TLS, is counter to that. I do understand arguments for (e.g. systems that require checking password complexity), but I wonder if it's better for us to delegate that to an external auth system. Regardless, I can get behind Andres' point to "check Port->ssl_in_use before sendAuthRequest(AUTH_REQ_PASSWORD)". I'm generally in favor of being able to support additional authentication methods, the first one coming to mind is supporting OIDC. Having a pluggable auth infrastructure could possibly make such efforts easier. I'm definitely intrigued. Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: Proposal: Support custom authentication methods using hooks
Hi, On 2022-02-25 09:33:45 -0800, Jeff Davis wrote: > On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: > > ... and, since we can't readily enforce that the client only sends > > those cleartext passwords over suitably-encrypted connections, this > > could easily be a net negative for security. Not sure that I think > > it's a good idea. > > I don't understand your point. Can't you just use "hostssl" rather than > "host"? And the extension could check Port->ssl_in_use before sendAuthRequest(AUTH_REQ_PASSWORD) if it wanted to restrict it. Greetings, Andres Freund
Re: Proposal: Support custom authentication methods using hooks
Jeff Davis writes: > On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: >> ... and, since we can't readily enforce that the client only sends >> those cleartext passwords over suitably-encrypted connections, this >> could easily be a net negative for security. Not sure that I think >> it's a good idea. > I don't understand your point. Can't you just use "hostssl" rather than > "host"? My point is that sending cleartext passwords over the wire is an insecure-by-definition protocol that we shouldn't be encouraging more use of. regards, tom lane
Re: Proposal: Support custom authentication methods using hooks
On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote: > ... and, since we can't readily enforce that the client only sends > those cleartext passwords over suitably-encrypted connections, this > could easily be a net negative for security. Not sure that I think > it's a good idea. I don't understand your point. Can't you just use "hostssl" rather than "host"? Also there are some useful cases that don't really require SSL, like when the client and host are on the same machine, or if you have a network secured some other way. Regards, Jeff Davis
