RFC: SecurityManager merge

RFC: SecurityManager merge

[Gary Benson]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

Hi all,

The attached patch merges most of GCJ's java.lang.SecurityManager with
Classpath, making use of the now-working AccessController.  The patch
is based on http://gcc.gnu.org/ml/java-patches/2004-q3/msg00372.html.
For reference I've also attached a diff of the remaining differences
from Classpath.

Comments?

Cheers,
Gary

[-- Attachment #2: securitymanager-merge.patch --]
[-- Type: text/plain, Size: 7484 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 115901)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@
+2006-08-03  Gary Benson  <gbenson@redhat.com>
+	    Casey Marshall <csm@gnu.org>
+
+	* java/lang/SecurityManager.java (getSecurityContext,
+	checkPermission, checkAccess, checkRead, checkConnect,
+	checkPackageAccess, checkPackageDefinition, checkPackageList):
+	Merge with Classpath.
+	(SecurityContext): Remove.
+
 2006-08-02  Andreas Tobler  <a.tobler@schweiz.ch>
 
 	PR libgcj/28546
Index: java/lang/SecurityManager.java
===================================================================
--- java/lang/SecurityManager.java	(revision 115901)
+++ java/lang/SecurityManager.java	(working copy)
@@ -1,5 +1,6 @@
 /* SecurityManager.java -- security checks for privileged actions
-   Copyright (C) 1998, 1999, 2001, 2002, 2005  Free Software Foundation, Inc.
+   Copyright (C) 1998, 1999, 2001, 2002, 2005, 2006
+   Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -45,11 +46,15 @@
 import java.lang.reflect.Member;
 import java.net.InetAddress;
 import java.net.SocketPermission;
+import java.security.AccessController;
+import java.security.AccessControlContext;
 import java.security.AllPermission;
 import java.security.Permission;
+import java.security.PrivilegedAction;
 import java.security.Security;
 import java.security.SecurityPermission;
 import java.util.PropertyPermission;
+import java.util.StringTokenizer;
 
 /**
  * SecurityManager is a class you can extend to create your own Java
@@ -315,8 +320,7 @@
    */
   public Object getSecurityContext()
   {
-    // XXX Should be: return AccessController.getContext();
-    return new SecurityContext(getClassContext());
+    return AccessController.getContext();
   }
 
   /**
@@ -331,8 +335,7 @@
    */
   public void checkPermission(Permission perm)
   {
-    // XXX Should be: AccessController.checkPermission(perm);
-    //.throw new SecurityException("Operation not allowed");
+    AccessController.checkPermission(perm);
   }
 
   /**
@@ -353,11 +356,9 @@
    */
   public void checkPermission(Permission perm, Object context)
   {
-    // XXX Should be:
-    // if (! (context instanceof AccessControlContext))
-    //   throw new SecurityException("Missing context");
-    // ((AccessControlContext) context).checkPermission(perm);
-    throw new SecurityException("Operation not allowed");
+    if (! (context instanceof AccessControlContext))
+      throw new SecurityException("Missing context");
+    ((AccessControlContext) context).checkPermission(perm);
   }
 
   /**
@@ -402,7 +403,7 @@
   public void checkAccess(Thread thread)
   {
     if (thread.getThreadGroup() != null 
-	&& thread.getThreadGroup().getParent() != null)
+	&& thread.getThreadGroup().getParent() == null)
       checkPermission(new RuntimePermission("modifyThread"));
   }
 
@@ -435,7 +436,7 @@
    */
   public void checkAccess(ThreadGroup g)
   {
-    if (g.getParent() != null)
+    if (g.getParent() == null)
       checkPermission(new RuntimePermission("modifyThreadGroup"));
   }
 
@@ -556,12 +557,10 @@
    */
   public void checkRead(String filename, Object context)
   {
-    // XXX Should be:
-    // if (! (context instanceof AccessControlContext))
-    //   throw new SecurityException("Missing context");
-    // AccessControlContext ac = (AccessControlContext) context;
-    // ac.checkPermission(new FilePermission(filename, "read"));
-    // throw new SecurityException("Cannot read files via file names.");
+    if (! (context instanceof AccessControlContext))
+      throw new SecurityException("Missing context");
+    AccessControlContext ac = (AccessControlContext) context;
+    ac.checkPermission(new FilePermission(filename, "read"));
   }
 
   /**
@@ -675,17 +674,15 @@
    */
   public void checkConnect(String host, int port, Object context)
   {
-    // XXX Should be:
-    // if (! (context instanceof AccessControlContext))
-    //   throw new SecurityException("Missing context");
-    // AccessControlContext ac = (AccessControlContext) context;
-    // if (port == -1)
-    //   ac.checkPermission(new SocketPermission(host, "resolve"));
-    // else
-    //   // Use the toString() hack to do the null check.
-    //   ac.checkPermission(new SocketPermission(host.toString + ":" +port,
-    //                                           "connect"));
-    // throw new SecurityException("Cannot make network connections.");
+    if (! (context instanceof AccessControlContext))
+      throw new SecurityException("Missing context");
+    AccessControlContext ac = (AccessControlContext) context;
+    if (port == -1)
+      ac.checkPermission(new SocketPermission(host, "resolve"));
+    else
+      // Use the toString() hack to do the null check.
+      ac.checkPermission(new SocketPermission(host.toString() + ":" + port,
+                                              "connect"));
   }
 
   /**
@@ -902,7 +899,7 @@
    */
   public void checkPackageAccess(String packageName)
   {
-    checkPackageList(packageName, "access", "accessClassInPackage.");
+    checkPackageList(packageName, "package.access", "accessClassInPackage.");
   }
 
   /**
@@ -924,7 +921,7 @@
    */
   public void checkPackageDefinition(String packageName)
   {
-    checkPackageList(packageName, "definition", "defineClassInPackage.");
+    checkPackageList(packageName, "package.definition", "defineClassInPackage.");
   }
 
   /**
@@ -1027,38 +1024,34 @@
    * @see #checkPackageAccess(String)
    * @see #checkPackageDefinition(String)
    */
-  void checkPackageList(String packageName, String restriction,
+  void checkPackageList(String packageName, final String restriction,
                         String permission)
   {
-    // Use the toString() hack to do the null check.
-    Permission p = new RuntimePermission(permission + packageName.toString());
-    String list = Security.getProperty("package." + restriction);
-    if (list == null)
+    if (packageName == null)
+      throw new NullPointerException();
+
+    String list = (String)AccessController.doPrivileged(new PrivilegedAction()
+      {
+	public Object run()
+        {
+	  return Security.getProperty(restriction);
+	}
+      });
+
+    if (list == null || list.equals(""))
       return;
-    while (! "".equals(packageName))
+
+    String packageNamePlusDot = packageName + ".";
+
+    StringTokenizer st = new StringTokenizer(list, ",");
+    while (st.hasMoreTokens())
       {
-        for (int index = list.indexOf(packageName);
-             index != -1; index = list.indexOf(packageName, index + 1))
-          {
-            // Exploit package visibility for speed.
-	    int packageNameCount = packageName.length();
-            if (index + packageNameCount == list.length()
-                || list.charAt(index + packageNameCount) == ',')
-              {
-                checkPermission(p);
-                return;
-              }
-          }
-        int index = packageName.lastIndexOf('.');
-        packageName = index < 0 ? "" : packageName.substring(0, index);
+	if (packageNamePlusDot.startsWith(st.nextToken()))
+	  {
+	    Permission p = new RuntimePermission(permission + packageName);
+	    checkPermission(p);
+	    return;
+	  }
       }
   }
-} // class SecurityManager
-
-// XXX This class is unnecessary.
-class SecurityContext {
-	Class[] classes;
-	SecurityContext(Class[] classes) {
-		this.classes = classes;
-	}
 }

[-- Attachment #3: java.lang.SecurityManager.diff --]
[-- Type: text/plain, Size: 2176 bytes --]

--- classpath/java/lang/SecurityManager.java	2006-07-26 09:24:57.000000000 +0100
+++ java/lang/SecurityManager.java	2006-08-02 16:41:32.000000000 +0100
@@ -1,5 +1,6 @@
 /* SecurityManager.java -- security checks for privileged actions
-   Copyright (C) 1998, 1999, 2001, 2002, 2005  Free Software Foundation, Inc.
+   Copyright (C) 1998, 1999, 2001, 2002, 2005, 2006
+   Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -38,35 +39,20 @@
 
 package java.lang;
 
-import gnu.classpath.VMStackWalker;
-
 import java.awt.AWTPermission;
 import java.io.File;
 import java.io.FileDescriptor;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.FilePermission;
-import java.io.RandomAccessFile;
 import java.lang.reflect.Member;
 import java.net.InetAddress;
-import java.net.ServerSocket;
-import java.net.Socket;
-import java.net.SocketImplFactory;
 import java.net.SocketPermission;
-import java.net.URL;
-import java.net.URLStreamHandlerFactory;
-import java.security.AccessControlContext;
-import java.security.AccessControlException;
 import java.security.AccessController;
+import java.security.AccessControlContext;
 import java.security.AllPermission;
-import java.security.BasicPermission;
 import java.security.Permission;
-import java.security.Policy;
 import java.security.PrivilegedAction;
-import java.security.ProtectionDomain;
 import java.security.Security;
 import java.security.SecurityPermission;
-import java.util.Properties;
 import java.util.PropertyPermission;
 import java.util.StringTokenizer;
 
@@ -194,10 +180,7 @@
    */
   protected Class[] getClassContext()
   {
-    Class[] stack1 = VMStackWalker.getClassContext();
-    Class[] stack2 = new Class[stack1.length - 1];
-    System.arraycopy(stack1, 1, stack2, 0, stack1.length - 1);
-    return stack2;
+    return VMSecurityManager.getClassContext(SecurityManager.class);
   }
 
   /**
@@ -219,8 +202,7 @@
    */
   protected ClassLoader currentClassLoader()
   {
-    Class cl = currentLoadedClass();
-    return cl != null ? cl.getClassLoader() : null;
+    return VMSecurityManager.currentClassLoader(SecurityManager.class);
   }
 
   /**

Re: RFC: SecurityManager merge

[Tom Tromey]

>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> The attached patch merges most of GCJ's java.lang.SecurityManager with
Gary> Classpath, making use of the now-working AccessController.  The patch
Gary> is based on http://gcc.gnu.org/ml/java-patches/2004-q3/msg00372.html.

Looks good, thanks.

Gary> For reference I've also attached a diff of the remaining differences
Gary> from Classpath.

Thanks for including this detail in the email; I appreciated it.

Tom

Re: RFC: SecurityManager merge

[Gary Benson]

Tom Tromey wrote:
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
> 
> Gary> The attached patch merges most of GCJ's
> Gary> java.lang.SecurityManager with  Classpath, making use of
> Gary> the now-working AccessController.  The patch is based on
> Gary> http://gcc.gnu.org/ml/java-patches/2004-q3/msg00372.html.
> 
> Looks good, thanks.

Committed.

> Gary> For reference I've also attached a diff of the remaining
> Gary> differences from Classpath.
> 
> Thanks for including this detail in the email; I appreciated it.

Cool :)