Thread: Thoughts on pg_hba.conf rejection
When there is a specific reject rule, why does the server say FATAL: no pg_hba.conf entry That sounds like an administrative error, rather than a specific decision on the part of an admin to reject the connection. Suggested message would be FATAL: connection rejected for host "xxx", user "xxxx", database "xxx" Clearly needs to be secure. Does the second message give any information to a would-be hacker than the first? I don't think so, but it certainly helps an admin work out if they've missed something. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> When there is a specific reject rule, why does the server say
> FATAL: no pg_hba.conf entry
It's intentional. We try to expose the minimum amount of knowledge
about the contents of pg_hba.conf to potential attackers.
regards, tom lane
On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: >> When there is a specific reject rule, why does the server say >> FATAL: no pg_hba.conf entry > > It's intentional. We try to expose the minimum amount of knowledge > about the contents of pg_hba.conf to potential attackers. > i just tried it in CVS and in 8.4 and when i put a reject rule on pg_hba.conf what i get is: psql: FATAL: no pg_hba.conf entry for host "127.0.0.1", user "mic", database "mic" so we are giving a lot of info already changing "no pg_hba.conf entry" for "connection rejected" doesn't seem like a lot more and the change could be useful for a DBA understanding what happens -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
Jaime Casanova <jcasanov@systemguards.com.ec> writes:
> On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It's intentional. We try to expose the minimum amount of knowledge
>> about the contents of pg_hba.conf to potential attackers.
> i just tried it in CVS and in 8.4 and when i put a reject rule on
> pg_hba.conf what i get is:
> psql: FATAL: no pg_hba.conf entry for host "127.0.0.1", user "mic",
> database "mic"
> so we are giving a lot of info already
All three of those data values are known to the client; they don't add
knowledge about what is in pg_hba.conf.
regards, tom lane
> Clearly needs to be secure. Does the second message give any information
> to a would-be hacker than the first? I don't think so, but it certainly
> helps an admin work out if they've missed something.
I think this question needs a bona fide network security geek to decide,
rather than us database geeks. Hello! Is there a security hacker in
the house?
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: >> When there is a specific reject rule, why does the server say >> FATAL: no pg_hba.conf entry > > It's intentional. We try to expose the minimum amount of knowledge > about the contents of pg_hba.conf to potential attackers. The problem with the message is not that it's uninformative, but that it's counterfactual. ...Robert
On Wed, Apr 07, 2010 at 01:07:21PM -0400, Robert Haas wrote: > On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Simon Riggs <simon@2ndQuadrant.com> writes: > >> When there is a specific reject rule, why does the server say > >> FATAL: no pg_hba.conf entry > > > > It's intentional. We try to expose the minimum amount of knowledge > > about the contents of pg_hba.conf to potential attackers. > > The problem with the message is not that it's uninformative, but that > it's counterfactual. > > ...Robert I agree (I noticed and was bothered by this today, as a matter of irrelevant fact). I can support the idea of exposing as little as possible of pg_hba.conf, but ISTM the "no pg_hba.conf entry" is exposing too much, by that standard. Just say something like "connection disallowed" and leave it at that -- either it's disallowed by lack of a rule, or by existence of a "reject" rule, or by something else entirely. As long as the message isn't clearly wrong in the "reject" case, as it is now. -- Joshua Tolley / eggyknap End Point Corporation http://www.endpoint.com
Joshua Tolley wrote: -- Start of PGP signed section. > On Wed, Apr 07, 2010 at 01:07:21PM -0400, Robert Haas wrote: > > On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > Simon Riggs <simon@2ndQuadrant.com> writes: > > >> When there is a specific reject rule, why does the server say > > >> FATAL: ?no pg_hba.conf entry > > > > > > It's intentional. ?We try to expose the minimum amount of knowledge > > > about the contents of pg_hba.conf to potential attackers. > > > > The problem with the message is not that it's uninformative, but that > > it's counterfactual. > > > > ...Robert > > I agree (I noticed and was bothered by this today, as a matter of irrelevant > fact). I can support the idea of exposing as little as possible of > pg_hba.conf, but ISTM the "no pg_hba.conf entry" is exposing too much, by that > standard. Just say something like "connection disallowed" and leave it at that > -- either it's disallowed by lack of a rule, or by existence of a "reject" > rule, or by something else entirely. As long as the message isn't clearly > wrong in the "reject" case, as it is now. Did we come to any conclusion on this? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com
* Bruce Momjian <bruce@momjian.us> [100414 16:20]: > Joshua Tolley wrote: > -- Start of PGP signed section. > > On Wed, Apr 07, 2010 at 01:07:21PM -0400, Robert Haas wrote: > > > On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > Simon Riggs <simon@2ndQuadrant.com> writes: > > > >> When there is a specific reject rule, why does the server say > > > >> FATAL: ?no pg_hba.conf entry > > > > > > > > It's intentional. ?We try to expose the minimum amount of knowledge > > > > about the contents of pg_hba.conf to potential attackers. > > > > > > The problem with the message is not that it's uninformative, but that > > > it's counterfactual. > > > > > > ...Robert > > > > I agree (I noticed and was bothered by this today, as a matter of irrelevant > > fact). I can support the idea of exposing as little as possible of > > pg_hba.conf, but ISTM the "no pg_hba.conf entry" is exposing too much, by that > > standard. Just say something like "connection disallowed" and leave it at that > > -- either it's disallowed by lack of a rule, or by existence of a "reject" > > rule, or by something else entirely. As long as the message isn't clearly > > wrong in the "reject" case, as it is now. > > Did we come to any conclusion on this? I think it sort of just died. I'm in favour of making sure we don't give out any extra information, so if the objection to the message is simply that "no pg_hba.conf entry" is "counterfactual" when there is an entry rejecting it, how about: "No pg_hba.conf authorizing entry" That's no longer counter-factual, and works for both no entry, and a rejecting entry... a. -- Aidan Van Dyk Create like a god, aidan@highrise.ca command like a king, http://www.highrise.ca/ work like a slave.
On Wed, Apr 14, 2010 at 4:24 PM, Aidan Van Dyk <aidan@highrise.ca> wrote: > I think it sort of just died. I'm in favour of making sure we don't > give out any extra information, so if the objection to the message is > simply that "no pg_hba.conf entry" is "counterfactual" when there is an > entry rejecting it, how about: > "No pg_hba.conf authorizing entry" > > That's no longer counter-factual, and works for both no entry, and a > rejecting entry... That works for me. I don't have strong feelings about it so I'd probably be OK to a variety of solutions subject to my previous remarks, but that seems as good as anything. ...Robert
On Wed, Apr 14, 2010 at 4:28 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Apr 14, 2010 at 4:24 PM, Aidan Van Dyk <aidan@highrise.ca> wrote: >> I think it sort of just died. I'm in favour of making sure we don't >> give out any extra information, so if the objection to the message is >> simply that "no pg_hba.conf entry" is "counterfactual" when there is an >> entry rejecting it, how about: >> "No pg_hba.conf authorizing entry" >> >> That's no longer counter-factual, and works for both no entry, and a >> rejecting entry... > > That works for me. I don't have strong feelings about it so I'd > probably be OK to a variety of solutions subject to my previous > remarks, but that seems as good as anything. Although on further reflection, part of me feels like it might be even simpler and clearer to simply say: connection not authorized ...Robert
On Wed, Apr 14, 2010 at 4:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Apr 14, 2010 at 4:28 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Wed, Apr 14, 2010 at 4:24 PM, Aidan Van Dyk <aidan@highrise.ca> wrote: >>> I think it sort of just died. I'm in favour of making sure we don't >>> give out any extra information, so if the objection to the message is >>> simply that "no pg_hba.conf entry" is "counterfactual" when there is an >>> entry rejecting it, how about: >>> "No pg_hba.conf authorizing entry" >>> >>> That's no longer counter-factual, and works for both no entry, and a >>> rejecting entry... >> >> That works for me. I don't have strong feelings about it so I'd >> probably be OK to a variety of solutions subject to my previous >> remarks, but that seems as good as anything. > > Although on further reflection, part of me feels like it might be even > simpler and clearer to simply say: > > connection not authorized > +1 -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 14, 2010 at 4:24 PM, Aidan Van Dyk <aidan@highrise.ca> wrote:
>> I think it sort of just died. �I'm in favour of making sure we don't
>> give out any extra information, so if the objection to the message is
>> simply that "no pg_hba.conf entry" is "counterfactual" when there is an
>> entry rejecting it, how about:
>> � "No pg_hba.conf authorizing entry"
>>
>> That's no longer counter-factual, and works for both no entry, and a
>> rejecting entry...
> That works for me.
It needs copy-editing. Maybeno pg_hba.conf entry allows access for host ... user ...
regards, tom lane
I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Apr 14, 2010 at 4:24 PM, Aidan Van Dyk <aidan@highrise.ca> wrote:
>>> I think it sort of just died. �I'm in favour of making sure we don't
>>> give out any extra information, so if the objection to the message is
>>> simply that "no pg_hba.conf entry" is "counterfactual" when there is an
>>> entry rejecting it, how about:
>>> � "No pg_hba.conf authorizing entry"
>>>
>>> That's no longer counter-factual, and works for both no entry, and a
>>> rejecting entry...
>> That works for me.
> It needs copy-editing. Maybe
> no pg_hba.conf entry allows access for host ... user ...
Actually, on reflection, I'm not sure that these suggestions really do
anything for the "counter-factual" complaint. The case where you'd
normally use an explicit REJECT entry is where you're REJECTing some
limited case in an entry that is before a wider-scope entry that would
accept it. So it doesn't seem entirely accurate to say that there is no
pg_hba.conf entry that would accept the connection. There is one but
it's not the one we chose.
I'm thinking there isn't anything much we can do here without using a
different message wording for a match to a REJECT entry. So it's a
straight-up tradeoff of possible security information leakage against
whether a different wording is really helpful to the admin. Both of
those seem like fairly marginal concerns, really, so I'm having a hard
time deciding which one ought to win. But given that nobody complained
before this, is it worth changing?
regards, tom lane
On Wed, Apr 14, 2010 at 8:19 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm thinking there isn't anything much we can do here without using a > different message wording for a match to a REJECT entry. So it's a > straight-up tradeoff of possible security information leakage against > whether a different wording is really helpful to the admin. Both of > those seem like fairly marginal concerns, really, so I'm having a hard > time deciding which one ought to win. But given that nobody complained > before this, is it worth changing? What's wrong with something like "connection not permitted" or "connection not authorized"? ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> What's wrong with something like "connection not permitted" or
> "connection not authorized"?
The case that we're trying to cater to with the existing wording is
novice DBAs, who are likely to stare at such a message and not even
realize that pg_hba.conf is what they need to change. Frankly, by
the time anyone is using REJECT entries they are probably advanced
enough to not need much help from the error message; but what you
propose is an absolute lock to increase the number of newbie questions
on the lists by a large factor.
regards, tom lane
Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > What's wrong with something like "connection not permitted" or > > "connection not authorized"? > > The case that we're trying to cater to with the existing wording is > novice DBAs, who are likely to stare at such a message and not even > realize that pg_hba.conf is what they need to change. Frankly, by > the time anyone is using REJECT entries they are probably advanced > enough to not need much help from the error message; but what you > propose is an absolute lock to increase the number of newbie questions > on the lists by a large factor. Agreed. I would rather have an inaccurate error message that mentions pg_hba.conf than an accurate one that doesn't. Error messages should always point at a solution, if possible. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com
On Wed, Apr 14, 2010 at 8:31 PM, Bruce Momjian <bruce@momjian.us> wrote: > Tom Lane wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >> > What's wrong with something like "connection not permitted" or >> > "connection not authorized"? >> >> The case that we're trying to cater to with the existing wording is >> novice DBAs, who are likely to stare at such a message and not even >> realize that pg_hba.conf is what they need to change. Frankly, by >> the time anyone is using REJECT entries they are probably advanced >> enough to not need much help from the error message; but what you >> propose is an absolute lock to increase the number of newbie questions >> on the lists by a large factor. > > Agreed. I would rather have an inaccurate error message that mentions > pg_hba.conf than an accurate one that doesn't. > > Error messages should always point at a solution, if possible. OK, how about "connection not authorized by pg_hba.conf"? ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> OK, how about "connection not authorized by pg_hba.conf"?
This is still not especially helpful for novice DBAs. We want to point
them in the direction that they need to add an entry to pg_hba.conf,
which is 99% likely to be what's wanted. The current wording provides
that hint; vague statements like the above don't.
regards, tom lane
On Wed, Apr 14, 2010 at 10:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> OK, how about "connection not authorized by pg_hba.conf"? > > This is still not especially helpful for novice DBAs. We want to point > them in the direction that they need to add an entry to pg_hba.conf, > which is 99% likely to be what's wanted. The current wording provides > that hint; vague statements like the above don't. *scratches head* So you'd prefer a message that is sometimes flat-out wrong over a message that is correct but less informative in the common case? I guess that could be right call, but it's not what I'd pick. ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> So you'd prefer a message that is sometimes flat-out wrong over a
> message that is correct but less informative in the common case? I
> guess that could be right call, but it's not what I'd pick.
Well, as I said, I think the only way to really improve this message
is to use a different wording for the REJECT case. I'm unconvinced
that the problem justifies that, but if you're sufficiently hot about
it, that is the direction to go in; not making the the message less
useful for the 99% case.
regards, tom lane
Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> So you'd prefer a message that is sometimes flat-out wrong over a >> message that is correct but less informative in the common case? I >> guess that could be right call, but it's not what I'd pick. > > Well, as I said, I think the only way to really improve this message > is to use a different wording for the REJECT case. I'm unconvinced > that the problem justifies that, but if you're sufficiently hot about > it, that is the direction to go in; not making the the message less > useful for the 99% case. How about a hint? FATAL: connection not authorized for host "[local]", user "foo", database "postgres" HINT: Make sure that you have a matching accept line in pg_hba.conf -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Thu, 2010-04-15 at 00:24 -0400, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > So you'd prefer a message that is sometimes flat-out wrong over a > > message that is correct but less informative in the common case? I > > guess that could be right call, but it's not what I'd pick. > > Well, as I said, I think the only way to really improve this message > is to use a different wording for the REJECT case. I'm unconvinced > that the problem justifies that, but if you're sufficiently hot about > it, that is the direction to go in; not making the the message less > useful for the 99% case. I think that would solve my original gripe, if I understood the idea. So instead of the typical "reject" instruction we also add a "rejectverbose" instruction that has a more verbose message. Docs would describe it as an additional instruction to assist with debugging a complex pg_hba.conf, with warning that if used it may assist the bad guys also. "pg_hba.conf rejects entry for host..." Patch for that would be simple and clear; I can add that if we agree. -- Simon Riggs www.2ndQuadrant.com
Simon,
* Simon Riggs (simon@2ndQuadrant.com) wrote:
> So instead of the typical "reject" instruction we also add a
> "rejectverbose" instruction that has a more verbose message. Docs would
> describe it as an additional instruction to assist with debugging a
> complex pg_hba.conf, with warning that if used it may assist the bad
> guys also.
Erm, so we'd add an option for this? That strikes me as pretty
excessive. Not to be a pain, but I feel like the 'connection not
authorized' argument plus a hint makes alot more sense.
> "pg_hba.conf rejects entry for host..."
"connection not authorized for host X user Y database Z"
"HINT: Make sure your pg_hba.conf has the entries needed and the user
has CONNECT privileges for the database"
Or something along those lines (I added the other CONNECT issue because
it's one I've run into in the past.. :).
I do also wonder if we should consider having the error that's reported
to the log differ from that which is sent to the user.. I realize
that's a much bigger animal and might not help the novice, but it could
help with debugging complex pg_hba's without exposing info to possible
bad guys.
Thanks,
Stephen
Stephen Frost <sfrost@snowman.net> writes:
> * Simon Riggs (simon@2ndQuadrant.com) wrote:
>> So instead of the typical "reject" instruction we also add a
>> "rejectverbose" instruction that has a more verbose message.
> Erm, so we'd add an option for this? That strikes me as pretty
> excessive.
I think Simon's point was that we'd need a different uaReject enum
value internally in the code, so that the place where the message
gets issued would be able to distinguish explicit REJECT entry from
falling off the end of the file. Changing what the user is expected
to put in the file would be silly. (I don't care for "rejectverbose"
though. Maybe uaImplicitReject for the end-of-file case would be
the most readable way.)
regards, tom lane
On Wed, Apr 14, 2010 at 08:37:18PM -0400, Robert Haas wrote: > On Wed, Apr 14, 2010 at 8:31 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Tom Lane wrote: > >> Robert Haas <robertmhaas@gmail.com> writes: > >> > What's wrong with something like "connection not permitted" or > >> > "connection not authorized"? > >> > >> The case that we're trying to cater to with the existing wording > >> is novice DBAs, who are likely to stare at such a message and not > >> even realize that pg_hba.conf is what they need to change. > >> Frankly, by the time anyone is using REJECT entries they are > >> probably advanced enough to not need much help from the error > >> message; but what you propose is an absolute lock to increase the > >> number of newbie questions on the lists by a large factor. > > > > Agreed. I would rather have an inaccurate error message that > > mentions pg_hba.conf than an accurate one that doesn't. > > > > Error messages should always point at a solution, if possible. > > OK, how about "connection not authorized by pg_hba.conf"? +1. It's clear, and if an attacker can compromise pg_hba.conf, there's nothing PostgreSQL can do to help. I'd like to bring up the idea of an attacker who both has that access and doesn't know about pg_hba.conf just to dismiss it. Such a person might exist, but we don't need to bend things around a case so rare that it makes being struck by lightning look like a certainty. :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Thu, 2010-04-15 at 09:44 -0400, Tom Lane wrote: > Maybe uaImplicitReject for the end-of-file case would be > the most readable way. uaImplicitReject capability added. We're now free to bikeshed on exact wording. After much heavy thinking, message is "pg_hba.conf rejects..." with no hint (yet?). Point of note on giving information to the bad guys: if a should-be-rejected connection request attempts to connect to a non-existent database, we say "database does not exist". If db does exist we say "pg_hba.conf rejects...". To me that looks like giving info away... if an IP address range is rejected always then telling them whether or not a particular database name exists seems like something I would not wish to expose. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> Point of note on giving information to the bad guys: if a
> should-be-rejected connection request attempts to connect to a
> non-existent database, we say "database does not exist".
Yeah. This was an acknowledged shortcoming of the changes to eliminate
flat-file storage of authentication information --- as of 9.0, it's
necessary to connect to some database in order to proceed with auth
checking. We discussed it at the time and agreed it was an acceptable
loss.
The only way I can think of to improve that without going back to flat
files would be to develop a way for backends to switch databases after
initial startup, so that auth could be done in a predetermined database
(say, "postgres") before switching to the requested DB. This has enough
potential gotchas, in regards to catalog caching for instance, that I'm
not eager to go there.
Alternatively we could lie, and produce an auth failure message of some
sort rather than admitting the DB doesn't exist. But that seems like
it's going to create enough confusion to not be acceptable.
regards, tom lane
On Mon, 2010-04-19 at 16:30 -0400, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > Point of note on giving information to the bad guys: if a > > should-be-rejected connection request attempts to connect to a > > non-existent database, we say "database does not exist". > > Yeah. This was an acknowledged shortcoming of the changes to eliminate > flat-file storage of authentication information --- as of 9.0, it's > necessary to connect to some database in order to proceed with auth > checking. With code as currently, yes, though I see that there is a way to do this. Rules that have an "all" in the database field of the hba can be applied prior to attempting to select the database, as long as the "all" rule is above any database-specific rules. So its possible, we just need to special case the code so we call it once before db is assigned for "all" rules only and then again later, as is now, including 100% of rules. (I say 100% to avoid using the word all in two contexts in same sentence). > We discussed it at the time and agreed it was an acceptable > loss. > > The only way I can think of to improve that without going back to flat > files would be to develop a way for backends to switch databases after > initial startup, so that auth could be done in a predetermined database > (say, "postgres") before switching to the requested DB. This has enough > potential gotchas, in regards to catalog caching for instance, that I'm > not eager to go there. > > Alternatively we could lie, and produce an auth failure message of some > sort rather than admitting the DB doesn't exist. But that seems like > it's going to create enough confusion to not be acceptable. -- Simon Riggs www.2ndQuadrant.com
On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: >> Point of note on giving information to the bad guys: if a >> should-be-rejected connection request attempts to connect to a >> non-existent database, we say "database does not exist". > > Yeah. This was an acknowledged shortcoming of the changes to eliminate > flat-file storage of authentication information --- as of 9.0, it's > necessary to connect to some database in order to proceed with auth > checking. We discussed it at the time and agreed it was an acceptable > loss. > > The only way I can think of to improve that without going back to flat > files would be to develop a way for backends to switch databases after > initial startup, so that auth could be done in a predetermined database > (say, "postgres") before switching to the requested DB. This has enough > potential gotchas, in regards to catalog caching for instance, that I'm > not eager to go there. Would it be possible to set up a skeleton environment where we can access shared catalogs only and then decide on which database we're using later? ...Robert
Robert Haas escribió: > On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > The only way I can think of to improve that without going back to flat > > files would be to develop a way for backends to switch databases after > > initial startup, so that auth could be done in a predetermined database > > (say, "postgres") before switching to the requested DB. This has enough > > potential gotchas, in regards to catalog caching for instance, that I'm > > not eager to go there. > > Would it be possible to set up a skeleton environment where we can > access shared catalogs only and then decide on which database we're > using later? Eh? We already do that ... In fact the autovac launcher is always connected to shared catalogs, without being connected to any one database in particular (cf. get_database_list) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
On Mon, Apr 19, 2010 at 5:04 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Robert Haas escribió: >> On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> > The only way I can think of to improve that without going back to flat >> > files would be to develop a way for backends to switch databases after >> > initial startup, so that auth could be done in a predetermined database >> > (say, "postgres") before switching to the requested DB. This has enough >> > potential gotchas, in regards to catalog caching for instance, that I'm >> > not eager to go there. >> >> Would it be possible to set up a skeleton environment where we can >> access shared catalogs only and then decide on which database we're >> using later? > > Eh? We already do that ... In fact the autovac launcher is always > connected to shared catalogs, without being connected to any one > database in particular (cf. get_database_list) Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to connect to some database in order to proceed with auth checking". Why is that necessary, if we can access shared catalogs without it? ...Robert
Robert Haas escribió: > On Mon, Apr 19, 2010 at 5:04 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > Robert Haas escribió: > >> On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > >> > The only way I can think of to improve that without going back to flat > >> > files would be to develop a way for backends to switch databases after > >> > initial startup, so that auth could be done in a predetermined database > >> > (say, "postgres") before switching to the requested DB. This has enough > >> > potential gotchas, in regards to catalog caching for instance, that I'm > >> > not eager to go there. > >> > >> Would it be possible to set up a skeleton environment where we can > >> access shared catalogs only and then decide on which database we're > >> using later? > > > > Eh? We already do that ... In fact the autovac launcher is always > > connected to shared catalogs, without being connected to any one > > database in particular (cf. get_database_list) > > Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to > connect to some database in order to proceed with auth checking". Why > is that necessary, if we can access shared catalogs without it? Hmm, yeah, why did he say that? Maybe the order of operations during startup is such that we only do auth checking after connecting to a database for some reason. Whatever it is, I don't think a badly worded error message is enough grounds for fooling with this at this time of release process, though. To be discussed for 9.1? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote: > Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to > connect to some database in order to proceed with auth checking". Why > is that necessary It's not, I just explained how to do it without. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs escribió: > On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote: > > > Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to > > connect to some database in order to proceed with auth checking". Why > > is that necessary > > It's not, I just explained how to do it without. You mean purely using pg_hba.conf "all" rules? That seems a bit unsatisfactory ... -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, Apr 19, 2010 at 5:12 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Robert Haas escribió: >> On Mon, Apr 19, 2010 at 5:04 PM, Alvaro Herrera >> <alvherre@commandprompt.com> wrote: >> > Robert Haas escribió: >> >> On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> > >> >> > The only way I can think of to improve that without going back to flat >> >> > files would be to develop a way for backends to switch databases after >> >> > initial startup, so that auth could be done in a predetermined database >> >> > (say, "postgres") before switching to the requested DB. This has enough >> >> > potential gotchas, in regards to catalog caching for instance, that I'm >> >> > not eager to go there. >> >> >> >> Would it be possible to set up a skeleton environment where we can >> >> access shared catalogs only and then decide on which database we're >> >> using later? >> > >> > Eh? We already do that ... In fact the autovac launcher is always >> > connected to shared catalogs, without being connected to any one >> > database in particular (cf. get_database_list) >> >> Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to >> connect to some database in order to proceed with auth checking". Why >> is that necessary, if we can access shared catalogs without it? > > Hmm, yeah, why did he say that? Maybe the order of operations during > startup is such that we only do auth checking after connecting to a > database for some reason. > > Whatever it is, I don't think a badly worded error message is enough > grounds for fooling with this at this time of release process, though. > To be discussed for 9.1? I'm not proposing to fix the issue right now; but I wanted to try to understand it while it's fresh in my mind. I'm still not seeing the issue for some reason. ...Robert
On Mon, Apr 19, 2010 at 5:22 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote: > >> Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to >> connect to some database in order to proceed with auth checking". Why >> is that necessary > > It's not, I just explained how to do it without. Your explanation seems to presuppose that we somehow can't process the database-specific rules before selecting a database. I don't understand why that would be the case. Why can't we just check all the rules and then, if we decide to allow the connection, select the database? ...Robert
On Mon, 2010-04-19 at 17:52 -0400, Robert Haas wrote: > On Mon, Apr 19, 2010 at 5:22 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > > On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote: > > > >> Oh. Then I'm confused. Tom said: "as of 9.0, it's necessary to > >> connect to some database in order to proceed with auth checking". Why > >> is that necessary > > > > It's not, I just explained how to do it without. > > Your explanation seems to presuppose that we somehow can't process the > database-specific rules before selecting a database. I don't > understand why that would be the case. Why can't we just check all > the rules and then, if we decide to allow the connection, select the > database? Some rules are user-specific, but I see that doesn't matter and you are right. We can process the whole pg_hba.conf to see if it returns reject or implicitreject before attempting to confirm the existence of any database or any user. Any other result must be implemented during ClientAuthentication(). So we may as well run the whole set of rules, work out which rule applies and then remember that for later use. Just as efficient, better security. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> With code as currently, yes, though I see that there is a way to do
> this.
> Rules that have an "all" in the database field of the hba can be applied
> prior to attempting to select the database, as long as the "all" rule is
> above any database-specific rules.
Well, that's nice, but it's an awfully small subset of what the
pg_hba.conf rules might contain. In particular you can't do anything
that involves group membership checks without access to the catalogs;
and I think a large fraction of installations that are exposed to
untrustworthy connections will be using password auth for them, which
means they still need to connect to the catalogs to get the password.
Now it's possible that we could have a prefilter that rejects
connections if they're coming from an IP address that cannot match
*any* of the pg_hba.conf rules. Not sure how useful that would really
be in practice though. It wouldn't do anything extra for people who
keep their DB server behind a firewall.
regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Robert Haas escribi�:
>> Would it be possible to set up a skeleton environment where we can
>> access shared catalogs only and then decide on which database we're
>> using later?
> Eh? We already do that ... In fact the autovac launcher is always
> connected to shared catalogs, without being connected to any one
> database in particular (cf. get_database_list)
Hmm. The AV launcher is only permitted to touch pg_database.
At the time there were considerable advantages to that restriction,
but subsequent changes (like getting rid of the need for manual
maintenance of pg_attribute entries for bootstrap catalogs) might
mean that it wouldn't be too painful to extend this capability to
pg_authid etc. Could be worth thinking about.
regards, tom lane
On Mon, Apr 19, 2010 at 7:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: >> Robert Haas escribió: >>> Would it be possible to set up a skeleton environment where we can >>> access shared catalogs only and then decide on which database we're >>> using later? > >> Eh? We already do that ... In fact the autovac launcher is always >> connected to shared catalogs, without being connected to any one >> database in particular (cf. get_database_list) > > Hmm. The AV launcher is only permitted to touch pg_database. > At the time there were considerable advantages to that restriction, > but subsequent changes (like getting rid of the need for manual > maintenance of pg_attribute entries for bootstrap catalogs) might > mean that it wouldn't be too painful to extend this capability to > pg_authid etc. Could be worth thinking about. Perhaps we should add a TODO. ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Apr 19, 2010 at 7:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hmm. �The AV launcher is only permitted to touch pg_database.
> Perhaps we should add a TODO.
Actually, while I'm looking at that code, a more immediate TODO is
"fix walsender". Somebody has inserted an absolutely flight-of-fantasy
code path into InitPostgres. (Hint: template1 can be dropped.
ESPECIALLY when you're deliberately not taking any lock on it.)
regards, tom lane
On Mon, Apr 19, 2010 at 8:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Mon, Apr 19, 2010 at 7:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Hmm. The AV launcher is only permitted to touch pg_database. > >> Perhaps we should add a TODO. > > Actually, while I'm looking at that code, a more immediate TODO is > "fix walsender". Somebody has inserted an absolutely flight-of-fantasy > code path into InitPostgres. (Hint: template1 can be dropped. > ESPECIALLY when you're deliberately not taking any lock on it.) Off-topic to that, but on-topic to the original topic of this thread, check out this link that Karen Padir just blogged about on planet.postgresql.org: http://blog.metasploit.com/2010/02/postgres-fingerprinting.html Assuming the situation really is as described here, I am wondering if we should suppress the F, L, and R output in this and similar cases and back-patch it all the way back. This seems like it is entirely too helpful. ...Robert
Robert Haas <robertmhaas@gmail.com> writes: > http://blog.metasploit.com/2010/02/postgres-fingerprinting.html > Assuming the situation really is as described here, I am wondering if > we should suppress the F, L, and R output in this and similar cases > and back-patch it all the way back. This seems like it is entirely > too helpful. [ yawn.. ] I'm unimpressed: should we also ensure that neither ASCII nor translated texts of authentication failure messages ever change? IIRC, you were lobbying *for* such a change only a day or two ago. regards, tom lane
I wrote:
> Actually, while I'm looking at that code, a more immediate TODO is
> "fix walsender". Somebody has inserted an absolutely flight-of-fantasy
> code path into InitPostgres. (Hint: template1 can be dropped.
> ESPECIALLY when you're deliberately not taking any lock on it.)
Now that I look more closely, it seems what we have actually got there
is an incorrect attempt to solve the problem of authenticating without
selecting any particular database. So we could solve both this and
the original complaint in the thread if we can arrange for all
authentication to be done on the basis of shared-catalog access under
rules similar to what the AV launcher does with pg_database. At a
minimum that will require marking the pg_auth catalogs as
BKI_SCHEMA_MACRO, but that's far less painful than it used to be.
I don't recall what other consequences there are, but will go looking.
regards, tom lane
I wrote:
> ... So we could solve both this and
> the original complaint in the thread if we can arrange for all
> authentication to be done on the basis of shared-catalog access under
> rules similar to what the AV launcher does with pg_database. At a
> minimum that will require marking the pg_auth catalogs as
> BKI_SCHEMA_MACRO, but that's far less painful than it used to be.
> I don't recall what other consequences there are, but will go looking.
I've been looking at this and it seems do-able, though I don't have
working code yet. Downsides appear to be:
1. We'd have to force an initdb because of a couple of small catalog
changes. This doesn't seem like a showstopper at this phase of the
release cycle, but it's slightly annoying. pg_migrator could be used
if anyone's really in need of it.
2. We don't have infrastructure that would allow access to out-of-line
toasted fields during startup. Rather than try to add such, I propose
removing pg_authid's toast table, with the consequence that rolpassword
cannot be long enough to require out-of-line storage (note it could
still be compressed in-line). I cannot imagine any real situation where
this would be an issue --- does anyone else? (BTW, I'm fairly sure that
we couldn't support an out-of-line rolpassword in the past anyway,
because of restrictions in the old flatfiles code.)
3. We'd have to nail pg_authid, pg_auth_members, and their indexes into
relcache, because relcache.c isn't prepared to cope otherwise. I doubt
this would affect performance in any material way, but it would eat a
few more kbytes of storage per backend.
None of these seem like reasons not to do it. Objections?
regards, tom lane
Tom Lane escribió: > 1. We'd have to force an initdb because of a couple of small catalog > changes. This doesn't seem like a showstopper at this phase of the > release cycle, but it's slightly annoying. pg_migrator could be used > if anyone's really in need of it. check > 2. We don't have infrastructure that would allow access to out-of-line > toasted fields during startup. Rather than try to add such, I propose > removing pg_authid's toast table, with the consequence that rolpassword > cannot be long enough to require out-of-line storage (note it could > still be compressed in-line). I cannot imagine any real situation where > this would be an issue --- does anyone else? (BTW, I'm fairly sure that > we couldn't support an out-of-line rolpassword in the past anyway, > because of restrictions in the old flatfiles code.) In the past rolconfig could have been a problem too, but fortunately we got that moved out. I really doubt that a password of "only" about 2kB compressed is going to be a limitation to anyone on this planet. (Hmm, isn't it really 8kB in row length that's the hard limit?) This could perhaps be an issue if we were to store an SSL certificate in rolpassword or something like that, but I don't think we support that. > 3. We'd have to nail pg_authid, pg_auth_members, and their indexes into > relcache, because relcache.c isn't prepared to cope otherwise. I doubt > this would affect performance in any material way, but it would eat a > few more kbytes of storage per backend. This doesn't limit that VACUUM FULL or other commands are applied to those catalogs, right? > None of these seem like reasons not to do it. Objections? None here. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, Apr 20, 2010 at 2:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > 1. We'd have to force an initdb because of a couple of small catalog > changes. This doesn't seem like a showstopper at this phase of the > release cycle, but it's slightly annoying. pg_migrator could be used > if anyone's really in need of it. Fine. > 2. We don't have infrastructure that would allow access to out-of-line > toasted fields during startup. Rather than try to add such, I propose > removing pg_authid's toast table, with the consequence that rolpassword > cannot be long enough to require out-of-line storage (note it could > still be compressed in-line). I cannot imagine any real situation where > this would be an issue --- does anyone else? (BTW, I'm fairly sure that > we couldn't support an out-of-line rolpassword in the past anyway, > because of restrictions in the old flatfiles code.) I think that's OK. > 3. We'd have to nail pg_authid, pg_auth_members, and their indexes into > relcache, because relcache.c isn't prepared to cope otherwise. I doubt > this would affect performance in any material way, but it would eat a > few more kbytes of storage per backend. Hmm, I'm not sure I understand why this is necessary or what our other options are. ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Apr 20, 2010 at 2:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 3. We'd have to nail pg_authid, pg_auth_members, and their indexes into
>> relcache, because relcache.c isn't prepared to cope otherwise. �I doubt
>> this would affect performance in any material way, but it would eat a
>> few more kbytes of storage per backend.
> Hmm, I'm not sure I understand why this is necessary or what our other
> options are.
relcache.c assumes that "critical" relations (those for which we have
hard-wired descriptors in schemapg.h) are always nailed-in-cache. In
the general case this is necessary because we'd not be able to rebuild
the cache entry if it got discarded; eg, without a pg_class entry you're
dead in the water. It's possible we could decouple these attributes;
for instance develop a notion of being nailed only until authentication
finishes, or something like that. I'm not thinking it's worth it
though.
regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane escribió:
>> 2. We don't have infrastructure that would allow access to out-of-line
>> toasted fields during startup. Rather than try to add such, I propose
>> removing pg_authid's toast table, with the consequence that rolpassword
>> cannot be long enough to require out-of-line storage (note it could
>> still be compressed in-line). I cannot imagine any real situation where
>> this would be an issue --- does anyone else? (BTW, I'm fairly sure that
>> we couldn't support an out-of-line rolpassword in the past anyway,
>> because of restrictions in the old flatfiles code.)
> In the past rolconfig could have been a problem too, but fortunately we
> got that moved out. I really doubt that a password of "only" about 2kB
> compressed is going to be a limitation to anyone on this planet. (Hmm,
> isn't it really 8kB in row length that's the hard limit?)
Actually, rolconfig would be OK because it doesn't have to be accessed
until after we've completed authentication. However there's no really
nice way to ensure that rolpassword doesn't get toasted if another
column can be. I suppose we could have initdb force its attstorage to
PLAIN or some such.
> This could perhaps be an issue if we were to store an SSL certificate in
> rolpassword or something like that, but I don't think we support that.
Nope, not that I know of. Anyway we could solve the problem if it ever
came up --- I don't think there's anything insurmountable about
accessing shared toast tables here, we'd just need some support to allow
them to be nailed-in-cache.
>> 3. We'd have to nail pg_authid, pg_auth_members, and their indexes into
>> relcache, because relcache.c isn't prepared to cope otherwise. I doubt
>> this would affect performance in any material way, but it would eat a
>> few more kbytes of storage per backend.
> This doesn't limit that VACUUM FULL or other commands are applied to
> those catalogs, right?
Nope, it's no different from pg_database.
Attached is a draft patch --- it looks pretty reasonable, but I've not
tested the impact on walsender yet.
regards, tom lane
Index: src/backend/catalog/catalog.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/catalog.c,v
retrieving revision 1.89
diff -c -r1.89 catalog.c
*** src/backend/catalog/catalog.c 26 Feb 2010 02:00:36 -0000 1.89
--- src/backend/catalog/catalog.c 20 Apr 2010 21:28:40 -0000
***************
*** 327,335 ****
relationId == DbRoleSettingDatidRolidIndexId)
return true;
/* These are their toast tables and toast indexes (see toasting.h) */
! if (relationId == PgAuthidToastTable ||
! relationId == PgAuthidToastIndex ||
! relationId == PgDatabaseToastTable ||
relationId == PgDatabaseToastIndex ||
relationId == PgShdescriptionToastTable ||
relationId == PgShdescriptionToastIndex ||
--- 327,333 ----
relationId == DbRoleSettingDatidRolidIndexId)
return true;
/* These are their toast tables and toast indexes (see toasting.h) */
! if (relationId == PgDatabaseToastTable ||
relationId == PgDatabaseToastIndex ||
relationId == PgShdescriptionToastTable ||
relationId == PgShdescriptionToastIndex ||
Index: src/backend/catalog/genbki.pl
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/genbki.pl,v
retrieving revision 1.7
diff -c -r1.7 genbki.pl
*** src/backend/catalog/genbki.pl 22 Jan 2010 16:40:18 -0000 1.7
--- src/backend/catalog/genbki.pl 20 Apr 2010 21:28:40 -0000
***************
*** 183,195 ****
# Generate entries for user attributes.
my $attnum = 0;
my @user_attrs = @{ $table->{columns} };
foreach my $attr (@user_attrs)
{
$attnum++;
! my $row = emit_pgattr_row($table_name, $attr);
$row->{attnum} = $attnum;
$row->{attstattarget} = '-1';
# If it's bootstrapped, put an entry in postgres.bki.
if ($is_bootstrap eq ' bootstrap')
--- 183,197 ----
# Generate entries for user attributes.
my $attnum = 0;
+ my $priornotnull = 1;
my @user_attrs = @{ $table->{columns} };
foreach my $attr (@user_attrs)
{
$attnum++;
! my $row = emit_pgattr_row($table_name, $attr, $priornotnull);
$row->{attnum} = $attnum;
$row->{attstattarget} = '-1';
+ $priornotnull &= ($row->{attnotnull} eq 't');
# If it's bootstrapped, put an entry in postgres.bki.
if ($is_bootstrap eq ' bootstrap')
***************
*** 221,227 ****
foreach my $attr (@SYS_ATTRS)
{
$attnum--;
! my $row = emit_pgattr_row($table_name, $attr);
$row->{attnum} = $attnum;
$row->{attstattarget} = '0';
--- 223,229 ----
foreach my $attr (@SYS_ATTRS)
{
$attnum--;
! my $row = emit_pgattr_row($table_name, $attr, 1);
$row->{attnum} = $attnum;
$row->{attstattarget} = '0';
***************
*** 308,317 ****
# Given a system catalog name and a reference to a key-value pair corresponding
# to the name and type of a column, generate a reference to a hash that
! # represents a pg_attribute entry
sub emit_pgattr_row
{
! my ($table_name, $attr) = @_;
my ($attname, $atttype) = %$attr;
my %row;
--- 310,320 ----
# Given a system catalog name and a reference to a key-value pair corresponding
# to the name and type of a column, generate a reference to a hash that
! # represents a pg_attribute entry. We must also be told whether preceding
! # columns were all not-null.
sub emit_pgattr_row
{
! my ($table_name, $attr, $priornotnull) = @_;
my ($attname, $atttype) = %$attr;
my %row;
***************
*** 337,351 ****
$row{attalign} = $type->{typalign};
# set attndims if it's an array type
$row{attndims} = $type->{typcategory} eq 'A' ? '1' : '0';
! # This approach to attnotnull is not really good enough;
! # we need to know about prior column types too. Look at
! # DefineAttr in bootstrap.c. For the moment it's okay for
! # the column orders appearing in bootstrapped catalogs.
! $row{attnotnull} =
! $type->{typname} eq 'oidvector' ? 't'
! : $type->{typname} eq 'int2vector' ? 't'
! : $type->{typlen} eq 'NAMEDATALEN' ? 't'
! : $type->{typlen} > 0 ? 't' : 'f';
last;
}
}
--- 340,360 ----
$row{attalign} = $type->{typalign};
# set attndims if it's an array type
$row{attndims} = $type->{typcategory} eq 'A' ? '1' : '0';
! # attnotnull must be set true if the type is fixed-width and
! # prior columns are too --- compare DefineAttr in bootstrap.c.
! # oidvector and int2vector are also treated as not-nullable.
! if ($priornotnull)
! {
! $row{attnotnull} =
! $type->{typname} eq 'oidvector' ? 't'
! : $type->{typname} eq 'int2vector' ? 't'
! : $type->{typlen} eq 'NAMEDATALEN' ? 't'
! : $type->{typlen} > 0 ? 't' : 'f';
! }
! else
! {
! $row{attnotnull} = 'f';
! }
last;
}
}
Index: src/backend/utils/cache/catcache.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v
retrieving revision 1.151
diff -c -r1.151 catcache.c
*** src/backend/utils/cache/catcache.c 14 Feb 2010 18:42:17 -0000 1.151
--- src/backend/utils/cache/catcache.c 20 Apr 2010 21:28:40 -0000
***************
*** 981,1014 ****
* certain system indexes that support critical syscaches.
* We can't use an indexscan to fetch these, else we'll get into
* infinite recursion. A plain heap scan will work, however.
- *
* Once we have completed relcache initialization (signaled by
* criticalRelcachesBuilt), we don't have to worry anymore.
*/
static bool
IndexScanOK(CatCache *cache, ScanKey cur_skey)
{
! if (cache->id == INDEXRELID)
{
! /*
! * Rather than tracking exactly which indexes have to be loaded before
! * we can use indexscans (which changes from time to time), just force
! * all pg_index searches to be heap scans until we've built the
! * critical relcaches.
! */
! if (!criticalRelcachesBuilt)
return false;
! }
! else if (cache->id == AMOID ||
! cache->id == AMNAME)
! {
! /*
! * Always do heap scans in pg_am, because it's so small there's not
! * much point in an indexscan anyway. We *must* do this when
! * initially building critical relcache entries, but we might as well
! * just always do it.
! */
! return false;
}
/* Normal case, allow index scan */
--- 981,1032 ----
* certain system indexes that support critical syscaches.
* We can't use an indexscan to fetch these, else we'll get into
* infinite recursion. A plain heap scan will work, however.
* Once we have completed relcache initialization (signaled by
* criticalRelcachesBuilt), we don't have to worry anymore.
+ *
+ * Similarly, during backend startup we have to be able to use the
+ * pg_authid and pg_auth_members syscaches for authentication even if
+ * we don't yet have relcache entries for those catalogs' indexes.
*/
static bool
IndexScanOK(CatCache *cache, ScanKey cur_skey)
{
! switch (cache->id)
{
! case INDEXRELID:
! /*
! * Rather than tracking exactly which indexes have to be loaded
! * before we can use indexscans (which changes from time to time),
! * just force all pg_index searches to be heap scans until we've
! * built the critical relcaches.
! */
! if (!criticalRelcachesBuilt)
! return false;
! break;
!
! case AMOID:
! case AMNAME:
! /*
! * Always do heap scans in pg_am, because it's so small there's
! * not much point in an indexscan anyway. We *must* do this when
! * initially building critical relcache entries, but we might as
! * well just always do it.
! */
return false;
!
! case AUTHNAME:
! case AUTHOID:
! case AUTHMEMMEMROLE:
! /*
! * Protect authentication lookups occurring before relcache has
! * collected entries for shared indexes.
! */
! if (!criticalSharedRelcachesBuilt)
! return false;
! break;
!
! default:
! break;
}
/* Normal case, allow index scan */
***************
*** 1397,1403 ****
scandesc = systable_beginscan(relation,
cache->cc_indexoid,
! true,
SnapshotNow,
nkeys,
cur_skey);
--- 1415,1421 ----
scandesc = systable_beginscan(relation,
cache->cc_indexoid,
! IndexScanOK(cache, cur_skey),
SnapshotNow,
nkeys,
cur_skey);
Index: src/backend/utils/cache/relcache.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v
retrieving revision 1.309
diff -c -r1.309 relcache.c
*** src/backend/utils/cache/relcache.c 14 Apr 2010 21:31:11 -0000 1.309
--- src/backend/utils/cache/relcache.c 20 Apr 2010 21:28:40 -0000
***************
*** 43,48 ****
--- 43,49 ----
#include "catalog/pg_amproc.h"
#include "catalog/pg_attrdef.h"
#include "catalog/pg_authid.h"
+ #include "catalog/pg_auth_members.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_database.h"
#include "catalog/pg_namespace.h"
***************
*** 87,99 ****
#define RELCACHE_INIT_FILEMAGIC 0x573265 /* version ID value */
/*
! * hardcoded tuple descriptors. see include/catalog/pg_attribute.h
*/
static const FormData_pg_attribute Desc_pg_class[Natts_pg_class] = {Schema_pg_class};
static const FormData_pg_attribute Desc_pg_attribute[Natts_pg_attribute] = {Schema_pg_attribute};
static const FormData_pg_attribute Desc_pg_proc[Natts_pg_proc] = {Schema_pg_proc};
static const FormData_pg_attribute Desc_pg_type[Natts_pg_type] = {Schema_pg_type};
static const FormData_pg_attribute Desc_pg_database[Natts_pg_database] = {Schema_pg_database};
static const FormData_pg_attribute Desc_pg_index[Natts_pg_index] = {Schema_pg_index};
/*
--- 88,102 ----
#define RELCACHE_INIT_FILEMAGIC 0x573265 /* version ID value */
/*
! * hardcoded tuple descriptors, generated by genbki.pl
*/
static const FormData_pg_attribute Desc_pg_class[Natts_pg_class] = {Schema_pg_class};
static const FormData_pg_attribute Desc_pg_attribute[Natts_pg_attribute] = {Schema_pg_attribute};
static const FormData_pg_attribute Desc_pg_proc[Natts_pg_proc] = {Schema_pg_proc};
static const FormData_pg_attribute Desc_pg_type[Natts_pg_type] = {Schema_pg_type};
static const FormData_pg_attribute Desc_pg_database[Natts_pg_database] = {Schema_pg_database};
+ static const FormData_pg_attribute Desc_pg_authid[Natts_pg_authid] = {Schema_pg_authid};
+ static const FormData_pg_attribute Desc_pg_auth_members[Natts_pg_auth_members] = {Schema_pg_auth_members};
static const FormData_pg_attribute Desc_pg_index[Natts_pg_index] = {Schema_pg_index};
/*
***************
*** 118,124 ****
/*
* This flag is false until we have prepared the critical relcache entries
! * for shared catalogs (specifically, pg_database and its indexes).
*/
bool criticalSharedRelcachesBuilt = false;
--- 121,127 ----
/*
* This flag is false until we have prepared the critical relcache entries
! * for shared catalogs (which are the tables needed for login).
*/
bool criticalSharedRelcachesBuilt = false;
***************
*** 1379,1386 ****
* quite a lot since we only need to work for a few basic system
* catalogs.
*
! * formrdesc is currently used for: pg_database, pg_class, pg_attribute,
! * pg_proc, and pg_type (see RelationCacheInitializePhase2/3).
*
* Note that these catalogs can't have constraints (except attnotnull),
* default values, rules, or triggers, since we don't cope with any of that.
--- 1382,1390 ----
* quite a lot since we only need to work for a few basic system
* catalogs.
*
! * formrdesc is currently used for: pg_database, pg_authid, pg_auth_members,
! * pg_class, pg_attribute, pg_proc, and pg_type
! * (see RelationCacheInitializePhase2/3).
*
* Note that these catalogs can't have constraints (except attnotnull),
* default values, rules, or triggers, since we don't cope with any of that.
***************
*** 1461,1468 ****
* initialize attribute tuple form
*
* Unlike the case with the relation tuple, this data had better be right
! * because it will never be replaced. The input values must be correctly
! * defined by macros in src/include/catalog/ headers.
*/
relation->rd_att = CreateTemplateTupleDesc(natts, hasoids);
relation->rd_att->tdrefcount = 1; /* mark as refcounted */
--- 1465,1472 ----
* initialize attribute tuple form
*
* Unlike the case with the relation tuple, this data had better be right
! * because it will never be replaced. The data comes from
! * src/include/catalog/ headers via genbki.pl.
*/
relation->rd_att = CreateTemplateTupleDesc(natts, hasoids);
relation->rd_att->tdrefcount = 1; /* mark as refcounted */
***************
*** 2455,2460 ****
--- 2459,2466 ----
switch (relid)
{
case DatabaseRelationId:
+ case AuthIdRelationId:
+ case AuthMemRelationId:
case RelationRelationId:
case AttributeRelationId:
case ProcedureRelationId:
***************
*** 2750,2761 ****
/*
* RelationCacheInitializePhase2
*
! * This is called to prepare for access to pg_database during startup.
! * We must at least set up a nailed reldesc for pg_database. Ideally
! * we'd like to have reldescs for its indexes, too. We attempt to
! * load this information from the shared relcache init file. If that's
! * missing or broken, just make a phony entry for pg_database.
! * RelationCacheInitializePhase3 will clean up as needed.
*/
void
RelationCacheInitializePhase2(void)
--- 2756,2768 ----
/*
* RelationCacheInitializePhase2
*
! * This is called to prepare for access to shared catalogs during startup.
! * We must at least set up nailed reldescs for pg_database, pg_authid,
! * and pg_auth_members. Ideally we'd like to have reldescs for their
! * indexes, too. We attempt to load this information from the shared
! * relcache init file. If that's missing or broken, just make phony
! * entries for the catalogs themselves. RelationCacheInitializePhase3
! * will clean up as needed.
*/
void
RelationCacheInitializePhase2(void)
***************
*** 2768,2774 ****
RelationMapInitializePhase2();
/*
! * In bootstrap mode, pg_database isn't there yet anyway, so do nothing.
*/
if (IsBootstrapProcessingMode())
return;
--- 2775,2782 ----
RelationMapInitializePhase2();
/*
! * In bootstrap mode, the shared catalogs aren't there yet anyway,
! * so do nothing.
*/
if (IsBootstrapProcessingMode())
return;
***************
*** 2780,2793 ****
/*
* Try to load the shared relcache cache file. If unsuccessful, bootstrap
! * the cache with a pre-made descriptor for pg_database.
*/
if (!load_relcache_init_file(true))
{
formrdesc("pg_database", DatabaseRelation_Rowtype_Id, true,
true, Natts_pg_database, Desc_pg_database);
! #define NUM_CRITICAL_SHARED_RELS 1 /* fix if you change list above */
}
MemoryContextSwitchTo(oldcxt);
--- 2788,2805 ----
/*
* Try to load the shared relcache cache file. If unsuccessful, bootstrap
! * the cache with pre-made descriptors for the critical shared catalogs.
*/
if (!load_relcache_init_file(true))
{
formrdesc("pg_database", DatabaseRelation_Rowtype_Id, true,
true, Natts_pg_database, Desc_pg_database);
+ formrdesc("pg_authid", AuthIdRelation_Rowtype_Id, true,
+ true, Natts_pg_authid, Desc_pg_authid);
+ formrdesc("pg_auth_members", AuthMemRelation_Rowtype_Id, true,
+ false, Natts_pg_auth_members, Desc_pg_auth_members);
! #define NUM_CRITICAL_SHARED_RELS 3 /* fix if you change list above */
}
MemoryContextSwitchTo(oldcxt);
***************
*** 2910,2916 ****
* DatabaseNameIndexId isn't critical for relcache loading, but rather for
* initial lookup of MyDatabaseId, without which we'll never find any
* non-shared catalogs at all. Autovacuum calls InitPostgres with a
! * database OID, so it instead depends on DatabaseOidIndexId.
*/
if (!criticalSharedRelcachesBuilt)
{
--- 2922,2930 ----
* DatabaseNameIndexId isn't critical for relcache loading, but rather for
* initial lookup of MyDatabaseId, without which we'll never find any
* non-shared catalogs at all. Autovacuum calls InitPostgres with a
! * database OID, so it instead depends on DatabaseOidIndexId. We also
! * need to nail up some indexes on pg_authid and pg_auth_members for use
! * during client authentication.
*/
if (!criticalSharedRelcachesBuilt)
{
***************
*** 2918,2925 ****
DatabaseRelationId);
load_critical_index(DatabaseOidIndexId,
DatabaseRelationId);
! #define NUM_CRITICAL_SHARED_INDEXES 2 /* fix if you change list above */
criticalSharedRelcachesBuilt = true;
}
--- 2932,2945 ----
DatabaseRelationId);
load_critical_index(DatabaseOidIndexId,
DatabaseRelationId);
+ load_critical_index(AuthIdRolnameIndexId,
+ AuthIdRelationId);
+ load_critical_index(AuthIdOidIndexId,
+ AuthIdRelationId);
+ load_critical_index(AuthMemMemRoleIndexId,
+ AuthMemRelationId);
! #define NUM_CRITICAL_SHARED_INDEXES 5 /* fix if you change list above */
criticalSharedRelcachesBuilt = true;
}
Index: src/backend/utils/init/miscinit.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/init/miscinit.c,v
retrieving revision 1.183
diff -c -r1.183 miscinit.c
*** src/backend/utils/init/miscinit.c 26 Feb 2010 02:01:13 -0000 1.183
--- src/backend/utils/init/miscinit.c 20 Apr 2010 21:28:40 -0000
***************
*** 429,438 ****
* These next checks are not enforced when in standalone mode, so that
* there is a way to recover from sillinesses like "UPDATE pg_authid SET
* rolcanlogin = false;".
- *
- * We do not enforce them for the autovacuum process either.
*/
! if (IsUnderPostmaster && !IsAutoVacuumWorkerProcess())
{
/*
* Is role allowed to login at all?
--- 429,436 ----
* These next checks are not enforced when in standalone mode, so that
* there is a way to recover from sillinesses like "UPDATE pg_authid SET
* rolcanlogin = false;".
*/
! if (IsUnderPostmaster)
{
/*
* Is role allowed to login at all?
***************
*** 479,485 ****
void
InitializeSessionUserIdStandalone(void)
{
! /* This function should only be called in a single-user backend. */
AssertState(!IsUnderPostmaster || IsAutoVacuumWorkerProcess());
/* call only once */
--- 477,486 ----
void
InitializeSessionUserIdStandalone(void)
{
! /*
! * This function should only be called in single-user mode and in
! * autovacuum workers.
! */
AssertState(!IsUnderPostmaster || IsAutoVacuumWorkerProcess());
/* call only once */
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.209
diff -c -r1.209 postinit.c
*** src/backend/utils/init/postinit.c 20 Apr 2010 01:38:52 -0000 1.209
--- src/backend/utils/init/postinit.c 20 Apr 2010 21:28:40 -0000
***************
*** 552,558 ****
/*
* Load relcache entries for the shared system catalogs. This must create
! * at least an entry for pg_database.
*/
RelationCacheInitializePhase2();
--- 552,558 ----
/*
* Load relcache entries for the shared system catalogs. This must create
! * at least entries for pg_database and catalogs used for authentication.
*/
RelationCacheInitializePhase2();
***************
*** 586,598 ****
}
/*
* Set up the global variables holding database id and default tablespace.
* But note we won't actually try to touch the database just yet.
*
! * We take a shortcut in the bootstrap and walsender case, otherwise we
! * have to look up the db's entry in pg_database.
*/
! if (bootstrap || am_walsender)
{
MyDatabaseId = TemplateDbOid;
MyDatabaseTableSpace = DEFAULTTABLESPACE_OID;
--- 586,644 ----
}
/*
+ * Perform client authentication if necessary, then figure out our
+ * postgres user ID, and see if we are a superuser.
+ *
+ * In standalone mode and in autovacuum worker processes, we use a fixed
+ * ID, otherwise we figure it out from the authenticated user name.
+ */
+ if (bootstrap || IsAutoVacuumWorkerProcess())
+ {
+ InitializeSessionUserIdStandalone();
+ am_superuser = true;
+ }
+ else if (!IsUnderPostmaster)
+ {
+ InitializeSessionUserIdStandalone();
+ am_superuser = true;
+ if (!ThereIsAtLeastOneRole())
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("no roles are defined in this database system"),
+ errhint("You should immediately run CREATE USER \"%s\" SUPERUSER;.",
+ username)));
+ }
+ else
+ {
+ /* normal multiuser case */
+ Assert(MyProcPort != NULL);
+ PerformAuthentication(MyProcPort);
+ InitializeSessionUserId(username);
+ am_superuser = superuser();
+ }
+
+ /*
+ * If walsender, we're done here --- we don't want to connect to any
+ * particular database.
+ */
+ if (am_walsender)
+ {
+ Assert(!bootstrap);
+ /* report this backend in the PgBackendStatus array */
+ pgstat_bestart();
+ /* close the transaction we started above */
+ CommitTransactionCommand();
+ return;
+ }
+
+ /*
* Set up the global variables holding database id and default tablespace.
* But note we won't actually try to touch the database just yet.
*
! * We take a shortcut in the bootstrap case, otherwise we have to look up
! * the db's entry in pg_database.
*/
! if (bootstrap)
{
MyDatabaseId = TemplateDbOid;
MyDatabaseTableSpace = DEFAULTTABLESPACE_OID;
***************
*** 655,661 ****
* AccessShareLock for such sessions and thereby not conflict against
* CREATE DATABASE.
*/
! if (!bootstrap && !am_walsender)
LockSharedObject(DatabaseRelationId, MyDatabaseId, 0,
RowExclusiveLock);
--- 701,707 ----
* AccessShareLock for such sessions and thereby not conflict against
* CREATE DATABASE.
*/
! if (!bootstrap)
LockSharedObject(DatabaseRelationId, MyDatabaseId, 0,
RowExclusiveLock);
***************
*** 664,670 ****
* If there was a concurrent DROP DATABASE, this ensures we will die
* cleanly without creating a mess.
*/
! if (!bootstrap && !am_walsender)
{
HeapTuple tuple;
--- 710,716 ----
* If there was a concurrent DROP DATABASE, this ensures we will die
* cleanly without creating a mess.
*/
! if (!bootstrap)
{
HeapTuple tuple;
***************
*** 684,690 ****
*/
fullpath = GetDatabasePath(MyDatabaseId, MyDatabaseTableSpace);
! if (!bootstrap && !am_walsender)
{
if (access(fullpath, F_OK) == -1)
{
--- 730,736 ----
*/
fullpath = GetDatabasePath(MyDatabaseId, MyDatabaseTableSpace);
! if (!bootstrap)
{
if (access(fullpath, F_OK) == -1)
{
***************
*** 715,765 ****
*/
RelationCacheInitializePhase3();
- /*
- * Perform client authentication if necessary, then figure out our
- * postgres user ID, and see if we are a superuser.
- *
- * In standalone mode and in autovacuum worker processes, we use a fixed
- * ID, otherwise we figure it out from the authenticated user name.
- */
- if (bootstrap || IsAutoVacuumWorkerProcess())
- {
- InitializeSessionUserIdStandalone();
- am_superuser = true;
- }
- else if (!IsUnderPostmaster)
- {
- InitializeSessionUserIdStandalone();
- am_superuser = true;
- if (!ThereIsAtLeastOneRole())
- ereport(WARNING,
- (errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("no roles are defined in this database system"),
- errhint("You should immediately run CREATE USER \"%s\" SUPERUSER;.",
- username)));
- }
- else
- {
- /* normal multiuser case */
- Assert(MyProcPort != NULL);
- PerformAuthentication(MyProcPort);
- InitializeSessionUserId(username);
- am_superuser = superuser();
- }
-
/* set up ACL framework (so CheckMyDatabase can check permissions) */
initialize_acl();
- /* Process pg_db_role_setting options */
- process_settings(MyDatabaseId, GetSessionUserId());
-
/*
* Re-read the pg_database row for our database, check permissions and set
* up database-specific GUC settings. We can't do this until all the
* database-access infrastructure is up. (Also, it wants to know if the
* user is a superuser, so the above stuff has to happen first.)
*/
! if (!bootstrap && !am_walsender)
CheckMyDatabase(dbname, am_superuser);
/*
--- 761,776 ----
*/
RelationCacheInitializePhase3();
/* set up ACL framework (so CheckMyDatabase can check permissions) */
initialize_acl();
/*
* Re-read the pg_database row for our database, check permissions and set
* up database-specific GUC settings. We can't do this until all the
* database-access infrastructure is up. (Also, it wants to know if the
* user is a superuser, so the above stuff has to happen first.)
*/
! if (!bootstrap)
CheckMyDatabase(dbname, am_superuser);
/*
***************
*** 841,846 ****
--- 852,860 ----
}
}
+ /* Process pg_db_role_setting options */
+ process_settings(MyDatabaseId, GetSessionUserId());
+
/* Apply PostAuthDelay as soon as we've read all options */
if (PostAuthDelay > 0)
pg_usleep(PostAuthDelay * 1000000L);
***************
*** 856,865 ****
/* initialize client encoding */
InitializeClientEncoding();
- /* reset the database for walsender */
- if (am_walsender)
- MyProc->databaseId = MyDatabaseId = InvalidOid;
-
/* report this backend in the PgBackendStatus array */
if (!bootstrap)
pgstat_bestart();
--- 870,875 ----
Index: src/include/catalog/catversion.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/catversion.h,v
retrieving revision 1.585
diff -c -r1.585 catversion.h
*** src/include/catalog/catversion.h 16 Feb 2010 22:34:54 -0000 1.585
--- src/include/catalog/catversion.h 20 Apr 2010 21:28:40 -0000
***************
*** 53,58 ****
*/
/* yyyymmddN */
! #define CATALOG_VERSION_NO 201002161
#endif
--- 53,58 ----
*/
/* yyyymmddN */
! #define CATALOG_VERSION_NO 201004201
#endif
Index: src/include/catalog/pg_auth_members.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_auth_members.h,v
retrieving revision 1.8
diff -c -r1.8 pg_auth_members.h
*** src/include/catalog/pg_auth_members.h 5 Jan 2010 01:06:56 -0000 1.8
--- src/include/catalog/pg_auth_members.h 20 Apr 2010 21:28:40 -0000
***************
*** 27,34 ****
* ----------------
*/
#define AuthMemRelationId 1261
! CATALOG(pg_auth_members,1261) BKI_SHARED_RELATION BKI_WITHOUT_OIDS
{
Oid roleid; /* ID of a role */
Oid member; /* ID of a member of that role */
--- 27,35 ----
* ----------------
*/
#define AuthMemRelationId 1261
+ #define AuthMemRelation_Rowtype_Id 2843
! CATALOG(pg_auth_members,1261) BKI_SHARED_RELATION BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(2843) BKI_SCHEMA_MACRO
{
Oid roleid; /* ID of a role */
Oid member; /* ID of a member of that role */
Index: src/include/catalog/pg_authid.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_authid.h,v
retrieving revision 1.12
diff -c -r1.12 pg_authid.h
*** src/include/catalog/pg_authid.h 5 Jan 2010 01:06:56 -0000 1.12
--- src/include/catalog/pg_authid.h 20 Apr 2010 21:28:40 -0000
***************
*** 40,47 ****
* ----------------
*/
#define AuthIdRelationId 1260
! CATALOG(pg_authid,1260) BKI_SHARED_RELATION
{
NameData rolname; /* name of role */
bool rolsuper; /* read this field via superuser() only! */
--- 40,48 ----
* ----------------
*/
#define AuthIdRelationId 1260
+ #define AuthIdRelation_Rowtype_Id 2842
! CATALOG(pg_authid,1260) BKI_SHARED_RELATION BKI_ROWTYPE_OID(2842) BKI_SCHEMA_MACRO
{
NameData rolname; /* name of role */
bool rolsuper; /* read this field via superuser() only! */
***************
*** 71,77 ****
* compiler constants for pg_authid
* ----------------
*/
! #define Natts_pg_authid 11
#define Anum_pg_authid_rolname 1
#define Anum_pg_authid_rolsuper 2
#define Anum_pg_authid_rolinherit 3
--- 72,78 ----
* compiler constants for pg_authid
* ----------------
*/
! #define Natts_pg_authid 10
#define Anum_pg_authid_rolname 1
#define Anum_pg_authid_rolsuper 2
#define Anum_pg_authid_rolinherit 3
Index: src/include/catalog/toasting.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/toasting.h,v
retrieving revision 1.13
diff -c -r1.13 toasting.h
*** src/include/catalog/toasting.h 6 Jan 2010 03:04:03 -0000 1.13
--- src/include/catalog/toasting.h 20 Apr 2010 21:28:40 -0000
***************
*** 49,57 ****
DECLARE_TOAST(pg_trigger, 2336, 2337);
/* shared catalogs */
- DECLARE_TOAST(pg_authid, 2842, 2843);
- #define PgAuthidToastTable 2842
- #define PgAuthidToastIndex 2843
DECLARE_TOAST(pg_database, 2844, 2845);
#define PgDatabaseToastTable 2844
#define PgDatabaseToastIndex 2845
--- 49,54 ----
On Tue, Apr 20, 2010 at 5:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Apr 20, 2010 at 2:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> 3. We'd have to nail pg_authid, pg_auth_members, and their indexes into >>> relcache, because relcache.c isn't prepared to cope otherwise. I doubt >>> this would affect performance in any material way, but it would eat a >>> few more kbytes of storage per backend. > >> Hmm, I'm not sure I understand why this is necessary or what our other >> options are. > > relcache.c assumes that "critical" relations (those for which we have > hard-wired descriptors in schemapg.h) are always nailed-in-cache. In > the general case this is necessary because we'd not be able to rebuild > the cache entry if it got discarded; eg, without a pg_class entry you're > dead in the water. It's possible we could decouple these attributes; > for instance develop a notion of being nailed only until authentication > finishes, or something like that. I'm not thinking it's worth it > though. Well that just begs the question - why do we need a hard-wired descriptor? Presumably we should only need to hard-wired descriptors for the relations are used by the relcache code itself to build more descriptors - so clearly pg_cache and pg_attribute, but beyond that I don't get it. In particular, I can't see any reason why we couldn't just build the descriptor for pg_authid etc. by scanning pg_class and pg_attribute. ...Robert
On Tue, Apr 20, 2010 at 5:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Apr 20, 2010 at 5:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >>> On Tue, Apr 20, 2010 at 2:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> 3. We'd have to nail pg_authid, pg_auth_members, and their indexes into >>>> relcache, because relcache.c isn't prepared to cope otherwise. I doubt >>>> this would affect performance in any material way, but it would eat a >>>> few more kbytes of storage per backend. >> >>> Hmm, I'm not sure I understand why this is necessary or what our other >>> options are. >> >> relcache.c assumes that "critical" relations (those for which we have >> hard-wired descriptors in schemapg.h) are always nailed-in-cache. In >> the general case this is necessary because we'd not be able to rebuild >> the cache entry if it got discarded; eg, without a pg_class entry you're >> dead in the water. It's possible we could decouple these attributes; >> for instance develop a notion of being nailed only until authentication >> finishes, or something like that. I'm not thinking it's worth it >> though. > > Well that just begs the question - why do we need a hard-wired > descriptor? Presumably we should only need to hard-wired descriptors > for the relations are used by the relcache code itself to build more > descriptors - so clearly pg_cache and pg_attribute, but beyond that I > don't get it. In particular, I can't see any reason why we couldn't > just build the descriptor for pg_authid etc. by scanning pg_class and > pg_attribute. I suppose the problem here is that pg_attribute and pg_class are not shared catalogs, so we can't read them without selecting a database. What about making a fake version of these relations that includes only the shared catalogs? ...Robert
Robert Haas escribió: > I suppose the problem here is that pg_attribute and pg_class are not > shared catalogs, so we can't read them without selecting a database. > What about making a fake version of these relations that includes only > the shared catalogs? Hmm, interesting. I wonder if something of this sort would allow one to create a shared relation at the user level -- right now the set of shared relations is hardcoded so this cannot work. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Robert Haas <robertmhaas@gmail.com> writes:
> I suppose the problem here is that pg_attribute and pg_class are not
> shared catalogs, so we can't read them without selecting a database.
Among other things.
> What about making a fake version of these relations that includes only
> the shared catalogs?
Well, after you solve the few dozen problems standing in the way
of that, go right ahead. I'm not holding up 9.0 for it though.
(You might want to look back at the archived discussions about how to
avoid storing entries for temp tables in these catalogs; that poses
many of the same issues.)
regards, tom lane
On Tue, Apr 20, 2010 at 7:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I suppose the problem here is that pg_attribute and pg_class are not >> shared catalogs, so we can't read them without selecting a database. > > Among other things. > >> What about making a fake version of these relations that includes only >> the shared catalogs? > > Well, after you solve the few dozen problems standing in the way > of that, go right ahead. I'm not holding up 9.0 for it though. > > (You might want to look back at the archived discussions about how to > avoid storing entries for temp tables in these catalogs; that poses > many of the same issues.) Do you happen to know what a good search term might be? I tried searching for things like "pg_class temp tables" and "pg_class temporary tables" and didn't come up with much. The closest thing I found was a discussion about global temp tables (subject aws "idea: global temp tables") in which Greg Stark was arguing that there wasn't much point in implementing them without solving this issue (and others were disagreeing) but it didn't get into any of the technical issues at all. ...Robert
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Apr 20, 2010 at 7:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> (You might want to look back at the archived discussions about how to >> avoid storing entries for temp tables in these catalogs; that poses >> many of the same issues.) > Do you happen to know what a good search term might be? I tried > searching for things like "pg_class temp tables" and "pg_class > temporary tables" and didn't come up with much. I found this thread: http://archives.postgresql.org/pgsql-hackers/2008-07/msg00593.php I claimed in that message that there were previous discussions but I did not come across them right away. regards, tom lane
Tom Lane escribió: > Robert Haas <robertmhaas@gmail.com> writes: > > On Tue, Apr 20, 2010 at 7:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> (You might want to look back at the archived discussions about how to > >> avoid storing entries for temp tables in these catalogs; that poses > >> many of the same issues.) > > > Do you happen to know what a good search term might be? I tried > > searching for things like "pg_class temp tables" and "pg_class > > temporary tables" and didn't come up with much. > > I found this thread: > http://archives.postgresql.org/pgsql-hackers/2008-07/msg00593.php > I claimed in that message that there were previous discussions but > I did not come across them right away. I vaguely remember that there was a discussion about pg_attribute and the extra rows for system rows for all tables, which diverged into a discussion about temp tables and those other extra rows. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support