Subject: [pgjdbc/pgjdbc] PR #3700: Add PEMKeyManager to handle PEM based certs and keys.

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Mon, 30 Jun 2025 20:04:07 +0000

### Context

I found that PGJDBC currently lacks support for PEM based certs and keys.

We have a use case where PEM files are auto renewed on disk and
converting them to DER format requires running something that watches
files on disk and auto-converts to DER.

Hence I would like to propose a patch for supporting PEM based certs, keys.

### New Feature Submissions:

1. [x] Does your submission pass tests?
2. [x] Does `./gradlew styleCheck` pass ?
3. [x] Have you added your new test classes to an existing test suite in alphabetical order?

### Changes to Existing Features:

- Adding support for PEM based certs will not affect the existing behavior.
- Added new tests for PEMKeyManager

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 01 Jul 2025 18:52:51 +0000

(on pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java)

this should not be necessary, since you set a default value

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 01 Jul 2025 18:57:24 +0000

Why do we need a `PEM_KEY_ALGORITHM("pemKeyAlgorithm", "RSA", "Algorithm of the PEM key")` setting when the `PemKeyManager` only supports `RSA` ?

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Tue, 01 Jul 2025 21:11:17 +0000

initially I didn't have this, but one of the checks in  PRB started failing (My local setup gradle build was fine).
Hence I had to add this line.

Probably I can make the `algorithm` in PEMKeyManager constructor as nullable, but I didn't prefer it.

So, i believe adding this line is kind of necessary because `getOrDefault` returns nullable.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Tue, 01 Jul 2025 21:13:17 +0000

>Why do we need a PEM_KEY_ALGORITHM("pemKeyAlgorithm", "RSA", "Algorithm of the PEM key") setting when the PemKeyManager only supports RSA ?

@davecramer `PemKeyManager` should also work for `EC` algorithm, along with `RSA`. So, I had to introduce a new setting.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 01 Jul 2025 21:18:30 +0000

I presume it failed because it was returning an @Nullable you can cast it to notNull to fix it. You set a default in PEM_KEY_ALGORITHM so it will not return null

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 01 Jul 2025 21:22:57 +0000

> @davecramer `PemKeyManager` should also work for `EC` algorithm, along with `RSA`. So, I had to introduce a new setting.

Sorry, you are correct.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Wed, 02 Jul 2025 04:18:40 +0000

@davecramer sure, i typecasted to nonNull value, appreciate if anyone can approve the 4 pending PRB workflows, so that it can go through.

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Wed, 02 Jul 2025 12:05:14 +0000

PEMKeyManager reads its material from file every time. This is different from the existing key managers (`PKCS12KeyManager`, `LazyKeyManager`) which only reads from file once.
I think the behaviour should be consistent across all key managers, anything else would be confusing. Should the existing key managers be updated to always read from file to allow reloading/auto renewal as well?

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Wed, 02 Jul 2025 20:05:42 +0000

@cfredri4 as the `getPrivateKey` method is called during the SSL Handshake, I believe the read happens during the connection establishment , and there might NOT be any reads further in the entire lifetime of connection.

Subsequent new connections anyways should create new objects of `LibPQFactory` and `KeyManagers` which will trigger reads again.

In case if a cert expires during the lifetime of a connection, probably the connection terminates and that will lead to creation of new one.

So, do you think reading the material  from file every time (or caching the content) cause any issues ?

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Thu, 03 Jul 2025 09:03:34 +0000

> @cfredri4 as the `getPrivateKey` method is called during the SSL Handshake, I believe the read happens during the connection establishment , and there might NOT be any reads further in the entire l…

Correct.

> Subsequent new connections anyways should create new objects of `LibPQFactory` and `KeyManagers` which will trigger reads again.

You're right, I missed this part.
This means that there is really no point that the existing key managers (`PKCS12KeyManager`, `LazyKeyManager`) cache the key material and only read once.

> In case if a cert expires during the lifetime of a connection, probably the connection terminates and that will lead to creation of new one.

Of topic, but in general this does _not_ happen in TLS; certificate expiry is checked only during handshake so any connection will remain active when the certificate expires.

> So, do you think reading the material from file every time (or caching the content) cause any issues ?

No real issue. I only reacted to that it was done differently from the existing key managers. For consistency maybe the existing key managers should be updated to always read from file, it would slightly simplify things.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 13:14:40 +0000

(on pgjdbc/src/main/java/org/postgresql/ssl/PEMKeyManager.java)

Should it stop processing the contents after `END PRIVATE KEY`?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 13:20:20 +0000

(on pgjdbc/src/main/java/org/postgresql/ssl/PEMKeyManager.java:70)

WDYT of adding `Arrays.fill(privateKeyDERBytes, 0)` and the corresponding zeroing of `keyContent` the  to reduce the retention of the secret keys in memory?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 13:21:59 +0000

Does this duplicate `PKCS12KeyManager` logic? Have you considered de-duplicating it?

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Thu, 03 Jul 2025 17:01:11 +0000

thanks for bringing this up !

Yes, I created `BaseX509KeyManager` to have the common logic between `PKCS12KeyManager ` and `PEMKeyManager` extends from it.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Thu, 03 Jul 2025 17:02:39 +0000

good point, 

I updated the code to zero the content of  `privateKeyDERBytes `, `keyContent`  to prevent security concerns.

Thank you

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 17:04:54 +0000

(on pgjdbc/src/main/java/org/postgresql/ssl/BaseX509KeyManager.java)

I'm not a lawyer but having copyright here looks a bit weird. Did you intend adding "copyright Apple"?

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Thu, 03 Jul 2025 17:05:34 +0000

sure, thats doable, if thats preferred. I updated the code to stop processing after `END PRIVATE KEY`. Thanks.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Thu, 03 Jul 2025 17:07:54 +0000

ohh, thanks for catching this, this is due to my editor settings, removed it :)

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Tue, 08 Jul 2025 21:25:36 +0000

@vlsi I have resolved you comments, can you please take a look now ? thanks.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Wed, 30 Jul 2025 18:23:30 +0000

hello @davecramer @vlsi 
Just a gentle reminder to take a look at this PR when you get a chance. Appreciate your time and feedback! 
Thank you.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Wed, 13 Aug 2025 20:52:36 +0000

git found some small conflict, so just rebased my changes and resolved it.

---

From: "soumava (@soumava)" <[email-masked]>
Date: Sun, 28 Sep 2025 19:48:51 +0000

Thanks for the PR @harinath001. I'm hoping we can start using PEM formatted certificates with PGJDBC soon! 

Are we waiting for any further reviews/approvals for this PR? cc @davecramer @vlsi if not, is there a preferred alternative approach to be implementing this?

---

From: "ZwS (@ZwS)" <[email-masked]>
Date: Tue, 28 Oct 2025 08:58:44 +0000

I'm looking forward this PR will be merged. Support of the PEM certs in the driver will reduce the hustle with change monitoring and certificates converting.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Mon, 03 Nov 2025 20:55:48 +0000

We need to ensure that private key has owner only read permissions

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Mon, 03 Nov 2025 23:16:13 +0000

thanks @davecramer for the comment. I added the code to verify the key file permissions before reading it.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Tue, 04 Nov 2025 18:13:56 +0000

@davecramer I have added new code for checking the file permissions (had to handle posix + windows) but wondering if this kind of checks already happening in other places ? Would like to reuse it if possible. Thanks.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Mon, 10 Nov 2025 19:36:01 +0000

@davecramer 
i see that the file permissions of the existing key files committed in the repo don't have the correct permissions.
```
-rw-r--r--@ 1 harinath  staff  1570 Nov 10 11:24 badclient.crt
-rw-r--r--@ 1 harinath  staff  1704 Nov 10 11:24 badclient.key
-rw-r--r--@ 1 harinath  staff  2940 Nov 10 11:24 badclient.p12
-rw-r--r--@ 1 harinath  staff  1956 Nov 10 11:24 badroot.crt
-rw-r--r--@ 1 harinath  staff  3446 Nov 10 11:24 badroot.key
-rw-r--r--@ 1 harinath  staff    41 Nov 10 11:24 badroot.srl
-rw-r--r--@ 1 harinath  staff  1570 Nov 10 11:24 goodclient.crt
-rw-r--r--@ 1 harinath  staff  1704 Nov 10 11:24 goodclient.key
-rw-r--r--@ 1 harinath  staff  2940 Nov 10 11:24 goodclient.p12
-rw-r--r--@ 1 harinath  staff  1956 Nov 10 11:24 goodroot.crt
-rw-r--r--@ 1 harinath  staff  2932 Nov 10 11:31 Makefile
-rw-r--r--@ 1 harinath  staff  1353 Nov 10 11:24 README.md
drwxr-xr-x@ 8 harinath  staff   256 Nov 10 11:24 server
```

We can update the `Makefile` to ensure `goodclient.key` have owner-read-only permissions. But I am wondering whats the way to change the permissions of the existing files. 

It seems one of the test case is failing due to the new check to verify the file permissions we added.

```
at platform/[email-masked]/java.sql.DriverManager.getConnection(DriverManager.java:191)
      at app//org.postgresql.test.TestUtil.openDB(TestUtil.java:358)
      at app//org.postgresql.test.ssl.PEMKeyManagerTest.TestGoodClientPEM(PEMKeyManagerTest.java:60)
      Caused by: org.postgresql.util.PSQLException: Private key file "/home/runner/work/pgjdbc/pgjdbc/pgjdbc/../certdir/goodclient.key" has insecure permissions. Permissions for group and other must be revoked. Current permissions: rw-r--r--
```

can we update the file permissions of these files to fix the failure ?

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 11 Nov 2025 13:12:08 +0000

seems git doesn't allow one to set permissions. We can add a task to make sure the permissions are correct before running the test.

---

From: "harinath001 (@harinath001)" <[email-masked]>
Date: Tue, 11 Nov 2025 21:24:13 +0000

ok, i have added a step to change the permissions of the key file `goodclient.key` `owner read-only` before the test.
please take a look, thanks.