tada / pljava

PL/Java is a free add-on module that brings Java™ Stored Procedures, Triggers, Functions, Aggregates, Operators, Types, etc., to the PostgreSQL™ backend.
http://tada.github.io/pljava/
Other
242 stars 77 forks source link

pljava 1.6.1 permissions #331

Closed fmbiete closed 3 years ago

fmbiete commented 3 years ago

Hi,

After upgrading to pljava 1.6.1 I'm seeing this error:

[local] user@database=> CREATE FUNCTION pljava_test(VARCHAR)
[more] - > RETURNS VARCHAR
[more] - > AS 'java.lang.System.getProperty'
[more] - > LANGUAGE java;
ERROR:  must be superuser or a member of pg_read_all_settings to examine "pljava.policy_urls"

Is there any workaround to avoid granting pg_read_all_settings?

And after granting that role I can create the function but the policy kicks in. Is there any way of going back to the previous behaviour?, this change will break our applications until the third party providers change their side

[local] user@database=> select pljava_test('user.home');
ERROR:  java.sql.SQLSyntaxErrorException: access denied ("java.util.PropertyPermission" "user.home" "read")
[local] user@database=> select name, setting from pg_settings where name like 'pljava%';
                name                 |                                                                          setting
-------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------
 pljava.debug                        | off
 pljava.enable                       | on
 pljava.implementors                 | postgresql
 pljava.java_thread_pg_entry         | error
 pljava.libjvm_location              | /home/pgsqladm/openjdk/lib/server/libjvm.so
 pljava.module_path                  | /pgsqladm/postgresql-server-11.10_el7_x86_64/share/pljava/pljava-1.6.1.jar:/pgsqladm/postgresql-server-11.10_el7_x86_64/share/pljava/pljava-api-1.6.1.jar
 pljava.policy_urls                  | "file:${org.postgresql.sysconfdir}/pljava.policy","="
 pljava.release_lingering_savepoints | off
 pljava.statement_cache_size         | 100
 pljava.vmoptions                    | -Xms32M -Xmx128M -XX:ParallelGCThreads=2 -Xss2m
(10 rows)

pljava.policy

//
// Security policy for PL/Java. These grants are intended to add to those
// contained in the java.policy file of the standard Java installation.
//

//
// This grant is unconditional. It adds these properties to the standard Java
// list of system properties that any code may read.
//
grant {
        // "standard" properties that can be read by anyone, by analogy to the
        // ones so treated in Java itself.
        //
        permission java.util.PropertyPermission
                "org.postgresql.version", "read";
        permission java.util.PropertyPermission
                "org.postgresql.pljava.version", "read";
        permission java.util.PropertyPermission
                "org.postgresql.pljava.native.version", "read";

        permission java.util.PropertyPermission
                "org.postgresql.pljava.udt.byteorder.*", "read";

        permission java.util.PropertyPermission
                "org.postgresql.server.encoding", "read";

        // PostgreSQL allows SELECT current_database() or SHOW cluster_name anyway.
        //
        permission java.util.PropertyPermission
                "org.postgresql.database", "read";
        permission java.util.PropertyPermission
                "org.postgresql.cluster", "read";

        // SQL/JRT specifies this property.
        //
        permission java.util.PropertyPermission
                "sqlj.defaultconnection", "read";

        // This property is read in the innards of Java 9 and 10, but they forgot
        // to add a permission for it. Not needed for Java 11 and later.
        //
        permission java.util.PropertyPermission
                "jdk.lang.ref.disableClearBeforeEnqueue", "read";
};

//
// This grant is specific to the internal implementation of PL/Java itself,
// which needs these permissions for its own operations.
//
// Historically, PL/Java has been able to read any file on the server filesystem
// when a file: URL is passed to sqlj.install_jar or sqlj.replace_jar. Such a
// broad grant is not necessary, and can be narrowed below if desired.
//
grant codebase "${org.postgresql.pljava.codesource}" {
        permission java.lang.RuntimePermission
                "charsetProvider";
        permission java.lang.RuntimePermission
                "createClassLoader";
        permission java.net.NetPermission
                "specifyStreamHandler";
        permission java.util.logging.LoggingPermission
                "control";
        permission java.security.SecurityPermission
                "createAccessControlContext";

        // This gives the PL/Java implementation code permission to read
        // any file, which it only exercises on behalf of sqlj.install_jar()
        // or sqlj.replace_jar() when called with a file: URL.
        //
        // There would be nothing wrong with restricting this permission to
        // a specific directory, if all jar files to be loaded will be found there,
        // or replacing it with a URLPermission if they will be hosted on a remote
        // server, etc.
        //
        permission java.io.FilePermission
                "<<ALL FILES>>", "read";
};

//
// This grant defines the mapping onto Java of PostgreSQL's "trusted language"
// category. When PL/Java executes a function whose SQL declaration names
// a language that was declared WITH the TRUSTED keyword, it will have these
// permissions, if any (in addition to whatever others might be granted to all
// code, or to its specific jar, etc.).
//
grant principal org.postgresql.pljava.PLPrincipal$Sandboxed * {
};

//
// This grant defines the mapping onto Java of PostgreSQL's "untrusted language"
// category. When PL/Java executes a function whose SQL declaration names
// a language that was declared WITHOUT the TRUSTED keyword, it will have these
// permissions (in addition to whatever others might be granted to all code, or
// to its specific jar, etc.).
//
grant principal org.postgresql.pljava.PLPrincipal$Unsandboxed * {

        // Java does not circumvent operating system access controls; this grant
        // will still be limited to what the OS allows a PostgreSQL backend process
        // to do.
        permission java.io.FilePermission
                "<<ALL FILES>>", "read,readlink,write,delete";
};

//
// This grant applies to a specific PL/Java sandboxed language named java_tzset
// (if such a language exists) and grants functions created in that language
// permission to adjust the time zone. There is an example method in the
// org.postgresql.pljava.example.annotation.PreJSR310 class, which needs to
// temporarily adjust the time zone for a test. That example also uses
// sqlj.alias_java_language to create the java_tzset "language" when deployed,
// and DROP LANGUAGE to remove it when undeployed.
//
grant principal org.postgresql.pljava.PLPrincipal$Sandboxed "java_tzset" {
        permission java.util.PropertyPermission "user.timezone", "write";
};
jcflack commented 3 years ago

Is there any workaround to avoid granting pg_read_all_settings?

Hmm. That appears to be a bug. Thanks for the report.

this change will break our applications until the third party providers change their side

[local] user@database=> select pljava_test('user.home'); ERROR: java.sql.SQLSyntaxErrorException: access denied ("java.util.PropertyPermission" "user.home" "read")

Is this example (reading the user.home property) something your applications actually do, or more of a manual check you do by habit? I've noticed over the years that lots of PL/Java-related blog posts suggest reading a property as a quick check that it's installed, and often the property is something like user.home that Java protects by default.

If you are able to change your manual check to reading some other property that is readable by default, select pljava_test('java.version'); or the like, that solves that problem. On the other hand, if your applications really depend on reading that, you should edit the policy to allow it.

Are you saying that your PostgreSQL cluster is running on a third-party provider, and they don't have a way for you to edit pljava.policy? I hope they didn't just automatically migrate you from 1.5 to 1.6 ... the release announcement suggested not to do that, to allow planned migrations and a way of editing policy.

It appears that you were able to read the pljava.policy file successfully. Does the provider have the permissions set so you are unable to write it?

Are you able to copy it to some other location, edit it there, and point pljava.policy_urls to that copy?

I can look into adding a 'transitional' policy implementation that you could select in pljava.vmoptions, that would only log denials, for use while determining what grants you should add to the policy.

fmbiete commented 3 years ago

What I meant in the second part is that we won't be able to modify the code to fix the application behaviour. Is there any defined policy equivalent to the same level of permissions that 1.5 had?

For now I have enabled the access logging to capture any denied rule, but to catch everything we'll need to hit every functionality.

jcflack commented 3 years ago

Are you in a position to build from the bug/REL1_6_STABLE/issue331 branch and see what it does for you, in advance of another release?

It should eliminate the pg_read_all_settings requirement, and it adds a new Policy implementation intended for temporary use while checking pre-existing code.

It works like this: point pljava.policy_urls to your normal policy file(s), as usual. That's where you will develop your ultimate policy for production.

Write another policy file somewhere, and point to it with -Dorg.postgresql.pljava.policy.trial=url added to pljava.vmoptions. Anything this policy allows will be allowed, but will be logged if the regular policy would have denied it. So you can make this one more generous than the regular policy, and use the log entries to identify grants that might belong in the regular policy. As you add the missing ones to the real policy, they stop getting logged by this one, and the log gets quieter. You can make this one as generous as you are comfortable making it during the period of testing and tuning.

At one extreme it could be this:

grant {
  permission java.security.AllPermission;
};

and it would happily allow your code under test to do anything at all, while logging whatever permissions aren't in the regular policy. (A side effect of this would be to erase any distinction between java and javaU.)

If you are not completely comfortable with that, there is the difficulty that Java's permission model does not have a subtractive mode; you can't say "grant AllPermission except for this list of the ones I'd really rather not." So I added a new permission:

grant {
  permission org.postgresql.pljava.policy.TrialPolicy$Permission;
};

which is pretty much exactly a hard-coded version of "AllPermission except not any FilePermission (so the java/javaU distinction stays meaningful) or a couple dozen other various SecurityPermission/ReflectPermission/RuntimePermission instances that you'd probably rather not". If its hard-coded exclusion list excludes any permissions you think you might need, you can just explicitly add them to the trial policy too.

So setting up the trial policy can be a bit of a balancing act: if you make it very generous, you minimize the chance of the app failing because of a denied permission, but increase your exposure. By making it more limited, you decrease exposure but increase the risk of the app falling over because it really did need some permission you weren't comfortable putting in the trial policy. I tried to set the TrialPolicy$Permission somewhere in the sweet spot.

You can also use all of the normal policy features in the trial policy. If your apps are installed in several different jars, you can use grant codebase separately to put different outer limits around different jars, and completely remove the grants for one jar after another as you are satisfied you have added the right things for each one in the regular policy. You could also set different limits for java and javaU by granting to the PLPrincipal, just as you can in the regular policy.

One thing to be aware of is that the trial policy can give you false alarms. It's not that uncommon for software to include configuration-dependent bits that tentatively try certain actions, catch exceptions, and proceed normally, having discovered what it should and shouldn't do. The trial policy can log permission denials that happen in the course of that kind of activity, but where if you weren't using the trial policy you would never notice a problem from the app not being granted those permissions.

I don't think there's any perfect way to tell which denials being logged by the trial policy are false alarms. One approach would be to collect a bunch of the log entries, figure out what functions of the app they were coming from, and then start a dedicated session without the -Dorg.postgresql.pljava.policy.trial setting (or with it pointing to a different, more restrictive version of the policy, not granting the permissions you're curious about), then exercise those functions of the app and see if anything breaks. Other users could still have the more generous trial setting in their sessions, so your experiments aren't affecting them.

To avoid bloating logs too much, I implemented an abbreviated form of stack trace for each entry. The approach is to keep one stack frame above and one below each crossing of a module or protection-domain boundary, with ... replacing intermediate frames within the same module/domain, and the code source/principals of the denied domain shown at the appropriate position in the trace. I think that should be more useful for the purpose than the miles-long full traces you get from the java.security.debug settings.

The messages are sent through the PostgreSQL log if the thread making the permission check knows it can do so without blocking; otherwise they just go to stderr (which should wind up in the PostgreSQL log anyway, if logging_collector is on, otherwise who knows where). There isn't really a reliable "can I do so without blocking?" check for every flavor of pljava.java_thread_pg_entry. I noticed in your report that you have it set for error. If you set it for throw, the logging behavior will be more predictable; entries from the main thread will go through PostgreSQL's log facility always, and those from any other thread will go to stderr.

fmbiete commented 3 years ago

Yes, I can replace the extensions files with a custom build.

I confirm branch code fixes the issue for pg_read_all_settings.

Thank you! That trial option is perfect to identify what changes we need to request to our software providers.

jcflack commented 3 years ago

I'm glad it's helpful.

Now, it's quite possible that any permissions being requested by the third-party software could be perfectly legitimate and inherent to what you want the software to do, so it would be reasonable to just add grants in pljava.policy to allow them. The shipped pljava.policy is deliberately minimal, so it would not be unusual to add grants to it. (Partly that's because, again, Java doesn't offer a subtractive mode, so it would be more difficult to start with a more generous policy and remove permissions from it. And an incidental benefit is after you are done, just looking at pljava.policy tells you what permissions you have granted.)

Changes from the software providers might not be needed unless you notice that the software is requesting permissions it shouldn't need to, that you are not willing to grant.

It might also be useful here (if you can without disclosing anything proprietary) to identify third-party software you are using and what permissions you find necessary for it; others using the same software might benefit from that. Perhaps there should be a wiki page here for such information.

df7cb commented 3 years ago

I ran into an issue while updating the Debian package that looks related:

### Installing for PostgreSQL 13
+ java -Dpgsql.pgconfig=/usr/lib/postgresql/13/bin/pg_config -Dpgconfig.pkglibdir=debian/postgresql-13-pljava/usr/lib/postgresql/13/lib -Dpgconfig.sharedir=debian/postgresql-13-pljava/usr/share/postgresql/13 -jar build-13/pljava-pg13.jar
debian/postgresql-13-pljava/usr/lib/postgresql/13/lib/libpljava-so-1.6.1.so as bytes
...
debian/postgresql-13-pljava/usr/share/postgresql/13/pljava/pljava--unpackaged.sql as lines (UTF8)
/etc/postgresql-common/pljava.policy Exception in thread "main" java.io.IOException: Permission denied
    at java.base/java.io.UnixFileSystem.createFileExclusively(Native Method)
    at java.base/java.io.File.createTempFile(File.java:2092)
    at org.gjt.cuspy.JarX.extract(JarX.java:749)
    at org.gjt.cuspy.JarX.extract(JarX.java:462)
    at org.postgresql.pljava.packaging.Node.extract(Node.java:181)
    at org.postgresql.pljava.packaging.Node.main(Node.java:165)
make[1]: *** [debian/rules:43: override_dh_auto_install] Error 1

At build time, we can't write to /etc/* and the file should go to DESTDIR instead which is $PWD/debian/postgresql-13-pljava/.

Is the presence of that file at runtime required? That will need some extra package juggling because I cannot install /etc/postgresql-common/pljava.policy as part of multiple packages (postgresql-{10,11,12,13}-pljava). Could I omit that file instead and still have usable, secure packages?

You mention that users should read the release announcement when upgrading, but I had problems finding that on https://github.com/tada/pljava/wiki. Perhaps that should be linked more prominently. (Generally, the duplication between the wiki and http://tada.github.io/ is confusing.)

fmbiete commented 3 years ago

I'm glad it's helpful.

Now, it's quite possible that any permissions being requested by the third-party software could be perfectly legitimate and inherent to what you want the software to do...

It might also be useful here (if you can without disclosing anything proprietary) to identify third-party software you are using and what permissions you find necessary for it; others using the same software might benefit from that. Perhaps there should be a wiki page here for such information.

Yes, the permissions required could be totally valid for the existing use case, this trial will help us identify that.

Third-party software is PEGA, so far I haven't seen anything from their standard package not covered by the default pljava policy.

For anyone reading this in the future, this is what it takes to enable the training option:

_/var/lib/postgresql/pljavatrial.policy (or anywhere where PostgreSQL can read)

grant {
  permission java.security.AllPermission;
};

Settings

alter system set pljava.vmoptions = "-Dorg.postgresql.pljava.policy.trial=file:/var/lib/postgresql/pljava_trial.policy ... anything you already had..."

And this is what we get when the enabled policy would block something but the trial policy allows:

POLICY DENIES/TRIAL POLICY ALLOWS: ("java.util.PropertyPermission" "user.home" "read")
java.base/java.security.ProtectionDomain.implies(ProtectionDomain.java:321)
...
java.base/java.lang.System.getProperty(System.java:816)
>> null [PLPrincipal.Sandboxed: java] <<
jcflack commented 3 years ago
java -Dpgsql.pgconfig=/usr/lib/postgresql/13/bin/pg_config -Dpgconfig.pkglibdir=debian/postgresql-13-pljava/usr/lib/postgresql/13/lib -Dpgconfig.sharedir=debian/postgresql-13-pljava/usr/share/postgresql/13 -jar build-13/pljava-pg13.jar

At build time, we can't write to /etc/* and the file should go to DESTDIR instead which is $PWD/debian/postgresql-13-pljava/.

For that, I see that you are already passing most of the -Dpgconfig.pkglibdir=... locations individually to the extractor anyway, so just adding a -Dpgconfig.sysconfdir= should put the policy file wherever you want it.

Is the presence of that file at runtime required? That will need some extra package juggling because I cannot install /etc/postgresql-common/pljava.policy as part of multiple packages (postgresql-{10,11,12,13}-pljava). Could I omit that file instead and still have usable, secure packages?

The file does need to be somewhere at runtime, readable by the server, with the pljava.policy_urls GUC pointing to it. The compiled-in default for the GUC assumes $(pg_config --sysconfdir)/pljava.policy and it is nice to have the default be right, to save the admin fiddling with GUCs. I could perhaps add another Maven build option, like pljava.libjvmdefault, to compile in a different default location at build time, and you could use that.

It is a configuration file, with some likelihood that the local admin is going to customize it (and therefore that the next update should do something more clever than just clobbering it with the uncustomized new version). I assume apt has some way of handling those? If I remember from my days using Red Hat, rpm would notice it was customized, and install the new packaged version as something like pljava.policy.rpmnew and issue a warning to look for changes. (And for quite some time, I would see those files around and think my boss must have been working on them, whose initials were rpm.)

I am intending to make a 1.6.2 release (today, barring surprises); I consider the pg_read_all_settings bug at the top of this issue to be worth fixing quickly. If you have suggestions for making the policy file easier to deal with, let me know. :)

jcflack commented 3 years ago

Hi,

The compiled-in default for the GUC assumes $(pg_config --sysconfdir)/pljava.policy and it is nice to have the default be right, to save the admin fiddling with GUCs. I could perhaps add another Maven build option, like pljava.libjvmdefault, to compile in a different default location at build time, and you could use that.

Would that be useful? I haven't made the 1.6.2 release yet....

df7cb commented 3 years ago

I have already put the extra package in place that will ship the config file so that's not a problem and let's not overcomplicate the solution by adding more switches. (It needs to be a single separate package because I can't put the file into postgresql-12-pljava because then it would conflict with postgresql-13-pljava.)

Let's continue the other packaging questions in #336, I shouldn't have captured this issue with those qualms. Thanks!

jcflack commented 3 years ago

Ok. The switch wasn't too much trouble and is implemented in explorative/REL1_6_STABLE/policydefault in case it is ever needed, but I won't bother merging it unless there's a request.

Over to #336 for more.

jcflack commented 3 years ago

Returning to the original topic: believed resolved in release 1.6.2.

For the use of future readers:

grant {
  permission java.security.AllPermission;
};

Well, yes, that will work ... but some might consider AllPermission to be living rather close to the edge. It's ok if the only code you are testing is code that you have been using for years and already completely trust (and it doesn't have any bugs that your database users could try to exploit, and you have done REVOKE USAGE ON LANGUAGE java FROM PUBLIC so nobody can create some other Java function while you are in trial mode and call it, or all of the testing is done on a dev server with nothing important, etc.).

Otherwise,

grant {
  permission org.postgresql.pljava.policy.TrialPolicy$Permission;
};

will probably still cover the most likely legitimate permissions your code under test might need, but with a reduced potential for excitement.

POLICY DENIES/TRIAL POLICY ALLOWS: ("java.util.PropertyPermission" "user.home" "read")

So, I'm wondering: is there anything in the code you're testing that actually has a reason to read user.home (which is likely to come out as whatever directory is assigned to the OS user that runs postgres)? Or is that just a result of some habit of testing PL/Java by calling System.getProperty on user.home?

I've noticed that lots of PL/Java first-time blog posts suggest something like that as a test after installation ... this might call for some new blog posts that suggest using some other property that is readable by default, like, for example, java.version.

fmbiete commented 3 years ago

In my scenario it was a previously running application and we enabled this in a development environment. For new applications we will use TrialPolicy$Permission.

I used "user.home" as an example because I knew that would be blocked and it wouldn't expose any internals from our application :smile: But, you are totally right, I cannot think either of a good reason for an application to access the home dir of postgres.

jcflack commented 3 years ago

I hope I didn't come across critical of your process ... for a previously running application and in a dev environment it makes sense. I just didn't want any future readers of this issue to mistake it for a general recommendation.

As 1.6.2 is released now, I'll mark this issue closed.

samrose commented 4 months ago
java -Dpgsql.pgconfig=/usr/lib/postgresql/13/bin/pg_config -Dpgconfig.pkglibdir=debian/postgresql-13-pljava/usr/lib/postgresql/13/lib -Dpgconfig.sharedir=debian/postgresql-13-pljava/usr/share/postgresql/13 -jar build-13/pljava-pg13.jar

At build time, we can't write to /etc/* and the file should go to DESTDIR instead which is $PWD/debian/postgresql-13-pljava/.

For that, I see that you are already passing most of the -Dpgconfig.pkglibdir=... locations individually to the extractor anyway, so just adding a -Dpgconfig.sysconfdir= should put the policy file wherever you want it.

Is the presence of that file at runtime required? That will need some extra package juggling because I cannot install /etc/postgresql-common/pljava.policy as part of multiple packages (postgresql-{10,11,12,13}-pljava). Could I omit that file instead and still have usable, secure packages?

The file does need to be somewhere at runtime, readable by the server, with the pljava.policy_urls GUC pointing to it. The compiled-in default for the GUC assumes $(pg_config --sysconfdir)/pljava.policy and it is nice to have the default be right, to save the admin fiddling with GUCs. I could perhaps add another Maven build option, like pljava.libjvmdefault, to compile in a different default location at build time, and you could use that.

It is a configuration file, with some likelihood that the local admin is going to customize it (and therefore that the next update should do something more clever than just clobbering it with the uncustomized new version). I assume apt has some way of handling those? If I remember from my days using Red Hat, rpm would notice it was customized, and install the new packaged version as something like pljava.policy.rpmnew and issue a warning to look for changes. (And for quite some time, I would see those files around and think my boss must have been working on them, whose initials were rpm.)

I am intending to make a 1.6.2 release (today, barring surprises); I consider the pg_read_all_settings bug at the top of this issue to be worth fixing quickly. If you have suggestions for making the policy file easier to deal with, let me know. :)

Hey just for fun, in case someone tumbles their way down this rabbit hole from the https://nixos.org world:

the solution above that gives java an install dir in a debian packaging scenario also works in a nix packages scenario

in my case it was like this

  installPhase = ''
    mkdir -p $out/share
    mkdir -p $out/lib
    mkdir -p $out/etc
    java -Dpgconfig=${postgresql}/bin/pg_config \
      -Dpgconfig.sharedir=$out/share \
      -Dpgconfig.sysconfdir==$out/etc/pljava.policy \
      -Dpgconfig.pkglibdir=$out/lib \
      -jar $out/pljava-packaging/target/pljava-pg15.jar
  '';