[rfc][icedtea-web] Reproducer & unit test for unsigned content in META-INF/
Adam Domurad
adomurad at redhat.com
Mon Aug 13 11:30:26 PDT 2012
On Mon, 2012-08-13 at 10:59 +0200, Jiri Vanek wrote:
> On 07/05/2012 05:27 PM, Adam Domurad wrote:
> > Note, bugfix already in HEAD. Reproducer sanity checked by backing out
> > -r429 in HEAD.
> >
> > Reproducer functions by copying and modifying ReadPropertiesSigned.jar
> > to have unsigned content in META-INF/ directory. If it is felt that it
> > should sign and create its own jar let me know.
> >
> > Unit test changelog:
> > 2012-07-05 Adam Domurad<adomurad at redhat.com>
> >
> > Unit test for method in JCV, isMetaInfFile()
> > * netx/net/sourceforge/jnlp/tools/JarCertVerifier.java:
> > Made isMetaInfFile package-private for testing purposes.
> > * tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java:
> > New, tests isMetaInfFile()
>
> Unittest seams ok. As it is modifying also a code a bit, do you mind head + 1.3?
Sure. OK for 1.3/HEAD I'm assuming ?
>
> >
> > Reproducer changelog:
> > 2012-07-05 Adam Domurad<adomurad at redhat.com>
> >
> > Reproducer for allowing unsigned content in META-INF/ folder.
> > Derives from ReadPropertiesSigned test's signed jar.
> > *
> > tests/reproducers/custom/UnsignedContentInMETAINF/resources/UnsignedContentInMETAINF.jnlp:
> > New, runs a modified version of ReadPropertiesSigned.jar
> > (UnsignedContentInMETAINF.jar)
> > *
> > tests/reproducers/custom/UnsignedContentInMETAINF/srcs/META-INF/unsigned_file_in_metainf:
> > New, placed into a modified version of ReadPropertiesSigned.jar
> > (UnsignedContentInMETAINF.jar) so that there is unsigned content in the
> > META-INF/ folder.
> > * tests/reproducers/custom/UnsignedContentInMETAINF/srcs/Makefile:
> > New, creates a modified version of ReadPropertiesSigned.jar, named
> > UnsignedContentInMETAINF.jar, and places unsigned content inside its
> > META-INF/ folder
> > *
> > tests/reproducers/custom/UnsignedContentInMETAINF/testcases/UnsignedContentInMETAINF.java:
> > Test driver for jnlp file
> >
> > Thanks,
> > Adam
> >
>
>
> Thanx for reproducer. Overall looks good, one clarification below however needed.
>
>
> >
> > unsigned-metainf-content-reproducer.patch
> >
>
> ...snip...
>
> > +</jnlp>
> > diff --git a/tests/reproducers/custom/UnsignedContentInMETAINF/srcs/META-INF/unsigned_file_in_metainf b/tests/reproducers/custom/UnsignedContentInMETAINF/srcs/META-INF/unsigned_file_in_metainf
> > new file mode 100644
>
> Purpose of this is to create new file (I have not seen it used in reproducer) or empty directory in
> src (which I have not seen used too)?
>
> If there is need for empty file or (or dir) then touch (mkdir) in custom Makefile perhaps? Although
> I agree that have those files in file tree is little bit clearer, mercurial do not like empty
> directories :(
Added content so the file in the META-INF/ folder is no longer empty. I
have not taking the mkdir + touch approach, as we discussed.
>
> Also - it is in src so you expect to have it compiled (as you are overwriting embedded jar's work)
> or copied? it is not clear from makefile (but I can be dummy enough to miss it)
It is used to manually update the jar
>
>
> > diff --git a/tests/reproducers/custom/UnsignedContentInMETAINF/srcs/Makefile b/tests/reproducers/custom/UnsignedContentInMETAINF/srcs/Makefile
> > new file mode 100644
> > --- /dev/null
> > +++ b/tests/reproducers/custom/UnsignedContentInMETAINF/srcs/Makefile
> > @@ -0,0 +1,17 @@
> > +TESTNAME=UnsignedContentInMETAINF
> > +JAVAC_CLASSPATH=$(JNLP_TESTS_ENGINE_DIR):$(NETX_DIR)/lib/classes.jar
> > +DEPLOY_DIR=$(JNLP_TESTS_SERVER_DEPLOYDIR)
> > +JAVAC=$(BOOT_DIR)/bin/javac
> > +JAR=$(BOOT_DIR)/bin/jar
> > +
> > +prepare-reproducer:
> > + echo PREPARING REPRODUCER $(TESTNAME)
>
> > + cp $(DEPLOY_DIR)/ReadPropertiesSigned.jar $(DEPLOY_DIR)/UnsignedContentInMETAINF.jar
> > + $(JAR) uf $(DEPLOY_DIR)/UnsignedContentInMETAINF.jar META-INF
>
> those two lines above are what is confusing me:
> you are coping ReadPropertiesSigned.jar and overwriting UnsignedContentInMETAINF.jar.
> Then you are updating this copy for META-INF (directory?) which do not exists(?)
> Although if it would exists, then it would be nice to have deirectory where this jar u is happening.
>
> From both above - probably wrongly placed META-INF and strange cp and jar u, I think that
> reproducer is not testing what it should.
>
> Or am I wrong and pure $(JAR) uf $(DEPLOY_DIR)/UnsignedContentInMETAINF.jar META-INF, with
> non-existent META-INF directory will cause timestamps in jar to change and so make signatures of
> metainf invalid? But then you do not need this
> tests/reproducers/custom/UnsignedContentInMETAINF/srcs/META-INF/unsigned_file_in_metainf misleading
> file.
The file is copied into the jar's META-INF/ by this command. I have
added a redundant cd before the jar command as we discussed.
> If this is true, then perhaps more visible approach then just modifying the time-stamp can be used?
>
>
> Rest of the reproducer is nice work, thanks for it!
>
> J.
> > + echo PREPARED REPRODUCER $(TESTNAME)s
> > +
> > +clean-reproducer:
> > + echo CLEANING REPRODUCER $(TESTNAME)
> > + rm -f UnsignedContentInMETAINF.jar
> > + echo CLEANED REPRODUCER $(TESTNAME)
> > +
> > diff --git a/tests/reproducers/custom/UnsignedContentInMETAINF/testcases/UnsignedContentInMETAINF.java b/tests/reproducers/custom/UnsignedContentInMETAINF/testcases/UnsignedContentInMETAINF.java
> > new file mode 100644
> > --- /dev/null
> > +++ b/tests/reproducers/custom/UnsignedContentInMETAINF/testcases/UnsignedContentInMETAINF.java
> > @@ -0,0 +1,65 @@
> > +/* UnsignedContentInMETAINF.java
> > +Copyright (C) 2011 Red Hat, Inc.
> > +
> > +This file is part of IcedTea.
> > +
> > +IcedTea is free software; you can redistribute it and/or
> > +modify it under the terms of the GNU General Public License as published by
> > +the Free Software Foundation, version 2.
> > +
> > +IcedTea is distributed in the hope that it will be useful,
> > +but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > +General Public License for more details.
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with IcedTea; see the file COPYING. If not, write to
> > +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +02110-1301 USA.
> > +
> > +Linking this library statically or dynamically with other modules is
> > +making a combined work based on this library. Thus, the terms and
> > +conditions of the GNU General Public License cover the whole
> > +combination.
> > +
> > +As a special exception, the copyright holders of this library give you
> > +permission to link this library with independent modules to produce an
> > +executable, regardless of the license terms of these independent
> > +modules, and to copy and distribute the resulting executable under
> > +terms of your choice, provided that you also meet, for each linked
> > +independent module, the terms and conditions of the license of that
> > +module. An independent module is a module which is not derived from
> > +or based on this library. If you modify this library, you may extend
> > +this exception to your version of the library, but you are not
> > +obligated to do so. If you do not wish to do so, delete this
> > +exception statement from your version.
> > + */
> > +
> > +
> > +import java.util.Arrays;
> > +import java.util.Collections;
> > +import java.util.List;
> > +import net.sourceforge.jnlp.ServerAccess;
> > +import org.junit.Assert;
> > +import org.junit.Test;
> > +import net.sourceforge.jnlp.ProcessResult;;
> > +
> > +public class UnsignedContentInMETAINF {
> > +
> > + private static ServerAccess server = new ServerAccess();
> > + private final List<String> l=Collections.unmodifiableList(Arrays.asList(new String[] {"-Xtrustall"}));
> > +
> > + String accessMatcher = "(?s).*java.security.AccessControlException.{0,5}access denied.{0,5}java.util.PropertyPermission.{0,5}" + "user.name.{0,5}read" + ".*";
> > +
> > + @Test
> > + public void ReadSignedPropertiesWithUnsignedContentInMETAINF() throws Exception {
> > + //request for allpermissions
> > + ProcessResult pr=server.executeJavawsHeadless(l,"/UnsignedContentInMETAINF.jnlp");
> > + Assert.assertFalse("Stderr should NOT match "+accessMatcher+" but did",pr.stderr.matches(accessMatcher));
> > + String ss="ClassNotFoundException";
> > + Assert.assertFalse("Stderr should not contain "+ss+" but did",pr.stderr.contains(ss));
> > + Assert.assertTrue("Stdout length should be >= 4 but was "+pr.stdout.length(),pr.stdout.length()>=4); // /home/user or /root or anything else :(
> > + Assert.assertFalse("Should not be terminated but was",pr.wasTerminated);
> > + Assert.assertEquals((Integer)0, pr.returnValue);
> > + }
> > + }
> >
> >
> > JCVIsMetaInfFile.patch
> >
> >
> > diff --git a/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java b/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
> > --- a/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
> > +++ b/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
> > @@ -441,7 +441,7 @@ public class JarCertVerifier implements
> > * . META-INF/*.DSA
> > * . META-INF/*.RSA
> > */
> > - static private boolean isMetaInfFile(String name) {
> > + static boolean isMetaInfFile(String name) {
> > String ucName = name.toUpperCase();
> > return ucName.startsWith(META_INF);
> > }
> > diff --git a/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java b/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java
> > new file mode 100644
> > --- /dev/null
> > +++ b/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java
> > @@ -0,0 +1,17 @@
> > +package net.sourceforge.jnlp.tools;
> > +
> > +import static org.junit.Assert.*;
> > +
> > +import org.junit.Test;
> > +
> > +public class JarCertVerifierTest {
> > +
> > + @Test
> > + public void testIsMetaInfFile() {
> > + final String METAINF ="META-INF";
> > + assertFalse(JarCertVerifier.isMetaInfFile("some_dir/" + METAINF + "/filename"));
> > + assertFalse(JarCertVerifier.isMetaInfFile(METAINF + "filename"));
> > + assertTrue(JarCertVerifier.isMetaInfFile(METAINF + "/filename"));
> > + }
> > +
> > +}
> >
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unsigned-metainf-content-reproducer2.patch
Type: text/x-patch
Size: 7568 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120813/76ea80ea/unsigned-metainf-content-reproducer2.patch
More information about the distro-pkg-dev
mailing list