On Sun, Jul 23, 2017 at 4:55 PM, Felix Schumacher < felix.schumac...@internetallee.de> wrote:
> Am 23.07.2017 um 16:53 schrieb Philippe Mouawad: > >> Hi Felix, >> >> Can we fix it without breaking existing plugins ? >> Let's start fixing it if you have time and idea. >> > As we don't know what existing plugins are using we can't be sure that we > break things. But when we add a property to add whitelist the users can > make it work. > Can we describe the scope of what we are fixing ?: - 1) Are we trying to protect users that would load any JMX test plan ? without knowing where it comes from ? - 2) Does this protection extend to running a dangerous plan ? - 3) I don't understand what you exactly means by "someone modifying/producing bad content in the networked client setup.". How would that relate to XStream ? If 2) is not in the scope, if possible, we should also provide an API to register a white list to avoid making 3rd party plugins harder to configure. If 2) is in the scope, doesn't it impose that the property interpretation is done in a way that it cannot be changed from Test plan execution ? Or is it an acceptable limitation ? > Felix > > > >> Thanks >> Regards >> >> >> On Sun, Jul 23, 2017 at 4:50 PM, Felix Schumacher < >> felix.schumac...@internetallee.de> wrote: >> >> Am 23.07.2017 um 16:24 schrieb pmoua...@apache.org: >>> >>> Author: pmouawad >>>> Date: Sun Jul 23 14:24:36 2017 >>>> New Revision: 1802731 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1802731&view=rev >>>> Log: >>>> Bug 61329 - Warning on console "Security framework of XStream not >>>> initialized, XStream is probably vulnerable." >>>> Bugzilla Id: 61329 >>>> >>>> Modified: >>>> jmeter/trunk/src/core/org/apache/jmeter/gui/action/template >>>> /TemplateManager.java >>>> jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java >>>> jmeter/trunk/src/core/org/apache/jmeter/util/JMeterUtils.java >>>> jmeter/trunk/src/protocol/jms/org/apache/jmeter/ >>>> protocol/jms/sampler/render/ObjectMessageRenderer.java >>>> >>>> Modified: jmeter/trunk/src/core/org/apache/jmeter/gui/action/template/ >>>> TemplateManager.java >>>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apach >>>> e/jmeter/gui/action/template/TemplateManager.java?rev= >>>> 1802731&r1=1802730&r2=1802731&view=diff >>>> ============================================================ >>>> ================== >>>> --- jmeter/trunk/src/core/org/apache/jmeter/gui/action/template/ >>>> TemplateManager.java >>>> (original) >>>> +++ jmeter/trunk/src/core/org/apache/jmeter/gui/action/template/ >>>> TemplateManager.java >>>> Sun Jul 23 14:24:36 2017 >>>> @@ -87,6 +87,7 @@ public class TemplateManager { >>>> return factory; >>>> } >>>> }); >>>> + JMeterUtils.setupXStreamSecurityPolicy(xstream); >>>> xstream.alias("template", Template.class); >>>> xstream.alias("templates", Templates.class); >>>> xstream.useAttributeFor(Template.class, "isTestPlan"); >>>> >>>> Modified: jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java >>>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apach >>>> e/jmeter/save/SaveService.java?rev=1802731&r1=1802730& >>>> r2=1802731&view=diff >>>> ============================================================ >>>> ================== >>>> --- jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java >>>> (original) >>>> +++ jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java Sun >>>> Jul 23 14:24:36 2017 >>>> @@ -114,6 +114,8 @@ public class SaveService { >>>> private static final XStream JTLSAVER = new XStreamWrapper(new >>>> PureJavaReflectionProvider()); >>>> static { >>>> JTLSAVER.setMode(XStream.NO_REFERENCES); // This is needed >>>> to >>>> stop XStream keeping copies of each class >>>> + JMeterUtils.setupXStreamSecurityPolicy(JMXSAVER); >>>> + JMeterUtils.setupXStreamSecurityPolicy(JTLSAVER); >>>> } >>>> // The XML header, with placeholder for encoding, since that is >>>> controlled by property >>>> >>>> Modified: jmeter/trunk/src/core/org/apache/jmeter/util/JMeterUtils.java >>>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apach >>>> e/jmeter/util/JMeterUtils.java?rev=1802731&r1=1802730& >>>> r2=1802731&view=diff >>>> ============================================================ >>>> ================== >>>> --- jmeter/trunk/src/core/org/apache/jmeter/util/JMeterUtils.java >>>> (original) >>>> +++ jmeter/trunk/src/core/org/apache/jmeter/util/JMeterUtils.java Sun >>>> Jul 23 14:24:36 2017 >>>> @@ -69,6 +69,10 @@ import org.apache.oro.text.regex.Perl5Ma >>>> import org.slf4j.Logger; >>>> import org.slf4j.LoggerFactory; >>>> +import com.thoughtworks.xstream.XStream; >>>> +import com.thoughtworks.xstream.security.AnyTypePermission; >>>> +import com.thoughtworks.xstream.security.NoTypePermission; >>>> + >>>> /** >>>> * This class contains the static utility methods used by JMeter. >>>> * >>>> @@ -1250,4 +1254,17 @@ public class JMeterUtils implements Unit >>>> } >>>> } >>>> } >>>> + >>>> + /** >>>> + * Setup default security policy >>>> + * @param xstream {@link XStream} >>>> + */ >>>> + public static void setupXStreamSecurityPolicy(XStream xstream) { >>>> + // This will lift the insecure warning >>>> + xstream.addPermission(NoTypePermission.NONE); >>>> + // We reapply very permissive policy >>>> + // See https://groups.google.com/foru >>>> m/#!topic/xstream-user/wiKfdJPL8aY >>>> + // TODO : How much are we concerned by CVE-2013-7285 >>>> >>>> Instead of adding another TODO, maybe we should address it right now. >>> The >>> demos to exploit the framework are easily found and are a threat for >>> users >>> downloading sample jmx files. Another vector of attack would be a someone >>> modifying/producing bad content in the networked client setup. >>> >>> I think we should go for a relatively strict setup with a regex >>> white-list, that users can adapt. >>> >>> Felix >>> >>> >>> + xstream.addPermission(AnyTypePermission.ANY); >>> >>>> + } >>>> } >>>> >>>> Modified: jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms >>>> /sampler/render/ObjectMessageRenderer.java >>>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/jms/o >>>> rg/apache/jmeter/protocol/jms/sampler/render/ObjectMessageRe >>>> nderer.java?rev=1802731&r1=1802730&r2=1802731&view=diff >>>> ============================================================ >>>> ================== >>>> --- jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms >>>> /sampler/render/ObjectMessageRenderer.java (original) >>>> +++ jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms >>>> /sampler/render/ObjectMessageRenderer.java Sun Jul 23 14:24:36 2017 >>>> @@ -29,6 +29,7 @@ import javax.xml.stream.XMLStreamExcepti >>>> import javax.xml.stream.XMLStreamReader; >>>> import org.apache.jmeter.protocol.jms.sampler.PublisherSampler; >>>> +import org.apache.jmeter.util.JMeterUtils; >>>> import com.github.benmanes.caffeine.cache.Cache; >>>> import com.thoughtworks.xstream.XStream; >>>> @@ -66,6 +67,7 @@ class ObjectMessageRenderer implements M >>>> Serializable readObject = null; >>>> try { >>>> XStream xstream = new XStream(); >>>> + JMeterUtils.setupXStreamSecurityPolicy(xstream); >>>> readObject = (Serializable) xstream.fromXML(xmlMessage, >>>> readObject); >>>> } catch (Exception e) { >>>> throw new IllegalStateException("Unable to load object >>>> instance from text", e); >>>> >>>> >>>> >>>> >> > -- Cordialement. Philippe Mouawad.
