From 18e42286e2c2f237c35f5b7cd6aeb73e17bd564f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez?= <me@alegon.dev>
Date: Sat, 7 Dec 2024 16:33:15 +0100
Subject: [PATCH] Make `PrivateKeyUtils#load` method file extension agnostic

As outlined in #264, this is achieved by trying to parse the key file
according to one of the supported formats in sequence until one works.
Given that there are only two supported formats at the moment, and that
PEM files are attempted first and more common in the wild than PVK, this
approach should have good enough performance. Because a Java exception
can only have a single cause, I've attached the underlying parse
exceptions to the higher-level `KeyException` as supressed exceptions.
These get displayed to consumers on e.g. stacktraces, allowing them to
know what exactly went wrong when parsing either format.

While at it, I've added a test to ensure that this extension-agnostic
behavior is maintained over time.

Resolves #264.
---
 .../main/java/net/jsign/PrivateKeyUtils.java  | 29 ++++++++++++-------
 .../java/net/jsign/PrivateKeyUtilsTest.java   | 14 +++++++--
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/jsign-crypto/src/main/java/net/jsign/PrivateKeyUtils.java b/jsign-crypto/src/main/java/net/jsign/PrivateKeyUtils.java
index ebd8bb14..e3bd63a2 100644
--- a/jsign-crypto/src/main/java/net/jsign/PrivateKeyUtils.java
+++ b/jsign-crypto/src/main/java/net/jsign/PrivateKeyUtils.java
@@ -54,26 +54,33 @@ private PrivateKeyUtils() {
 
     /**
      * Load the private key from the specified file. Supported formats are PVK and PEM,
-     * encrypted or not. The type of the file is inferred from its extension (<code>.pvk</code>
-     * for PVK files, <code>.pem</code> for PEM files).
-     * 
+     * encrypted or not. The type of the file is inferred by trying the supported formats
+     * in sequence until one parses successfully.
+     *
      * @param file     the file to load the key from
      * @param password the password protecting the key
      * @return the private key loaded
      * @throws KeyException if the key cannot be loaded
      */
     public static PrivateKey load(File file, String password) throws KeyException {
+        Exception pemParseException;
         try {
-            if (file.getName().endsWith(".pvk")) {
-                return PVK.parse(file, password);
-            } else if (file.getName().endsWith(".pem")) {
-                return readPrivateKeyPEM(file, password != null ? password.toCharArray() : null);
-            }
+            return readPrivateKeyPEM(file, password != null ? password.toCharArray() : null);
+        } catch (Exception e) {
+            pemParseException = e;
+        }
+
+        Exception pvkParseException;
+        try {
+            return PVK.parse(file, password);
         } catch (Exception e) {
-            throw new KeyException("Failed to load the private key from " + file, e);
+            pvkParseException = e;
         }
-        
-        throw new IllegalArgumentException("Unsupported private key format (PEM or PVK file expected");
+
+        KeyException keyException = new KeyException("Failed to load the private key from " + file + " (valid PEM or PVK file expected)");
+        keyException.addSuppressed(pemParseException);
+        keyException.addSuppressed(pvkParseException);
+        throw keyException;
     }
 
     /**
diff --git a/jsign-crypto/src/test/java/net/jsign/PrivateKeyUtilsTest.java b/jsign-crypto/src/test/java/net/jsign/PrivateKeyUtilsTest.java
index cbcac906..f5af2ec7 100644
--- a/jsign-crypto/src/test/java/net/jsign/PrivateKeyUtilsTest.java
+++ b/jsign-crypto/src/test/java/net/jsign/PrivateKeyUtilsTest.java
@@ -19,6 +19,8 @@
 import java.io.File;
 import java.io.FileWriter;
 import java.math.BigInteger;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.security.KeyException;
 import java.security.PrivateKey;
 import java.security.interfaces.ECPrivateKey;
@@ -54,6 +56,14 @@ public void testLoadPKCS1PEM() throws Exception {
         testLoadPEM(new File("target/test-classes/keystores/privatekey.pkcs1.pem"), null);
     }
 
+    @Test
+    public void testLoadPKCS1PEMNonPEMExtension() throws Exception {
+        File targetFile = new File("target/test-classes/keystores/privatekey.pkcs1.pem.key");
+        Files.copy(new File("target/test-classes/keystores/privatekey.pkcs1.pem").toPath(), targetFile.toPath());
+
+        testLoadPEM(targetFile, null);
+    }
+
     @Test
     public void testLoadEncryptedPKCS1PEM() throws Exception {
         testLoadPEM(new File("target/test-classes/keystores/privatekey-encrypted.pkcs1.pem"), "password");
@@ -71,7 +81,7 @@ private void testLoadPEM(File file, String password) throws Exception {
     @Test
     public void testLoadWrongPEMObject() {
         Exception e = assertThrows(KeyException.class, () -> PrivateKeyUtils.load(new File("target/test-classes/keystores/jsign-test-certificate.pem"), null));
-        assertEquals("message", "Unsupported PEM object: X509CertificateHolder", e.getCause().getMessage());
+        assertEquals("message", "Unsupported PEM object: X509CertificateHolder", e.getSuppressed()[0].getMessage());
     }
 
     @Test
@@ -82,7 +92,7 @@ public void testLoadEmptyPEM() throws Exception {
         writer.close();
 
         Exception e = assertThrows(KeyException.class, () -> PrivateKeyUtils.load(file, null));
-        assertTrue(e.getCause().getMessage().startsWith("No key found in"));
+        assertTrue(e.getSuppressed()[0].getMessage().startsWith("No key found in"));
     }
 
     @Test