Skip to content

Conversation

@jcflack
Copy link
Contributor

@jcflack jcflack commented Nov 24, 2020

Issue #331 prevents extension startup by a normal (non-superuser, non-pg_read_all_settings) user. Fix that.

Further, by request, make available a custom security policy that can log permission requests that the regular policy would deny, but then allow them, so that existing code being migrated from PL/Java 1.5 can be run without disruption while evaluating it for permission grants that may need to be added to the regular policy.

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

You can create another policy file somewhere, the "trial" policy, 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 the very extreme of generosity it could be this:

grant {
  permission java.security.AllPermission;
};

and it would happily allow the 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.) Such a setting would be difficult to recommend in general, but it might suffice if the only code being tested has already been in use for years under PL/Java 1.5 and is well trusted, and if the purpose of testing is only to learn what permissions it uses that may need to be granted in the 1.6 policy.

When AllPermission is too broad, 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 this PR adds 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 in the "really rather not" category. If its hard-coded exclusion list excludes any permissions that some unusual code under test might need, those can be explicitly added 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. The meta-permission TrialPolicy$Permission tries to come somewhere near 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.

About false positives

One thing to be aware of is that the trial policy can give 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 the configuration allows. 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 code not being granted those permissions.

There may be no 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 code 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 code 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.

False positives, of course, are also affected by the choice of how generous to make the trial policy. Log entries are only produced for permissions that the regular policy denies but the trial policy allows. If the permissions being silently checked by benign code are not granted in the trial policy, they will be silently denied as in normal operation, and produce no log entries.

Format of the log entries

To avoid bloating logs too much, TrialPolicy emits 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. For the purpose of identifying the source of a permission request and the appropriate domain(s) to be granted the permission, this is probably more usable than the very long full traces output by java.security.debug.

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. If it is set to 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.

Have to access the value directly for internal purposes, as
GetConfigOption will honor the GUC_SUPERUSER_ONLY on it.

Add tests as non-superuser to the Travis and AppVeyor configs.
Not having any was an oversight.

In passing, try adding PG 13 and Java 15 to the Travis and AppVeyor
builds. I have not otherwise checked whether those are available
yet in those services. This should be a way to find out.
It appears that the time is not yet ripe.
The system property org.postgresql.pljava.policy.trial, if supplied,
is a URI naming another policy file that can serve as a second chance
for accesses denied by the real policy laid out by pljava.policy_urls.
If the denied access is allowed by this policy, it is allowed, but
logged, so needed permissions can be identified while running code.
If denied by this policy, the denial is final.

A good bit of honest work in this policy goes to abbreviating the
log output in a useful way. 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 failing domain
shown at the appropriate position in the trace.
Java has an AllPermission, which can be granted in a TrialPolicy
in order to identify, log, and allow any and all requested permissions
that are not granted in the normal policy.

Some may not be comfortable proceeding that way, and because Java
does not have negative permissions, there is no way to say "grant
AllPermission except for these that I would like to exclude".

Therefore, org.postgresql.pljava.policy.TrialPolicy$Permission is
provided here, as pretty much exactly "AllPermission except for
a couple dozen unsurprising exclusions." This permission can be
granted in a TrialPolicy (and, in the unlikely event that any of
the permissions it excludes also have to be granted there, they
can be granted explicitly, in the usual way).

A package in a named module that implements any new Permission
must be exported to java.base; this is moved to the new package
org.postgresql.pljava.policy and so exported.
The provider() method on a service provider has to be static.
Misdeclaring this one never broke anything, as the service loader
simply fell back on the constructor, but later service loader
passes after the policy gets installed would cause false-alarm
log entries from the constructor.
There isn't a perfect method available (yet) to inquire "could I
doInPG() or log() at this moment without blocking?", so this check
may unnecessarily send the message off to System.err on occasions
when log() would have worked fine. But it's better than blocking.

In passing, add a permission to read another property that the
Java runtime started using in Java 14 but forgot to give itself
permission to read. It's at the false-alarm level of severity, as
the runtime behaves gracefully when unable to read the property,
but again, if using TrialPolicy, it kind of spams the log.
Such a build appears to work, and is listed as PG 13, but on
closer inspection turns out to have built with 12 anyway,
which apparently is what's in pacman.
Factoring out some of the common elements of the Travis and AppVeyor
(and, one day, GitHub Actions) scripts can be a project for another day.
@jcflack jcflack merged commit 700b8b8 into REL1_6_STABLE Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants