Yes, we should hide this impl from the public somehow. In an EE server I guess this could be done via OSGi.
Anyway, it's actually not more bad than having a public SecurityUtil class with public static methods of the same signature... Both are kind of broken, but the Service might at least theoretically get shielded way better. Btw imo the whole SecurityManager stuff is broken by design, ever was... LieGrue, strub --- On Tue, 3/15/11, David Jencks <david_jen...@yahoo.com> wrote: > From: David Jencks <david_jen...@yahoo.com> > Subject: Re: svn commit: r1081681 - in /openwebbeans/trunk: > webbeans-impl/src/main/java/org/apache/webbeans/corespi/ > webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ > webbeans-impl/src/main/resources/META-INF/openwebbeans/ > webbeans-openejb/src/main/... > To: dev@openwebbeans.apache.org > Date: Tuesday, March 15, 2011, 6:25 PM > I'm wondering how this is supposed to > work. I haven't looked at the code in detail but it > looks to me like you've introduced a public class with a > default constructor that anyone with malicious intent can > instantiate and call all these methods on to do unrestricted > reflection that the security manager setup was supposed to > prevent. > > Have I misunderstood how this works? > > I wonder if having a default constructor that checked for > reflection privileges would suffice to restrict access to > this class, and calling it from within a protected > action. I think we'd still have to be very careful not > to pass the instance around in such a way as to make it > accessible from outside owb. > > hopefully I've misunderstood... > > thanks > david jencks > > On Mar 15, 2011, at 1:27 AM, strub...@apache.org > wrote: > > > Author: struberg > > Date: Tue Mar 15 08:27:37 2011 > > New Revision: 1081681 > > > > URL: http://svn.apache.org/viewvc?rev=1081681&view=rev > > Log: > > OWB-545 introduce ManagedSecurityService > > > > Added: > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ManagedSecurityService.java > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/SimpleSecurityService.java > > - copied, changed from r1081676, > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/SimpleSecurityService.java > > Removed: > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/SimpleSecurityService.java > > Modified: > > > openwebbeans/trunk/webbeans-impl/src/main/resources/META-INF/openwebbeans/openwebbeans.properties > > > openwebbeans/trunk/webbeans-openejb/src/main/java/org/apache/webbeans/ejb/service/OpenEJBSecurityService.java > > > openwebbeans/trunk/webbeans-spi/src/main/java/org/apache/webbeans/spi/SecurityService.java > > > openwebbeans/trunk/webbeans-tomcat6/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > > > openwebbeans/trunk/webbeans-tomcat7/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > > > > Added: > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ManagedSecurityService.java > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ManagedSecurityService.java?rev=1081681&view=auto > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ManagedSecurityService.java > (added) > > +++ > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/ManagedSecurityService.java > Tue Mar 15 08:27:37 2011 > > @@ -0,0 +1,329 @@ > > +/* > > + * Licensed to the Apache Software Foundation (ASF) > under one > > + * or more contributor license agreements. See the > NOTICE file > > + * distributed with this work for additional > information > > + * regarding copyright ownership. The ASF licenses > this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in > compliance > > + * with the License. You may obtain a copy of the > License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in > writing, > > + * software distributed under the License is > distributed on an > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF > ANY > > + * KIND, either express or implied. See the License > for the > > + * specific language governing permissions and > limitations > > + * under the License. > > + */ > > +package org.apache.webbeans.corespi.security; > > + > > +import > org.apache.webbeans.exception.WebBeansException; > > +import org.apache.webbeans.spi.SecurityService; > > + > > +import java.lang.reflect.AccessibleObject; > > +import java.lang.reflect.Constructor; > > +import java.lang.reflect.Field; > > +import java.lang.reflect.Method; > > +import java.security.AccessController; > > +import java.security.Principal; > > +import java.security.PrivilegedAction; > > +import java.security.PrivilegedActionException; > > +import java.security.PrivilegedExceptionAction; > > +import java.util.Properties; > > + > > +/** > > + * This version of the {@link SecurityService} uses > the java.lang.SecurityManager > > + * to check low level access to the underlying > functions via a doPriviliged block. > > + */ > > +public class ManagedSecurityService implements > SecurityService > > +{ > > + private static final int > METHOD_CLASS_GETDECLAREDCONSTRUCTOR = 0x01; > > + > > + private static final int > METHOD_CLASS_GETDECLAREDCONSTRUCTORS = 0x02; > > + > > + private static final int > METHOD_CLASS_GETDECLAREDMETHOD = 0x03; > > + > > + private static final int > METHOD_CLASS_GETDECLAREDMETHODS = 0x04; > > + > > + private static final int > METHOD_CLASS_GETDECLAREDFIELD = 0x05; > > + > > + private static final int > METHOD_CLASS_GETDECLAREDFIELDS = 0x06; > > + > > + private static final > PrivilegedActionGetSystemProperties SYSTEM_PROPERTY_ACTION = > new PrivilegedActionGetSystemProperties(); > > + > > + > > + > > + @Override > > + public Principal getCurrentPrincipal() > > + { > > + // no pricipal by > default > > + return null; > > + } > > + > > + @Override > > + public <T> Constructor<T> > doPrivilegedGetDeclaredConstructor(Class<T> clazz, > Class<?>... parameterTypes) throws > NoSuchMethodException > > + { > > + Object obj = > AccessController.doPrivileged( > > + > new PrivilegedActionForClass(clazz, parameterTypes, > METHOD_CLASS_GETDECLAREDCONSTRUCTOR)); > > + if (obj instanceof > NoSuchMethodException) > > + { > > + throw > (NoSuchMethodException)obj; > > + } > > + return > (Constructor<T>)obj; > > + } > > + > > + @Override > > + public <T> Constructor<?>[] > doPrivilegedGetDeclaredConstructors(Class<T> clazz) > > + { > > + Object obj = > AccessController.doPrivileged( > > + > new PrivilegedActionForClass(clazz, null, > METHOD_CLASS_GETDECLAREDCONSTRUCTORS)); > > + return > (Constructor<T>[])obj; > > + } > > + > > + @Override > > + public <T> Method > doPrivilegedGetDeclaredMethod(Class<T> clazz, String > name, Class<?>... parameterTypes) > > + throws NoSuchMethodException > > + { > > + Object obj = > AccessController.doPrivileged( > > + > new PrivilegedActionForClass(clazz, new Object[] > {name, parameterTypes}, METHOD_CLASS_GETDECLAREDMETHOD)); > > + if (obj instanceof > NoSuchMethodException) > > + { > > + throw > (NoSuchMethodException)obj; > > + } > > + return (Method)obj; > > + } > > + > > + @Override > > + public <T> Method[] > doPrivilegedGetDeclaredMethods(Class<T> clazz) > > + { > > + Object obj = > AccessController.doPrivileged( > > + > new PrivilegedActionForClass(clazz, null, > METHOD_CLASS_GETDECLAREDMETHODS)); > > + return (Method[])obj; > > + } > > + > > + @Override > > + public <T> Field > doPrivilegedGetDeclaredField(Class<T> clazz, String > name) throws NoSuchFieldException > > + { > > + Object obj = > AccessController.doPrivileged( > > + > new PrivilegedActionForClass(clazz, name, > METHOD_CLASS_GETDECLAREDFIELD)); > > + if (obj instanceof > NoSuchFieldException) > > + { > > + throw > (NoSuchFieldException)obj; > > + } > > + return (Field)obj; > > + } > > + > > + @Override > > + public <T> Field[] > doPrivilegedGetDeclaredFields(Class<T> clazz) > > + { > > + Object obj = > AccessController.doPrivileged( > > + > new PrivilegedActionForClass(clazz, null, > METHOD_CLASS_GETDECLAREDFIELDS)); > > + return (Field[])obj; > > + } > > + > > + @Override > > + public void > doPrivilegedSetAccessible(AccessibleObject obj, boolean > flag) > > + { > > + > AccessController.doPrivileged(new > PrivilegedActionForSetAccessible(obj, flag)); > > + } > > + > > + @Override > > + public boolean > doPrivilegedIsAccessible(AccessibleObject obj) > > + { > > + return (Boolean) > AccessController.doPrivileged(new > PrivilegedActionForIsAccessible(obj)); > > + } > > + > > + @Override > > + public <T> T > doPrivilegedObjectCreate(Class<T> clazz) throws > PrivilegedActionException, IllegalAccessException, > InstantiationException > > + { > > + return (T) > AccessController.doPrivileged(new > PrivilegedActionForObjectCreation(clazz)); > > + } > > + > > + @Override > > + public void > doPrivilegedSetSystemProperty(String propertyName, String > value) > > + { > > + > AccessController.doPrivileged(new > PrivilegedActionForSetProperty(propertyName, value)); > > + } > > + > > + @Override > > + public String > doPrivilegedGetSystemProperty(String propertyName, String > defaultValue) > > + { > > + return > AccessController.doPrivileged(new > PrivilegedActionForProperty(propertyName, defaultValue)); > > + } > > + > > + @Override > > + public Properties > doPrivilegedGetSystemProperties() > > + { > > + return > AccessController.doPrivileged(SYSTEM_PROPERTY_ACTION); > > + } > > + > > + > > + // the following block contains > internal wrapper classes for doPrivileged actions > > + > > + protected static class > PrivilegedActionForClass implements > PrivilegedAction<Object> > > + { > > + private Class<?> > clazz; > > + > > + private Object > parameters; > > + > > + private int method; > > + > > + protected > PrivilegedActionForClass(Class<?> clazz, Object > parameters, int method) > > + { > > + this.clazz > = clazz; > > + > this.parameters = parameters; > > + this.method > = method; > > + } > > + > > + public Object run() > > + { > > + try > > + { > > + > switch (method) > > + > { > > + > case > METHOD_CLASS_GETDECLAREDCONSTRUCTOR: > > + > return > clazz.getDeclaredConstructor((Class<?>[])parameters); > > + > case > METHOD_CLASS_GETDECLAREDCONSTRUCTORS: > > + > return > clazz.getDeclaredConstructors(); > > + > case METHOD_CLASS_GETDECLAREDMETHOD: > > + > String name = > (String)((Object[])parameters)[0]; > > + > Class<?>[] > realParameters = > (Class<?>[])((Object[])parameters)[1]; > > + > return > clazz.getDeclaredMethod(name, realParameters); > > + > case METHOD_CLASS_GETDECLAREDMETHODS: > > + > return > clazz.getDeclaredMethods(); > > + > case METHOD_CLASS_GETDECLAREDFIELD: > > + > return > clazz.getDeclaredField((String)parameters); > > + > case METHOD_CLASS_GETDECLAREDFIELDS: > > + > return > clazz.getDeclaredFields(); > > + > > + > default: > > + > return new > WebBeansException("unknown security method: " + method); > > + > } > > + } > > + catch > (Exception exception) > > + { > > + > return exception; > > + } > > + } > > + > > + } > > + > > + protected static class > PrivilegedActionForSetAccessible implements > PrivilegedAction<Object> > > + { > > + > > + private AccessibleObject > object; > > + > > + private boolean flag; > > + > > + protected > PrivilegedActionForSetAccessible(AccessibleObject object, > boolean flag) > > + { > > + this.object > = object; > > + this.flag = > flag; > > + } > > + > > + public Object run() > > + { > > + > object.setAccessible(flag); > > + return > null; > > + } > > + } > > + > > + protected static class > PrivilegedActionForIsAccessible implements > PrivilegedAction<Object> > > + { > > + > > + private AccessibleObject > object; > > + > > + protected > PrivilegedActionForIsAccessible(AccessibleObject object) > > + { > > + this.object > = object; > > + } > > + > > + public Object run() > > + { > > + return > object.isAccessible(); > > + } > > + } > > + > > + protected static class > PrivilegedActionForProperty implements > PrivilegedAction<String> > > + { > > + private final String > propertyName; > > + > > + private final String > defaultValue; > > + > > + protected > PrivilegedActionForProperty(String propertyName, String > defaultValue) > > + { > > + > this.propertyName = propertyName; > > + > this.defaultValue = defaultValue; > > + } > > + > > + @Override > > + public String run() > > + { > > + return > System.getProperty(this.propertyName,this.defaultValue); > > + } > > + > > + } > > + > > + protected static class > PrivilegedActionForSetProperty implements > PrivilegedAction<Object> > > + { > > + private final String > propertyName; > > + > > + private final String > value; > > + > > + protected > PrivilegedActionForSetProperty(String propertyName, String > value) > > + { > > + > this.propertyName = propertyName; > > + this.value > = value; > > + } > > + > > + @Override > > + public String run() > > + { > > + > System.setProperty(propertyName, value); > > + return > null; > > + } > > + > > + } > > + > > + protected static class > PrivilegedActionGetSystemProperties implements > PrivilegedAction<Properties> > > + { > > + > > + @Override > > + public Properties run() > > + { > > + return > System.getProperties(); > > + } > > + > > + } > > + > > + protected static class > PrivilegedActionForObjectCreation implements > PrivilegedExceptionAction<Object> > > + { > > + private Class<?> > clazz; > > + > > + protected > PrivilegedActionForObjectCreation(Class<?> clazz) > > + { > > + this.clazz > = clazz; > > + } > > + > > + @Override > > + public Object run() > throws Exception > > + { > > + try > > + { > > + > return clazz.newInstance(); > > + } > > + catch > (InstantiationException e) > > + { > > + > throw e; > > + } > > + catch > (IllegalAccessException e) > > + { > > + > throw e; > > + } > > + } > > + > > + } > > + > > + > > +} > > > > Copied: > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/SimpleSecurityService.java > (from r1081676, > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/SimpleSecurityService.java) > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/SimpleSecurityService.java?p2=openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/SimpleSecurityService.java&p1=openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/SimpleSecurityService.java&r1=1081676&r2=1081681&rev=1081681&view=diff > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/SimpleSecurityService.java > (original) > > +++ > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/corespi/security/SimpleSecurityService.java > Tue Mar 15 08:27:37 2011 > > @@ -16,7 +16,7 @@ > > * specific language governing permissions and > limitations > > * under the License. > > */ > > -package org.apache.webbeans.corespi; > > +package org.apache.webbeans.corespi.security; > > > > import org.apache.webbeans.spi.SecurityService; > > > > @@ -46,6 +46,12 @@ public class SimpleSecurityService > imple > > } > > > > @Override > > + public <T> Constructor<T> > doPrivilegedGetDeclaredConstructor(Class<T> clazz, > Class<?>... parameterTypes) throws > NoSuchMethodException > > + { > > + return > clazz.getDeclaredConstructor(parameterTypes); > > + } > > + > > + @Override > > public <T> > Constructor<?>[] > doPrivilegedGetDeclaredConstructors(Class<T> clazz) > > { > > return > clazz.getDeclaredConstructors(); > > > > Modified: > openwebbeans/trunk/webbeans-impl/src/main/resources/META-INF/openwebbeans/openwebbeans.properties > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/resources/META-INF/openwebbeans/openwebbeans.properties?rev=1081681&r1=1081680&r2=1081681&view=diff > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-impl/src/main/resources/META-INF/openwebbeans/openwebbeans.properties > (original) > > +++ > openwebbeans/trunk/webbeans-impl/src/main/resources/META-INF/openwebbeans/openwebbeans.properties > Tue Mar 15 08:27:37 2011 > > @@ -58,7 +58,7 @@ > org.apache.webbeans.spi.ContextsService= > > ################################### Default Contexts > Service #################################### > > # Default SecurityService implementation which > directly invokes underlying classes > > # without using a SecurityManager > > > -org.apache.webbeans.spi.SecurityService=org.apache.webbeans.corespi.SimpleSecurityService > > > +org.apache.webbeans.spi.SecurityService=org.apache.webbeans.corespi.security.SimpleSecurityService > > > ################################################################################################ > > > > > ################################################################################################ > > > > > Modified: > openwebbeans/trunk/webbeans-openejb/src/main/java/org/apache/webbeans/ejb/service/OpenEJBSecurityService.java > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-openejb/src/main/java/org/apache/webbeans/ejb/service/OpenEJBSecurityService.java?rev=1081681&r1=1081680&r2=1081681&view=diff > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-openejb/src/main/java/org/apache/webbeans/ejb/service/OpenEJBSecurityService.java > (original) > > +++ > openwebbeans/trunk/webbeans-openejb/src/main/java/org/apache/webbeans/ejb/service/OpenEJBSecurityService.java > Tue Mar 15 08:27:37 2011 > > @@ -21,7 +21,7 @@ package > org.apache.webbeans.ejb.service; > > import java.security.Principal; > > > > import org.apache.openejb.loader.SystemInstance; > > -import > org.apache.webbeans.corespi.SimpleSecurityService; > > +import > org.apache.webbeans.corespi.security.SimpleSecurityService; > > import org.apache.webbeans.spi.SecurityService; > > > > public class OpenEJBSecurityService extends > SimpleSecurityService implements SecurityService > > > > Modified: > openwebbeans/trunk/webbeans-spi/src/main/java/org/apache/webbeans/spi/SecurityService.java > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-spi/src/main/java/org/apache/webbeans/spi/SecurityService.java?rev=1081681&r1=1081680&r2=1081681&view=diff > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-spi/src/main/java/org/apache/webbeans/spi/SecurityService.java > (original) > > +++ > openwebbeans/trunk/webbeans-spi/src/main/java/org/apache/webbeans/spi/SecurityService.java > Tue Mar 15 08:27:37 2011 > > @@ -48,6 +48,12 @@ public interface SecurityService > > public Principal > getCurrentPrincipal(); > > > > /** > > + * @see > Class#getDeclaredConstructor(Class[]) > > + */ > > + public <T> Constructor<T> > doPrivilegedGetDeclaredConstructor(Class<T> clazz, > Class<?>... parameterTypes) > > + throws NoSuchMethodException; > > + > > + /** > > * @see > Class#getDeclaredConstructors() > > */ > > public <T> > Constructor<?>[] > doPrivilegedGetDeclaredConstructors(Class<T> clazz); > > @@ -55,7 +61,8 @@ public interface SecurityService > > /** > > * @see > Class#getDeclaredMethod(String, Class[]) > > */ > > - public <T> Method > doPrivilegedGetDeclaredMethod(Class<T> clazz, String > name, Class<?>... parameterTypes) throws > NoSuchMethodException; > > + public <T> Method > doPrivilegedGetDeclaredMethod(Class<T> clazz, String > name, Class<?>... parameterTypes) > > + throws NoSuchMethodException; > > > > /** > > * @see Class#getDeclaredMethods() > > > > Modified: > openwebbeans/trunk/webbeans-tomcat6/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-tomcat6/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java?rev=1081681&r1=1081680&r2=1081681&view=diff > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-tomcat6/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > (original) > > +++ > openwebbeans/trunk/webbeans-tomcat6/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > Tue Mar 15 08:27:37 2011 > > @@ -20,7 +20,7 @@ package > org.apache.webbeans.web.tomcat; > > > > import java.security.Principal; > > > > -import > org.apache.webbeans.corespi.SimpleSecurityService; > > +import > org.apache.webbeans.corespi.security.SimpleSecurityService; > > import org.apache.webbeans.spi.SecurityService; > > > > public class TomcatSecurityService extends > SimpleSecurityService implements SecurityService > > > > Modified: > openwebbeans/trunk/webbeans-tomcat7/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > > URL: > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-tomcat7/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java?rev=1081681&r1=1081680&r2=1081681&view=diff > > > ============================================================================== > > --- > openwebbeans/trunk/webbeans-tomcat7/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > (original) > > +++ > openwebbeans/trunk/webbeans-tomcat7/src/main/java/org/apache/webbeans/web/tomcat/TomcatSecurityService.java > Tue Mar 15 08:27:37 2011 > > @@ -20,7 +20,7 @@ package > org.apache.webbeans.web.tomcat; > > > > import java.security.Principal; > > > > -import > org.apache.webbeans.corespi.SimpleSecurityService; > > +import > org.apache.webbeans.corespi.security.SimpleSecurityService; > > import org.apache.webbeans.spi.SecurityService; > > > > public class TomcatSecurityService extends > SimpleSecurityService implements SecurityService > > > > > >
