New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Add pkcs12 key functionality #1599
Conversation
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
✅ Build pgjdbc 1.0.494 completed (commit 40d7cb7a51 by @davecramer) |
Codecov Report
@@ 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 |
✅ Build pgjdbc 1.0.495 completed (commit 56e9b20a39 by @davecramer) |
// 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"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no good reason
@Test | ||
public void testLoadP12Key() throws Exception { | ||
String certdir = "../certdir/"; | ||
PKCS12KeyManager pkcs12KeyManager = new PKCS12KeyManager(certdir + "goodclient.p12", new TestCallbackHandler("sslpwd")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it is better to take sslpwd
from the properties file.
Otherwise, the failures would be hard to analyze.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
public void testLoadP12Key() throws Exception { | ||
String certdir = "../certdir/"; | ||
PKCS12KeyManager pkcs12KeyManager = new PKCS12KeyManager(certdir + "goodclient.p12", new TestCallbackHandler("sslpwd")); | ||
PrivateKey pk = pkcs12KeyManager.getPrivateKey("user"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya, very hard to debug
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:
New Feature Submissions:
Changes to Existing Features: