Mercurial > hg > release > icedtea-web-1.0
changeset 89:3bd328e4b515
RH672262, CVE-2011-0025: IcedTea jarfile signature verification bypass
Fixes JAR signature handling so that multiply/partially signed jars
are correctly handled.
| author | Deepak Bhole <dbhole@redhat.com> |
|---|---|
| date | Tue, 01 Feb 2011 10:53:44 -0500 |
| parents | e84432bca9a3 |
| children | 0db0d2392ef2 |
| files | ChangeLog NEWS netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java netx/net/sourceforge/jnlp/security/CertVerifier.java netx/net/sourceforge/jnlp/security/CertsInfoPane.java netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java netx/net/sourceforge/jnlp/tools/JarSigner.java |
| diffstat | 7 files changed, 109 insertions(+), 40 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog Fri Jan 28 15:51:03 2011 -0500 +++ b/ChangeLog Tue Feb 01 10:53:44 2011 -0500 @@ -1,3 +1,33 @@ +2011-01-24 Deepak Bhole <dbhole@redhat.com> + + RH672262, CVE-2011-0025: IcedTea jarfile signature verification bypass + * rt/net/sourceforge/jnlp/runtime/JNLPClassLoader.java + (initializeResources): Prompt user only if there is a single certificate + that signs all jars in the jnlp file, otherwise treat as unsigned. + * rt/net/sourceforge/jnlp/security/CertVerifier.java: Rename getCerts to + getCertPath and make it return a CertPath. + * rt/net/sourceforge/jnlp/security/CertsInfoPane.java: Rename certs + variable to certPath and change its type to CertPath. + (buildTree): Use new certPath variable. + (populateTable): Same. + * rt/net/sourceforge/jnlp/security/HttpsCertVerifier.java: Rename getCerts + to getCertPath and make it return a CertPath. + * rt/net/sourceforge/jnlp/tools/JarSigner.java: Change type for certs + variable to be a hashmap that stores certs and the number of entries they + have signed. + (totalSignableEntries): New variable to track how many signable entries + have been encountered. + (getCerts): Updated method to return certs from new hashmap. + (isFullySignedByASingleCert): New method. Returns if there is a single + cert that signs all the entries in the jars specified in the jnlp file. + (verifyJars): Move verifiedJars and unverifiedJars out of the for loop so + that the data is not lost when the next jar is processed. After verifying + each jar, see if there is a single signer, and prompt the user if there is + such an untrusted signer. + (verifyJar): Increment totalSignableEntries for each signable entry + encountered and the count for each cert when it signs an entry. Move + checkTrustedCerts() out of the function into verifyJars(). + 2011-01-28 Deepak Bhole <dbhole@redhat.com> * Makefile.am
--- a/NEWS Fri Jan 28 15:51:03 2011 -0500 +++ b/NEWS Tue Feb 01 10:53:44 2011 -0500 @@ -14,6 +14,7 @@ * Security updates - RH645843, CVE-2010-3860: IcedTea System property information leak via public static - RH663680, CVE-2010-4351: IcedTea JNLP SecurityManager bypass + - RH672262, CVE-2011-0025: IcedTea jarfile signature verification bypass * New Features - IcedTea-Web now uses a deployment.properties file to specify configuration - System-level as well as user-level deployment.properties files with locked configuration are supported
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Fri Jan 28 15:51:03 2011 -0500 +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Feb 01 10:53:44 2011 -0500 @@ -430,7 +430,7 @@ } //Case when at least one jar has some signing - if (js.anyJarsSigned()) { + if (js.anyJarsSigned() && js.isFullySignedByASingleCert()) { signing = true; if (!js.allJarsSigned() &&
--- a/netx/net/sourceforge/jnlp/security/CertVerifier.java Fri Jan 28 15:51:03 2011 -0500 +++ b/netx/net/sourceforge/jnlp/security/CertVerifier.java Tue Feb 01 10:53:44 2011 -0500 @@ -76,7 +76,7 @@ * Return a valid certificate path to this certificate(s) being verified * @return The CertPath */ - public ArrayList<CertPath> getCerts(); + public CertPath getCertPath(); /** * Returns the application's publisher's certificate. @@ -89,4 +89,5 @@ * the event that the application is self signed. */ public abstract Certificate getRoot(); + }
--- a/netx/net/sourceforge/jnlp/security/CertsInfoPane.java Fri Jan 28 15:51:03 2011 -0500 +++ b/netx/net/sourceforge/jnlp/security/CertsInfoPane.java Tue Feb 01 10:53:44 2011 -0500 @@ -67,7 +67,7 @@ */ public class CertsInfoPane extends SecurityDialogPanel { - private ArrayList<CertPath> certs; + private CertPath certPath; private JList list; protected JTree tree; private JTable table; @@ -87,12 +87,9 @@ * Builds the JTree out of CertPaths. */ void buildTree() { - certs = parent.getJarSigner().getCerts(); - //for now, we're only going to display the first signer, even though - //jars can be signed by multiple people. - CertPath firstPath = certs.get(0); + certPath = parent.getJarSigner().getCertPath(); X509Certificate firstCert = - ((X509Certificate) firstPath.getCertificates().get(0)); + ((X509Certificate) certPath.getCertificates().get(0)); String subjectString = SecurityUtil.getCN(firstCert.getSubjectX500Principal().getName()); String issuerString = @@ -104,9 +101,9 @@ //not self signed if (!firstCert.getSubjectDN().equals(firstCert.getIssuerDN()) - && (firstPath.getCertificates().size() > 1)) { + && (certPath.getCertificates().size() > 1)) { X509Certificate secondCert = - ((X509Certificate) firstPath.getCertificates().get(1)); + ((X509Certificate) certPath.getCertificates().get(1)); subjectString = SecurityUtil.getCN(secondCert.getSubjectX500Principal().getName()); issuerString = @@ -125,12 +122,12 @@ * Fills in certsNames, certsData with data from the certificates. */ protected void populateTable() { - certNames = new String[certs.get(0).getCertificates().size()]; + certNames = new String[certPath.getCertificates().size()]; certsData = new ArrayList<String[][]>(); - for (int i = 0; i < certs.get(0).getCertificates().size(); i++) { + for (int i = 0; i < certPath.getCertificates().size(); i++) { - X509Certificate c = (X509Certificate) certs.get(0).getCertificates().get(i); + X509Certificate c = (X509Certificate) certPath.getCertificates().get(i); certsData.add(parseCert(c)); certNames[i] = SecurityUtil.getCN(c.getSubjectX500Principal().getName()) + " (" + SecurityUtil.getCN(c.getIssuerX500Principal().getName()) + ")";
--- a/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java Fri Jan 28 15:51:03 2011 -0500 +++ b/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java Tue Feb 01 10:53:44 2011 -0500 @@ -83,7 +83,7 @@ return isTrusted; } - public ArrayList<CertPath> getCerts() { + public CertPath getCertPath() { ArrayList<X509Certificate> list = new ArrayList<X509Certificate>(); for (int i = 0; i < chain.length; i++) @@ -99,7 +99,7 @@ // carry on } - return certPaths; + return certPaths.get(0); } public ArrayList<String> getDetails() {
--- a/netx/net/sourceforge/jnlp/tools/JarSigner.java Fri Jan 28 15:51:03 2011 -0500 +++ b/netx/net/sourceforge/jnlp/tools/JarSigner.java Tue Feb 01 10:53:44 2011 -0500 @@ -138,11 +138,13 @@ private ArrayList<String> unverifiedJars = null; /** the certificates used for jar verification */ - private ArrayList<CertPath> certs = null; + private HashMap<CertPath, Integer> certs = new HashMap<CertPath, Integer>(); /** details of this signing */ private ArrayList<String> details = new ArrayList<String>(); + private int totalSignableEntries = 0; + /* (non-Javadoc) * @see net.sourceforge.jnlp.tools.CertVerifier2#getAlreadyTrustPublisher() */ @@ -191,18 +193,41 @@ * @see net.sourceforge.jnlp.tools.CertVerifier2#getCerts() */ public ArrayList<CertPath> getCerts() { - return certs; + return new ArrayList<CertPath>(certs.keySet()); + } + + /** + * Returns whether or not all entries have a common signer. + * + * It is possible to create jars where only some entries are signed. In + * such cases, we should not prompt the user to accept anything, as the whole + * application must be treated as unsigned. This method should be called by a + * caller before it is about to ask the user to accept a cert and determine + * whether the application is trusted or not. + * + * @return Whether or not all entries have a common signer + */ + public boolean isFullySignedByASingleCert() { + + for (CertPath cPath : certs.keySet()) { + // If this cert has signed everything, return true + if (certs.get(cPath) == totalSignableEntries) + return true; + } + + // No cert found that signed all entries. Return false. + return false; } public void verifyJars(List<JARDesc> jars, ResourceTracker tracker) throws Exception { - certs = new ArrayList<CertPath>(); + verifiedJars = new ArrayList<String>(); + unverifiedJars = new ArrayList<String>(); + for (int i = 0; i < jars.size(); i++) { JARDesc jar = jars.get(i); - verifiedJars = new ArrayList<String>(); - unverifiedJars = new ArrayList<String>(); try { @@ -231,6 +256,22 @@ throw e; } } + + //we really only want the first certPath + for (CertPath cPath : certs.keySet()) { + + if (certs.get(cPath) != totalSignableEntries) + continue; + else + certPath = cPath; + + // check if the certs added above are in the trusted path + checkTrustedCerts(); + + if (alreadyTrustPublisher || rootInCacerts) + break; + } + } public verifyResult verifyJar(String jarName) throws Exception { @@ -238,10 +279,6 @@ boolean hasUnsignedEntry = false; JarFile jarFile = null; - // certs could be uninitialized if one calls this method directly - if (certs == null) - certs = new ArrayList<CertPath>(); - try { jarFile = new JarFile(jarName, true); Vector<JarEntry> entriesVec = new Vector<JarEntry>(); @@ -280,21 +317,22 @@ CodeSigner[] signers = je.getCodeSigners(); boolean isSigned = (signers != null); anySigned |= isSigned; - hasUnsignedEntry |= !je.isDirectory() && !isSigned - && !signatureRelated(name); + + boolean shouldHaveSignature = !je.isDirectory() + && !signatureRelated(name); + + hasUnsignedEntry |= shouldHaveSignature && !isSigned; + + if (shouldHaveSignature) + totalSignableEntries++; + if (isSigned) { - // TODO: Perhaps we should check here that - // signers.length is only of size 1, and throw an - // exception if it's not? for (int i = 0; i < signers.length; i++) { CertPath certPath = signers[i].getSignerCertPath(); - if (!certs.contains(certPath)) - certs.add(certPath); - - //we really only want the first certPath - if (!certPath.equals(this.certPath)) { - this.certPath = certPath; - } + if (!certs.containsKey(certPath)) + certs.put(certPath, 1); + else + certs.put(certPath, certs.get(certPath) + 1); Certificate cert = signers[i].getSignerCertPath() .getCertificates().get(0); @@ -314,7 +352,12 @@ } } } //while e has more elements - } //if man not null + } else { //if man not null + + // Else increment totalEntries by 1 so that unsigned jars with + // no manifests can't sneak in + totalSignableEntries++; + } //Alert the user if any of the following are true. if (!anySigned) { @@ -355,9 +398,6 @@ } } - // check if the certs added above are in the trusted path - checkTrustedCerts(); - //anySigned does not guarantee that all files were signed. return (anySigned && !(hasUnsignedEntry || hasExpiredCert || badKeyUsage || badExtendedKeyUsage || badNetscapeCertType || notYetValidCert)) ? verifyResult.SIGNED_OK : verifyResult.SIGNED_NOT_OK;