From 1b30b37cea8d63a617f8a78bb7e71bc705a2cfea Mon Sep 17 00:00:00 2001
From: Tres Finocchiaro <tres.finocchiaro@gmail.com>
Date: Fri, 7 May 2021 15:15:58 -0400
Subject: [PATCH] Better certificate handling when organization is blank (#804)

---
 src/qz/auth/Certificate.java     | 26 +++++++++++++++++++++-----
 src/qz/ui/SiteManagerDialog.java |  2 ++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/qz/auth/Certificate.java b/src/qz/auth/Certificate.java
index 001640ccb..9ce511d04 100644
--- a/src/qz/auth/Certificate.java
+++ b/src/qz/auth/Certificate.java
@@ -6,6 +6,7 @@
 import org.apache.commons.ssl.Base64;
 import org.apache.commons.ssl.X509CertificateChainBuilder;
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
+import org.bouncycastle.asn1.x500.style.BCStyle;
 import org.bouncycastle.asn1.x509.X509Name;
 import org.bouncycastle.jce.PrincipalUtil;
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
@@ -58,6 +59,8 @@ public enum Algorithm {
     private static CertPathValidator validator;
     private static CertificateFactory factory;
     private static boolean trustBuiltIn = false;
+    // id-at-description used for storing renewal information
+    private static ASN1ObjectIdentifier RENEWAL_OF = new ASN1ObjectIdentifier("2.5.4.13");
 
     public static final String[] saveFields = new String[] {"fingerprint", "commonName", "organization", "validFrom", "validTo", "valid"};
 
@@ -178,7 +181,6 @@ public Certificate(Path path) throws IOException, CertificateException {
     }
 
     /** Decodes a certificate and intermediate certificate from the given string */
-    @SuppressWarnings("deprecation")
     public Certificate(String in) throws CertificateException {
         try {
             //Strip beginning and end
@@ -195,9 +197,12 @@ public Certificate(String in) throws CertificateException {
 
             //Generate cert
             theCertificate = (X509Certificate)factory.generateCertificate(new ByteArrayInputStream(serverCertificate));
-            commonName = String.valueOf(PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(X509Name.CN).get(0));
+            commonName = getSubjectX509Principal(theCertificate, BCStyle.CN);
+            if(commonName.isEmpty()) {
+                throw new CertificateException("Common Name cannot be blank.");
+            }
             fingerprint = makeThumbPrint(theCertificate);
-            organization = String.valueOf(PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(X509Name.O).get(0));
+            organization = getSubjectX509Principal(theCertificate, BCStyle.O);
             validFrom = theCertificate.getNotBefore().toInstant();
             validTo = theCertificate.getNotAfter().toInstant();
 
@@ -269,8 +274,7 @@ public Certificate(String in) throws CertificateException {
     }
 
     private void readRenewalInfo() throws Exception {
-        // "id-at-description" = "2.5.4.13"
-        Vector values = PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(new ASN1ObjectIdentifier("2.5.4.13"));
+        Vector values = PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(RENEWAL_OF);
         Iterator renewals = values.iterator();
 
         while(renewals.hasNext()) {
@@ -500,4 +504,16 @@ public static boolean hasAdditionalCAs() {
         return rootCAs.size() > (isTrustBuiltIn() ? 1 : 0);
     }
 
+    private static String getSubjectX509Principal(X509Certificate cert, ASN1ObjectIdentifier key) {
+        try {
+            Vector v = PrincipalUtil.getSubjectX509Principal(cert).getValues(key);
+            if(v.size() > 0) {
+                return String.valueOf(v.get(0));
+            }
+        } catch(CertificateEncodingException e) {
+            log.warn("Certificate encoding exception occurred", e);
+        }
+        return "";
+    }
+
 }
diff --git a/src/qz/ui/SiteManagerDialog.java b/src/qz/ui/SiteManagerDialog.java
index b727ff41a..18ca7f7b2 100644
--- a/src/qz/ui/SiteManagerDialog.java
+++ b/src/qz/ui/SiteManagerDialog.java
@@ -42,6 +42,7 @@ public class SiteManagerDialog extends BasicDialog implements Runnable {
     private static final String IMPORT_NEEDED = "The provided certificate \"%s\" is unrecognized and not yet trusted.\n"  +
             "Would you like to automatically copy it to \"%s\"?";
     private static final String IMPORT_FAILED = "Failed to import certificate.  Please import manually.";
+    private static final String INVALID_CERTIFICATE = "An exception occurred importing the certificate.  Please check the logs for details.";
     private static final String IMPORT_QUESTION = "Successfully created a new demo keypair.  Automatically install?";
 
     private static final String DEMO_CERT_QUESTION = "Create a new demo keypair for %s?\n" +
@@ -449,6 +450,7 @@ private void addCertificates(File[] certFiles, ContainerList<CertificateDisplay>
             }
             catch(CertificateException | IOException e) {
                 log.warn("Unable to import cert {}", file, e);
+                JOptionPane.showMessageDialog(this, String.format(INVALID_CERTIFICATE), "Import failed", JOptionPane.ERROR_MESSAGE);
             }
         }
     }