[rfc][icedtea-web] DeploymentPropertiesAreExposed reproducer fix

[Jiri Vanek]

ok, discussion have been taken :))

now whats about original patch?



Changelog:
* netx/net/sourceforge/jnlp/config/Defaults.java: (USER_CONFIG_HOME) made public
* 
tests/reproducers/signed/DeploymentPropertiesAreExposed/testcases/DeploymentPropertiesAreExposedTest.java: 
fixed log dir location


Another casualty of the XDG spec change ;)

Thanks,

-- 
Andrew A


DeploymentPropertiesAreExposed.patch

diff --git a/netx/net/sourceforge/jnlp/config/Defaults.java 
b/netx/net/sourceforge/jnlp/config/Defaults.java
--- a/netx/net/sourceforge/jnlp/config/Defaults.java
+++ b/netx/net/sourceforge/jnlp/config/Defaults.java
@@ -54,7 +54,7 @@

      final static String SYSTEM_HOME = System.getProperty("java.home");
      final static String SYSTEM_SECURITY = SYSTEM_HOME + File.separator + "lib" + File.separator + 
"security";
-    final static String USER_CONFIG_HOME;
+    public final static String USER_CONFIG_HOME;
      public final static String USER_CACHE_HOME;
      final static String USER_SECURITY;
      final static String LOCKS_DIR = System.getProperty("java.io.tmpdir") + File.separator
diff --git 
a/tests/reproducers/signed/DeploymentPropertiesAreExposed/testcases/DeploymentPropertiesAreExposedTest.java 
b/tests/reproducers/signed/DeploymentPropertiesAreExposed/testcases/DeploymentPropertiesAreExposedTest.java
--- 
a/tests/reproducers/signed/DeploymentPropertiesAreExposed/testcases/DeploymentPropertiesAreExposedTest.java
+++ 
b/tests/reproducers/signed/DeploymentPropertiesAreExposed/testcases/DeploymentPropertiesAreExposedTest.java
@@ -42,11 +42,11 @@
  import java.util.Collections;
  import java.util.List;
  import net.sourceforge.jnlp.ServerAccess;
+import net.sourceforge.jnlp.config.Defaults;
  import org.junit.Test;

  public class DeploymentPropertiesAreExposedTest {

-
      @Test
      public void verifyDeploymentConfigrationIsExposedAsSystemProperties() throws Exception {
          ServerAccess server = new ServerAccess();
@@ -55,7 +55,7 @@
          ServerAccess.ProcessResult result = server.executeJavawsHeadless(
              trustCertificates, "/DeploymentPropertiesAreExposed.jnlp");

-        String expectedRegex = ".*" + File.separator + ".icedtea" + File.separator + "log" + ".*";
+        String expectedRegex = Defaults.USER_CONFIG_HOME + File.separator + "log" + ".*";
          String actual = result.stdout.trim();

          boolean stdOutMatches = actual.matches(expectedRegex);



On 09/19/2013 05:04 PM, Omair Majid wrote:
> Hi Jacob,
>
> On 09/19/2013 06:29 AM, Jacob Wisor wrote:
>> Now I am feeling uncomfortable with IcedTea-Web
>> restricting access to the net.sourceforge.jnlp package namespace
>> entirely because this package namespace is not reserved by any public
>> specification. Although this probably does not pose a problem in
>> practice, I am thinking whether IcedTea-Web should not take a more
>> granular approach when restricting access. Should not it only restrict
>> access to actually existing IcedTea-Web classes?
>
> Unfortunately, I don't think that's an option we have right now. The
> restriction mechanism is based on a package prefix. So all
> net.sourceforge.jnlp package (and subpackages) are restricted.
>
> The only project that should be using this namespace at all would be
> netx (which we forked), and that provides a javaws implementation, so I
> am not too worried about something using this package.
>
>> And, since signed jars get AllPermissions by default it is still advised
>> to keep access to fields and methods as limited as possible but as open
>> as necessary.
>
> Yeah, but then the issue goes from a security issue to a bad code issue.
> I am all for restricting access too, but if a code has permissions, it
> can use reflection to do pretty much anything to any object.
>
>> Nevertheless, the fields of concern in
>> net.sourceforge.jnlp.config.Defaults should still be kept to default
>> access modifiers.
>
> I agree 100%. Fields (and methods) should be kept as restricted as
> possible.
>
>> The reproducer can load javaws with the system class
>> loader that has an AllPermissions security context. Because of that it
>> can also load its helper class in net.sourceforge.jnlp.config into
>> javaws' net.sourceforge.jnlp.config run-time package by using
>> JNLPClassLoader providing its AllPermissions security context. Then the
>> reproducer should be able to access the fields in question via its
>> helper class.
>
> Yeah, except if something can has AllPermissions (or can access the
> system classloader) it can do pretty much anything already. In this
> case, you can either use reflection to read the fields directly or even
> just read System.getProperty("user.home").
>
> Thanks,
> Omair
>