Package namespace and Safe init cleanup for plperl[PATCH]
in [PgSql]
|
Prev: [HACKERS] C function accepting/returning cstring vs. text
Next: Dividing progress/debug information in pg_standby, and stat before copy
From: Tim Bunce on 25 Jan 2010 14:53 On Fri, Jan 15, 2010 at 04:02:02AM +0000, Tim Bunce wrote: > This is the final plperl patch in the series from me. > > Changes in this patch: > > - Moved internal functions out of main:: namespace > into PostgreSQL::InServer and PostgreSQL::InServer::safe > > - Restructured Safe compartment setup code > to generalize and separate the data from the logic. > > Neither change has any user visible effects. This is a revised version of the patch with the following additional changes: - Further generalized the 'what to load into Safe compartment' logic. - Added the 'warnings' pragma to the list of modules to load into Safe. So plperl functions can now "use warnings;" - added test for that. - Added 'use 5.008001;' to plc_perlboot.pl as a run-time check to complement the configure-time check added by Tom Lane recently. Tim.
From: Tim Bunce on 30 Jan 2010 09:51 On Fri, Jan 29, 2010 at 08:07:30PM -0700, Alex Hunsaker wrote: > A couple of comments. *note* I have not tested this as a whole yet > (due to rejects). > > in plc_perboot.pl > +$funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub { > $BEGIN $prolog $src } ]; > > Any thoughts on using a package other than main? Maybe something like > package PlPerl or what not? Not that I think it really matters... > But it might be nicer to see in say NYTprof ? The only interface Safe provides for sharing things with the compartment shares them with the root package of the compartment which, from within the compartment, is 'main::'. It's an unfortunate limitation of Safe. I could have added more code to wordaround that but opted to keep it simple for this commitfest. > in plc_safe_ok.pl > +use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe); > > Is there some reason not to use my here? The only reason I can think > of is you want the *_init gucs to be able to stick things into > @ShareIntoSafe and friends? And if so should we document that > somewhere (or was that in an earlier patch that i missed?)? It's a step towards providing an interface to give the DBA control over what gets loaded into the Safe compartment and what gets shared with it. I opted to not try to define a formal documented interface because I felt it was likely to be a source of controversy and/or bikeshedding. This late in the game I didn't want to take that chance. I'd rather a few brave souls get some experience with it as-as, and collect some good use-cases, before proposing a formal documented API. > Also whats the use case for $SafeClass? Multiple versions of Safe.pm? > Other modules that are like Safe.pm? Im just curious... It's possible someone might want to use an alternative module. Most likely their own local subclass of Safe that adds extra features. I'd explored that when trying to integrate NYTProf. It seemed worth at least making possible. > Hrm I also dont see the point of the new "use Safe;" as we still eval > it in in plperl_safe_init() care to enlighten a lost soul? It just makes the file more self-contained for syntax checking: perl -cw plc_safeboot.pl > Other than those really quite minor questions that are arguably me > nitpicking... It looks great to me. Great, thanks Alex! Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tim Bunce on 30 Jan 2010 13:37 On Sat, Jan 30, 2010 at 11:08:26AM -0700, Alex Hunsaker wrote: > On Sat, Jan 30, 2010 at 07:51, Tim Bunce <Tim.Bunce(a)pobox.com> wrote: > > On Fri, Jan 29, 2010 at 08:07:30PM -0700, Alex Hunsaker wrote: > > > >> Other than those really quite minor questions that are arguably me > >> nitpicking... �It looks great to me. > > > > Great, thanks Alex! > > I marked it as ready, though ill add a comment that it depends on the > other patch to apply cleanly (and if that one gets rebased... > obviously this one probably will need to as well). I've rebased and reposted the other patch ("Add on_trusted_init and on_untrusted_init to plperl") and added it to the commitfest. I've rebasing this one now and making one minor change: I'm adding an if (not our $_init++) { ... } around the code that adds items to @EvalInSafe and @ShareIntoSafe to avoid any problems with on_trusted_init code that causes exceptions. Currently an exception from on_trusted_init will leave the plperl interpreter in the 'not yet setup' state. So the next use runs the plc_safe_ok code and the on_trusted_init code again. This change makes the plc_safe_ok code idempotent so the re-execution won't cause any problems (except for harmless warnings about the safe_eval and mksafefunc subs being redefined). Patch to follow (after getting the kids to bed). Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tim Bunce on 12 Feb 2010 18:45 On Fri, Feb 12, 2010 at 02:30:37PM -0700, Alex Hunsaker wrote: > On Fri, Feb 12, 2010 at 13:42, Andrew Dunstan <andrew(a)dunslane.net> wrote: > > Alex Hunsaker wrote: > >>>> > > and leave the rest for the next release, unless you can > > convince me real fast that we're not opening a back door here. If we're > > going to allow DBAs to add things to the Safe container, it's going to be up > > front or not at all, as far as I'm concerned. > > I think backdoor is a bit extreme. Yes it could allow people who > can set the plperl.*_init functions to muck with the safe. As an > admin I could also do that by setting plperl.on_init and overloading > subs in the Safe namespace or switching out Safe.pm. Exactly. [As I mentioned before, the docs for Devel::NYTProf::PgPLPerl http://code.google.com/p/perl-devel-nytprof/source/browse/trunk/lib/Devel/NYTProf/PgPLPerl.pm talk about the need to _hack_ perl standard library modules in order to be able to run NYTProf on plperl code. The PERL5OPT environment variable could be used as an alternative vector.] Is there any kind of threat model (http://en.wikipedia.org/wiki/Threat_model) that PostgreSQL developers use when making design decisions? Clearly the PostgreSQL developers take security very seriously, and rightly so. An explicit threat model document would provide a solid framework to assess potential security issues when their raised. The key issue here seems to be the trust, or lack of it, placed in DBAs. > Anyway reasons I can come up for it are: > -its general so we can add other modules/pragmas easily in the future > -helps with the plperl/plperlu all or nothing situation > -starts to flesh out how an actual exposed (read documented) interface > should look for 9.1 > > ... I know Tim mentioned David as having some use cases (cc'ed) I've just posted the docs for a module that's a good example of the kind of module a DBA might want to allow plperl code to use http://archives.postgresql.org/pgsql-hackers/2010-02/msg01059.php I'd be happy to add a large scary warning in the plc_safe_ok.pl file saying that any manipulation of the (undocumented) variables could cause a security risk and is not supported in any way. Tim. > So my $0.2 is I don't have any strong feelings for/against it. I > think if we could expose *something* (even if you can only get to it > in a C contrib module) that would be great. But I realize it might be > impractical at this stage in the game. > > *shrug* > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tim Bunce on 13 Feb 2010 05:17
On Fri, Feb 12, 2010 at 07:57:15PM -0500, Andrew Dunstan wrote: > Alex Hunsaker wrote: > > Yes it could allow people who > >can set the plperl.*_init functions to muck with the safe. As an > >admin I could also do that by setting plperl.on_init and overloading > >subs in the Safe namespace or switching out Safe.pm. > > It's quite easy to subvert Safe.pm today, sadly. You don't need > on*init, nor do you need to replace the System's Safe.pm. I'm not > going to tell people how here, but it took me all of a few minutes > to discover and verify when I went looking a few weeks ago, once I > thought about it. > > But that's quite different from us providing an undocumented way to > expose arbitrary objects to the Safe container. In that case *we* > become responsible for any insecure uses, and we don't even have the > luxury of having put large warnings in the docs, because there > aren't any docs. I wish I understood better how PostgreSQL developers would be responsible for a DBA using an undocumented hack. In my view the DBA is clearly responsible in that case. If it's not documented it's not supported. > Another objection is that discussion on this facility has been > pretty scant. I think that's putting it mildly. Maybe it's been > apparent to a few people what the implications are, but I doubt it's > been apparent to many. That makes me quite nervous, which is why I'm > raising it now. > > Tim mentioned in his reply that he'd be happy to put warnings in > plc_safe_ok.pl. But that file only exists in the source - it's > turned into header file data as part of the build process, so > warnings there will be seen by roughly nobody. > > I still think if we do this at all it needs to be documented and > surrounded with appropriate warnings. I'm away for a day or so (for my wedding anniversary) but I'll have an updated patch, with a clean well-documented API, for consideration by Monday morning. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |