fix: Add pkcs12 key functionality #1599

davecramer · 2019-11-01T12:26:39Z

WIP
PBE-MD5-DES might be inadequate in environments where high level of security is needed and the key is not protected
pkcs12 uses 3DES which also isn't completely adequate but is certainly more secure than MD5-DES
needs documentation
Also it is possible to use stronger encryption in java 12

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?
Does mvn checkstyle:check pass ?
Have you added your new test classes to an existing test suite in alphabetical order?

Changes to Existing Features:

Does this break existing behaviour? If so please explain.
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

WIP PBE-MD5-DES might be inadequate in environments where high level of security is needed and the key is not protected pkcs12 uses 3DES which also isn't completely adequate but is certainly more secure than MD5-DES

AppVeyorBot · 2019-11-01T12:45:22Z

✅ Build pgjdbc 1.0.494 completed (commit 40d7cb7a51 by @davecramer)

codecov-io · 2019-11-01T13:16:03Z

Codecov Report

Merging #1599 into master will decrease coverage by 0.03%.
The diff coverage is 46.15%.

@@             Coverage Diff              @@
##             master    #1599      +/-   ##
============================================
- Coverage     68.87%   68.83%   -0.04%     
- Complexity     3983     4005      +22     
============================================
  Files           179      180       +1     
  Lines         16624    16699      +75     
  Branches       2708     2720      +12     
============================================
+ Hits          11450    11495      +45     
- Misses         3911     3945      +34     
+ Partials       1263     1259       -4

AppVeyorBot · 2019-11-01T13:53:45Z

✅ Build pgjdbc 1.0.495 completed (commit 56e9b20a39 by @davecramer)

vlsi · 2020-02-13T11:29:29Z

pgjdbc/src/main/java/org/postgresql/ssl/PKCS12KeyManager.java

+      // Sending a wrong certificate makes the connection rejected, even, if clientcert=0 in
+      // pg_hba.conf.
+      // therefore we only send our certificate, if the issuer is listed in issuers
+      X509Certificate[] certchain = getCertificateChain("user");


Is there a good reason to hard-code this literal?
I think it must be moved to a static final constant with a comment then.

no good reason

vlsi · 2020-02-13T11:31:03Z

pgjdbc/src/test/java/org/postgresql/test/ssl/LazyKeyManagerTest.java

+  @Test
+  public void testLoadP12Key() throws Exception {
+    String certdir = "../certdir/";
+    PKCS12KeyManager pkcs12KeyManager = new PKCS12KeyManager(certdir + "goodclient.p12", new TestCallbackHandler("sslpwd"));


I guess it is better to take sslpwd from the properties file.
Otherwise, the failures would be hard to analyze.

vlsi · 2020-02-13T11:32:38Z

pgjdbc/src/test/java/org/postgresql/test/ssl/LazyKeyManagerTest.java

+  public void testLoadP12Key() throws Exception {
+    String certdir = "../certdir/";
+    PKCS12KeyManager pkcs12KeyManager = new PKCS12KeyManager(certdir + "goodclient.p12", new TestCallbackHandler("sslpwd"));
+    PrivateKey pk = pkcs12KeyManager.getPrivateKey("user");


It is unfortunate that getPrivateKey does not throw exceptions.
In other words, this method silently returns null, and the exception is not visible to the caller :(

ya, very hard to debug

fix: Add pkcs12 key functionality

3482a91

WIP PBE-MD5-DES might be inadequate in environments where high level of security is needed and the key is not protected pkcs12 uses 3DES which also isn't completely adequate but is certainly more secure than MD5-DES

fix docs to add PKCS12 archive format

8900e35

davecramer merged commit 82c2008 into pgjdbc:master Nov 29, 2019

davecramer deleted the encrypt branch November 29, 2019 15:01

vlsi reviewed Feb 13, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Add pkcs12 key functionality #1599

fix: Add pkcs12 key functionality #1599

davecramer commented Nov 1, 2019

AppVeyorBot commented Nov 1, 2019

codecov-io commented Nov 1, 2019 •

edited

AppVeyorBot commented Nov 1, 2019

vlsi Feb 13, 2020 •

edited

davecramer Feb 13, 2020

vlsi Feb 13, 2020

davecramer Feb 13, 2020

vlsi Feb 13, 2020

davecramer Feb 13, 2020

fix: Add pkcs12 key functionality #1599

fix: Add pkcs12 key functionality #1599

Conversation

davecramer commented Nov 1, 2019

All Submissions:

New Feature Submissions:

Changes to Existing Features:

AppVeyorBot commented Nov 1, 2019

codecov-io commented Nov 1, 2019 • edited

Codecov Report

AppVeyorBot commented Nov 1, 2019

vlsi Feb 13, 2020 • edited

Choose a reason for hiding this comment

davecramer Feb 13, 2020

Choose a reason for hiding this comment

vlsi Feb 13, 2020

Choose a reason for hiding this comment

davecramer Feb 13, 2020

Choose a reason for hiding this comment

vlsi Feb 13, 2020

Choose a reason for hiding this comment

davecramer Feb 13, 2020

Choose a reason for hiding this comment

codecov-io commented Nov 1, 2019 •

edited

vlsi Feb 13, 2020 •

edited