fix issue-2530 encrypted private keys is not working issue#2532
fix issue-2530 encrypted private keys is not working issue#2532muskaan62 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
@lilgreenbird FYI |
|
thanks the team will take a look |
|
Thanks @lilgreenbird @Jeffery-Wasty can we expect this fix in 12.8.1 version or it will be available with new version of driver? |
|
@muskaan62 please see above you will need to agree to CLA before the PR can be considered and tests run on it |
| import org.bouncycastle.openssl.jcajce.JceOpenSSLPKCS8DecryptorProviderBuilder; | ||
| import org.bouncycastle.operator.InputDecryptorProvider; | ||
| import org.bouncycastle.operator.OperatorCreationException; | ||
| import org.bouncycastle.pkcs.PKCS8EncryptedPrivateKeyInfo; | ||
| import org.bouncycastle.pkcs.PKCSException; |
There was a problem hiding this comment.
You'll need to fix the issue without using Bouncy Castle. Try to use java.security instead. We want to limit Bouncy Castle use to Java 8 and drop it when Java 8 support ends.
There was a problem hiding this comment.
@David-Engel Thanks..The existing code is already using bouncy castle for private keys, i've just added one more if condition for encrypted keys.
There was a problem hiding this comment.
I understand that. But the existing code is only used within the context of the Always Encrypted with secure enclaves feature with Java 8 only, (https://github.com/microsoft/mssql-jdbc/blob/main/README.md#dependencies)
We can't extend the use of Bouncy Castle for other contexts/features without approval from our security team, as they didn't like the use of an external encryption library at all. We only got approval because it would be dropped when support for Java 8 ends.
If you have to limit the fix to Java 11+ only, I'm fine with that. We just need to update the docs appropriately.
There was a problem hiding this comment.
Thanks @David-Engel I tried to use java security but looks with java 11 we do not have security providers to decrypt the keys and has to fall back on bouncy castle only...Yes, i want to limit this fix to java 11 only. please do lemme know if anything else needs to be done be from my end.
There was a problem hiding this comment.
To clarify, I meant: If you have to limit this fix to Java 11+ only without using Bouncy Castle, I'm fine with that. I was assuming Bouncy Castle was only required for Java 8.
We can't extend our usage of Bouncy Castle to Java 11.
Can you fix it for Java 17+ without Bouncy Castle?
There was a problem hiding this comment.
Bouncy Castle has security issues it is not recommended to be used and is not approved by Microsoft. We are looking into a solution that does not require Bouncy Castle.
There was a problem hiding this comment.
@lilgreenbird , thanks for the reply.
Can you pls help me more details around the security issues being present in the bouncy castle library? That will help me to see if I also need to have BC in my app or not. The more details around it would be appreciated.
There was a problem hiding this comment.
Any response here @lilgreenbird or @David-Engel ?
There was a problem hiding this comment.
I'm not aware of any security issues in BC. For us in Microsoft, it's just not an approved encryption library (barring limited exceptions) for use in MS products.
There was a problem hiding this comment.
Thanks @David-Engel If i Understood correctly, then you are saying to fix this issue for java 17+ and the issue will still exist for java 8 and java 11?? We are using Java 11 in our applications and require a fix that works specifically for this version. I attempted to address the issue using the Java Security API, but encountered errors related to the provider. Upon investigation, it appears these errors stem from a known bug in JDK 11 (JDK-8231581) that remains unresolved. we need a solution for Java 11. Most suggestions I found online recommend using Bouncy Castle. If you have any references or approaches to resolve this for Java 11, I would be glad to explore them. Thank you for your help!
@muskaan62 I'm just saying we can't expand the scenarios under which we use BC. If this means the fix is limited, so be it.
|
@microsoft-github-policy-service agree company="Gaian Solutions" |
@lilgreenbird Done, lemme know anything else to be done from my end. |
|
I'm also encountering the same issue regarding to the encrypted private key when using |
|
The PR is still open, for now i have the workaround from my end by using keystore to put the certificate key and when getting the key back from keystore iam getting unencrypted key. |
client certificate without password (the test you provide for this scenario) [working]
client certificate with password [working]
client certificate with private key [working]
client certificate with private key and key password (i.e means encrypted key) [bug]
The above 3 ways are working fine but fourth is not ..
The logic for decrypt the private key is restricted to pvk though providing pvk key also its not working types and RSA private keys when decrypting also failing so added a fix for this. related to https://github.com/microsoft/mssql-jdbc/issues/2530