diff options
author | Deepak Bhole <[email protected]> | 2011-02-01 10:53:44 -0500 |
---|---|---|
committer | Deepak Bhole <[email protected]> | 2011-02-01 10:53:44 -0500 |
commit | 1a96cc8537ee8a6e9aff7465568ba76b949b1535 (patch) | |
tree | 24c7eea3467d44d5c722509164318270b466ff83 /netx/net | |
parent | f64c8bd3c5ad5b3e12c2f767008944df7a79eea0 (diff) |
RH672262, CVE-2011-0025: IcedTea jarfile signature verification bypass
Fixes JAR signature handling so that multiply/partially signed jars
are correctly handled.
Diffstat (limited to 'netx/net')
5 files changed, 78 insertions, 40 deletions
diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java index ebea041..acadde0 100644 --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java @@ -430,7 +430,7 @@ public class JNLPClassLoader extends URLClassLoader { } //Case when at least one jar has some signing - if (js.anyJarsSigned()) { + if (js.anyJarsSigned() && js.isFullySignedByASingleCert()) { signing = true; if (!js.allJarsSigned() && diff --git a/netx/net/sourceforge/jnlp/security/CertVerifier.java b/netx/net/sourceforge/jnlp/security/CertVerifier.java index cef69c3..842a865 100644 --- a/netx/net/sourceforge/jnlp/security/CertVerifier.java +++ b/netx/net/sourceforge/jnlp/security/CertVerifier.java @@ -76,7 +76,7 @@ public interface CertVerifier { * 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 @@ public interface CertVerifier { * the event that the application is self signed. */ public abstract Certificate getRoot(); + } diff --git a/netx/net/sourceforge/jnlp/security/CertsInfoPane.java b/netx/net/sourceforge/jnlp/security/CertsInfoPane.java index 4571b4e..ebf8b3f 100644 --- a/netx/net/sourceforge/jnlp/security/CertsInfoPane.java +++ b/netx/net/sourceforge/jnlp/security/CertsInfoPane.java @@ -64,7 +64,7 @@ import javax.swing.tree.TreeSelectionModel; */ public class CertsInfoPane extends SecurityDialogPanel { - private ArrayList<CertPath> certs; + private CertPath certPath; private JList list; protected JTree tree; private JTable table; @@ -84,12 +84,9 @@ public class CertsInfoPane extends SecurityDialogPanel { * 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 = @@ -101,9 +98,9 @@ public class CertsInfoPane extends SecurityDialogPanel { //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 = @@ -122,12 +119,12 @@ public class CertsInfoPane extends SecurityDialogPanel { * 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()) + ")"; diff --git a/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java b/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java index 3593291..a7787d2 100644 --- a/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java +++ b/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java @@ -83,7 +83,7 @@ public class HttpsCertVerifier implements CertVerifier { 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 @@ public class HttpsCertVerifier implements CertVerifier { // carry on } - return certPaths; + return certPaths.get(0); } public ArrayList<String> getDetails() { diff --git a/netx/net/sourceforge/jnlp/tools/JarSigner.java b/netx/net/sourceforge/jnlp/tools/JarSigner.java index 06b962f..4e246f7 100644 --- a/netx/net/sourceforge/jnlp/tools/JarSigner.java +++ b/netx/net/sourceforge/jnlp/tools/JarSigner.java @@ -96,11 +96,13 @@ public class JarSigner implements CertVerifier { 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() */ @@ -149,18 +151,41 @@ public class JarSigner implements CertVerifier { * @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 { @@ -189,6 +214,22 @@ public class JarSigner implements CertVerifier { 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 { @@ -196,10 +237,6 @@ public class JarSigner implements CertVerifier { 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>(); @@ -238,21 +275,22 @@ public class JarSigner implements CertVerifier { 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); @@ -272,7 +310,12 @@ public class JarSigner implements CertVerifier { } } } //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) { @@ -313,9 +356,6 @@ public class JarSigner implements CertVerifier { } } - // 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; |