Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Now the dust is settling on the on_perl_init patch I'd like to ask for clarification on this next patch. On Fri, Jan 15, 2010 at 12:35:06AM +, Tim Bunce wrote: This is the fourth of the patches to be split out from the former 'plperl feature patch 1'. Changes in this patch: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. Would changing plperl.on_trusted_init and plperl.on_untrusted_init to PGC_BACKEND, so the user can't change the value after the session has started, resolve those concerns? Any other concerns with this patch? Tim. - select_perl_context() state management improved An error during interpreter initialization will leave the state (interp_state etc) unchanged. - The utf8fix code has been greatly simplified. Tim. diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index 0054f5a..f2c91a9 100644 *** a/doc/src/sgml/plperl.sgml --- b/doc/src/sgml/plperl.sgml *** plplerl.on_perl_init = 'use lib /my/app *** 1079,1084 --- 1079,1120 /listitem /varlistentry + varlistentry id=guc-plperl-on-trusted-init xreflabel=plperl.on_trusted_init + termvarnameplperl.on_trusted_init/varname (typestring/type)/term + indexterm +primaryvarnameplperl.on_trusted_init/ configuration parameter/primary + /indexterm + listitem +para +Specifies perl code to be executed when the literalplperl/ perl interpreter +is first initialized in a session. The perl code can only perform trusted operations. +The SPI functions are not available when this code is executed. +Changes made after a literalplperl/ perl interpreter has been initialized will have no effect. +If the code fails with an error it will abort the initialization of the interpreter +and propagate out to the calling query, causing the current transaction +or subtransaction to be aborted. +/para + /listitem + /varlistentry + + varlistentry id=guc-plperl-on-untrusted-init xreflabel=plperl.on_untrusted_init + termvarnameplperl.on_untrusted_init/varname (typestring/type)/term + indexterm +primaryvarnameplperl.on_untrusted_init/ configuration parameter/primary + /indexterm + listitem +para +Specifies perl code to be executed when the literalplperlu/ perl interpreter +is first initialized in a session. +The SPI functions are not available when this code is executed. +Changes made after a literalplperlu/ perl interpreter has been initialized will have no effect. +If the code fails with an error it will abort the initialization of the interpreter +and propagate out to the calling query, causing the current transaction +or subtransaction to be aborted. +/para + /listitem + /varlistentry + varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict termvarnameplperl.use_strict/varname (typeboolean/type)/term indexterm diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 7cd5721..f3cabad 100644 *** a/src/pl/plperl/GNUmakefile --- b/src/pl/plperl/GNUmakefile *** PERLCHUNKS = plc_perlboot.pl plc_safe_ba *** 41,47 SHLIB_LINK = $(perl_embed_ldflags) REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu ! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperlu # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) --- 41,47 SHLIB_LINK = $(perl_embed_ldflags) REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu ! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperl_init plperlu # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out index ...e69de29 . diff --git a/src/pl/plperl/expected/plperl_shared.out b/src/pl/plperl/expected/plperl_shared.out index 72ae1ba..c1c12c1 100644 *** a/src/pl/plperl/expected/plperl_shared.out --- b/src/pl/plperl/expected/plperl_shared.out *** *** 1,3 --- 1,7 + -- test plperl.on_plperl_init via the shared hash + -- (must be done before plperl is initialized) + -- testing on_trusted_init gets run, and that it can alter %_SHARED + SET
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Tim Bunce tim.bu...@pobox.com writes: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. IIRC, what I was unhappy about was exposing shared library load as a time when interesting-to-the-user things would happen. It looks like you have got rid of that and instead made it happen at the first call of a plperl or plperlu function (respectively). That seems like a fairly reasonable way to work, and if it's defined that way, there doesn't seem any reason not to allow them to be USERSET/SUSET. But it ought to be documented like that, not with vague phrases like when the interpreter is initialized --- that means nothing to users. One issue that ought to be mentioned is what about transaction rollback. One could argue that if the first plperl call is inside a transaction that later rolls back, then all the side effects of that should be undone. But we have no way to undo whatever happened inside perl, and at least in most likely uses of on_init there wouldn't be any advantage in trying to force a redo anyway. I think it's okay to just define it as happening once regardless of any transaction failure (but of course this had better be documented). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. Right, sorry, got 'em backwards. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
On Thu, Jan 28, 2010 at 12:12:58PM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. Right, sorry, got 'em backwards. I've done that several times. The naming is tricky because it's very dependent on your point of view. The 'trusted' language is for running 'untrusted' code and the 'untrusted' language is for running 'trusted' code. The naming convention is unfortunate. Just an observation from a newbie. I imagine it's been pointed out before. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
On Thu, Jan 28, 2010 at 11:54:08AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. Yes. Good point (though ITYM on_UNtrusted_init). I'll change that. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. IIRC, what I was unhappy about was exposing shared library load as a time when interesting-to-the-user things would happen. It looks like you have got rid of that and instead made it happen at the first call of a plperl or plperlu function (respectively). That seems like a fairly reasonable way to work, and if it's defined that way, there doesn't seem any reason not to allow them to be USERSET/SUSET. Great. But it ought to be documented like that, not with vague phrases like when the interpreter is initialized --- that means nothing to users. I'll change that. One issue that ought to be mentioned is what about transaction rollback. One could argue that if the first plperl call is inside a transaction that later rolls back, then all the side effects of that should be undone. But we have no way to undo whatever happened inside perl, and at least in most likely uses of on_init there wouldn't be any advantage in trying to force a redo anyway. I think it's okay to just define it as happening once regardless of any transaction failure (but of course this had better be documented). Will do. Once the previous patch lands I'll post an update to this patch with those changes applied. Thanks. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
On Jan 28, 2010, at 12:01 PM, Tim Bunce wrote: Once the previous patch lands I'll post an update to this patch with those changes applied. Ds the Add on_perl_init and proper destruction to plperl patch the one you're waiting on, then? Best, David, who loses track of these things -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
