[cp-patches] Patch: PrivilegedAction helper classes

[Bryce McKinlay]

This patch adds two classes which are implementations of 
PrivilagedAction to be used for AccessController.doPrivilagedCalls: 
GetPropertyAction and SetAccessibleAction, and updates some callers to 
use them.

The idea is that these give us a) much cleaner syntax to make privileged 
called, compared with anonymous class instantiations; and b) allow us to 
reuse a single PrivilegedAction implementation across multiple 
privileged calls, which reduces overhead.

For example, where we previously had something like:

 String prop = (String) AccessController.doPrivileged(new 
PrivilegedAction()
 {
   public Object run()
   {
     return System.getProperty("foo.bar");
   }
 };

Can now be written as:

String prop = (String) AccessController.doPrivileged(new 
GetPropertyAction("foo.bar"));

Please use these new classes when adding new doPrivileged() calls, and 
consider adding new ones for security-checked calls that are likely to 
be frequently used within the runtime. We also need to be prudent when 
adding such calls to ensure that the overhead added by implementing the 
security model remains low. For example, System.getProperty() calls 
should be moved to static initializers wherever possible, and where 
doPrivilaged() calls are likely to be made frequently, consider caching 
PrivilegedAction objects in local or static variables where it is safe 
to do so (eg: in a synchronized block or where instances of the object 
are not expected to be thread-safe).

Regards

Bryce


2004-07-23  Bryce McKinlay  <address@hidden>

        * gnu/java/net/protocol/http/Connection.java: Use GetPropertyAction
        for privilaged getProperty calls.
        * java/io/ObjectOutputStream.java (getField): No longer static. Use
        SetAccessibleAction instead of anonymous class for doPrivilaged call.
        (getMethod): Likewise.
        (setAccessible): New field. PrivilagedAction object to use when
        calling setAccessible.
        * java/io/ObjectStreamClass.java (calculateOffsets): Use
        SetAccessibleAction instead of anonymous class for diPrivilaged call.
        (setFields): Likewise.
        (getClassUID): Likewise.
        (findMethod): Likewise.
        * gnu/java/security/action/GetPropertyAction.java: New class.
        * gnu/java/security/action/SetAccessibleAction.java: New class. 

Index: gnu/java/security/action/GetPropertyAction.java
===================================================================
RCS file: gnu/java/security/action/GetPropertyAction.java
diff -N gnu/java/security/action/GetPropertyAction.java
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ gnu/java/security/action/GetPropertyAction.java     23 Jul 2004 21:56:47 
-0000
@@ -0,0 +1,75 @@
+/* GetPropertyAction.java
+   Copyright (C) 2004 Free Software Foundation, Inc.
+
+This file is part of GNU Classpath.
+
+GNU Classpath is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2, or (at your option)
+any later version.
+
+GNU Classpath is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Classpath; see the file COPYING.  If not, write to the
+Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+02111-1307 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version. */
+
+package gnu.java.security.action;
+
+import java.security.PrivilegedAction;
+
+/**
+ * PrivilegedAction implementation that calls System.getProperty() with
+ * the property name passed to its constructor.
+ *
+ * Example of use:
+ * <code>
+ * GetPropertyAction action = new GetPropertyAction("http.proxyPort");
+ * String port = AccessController.doPrivileged(action);
+ * </code>
+ */
+public class GetPropertyAction implements PrivilegedAction
+{
+  String propName;
+
+  public GetPropertyAction()
+  {
+  }
+  
+  public GetPropertyAction(String propName)
+  {
+    this.propName = propName;
+  }
+  
+  public Object run()
+  {
+    return System.getProperty(propName);
+  }
+  
+  public GetPropertyAction setName(String propName)
+  {
+    this.propName = propName;
+    return this;
+  }
+}
Index: gnu/java/security/action/SetAccessibleAction.java
===================================================================
RCS file: gnu/java/security/action/SetAccessibleAction.java
diff -N gnu/java/security/action/SetAccessibleAction.java
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ gnu/java/security/action/SetAccessibleAction.java   23 Jul 2004 21:56:47 
-0000
@@ -0,0 +1,77 @@
+/* SetAccessibleAction.java
+   Copyright (C) 2004 Free Software Foundation, Inc.
+
+This file is part of GNU Classpath.
+
+GNU Classpath is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2, or (at your option)
+any later version.
+
+GNU Classpath is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Classpath; see the file COPYING.  If not, write to the
+Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+02111-1307 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version. */
+
+package gnu.java.security.action;
+
+import java.lang.reflect.AccessibleObject;
+import java.security.PrivilegedAction;
+
+/**
+ * PrivilagedAction implementation that calls setAccessible(true) on the 
+ * AccessibleObject passed to its constructor.
+ *
+ * Example of use:
+ * <code>
+ * Field dataField = cl.getDeclaredField("data");
+ * AccessController.doPrivilaged(new SetAccessibleAction(dataField));
+ * </code>
+ */
+public class SetAccessibleAction implements PrivilegedAction
+{
+  AccessibleObject member;
+  
+  public SetAccessibleAction()
+  {
+  }
+  
+  public SetAccessibleAction(AccessibleObject member)
+  {
+    this.member = member;
+  }
+  
+  public Object run()
+  {
+    member.setAccessible(true);
+    return null;
+  }
+  
+  public SetAccessibleAction setMember(AccessibleObject member)
+  {
+    this.member = member;
+    return this;
+  }
+}
Index: java/io/ObjectOutputStream.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/io/ObjectOutputStream.java,v
retrieving revision 1.25
diff -u -r1.25 ObjectOutputStream.java
--- java/io/ObjectOutputStream.java     21 Jun 2004 16:52:14 -0000      1.25
+++ java/io/ObjectOutputStream.java     23 Jul 2004 21:56:47 -0000
@@ -48,6 +48,7 @@
 
 import gnu.java.io.ObjectIdentityWrapper;
 import gnu.java.lang.reflect.TypeSignature;
+import gnu.java.security.action.SetAccessibleAction;
 import gnu.classpath.Configuration;
 
 /**
@@ -1516,20 +1517,14 @@
       }    
   }
 
-  private static Field getField (Class klass, String name)
+  private Field getField (Class klass, String name)
     throws java.io.InvalidClassException
   {
     try
       {
        final Field f = klass.getDeclaredField(name);
-       AccessController.doPrivileged(new PrivilegedAction()
-         {
-           public Object run()
-           {
-             f.setAccessible(true);
-             return null;
-           }
-         });
+       setAccessible.setMember(f);
+       AccessController.doPrivileged(setAccessible);
        return f;
       }
     catch (java.lang.NoSuchFieldException e)
@@ -1539,18 +1534,12 @@
       }
   }
 
-  private static Method getMethod (Class klass, String name, Class[] args)
+  private Method getMethod (Class klass, String name, Class[] args)
     throws java.lang.NoSuchMethodException
   {
     final Method m = klass.getDeclaredMethod(name, args);
-    AccessController.doPrivileged(new PrivilegedAction()
-      {
-       public Object run()
-       {
-         m.setAccessible(true);
-         return null;
-       }
-      });
+    setAccessible.setMember(m);
+    AccessController.doPrivileged(setAccessible);
     return m;
   }
 
@@ -1583,6 +1572,7 @@
   private Hashtable OIDLookupTable;
   private int protocolVersion;
   private boolean useSubclassMethod;
+  private SetAccessibleAction setAccessible = new SetAccessibleAction();
 
   // The nesting depth for debugging output
   private int depth = 0;
Index: java/io/ObjectStreamClass.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/io/ObjectStreamClass.java,v
retrieving revision 1.21
diff -u -r1.21 ObjectStreamClass.java
--- java/io/ObjectStreamClass.java      20 Apr 2004 11:37:41 -0000      1.21
+++ java/io/ObjectStreamClass.java      23 Jul 2004 21:56:47 -0000
@@ -57,6 +57,7 @@
 import java.util.Vector;
 import gnu.java.io.NullOutputStream;
 import gnu.java.lang.reflect.TypeSignature;
+import gnu.java.security.action.SetAccessibleAction;
 import gnu.java.security.provider.Gnu;
 
 
@@ -470,14 +471,8 @@
                    }
                }
                final Method m = methods[i];
-               AccessController.doPrivileged(new PrivilegedAction()
-               {
-                   public Object run()
-                   {
-                       m.setAccessible(true);
-                       return null;
-                   }
-               });
+               SetAccessibleAction setAccessible = new SetAccessibleAction(m);
+               AccessController.doPrivileged(setAccessible);
                return m;
            }
        }
@@ -543,6 +538,8 @@
   // clazz.
   private void setFields(Class cl)
   {
+    SetAccessibleAction setAccessible = new SetAccessibleAction();
+
     if (!isSerializable() || isExternalizable())
       {
        fields = NO_FIELDS;
@@ -551,17 +548,11 @@
 
     try
       {
-       final Field serialPersistentFields =
+       final Field f =
          cl.getDeclaredField("serialPersistentFields");
-       AccessController.doPrivileged(new PrivilegedAction()
-       {
-           public Object run()
-           {
-               serialPersistentFields.setAccessible(true);
-               return null;
-           }
-       });
-       int modifiers = serialPersistentFields.getModifiers();
+       setAccessible.setMember(f);
+       AccessController.doPrivileged(setAccessible);
+       int modifiers = f.getModifiers();
 
        if (Modifier.isStatic(modifiers)
            && Modifier.isFinal(modifiers)
@@ -617,14 +608,8 @@
       if (all_fields[from] != null)
        {
          final Field f = all_fields[from];
-         AccessController.doPrivileged(new PrivilegedAction()
-         {
-             public Object run()
-             {
-                 f.setAccessible(true);
-                 return null;
-             }
-         });
+         setAccessible.setMember(f);
+         AccessController.doPrivileged(setAccessible);
          fields[to] = new ObjectStreamField(all_fields[from]);
          to++;
        }
@@ -651,14 +636,8 @@
        // may not be public AND we only want the serialVersionUID of this
        // class, not a superclass or interface.
        final Field suid = cl.getDeclaredField("serialVersionUID");
-       AccessController.doPrivileged(new PrivilegedAction()
-       {
-           public Object run()
-           {
-               suid.setAccessible(true);
-               return null;
-           }
-       });
+       SetAccessibleAction setAccessible = new SetAccessibleAction(suid);
+       AccessController.doPrivileged(setAccessible);
        int modifiers = suid.getModifiers();
 
        if (Modifier.isStatic(modifiers)
Index: gnu/java/net/protocol/http/Connection.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/java/net/protocol/http/Connection.java,v
retrieving revision 1.17
diff -u -r1.17 Connection.java
--- gnu/java/net/protocol/http/Connection.java  23 Jul 2004 11:49:59 -0000      
1.17
+++ gnu/java/net/protocol/http/Connection.java  23 Jul 2004 21:56:47 -0000
@@ -59,6 +59,7 @@
 import java.util.Iterator;
 import java.util.Map;
 import gnu.java.net.HeaderFieldHelper;
+import gnu.java.security.action.GetPropertyAction;
 
 /**
  * This subclass of java.net.URLConnection models a URLConnection via
@@ -88,36 +89,31 @@
 
   static 
   {
-    // Make sure access control for system properties depends only on
-    // our class ProtectionDomain, not on any (indirect) callers.
-    AccessController.doPrivileged(new PrivilegedAction() {
-       public Object run()
-       {
-         // Recognize some networking properties listed at
-         // http://java.sun.com/j2se/1.4/docs/guide/net/properties.html.
-         String port = null;
-         proxyHost = System.getProperty("http.proxyHost");
-         if (proxyHost != null)
-           {
-             proxyInUse = true;
-             if ((port = System.getProperty("http.proxyPort")) != null)
-               {
-                 try
-                   {
-                     proxyPort = Integer.parseInt(port);
-                   }
-                 catch (Throwable t)
-                   {
-                     // Nothing.  
-                   }
-               }
-           }
-         
-         userAgent = System.getProperty("http.agent");
+    // Recognize some networking properties listed at
+    // http://java.sun.com/j2se/1.4/docs/guide/net/properties.html.
+    String port = null;
+    GetPropertyAction getProperty = new GetPropertyAction("http.proxyHost");
+    proxyHost = (String) AccessController.doPrivileged(getProperty);
+    if (proxyHost != null)
+      {
+       proxyInUse = true;
+       getProperty.setName("http.proxyPort");
+       port = (String) AccessController.doPrivileged(getProperty);
+       if (port != null)
+         {
+           try
+             {
+               proxyPort = Integer.parseInt(port);
+             }
+           catch (NumberFormatException ex)
+             {
+               // Nothing.  
+             }
+         }
+      }
 
-         return null;
-       }
-      });
+    getProperty.setName("http.agent");
+    userAgent = (String) AccessController.doPrivileged(getProperty);
   }
 
   /**