pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feedFrom: vlsi (@vlsi) <[email protected]>
To: pgjdbc/pgjdbc <[email protected]>
Subject: [pgjdbc/pgjdbc] PR #3946: fix: auto-detect SSL key format instead of relying on .key extension
Date: Tue, 24 Feb 2026 12:16:34 +0000
Message-ID: <[email protected]> (raw)
## Why
The `.key` extension is generic: it can hold either a PEM or a DER/PKCS-8 key. pgJDBC mapped every `.key` file to PEM, so a DER-encoded `.key` failed with `MalformedInputException` and the connection never came up (#3942).
## What
- Key format now follows the file extension, with bounded auto-detection as the default:
- `.p12` / `.pfx` → PKCS-12
- `.pem` → PEM
- `.der` → DER/PKCS-8 (escape hatch to skip auto-detection)
- anything else, including `.key` → scan the first 64 KiB for the `-----BEGIN PRIVATE KEY-----` header; use PEM when found, otherwise DER/PKCS-8
- The 64 KiB bound avoids excessive work during format detection on malformed files.
- This preserves the libpq preference for PEM before DER, although libpq attempts to load PEM before falling back to DER rather than scanning for a marker.
- New `Pk8OrPemKeyManager` wraps `PEMKeyManager` and `LazyKeyManager` and resolves the delegate lazily on the first `X509KeyManager` call, so it works regardless of which method the TLS engine calls first.
- Key-file permissions are validated once, before format detection, so the DER/PKCS-8 fallback cannot bypass the permission check that PEM keys already get.
- `sslkey` property docs and the SSL documentation describe the format mapping and bounded detection.
## How to verify
- `enable_ssl_tests=true ./gradlew :postgresql:test --tests 'org.postgresql.test.ssl.*'`
- New `Pk8OrPemKeyManagerTest` covers the cases: a DER `.key` connects over SSL, a PEM `.key` connects, and a key file with insecure permissions is rejected rather than loaded through the fallback.
Fixes #3942
diff --git a/README.md b/README.md
index b7ebf9954c..1267c87968 100644
--- a/README.md
+++ b/README.md
@@ -102,7 +102,7 @@ In addition to the standard connection parameters the driver supports a number o
| sslfactoryarg (deprecated) | String | null | Argument forwarded to constructor of SSLSocketFactory class. |
| sslmode | String | prefer | Controls the preference for opening using an SSL encrypted connection. |
| sslcert | String | null | The location of the client's SSL certificate |
-| sslkey | String | null | The location of the client's PKCS#8 or PKCS#12 SSL key, for PKCS the extension must be .p12 or .pfx and the alias must be `user` |
+| sslkey | String | null | The location of the client's SSL key. `.p12`/`.pfx` select PKCS-12, `.pem` selects PEM, and `.der` selects DER/PKCS-8. For other extensions, the first 64 KiB is scanned for a PEM private-key header, otherwise DER/PKCS-8 is used. The PKCS-12 alias must be `user`. |
| sslrootcert | String | null | The location of the root certificate for authenticating the server. |
| sslhostnameverifier | String | null | The name of a class (for use in [Class.forName(String)](https://docs.oracle.com/javase/6/docs/api/java/lang/Class.html#forName%28java.lang.String%29)) that implements javax.net.ssl.HostnameVerifier and can verify the server hostname. |
| sslpasswordcallback | String | null | The name of a class (for use in [Class.forName(String)](https://docs.oracle.com/javase/6/docs/api/java/lang/Class.html#forName%28java.lang.String%29)) that implements javax.security.auth.callback.CallbackHandler and can handle PasswordCallback for the ssl password. |
diff --git a/certdir/Makefile b/certdir/Makefile
index 9f5e01068d..9d54e6b7ba 100644
--- a/certdir/Makefile
+++ b/certdir/Makefile
@@ -5,7 +5,7 @@ SERVER_CRT_DIR=server/
all : $(SERVER_CRT_DIR)root.key $(SERVER_CRT_DIR)root.crt $(SERVER_CRT_DIR)server.crt goodroot.crt goodclient badclient
-goodclient: goodclient.crt goodclient.p12
+goodclient: goodclient.crt goodclient.p12 goodclient.pk8
badclient: badclient.crt badclient.p12
@@ -13,9 +13,14 @@ badclient: badclient.crt badclient.p12
clean:
@echo Removing certificate files
@rm -f *.crt *.key *.csr *.srl *.p12
+ @rm -f *.pk8
@rm -rf $(SERVER_CRT_DIR)*.crt $(SERVER_CRT_DIR)*.key $(SERVER_CRT_DIR)*.csr $(SERVER_CRT_DIR)*.srl $(SERVER_CRT_DIR)*.p12 $(SERVER_CRT_DIR)*.pk8
@echo
+%.pk8 : %.key
+ @echo Converting key to DER PKCS#8 format $@
+ openssl pkcs8 -topk8 -nocrypt -inform PEM -outform DER -in $< -out $@
+
%.p12 : %.crt
@echo Exporting certificate $@
openssl pkcs12 -export -in $< -inkey $*.key -out $@ -name user -CAfile $(SERVER_CRT_DIR)root.crt -caname local -passout pass:$(P12_PASSWORD)
diff --git a/certdir/goodclient.pk8 b/certdir/goodclient.pk8
new file mode 100644
index 0000000000..1dad18c85d
Binary files /dev/null and b/certdir/goodclient.pk8 differ
diff --git a/docs/content/documentation/ssl.md b/docs/content/documentation/ssl.md
index 28fb99b4b2..074ca48fc3 100644
--- a/docs/content/documentation/ssl.md
+++ b/docs/content/documentation/ssl.md
@@ -86,10 +86,13 @@ The location of the client certificate, the PKCS-12 client key and root certific
and `/defaultdir/root.crt` respectively where defaultdir is `${user.home}/.postgresql/` in *nix systems and `%appdata%/postgresql/`
on windows.
-As of version 42.2.9 PKCS-12 is also supported. In this archive format the client key and the client certificate are in
-one file, which needs to be set with the `sslkey` parameter. For the PKCS-12 format to be recognized, the file extension
+As of version 42.2.9 PKCS-12 is also supported. In this archive format the client key and the client certificate are in
+one file, which needs to be set with the `sslkey` parameter. For the PKCS-12 format to be recognized, the file extension
must be ".p12" (supported since 42.2.9) or ".pfx" (since 42.2.16). (In this case the `sslcert` parameter is ignored.)
+The key format follows the file extension (case-insensitive): ".p12"/".pfx" for PKCS-12, ".pem" for PEM, ".der" for DER/PKCS-8.
+For any other extension (including ".key"), the driver inspects the first 64 KiB of the file: it reads the key as PEM if that prefix contains the `-----BEGIN PRIVATE KEY-----` header, otherwise as DER/PKCS-8. The scan is bounded to avoid excessive work on malformed files. This preserves libpq's preference for PEM before DER, although libpq detects the format by attempting to load the key as PEM before falling back to DER.
+
> **NOTE**
>
> When using a PKCS-12 client certificate the name or alias *MUST* be `user` when using `openssl pkcs12 -export -name user ...`
diff --git a/docs/content/documentation/use.md b/docs/content/documentation/use.md
index b87d221132..aad6822588 100644
--- a/docs/content/documentation/use.md
+++ b/docs/content/documentation/use.md
@@ -130,13 +130,21 @@ It can be a PEM encoded X509v3 certificate
* **`sslkey (`*String*`)`** *Default `defaultdir/postgresql.pk8`*\
Provide the full path for the key file. Defaults to `defaultdir/postgresql.pk8`. See sslcert for defaultdir.
+The driver selects the key format from the file extension (case-insensitive) where it recognises one, and otherwise from the file content:
+
+| Extension | Format |
+|---|---|
+| `.p12`, `.pfx` | [PKCS-12](https://en.wikipedia.org/wiki/PKCS_12) (42.2.9+ / 42.2.16+) |
+| `.pem` | PEM-encoded [PKCS-8](https://en.wikipedia.org/wiki/PKCS_8) |
+| `.der` | [DER](https://wiki.openssl.org/index.php/DER)-encoded PKCS-8 |
+| anything else (e.g. `.key`) | Detected from the first 64 KiB: read as PEM if that prefix contains the `-----BEGIN PRIVATE KEY-----` header, otherwise as DER/PKCS-8 |
+
+The auto-detection scan is bounded to avoid excessive work on malformed files. This preserves libpq's preference for PEM before DER, although libpq attempts to load the key as PEM before falling back to DER.
+
> **NOTE**
>
-> The key file **must** be in [PKCS-12](https://en.wikipedia.org/wiki/PKCS_12) or in [PKCS-8](https://en.wikipedia.org/wiki/PKCS_8) [DER format](https://wiki.openssl.org/index.php/DER).
+> When you create a PKCS-12 key the `alias` or the `name` must be *user*. The test codes uses the following to create a .p12 key `openssl pkcs12 -export -in $< -inkey $*.key -out $@ -name user -CAfile $(SERVER_CRT_DIR)root.crt -caname local -passout pass:$(P12_PASSWORD)`
A PEM key can be converted to DER format using the openssl command: `openssl pkcs8 -topk8 -inform PEM -in postgresql.key -outform DER -out postgresql.pk8 -v1 PBE-MD5-DES`
- When you create the key the `alias` or the `name` must be *user*. The test codes uses the following to create a .p12 key `openssl pkcs12 -export -in $< -inkey $*.key -out $@ -name user -CAfile $(SERVER_CRT_DIR)root.crt -caname local -passout pass:$(P12_PASSWORD)`
-
-PKCS-12 key files are only recognized if they have the ".p12" (42.2.9+) or the ".pfx" (42.2.16+) extension.
If your key has a password, provide it using the `sslpassword` connection parameter described below. Otherwise, you can add the flag `-nocrypt` to the above command to prevent the driver from requesting a password.
diff --git a/pgjdbc/src/main/java/org/postgresql/PGProperty.java b/pgjdbc/src/main/java/org/postgresql/PGProperty.java
index 01782ae47c..b5c449d2a3 100644
--- a/pgjdbc/src/main/java/org/postgresql/PGProperty.java
+++ b/pgjdbc/src/main/java/org/postgresql/PGProperty.java
@@ -769,11 +769,19 @@ public enum PGProperty {
/**
* File containing the SSL Key. Default will be the file {@code postgresql.pk8} in {@code $HOME/.postgresql} (*nix)
* or {@code %APPDATA%\postgresql} (windows).
+ *
+ * <p>The key format follows the file extension (case-insensitive):
+ * {@code .p12}/{@code .pfx} for PKCS-12,
+ * {@code .pem} for PEM,
+ * {@code .der} for DER/PKCS-8.
+ * For any other extension (including {@code .key}), the driver inspects the first
+ * 64 KiB of the file: it reads the key as PEM if that prefix contains the
+ * {@code -----BEGIN PRIVATE KEY-----} header, otherwise as DER/PKCS-8.</p>
*/
SSL_KEY(
"sslkey",
null,
- "The location of the client's PKCS#8 SSL key"),
+ "The location of the client's SSL key"),
/**
* Parameter governing the use of SSL. The allowed values are {@code disable}, {@code allow},
diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java b/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
index cf0a0eaabf..5cf54ad1a0 100644
--- a/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
+++ b/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
@@ -99,6 +99,34 @@ private void initP12(
km = new PKCS12KeyManager(sslkeyfile, getCallbackHandler(info));
}
+ /**
+ * Case-insensitive {@link String#endsWith(String)}, so {@code .PEM} and {@code .pem}
+ * map to the same format on case-insensitive filesystems. Returns {@code false} when
+ * {@code value} is shorter than {@code suffix}.
+ */
+ private static boolean endsWithIgnoreCase(String value, String suffix) {
+ return value.regionMatches(true, value.length() - suffix.length(), suffix, 0, suffix.length());
+ }
+
+ private void initPk8OrPem(
+ @UnderInitialization(WrappedFactory.class)LibPQFactory this,
+ String sslkeyfile, String defaultdir, Properties info) throws PSQLException {
+ try {
+ String sslcertfile = getCertFilePath(defaultdir, info);
+ String algorithm = castNonNull(PGProperty.PEM_KEY_ALGORITHM.getOrDefault(info));
+ PEMKeyManager pem = new PEMKeyManager(sslkeyfile, sslcertfile, algorithm);
+ LazyKeyManager pk8 = new LazyKeyManager(
+ ("".equals(sslcertfile) ? null : sslcertfile),
+ ("".equals(sslkeyfile) ? null : sslkeyfile),
+ getCallbackHandler(info), defaultfile);
+ km = new Pk8OrPemKeyManager(sslkeyfile, pem, pk8);
+ } catch (Exception ex) {
+ throw new PSQLException(
+ GT.tr("Could not initialize key manager."),
+ PSQLState.CONNECTION_FAILURE, ex);
+ }
+ }
+
private void initPEM(
@UnderInitialization(WrappedFactory.class)LibPQFactory this,
String sslKeyFile, String defaultdir, Properties info) throws PSQLException {
@@ -136,12 +164,14 @@ public LibPQFactory(Properties info) throws PSQLException {
sslkeyfile = defaultdir + "postgresql.pk8";
}
- if (sslkeyfile.endsWith(".p12") || sslkeyfile.endsWith(".pfx")) {
+ if (endsWithIgnoreCase(sslkeyfile, ".p12") || endsWithIgnoreCase(sslkeyfile, ".pfx")) {
initP12(sslkeyfile, info);
- } else if (sslkeyfile.endsWith(".key") || sslkeyfile.endsWith(".pem")) {
+ } else if (endsWithIgnoreCase(sslkeyfile, ".pem")) {
initPEM(sslkeyfile, defaultdir, info);
- } else {
+ } else if (endsWithIgnoreCase(sslkeyfile, ".der")) {
initPk8(sslkeyfile, defaultdir, info);
+ } else {
+ initPk8OrPem(sslkeyfile, defaultdir, info);
}
TrustManager[] tm;
@@ -233,6 +263,9 @@ public void throwKeyManagerException() throws PSQLException {
if (km instanceof PEMKeyManager) {
((PEMKeyManager) km).throwKeyManagerException();
}
+ if (km instanceof Pk8OrPemKeyManager) {
+ ((Pk8OrPemKeyManager) km).throwKeyManagerException();
+ }
}
}
diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/Pk8OrPemKeyManager.java b/pgjdbc/src/main/java/org/postgresql/ssl/Pk8OrPemKeyManager.java
new file mode 100644
index 0000000000..7cff81ae6b
--- /dev/null
+++ b/pgjdbc/src/main/java/org/postgresql/ssl/Pk8OrPemKeyManager.java
@@ -0,0 +1,190 @@
+/*
+ * Copyright (c) 2026, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.ssl;
+
+import org.postgresql.util.PSQLException;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.X509KeyManager;
+
+/**
+ * A key manager that selects PEM when its header is found within the first 64 KiB
+ * of the key file, and PK8/DER otherwise. This preserves libpq's preference for PEM
+ * before DER, while using a bounded marker scan rather than parsing the key during
+ * format detection.
+ *
+ * <p>The delegate is resolved lazily on the first call to any {@link X509KeyManager}
+ * method, so this works regardless of which method the TLS engine calls first.</p>
+ */
+public class Pk8OrPemKeyManager implements X509KeyManager {
+
+ // PKCS#8 PEM marker (ASCII per RFC 7468) used to tell PEM from binary DER. A key file
+ // is tiny, so the scan is bounded to a small prefix and uses O(1) match state, leaving
+ // a misconfigured sslkey that points at a huge or endless file unable to exhaust memory.
+ private static final byte[] PEM_HEADER = "BEGIN PRIVATE KEY".getBytes(StandardCharsets.US_ASCII);
+ private static final int MAX_PEM_SCAN_BYTES = 64 * 1024;
+ private static final int PEM_SCAN_CHUNK_BYTES = 8 * 1024;
+
+ private final String keyFilePath;
+ private final PEMKeyManager pem;
+ private final LazyKeyManager pk8;
+ private @Nullable X509KeyManager delegate;
+ private @Nullable PSQLException permissionError;
+
+ public Pk8OrPemKeyManager(String keyFilePath, PEMKeyManager pem, LazyKeyManager pk8) {
+ this.keyFilePath = keyFilePath;
+ this.pem = pem;
+ this.pk8 = pk8;
+ }
+
+ /**
+ * Resolves which key manager to use by probing the key file format, or {@code null}
+ * if the key file has insecure permissions.
+ */
+ private @Nullable X509KeyManager delegate() {
+ X509KeyManager d = delegate;
+ if (d != null) {
+ return d;
+ }
+ if (permissionError != null) {
+ return null;
+ }
+ // Validate permissions before probing the format, otherwise the PK8/DER fallback
+ // (which does not check permissions itself) would let an insecure key file load
+ // even though the PEM path would have rejected it.
+ try {
+ if (!keyFilePath.isEmpty()) {
+ Path keyPath = Paths.get(keyFilePath);
+ if (Files.exists(keyPath)) {
+ BaseX509KeyManager.validateKeyFilePermissions(keyPath);
+ }
+ }
+ } catch (PSQLException e) {
+ permissionError = e;
+ return null;
+ }
+ // Preserve libpq's PEM-before-DER preference, but detect the format with a bounded
+ // scan for the PEM private-key header rather than calling PEMKeyManager.getPrivateKey:
+ // a failed probe there builds a translated PSQLException, and loading that message
+ // bundle in the driver's class loader pins the loader and prevents it from being
+ // unloaded.
+ d = looksLikePem() ? pem : pk8;
+ delegate = d;
+ return d;
+ }
+
+ /**
+ * Returns {@code true} if the key file is a PKCS#8 PEM file, detected by the
+ * {@code BEGIN PRIVATE KEY} marker within the first {@link #MAX_PEM_SCAN_BYTES} bytes.
+ * Binary (DER) or unreadable content is not PEM.
+ *
+ * <p>The marker is matched with a streaming scan, so it is found even when it straddles
+ * a read boundary, while memory stays bounded to one chunk. The marker's first byte
+ * ({@code 'B'}) does not recur in it, so a mismatch can only start a fresh match and no
+ * KMP-style backtracking is needed.</p>
+ */
+ private boolean looksLikePem() {
+ if (keyFilePath.isEmpty()) {
+ return false;
+ }
+ Path keyPath = Paths.get(keyFilePath);
+ if (!Files.exists(keyPath)) {
+ return false;
+ }
+ byte[] chunk = new byte[PEM_SCAN_CHUNK_BYTES];
+ int matched = 0;
+ int scanned = 0;
+ try (InputStream in = Files.newInputStream(keyPath)) {
+ int read;
+ while (scanned < MAX_PEM_SCAN_BYTES && (read = in.read(chunk)) != -1) {
+ for (int i = 0; i < read; i++) {
+ byte b = chunk[i];
+ if (b == PEM_HEADER[matched]) {
+ if (++matched == PEM_HEADER.length) {
+ return true;
+ }
+ } else {
+ matched = b == PEM_HEADER[0] ? 1 : 0;
+ }
+ }
+ scanned += read;
+ }
+ } catch (IOException e) {
+ // Unreadable file: treat as non-PEM.
+ }
+ return false;
+ }
+
+ @Override
+ public String @Nullable [] getClientAliases(String keyType,
+ Principal @Nullable [] issuers) {
+ X509KeyManager d = delegate();
+ return d == null ? null : d.getClientAliases(keyType, issuers);
+ }
+
+ @Override
+ public @Nullable String chooseClientAlias(String[] keyType,
+ Principal @Nullable [] issuers, @Nullable Socket socket) {
+ X509KeyManager d = delegate();
+ return d == null ? null : d.chooseClientAlias(keyType, issuers, socket);
+ }
+
+ @Override
+ public String @Nullable [] getServerAliases(String keyType,
+ Principal @Nullable [] issuers) {
+ X509KeyManager d = delegate();
+ return d == null ? null : d.getServerAliases(keyType, issuers);
+ }
+
+ @Override
+ public @Nullable String chooseServerAlias(String keyType,
+ Principal @Nullable [] issuers, @Nullable Socket socket) {
+ X509KeyManager d = delegate();
+ return d == null ? null : d.chooseServerAlias(keyType, issuers, socket);
+ }
+
+ @Override
+ public X509Certificate @Nullable [] getCertificateChain(String alias) {
+ X509KeyManager d = delegate();
+ return d == null ? null : d.getCertificateChain(alias);
+ }
+
+ @Override
+ public @Nullable PrivateKey getPrivateKey(String alias) {
+ X509KeyManager d = delegate();
+ return d == null ? null : d.getPrivateKey(alias);
+ }
+
+ /**
+ * Propagates any exception from the resolved delegate key manager, including an
+ * insecure-permission error detected while probing the key file.
+ *
+ * @throws PSQLException if the delegate key manager has a stored exception
+ */
+ public void throwKeyManagerException() throws PSQLException {
+ X509KeyManager d = delegate();
+ if (permissionError != null) {
+ throw permissionError;
+ }
+ if (d instanceof LazyKeyManager) {
+ ((LazyKeyManager) d).throwKeyManagerException();
+ } else if (d instanceof BaseX509KeyManager) {
+ ((BaseX509KeyManager) d).throwKeyManagerException();
+ }
+ }
+}
diff --git a/pgjdbc/src/test/java/org/postgresql/test/ssl/Pk8OrPemKeyManagerTest.java b/pgjdbc/src/test/java/org/postgresql/test/ssl/Pk8OrPemKeyManagerTest.java
new file mode 100644
index 0000000000..caa6448fdb
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/ssl/Pk8OrPemKeyManagerTest.java
@@ -0,0 +1,241 @@
+/*
+ * Copyright (c) 2026, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.ssl;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import org.postgresql.PGProperty;
+import org.postgresql.core.ServerVersion;
+import org.postgresql.ssl.LazyKeyManager;
+import org.postgresql.ssl.PEMKeyManager;
+import org.postgresql.ssl.Pk8OrPemKeyManager;
+import org.postgresql.test.TestUtil;
+import org.postgresql.util.PSQLException;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.sql.Connection;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.Set;
+
+import javax.security.auth.x500.X500Principal;
+
+/**
+ * Tests for {@link Pk8OrPemKeyManager}, which auto-detects PEM vs PK8/DER format
+ * with a bounded scan while preserving libpq's preference for PEM before DER.
+ */
+class Pk8OrPemKeyManagerTest {
+
+ private static final X500Principal TEST_ISSUER =
+ new X500Principal("CN=root certificate, O=PgJdbc test, ST=CA, C=US");
+
+ @TempDir
+ Path tempDir;
+
+ /**
+ * Copies a cert-dir key file into the per-test temp dir and restricts it to
+ * owner-only access, so it satisfies the key-file permission check. The owner-only
+ * permission cannot be expressed on a non-POSIX filesystem, so the test is skipped there.
+ */
+ private Path secureKeyCopy(String keyFileName) throws IOException {
+ Path keyFile = tempDir.resolve(keyFileName);
+ Files.copy(Paths.get(TestUtil.getSslTestCertPath(keyFileName)), keyFile);
+ assumeTrue(Files.getFileAttributeView(keyFile, PosixFileAttributeView.class) != null,
+ "Test requires a POSIX filesystem to set owner-only key permissions");
+ Set<PosixFilePermission> perms = new HashSet<>();
+ perms.add(PosixFilePermission.OWNER_READ);
+ Files.setPosixFilePermissions(keyFile, perms);
+ return keyFile;
+ }
+
+ private Pk8OrPemKeyManager createManager(String keyFileName) throws IOException {
+ Path keyFile = secureKeyCopy(keyFileName);
+ String sslCertFile = TestUtil.getSslTestCertPath("goodclient.crt");
+
+ PEMKeyManager pem = new PEMKeyManager(keyFile.toString(), sslCertFile, "RSA");
+ LazyKeyManager pk8 = new LazyKeyManager(
+ sslCertFile, keyFile.toString(),
+ new PKCS12KeyManagerTest.TestCallbackHandler(null), false);
+ return new Pk8OrPemKeyManager(keyFile.toString(), pem, pk8);
+ }
+
+ /**
+ * A DER-encoded key should fall back to PK8 after PEM detection fails.
+ * This is the regression case from issue #3942: .key was previously mapped
+ * to PEM only, causing MalformedInputException for DER keys.
+ *
+ * <p>Verifies that chooseClientAlias (typically the first method called by
+ * the TLS engine) triggers format detection and delegates to PK8.</p>
+ */
+ @Test
+ void derKeyDelegatesPk8ViaChooseClientAlias() throws Exception {
+ Pk8OrPemKeyManager km = createManager("goodclient.pk8");
+
+ // chooseClientAlias is typically the first method called during TLS handshake
+ X500Principal[] issuers = new X500Principal[]{TEST_ISSUER};
+ String alias = km.chooseClientAlias(new String[]{"RSA"}, issuers, null);
+ assertNotNull(alias, "DER key should be loadable via PK8 fallback");
+
+ // Verify the full chain works after delegation is resolved
+ X509Certificate[] chain = km.getCertificateChain("user");
+ assertNotNull(chain, "Certificate chain should be available after delegation");
+
+ PrivateKey key = km.getPrivateKey("user");
+ assertNotNull(key, "Private key should be available after delegation");
+ }
+
+ /**
+ * A PEM-encoded key should be detected as PEM regardless of extension.
+ */
+ @Test
+ void pemKeyDelegatesPemViaChooseClientAlias() throws Exception {
+ Pk8OrPemKeyManager km = createManager("goodclient.key");
+
+ X500Principal[] issuers = new X500Principal[]{TEST_ISSUER};
+ String alias = km.chooseClientAlias(new String[]{"RSA"}, issuers, null);
+ assertNotNull(alias, "PEM key should be loadable via PEM detection");
+
+ PrivateKey key = km.getPrivateKey("user");
+ assertNotNull(key, "Private key should be available after PEM delegation");
+ }
+
+ /**
+ * When getPrivateKey is called first (instead of chooseClientAlias),
+ * delegation should still work correctly for DER keys.
+ */
+ @Test
+ void derKeyDelegatesPk8ViaGetPrivateKey() throws Exception {
+ Pk8OrPemKeyManager km = createManager("goodclient.pk8");
+
+ PrivateKey key = km.getPrivateKey("user");
+ assertNotNull(key, "DER key should be loadable when getPrivateKey is called first");
+ }
+
+ /**
+ * Invalid key type should return null alias even after successful delegation.
+ */
+ @Test
+ void invalidKeyTypeReturnsNull() throws Exception {
+ Pk8OrPemKeyManager km = createManager("goodclient.key");
+
+ X500Principal[] issuers = new X500Principal[]{TEST_ISSUER};
+ String alias = km.chooseClientAlias(new String[]{"EC"}, issuers, null);
+ assertNull(alias, "EC key type should not match RSA certificate");
+ }
+
+ /**
+ * A key file with insecure permissions must be rejected rather than loaded via the
+ * PK8/DER fallback, which would otherwise bypass the PEM path's permission check.
+ */
+ @Test
+ void insecurePermissionsRejectKey() throws Exception {
+ Path keyFile = tempDir.resolve("goodclient.key");
+ Files.copy(Paths.get(TestUtil.getSslTestCertPath("goodclient.pk8")), keyFile);
+ assumeTrue(Files.getFileAttributeView(keyFile, PosixFileAttributeView.class) != null,
+ "Test requires a POSIX filesystem to express insecure permissions");
+ // 0604: world-readable, insecure regardless of whether the owner is root
+ Set<PosixFilePermission> perms = new HashSet<>();
+ perms.add(PosixFilePermission.OWNER_READ);
+ perms.add(PosixFilePermission.OWNER_WRITE);
+ perms.add(PosixFilePermission.OTHERS_READ);
+ Files.setPosixFilePermissions(keyFile, perms);
+
+ String sslCertFile = TestUtil.getSslTestCertPath("goodclient.crt");
+ PEMKeyManager pem = new PEMKeyManager(keyFile.toString(), sslCertFile, "RSA");
+ LazyKeyManager pk8 = new LazyKeyManager(
+ sslCertFile, keyFile.toString(),
+ new PKCS12KeyManagerTest.TestCallbackHandler(null), false);
+ Pk8OrPemKeyManager km = new Pk8OrPemKeyManager(keyFile.toString(), pem, pk8);
+
+ assertNull(km.getPrivateKey("user"),
+ "Key with insecure permissions must not load via PK8 fallback");
+ assertThrows(PSQLException.class, km::throwKeyManagerException,
+ "Insecure permissions must surface as a key manager exception");
+ }
+
+ /**
+ * The PEM header is detected even when it straddles the streaming scan's read
+ * boundary, i.e. the file spans more than one internal read.
+ */
+ @Test
+ void pemHeaderAcrossReadBoundaryDetected() throws Exception {
+ String pem = new String(
+ Files.readAllBytes(Paths.get(TestUtil.getSslTestCertPath("goodclient.key"))),
+ StandardCharsets.UTF_8);
+ // Blank lines are ignored by the PEM parser, so the key still loads; the padding
+ // pushes "BEGIN PRIVATE KEY" past the 8 KiB scan chunk so it spans two reads.
+ StringBuilder padded = new StringBuilder();
+ for (int i = 0; i < 8180; i++) {
+ padded.append('\n');
+ }
+ padded.append(pem);
+ Path keyFile = tempDir.resolve("padded.key");
+ Files.write(keyFile, padded.toString().getBytes(StandardCharsets.UTF_8));
+ assumeTrue(Files.getFileAttributeView(keyFile, PosixFileAttributeView.class) != null,
+ "Test requires a POSIX filesystem to set owner-only key permissions");
+ Set<PosixFilePermission> perms = new HashSet<>();
+ perms.add(PosixFilePermission.OWNER_READ);
+ Files.setPosixFilePermissions(keyFile, perms);
+
+ String sslCertFile = TestUtil.getSslTestCertPath("goodclient.crt");
+ PEMKeyManager pem2 = new PEMKeyManager(keyFile.toString(), sslCertFile, "RSA");
+ LazyKeyManager pk8 = new LazyKeyManager(
+ sslCertFile, keyFile.toString(),
+ new PKCS12KeyManagerTest.TestCallbackHandler(null), false);
+ Pk8OrPemKeyManager km = new Pk8OrPemKeyManager(keyFile.toString(), pem2, pk8);
+
+ assertNotNull(km.getPrivateKey("user"),
+ "PEM key must be detected when its header straddles the scan read boundary");
+ }
+
+ /**
+ * Integration test: a DER key file with .key extension should work through
+ * the full LibPQFactory path. This is the exact regression from issue #3942.
+ */
+ @Test
+ void derKeyWithDotKeyExtensionConnects() throws Exception {
+ TestUtil.assumeHaveMinimumServerVersion(ServerVersion.v9_5);
+ TestUtil.assumeSslTestsEnabled();
+
+ // Copy the DER key to a .key file to simulate the regression scenario
+ Path derKeyFile = tempDir.resolve("goodclient.key");
+ Files.copy(Paths.get(TestUtil.getSslTestCertPath("goodclient.pk8")), derKeyFile);
+ // The key-file permission check requires owner-only POSIX permissions, which
+ // cannot be expressed on non-POSIX filesystems, so skip the test there.
+ assumeTrue(Files.getFileAttributeView(derKeyFile, PosixFileAttributeView.class) != null,
+ "Test requires a POSIX filesystem to set owner-only key permissions");
+ Set<PosixFilePermission> perms = new HashSet<>();
+ perms.add(PosixFilePermission.OWNER_READ);
+ Files.setPosixFilePermissions(derKeyFile, perms);
+
+ Properties props = new Properties();
+ PGProperty.SSL_MODE.set(props, "prefer");
+ PGProperty.SSL_KEY.set(props, derKeyFile.toString());
+ PGProperty.SSL_CERT.set(props, TestUtil.getSslTestCertPath("goodclient.crt"));
+ PGProperty.SSL_ROOT_CERT.set(props, TestUtil.getSslTestCertPath("goodroot.crt"));
+
+ try (Connection conn = TestUtil.openDB(props)) {
+ boolean sslUsed = TestUtil.queryForBoolean(conn,
+ "SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid()");
+ assertTrue(sslUsed, "SSL should be in use with DER .key file");
+ }
+ }
+}
view thread (2+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: github://pgjdbc/pgjdbc
Cc: [email protected], [email protected]
Subject: Re: [pgjdbc/pgjdbc] PR #3946: fix: auto-detect SSL key format instead of relying on .key extension
In-Reply-To: <<[email protected]>>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox