Ignore the order of the RDNs when validating the publisher of an APPX…

… file (#256)
ebourg · Oct 25, 2024 · 3ddf1a0 · 3ddf1a0
1 parent 49d9f10
commit 3ddf1a0
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 9 deletions.
diff --git a/jsign-core/src/main/java/net/jsign/appx/APPXFile.java b/jsign-core/src/main/java/net/jsign/appx/APPXFile.java
@@ -29,13 +29,13 @@
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import javax.security.auth.x500.X500Principal;
 
 import org.apache.commons.io.output.NullOutputStream;
 import org.apache.commons.text.StringEscapeUtils;
 import org.apache.poi.util.IOUtils;
 import org.bouncycastle.asn1.ASN1Object;
 import org.bouncycastle.asn1.DERNull;
+import org.bouncycastle.asn1.x500.X500Name;
 import org.bouncycastle.asn1.x509.AlgorithmIdentifier;
 import org.bouncycastle.asn1.x509.DigestInfo;
 import org.bouncycastle.cms.CMSSignedData;
@@ -167,21 +167,20 @@ public ASN1Object createIndirectData(DigestAlgorithm digestAlgorithm) throws IOE
      * Normalize the X500 name specified.
      */
     private String normalize(String name) {
-        if (name == null) {
-            return null;
+        if (name != null) {
+            // replace the non-standard S abbreviation used by Microsoft with ST for the stateOrProvinceName attribute
+            name = name.replaceAll(",\\s*S\\s*=", ",ST=");
         }
 
-        // replace the non-standard S abbreviation used by Microsoft with ST for the stateOrProvinceName attribute
-        name = name.replaceAll(",\\s*S\\s*=", ",ST=");
-
-        return new X500Principal(name).getName(X500Principal.CANONICAL);
+        return name;
     }
 
     @Override
     public void validate(Certificate certificate) throws IOException, IllegalArgumentException {
-        String name = ((X509Certificate) certificate).getSubjectX500Principal().getName();
+        X500Name name = X500Name.getInstance(((X509Certificate) certificate).getSubjectX500Principal().getEncoded());
         String publisher = getPublisher();
-        if (!normalize(name).equals(normalize(publisher))) {
+
+        if (publisher == null || !name.equals(new X500Name(normalize(publisher)))) {
             throw new IllegalArgumentException("The app manifest publisher name (" + publisher + ") must match the subject name of the signing certificate (" + name + ")");
         }
     }

diff --git a/jsign-core/src/test/java/net/jsign/appx/APPXFileTest.java b/jsign-core/src/test/java/net/jsign/appx/APPXFileTest.java
@@ -17,7 +17,14 @@
 package net.jsign.appx;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
 import java.security.KeyStore;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509Certificate;
+import javax.security.auth.x500.X500Principal;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
@@ -31,6 +38,7 @@
 import static net.jsign.DigestAlgorithm.*;
 import static net.jsign.SignatureAssert.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 public class APPXFileTest {
 
@@ -110,4 +118,54 @@ public void testGetBundlePublisher() throws Exception {
             assertEquals("Publisher", "CN=Jsign Code Signing Test Certificate 2024 (RSA)", file.getPublisher());
         }
     }
+
+    public static Certificate getCertificate() throws IOException, CertificateException {
+        try (FileInputStream in = new FileInputStream("target/test-classes/keystores/jsign-test-certificate.pem")) {
+            return CertificateFactory.getInstance("X.509").generateCertificates(in).iterator().next();
+        }
+    }
+
+    @Test
+    public void testValidateWithMatchingPublisher() throws Exception {
+        try (APPXFile file = new APPXFile(new File("target/test-classes/minimal.msix"))) {
+            file.validate(getCertificate());
+        }
+    }
+
+    @Test
+    public void testValidateWithMismatchingPublisher() throws Exception {
+        try (APPXFile file = spy(new APPXFile(new File("target/test-classes/minimal.msix")))) {
+            when(file.getPublisher()).thenReturn("CN=Jsign Code Signing Test Certificate 1977 (RSA)");
+            try {
+                file.validate(getCertificate());
+                fail("Exception not thrown");
+            } catch (IllegalArgumentException e) {
+                assertEquals("message", "The app manifest publisher name (CN=Jsign Code Signing Test Certificate 1977 (RSA)) must match the subject name of the signing certificate (CN=Jsign Code Signing Test Certificate 2024 (RSA))", e.getMessage());
+            }
+        }
+    }
+
+    @Test
+    public void testValidateWithReorderedPublisher() throws Exception {
+        try (APPXFile file = spy(new APPXFile(new File("target/test-classes/minimal.msix")))) {
+            when(file.getPublisher()).thenReturn("C=US, S=New York,  L=New York, O=\"COMPANY, INC.\",CN=\"COMPANY, INC.\"");
+
+            X509Certificate certificate = spy((X509Certificate) getCertificate());
+            when(certificate.getSubjectX500Principal()).thenReturn(new X500Principal("CN=\"COMPANY, INC.\",O=\"COMPANY, INC.\",L=New York,ST=New York,C=US"));
+            file.validate(certificate);
+        }
+    }
+
+    @Test
+    public void testValidateWithMissingPublisher() throws Exception {
+        try (APPXFile file = spy(new APPXFile(new File("target/test-classes/minimal.msix")))) {
+            when(file.getPublisher()).thenReturn(null);
+            try {
+                file.validate(getCertificate());
+                fail("Exception not thrown");
+            } catch (IllegalArgumentException e) {
+                assertEquals("message", "The app manifest publisher name (null) must match the subject name of the signing certificate (CN=Jsign Code Signing Test Certificate 2024 (RSA))", e.getMessage());
+            }
+        }
+    }
 }