[RFC][icedtea-web] Major rework of JarCertVerifier
Danesh Dadachanji
ddadacha at redhat.com
Fri Aug 24 13:04:03 PDT 2012
On 21/08/12 04:26 PM, Danesh Dadachanji wrote:
> Hello,
>
> For the last few months, I've been working on completely redoing JarCertVerifier to allow for the ability to handle multiple signers
> and loosen the restriction ITW has on applets requiring to be completely signed. So naturally, this is a pretty big patch. =)
>
> This patch is heavily *untested*. I'm only posting it to get a head start on review and because my term's going to end on the 24th. :(
> I am trying to write up tests now and make sure that this acts sanely. So if you find horrible mistakes that I've overlooked, please
> let me know!
>
> The very long ChangeLog:
> +2012-08-21 Danesh Dadachanji <ddadacha at redhat.com>
> + Rework JarCertVerifier certificate management to handle multiple
> + certificates and use different algorithms to verify JNLPs and Applets.
> + * netx/net/sourceforge/jnlp/resources/Messages.properties:
> + Removed SHasUnsignedEntry.
> + * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:
> + Set JCV instance to final but uninitialized.
> + (JNLPClassLoader): Initialized JCV with runtime dependent verifier.
> + (addNewJar), (initializeResources), (verifySignedJNLP):
> + Replaced use of local JarCertVerifier variable with the instance variable.
> + Added calls to isFullySigned wherever signer verification is done.
> + (activateJars): No longer verifies nested jars. These receive the same
> + security permissions as their parent jar, regardless of the nested
> + jar's signing.
> + (checkTrustWithUser): Removed JCV param, reimplemented to wrap around
> + JCV's checkTrustWithUser method.
> + (verifyJars): Removed.
> + * netx/net/sourceforge/jnlp/security/AppVerifier.java:
> + New strategy pattern interface that specifies verification methods
> + required regardless of the runtime.
> + * netx/net/sourceforge/jnlp/security/JNLPAppVerifier.java:
> + * netx/net/sourceforge/jnlp/security/PluginAppVerifier.java:
> + New strategy pattern classes used to determine which algorithms to use
> + depending on the runtime.
> + * netx/net/sourceforge/jnlp/security/CertVerifier.java:
> + Added CertPath param to all the methods.
> + (noSigningIssues): Removed.
> + * netx/net/sourceforge/jnlp/security/CertWarningPane.java:
> + * netx/net/sourceforge/jnlp/security/CertsInfoPane.java:
> + * netx/net/sourceforge/jnlp/security/MoreInfoPane.java:
> + Updated calls to the verifier's methods with the new CertPath param. All
> + are set to null so far.
> + * netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java:
> + Added CertPath param to all the methods. It's mostly ignored though.
> + * netx/net/sourceforge/jnlp/tools/CertInformation.java:
> + New class to represent all the information about a signer with
> + with respect to all of the entries it has signed for the app.
> + * netx/net/sourceforge/jnlp/tools/JarCertVerifier.java:
> + Completely reworked to use CertInformation and AppVerifier functionality.
> + (getCertPath), (getCertInformation), (checkTrustWithUser),
> + (getJarSignableEntries), (getTotalJarEntries): New method.
> + (noSigningIssues), (anyJarsSigned): Removed.
> + (verifyResult): Renamed enum to VerifyResult
> + (JarCertVerifier): New constructor used to set AppVerifier instance.
> + (getAlreadyTrustPublisher), (getRootInCacerts): Now uses strategy pattern.
> + (hasSigningIssues), (getDetails), (checkTrustedCerts), (checkCertUsage):
> + Now uses cert info class.
> + (getCerts): Renamed to getCertsList.
> + (isFullySignedByASingleCert): renamed to isFullySigned and to use
> + the strategy pattern.
> + (add): New public method that resets some instance vars and
> + calls verifyJars.
> + (verifyJars): Modifier changed to private, above method should be used.
> + Also skips jars that have been verified before.
> + (verifyJar): Removed actual verification code, only reads jars into the JVM.
> + (verifyJarEntryCerts): New method. Does actual verification of jars.
> + (getPublisher), (getRoot): Use hacky currentlyUsed variable as the signer.
> + * tests/netx/unit/net/sourceforge/jnlp/tools/VerifyJarEntryCertsTest.java:
> + Unit test JCV's verifyJarEntryCerts method.
> + * tests/test-extensions/net/sourceforge/jnlp/tools/CodeSignerCreator.java:
> + Unit test helper that creates CodeSigner instances.
> +
>
> Here's a more informal overview of the changes that I hope will help make reviewing easier. I've listed them in an order that, to me,
> makes it easier to understand all the heavy changes in JCV itself. The ChangeLog is a bit more specific regarding various method
> changes so refer to that too.
>
> ===========================================================================
>
> * Strategy pattern classes *
> - netx/net/sourceforge/jnlp/security/AppVerifier.java
> - netx/net/sourceforge/jnlp/security/JNLPAppVerifier.java
> - netx/net/sourceforge/jnlp/security/PluginAppVerifier.java
>
> These are used to determine certain verification algorithms. For example, figuring out if the publisher is already trusted. JNLPs can
> just be checked for a publisher that is trusted and has signed every entry in the app. Applets on the other hand can have different
> signers across jars so I had to iterate over jars and figure out if they were individually trusted by one signer. These are used to
> make decisions based on runtime.
>
> ===========================================================================
>
> * The information storage class *
> - netx/net/sourceforge/jnlp/tools/CertInformation.java
>
> All signers that have signed even one entry of any jar brought in by the app are given a CertInfo instance. This class then maintains
> all the info needed regarding that signer. Note that it does not actually change the signer's properties, it just keeps track of them!
> Things like the number of entries it has signed across a jar, whether or not it is expired/expiring/not yet valid etc. JCV had a ton of
> instance vars for things like this. I have moved all of those into this class and kept track of them by the signer.
>
> ===========================================================================
>
> * JCV fun *
> - netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
>
> To verify a new list of jars, the order of the JCV method calls are:
> 1. add: The public method called by JNLPClassLoader to tell this instance to verify new jars
> 2. verifyJars: The private method that keeps track of the jars that have been or are being verified
> 3. verifyJar: Pulls in the jar from the FS into the JVM
> 4. verifyJarEntryCerts: The method that does all the verifying of this individual jar. It sets up/updates a lot (but not all) of the
> CertInfo flags for each particular signer.
>
> The fourth method is where a _lot_ of the magic happens. Fortunately, it's the one method I wrote unit tests for earlier on! See below.
> Keep in mind, it first goes through all the entries and sorts them out, storing info in a map local to its scope. It then starts
> updating the instance var info map.
>
> See the ChangeLog for a lot of the other methods. One thing to note is some of the methods that use the CertPath instance var,
> currentlyUsed. See the reasons in the "UI class changes" section below.
>
> ===========================================================================
>
> * Classloader changes *
> - netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>
> This has 3 updates:
> 1. Updated it to handle a global JCV rather than creating instances on its own.
> 2. Made it use the strategy pattern to create the global JCV.
> 3. Updated the checkTrustWithUser method to wrap around JCV's one.
>
> ===========================================================================
>
> * UI class changes *
> - netx/net/sourceforge/jnlp/security/CertWarningPane.java
> - netx/net/sourceforge/jnlp/security/CertsInfoPane.java
> - netx/net/sourceforge/jnlp/security/MoreInfoPane.java
>
> My original idea for dialogs was to have them store their respective CertPath that they were looking at. However, there are so many
> coupled links between their super classes. Replacing this JCV instance var with a CertPath is a task for which I no longer have time.
> :( Instead, I've created a currentlyUsed CertPath instance var in JCV that is set before each dialog is called, telling them which one
> is used. However, I've left the parameter in place for things like getDetails/getRoot/getPublisher and simply passed in null in the
> above few calls. In a future patch, we can then remove currentlyUsed and go straight for the CertPath passed through.
>
> ===========================================================================
>
> * General CertVerifier changes *
> - netx/net/sourceforge/jnlp/security/CertVerifier.java
> - netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java
>
> Added CertPath params to all the methods.
>
> ===========================================================================
>
> * Unit Tests: Checks verifying a _single_ jar *
> - tests/netx/unit/net/sourceforge/jnlp/tools/VerifyJarEntryCertsTest.java
> - tests/test-extensions/net/sourceforge/jnlp/tools/CodeSignerCreator.java
>
> The tests I have so far. Trying to write more!
>
> ===========================================================================
>
> * Other minor changes *
> - netx/net/sourceforge/jnlp/resources/Messages.properties
>
> Removed a symbol that was no longer being used.
>
> ===========================================================================
>
> So again, there are most likely still bugs in this patch. However, if I could get feedback on it, that would be great! Thanks in advance.
Updating this with the latest patch. The attached one has the fixes recommended by Omair in his first reply. I have not had time to
handle his second reply's requests. I've also added fixes to some bugs I noticed with the UI prompt failing. See
AppVerifier#checkTrustWithUser methods for the new logic, a decent bit is changed.
I realize I did not mention what some of these do properly. Here's what I used to test and the result they gave (and should give now).
Hopefully this helps make what checkTrustWithUser does more clear.
===========================================================================
JNLP - Expect result for application-desc, applet-desc and in plugin through jnlp_href:
- a.jar contains Foo.class, signed by signers Alpha, Beta and Charlie.
- Foo.class has some code that requires all permissions.
- This jar is fully signed by 3 signers.
- Run javaws/firefox with jnlp_href
- Prompt should show for the first signer. (Which signer doesn't matter, not too sure about the order in which the JVM finds them)
- Hit cancel, prompted by the second signer (note that it is different).
- Hit cancel, prompted by the third signer (again different).
- Hit:
* run: the app runs fine
* cancel: app will fatally crash.
- Run it again, approve the first cert, app should run immediately (no prompts for other signers).
===========================================================================
Applet in Plugin - no jnlp_href:
- b.jar contains Bar.class, has signer Beta
- a.jar contains Foo.class, has signer Alpha.
- Foo.class is applet class, has init(). It calls Bar.class as well. Both do something requiring all permissions.
- Run firefox, load the webpage that has the applet tag with main='Foo'.
- Prompt should show for a signer.
- Hit cancel, app should not run and plugin should have stopped with fatal exception.
- Rerun, hit approve.
- Another prompt with the second signer should be shown.
- Hit:
* cancel: leads to fatal exception
* run: applet should run normally
===========================================================================
Known Bug:
1. One of the AWT threads hangs and the JVM does not shut down:
Run javaws on your applet/application, if you hit cancel on the first signer and then *very quickly* hit run on the second signer. Your
terminal prompt won't be returned until you send a ^C to it. I looked into it a bit and was a bit baffled. If you sometimes delay
approving the second prompt, it will run fine and exit fine. If you debug it and break after the prompts, it runs fine as well. I think
one of the AWT threads is being blocked by something and it never receives its unblock signal. The launcher thread and everything else
run fine and exit cleanly, it's just the prompt's thread that are left.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jcv-major-rework-02.patch
Type: text/x-patch
Size: 118985 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120824/578b914b/jcv-major-rework-02.patch
More information about the distro-pkg-dev
mailing list