RFR: 8296442: EncryptedPrivateKeyInfo can be created with an uninitialized AlgorithmParameters [v4]

[Sean Mullan]

On Wed, 16 Nov 2022 03:41:11 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> An `EncryptedPrivateKeyInfo` object can be created with an uninitialized `AlgorithmParameters`, but before you call `getEncoded` on it you need to remember to initialize the params. This is unfortunate but since this is a public API, I hesitate to make a change.
>> 
>> Instead, this code change fixes the much more widely used internal class `AlgorithmId` so that it cannot be created with an uninitialized `AlgorithmParameters`. `EncryptedPrivateKeyInfo` now works with both initialized and uninitialized params, and it's immutable.
>> 
>> No intention to make `AlgorithmId` immutable this time. It has a child class named `AlgIdDSA` which makes things complicated.
>
> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comment and exception message

Did you consider changing `EncryptedPrivateKeyInfo(...,AlgorithmParameters)` to throw `IllegalStateException` if the parameters were not initialized? I know you said you were worried about changing the API, but it would be a cleaner option. I wonder if there is really any code that is initializing the parameters after creating the EPKI.

src/java.base/share/classes/sun/security/x509/AlgorithmId.java line 103:

> 101:      * @param oid the identifier for the algorithm.
> 102:      * @param algparams the associated algorithm parameters, can be null.
> 103:      * @exception IllegalStateException if algparams is not initialized

add "or cannot be decoded"

src/java.base/share/classes/sun/security/x509/AlgorithmId.java line 493:

> 491:      * @param algparams the associated algorithm parameters.
> 492:      * @exception NoSuchAlgorithmException on error.
> 493:      * @exception IllegalStateException if algparams is not initialized

Add "or cannot be decoded"

test/jdk/javax/crypto/EncryptedPrivateKeyInfo/GetAlgName.java line 64:

> 62:             }
> 63: 
> 64:             AlgorithmParameters ap2 = AlgorithmParameters.getInstance(ap.getAlgorithm());

It would be useful to add a comment here that you are testing that an EPKI can be created with an uninitialized AP.

test/jdk/sun/security/x509/AlgorithmId/Uninitialized.java line 35:

> 33: import java.security.AlgorithmParameters;
> 34: 
> 35: public class Uninitialized {

Is this test necessary? It seems to be duplicating the additional test you added to GetAlgName.java

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

PR: https://git.openjdk.org/jdk/pull/11067