From 7904bba55374eff07e35e77e34a5a7cd6218e692 Mon Sep 17 00:00:00 2001 From: Clement Escoffier Date: Mon, 4 Mar 2024 20:20:52 +0100 Subject: [PATCH] Improve error message when the keystore/truststore type cannot be detected from the file extension. --- .../vertx/http/runtime/options/TlsUtils.java | 78 ++++++++++++------- .../http/runtime/options/TlsUtilsTest.java | 76 ++++++++++++++++++ 2 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/options/TlsUtilsTest.java diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/TlsUtils.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/TlsUtils.java index 7b2a08c729cb6..cbeb165df3753 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/TlsUtils.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/TlsUtils.java @@ -24,6 +24,7 @@ private TlsUtils() { public static KeyCertOptions computeKeyStoreOptions(CertificateConfig certificates, Optional keyStorePassword, Optional keyStoreAliasPassword) throws IOException { + if (certificates.keyFiles.isPresent() || certificates.files.isPresent()) { if (certificates.keyFiles.isEmpty()) { throw new IllegalArgumentException("You must specify the key files when specifying the certificate files"); @@ -35,13 +36,13 @@ public static KeyCertOptions computeKeyStoreOptions(CertificateConfig certificat throw new IllegalArgumentException( "The number of certificate files and key files must be the same, and be given in the same order"); } - return createPemKeyCertOptions(certificates.files.get(), certificates.keyFiles.get()); } else if (certificates.keyStoreFile.isPresent()) { + var type = getKeyStoreType(certificates.keyStoreFile.get(), certificates.keyStoreFileType); return createKeyStoreOptions( certificates.keyStoreFile.get(), keyStorePassword, - certificates.keyStoreFileType, + type, certificates.keyStoreProvider, or(certificates.keyStoreAlias, certificates.keyStoreKeyAlias), keyStoreAliasPassword); @@ -52,21 +53,10 @@ public static KeyCertOptions computeKeyStoreOptions(CertificateConfig certificat public static TrustOptions computeTrustOptions(CertificateConfig certificates, Optional trustStorePassword) throws IOException { // Decide if we have a single trust store file or multiple trust store files (PEM) - Path singleTrustStoreFile = null; - if (certificates.trustStoreFile.isPresent()) { - singleTrustStoreFile = certificates.trustStoreFile.get(); - } - if (certificates.trustStoreFiles.isPresent()) { - if (singleTrustStoreFile != null) { - throw new IllegalArgumentException("You cannot specify both `trustStoreFile` and `trustStoreFiles`"); - } - if (certificates.trustStoreFiles.get().size() == 1) { - singleTrustStoreFile = certificates.trustStoreFiles.get().get(0); - } - } + Path singleTrustStoreFile = getSingleTrustStoreFile(certificates); if (singleTrustStoreFile != null) { // We have a single trust store file. - String type = certificates.trustStoreFileType.orElse(getTypeFromFileName(singleTrustStoreFile)); + String type = getTruststoreType(singleTrustStoreFile, certificates.trustStoreFileType); if (type.equalsIgnoreCase("pem")) { byte[] cert = getFileContent(singleTrustStoreFile); return new PemTrustOptions() @@ -83,7 +73,7 @@ public static TrustOptions computeTrustOptions(CertificateConfig certificates, O return createKeyStoreOptions( singleTrustStoreFile, trustStorePassword, - certificates.trustStoreFileType, + type, certificates.trustStoreProvider, certificates.trustStoreCertAlias, Optional.empty()); @@ -103,7 +93,33 @@ public static TrustOptions computeTrustOptions(CertificateConfig certificates, O return null; } - private static String getTypeFromFileName(Path path) { + private static Path getSingleTrustStoreFile(CertificateConfig certificates) { + Path singleTrustStoreFile = null; + if (certificates.trustStoreFile.isPresent()) { + singleTrustStoreFile = certificates.trustStoreFile.get(); + } + if (certificates.trustStoreFiles.isPresent()) { + if (singleTrustStoreFile != null) { + throw new IllegalArgumentException("You cannot specify both `trustStoreFile` and `trustStoreFiles`"); + } + if (certificates.trustStoreFiles.get().size() == 1) { + singleTrustStoreFile = certificates.trustStoreFiles.get().get(0); + } + } + return singleTrustStoreFile; + } + + static String getTruststoreType(Path singleTrustStoreFile, Optional userType) { + String type; + if (userType.isPresent()) { + type = userType.get().toLowerCase(); + } else { + type = getTypeFromFileName("truststore", singleTrustStoreFile); + } + return type; + } + + private static String getTypeFromFileName(String keystoreOrTruststore, Path path) { String name = path.getFileName().toString().toLowerCase(); if (name.endsWith(".p12") || name.endsWith(".pkcs12") || name.endsWith(".pfx")) { return "pkcs12"; @@ -112,21 +128,17 @@ private static String getTypeFromFileName(Path path) { } else if (name.endsWith(".key") || name.endsWith(".crt") || name.endsWith(".pem")) { return "pem"; } else { - throw new IllegalArgumentException("Could not determine the trust store type from the file name: " + path - + ". Configure the file type property."); + throw new IllegalArgumentException("Could not determine the " + keystoreOrTruststore + + " type from the file name: " + path + + ". Configure the `quarkus.http.ssl.certificate.[key-store|trust-store]-file-type` property."); + } } - private static KeyStoreOptions createKeyStoreOptions(Path path, Optional password, Optional fileType, - Optional provider, Optional alias, Optional aliasPassword) throws IOException { - final String type; - if (fileType.isPresent()) { - type = fileType.get().toLowerCase(); - } else { - type = getTypeFromFileName(path); - } - + private static KeyStoreOptions createKeyStoreOptions(Path path, Optional password, String type, + Optional provider, Optional alias, + Optional aliasPassword) throws IOException { byte[] data = getFileContent(path); return new KeyStoreOptions() .setPassword(password.orElse(null)) @@ -137,6 +149,16 @@ private static KeyStoreOptions createKeyStoreOptions(Path path, Optional .setAliasPassword(aliasPassword.orElse(null)); } + static String getKeyStoreType(Path path, Optional fileType) { + final String type; + if (fileType.isPresent()) { + type = fileType.get().toLowerCase(); + } else { + type = getTypeFromFileName("keystore", path); + } + return type; + } + private static PemKeyCertOptions createPemKeyCertOptions(List certFile, List keyFile) throws IOException { List certificates = new ArrayList<>(); List keys = new ArrayList<>(); diff --git a/extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/options/TlsUtilsTest.java b/extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/options/TlsUtilsTest.java new file mode 100644 index 0000000000000..802237f8cd716 --- /dev/null +++ b/extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/options/TlsUtilsTest.java @@ -0,0 +1,76 @@ +package io.quarkus.vertx.http.runtime.options; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.File; +import java.nio.file.Path; +import java.util.Optional; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class TlsUtilsTest { + + @ParameterizedTest + @CsvSource({ + "server-keystore.jks, JKS, JKS", + "server-keystore.jks, jKs, JKS", + "server-keystore.jks, null, JKS", + "server-keystore.jks, PKCS12, PKCS12", + "server.keystore, null, null", // Failure expected + "server-keystore.p12, PKCS12, PKCS12", + "server-keystore.p12, pKCs12, PKCS12", + "server-keystore.p12, null, PKCS12", + "server-keystore.pfx, null, PKCS12", + "server-keystore.pkcs12, null, PKCS12", + "server-keystore.pkcs12, JKS, JKS", + "server.keystore.key, null, PEM", + "server.keystore.crt, null, PEM", + "server.keystore.pem, null, PEM", + "server.keystore.key, JKS, JKS", + "server.keystore.pom, PeM, PEM", + }) + void testKeyStoreTypeDetection(String file, String userType, String expectedType) { + Path path = new File("target/certs/" + file).toPath(); + Optional type = Optional.ofNullable(userType.equals("null") ? null : userType); + if (expectedType.equals("null")) { + String message = assertThrows(IllegalArgumentException.class, () -> TlsUtils.getKeyStoreType(path, type)) + .getMessage(); + assertTrue(message.contains("keystore")); + } else { + assertEquals(expectedType.toLowerCase(), TlsUtils.getKeyStoreType(path, type)); + } + } + + @ParameterizedTest + @CsvSource({ + "server-truststore.jks, JKS, JKS", + "server-truststore.jks, jKs, JKS", + "server-truststore.jks, null, JKS", + "server-truststore.jks, PKCS12, PKCS12", + "server.truststore, null, null", // Failure expected + "server-truststore.p12, PKCS12, PKCS12", + "server-truststore.p12, pKCs12, PKCS12", + "server-truststore.p12, null, PKCS12", + "server-truststore.pfx, null, PKCS12", + "server-truststore.pkcs12, null, PKCS12", + "server-truststore.pkcs12, JKS, JKS", + "server.truststore.key, null, PEM", + "server.truststore.crt, null, PEM", + "server.truststore.pem, null, PEM", + "server.truststore.key, JKS, JKS", + "server.truststore.pom, PeM, PEM", + }) + void testTrustStoreTypeDetection(String file, String userType, String expectedType) { + Path path = new File("target/certs/" + file).toPath(); + Optional type = Optional.ofNullable(userType.equals("null") ? null : userType); + if (expectedType.equals("null")) { + String message = assertThrows(IllegalArgumentException.class, () -> TlsUtils.getTruststoreType(path, type)) + .getMessage(); + assertTrue(message.contains("truststore")); + } else { + assertEquals(expectedType.toLowerCase(), TlsUtils.getTruststoreType(path, type)); + } + } + +}