You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@netbeans.apache.org by ma...@apache.org on 2019/12/10 19:45:29 UTC
[netbeans] 02/09: Require NBMs to be fully signed
This is an automated email from the ASF dual-hosted git repository.
matthiasblaesing pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/netbeans.git
commit d09dc3a37f84efcde3d276697db15ac4ffddb596
Author: Matthias Bläsing <mb...@doppel-helix.eu>
AuthorDate: Sat Nov 2 22:27:44 2019 +0100
Require NBMs to be fully signed
After this change every file contained in a NBM is checked and required
to be signed. Partitial/inconsistent signatures will be reported as a
modified package.
---
platform/autoupdate.services/build.xml | 12 ++
.../autoupdate/services/InstallSupportImpl.java | 50 +++++--
.../modules/autoupdate/services/Utilities.java | 155 ++++++++++++++++-----
.../autoupdate/services/VerifyFileTest.java | 37 ++++-
4 files changed, 202 insertions(+), 52 deletions(-)
diff --git a/platform/autoupdate.services/build.xml b/platform/autoupdate.services/build.xml
index 8e0f1ed..971c989 100644
--- a/platform/autoupdate.services/build.xml
+++ b/platform/autoupdate.services/build.xml
@@ -53,6 +53,7 @@
<target name="do-unit-test-build" depends="projectized.do-unit-test-build">
<touch file="${build.dir}/Dummy.class" />
+ <touch file="${build.dir}/Dummy2.class" />
<jar destfile="${build.test.unit.classes.dir}/org/netbeans/api/autoupdate/data/dummy-signed.jar">
<zipfileset prefix="dummy/" file="${build.dir}/Dummy.class"/>
</jar>
@@ -71,6 +72,17 @@
keystore="${test.unit.src.dir}/org/netbeans/api/autoupdate/data/test-keystore.jks"
storepass="password"
alias="test-two" />
+ <jar destfile="${build.test.unit.classes.dir}/org/netbeans/api/autoupdate/data/dummy-partial-signed.jar">
+ <zipfileset prefix="dummy/" file="${build.dir}/Dummy.class"/>
+ </jar>
+ <signjar jar="${build.test.unit.classes.dir}/org/netbeans/api/autoupdate/data/dummy-partial-signed.jar"
+ keystore="${test.unit.src.dir}/org/netbeans/api/autoupdate/data/test-keystore.jks"
+ storepass="password"
+ alias="test-one" />
+ <zip destfile="${build.test.unit.classes.dir}/org/netbeans/api/autoupdate/data/dummy-partial-signed.jar"
+ update="true">
+ <zipfileset prefix="dummy/" file="${build.dir}/Dummy2.class" />
+ </zip>
</target>
</project>
diff --git a/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/InstallSupportImpl.java b/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/InstallSupportImpl.java
index 1644392..43e7f84 100644
--- a/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/InstallSupportImpl.java
+++ b/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/InstallSupportImpl.java
@@ -30,11 +30,11 @@ import java.net.URLConnection;
import java.net.UnknownHostException;
import java.security.KeyStore;
import java.security.KeyStoreException;
+import java.security.cert.CertPath;
import java.security.cert.Certificate;
import java.util.*;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicLong;
-import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.logging.Level;
@@ -66,6 +66,8 @@ import org.openide.util.NbBundle;
import org.openide.util.NbCollections;
import org.xml.sax.SAXException;
+import static org.netbeans.modules.autoupdate.services.Utilities.VERIFICATION_RESULT_COMPARATOR;
+
/**
*
* @author Jiri Rechtacek
@@ -1050,29 +1052,55 @@ public class InstallSupportImpl {
}
private int verifyNbm (UpdateElement el, File nbmFile, ProgressHandle progress, int verified) throws OperationException {
- String res;
+ String res = null;
try {
// get trusted certificates
- List<Certificate> trustedCerts = new ArrayList<Certificate> ();
+ Set<Certificate> trustedCerts = new HashSet<> ();
for (KeyStore ks : Utilities.getKeyStore ()) {
- trustedCerts.addAll (Utilities.getCertificates (ks));
+ trustedCerts.addAll(Utilities.getCertificates(ks));
}
// load user certificates
KeyStore ks = Utilities.loadKeyStore ();
if (ks != null) {
- trustedCerts.addAll (Utilities.getCertificates (ks));
+ trustedCerts.addAll(Utilities.getCertificates(ks));
}
-
+
verified += el.getDownloadSize ();
if (progress != null) {
progress.progress (el.getDisplayName (), verified < wasDownloaded ? verified : wasDownloaded);
}
- Collection<Certificate> nbmCerts = Utilities.getNbmCertificates (nbmFile);
- if (nbmCerts != null && nbmCerts.size () > 0) {
- certs.put (el, nbmCerts);
- }
- res = Utilities.verifyCertificates(nbmCerts, trustedCerts);
+
UpdateElementImpl impl = Trampoline.API.impl(el);
+
+ try {
+ Collection<CertPath> nbmCerts = Utilities.getNbmCertificates(nbmFile);
+ if(nbmCerts == null) {
+ res = Utilities.N_A;
+ } else if (nbmCerts.isEmpty()) {
+ res = Utilities.UNSIGNED;
+ } else {
+ // Iterate all certpaths that can be considered for the NBM
+ // choose the certpath, that has the highest trust level
+ // TRUSTED -> SIGNATURE_VERIFIED -> SIGNATURE_UNVERIFIED
+ // or comes first
+ for(CertPath cp: nbmCerts) {
+ String localRes = Utilities.verifyCertificates(cp, trustedCerts);
+ // If there is no previous result or if the local
+ // verification yielded a better result than the
+ // previous result, replace it
+ if (res == null
+ || VERIFICATION_RESULT_COMPARATOR.compare(res, localRes) > 0) {
+ res = localRes;
+ certs.put(el, (List<Certificate>) cp.getCertificates());
+ }
+ }
+ }
+ } catch (SecurityException ex) {
+ LOG.log(Level.INFO, "The content of the jar/nbm has been modified or certificate paths were inconsistent - " + ex.getMessage(), ex);
+ res = Utilities.MODIFIED;
+ modified.add(impl);
+ }
+
if (res != null) {
switch (res) {
case Utilities.MODIFIED:
diff --git a/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/Utilities.java b/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/Utilities.java
index f5d886b..fcd3139 100644
--- a/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/Utilities.java
+++ b/platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/Utilities.java
@@ -22,6 +22,7 @@ package org.netbeans.modules.autoupdate.services;
import java.io.*;
import java.lang.ref.Reference;
import java.lang.ref.WeakReference;
+import java.security.CodeSigner;
import java.security.InvalidAlgorithmParameterException;
import java.security.KeyStore;
import java.security.KeyStoreException;
@@ -46,6 +47,7 @@ import java.util.jar.JarFile;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.prefs.Preferences;
+import java.util.regex.Pattern;
import org.netbeans.Module;
import org.netbeans.ModuleManager;
import org.netbeans.api.autoupdate.UpdateElement;
@@ -104,10 +106,9 @@ public class Utilities {
private static final String KS_USER_PASSWORD = "open4user";
private static Lookup.Result<KeyStoreProvider> result;
private static final Logger err = Logger.getLogger(Utilities.class.getName ());
-
-
+
public static Collection<KeyStore> getKeyStore () {
- if (result == null) {
+ if (result == null) {
result = Lookup.getDefault ().lookupResult (KeyStoreProvider.class);
result.addLookupListener (new KeyStoreProviderListener ());
}
@@ -115,27 +116,32 @@ public class Utilities {
if (c == null || c.isEmpty ()) {
return Collections.emptyList ();
}
- List<KeyStore> kss = new ArrayList<>();
-
+
+ List<KeyStore> kss = new ArrayList<>();
for (KeyStoreProvider provider : c) {
KeyStore ks = provider.getKeyStore ();
if (ks != null) {
kss.add (ks);
}
}
-
+
return kss;
}
-
- public static String verifyCertificates(Collection<Certificate> archiveCertificates, Collection<Certificate> trustedCertificates) {
- if (archiveCertificates == null) {
- return N_A;
- }
+
+ /**
+ *
+ * @param archiveCertPath
+ * @param trustedCertificates
+ * @return
+ */
+ public static String verifyCertificates(CertPath archiveCertPath, Collection<? extends Certificate> trustedCertificates) {
+ assert archiveCertPath != null;
+ List<? extends Certificate> archiveCertificates = archiveCertPath.getCertificates();
if (!archiveCertificates.isEmpty()) {
Collection<Certificate> c = new HashSet<>(trustedCertificates);
c.retainAll(archiveCertificates);
- if (c.isEmpty()) {
- Map<Principal, X509Certificate> certSubjectsMap = new HashMap<>();
+ if (c.isEmpty()) {
+ Map<Principal, X509Certificate> certSubjectsMap = new HashMap<>();
Set<Principal> certIssuersSet = new HashSet<>();
for (Certificate cert : archiveCertificates) {
if (cert != null) {
@@ -146,17 +152,15 @@ public class Utilities {
}
}
}
-
Map<X509Certificate, X509Certificate> candidates = new HashMap<>();
-
for (Principal p : certSubjectsMap.keySet()) {
// cert chain may not be ordered - trust anchor could before certificate itself
if (certIssuersSet.contains(p)) {
continue;
}
-
+
X509Certificate cert = certSubjectsMap.get(p);
-
+
Principal tap = cert.getIssuerDN();
if (tap != null) {
X509Certificate tempTrustAnchor = certSubjectsMap.get(tap);
@@ -164,12 +168,12 @@ public class Utilities {
candidates.put(cert, tempTrustAnchor);
}
}
- }
+ }
// TRUSTED = 2
// SIGNATURE_VERIFIED = 1
// SIGNATURE_UNVERIFIED = 0
- int res = 0;
+ int res = 0;
for (X509Certificate cert : candidates.keySet()) {
X509Certificate trustCert = candidates.get(cert);
PKIXCertPathValidatorResult validResult = null;
@@ -196,9 +200,9 @@ public class Utilities {
err.log(Level.INFO, "Cannot validate certificate path - " + ex.getMessage(), ex);
//SIGNATURE_UNVERIFIED - result = 0;
} catch (SecurityException ex) {
- // When jar/nbm correctly signed, but content modified
+ // When jar/nbm correctly signed, but content modified
err.log(Level.INFO, "The content of the jar/nbm has been modified - " + ex.getMessage(), ex);
- return MODIFIED;
+ return MODIFIED;
}
if (validResult != null) {
@@ -209,17 +213,17 @@ public class Utilities {
break;
} else {
res = 1;
- }
+ }
}
}
-
+
switch (res) {
case 2:
return TRUSTED;
case 1:
return SIGNATURE_VERIFIED;
default:
- return SIGNATURE_UNVERIFIED;
+ return SIGNATURE_UNVERIFIED;
}
} else {
// signed by trusted certificate stored in user's keystore od ide.ks
@@ -228,7 +232,43 @@ public class Utilities {
}
return UNSIGNED;
}
-
+
+ /**
+ * Establishes an order for verificication result strings. Only the values
+ * <ul>
+ * <li>{@code null}</li>
+ * <li>{@link #UNSIGNED}</li>
+ * <li>{@link #SIGNATURE_UNVERIFIED}</li>
+ * <li>{@link #SIGNATURE_VERIFIED}</li>
+ * <li>{@link #TRUSTED}</li>
+ * </ul>
+ *
+ * @param verificationResult1
+ * @param verificationResult2
+ * @return
+ */
+ public static final Comparator<String> VERIFICATION_RESULT_COMPARATOR = new Comparator<String>() {
+ @Override
+ public int compare(String o1, String o2) {
+ int i1 = mapVerificationResultToInt(o1);
+ int i2 = mapVerificationResultToInt(o2);
+ return i1 - i2;
+ }
+ };
+
+ private static int mapVerificationResultToInt(String input) {
+ if(input == null) {
+ return 0;
+ }
+ switch(input) {
+ case UNSIGNED: return 1;
+ case SIGNATURE_UNVERIFIED: return 2;
+ case SIGNATURE_VERIFIED: return 3;
+ case TRUSTED: return 4;
+ default: throw new IllegalArgumentException("Unmappable value: " + input);
+ }
+ }
+
public static Collection<Certificate> getCertificates (KeyStore keyStore) throws KeyStoreException {
Set<Certificate> certs = new HashSet<Certificate> ();
for (String alias: Collections.list (keyStore.aliases ())) {
@@ -240,33 +280,72 @@ public class Utilities {
}
return certs;
}
-
- public static Collection<Certificate> getNbmCertificates (File nbmFile) throws IOException {
- Set<Certificate> certs = new HashSet<Certificate>();
- JarFile jf = new JarFile(nbmFile);
+
+ /**
+ * Get the certpaths that were used to sign the NBM content.
+ *
+ * @param nbmFile
+ * @return collection of CertPaths, that were used to sign the non-signature
+ * entries of the NBM
+ * @throws IOException
+ * @throws SecurityException if JAR was tampered with or if the certificate
+ * chains are not consistent
+ */
+ public static Collection<CertPath> getNbmCertificates (File nbmFile) throws IOException, SecurityException {
+ Set<CertPath> certs = null;
+
+ // Empty means only the MANIFEST.MF is present - special cased to be in
+ // line with established behaviour
boolean empty = true;
- try {
+
+ // The idea:
+ // - iterate over all JAR entries
+ // - read each entry (as required by the JarFile specificiation for verification)
+ // - extract the certificate paths, that were used to sign the
+ // entry
+ // - compare it with the previously read entries. If they are not
+ // identical, raise a SecurityException.
+ //
+ // Excluded from the above algorithm are:
+ // - directory entries
+ // - files that are part of the signature (each entry in META-INF, that
+ // ends with .SF, .DSA, .RSA or .EC
+ try (JarFile jf = new JarFile(nbmFile)) {
for (JarEntry entry : Collections.list(jf.entries())) {
verifyEntry(jf, entry);
- if (!entry.getName().startsWith("META-INF/")) {
- empty = false;
- if (entry.getCertificates() != null) {
- certs.addAll(Arrays.asList(entry.getCertificates()));
+ if ((! entry.isDirectory()) && (! isSignatureEntry(entry))) {
+ if(! entry.getName().equals("META-INF/MANIFEST.MF")) {
+ empty = false;
+ }
+ Set<CertPath> entryCerts = new HashSet<>();
+ CodeSigner[] codeSigners = entry.getCodeSigners();
+ if (codeSigners != null) {
+ for (CodeSigner cs : entry.getCodeSigners()) {
+ entryCerts.add(cs.getSignerCertPath());
+ }
+ }
+ if(certs == null) {
+ certs = entryCerts;
+ } else if (! certs.equals(entryCerts)) {
+ throw new SecurityException("Inconsistent certificate paths used for signing");
}
}
}
- } finally {
- jf.close();
}
return empty ? null : certs;
}
-
+
+ private static final Pattern SIGNATURE_PATTERN = Pattern.compile("META-INF/([^/]+)\\.(SF|DSA|RSA|EC)");
+ private static boolean isSignatureEntry(JarEntry je) {
+ return SIGNATURE_PATTERN.matcher(je.getName()).matches();
+ }
+
/**
* @throws SecurityException
*/
@SuppressWarnings("empty-statement")
- private static void verifyEntry (JarFile jf, JarEntry je) throws IOException {
+ private static void verifyEntry (JarFile jf, JarEntry je) throws IOException, SecurityException {
InputStream is = null;
try {
is = jf.getInputStream (je);
diff --git a/platform/autoupdate.services/test/unit/src/org/netbeans/modules/autoupdate/services/VerifyFileTest.java b/platform/autoupdate.services/test/unit/src/org/netbeans/modules/autoupdate/services/VerifyFileTest.java
index 9b6da19..b55e48f 100644
--- a/platform/autoupdate.services/test/unit/src/org/netbeans/modules/autoupdate/services/VerifyFileTest.java
+++ b/platform/autoupdate.services/test/unit/src/org/netbeans/modules/autoupdate/services/VerifyFileTest.java
@@ -27,6 +27,7 @@ import java.net.URISyntaxException;
import java.net.URL;
import java.security.KeyStore;
import java.security.KeyStoreException;
+import java.security.cert.CertPath;
import java.security.cert.Certificate;
import java.util.Collection;
import java.util.logging.Level;
@@ -34,6 +35,8 @@ import java.util.logging.Logger;
import org.netbeans.api.autoupdate.TestUtils;
import org.netbeans.junit.NbTestCase;
+import static org.netbeans.modules.autoupdate.services.Utilities.VERIFICATION_RESULT_COMPARATOR;
+
public class VerifyFileTest extends NbTestCase {
private static final Logger LOG = Logger.getLogger(VerifyFileTest.class.getName());
@@ -58,9 +61,33 @@ public class VerifyFileTest extends NbTestCase {
assertNotNull(urlToFile);
File jar = org.openide.util.Utilities.toFile(urlToFile.toURI());
assertTrue(jar.exists());
- Collection<Certificate> nbmCertificates = Utilities.getNbmCertificates(jar);
- Collection<Certificate> trustedCertificates = Utilities.getCertificates(ks);
- return Utilities.verifyCertificates(nbmCertificates, trustedCertificates);
+ String res = null;
+ try {
+ Collection<CertPath> nbmCerts = Utilities.getNbmCertificates(jar);
+ Collection<Certificate> trustedCerts = Utilities.getCertificates(ks);
+ if (nbmCerts == null) {
+ res = Utilities.N_A;
+ } else if (nbmCerts.isEmpty()) {
+ res = Utilities.UNSIGNED;
+ } else {
+ // Iterate all certpaths that can be considered for the NBM
+ // choose the certpath, that has the highest trust level
+ // TRUSTED -> SIGNATURE_VERIFIED -> SIGNATURE_UNVERIFIED -> UNSIGNED
+ // or comes first
+ for (CertPath cp : nbmCerts) {
+ String localRes = Utilities.verifyCertificates(cp, trustedCerts);
+ if (res == null
+ || VERIFICATION_RESULT_COMPARATOR.compare(res, localRes) > 0) {
+ res = localRes;
+ }
+ }
+ }
+ } catch (SecurityException ex) {
+ LOG.log(Level.INFO, "The content of the jar/nbm has been modified or certificate paths were inconsistent - " + ex.getMessage(), ex);
+ res = Utilities.MODIFIED;
+ }
+
+ return res;
}
@SuppressWarnings("unchecked")
@@ -86,6 +113,10 @@ public class VerifyFileTest extends NbTestCase {
assertEquals(Utilities.TRUSTED, doVerification("data/dummy-signed-twice.jar"));
}
+ public void testUnsignedPartiallySigned() throws MalformedURLException, URISyntaxException, IOException, KeyStoreException {
+ assertEquals(Utilities.MODIFIED, doVerification("data/dummy-partial-signed.jar"));
+ }
+
private static KeyStore getKeyStore(File file, String password) {
if (file == null) {
return null;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@netbeans.apache.org
For additional commands, e-mail: commits-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists