Thread: GSSAPI, SSPI - include_realm default
Greetings,
The include_realm default for GSSAPI and SSPI is currently 'include_realm=0', meaning that the realm is stripped off
ofthe Kerberos principal (aka the 'system' username) prior to looking up the user in pg_authid.
This is fine in a single-realm environment but extremely dangerous in a multi-realm environment, as user@REALMA is
rarelythe same as user@REALMB. Worse, a given environment can go from single-realm to multi-realm with relative ease
andmost administrators aren't going to expect applications to have a problem with that change. Every other
Kerberos-enabledapplication which I'm aware of requires either the full principal (including realm) be considered, or
thatthe realm of the principal matches the realm of the system (which is what OpenSSH requires, as an example).
As such, I'd like to propose changing the default to be 'include_realm=1'.
Back when Kerberos support was originally added, we didn't have the pg_ident regex-based mapping capability. Today,
userswho wish to strip the realm off would be best served by configuring a mapping in pg_ident.conf which strips off
exactlythe realm name (or names, if they are multi-realm where the users actually are the same individuals in multiple
realms)instead of using 'include_realm=0'.
Users who really wish to strip off the realm for their environment would still be able to add 'include_realm=0' to
theirpg_hba.conf. We would recommend against that in the documentation, however, and explain how it's unsafe. I would
recommendthat this be coached as transistional support for users who wish to upgrade but don't want to (further) change
theirconfiguration immediately, with the implication that we might remove it some day.
This would be done for 9.5 and we would need to note it in the release notes, of course.
Shipping an insecure pg_hba.conf as the default (with 'trust') works because the distributions change it to a more
securesetting anyway. There's no similar option to change the default for include_realm short of hacking the source
codeand documentation, which would be much more invasive and likely invite complaints from users when their
configurationdoesn't work the way the postgresql.org docs claim it should.
Thoughts?
Thanks!
Stephen
On 11/26/14 2:01 PM, Stephen Frost wrote: > As such, I'd like to propose changing the default to be > 'include_realm=1'. Sounds reasonable to me. include_realm is supported back to 8.4, so affected users can set include_realm=0 in their existing installations.
* Peter Eisentraut (peter_e@gmx.net) wrote:
> On 11/26/14 2:01 PM, Stephen Frost wrote:
> > As such, I'd like to propose changing the default to be
> > 'include_realm=1'.
>
> Sounds reasonable to me.
>
> include_realm is supported back to 8.4, so affected users can set
> include_realm=0 in their existing installations.
Ah, yes, good point. Will include that suggestion also.
Thanks!
Stephen
On Wed, Nov 26, 2014 at 8:01 PM, Stephen Frost <sfrost@snowman.net> wrote: > Greetings, > > The include_realm default for GSSAPI and SSPI is currently > 'include_realm=0', meaning that the realm is stripped off of the > Kerberos principal (aka the 'system' username) prior to looking up the > user in pg_authid. > > This is fine in a single-realm environment but extremely dangerous > in a multi-realm environment, as user@REALMA is rarely the same as > user@REALMB. Worse, a given environment can go from single-realm to > multi-realm with relative ease and most administrators aren't going to > expect applications to have a problem with that change. Every other > Kerberos-enabled application which I'm aware of requires either the > full principal (including realm) be considered, or that the realm of > the principal matches the realm of the system (which is what OpenSSH > requires, as an example). > > As such, I'd like to propose changing the default to be > 'include_realm=1'. Per our previous discussions, but to make sure it's also on record for others, +1 for this suggestion. > Back when Kerberos support was originally added, we didn't have the > pg_ident regex-based mapping capability. Today, users who wish to > strip the realm off would be best served by configuring a mapping in > pg_ident.conf which strips off exactly the realm name (or names, if > they are multi-realm where the users actually are the same individuals > in multiple realms) instead of using 'include_realm=0'. > > Users who really wish to strip off the realm for their environment > would still be able to add 'include_realm=0' to their pg_hba.conf. > We would recommend against that in the documentation, however, and > explain how it's unsafe. I would recommend that this be coached as > transistional support for users who wish to upgrade but don't want to > (further) change their configuration immediately, with the implication > that we might remove it some day. > > This would be done for 9.5 and we would need to note it in the release > notes, of course. I suggest we also backpatch some documentation suggesting that people manually change the include_realm parameter (perhaps also with a note saying that the default will change in 9.5). -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
* Magnus Hagander (magnus@hagander.net) wrote:
> On Wed, Nov 26, 2014 at 8:01 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > This would be done for 9.5 and we would need to note it in the release
> > notes, of course.
>
> I suggest we also backpatch some documentation suggesting that people
> manually change the include_realm parameter (perhaps also with a note
> saying that the default will change in 9.5).
Agreed and will do (this was also suggested by Peter).
Thanks!
Stephen
* Magnus Hagander (magnus@hagander.net) wrote:
> On Wed, Nov 26, 2014 at 8:01 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > As such, I'd like to propose changing the default to be
> > 'include_realm=1'.
>
> Per our previous discussions, but to make sure it's also on record for
> others, +1 for this suggestion.
Patch attached which does this for master.
> > This would be done for 9.5 and we would need to note it in the release
> > notes, of course.
>
> I suggest we also backpatch some documentation suggesting that people
> manually change the include_realm parameter (perhaps also with a note
> saying that the default will change in 9.5).
I'll work on a patch for back-branches if everyone is alright with this
patch against master. Given my recent track record for changing
wording around, it seems prudent to get agreement on this first.
Thanks,
Stephen
Attachment
On 12/5/14 1:06 PM, Stephen Frost wrote: >> I suggest we also backpatch some documentation suggesting that people >> > manually change the include_realm parameter (perhaps also with a note >> > saying that the default will change in 9.5). > I'll work on a patch for back-branches if everyone is alright with this > patch against master. I don't think backpatching this is necessary or appropriate. First of all, this isn't even released, and it might very well change again later. The right time to publicly notify about this change is not before when 9.5 is released. Also, it's not like people keep re-reading the old documentation in order to get updated advice. It might very well be confusing if stable documentation changes because of future events. Users who are interested in knowing about changes in future releases should read the release notes of those future releases. My comment that include_realm is supported back to 8.4 was because there is an expectation that a pg_hba.conf file can be used unchanged across several major releases. So when 9.5 comes out and people update their pg_hba.conf files for 9.5, those files will still work in old releases.But the time to do those updates is then, not now.
<p dir="ltr"><br /> On Dec 9, 2014 10:52 PM, "Peter Eisentraut" <<a href="mailto:peter_e@gmx.net">peter_e@gmx.net</a>>wrote:<br /> ><br /> > On 12/5/14 1:06 PM, Stephen Frost wrote:<br/> > >> I suggest we also backpatch some documentation suggesting that people<br /> > >> >manually change the include_realm parameter (perhaps also with a note<br /> > >> > saying that the defaultwill change in 9.5).<br /> > > I'll work on a patch for back-branches if everyone is alright with this<br />> > patch against master.<br /> ><br /> > I don't think backpatching this is necessary or appropriate.<br />><br /> > First of all, this isn't even released, and it might very well change<br /> > again later. The righttime to publicly notify about this change is not<br /> > before when 9.5 is released.<br /> ><br /> > Also,it's not like people keep re-reading the old documentation in<br /> > order to get updated advice. It might verywell be confusing if stable<br /> > documentation changes because of future events. Users who are<br /> > interestedin knowing about changes in future releases should read the<br /> > release notes of those future releases.<br/> ><br /> > My comment that include_realm is supported back to 8.4 was because there<br /> > is anexpectation that a pg_hba.conf file can be used unchanged across<br /> > several major releases. So when 9.5 comesout and people update their<br /> > pg_hba.conf files for 9.5, those files will still work in old releases.<br />> But the time to do those updates is then, not now.<br /> ><p dir="ltr">I thought the idea was to backpatch documentationsaying "it's a good idea to change this value to x because of y". Not actually referring to the upcoming changedirectly. And I still think that part is a good idea, as it helps people avoid potential security pitfalls. <p dir="ltr">Sonot really a backpatch as so, rather a separate patch for the back branches. (and people definitely reread thedocs - since they deploy new systems on the existing versions...) <p dir="ltr">/Magnus <br />
* Peter Eisentraut (peter_e@gmx.net) wrote:
> On 12/5/14 1:06 PM, Stephen Frost wrote:
> >> I suggest we also backpatch some documentation suggesting that people
> >> > manually change the include_realm parameter (perhaps also with a note
> >> > saying that the default will change in 9.5).
> > I'll work on a patch for back-branches if everyone is alright with this
> > patch against master.
>
> I don't think backpatching this is necessary or appropriate.
Sorry if that wasn't clear but the idea was to *just* backpatch the
documentation comments, not to change the default in back-branches.
> First of all, this isn't even released, and it might very well change
> again later. The right time to publicly notify about this change is not
> before when 9.5 is released.
>
> Also, it's not like people keep re-reading the old documentation in
> order to get updated advice. It might very well be confusing if stable
> documentation changes because of future events. Users who are
> interested in knowing about changes in future releases should read the
> release notes of those future releases.
>
> My comment that include_realm is supported back to 8.4 was because there
> is an expectation that a pg_hba.conf file can be used unchanged across
> several major releases. So when 9.5 comes out and people update their
> pg_hba.conf files for 9.5, those files will still work in old releases.
> But the time to do those updates is then, not now.
The back-branches are being patched to discourage using the default
because it's not a secure approach. New users start using PG all the
time and so changing the existing documentation is worthwhile to ensure
those new users understand. A note in the release notes for whichever
minor release the change to the documentation shows up in would be a
good way to make existing users aware of the change and hopefully
encourage them to review their configuration.
If we don't agree that the change should be made then we can discuss
that, but everyone commenting so far has agreed on the change.
Thanks,
Stephen
* Magnus Hagander (magnus@hagander.net) wrote:
> On Dec 9, 2014 10:52 PM, "Peter Eisentraut" <peter_e@gmx.net> wrote:
> >
> > On 12/5/14 1:06 PM, Stephen Frost wrote:
> > >> I suggest we also backpatch some documentation suggesting that people
> > >> > manually change the include_realm parameter (perhaps also with a note
> > >> > saying that the default will change in 9.5).
> > > I'll work on a patch for back-branches if everyone is alright with this
> > > patch against master.
> >
> > I don't think backpatching this is necessary or appropriate.
> >
> > First of all, this isn't even released, and it might very well change
> > again later. The right time to publicly notify about this change is not
> > before when 9.5 is released.
> >
> > Also, it's not like people keep re-reading the old documentation in
> > order to get updated advice. It might very well be confusing if stable
> > documentation changes because of future events. Users who are
> > interested in knowing about changes in future releases should read the
> > release notes of those future releases.
> >
> > My comment that include_realm is supported back to 8.4 was because there
> > is an expectation that a pg_hba.conf file can be used unchanged across
> > several major releases. So when 9.5 comes out and people update their
> > pg_hba.conf files for 9.5, those files will still work in old releases.
> > But the time to do those updates is then, not now.
> >
>
> I thought the idea was to backpatch documentation saying "it's a good idea
> to change this value to x because of y". Not actually referring to the
> upcoming change directly. And I still think that part is a good idea, as it
> helps people avoid potential security pitfalls.
I agree with this but I don't really see why we wouldn't say "hey, this
is going to change in 9.5." Peter's argument sounds like he'd rather we
not make any changes to the existing documentation, and I don't agree
with that, and if we're making changes then, imv, we might as well
comment that the default is changed in 9.5.
> So not really a backpatch as so, rather a separate patch for the back
> branches. (and people definitely reread the docs - since they deploy new
> systems on the existing versions...)
Yes, I was going to write a different patch for the back-branches,
apologies if that wasn't clear. I'll see about drafting something up
soon as there doesn't seem to be any argument about the substance of the
proposed patch for master.
Thanks,
Stephen
On Tue, Dec 9, 2014 at 05:40:35PM -0500, Stephen Frost wrote: > > I thought the idea was to backpatch documentation saying "it's a good idea > > to change this value to x because of y". Not actually referring to the > > upcoming change directly. And I still think that part is a good idea, as it > > helps people avoid potential security pitfalls. > > I agree with this but I don't really see why we wouldn't say "hey, this > is going to change in 9.5." Peter's argument sounds like he'd rather we > not make any changes to the existing documentation, and I don't agree > with that, and if we're making changes then, imv, we might as well > comment that the default is changed in 9.5. I agree with Peter --- it is unwise to reference a future released feature in a backbranch doc patch. Updating the backbranch docs to add a recommendation is fine. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
* Bruce Momjian (bruce@momjian.us) wrote:
> On Tue, Dec 9, 2014 at 05:40:35PM -0500, Stephen Frost wrote:
> > > I thought the idea was to backpatch documentation saying "it's a good idea
> > > to change this value to x because of y". Not actually referring to the
> > > upcoming change directly. And I still think that part is a good idea, as it
> > > helps people avoid potential security pitfalls.
> >
> > I agree with this but I don't really see why we wouldn't say "hey, this
> > is going to change in 9.5." Peter's argument sounds like he'd rather we
> > not make any changes to the existing documentation, and I don't agree
> > with that, and if we're making changes then, imv, we might as well
> > comment that the default is changed in 9.5.
>
> I agree with Peter --- it is unwise to reference a future released
> feature in a backbranch doc patch. Updating the backbranch docs to add
> a recommendation is fine.
Alright, I don't agree but it's not worth the argument. I'll work on
the doc-update patch for the back-branches.
Thanks,
Stephen
On 12/9/14 5:40 PM, Stephen Frost wrote: > I agree with this but I don't really see why we wouldn't say "hey, this > is going to change in 9.5." Well, for one thing, we don't even know if it's going to be called 9.5. ;-) And there is always a chance for a technical reason popping up that we might not make the change after all in 9.5. I'd be fine with something more along the lines of "the default might change in the future ... if you want to be future-proof, set it explicitly". Let's see an actual patch.
* Peter Eisentraut (peter_e@gmx.net) wrote:
> On 12/9/14 5:40 PM, Stephen Frost wrote:
> > I agree with this but I don't really see why we wouldn't say "hey, this
> > is going to change in 9.5."
>
> Well, for one thing, we don't even know if it's going to be called 9.5. ;-)
Now that is certainly a very good point.
> And there is always a chance for a technical reason popping up that we
> might not make the change after all in 9.5.
I suppose.
> I'd be fine with something more along the lines of "the default might
> change in the future ... if you want to be future-proof, set it explicitly".
Sure, that'd work for me.
> Let's see an actual patch.
Will do. Might be a few days before I get to it but I don't think
there's any cause for rush anyway.
Thanks,
Stephen
On Wed, Dec 10, 2014 at 4:53 AM, Bruce Momjian <bruce@momjian.us> wrote: > On Tue, Dec 9, 2014 at 05:40:35PM -0500, Stephen Frost wrote: >> > I thought the idea was to backpatch documentation saying "it's a good idea >> > to change this value to x because of y". Not actually referring to the >> > upcoming change directly. And I still think that part is a good idea, as it >> > helps people avoid potential security pitfalls. >> >> I agree with this but I don't really see why we wouldn't say "hey, this >> is going to change in 9.5." Peter's argument sounds like he'd rather we >> not make any changes to the existing documentation, and I don't agree >> with that, and if we're making changes then, imv, we might as well >> comment that the default is changed in 9.5. > > I agree with Peter --- it is unwise to reference a future released > feature in a backbranch doc patch. Updating the backbranch docs to add > a recommendation is fine. I am strongly in agreement with that principle as well. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Dec 9, 2014 at 05:38:25PM -0500, Stephen Frost wrote: > > My comment that include_realm is supported back to 8.4 was because there > > is an expectation that a pg_hba.conf file can be used unchanged across > > several major releases. So when 9.5 comes out and people update their > > pg_hba.conf files for 9.5, those files will still work in old releases. > > But the time to do those updates is then, not now. > > The back-branches are being patched to discourage using the default > because it's not a secure approach. New users start using PG all the > time and so changing the existing documentation is worthwhile to ensure > those new users understand. A note in the release notes for whichever > minor release the change to the documentation shows up in would be a > good way to make existing users aware of the change and hopefully > encourage them to review their configuration. > > If we don't agree that the change should be made then we can discuss > that, but everyone commenting so far has agreed on the change. Where are we on this? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Bruce,
* Bruce Momjian (bruce@momjian.us) wrote:
> On Tue, Dec 9, 2014 at 05:38:25PM -0500, Stephen Frost wrote:
> > > My comment that include_realm is supported back to 8.4 was because there
> > > is an expectation that a pg_hba.conf file can be used unchanged across
> > > several major releases. So when 9.5 comes out and people update their
> > > pg_hba.conf files for 9.5, those files will still work in old releases.
> > > But the time to do those updates is then, not now.
> >
> > The back-branches are being patched to discourage using the default
> > because it's not a secure approach. New users start using PG all the
> > time and so changing the existing documentation is worthwhile to ensure
> > those new users understand. A note in the release notes for whichever
> > minor release the change to the documentation shows up in would be a
> > good way to make existing users aware of the change and hopefully
> > encourage them to review their configuration.
> >
> > If we don't agree that the change should be made then we can discuss
> > that, but everyone commenting so far has agreed on the change.
>
> Where are we on this?
Thanks for the reminder. I've not forgotten about it and will work on
crafting language in the next week or so.
Thanks again!
Stephen
Bruce, all,
* Bruce Momjian (bruce@momjian.us) wrote:
> On Tue, Dec 9, 2014 at 05:38:25PM -0500, Stephen Frost wrote:
> > > My comment that include_realm is supported back to 8.4 was because there
> > > is an expectation that a pg_hba.conf file can be used unchanged across
> > > several major releases. So when 9.5 comes out and people update their
> > > pg_hba.conf files for 9.5, those files will still work in old releases.
> > > But the time to do those updates is then, not now.
> >
> > The back-branches are being patched to discourage using the default
> > because it's not a secure approach. New users start using PG all the
> > time and so changing the existing documentation is worthwhile to ensure
> > those new users understand. A note in the release notes for whichever
> > minor release the change to the documentation shows up in would be a
> > good way to make existing users aware of the change and hopefully
> > encourage them to review their configuration.
> >
> > If we don't agree that the change should be made then we can discuss
> > that, but everyone commenting so far has agreed on the change.
>
> Where are we on this?
Patches for master and 9.4 attached. The 9.4 patch should cherry-pick
down to the other current releases just fine. Please provide any
comments or suggestions for changes. If all looks good, I'll push this
to change the default for 9.5 to be include_realm=1 and the
documentation updates to recommend it in back-branches.
Thanks!
Stephen
Attachment
Bruce,
* Bruce Momjian (bruce@momjian.us) wrote:
> On Tue, Dec 9, 2014 at 05:38:25PM -0500, Stephen Frost wrote:
> > > My comment that include_realm is supported back to 8.4 was because there
> > > is an expectation that a pg_hba.conf file can be used unchanged across
> > > several major releases. So when 9.5 comes out and people update their
> > > pg_hba.conf files for 9.5, those files will still work in old releases.
> > > But the time to do those updates is then, not now.
> >
> > The back-branches are being patched to discourage using the default
> > because it's not a secure approach. New users start using PG all the
> > time and so changing the existing documentation is worthwhile to ensure
> > those new users understand. A note in the release notes for whichever
> > minor release the change to the documentation shows up in would be a
> > good way to make existing users aware of the change and hopefully
> > encourage them to review their configuration.
> >
> > If we don't agree that the change should be made then we can discuss
> > that, but everyone commenting so far has agreed on the change.
>
> Where are we on this?
Thanks for the prod on this. I've now committed the changes which were
discussed. Please let me know if you see any issues or have any
concerns.
Thanks again!
Stephen