You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by FSchumacher <gi...@git.apache.org> on 2017/07/23 17:05:09 UTC
[GitHub] jmeter pull request #301: First stab at a whitelist for the xstream library.
GitHub user FSchumacher opened a pull request:
https://github.com/apache/jmeter/pull/301
First stab at a whitelist for the xstream library.
`ant test` will run with the current whitelist, but I fear that most plugins would fail.
It would probably be cumbersome for users to search for the correct whitelist to setup JMeter for their beloved plugins. But what are the alternatives?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/FSchumacher/jmeter more-xstream
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jmeter/pull/301.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #301
----
commit 8455581474bccc1c433e88b90efda0eeee723d1a
Author: Felix Schumacher <fe...@internetallee.de>
Date: 2017-07-23T16:57:54Z
First stab at a whitelist for the xstream library.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request #301: First stab at a whitelist for the xstream library.
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/301#discussion_r131525468
--- Diff: src/core/org/apache/jmeter/util/JMeterUtils.java ---
@@ -1261,10 +1265,39 @@ public static final void refreshUI() {
*/
public static void setupXStreamSecurityPolicy(XStream xstream) {
// This will lift the insecure warning
+ // disallow any class
xstream.addPermission(NoTypePermission.NONE);
- // We reapply very permissive policy
- // See https://groups.google.com/forum/#!topic/xstream-user/wiKfdJPL8aY
- // TODO : How much are we concerned by CVE-2013-7285
- xstream.addPermission(AnyTypePermission.ANY);
+ // allow some classes that are hopefully secure
+ for (TypePermission perm : Arrays.asList(NullPermission.NULL,
+ ArrayTypePermission.ARRAYS,
+ PrimitiveTypePermission.PRIMITIVES)) {
+ xstream.addPermission(perm);
+ }
+ for (Class<?> allowHierarchy : Arrays.asList(java.util.Collection.class,
+ org.apache.jmeter.testelement.TestElement.class,
+ org.apache.jorphan.collections.HashTree.class,
+ org.apache.jmeter.samplers.SampleSaveConfiguration.class)) {
+ xstream.allowTypeHierarchy(allowHierarchy);
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #301: First stab at a whitelist for the xstream library.
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on the issue:
https://github.com/apache/jmeter/pull/301
Now both the whitelist should be checked first and if the class is **not** allowed a warning will be logged (as info).
To support dynamic additions by plugins, we could have a look at Emi's suggestion of a ServiceLoader.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request #301: First stab at a whitelist for the xstream library.
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/301#discussion_r128925907
--- Diff: src/core/org/apache/jmeter/util/JMeterUtils.java ---
@@ -1261,10 +1265,39 @@ public static final void refreshUI() {
*/
public static void setupXStreamSecurityPolicy(XStream xstream) {
// This will lift the insecure warning
+ // disallow any class
xstream.addPermission(NoTypePermission.NONE);
- // We reapply very permissive policy
- // See https://groups.google.com/forum/#!topic/xstream-user/wiKfdJPL8aY
- // TODO : How much are we concerned by CVE-2013-7285
- xstream.addPermission(AnyTypePermission.ANY);
+ // allow some classes that are hopefully secure
+ for (TypePermission perm : Arrays.asList(NullPermission.NULL,
+ ArrayTypePermission.ARRAYS,
+ PrimitiveTypePermission.PRIMITIVES)) {
+ xstream.addPermission(perm);
+ }
+ for (Class<?> allowHierarchy : Arrays.asList(java.util.Collection.class,
+ org.apache.jmeter.testelement.TestElement.class,
+ org.apache.jorphan.collections.HashTree.class,
+ org.apache.jmeter.samplers.SampleSaveConfiguration.class)) {
+ xstream.allowTypeHierarchy(allowHierarchy);
--- End diff --
Shouldn't we also allow subclasses of SampleResult ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #301: First stab at a whitelist for the xstream library.
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/301
Hi @FSchumacher ,
Thanks for the PR.
Maybe we could add a interface that plugins could implement to register additional classes:
Too coarse and risky ?:
`SerializationRegistrator#registerClasses(XStream xstream)`
Other option:
`
Class[] SerializationRegistrator#getTypeHierarchies()
String[] SerializationRegistrator#getTypeByRegex()
String[] SerializationRegistrator#getTypeByWildcard()
`
Still considering what this could break, I am against including it in 3.3 (at least) and also it needs through testing by 3rd parties to have further feedback on what it breaks.
It looks to me like a Plugins Geddon that could hurt hardly our software.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---