aboutsummaryrefslogtreecommitdiffstats
path: root/netx/net
diff options
context:
space:
mode:
authorDeepak Bhole <[email protected]>2011-02-01 10:53:44 -0500
committerDeepak Bhole <[email protected]>2011-02-01 10:53:44 -0500
commit1a96cc8537ee8a6e9aff7465568ba76b949b1535 (patch)
tree24c7eea3467d44d5c722509164318270b466ff83 /netx/net
parentf64c8bd3c5ad5b3e12c2f767008944df7a79eea0 (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')
-rw-r--r--netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java2
-rw-r--r--netx/net/sourceforge/jnlp/security/CertVerifier.java3
-rw-r--r--netx/net/sourceforge/jnlp/security/CertsInfoPane.java19
-rw-r--r--netx/net/sourceforge/jnlp/security/HttpsCertVerifier.java4
-rw-r--r--netx/net/sourceforge/jnlp/tools/JarSigner.java90
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;