Thread: Re: [HACKERS] unprivileged contrib and pl install (formerly tsearch
On Wed, 24 Jan 2007, Jeremy Drake wrote:
> On Wed, 24 Jan 2007, Martijn van Oosterhout wrote:
>
> > Something I've wondered about before is the concept of having installed
> > Modules in the system. Let's say for example that while compiling
> > postgres it compiled the modules in contrib also and installed them in
> > a modules directory.
> >
> > Once installed there, unpriviledged users could say "INSTALL foo" and
> > it would install the module, even if they do not have the permissions
> > to create them themselves.
>
> That would be great, and also it would be great to be able to CREATE
> LANGUAGE as a regular user for a trusted pl that is already
> compiled/installed.
Something like the attached (simple) change to allow CREATE LANGUAGE by
unprivileged users for trusted languages already present in pg_pltemplate.
I'm not quite sure how one would go about doing the module thing, I think
that would be more complex. Something simple like allowing creation of C
language functions in libraries in $libdir would probably not be
sufficient, because an unprivileged user could create functions that have
the wrong paramters or return values and crash things pretty good that
way. Any ideas how this would work? Perhaps a sql script in
sharedir could be run by the backend as though by a superuser...
--
Ed Sullivan will be around as long as someone else has talent.
-- Fred Allen
Attachment
Jeremy Drake <pgsql@jdrake.com> writes:
> On Wed, 24 Jan 2007, Jeremy Drake wrote:
>> That would be great, and also it would be great to be able to CREATE
>> LANGUAGE as a regular user for a trusted pl that is already
>> compiled/installed.
> Something like the attached (simple) change to allow CREATE LANGUAGE by
> unprivileged users for trusted languages already present in pg_pltemplate.
If it were merely a matter of removing an error check I think we would
have done it already. However, pltemplate will have all the languages
in it whether the DBA wants to allow them to be used or not; so I'd say
that there really needs to be *some* sort of privilege check here.
What that is and how to implement it are the hard parts.
regards, tom lane
On Wed, 24 Jan 2007, Tom Lane wrote:
> Jeremy Drake <pgsql@jdrake.com> writes:
> > On Wed, 24 Jan 2007, Jeremy Drake wrote:
> >> That would be great, and also it would be great to be able to CREATE
> >> LANGUAGE as a regular user for a trusted pl that is already
> >> compiled/installed.
>
> > Something like the attached (simple) change to allow CREATE LANGUAGE by
> > unprivileged users for trusted languages already present in pg_pltemplate.
>
> If it were merely a matter of removing an error check I think we would
> have done it already. However, pltemplate will have all the languages
> in it whether the DBA wants to allow them to be used or not; so I'd say
> that there really needs to be *some* sort of privilege check here.
> What that is and how to implement it are the hard parts.
So I guess it depends on what you mean by "DBA". Perhaps the database
owner? Or some new privilege type (GRANT CREATE ON LANGUAGE ...? Or GRANT
CREATE LANGUAGE ON DATABASE...?) that the db owner has by default?
--
7:30, Channel 5: The Bionic Dog (Action/Adventure)
The Bionic Dog drinks too much and kicks over the National
Redwood Forest.
Jeremy Drake <pgsql@jdrake.com> writes:
> On Wed, 24 Jan 2007, Tom Lane wrote:
>> that there really needs to be *some* sort of privilege check here.
>> What that is and how to implement it are the hard parts.
> So I guess it depends on what you mean by "DBA". Perhaps the database
> owner? Or some new privilege type (GRANT CREATE ON LANGUAGE ...? Or GRANT
> CREATE LANGUAGE ON DATABASE...?) that the db owner has by default?
Not the DB owner. If you are worried about whether to allow use of PLs
it's almost certainly an installation-wide security concern, so I'd say
that the privilege has to flow from a superuser.
GRANT CREATE ON LANGUAGE feeding into a flag bit in pltemplate would
work, if restricted to superusers, but I suspect people would find this
confusing because it'd work completely differently from GRANT USAGE ON
LANGUAGE (eg, because the latter has only database-local effects).
Might be better to use a different syntax.
Note I'm not arguing against allowing it to be "on" by default, I just
want to be sure there is a way for paranoid DBAs to turn it off. Maybe
it'd be sufficient if the flag bit was there but "UPDATE pg_pltemplate"
was the only way to manipulate it --- we've gotten along with treating
datistemplate and datallowconn that way.
Or we could go the full nine yards and add ACLs to pltemplate, but
that's probably overkill.
regards, tom lane
On Wed, 24 Jan 2007, Tom Lane wrote: > Not the DB owner. If you are worried about whether to allow use of PLs > it's almost certainly an installation-wide security concern, so I'd say > that the privilege has to flow from a superuser. > > GRANT CREATE ON LANGUAGE feeding into a flag bit in pltemplate would > work, if restricted to superusers, but I suspect people would find this > confusing because it'd work completely differently from GRANT USAGE ON > LANGUAGE (eg, because the latter has only database-local effects). > Might be better to use a different syntax. I had thought that it would be database-local, but I understand now that it makes more sense to be global. > > Note I'm not arguing against allowing it to be "on" by default, I just > want to be sure there is a way for paranoid DBAs to turn it off. Maybe > it'd be sufficient if the flag bit was there but "UPDATE pg_pltemplate" > was the only way to manipulate it --- we've gotten along with treating > datistemplate and datallowconn that way. That sounds reasonable to me. I'll try to put together a patch like this (adding a boolean column to pg_pltemplate) and see if this is acceptable. I assume that only superusers can modify pg_pltemplate already ;) > Or we could go the full nine yards and add ACLs to pltemplate, but > that's probably overkill. Agreed. -- He thought he saw an albatross That fluttered 'round the lamp. He looked again and saw it was A penny postage stamp. "You'd best be getting home," he said, "The nights are rather damp."
On Wed, 24 Jan 2007, Tom Lane wrote: > In detail, it'd look something like: > > * For an untrusted language: must be superuser to either create or use > the language (no change from current rules). Ownership of the > pg_language entry is really irrelevant, as is its ACL. > > * For a trusted language: > > * if pg_pltemplate.something is ON: either a superuser or the current > DB's owner can CREATE the language. In either case the pg_language > entry will be marked as owned by the DB owner (pg_database.datdba), > which means that subsequently he (or a superuser) can grant or deny > USAGE within his DB. > > * if pg_pltemplate.something is OFF: must be superuser to CREATE the > language; subsequently it will be owned by you, so only you or another > superuser can grant or deny USAGE (same behavior as currently). I think I have what is described here implemented in this patch, so that it can be better understood. Thoughts? -- Nobody said computers were going to be polite.
Attachment
On Wed, 24 Jan 2007, Jeremy Drake wrote:
> On Wed, 24 Jan 2007, Tom Lane wrote:
>
> > In detail, it'd look something like:
> >
> > * For an untrusted language: must be superuser to either create or use
> > the language (no change from current rules). Ownership of the
> > pg_language entry is really irrelevant, as is its ACL.
> >
> > * For a trusted language:
> >
> > * if pg_pltemplate.something is ON: either a superuser or the current
> > DB's owner can CREATE the language. In either case the pg_language
> > entry will be marked as owned by the DB owner (pg_database.datdba),
> > which means that subsequently he (or a superuser) can grant or deny
> > USAGE within his DB.
> >
> > * if pg_pltemplate.something is OFF: must be superuser to CREATE the
> > language; subsequently it will be owned by you, so only you or another
> > superuser can grant or deny USAGE (same behavior as currently).
>
> I think I have what is described here implemented in this patch, so that
> it can be better understood. Thoughts?
This version of the patch creates a shared dependency on the language
owner.
I have thought of some other questions about the owner stuff which I will
send on -hackers...
--
Afternoon, n.:
That part of the day we spend worrying about how we wasted the
morning.
Attachment
On Thu, 25 Jan 2007, Jeremy Drake wrote: > I think that an ALTER LANGUAGE OWNER TO is the proper response to these > things, and unless I hear otherwise I will attempt to add this to my > patch. Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME TO for the owner, which I missed before. I would appreciate someone with more knowledge of the permissions infrastructure to take a look at it since I am fairly new to it and may not fully understand its intricacies. -- The makers may make And the users may use, But the fixers must fix With but minimal clues
Attachment
On Thu, 25 Jan 2007, Jeremy Drake wrote:
> On Thu, 25 Jan 2007, Jeremy Drake wrote:
>
> > I think that an ALTER LANGUAGE OWNER TO is the proper response to these
> > things, and unless I hear otherwise I will attempt to add this to my
> > patch.
>
> Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME
> TO for the owner, which I missed before. I would appreciate someone with
> more knowledge of the permissions infrastructure to take a look at it
> since I am fairly new to it and may not fully understand its intricacies.
>
I have refactored the owner checking of languages in the same manner as it
is for other owned objects. I have changed to using standard permissions
error messages (aclcheck_error) for the language permissions errors.
I consider this patch ready for review, assuming the permissions rules
outlined by Tom Lane on -hackers are valid. For reference, here are the
rules that this patch is intended to implement:
On Wed, 24 Jan 2007, Tom Lane wrote:
> In detail, it'd look something like:
>
> * For an untrusted language: must be superuser to either create or use
> the language (no change from current rules). Ownership of the
> pg_language entry is really irrelevant, as is its ACL.
>
> * For a trusted language:
>
> * if pg_pltemplate.something is ON: either a superuser or the current
> DB's owner can CREATE the language. In either case the pg_language
> entry will be marked as owned by the DB owner (pg_database.datdba),
> which means that subsequently he (or a superuser) can grant or deny
> USAGE within his DB.
>
> * if pg_pltemplate.something is OFF: must be superuser to CREATE the
> language; subsequently it will be owned by you, so only you or another
> superuser can grant or deny USAGE (same behavior as currently).
The only difference from this is, that when superuser is required, the
owner of the language is not the superuser who created it, but
BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the
"same behavior as currently" took precedence. The current behavior in cvs
is that languages have no owner, and for purposes where one would be
needed it is assumed to be BOOTSTRAP_SUPERUSERID.
Is this valid, or should I instead set the owner to GetUserId() in those
cases?
--
Academic politics is the most vicious and bitter form of politics,
because the stakes are so low.
-- Wallace Sayre
Attachment
Jeremy Drake <pgsql@jdrake.com> writes:
> The only difference from this is, that when superuser is required, the
> owner of the language is not the superuser who created it, but
> BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the
> "same behavior as currently" took precedence. The current behavior in cvs
> is that languages have no owner, and for purposes where one would be
> needed it is assumed to be BOOTSTRAP_SUPERUSERID.
> Is this valid, or should I instead set the owner to GetUserId() in those
> cases?
I'd go with GetUserId() in the cases where you're not explicitly
assigning ownership to the datdba role. AFAIR the assumption that
languages are owned by BOOTSTRAP_SUPERUSERID was just a kluge to use in
some bits of code that had to have a notion of a specific owner. Now
in reality every superuser has the same privileges as every other one,
and so it doesn't matter much which one you use, but we might as well
record who actually did the deed.
regards, tom lane
On Sat, 27 Jan 2007, Tom Lane wrote:
> I'd go with GetUserId() in the cases where you're not explicitly
> assigning ownership to the datdba role. AFAIR the assumption that
> languages are owned by BOOTSTRAP_SUPERUSERID was just a kluge to use in
> some bits of code that had to have a notion of a specific owner. Now
> in reality every superuser has the same privileges as every other one,
> and so it doesn't matter much which one you use, but we might as well
> record who actually did the deed.
Here is a version of the patch which does this. Very minor change from
the last version...
--
Begathon, n.:
A multi-day event on public television, used to raise money so
you won't have to watch commercials.
Attachment
On Fri, 26 Jan 2007, Jeremy Drake wrote: > On Sat, 27 Jan 2007, Tom Lane wrote: > > > I'd go with GetUserId() in the cases where you're not explicitly > > assigning ownership to the datdba role. AFAIR the assumption that > > languages are owned by BOOTSTRAP_SUPERUSERID was just a kluge to use in > > some bits of code that had to have a notion of a specific owner. Now > > in reality every superuser has the same privileges as every other one, > > and so it doesn't matter much which one you use, but we might as well > > record who actually did the deed. > > Here is a version of the patch which does this. Very minor change from > the last version... Oops, was missing an include which resulted in an implicitly defined function warning. This version fixes it... -- Anybody who doesn't cut his speed at the sight of a police car is probably parked.
Attachment
This version of the patch includes documentation changes. Please review... -- The more things change, the more they stay insane.
Attachment
The most recent version of this patch has been added.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Jeremy Drake wrote:
> On Thu, 25 Jan 2007, Jeremy Drake wrote:
>
> > On Thu, 25 Jan 2007, Jeremy Drake wrote:
> >
> > > I think that an ALTER LANGUAGE OWNER TO is the proper response to these
> > > things, and unless I hear otherwise I will attempt to add this to my
> > > patch.
> >
> > Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME
> > TO for the owner, which I missed before. I would appreciate someone with
> > more knowledge of the permissions infrastructure to take a look at it
> > since I am fairly new to it and may not fully understand its intricacies.
> >
>
> I have refactored the owner checking of languages in the same manner as it
> is for other owned objects. I have changed to using standard permissions
> error messages (aclcheck_error) for the language permissions errors.
>
> I consider this patch ready for review, assuming the permissions rules
> outlined by Tom Lane on -hackers are valid. For reference, here are the
> rules that this patch is intended to implement:
>
> On Wed, 24 Jan 2007, Tom Lane wrote:
>
> > In detail, it'd look something like:
> >
> > * For an untrusted language: must be superuser to either create or use
> > the language (no change from current rules). Ownership of the
> > pg_language entry is really irrelevant, as is its ACL.
> >
> > * For a trusted language:
> >
> > * if pg_pltemplate.something is ON: either a superuser or the current
> > DB's owner can CREATE the language. In either case the pg_language
> > entry will be marked as owned by the DB owner (pg_database.datdba),
> > which means that subsequently he (or a superuser) can grant or deny
> > USAGE within his DB.
> >
> > * if pg_pltemplate.something is OFF: must be superuser to CREATE the
> > language; subsequently it will be owned by you, so only you or another
> > superuser can grant or deny USAGE (same behavior as currently).
>
> The only difference from this is, that when superuser is required, the
> owner of the language is not the superuser who created it, but
> BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the
> "same behavior as currently" took precedence. The current behavior in cvs
> is that languages have no owner, and for purposes where one would be
> needed it is assumed to be BOOTSTRAP_SUPERUSERID.
>
> Is this valid, or should I instead set the owner to GetUserId() in those
> cases?
>
>
> --
> Academic politics is the most vicious and bitter form of politics,
> because the stakes are so low.
> -- Wallace Sayre
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On Tue, 20 Feb 2007, Bruce Momjian wrote: > > The most recent version of this patch has been added. > > Your patch has been added to the PostgreSQL unapplied patches list at: > > http://momjian.postgresql.org/cgi-bin/pgpatches > > It will be applied as soon as one of the PostgreSQL committers reviews > and approves it. Cool, I was going to bring this up again once the regexp patch got in. There is one thing in this patch I was not sure on, and that is in AlterLanguageOwner what should the second parameter of heap_close be? I have RowExclusiveLock in the patch, but I am not sure that is correct. It would be good if someone more knowledgeable about such things checked on this when applying it... The latest version of the patch is currently at http://momjian.us/mhonarc/patches/msg00014.html > --------------------------------------------------------------------------- > > > Jeremy Drake wrote: > > On Thu, 25 Jan 2007, Jeremy Drake wrote: > > > > > On Thu, 25 Jan 2007, Jeremy Drake wrote: > > > > > > > I think that an ALTER LANGUAGE OWNER TO is the proper response to these > > > > things, and unless I hear otherwise I will attempt to add this to my > > > > patch. > > > > > > Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME > > > TO for the owner, which I missed before. I would appreciate someone with > > > more knowledge of the permissions infrastructure to take a look at it > > > since I am fairly new to it and may not fully understand its intricacies. > > > > > > > I have refactored the owner checking of languages in the same manner as it > > is for other owned objects. I have changed to using standard permissions > > error messages (aclcheck_error) for the language permissions errors. > > > > I consider this patch ready for review, assuming the permissions rules > > outlined by Tom Lane on -hackers are valid. For reference, here are the > > rules that this patch is intended to implement: > > > > On Wed, 24 Jan 2007, Tom Lane wrote: > > > > > In detail, it'd look something like: > > > > > > * For an untrusted language: must be superuser to either create or use > > > the language (no change from current rules). Ownership of the > > > pg_language entry is really irrelevant, as is its ACL. > > > > > > * For a trusted language: > > > > > > * if pg_pltemplate.something is ON: either a superuser or the current > > > DB's owner can CREATE the language. In either case the pg_language > > > entry will be marked as owned by the DB owner (pg_database.datdba), > > > which means that subsequently he (or a superuser) can grant or deny > > > USAGE within his DB. > > > > > > * if pg_pltemplate.something is OFF: must be superuser to CREATE the > > > language; subsequently it will be owned by you, so only you or another > > > superuser can grant or deny USAGE (same behavior as currently). > > > > The only difference from this is, that when superuser is required, the > > owner of the language is not the superuser who created it, but > > BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the > > "same behavior as currently" took precedence. The current behavior in cvs > > is that languages have no owner, and for purposes where one would be > > needed it is assumed to be BOOTSTRAP_SUPERUSERID. > > > > Is this valid, or should I instead set the owner to GetUserId() in those > > cases? > > > > > > -- > > Academic politics is the most vicious and bitter form of politics, > > because the stakes are so low. > > -- Wallace Sayre > Content-Description: > > [ Attachment, skipping... ] > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 6: explain analyze is your friend > > -- A UNIX saleslady, Lenore, Enjoys work, but she likes the beach more. She found a good way To combine work and play: She sells C shells by the seashore.
Jeremy Drake <pgsql@jdrake.com> writes:
> This version of the patch includes documentation changes. Please
> review...
Applied with minor revisions --- in particular, I thought the initial
owner of a language should be its creator, full stop, rather than the
rather strange (and undocumented) behavior you had. Also you missed
updating pg_dump.
regards, tom lane
On Mon, 26 Mar 2007, Tom Lane wrote: > Jeremy Drake <pgsql@jdrake.com> writes: > > This version of the patch includes documentation changes. Please > > review... > > Applied with minor revisions --- in particular, I thought the initial > owner of a language should be its creator, full stop, rather than the > rather strange (and undocumented) behavior you had. The reason I did it like that was this email from you: http://archives.postgresql.org/pgsql-hackers/2007-01/msg01186.php > Also you missed updating pg_dump. Indeed. Now, all I need is the 'tsearch2 in core' patch to go in, and 8.3 will solve almost all of my problems :) Then I just need to nag my hosting provider to upgrade once it is released. I have been refraining from nagging about them still running 8.1.3 in anticipation of this ;) -- Five is a sufficiently close approximation to infinity. -- Robert Firth
Jeremy Drake <pgsql@jdrake.com> writes: > On Mon, 26 Mar 2007, Tom Lane wrote: >> Applied with minor revisions --- in particular, I thought the initial >> owner of a language should be its creator, full stop, rather than the >> rather strange (and undocumented) behavior you had. > The reason I did it like that was this email from you: > http://archives.postgresql.org/pgsql-hackers/2007-01/msg01186.php Yeah, but that idea predated the addition of ALTER LANGUAGE OWNER to the patch. Given that, a superuser can give away the language to someone else if he wants, and so there's no need for us to try to be fancy about guessing what he wants (which was more or less what that rule was). regards, tom lane