[RFC][icedtea-web] extend reproducers engine for signed applications
Omair Majid
omajid at redhat.com
Tue Sep 20 12:13:20 PDT 2011
On 09/20/2011 10:42 AM, Jiri Vanek wrote:
> Hi!
> This is second attempt to enchant reproducers engine for signed ones. It
> introduce new directory tests/jnlp_tests/signed/. Reproducers in this
> directory should follow simple reproducers rules and are threaten in
> same way as simple ones except fact that are signed at the end of
> preparation process.
> This changes are included in *signedReproducersEngine*.
> There exists also ReadPropertiesSigned demo reproducer for engine. This
> one is included in *signedReproducersExample*.
> Whole concept have one flaw - "accept certificate" dialog rised each
> time signed reproducer was lunched. Thats why I have added -XtrustAll
> option to icedtea-web source. This change is included in *xtrustAll*
> patch. Patch is quite simple and is affecting all methods which popuped
> dialog with certificate. It simply do not show dialog and return
> "accepted" to icedtea-web.
> I was afraid, tahat -XtrustAll could be misused in way described in
> ReadPropertiesBySignedHack (included in *xtrustAllHack*) - that
> xtrustall can be setted on by reflection from signed code (this is
> possible) and then unsigned content can be loaded but it is not
> (luckily) possible, as SecurityMAnager is compalining about different
> signature (which is correct behaviour)
I am not sure if there is much that we can do. After all, trusted code
can download and run arbitrary code on your machine already. We can try
to make it harder for developers to make mistakes, but there is not much
we can do to completely stop them.
> I'm not sure weather to include this patch into icedtea-web.
>
I dont have strong feelings either way, but I am leaning towards leaving
it out.
> All patches are available as one big patch in *reproducersAll* file.
>
More comments inline below.
>
> 2011-09-20 Jiri Vanek <jvanek at redhat.com>
> +++(signedReproducersEngine)+++
> Added signed reproducers engine and examples
> *Makefile.am added variable KEYSTORE_NAME
> (stamps/junit-jnlp-dist-dirs): creates stamp and depend on next two
> targets
> (junit-jnlp-dist-simple.txt): creates list of simple reproducers,
> extracted from ^
> (junit-jnlp-dist-signed.txt): creates list of signed reproducers
> (stamps/netx-dist-tests-prepare-reproducers.stamp): now traverse
> over signed and simple
> (stamps/netx-dist-tests-sign-some-reproducers.stamp): depends on ^,
> traverse through signed reproducers and sign them
> (stamps/netx-dist-tests-compile-testcases.stamp): now traverse over
> signed and simple
> (stamps/bootstrap-directory.stamp): creates symlinks/stubs to
> jarsigner and keytool
> (clean-netx-dist-tests):remove new stamps, signed and simple list
> and keysstore
> *acinclude.m4: declared to proceed IT_FIND_KEYTOOL and
> IT_FIND_JARSIGNER macro
> *configure.ac: declared macros to check for keytool and jarsigner
> *tests/jnlp_tests/README: mentioned signed directory
> +++(xtrustAll)+++
> *netx/net/sourceforge/jnlp/runtime/Boot.java: (main): added logic
> to handle -Xtrustall option
> *netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java: declared
> private static boolean trustAll=false; with public getter and
> pkg.private setter
> *netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:
> (checkTrustWithUser): modified, when XtrustAll declared, then user is
> not asked and certificate is trusted
> *netx/net/sourceforge/jnlp/security/VariableX509TrustManager.java:
> (askUser): --||--
> +++(xtrustAllHack)+++
> *tests/jnlp_tests/signed/ReadPropertiesBySignedHack/resources/ReadPropertiesBySignedHack.jnlp: jnlp file to lunch ReadPropertiesBySignedHack, notice please dependenci on ReadProperties.jar from simple reproducers
> *tests/jnlp_tests/signed/ReadPropertiesBySignedHack/srcs/ReadPropertiesBySignedHack.java - this reproducers verify, that even reflection-by enabled XtrustAll will not allow to lunch unsigned code
> *tests/jnlp_tests/signed/ReadPropertiesBySignedHack/testcases/ReadPropertiesBySignedHackTest.java: testcase for ^
> +++(signedReproducersExample)+++
> *tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned1.jnlp:
> *tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned2.jnlp:
> *tests/jnlp_tests/signed/ReadPropertiesSigned/testcases/ReadPropertiesSignedTest.java:
> *tests/jnlp_tests/signed/ReadPropertiesSigned/srcs/ReadPropertiesSigned.java: ^,^^, ^^^ example of signed reproducer
> *tests/jnlp_tests/simple/ReadProperties/srcs/ReadProperties.java:
> now prints out got variable for comparsion with ^
>
>
> +++(.*)+++ will not be included in changelog if patch applied as one.
>
Do you think the patches should be applied together?
> + public ReadPropertiesSignedTest() {
> + List<String> ll=new ArrayList<String>(1);
> + ll.add("-Xtrustall");
> + l=Collections.unmodifiableList(ll);
> + }
How about:
Collections.unmodifiableList(Arrays.asList(new String[] {"-Xtrustall"}));
> @@ -635,6 +665,20 @@
> ln -sf $(JAR) $(BOOT_DIR)/bin/jar
> ln -sf $(abs_top_builddir)/javac $(BOOT_DIR)/bin/javac
> ln -sf $(JAVADOC) $(BOOT_DIR)/bin/javadoc
> + if [ -e "$(KEYTOOL)" ] ; then \
> + ln -sf $(KEYTOOL) $(BOOT_DIR)/bin/keytool ;\
> + else \
> + echo "#! /bin/sh"> $(BOOT_DIR)/bin/keytool ;\
> + echo "echo \"keytool not exist on your system\"">> $(BOOT_DIR)/bin/keytool ;\
> + chmod 777 $(BOOT_DIR)/bin/keytool ;\
> + fi
> + if [ -e "$(JARSIGNER)" ] ; then \
> + ln -sf $(JARSIGNER) $(BOOT_DIR)/bin/jarsigner ;\
> + else \
> + echo "#! /bin/sh"> $(BOOT_DIR)/bin/jarsigner ;\
> + echo "echo \"jarsigner not exist on your system\"">> $(BOOT_DIR)/bin/jarsigner ;\
> + chmod 777 $(BOOT_DIR)/bin/jarsigner ;\
> + fi
:)
> mkdir -p $(BOOT_DIR)/jre/lib&& \
> ln -s $(SYSTEM_JRE_DIR)/lib/rt.jar $(BOOT_DIR)/jre/lib&& \
> if [ -e $(SYSTEM_JRE_DIR)/lib/jsse.jar ] ; then \
> diff -r e9a9792ee189 acinclude.m4
> --- a/acinclude.m4 Thu Sep 15 15:27:40 2011 +0200
> +++ b/acinclude.m4 Tue Sep 20 15:35:19 2011 +0200
> @@ -655,6 +655,56 @@
> AC_SUBST(JAVA)
> ])
>
> +AC_DEFUN_ONCE([IT_FIND_KEYTOOL],
> +[
> + AC_REQUIRE([IT_CHECK_FOR_JDK])
> + AC_MSG_CHECKING([for keytool])
> + AC_ARG_WITH([keytool],
> + [AS_HELP_STRING(--with-keytool,specify location of keytool for signed part of run-netx-dist)],
> + [
> + KEYTOOL="${withval}"
AFAIK, this will break if you run "./configure --with-keytool", because
${withval} is "yes" in that case.
> + ],
> + [
> + KEYTOOL=${SYSTEM_JDK_DIR}/bin/keytool
> + ])
> + if ! test -f "${KEYTOOL}"; then
> + AC_PATH_PROG(KEYTOOL, keytool)
> + fi
> + if ! test -f "${KEYTOOL}"; then
> + KEYTOOL=""
> + fi
> + if test -z "${KEYTOOL}" ; then
> + AC_MSG_WARN("keytool not found so signed part of run-netx-dist will fail")
> + fi
> + AC_MSG_RESULT(${KEYTOOL})
> + AC_SUBST(KEYTOOL)
> +])
> +
> +AC_DEFUN_ONCE([IT_FIND_JARSIGNER],
> +[
> + AC_REQUIRE([IT_CHECK_FOR_JDK])
> + AC_MSG_CHECKING([for jarsigner])
> + AC_ARG_WITH([jarsigner],
> + [AS_HELP_STRING(--with-jarsigner,specify location of jarsigner for signed part od run-netx-dist)],
> + [
> + JARSIGNER="${withval}"
Likewise.
> diff -r e9a9792ee189 tests/jnlp_tests/README
> --- a/tests/jnlp_tests/README Thu Sep 15 15:27:40 2011 +0200
> +++ b/tests/jnlp_tests/README Tue Sep 20 12:07:29 2011 +0200
> @@ -1,2 +1,3 @@
> Each file in directory simple must follows naming convention and is compiled/jared automatically into server's working directory and content of resources likewise. The name of jnlp is independent, and there can be even more jnlps for each future jar. Directories should be honored in srcs and in resources, but noty in testcases.
> +Directory signed is handling its content in similar way as simle's content is handled, but in addition final jars are signed with simple testkey.
> Files in advanced directory have to care about themselves, but even those can have some parts inside simple directory, so some parts of them are processed automatically. There are three reproducers – simpletest1, simpletest2 and deadlocktest, which tests test’s suite itself and serve as examples of behaviour.
>
Any chance this could be line-wrapped?
Cheers,
Omair
More information about the distro-pkg-dev
mailing list