RFR: 8298420: PEM API: Implementation (Preview) [v2]

[Weijun Wang]

On Mon, 29 Jul 2024 21:03:41 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/util/KeyUtil.java line 468:
>> 
>>> 466:         value = is.getDerValue();
>>> 467:         // This route is for:  RSAPublic, Encrypted RSAPrivate, EC Public,
>>> 468:         // Encrypted EC Private,
>> 
>> This looks a little too smart. I see it's only used by PKCS#8 and X.509 keys. Can we instead add 2 static methods in those 2 classes?
>
> I do not understand this comment.  Have two copies of this method in PKCS8EKS and X509EKS?  This sounds like duplicate code to me.

Maybe this is a similar opinion on the line endings. I don't want to be too flexible too soon.

>> src/java.base/share/classes/sun/security/util/Pem.java line 104:
>> 
>>> 102:     public static final String DEFAULT_ALGO;
>>> 103:     static {
>>> 104:         DEFAULT_ALGO = Security.getProperty("jdk.epkcs8.defaultAlgorithm");
>> 
>> Do you want to fail if the security property is not defined?
>
> I assuming you are suggesting to check when the property is used?  Having an error message when loading would probably be a customer call generator.
> PEM already throws an exception if the property is not set, EPKI.encryptKey() does not.  I could add one to the latter.

I'm afraid that someone will overwrite the java.security file and remove that property. Then wonder what will happen and if the error message is clear.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r1696058837
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r1696059545