db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kristian Waagan (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DERBY-5571) IndexStatisticsDaemonImpl.schedule should wrap Thread.setDaemon() in a privilege block
Date Tue, 17 Jan 2012 21:01:40 GMT

    [ https://issues.apache.org/jira/browse/DERBY-5571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13187996#comment-13187996
] 

Kristian Waagan commented on DERBY-5571:
----------------------------------------

I don't have any experience here, but note the JavaDoc for SecurityManager.checkAccess:
"""
public void checkAccess(Thread t)

Throws a SecurityException if the calling thread is not allowed to modify the thread argument.

This method is invoked for the current security manager by the stop, suspend, resume, setPriority,
setName, and setDaemon methods of class Thread.

If the thread argument is a system thread (belongs to the thread group with a null parent)
then this method calls checkPermission with the RuntimePermission("modifyThread") permission.
If the thread argument is not a system thread, this method just returns silently.

Applications that want a stricter policy should override this method. If this method is overridden,
the method that overrides it should additionally check to see if the calling thread has the
RuntimePermission("modifyThread") permission, and if so, return silently. This is to ensure
that code granted that permission (such as the JDK itself) is allowed to manipulate any thread.

If this method is overridden, then super.checkAccess should be called by the first statement
in the overridden method, or the equivalent security check should be placed in the overridden
method.
"""

My take on this is that in the normal case those permissions (modifyThread and modifyThreadGroup)
aren't required. I don't know in which "natural cases", i.e. where the user isn't making the
system thread group being used on purpose, they are required. Further, Derby should never
try to modify the state of any system threads.

I don't know if it would be benefical to require modifyThread in the documentation anyway,
but I fear that we then will allow Derby to do something it shouldn't be allowed to do. The
same reasoning goes fro modifyThreadGroup. Based on the documentation for Thread, it is not
easy to see that the permission is only required to modify system threads.
I don't know if it makes sense to use doPrivileged to allow people to easily implement a stricter
security policy/manager (by overriding the checkAccess method), i.e if you want to require
modifyThread for modification of threads in all thread groups.
                
> IndexStatisticsDaemonImpl.schedule  should wrap Thread.setDaemon() in a privilege block
> ---------------------------------------------------------------------------------------
>
>                 Key: DERBY-5571
>                 URL: https://issues.apache.org/jira/browse/DERBY-5571
>             Project: Derby
>          Issue Type: Bug
>            Reporter: Kathey Marsden
>
> IndexStatisticsDaemonImple.schedule() has the following code. setDaemon can throw a SecurityException
so should be wrapped. It says: SecurityException - if the current thread cannot modify this
thread.
> Does this mean that our documentation should require modifyThreadGroup privs too?
> Currently it is in our test policy but not the documentation:
> // These permissions are needed by AssertFailure to dump the thread stack
>   // traces upon failure.
>   //permission java.lang.RuntimePermission "getStackTrace";
>   permission java.lang.RuntimePermission "modifyThreadGroup";
>                // If we're idle, fire off the worker thread.
>                 if (runningThread == null) {
>                     runningThread = new Thread(this, "index-stat-thread");
>                     // Make the thread a daemon thread, we don't want it to stop
>                     // the JVM from exiting. This is a precaution.
>                     runningThread.setDaemon(true);
> Marking as a regression as a security violation could make existing statements fail.
>                     

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message