Skip to content

Commit

Permalink
Check if the certificate subject name matches the app manifest publis…
Browse files Browse the repository at this point in the history
…her name before signing APPX/MSIX packages (#196)
  • Loading branch information
ebourg committed Feb 14, 2024
1 parent a78ebc3 commit e861628
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ See https://ebourg.github.io/jsign for more information.
#### Version 6.1 (in development)

* Signing of NuGet packages has been implemented (contributed by Sebastian Stamm)
* Jsign now checks if the certificate subject matches the app manifest publisher before signing APPX/MSIX packages
* The APPX/MSIX bundles are now signed with the correct Authenticode UUID
* The error message displayed when the password of a PKCS#12 keystore is missing has been fixed

Expand Down
2 changes: 2 additions & 0 deletions jsign-core/src/main/java/net/jsign/AuthenticodeSigner.java
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,8 @@ public AuthenticodeSigner withSignatureProvider(Provider signatureProvider) {
* @throws Exception if signing fails
*/
public void sign(Signable file) throws Exception {
file.validate(chain[0]);

if (file instanceof PEFile) {
PEFile pefile = (PEFile) file;

Expand Down
12 changes: 12 additions & 0 deletions jsign-core/src/main/java/net/jsign/Signable.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.io.IOException;
import java.nio.charset.Charset;
import java.security.MessageDigest;
import java.security.cert.Certificate;
import java.util.List;
import java.util.ServiceLoader;

Expand Down Expand Up @@ -98,6 +99,17 @@ default byte[] computeDigest(DigestAlgorithm digestAlgorithm) throws IOException
*/
ASN1Object createIndirectData(DigestAlgorithm digestAlgorithm) throws IOException;

/**
* Checks if the specified certificate is suitable for signing the file.
*
* @param certificate the certificate to validate
* @throws IOException if an I/O error occurs
* @throws IllegalArgumentException if the certificate doesn't match the publisher identity
* @since 6.1
*/
default void validate(Certificate certificate) throws IOException, IllegalArgumentException {
}

/**
* Returns the Authenticode signatures on the file.
*
Expand Down
25 changes: 25 additions & 0 deletions jsign-core/src/main/java/net/jsign/appx/APPXFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@
import java.nio.channels.SeekableByteChannel;
import java.security.DigestOutputStream;
import java.security.MessageDigest;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.io.output.NullOutputStream;
import org.apache.poi.util.IOUtils;
Expand Down Expand Up @@ -163,13 +167,34 @@ public ASN1Object createIndirectData(DigestAlgorithm digestAlgorithm) throws IOE
return new SpcIndirectDataContent(data, digestInfo);
}

@Override
public void validate(Certificate certificate) throws IOException, IllegalArgumentException {
String name = ((X509Certificate) certificate).getSubjectX500Principal().getName();
String publisher = getPublisher();
if (!name.equals(publisher)) {
throw new IllegalArgumentException("The app manifest publisher name (" + publisher + ") must match the subject name of the signing certificate (" + name + ")");
}
}

/**
* Tells if the package is a bundle.
*/
boolean isBundle() {
return centralDirectory.entries.containsKey("AppxMetadata/AppxBundleManifest.xml");
}

/**
* Returns the publisher of the package.
*/
String getPublisher() throws IOException {
InputStream in = getInputStream(isBundle() ? "AppxMetadata/AppxBundleManifest.xml" : "AppxManifest.xml", 10 * 1024 * 1024 /* 10MB */);
String manifest = new String(IOUtils.toByteArray(in), UTF_8);

Pattern pattern = Pattern.compile("Publisher\\s*=\\s*\"([^\"]+)", Pattern.CASE_INSENSITIVE);
Matcher matcher = pattern.matcher(manifest);
return matcher.find() ? matcher.group(1) : null;
}

@Override
public List<CMSSignedData> getSignatures() throws IOException {
List<CMSSignedData> signatures = new ArrayList<>();
Expand Down
26 changes: 26 additions & 0 deletions jsign-core/src/test/java/net/jsign/APPXSignerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@

import org.apache.commons.compress.utils.SeekableInMemoryByteChannel;
import org.apache.commons.io.FileUtils;
import org.hamcrest.MatcherAssert;
import org.junit.Test;

import net.jsign.appx.APPXFile;

import static net.jsign.DigestAlgorithm.*;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;

public class APPXSignerTest {

Expand Down Expand Up @@ -172,4 +175,27 @@ public void testSignInMemory() throws Exception {
SignatureAssert.assertSigned(file, SHA256);
}
}

@Test
public void testSignWithMismatchedCertificate() throws Exception {
File sourceFile = new File("target/test-classes/minimal.msix");
File targetFile = new File("target/test-classes/minimal-signed-with-mismatched-certificate.msix");

FileUtils.copyFile(sourceFile, targetFile);

KeyStore keystore = new KeyStoreBuilder().storetype("NONE")
.keyfile("target/test-classes/keystores/privatekey.pkcs8.pem")
.certfile("target/test-classes/keystores/jsign-test-certificate-partial-chain.pem")
.build();
AuthenticodeSigner signer = new AuthenticodeSigner(keystore, "jsign", "").withTimestamping(false);

try (Signable file = Signable.of(targetFile)) {
try {
signer.sign(file);
fail("Exception not thrown");
} catch (Exception e) {
MatcherAssert.assertThat(e.getMessage(), matchesPattern("The app manifest publisher name (.*) must match the subject name of the signing certificate (.*)"));
}
}
}
}
14 changes: 14 additions & 0 deletions jsign-core/src/test/java/net/jsign/appx/APPXFileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,18 @@ public void testIsBundle() throws Exception {
assertTrue("minimal.appxbundle is not a bundle", file.isBundle());
}
}

@Test
public void testGetPackagePublisher() throws Exception {
try (APPXFile file = new APPXFile(new File("target/test-classes/minimal.msix"))) {
assertEquals("Publisher", "CN=Jsign Code Signing Test Certificate 2022 (RSA)", file.getPublisher());
}
}

@Test
public void testGetBundlePublisher() throws Exception {
try (APPXFile file = new APPXFile(new File("target/test-classes/minimal.appxbundle"))) {
assertEquals("Publisher", "CN=Jsign Code Signing Test Certificate 2022 (RSA)", file.getPublisher());
}
}
}

0 comments on commit e861628

Please sign in to comment.