Bug Fix - Resolve exception for system certificates on Mac and Linux (#…

…368) * Only create truststore if it is not empty * Log invalid certificates and proceed with the flow. * Follow symbolic links.
Hakky54 · Aug 4, 2023 · 424a1ab · 424a1ab
1 parent c698bae
commit 424a1ab
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 20 deletions.
diff --git a/sslcontext-kickstart/src/main/java/nl/altindag/ssl/util/CertificateUtils.java b/sslcontext-kickstart/src/main/java/nl/altindag/ssl/util/CertificateUtils.java
@@ -18,6 +18,8 @@
 import nl.altindag.ssl.exception.GenericCertificateException;
 import nl.altindag.ssl.exception.GenericIOException;
 import nl.altindag.ssl.util.internal.IOUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.net.ssl.X509TrustManager;
 import javax.security.auth.x500.X500Principal;
@@ -63,6 +65,8 @@
  */
 public final class CertificateUtils {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(CertificateUtils.class);
+
     private static final String CERTIFICATE_TYPE = "X.509";
     private static final String P7B_HEADER = "-----BEGIN PKCS7-----";
     private static final String P7B_FOOTER = "-----END PKCS7-----";
@@ -242,20 +246,14 @@ private static List<Certificate> parseCertificate(Matcher certificateMatcher) {
         return Collections.unmodifiableList(certificates);
     }
 
-    /**
-     * PKIX/RFC 5280 states that duplicate extensions are not allowed. See section 4.2 of it.
-     * A certificate which contains a duplicate extension is not parseable. Instead of throwing an exception, it will be ignored.
-     */
     public static List<Certificate> parseDerCertificate(InputStream certificateStream) {
         try(BufferedInputStream bufferedCertificateStream = new BufferedInputStream(certificateStream)) {
             return CertificateFactory.getInstance(CERTIFICATE_TYPE)
                     .generateCertificates(bufferedCertificateStream).stream()
                     .collect(toUnmodifiableList());
         } catch (CertificateException | IOException e) {
-            if (e.getMessage().contains("Duplicate extensions not allowed")) {
-                return Collections.emptyList();
-            }
-            throw new GenericCertificateException("There is no valid certificate present to parse. Please make sure to supply a valid der formatted certificate", e);
+            LOGGER.debug("There is no valid certificate present to parse. Please make sure to supply a valid der formatted certificate", e);
+            return Collections.emptyList();
         }
     }
 

diff --git a/sslcontext-kickstart/src/main/java/nl/altindag/ssl/util/KeyStoreUtils.java b/sslcontext-kickstart/src/main/java/nl/altindag/ssl/util/KeyStoreUtils.java
@@ -210,14 +210,18 @@ public static List<KeyStore> loadSystemKeyStores() {
                 createKeyStoreIfAvailable("KeychainStore", null).ifPresent(keyStores::add);
 
                 List<Certificate> systemTrustedCertificates = MacCertificateUtils.getCertificates();
-                KeyStore systemTrustStore = createTrustStore(systemTrustedCertificates);
-                keyStores.add(systemTrustStore);
+                if (!systemTrustedCertificates.isEmpty()) {
+                    KeyStore systemTrustStore = createTrustStore(systemTrustedCertificates);
+                    keyStores.add(systemTrustStore);
+                }
                 break;
             }
             case LINUX: {
                 List<Certificate> certificates = LinuxCertificateUtils.getCertificates();
-                KeyStore linuxTrustStore = createTrustStore(certificates);
-                keyStores.add(linuxTrustStore);
+                if (!certificates.isEmpty()) {
+                    KeyStore linuxTrustStore = createTrustStore(certificates);
+                    keyStores.add(linuxTrustStore);
+                }
                 break;
             }
             case ANDROID: {

diff --git a/sslcontext-kickstart/src/main/java/nl/altindag/ssl/util/LinuxCertificateUtils.java b/sslcontext-kickstart/src/main/java/nl/altindag/ssl/util/LinuxCertificateUtils.java
@@ -18,6 +18,7 @@
 import nl.altindag.ssl.exception.GenericIOException;
 
 import java.io.IOException;
+import java.nio.file.FileVisitOption;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -64,7 +65,7 @@ static List<Certificate> getCertificates() {
                         List<Certificate> certs = loadCertificate(path);
                         certificates.addAll(certs);
                     } else if (Files.isDirectory(path)) {
-                        try(Stream<Path> files = Files.walk(path, 1)) {
+                        try(Stream<Path> files = Files.walk(path, 1, FileVisitOption.FOLLOW_LINKS)) {
                             List<Certificate> certs = files
                                     .filter(Files::isRegularFile)
                                     .flatMap(file -> loadCertificate(file).stream())

diff --git a/sslcontext-kickstart/src/test/java/nl/altindag/ssl/util/CertificateUtilsShould.java b/sslcontext-kickstart/src/test/java/nl/altindag/ssl/util/CertificateUtilsShould.java
@@ -15,6 +15,7 @@
  */
 package nl.altindag.ssl.util;
 
+import nl.altindag.log.LogCaptor;
 import nl.altindag.ssl.TestConstants;
 import nl.altindag.ssl.exception.GenericCertificateException;
 import nl.altindag.ssl.exception.GenericIOException;
@@ -568,11 +569,13 @@ void throwsGenericIOExceptionWhenCloseOfTheStreamFails() throws IOException {
     }
 
     @Test
-    void throwGenericCertificateExceptionWhenUnsupportedDataIsProvided() throws IOException {
-        try(ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream("Hello".getBytes())) {
-            assertThatThrownBy(() -> CertificateUtils.parseDerCertificate(byteArrayInputStream))
-                    .isInstanceOf(GenericCertificateException.class)
-                    .hasMessage("There is no valid certificate present to parse. Please make sure to supply a valid der formatted certificate");
+    void generateDebugMessageWhenUnsupportedDataIsProvided() throws IOException {
+        try(LogCaptor logCaptor = LogCaptor.forClass(CertificateUtils.class);
+            ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream("Hello".getBytes())) {
+            List<Certificate> certificates = CertificateUtils.parseDerCertificate(byteArrayInputStream);
+
+            assertThat(certificates).isEmpty();
+            assertThat(logCaptor.getDebugLogs()).contains("There is no valid certificate present to parse. Please make sure to supply a valid der formatted certificate");
         }
     }
 

diff --git a/sslcontext-kickstart/src/test/java/nl/altindag/ssl/util/KeyStoreUtilsShould.java b/sslcontext-kickstart/src/test/java/nl/altindag/ssl/util/KeyStoreUtilsShould.java
@@ -254,7 +254,7 @@ void loadMacSystemKeyStore() {
         KeyStore keychainStore = mock(KeyStore.class);
         KeyStore systemTrustStore = mock(KeyStore.class);
 
-        try (MockedStatic<MacCertificateUtils> macCertificateUtilsMockedStatic = mockStatic(MacCertificateUtils.class);
+        try (MockedStatic<MacCertificateUtils> macCertificateUtilsMockedStatic = mockStatic(MacCertificateUtils.class, invocationOnMock -> Collections.singletonList(mock(X509Certificate.class)));
              MockedStatic<KeyStoreUtils> keyStoreUtilsMockedStatic = mockStatic(KeyStoreUtils.class, invocation -> {
             Method method = invocation.getMethod();
             if ("loadSystemKeyStores".equals(method.getName()) && method.getParameterCount() == 0) {
@@ -283,7 +283,7 @@ void loadLinuxSystemKeyStoreReturns() {
 
         KeyStore systemTrustStore = mock(KeyStore.class);
 
-        try (MockedStatic<LinuxCertificateUtils> linuxCertificateUtilsMockedStatic = mockStatic(LinuxCertificateUtils.class);
+        try (MockedStatic<LinuxCertificateUtils> linuxCertificateUtilsMockedStatic = mockStatic(LinuxCertificateUtils.class, invocationOnMock -> Collections.singletonList(mock(X509Certificate.class)));
              MockedStatic<KeyStoreUtils> keyStoreUtilsMockedStatic = mockStatic(KeyStoreUtils.class, invocation -> {
                  Method method = invocation.getMethod();
                  if ("loadSystemKeyStores".equals(method.getName()) && method.getParameterCount() == 0) {

diff --git a/sslcontext-kickstart/src/test/java/nl/altindag/ssl/util/LinuxCertificateUtilsShould.java b/sslcontext-kickstart/src/test/java/nl/altindag/ssl/util/LinuxCertificateUtilsShould.java
@@ -238,12 +238,16 @@ void getCertificatesReturnsCertificatesWhenFileExistWithinDirectory() {
                          return true;
                      } else if ("isRegularFile".equals(methodName) && "/etc/ssl/certs".equals(path)) {
                          return false;
+                     } else if ("isSymbolicLink".equals(methodName) && "/etc/ssl/certs".equals(path)) {
+                         return false;
                      } else if ("isDirectory".equals(methodName) && "/etc/ssl/certs".equals(path)) {
                          return true;
                      } else if ("walk".equals(methodName)) {
                          return Stream.of(Paths.get("/etc/ssl/certs/some-certificate.pem"));
                      } else if ("isRegularFile".equals(methodName) && "/etc/ssl/certs/some-certificate.pem".equals(path)) {
                          return true;
+                     } else if ("isSymbolicLink".equals(methodName) && "/etc/ssl/certs/some-certificate.pem".equals(path)) {
+                         return true;
                      } else if ("exists".equals(methodName)) {
                          return false;
                      } else {