Martin Zaun wrote:
My (detail) comments and a few questions below.
Daniel John Debrunner wrote:
Rick Hillegas wrote:
For this mode to be enabled some changes are needed to DERBY-2109:
- Enforce Derby's security permissions if there is a security
manager (regardless of Derby's authentication state)
Like Rick, I'm also ok with this. But I'd just like to refresh our
memory that the reason we'd made the checks for System Privileges
dependent on Derby's authentication was backward compatibility for
users
- with customized policy files running
- with a Security Manager (default) but
- no authentication (default);
because they will have to extend their policy files for two blocks
of permissions:
grant codeBase "${derby.install.url}derby.jar" {
...
permission javax.security.auth.AuthPermission "doAsPrivileged";
permission java.util.PropertyPermission "user.dir", "read"; //
unless granted already
permission java.io.FilePermission "${user.dir}${/}-", "read"; //
unless granted already
}
grant principal org.apache.derby.authentication.SystemPrincipal * { //
or specific user
permission org.apache.derby.security.SystemPermission "shutdown";
permission org.apache.derby.security.DatabasePermission
"directory:<<ALL FILES>>", "create";
};
I'm ok with this. However, I don't want to hold up the good work done
so far on DERBY-2109. Unless there are strong objections, I'm
inclined to commit the current patch. As a follow-on patch, we can
then make the Derby permissions-checker preserve the other identities
which the invoker may have.
I fine with the patch going ahead and the preserving of identities
being a follow on, but this point is about not checking for the
existance of Derby's authentication before invoking security manager
checks. It seems somewhat pointless to have that in the patch (along
with tests?) only to remove it as a follow on.
Glad to hear that there are no other major obstacles for going forward.
- Continue to support shutting the network server & engine down
without authentication credentials but only from within the same
virtual machine. This shutdown would require the Derby shutdown
permission if a security manager was installed.
Just to be clear: In order to gracefully shutdown the engine today,
you need to get a shutdown connection to the engine. If
authentication is enabled, then you must present credentials in order
to get that connection. If you can't get a shutdown connection, then
you can't shutdown the engine gracefully--although you can crash the
engine along with the rest of the VM by calling System.exit().
If the purpose of these changes is to avoid denial-of-service attacks
then I'm not sure how the engine is shutdown really matters. A
non-graceful shutdown doesn't really have any different external
visibility since recovery is automatic on the next boot.
Let me recap the shutdown behaviour with DERBY-2109 (as specified and
implemented) plus what I think you, Dan, would like to see changed:
1) To shutdown the engine:
a) From a jdbc client with a shutdown URL (remote and embedded):
clients need to provide user credentials if and only if running
with authentication "on". Nothing changes here with DERBY-2109.
Not sure: For the embedded case with Derby authentication "on",
was it suggested to drop the requirement for providing user
credentials with the URL shutdown request?
This is the issue I'm going to enter as a new jira entry, it's not an
issue of embedded, but an issue of code running within the same virtual
machine. It's based around the fact that since code within the vm is
pretty much free to do whatever it wants, what security does system
authentication provide? Thus maybe allowing system shutdown without
authentication is ok but only if it's from the same jvm. These checks
would still be subject to security manager checks, but having this would
allow a valid jmx-user to perform shutdown if they had that permission
without requiring an extra authentication step.
b) When running Derby within the same JVM as the application: I wonder
if user code can bypass our authentication and authorization checks
by calling directly:
org.apache.derby.iapi.services.monitor.Monitor.getMonitor().shutdown()
Right, and how Java enforces such security is to have security manager
checks in the sensitive code, which is what DERBY-2109 is doing. It may
turn out that after this patch more checks are needed but that could be
follow on work. Currently 2109 is addressing the shutdown api's
correctly (ie. requiring shutdown permission), it may be that there are
other holes that need to be closed, but then again they may just be
holes from within the same vm which may be less of a concern.
c) Also, I wonder if this Monitor.getMonitor().shutdown() trick can be
done from Procedures? Or is this prohibited by the ClassLoader?
Derby doesn't allow any calls into Derby (org.apache.derby.*) code from
procedures or functions.
2) To shutdown the server:
a) From a separate process (on the same host as the Derby server):
users can run NetworkServerControl "shutdown" command or call the
shutdown() method on a NetworkServerControl instance.
With DERBY-2109, the NetworkServerControl's "shutdown" command now
supports user/password arguments. Also, an extra constructor for
NetworkServerControl takes user credential arguments, which are
stored and passed when the shutdown() method is invoked (for API
backward compatibility). User credentials must be provided if and
only if running Derby with authentication "on". In addition,
DERBY-2109 checks for authorization if running with authentication
and a SecurityManager.
I understand that
- NetworkServerControl's shutdown authentication support as
implemented in DERBY-2109 is ok, but
- the shutdown authorization check should NOT be made dependent
upon prior authentication but Java Security only.
Right?
Yes. Any java security permission check should always be invoked
regardless of anything else. The only exception is no security manager
means no check needed.
b) For running a Derby server within the same JVM as the application
I see two options:
- user code may call the shutdown() method on any
NetworkServerControl instance (which sends the request via a
socket connection to the server within the same process);
- user code can create an instance of NetworkServerControlImpl,
start the network server by calling start() on this instance,
and later bring it orderly down by calling directShutdown() on
the same (!) instance.
With DERBY-2109, there's an additional NetworkServerControlImpl()
constructor taking user credential arguments. Again, credentials
are required if (and only if) running Derby with authentication
"on". We check for authorization if running with authentication
and a SecurityManager.
The formerly public method that unconditionally shuts down the
server has now become private to prevent a program from bypassing
the authentication and authorization checks and been renamed to
directShutdownInternal().
I understand that as with 2a), the authorization check should NOT
be made dependent upon prior authentication.
However, I'm not sure about the authentication behaviour: Do we
need a NetworkServerControlImpl method (for in-process-clients)
that bypasses Derby's authentication but does check for
authorization before shutting down the server?
Even more: Should the method directShutdownInternal() be made
public, so that callers within the same JVM can bypass both the
shutdown authentication and authorization checks?
Note that any direct shutdown needs to be invoked on the same
NetworkServerControlImpl instance that started the server, so,
any such caller first needs to get hold of a reference.
I think this is all the options for the follow on issue, the current
patch approach of always having authentication is good. We can then
improve from there.
Is there some other change we should make to the DERBY-2109 behavior
beyond the change discussed in the previous bullet?
I think there's additional functionality, but it's a follow on to
DERBY-2109. That issue is already taking on too much, better to have a
separate clear issue. I'll enter one (which will be dependent on
DERBY-2109).
Well, we've changed the DERBY-2109 spec before :) But I'd appreciate
if the patch can be committed soon and cleanup/extension/optimization
items on System Privileges are handled in a separate JIRA.
So the patch needs some more work, Rick tested it and saw failures so it
seems unlikely it will get committed in that state. One question is will
you be changing it so that permission checks are always executed and not
just when authentication is needed? It's good if you indicate what you
are planning to do so that it's clear what's going on, e.g. to avoid the
situation where a committer is waiting for a new version of the patch
and you are waiting for the current version to be committed.
Dan.
