diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java b/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java index 44510cc0bb5..3e2b1c7cb86 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java @@ -24,6 +24,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.security.AccessControlContext; import java.security.AccessController; +import java.security.PrivilegedAction; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; @@ -557,8 +558,18 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } final Method readMethod = pd.getReadMethod(); try { - if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) { - readMethod.setAccessible(true); + if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers()) && !readMethod.isAccessible()) { + if (System.getSecurityManager() != null) { + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + readMethod.setAccessible(true); + return null; + } + }); + } + else { + readMethod.setAccessible(true); + } } Object value = null; @@ -852,8 +863,18 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra else { if (isExtractOldValueForEditor() && pd.getReadMethod() != null) { final Method readMethod = pd.getReadMethod(); - if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) { - readMethod.setAccessible(true); + if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers()) && !readMethod.isAccessible()) { + if (System.getSecurityManager()!= null) { + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + readMethod.setAccessible(true); + return null; + } + }); + } + else { + readMethod.setAccessible(true); + } } try { if (System.getSecurityManager() != null) { @@ -882,8 +903,18 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra pv.getOriginalPropertyValue().conversionNecessary = (valueToApply != originalValue); } final Method writeMethod = pd.getWriteMethod(); - if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) { - writeMethod.setAccessible(true); + if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers()) && !writeMethod.isAccessible()) { + if (System.getSecurityManager()!= null) { + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + writeMethod.setAccessible(true); + return null; + } + }); + } + else { + writeMethod.setAccessible(true); + } } final Object value = valueToApply; diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index f5489814b1c..4d7afdc25a4 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1496,8 +1496,15 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac if (logger.isDebugEnabled()) { logger.debug("Invoking init method '" + initMethodName + "' on bean with name '" + beanName + "'"); } - ReflectionUtils.makeAccessible(initMethod); + if (System.getSecurityManager() != null) { + AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Object run() throws Exception { + ReflectionUtils.makeAccessible(initMethod); + return null; + } + }); + try { AccessController.doPrivileged(new PrivilegedExceptionAction() { @@ -1514,6 +1521,7 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac } else { try { + ReflectionUtils.makeAccessible(initMethod); initMethod.invoke(bean, (Object[]) null); } catch (InvocationTargetException ex) { diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 3ea75663537..69edb3cb2e8 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -209,16 +209,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp final String name, final Class requiredType, final Object[] args, final boolean typeCheckOnly) throws BeansException { - if (System.getSecurityManager() != null) { - return AccessController.doPrivileged(new PrivilegedAction() { - public T run() { - return doGetBeanRaw(name, requiredType, args, typeCheckOnly); - } - }); - } - else { return doGetBeanRaw(name, requiredType, args, typeCheckOnly); - } } /** * Return an instance, which may be shared or independent, of the specified bean. @@ -1446,7 +1437,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp @Override protected AccessControlContext getAccessControlContext() { SecurityContextProvider provider = getSecurityContextProvider(); - return (provider != null ? provider.getAccessControlContext(): null); + return (provider != null ? provider.getAccessControlContext(): AccessController.getContext()); } /** diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java index 66858699e7a..30b93f7f598 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java @@ -385,8 +385,23 @@ class ConstructorResolver { // Need to determine the factory method... // Try all methods with this name to see if they match the given arguments. factoryClass = ClassUtils.getUserClass(factoryClass); - Method[] rawCandidates = (mbd.isNonPublicAccessAllowed() ? - ReflectionUtils.getAllDeclaredMethods(factoryClass) : factoryClass.getMethods()); + Method[] rawCandidates = null; + + final Class factoryClazz = factoryClass; + if (System.getSecurityManager() != null) { + + rawCandidates = AccessController.doPrivileged(new PrivilegedAction() { + public Method[] run() { + return (mbd.isNonPublicAccessAllowed() ? + ReflectionUtils.getAllDeclaredMethods(factoryClazz) : factoryClazz.getMethods()); + } + }); + } + else { + rawCandidates = (mbd.isNonPublicAccessAllowed() ? + ReflectionUtils.getAllDeclaredMethods(factoryClazz) : factoryClazz.getMethods()); + } + List candidateSet = new ArrayList(); for (Method candidate : rawCandidates) { if (Modifier.isStatic(candidate.getModifiers()) == isStatic && diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java index b176b5026f8..540a83b3bb1 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java @@ -21,6 +21,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.security.AccessControlContext; import java.security.AccessController; +import java.security.PrivilegedAction; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; @@ -81,7 +82,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { * @param postProcessors the List of BeanPostProcessors * (potentially DestructionAwareBeanPostProcessor), if any */ - public DisposableBeanAdapter(Object bean, String beanName, RootBeanDefinition beanDefinition, + public DisposableBeanAdapter(final Object bean, String beanName, RootBeanDefinition beanDefinition, List postProcessors, AccessControlContext acc) { Assert.notNull(bean, "Bean must not be null"); @@ -92,14 +93,26 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { this.nonPublicAccessAllowed = beanDefinition.isNonPublicAccessAllowed(); this.acc = acc; - String destroyMethodName = beanDefinition.getDestroyMethodName(); + final String destroyMethodName = beanDefinition.getDestroyMethodName(); if (destroyMethodName != null && !(this.invokeDisposableBean && "destroy".equals(destroyMethodName)) && !beanDefinition.isExternallyManagedDestroyMethod(destroyMethodName)) { this.destroyMethodName = destroyMethodName; try { - this.destroyMethod = (this.nonPublicAccessAllowed ? - BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName) : - BeanUtils.findMethodWithMinimalParameters(bean.getClass().getMethods(), destroyMethodName)); + if (System.getSecurityManager() != null) { + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + destroyMethod = (nonPublicAccessAllowed ? + BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName) : + BeanUtils.findMethodWithMinimalParameters(bean.getClass().getMethods(), destroyMethodName)); + return null; + } + }); + } + else { + this.destroyMethod = (this.nonPublicAccessAllowed ? + BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName) : + BeanUtils.findMethodWithMinimalParameters(bean.getClass().getMethods(), destroyMethodName)); + } } catch (IllegalArgumentException ex) { throw new BeanDefinitionValidationException("Couldn't find a unique destroy method on bean with name '" + @@ -229,9 +242,15 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { logger.debug("Invoking destroy method '" + this.destroyMethodName + "' on bean with name '" + this.beanName + "'"); } - ReflectionUtils.makeAccessible(destroyMethod); try { if (System.getSecurityManager() != null) { + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + ReflectionUtils.makeAccessible(destroyMethod); + return null; + } + }); + try { AccessController.doPrivileged(new PrivilegedExceptionAction() { public Object run() throws Exception { @@ -244,6 +263,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { } } else { + ReflectionUtils.makeAccessible(destroyMethod); destroyMethod.invoke(bean, args); } } catch (InvocationTargetException ex) { diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java index 7f20da06b1a..79aa3441d01 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java @@ -89,9 +89,18 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy { public Object instantiate( RootBeanDefinition beanDefinition, String beanName, BeanFactory owner, - Constructor ctor, Object[] args) { + final Constructor ctor, Object[] args) { if (beanDefinition.getMethodOverrides().isEmpty()) { + if (System.getSecurityManager() != null) { + // use own privileged to change accessibility (when security is on) + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + ReflectionUtils.makeAccessible(ctor); + return null; + } + }); + } return BeanUtils.instantiateClass(ctor, args); } else { diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/CallbacksSecurityTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/CallbacksSecurityTests.java index 1392395a918..a7d8f331cc0 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/CallbacksSecurityTests.java +++ b/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/CallbacksSecurityTests.java @@ -25,18 +25,26 @@ import java.security.Principal; import java.security.PrivilegedAction; import java.security.PrivilegedExceptionAction; import java.security.ProtectionDomain; -import java.util.Iterator; import java.util.PropertyPermission; import java.util.Set; +import javax.security.auth.AuthPermission; import javax.security.auth.Subject; import junit.framework.TestCase; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.BeanNameAware; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.SmartFactoryBean; import org.springframework.beans.factory.config.ConfigurableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.factory.support.SecurityContextProvider; import org.springframework.beans.factory.support.security.support.ConstructorBean; import org.springframework.beans.factory.support.security.support.CustomCallbackBean; @@ -45,6 +53,13 @@ import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.Resource; /** + * Security test case. Checks whether the container uses its privileges for its + * internal work but does not leak them when touching/calling user code. + * + *t The first half of the test case checks that permissions are downgraded when + * calling user code while the second half that the caller code permission get + * through and Spring doesn't override the permission stack. + * * @author Costin Leau */ public class CallbacksSecurityTests extends TestCase { @@ -52,37 +67,160 @@ public class CallbacksSecurityTests extends TestCase { private XmlBeanFactory beanFactory; private SecurityContextProvider provider; + private static class NonPrivilegedBean { - private static class TestSecuredBean { + private String expectedName; + public static boolean destroyed = false; - private String userName; + public NonPrivilegedBean(String expected) { + this.expectedName = expected; + checkCurrentContext(); + } public void init() { - AccessControlContext acc = AccessController.getContext(); - Subject subject = Subject.getSubject(acc); - System.out.println("Current acc is " +acc +" subject = " + subject); - if (subject == null) { - return; - } - setNameFromPrincipal(subject.getPrincipals()); + checkCurrentContext(); } - private void setNameFromPrincipal(Set principals) { - if (principals == null) { - return; - } - for (Iterator it = principals.iterator(); it.hasNext();) { - Principal p = it.next(); - this.userName = p.getName(); - return; - } + public void destroy() { + checkCurrentContext(); + destroyed = true; } - public String getUserName() { - return this.userName; + public void setProperty(Object value) { + checkCurrentContext(); + } + + public Object getProperty() { + checkCurrentContext(); + return null; + } + + private void checkCurrentContext() { + assertEquals(expectedName, getCurrentSubjectName()); } } - + + private static class NonPrivilegedSpringCallbacksBean implements + InitializingBean, DisposableBean, BeanClassLoaderAware, + BeanFactoryAware, BeanNameAware { + + private String expectedName; + public static boolean destroyed = false; + + public NonPrivilegedSpringCallbacksBean(String expected) { + this.expectedName = expected; + checkCurrentContext(); + } + + public void afterPropertiesSet() { + checkCurrentContext(); + } + + public void destroy() { + checkCurrentContext(); + destroyed = true; + } + + public void setBeanName(String name) { + checkCurrentContext(); + } + + public void setBeanClassLoader(ClassLoader classLoader) { + checkCurrentContext(); + } + + public void setBeanFactory(BeanFactory beanFactory) + throws BeansException { + checkCurrentContext(); + } + + private void checkCurrentContext() { + assertEquals(expectedName, getCurrentSubjectName()); + } + } + + private static class NonPrivilegedFactoryBean implements SmartFactoryBean { + private String expectedName; + + public NonPrivilegedFactoryBean(String expected) { + this.expectedName = expected; + checkCurrentContext(); + } + + public boolean isEagerInit() { + checkCurrentContext(); + return false; + } + + public boolean isPrototype() { + checkCurrentContext(); + return true; + } + + public Object getObject() throws Exception { + checkCurrentContext(); + return new Object(); + } + + public Class getObjectType() { + checkCurrentContext(); + return Object.class; + } + + public boolean isSingleton() { + checkCurrentContext(); + return false; + } + + private void checkCurrentContext() { + assertEquals(expectedName, getCurrentSubjectName()); + } + } + + private static class NonPrivilegedFactory { + + private final String expectedName; + + public NonPrivilegedFactory(String expected) { + this.expectedName = expected; + assertEquals(expectedName, getCurrentSubjectName()); + } + + public static Object makeStaticInstance(String expectedName) { + assertEquals(expectedName, getCurrentSubjectName()); + return new Object(); + } + + public Object makeInstance() { + assertEquals(expectedName, getCurrentSubjectName()); + return new Object(); + } + } + + private static String getCurrentSubjectName() { + final AccessControlContext acc = AccessController.getContext(); + + return AccessController.doPrivileged(new PrivilegedAction() { + + public String run() { + Subject subject = Subject.getSubject(acc); + if (subject == null) { + return null; + } + + Set principals = subject.getPrincipals(); + + if (principals == null) { + return null; + } + for (Principal p : principals) { + return p.getName(); + } + return null; + } + }); + } + private static class TestPrincipal implements Principal { private String name; @@ -110,12 +248,14 @@ public class CallbacksSecurityTests extends TestCase { return this.name.hashCode(); } } - + public CallbacksSecurityTests() { // setup security if (System.getSecurityManager() == null) { Policy policy = Policy.getPolicy(); - URL policyURL = getClass().getResource("/org/springframework/beans/factory/support/security/policy.all"); + URL policyURL = getClass() + .getResource( + "/org/springframework/beans/factory/support/security/policy.all"); System.setProperty("java.security.policy", policyURL.toString()); System.setProperty("policy.allowSystemProperty", "true"); policy.refresh(); @@ -127,10 +267,12 @@ public class CallbacksSecurityTests extends TestCase { @Override protected void setUp() throws Exception { - final ProtectionDomain empty = new ProtectionDomain(null, new Permissions()); + final ProtectionDomain empty = new ProtectionDomain(null, + new Permissions()); provider = new SecurityContextProvider() { - private final AccessControlContext acc = new AccessControlContext(new ProtectionDomain[] { empty }); + private final AccessControlContext acc = new AccessControlContext( + new ProtectionDomain[] { empty }); public AccessControlContext getAccessControlContext() { return acc; @@ -138,9 +280,9 @@ public class CallbacksSecurityTests extends TestCase { }; DefaultResourceLoader drl = new DefaultResourceLoader(); - Resource config = drl.getResource("/org/springframework/beans/factory/support/security/callbacks.xml"); + Resource config = drl + .getResource("/org/springframework/beans/factory/support/security/callbacks.xml"); beanFactory = new XmlBeanFactory(config); - beanFactory.setSecurityContextProvider(provider); } @@ -171,12 +313,13 @@ public class CallbacksSecurityTests extends TestCase { final Class cl = ConstructorBean.class; try { - AccessController.doPrivileged(new PrivilegedExceptionAction() { + AccessController.doPrivileged( + new PrivilegedExceptionAction() { - public Object run() throws Exception { - return cl.newInstance(); - } - }, acc); + public Object run() throws Exception { + return cl.newInstance(); + } + }, acc); fail("expected security exception"); } catch (Exception ex) { } @@ -247,7 +390,7 @@ public class CallbacksSecurityTests extends TestCase { public void testTrustedFactoryMethod() throws Exception { try { - beanFactory.getBean("trusted-factory-method"); + beanFactory.getBean("privileged-static-factory-method"); fail("expected security exception"); } catch (BeanCreationException ex) { assertTrue(ex.getMostSpecificCause() instanceof SecurityException); @@ -276,7 +419,7 @@ public class CallbacksSecurityTests extends TestCase { } }, acc); } - + public void testPropertyInjection() throws Exception { try { beanFactory.getBean("property-injection"); @@ -284,26 +427,68 @@ public class CallbacksSecurityTests extends TestCase { } catch (BeanCreationException ex) { assertTrue(ex.getMessage().contains("security")); } - + beanFactory.getBean("working-property-injection"); } - + public void testInitSecurityAwarePrototypeBean() { final DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); - RootBeanDefinition bd = new RootBeanDefinition(TestSecuredBean.class); - bd.setScope(ConfigurableBeanFactory.SCOPE_PROTOTYPE); - bd.setInitMethodName("init"); - lbf.registerBeanDefinition("test", bd); + BeanDefinitionBuilder bdb = BeanDefinitionBuilder + .genericBeanDefinition(NonPrivilegedBean.class).setScope( + ConfigurableBeanFactory.SCOPE_PROTOTYPE) + .setInitMethodName("init").setDestroyMethodName("destroy") + .addConstructorArgValue("user1"); + lbf.registerBeanDefinition("test", bdb.getBeanDefinition()); final Subject subject = new Subject(); subject.getPrincipals().add(new TestPrincipal("user1")); - TestSecuredBean bean = (TestSecuredBean) Subject.doAsPrivileged(subject, - new PrivilegedAction() { + NonPrivilegedBean bean = (NonPrivilegedBean) Subject.doAsPrivileged( + subject, new PrivilegedAction() { public Object run() { return lbf.getBean("test"); } }, null); assertNotNull(bean); - assertEquals(null, bean.getUserName()); + } + + public void testTrustedExecution() throws Exception { + beanFactory.setSecurityContextProvider(null); + + Permissions perms = new Permissions(); + perms.add(new AuthPermission("getSubject")); + ProtectionDomain pd = new ProtectionDomain(null, perms); + + AccessControlContext acc = new AccessControlContext( + new ProtectionDomain[] { pd }); + + final Subject subject = new Subject(); + subject.getPrincipals().add(new TestPrincipal("user1")); + + // request the beans from non-privileged code + Subject.doAsPrivileged(subject, new PrivilegedAction() { + + public Object run() { + // sanity check + assertEquals("user1", getCurrentSubjectName()); + assertEquals(false, NonPrivilegedBean.destroyed); + + beanFactory.getBean("trusted-spring-callbacks"); + beanFactory.getBean("trusted-custom-init-destroy"); + // the factory is a prototype - ask for multiple instances + beanFactory.getBean("trusted-spring-factory"); + beanFactory.getBean("trusted-spring-factory"); + beanFactory.getBean("trusted-spring-factory"); + + beanFactory.getBean("trusted-factory-bean"); + beanFactory.getBean("trusted-static-factory-method"); + beanFactory.getBean("trusted-factory-method"); + beanFactory.getBean("trusted-property-injection"); + beanFactory.getBean("trusted-working-property-injection"); + + beanFactory.destroySingletons(); + assertEquals(true, NonPrivilegedBean.destroyed); + return null; + } + }, provider.getAccessControlContext()); } } \ No newline at end of file diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/callbacks.xml b/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/callbacks.xml index 16315db6b36..dcbad5a5213 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/callbacks.xml +++ b/org.springframework.beans/src/test/java/org/springframework/beans/factory/support/security/callbacks.xml @@ -23,8 +23,6 @@ - - @@ -44,5 +42,46 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file