Skip to content

Commit d42d729

Browse files
authored
Merge pull request #985 from AdoptOpenJDK/missing_root_cert_check
Handle missing root ca in signing cert chain
2 parents 6642242 + 2f20956 commit d42d729

2 files changed

Lines changed: 86 additions & 30 deletions

File tree

core/src/main/java/net/adoptopenjdk/icedteaweb/client/parts/dialogs/security/CertsInfoPane.java

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@
6969
import java.lang.reflect.Method;
7070
import java.security.MessageDigest;
7171
import java.security.cert.CertPath;
72+
import java.security.cert.Certificate;
7273
import java.security.cert.X509Certificate;
7374
import java.util.ArrayList;
75+
import java.util.List;
7476

7577
import static net.adoptopenjdk.icedteaweb.i18n.Translator.R;
7678

@@ -108,37 +110,44 @@ public CertsInfoPane(SecurityDialog x, CertVerifier certVerifier) {
108110
* Builds the JTree out of CertPaths.
109111
*/
110112
void buildTree() {
113+
LOG.debug("CertsInfoPanel Build Tree");
114+
111115
certPath = parent.getCertVerifier().getCertPath();
112-
X509Certificate firstCert =
113-
((X509Certificate) certPath.getCertificates().get(0));
114-
String subjectString =
115-
SecurityUtil.getCN(firstCert.getSubjectX500Principal().getName());
116-
String issuerString =
117-
SecurityUtil.getCN(firstCert.getIssuerX500Principal().getName());
118-
119-
DefaultMutableTreeNode top =
120-
new DefaultMutableTreeNode(subjectString
121-
+ " (" + issuerString + ")");
122-
123-
//not self signed
124-
if (!firstCert.getSubjectDN().equals(firstCert.getIssuerDN())
125-
&& (certPath.getCertificates().size() > 1)) {
126-
X509Certificate secondCert =
127-
((X509Certificate) certPath.getCertificates().get(1));
128-
subjectString =
129-
SecurityUtil.getCN(secondCert.getSubjectX500Principal().getName());
130-
issuerString =
131-
SecurityUtil.getCN(secondCert.getIssuerX500Principal().getName());
132-
top.add(new DefaultMutableTreeNode(subjectString
133-
+ " (" + issuerString + ")"));
116+
List<? extends Certificate> certificates = certPath.getCertificates();
117+
118+
if (certificates.isEmpty()) {
119+
LOG.warn("No certificates found in the certificate path.");
120+
return;
121+
}
122+
123+
DefaultMutableTreeNode top = createNode((X509Certificate) certificates.get(0));
124+
LOG.debug("First cert: " + top.toString() + " | Num Certs: " + certificates.size());
125+
126+
DefaultMutableTreeNode currentParent = top;
127+
for (int i = 1; i < certificates.size(); i++) {
128+
X509Certificate cert = (X509Certificate) certificates.get(i);
129+
X509Certificate prevCert = (X509Certificate) certificates.get(i - 1);
130+
131+
132+
DefaultMutableTreeNode child = createNode(cert);
133+
currentParent.add(child);
134+
currentParent = child;
135+
136+
LOG.debug("Cert level " + (i + 1) + ": " + child.toString());
137+
if (cert.getSubjectDN().equals(cert.getIssuerDN())) break; // Self-signed
134138
}
135139

136140
tree = new JTree(top);
137-
tree.getSelectionModel().setSelectionMode
138-
(TreeSelectionModel.SINGLE_TREE_SELECTION);
141+
tree.getSelectionModel().setSelectionMode(TreeSelectionModel.SINGLE_TREE_SELECTION);
139142
tree.addTreeSelectionListener(new TreeSelectionHandler());
140143
}
141144

145+
private DefaultMutableTreeNode createNode(X509Certificate cert) {
146+
String subject = SecurityUtil.getCN(cert.getSubjectX500Principal().getName());
147+
String issuer = SecurityUtil.getCN(cert.getIssuerX500Principal().getName());
148+
return new DefaultMutableTreeNode(subject + " (" + issuer + ")");
149+
}
150+
142151
/**
143152
* Fills in certsNames, certsData with data from the certificates.
144153
*/

core/src/main/java/net/sourceforge/jnlp/tools/JarCertVerifier.java

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,17 @@
4646
import java.io.InputStream;
4747
import java.lang.reflect.Method;
4848
import java.security.CodeSigner;
49+
import java.security.InvalidAlgorithmParameterException;
4950
import java.security.KeyStore;
51+
import java.security.KeyStoreException;
52+
import java.security.NoSuchAlgorithmException;
5053
import java.security.Timestamp;
5154
import java.security.cert.CertPath;
55+
import java.security.cert.CertPathValidator;
56+
import java.security.cert.CertPathValidatorException;
5257
import java.security.cert.Certificate;
58+
import java.security.cert.PKIXParameters;
59+
import java.security.cert.TrustAnchor;
5360
import java.security.cert.X509Certificate;
5461
import java.time.ZoneId;
5562
import java.time.ZonedDateTime;
@@ -58,9 +65,11 @@
5865
import java.util.Date;
5966
import java.util.Enumeration;
6067
import java.util.HashMap;
68+
import java.util.HashSet;
6169
import java.util.List;
6270
import java.util.Map;
6371
import java.util.Optional;
72+
import java.util.Set;
6473
import java.util.jar.JarEntry;
6574
import java.util.regex.Pattern;
6675
import java.util.zip.ZipException;
@@ -399,7 +408,7 @@ VerifyResult verifyJarEntryCerts(final String jarPath, final boolean jarHasManif
399408
}
400409
if (now.isAfter(tsaNotAfter) && now.isAfter(notAfter)) {
401410
certInfo.setHasExpiredCert();
402-
} else if (expiresSoon.isAfter(tsaNotAfter)) {
411+
} else if (expiresSoon.isAfter(tsaNotAfter)) {
403412
certInfo.setHasExpiringCert();
404413
}
405414

@@ -455,6 +464,14 @@ private boolean isTrustedTsa(CertPath certPath) {
455464
}
456465
}
457466
}
467+
for (final KeyStore caKeyStore : caKeyStores) {
468+
try {
469+
validateCertpathWhenRootCANotInChain(certPath, caKeyStore);
470+
return true;
471+
} catch (Exception ignore) {
472+
}
473+
}
474+
LOG.warn("Unable to validate root ca cert in TSA certpath.");
458475
return false;
459476
}
460477

@@ -500,6 +517,15 @@ private void checkTrustedCerts(final CertPath certPath) {
500517
}
501518
}
502519
}
520+
521+
for (final KeyStore caKeyStore : caKeyStores) {
522+
try {
523+
validateCertpathWhenRootCANotInChain(certPath, caKeyStore);
524+
info.setRootInCacerts();
525+
return;
526+
} catch (Exception ignore) {
527+
}
528+
}
503529
} catch (Exception e) {
504530
// TODO: Warn user about not being able to
505531
// look through their cacerts/trusted.certs
@@ -509,9 +535,30 @@ private void checkTrustedCerts(final CertPath certPath) {
509535
}
510536

511537
// Otherwise a parent cert was not found to be trusted.
538+
LOG.warn("Unable to validate root ca cert.");
512539
info.setUntrusted();
513540
}
514541

542+
private static void validateCertpathWhenRootCANotInChain(CertPath certPath, KeyStore caKeyStore) throws KeyStoreException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, CertPathValidatorException {
543+
final Set<TrustAnchor> anchors = new HashSet<>();
544+
545+
for (Enumeration<String> e = caKeyStore.aliases(); e.hasMoreElements(); ) {
546+
final String alias = e.nextElement();
547+
final Certificate c = caKeyStore.getCertificate(alias);
548+
if (c instanceof X509Certificate) {
549+
X509Certificate xc = (X509Certificate) c;
550+
anchors.add(new TrustAnchor(xc, null));
551+
}
552+
}
553+
554+
final PKIXParameters params = new PKIXParameters(anchors);
555+
params.setRevocationEnabled(false); // disable CRL/OCSP for this presence/anchoring check
556+
557+
558+
final CertPathValidator validator = CertPathValidator.getInstance("PKIX");
559+
validator.validate(certPath, params);
560+
}
561+
515562
public void setCurrentlyUsedCertPath(final CertPath certPath) {
516563
currentlyUsed = certPath;
517564
}
@@ -561,11 +608,11 @@ static boolean isMetaInfFile(final String name) {
561608
}
562609
return name.startsWith(META_INF) && (
563610
name.endsWith(".MF") ||
564-
name.endsWith(".SF") ||
565-
name.endsWith(".DSA") ||
566-
name.endsWith(".RSA") ||
567-
name.endsWith(".EC") ||
568-
SIG.matcher(name).matches()
611+
name.endsWith(".SF") ||
612+
name.endsWith(".DSA") ||
613+
name.endsWith(".RSA") ||
614+
name.endsWith(".EC") ||
615+
SIG.matcher(name).matches()
569616
);
570617
}
571618

0 commit comments

Comments
 (0)