# HG changeset patch # User Andrew John Hughes # Date 1295969955 0 # Node ID 8c1dc32cd05899dfa9c52644d0af31db259c4b75 # Parent 83b92103693a94f4c4ec6cc45c4c0cc2ae93284d RH672262, CVE-2011-0025: IcedTea jarfile signature verification bypass 2011-01-24 Deepak Bhole 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(). diff -r 83b92103693a -r 8c1dc32cd058 ChangeLog --- a/ChangeLog Thu Jan 20 11:11:20 2011 -0500 +++ b/ChangeLog Tue Jan 25 15:39:15 2011 +0000 @@ -1,3 +1,33 @@ +2011-01-24 Deepak Bhole + + 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-20 Deepak Bhole PR619: Improper finalization by the plugin can crash the browser diff -r 83b92103693a -r 8c1dc32cd058 NEWS --- a/NEWS Thu Jan 20 11:11:20 2011 -0500 +++ b/NEWS Tue Jan 25 15:39:15 2011 +0000 @@ -10,6 +10,8 @@ New in release 1.8.5 (2011-XX-XX): +* Security updates + - RH672262, CVE-2011-0025: IcedTea jarfile signature verification bypass * Backports - S6687968: PNGImageReader leaks native memory through an Inflater - S6541476, RH665355: PNG imageio plugin incorrectly handles iTXt chunk diff -r 83b92103693a -r 8c1dc32cd058 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Thu Jan 20 11:11:20 2011 -0500 +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jan 25 15:39:15 2011 +0000 @@ -398,7 +398,7 @@ } //Case when at least one jar has some signing - if (js.anyJarsSigned()){ + if (js.anyJarsSigned() && js.isFullySignedByASingleCert()){ signing = true; if (!js.allJarsSigned() && diff -r 83b92103693a -r 8c1dc32cd058 netx/net/sourceforge/jnlp/security/CertVerifier.java --- a/netx/net/sourceforge/jnlp/security/CertVerifier.java Thu Jan 20 11:11:20 2011 -0500 +++ b/netx/net/sourceforge/jnlp/security/CertVerifier.java Tue Jan 25 15:39:15 2011 +0000 @@ -76,7 +76,7 @@ * Return a valid certificate path to this certificate(s) being verified * @return The CertPath */ - public ArrayList getCerts(); + public CertPath getCertPath(); /** * Returns the application's publisher's certificate. diff -r 83b92103693a -r 8c1dc32cd058 netx/net/sourceforge/jnlp/security/CertsInfoPane.java --- a/netx/net/sourceforge/jnlp/security/CertsInfoPane.java Thu Jan 20 11:11:20 2011 -0500 +++ b/netx/net/sourceforge/jnlp/security/CertsInfoPane.java Tue Jan 25 15:39:15 2011 +0000 @@ -65,7 +65,7 @@ */ public class CertsInfoPane extends SecurityDialogUI { - private ArrayList certs; + private CertPath certPath; private JList list; protected JTree tree; private JTable table; @@ -84,12 +84,10 @@ * Builds the JTree out of CertPaths. */ void buildTree() { - certs = ((SecurityWarningDialog)optionPane).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 = ((SecurityWarningDialog)optionPane).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 +99,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 = @@ -122,12 +120,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(); - 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 -r 83b92103693a -r 8c1dc32cd058 netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java --- a/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java Thu Jan 20 11:11:20 2011 -0500 +++ b/netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java Tue Jan 25 15:39:15 2011 +0000 @@ -82,7 +82,7 @@ return isTrusted; } - public ArrayList getCerts() { + public CertPath getCertPath() { ArrayList list = new ArrayList(); for (int i=0; i < chain.length; i++) @@ -98,7 +98,7 @@ // carry on } - return certPaths; + return certPaths.get(0); } public ArrayList getDetails() { diff -r 83b92103693a -r 8c1dc32cd058 netx/net/sourceforge/jnlp/tools/JarSigner.java --- a/netx/net/sourceforge/jnlp/tools/JarSigner.java Thu Jan 20 11:11:20 2011 -0500 +++ b/netx/net/sourceforge/jnlp/tools/JarSigner.java Tue Jan 25 15:39:15 2011 +0000 @@ -142,11 +142,13 @@ private ArrayList unverifiedJars = null; /** the certificates used for jar verification */ - private ArrayList certs = null; + private HashMap certs = new HashMap(); /** details of this signing */ private ArrayList details = new ArrayList(); + private int totalSignableEntries = 0; + /* (non-Javadoc) * @see net.sourceforge.jnlp.tools.CertVerifier2#getAlreadyTrustPublisher() */ @@ -195,18 +197,41 @@ * @see net.sourceforge.jnlp.tools.CertVerifier2#getCerts() */ public ArrayList getCerts() { - return certs; + return new ArrayList(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 jars, ResourceTracker tracker) throws Exception { - certs = new ArrayList(); + verifiedJars = new ArrayList(); + unverifiedJars = new ArrayList(); + for (int i = 0; i < jars.size(); i++) { JARDesc jar = (JARDesc) jars.get(i); - verifiedJars = new ArrayList(); - unverifiedJars = new ArrayList(); try { @@ -235,6 +260,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 { @@ -242,10 +283,6 @@ boolean hasUnsignedEntry = false; JarFile jarFile = null; - // certs could be uninitialized if one calls this method directly - if (certs == null) - certs = new ArrayList(); - try { jarFile = new JarFile(jarName, true); Vector entriesVec = new Vector(); @@ -283,21 +320,23 @@ 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); @@ -317,7 +356,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) { @@ -357,9 +401,6 @@ jarFile.close(); } } - - // 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