Am 23.07.2017 um 17:06 schrieb Philippe Mouawad:
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.
Well, if the user loads a valid plan, that calls "remove all data from
production" over say a http call - maybe with asking the user to fill in
a production password, than: no we don't try to protect them.
I was aiming at the first scenario: A user reads a fine blog with an
example jmx that shows how to make a first call to some test site. The
jmx he downloads is altered (by whomever) to contain bad java classes
that get de-serialized while loading the test plan.
Test plan execution is a different problem and not the one that the
xstream developers tried to warn us about.
So a possible setup and hopefully secure and usable setup would be:
Deny everything
Whitelist
every class that extends from jmeters testelements
simple java classes like string or integer and lists or maps
add all regex based whitelists the user provides through properties
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 ?
test plan execution is out of scope of xstream.
Felix
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);
