Skip to content

Commit

Permalink
8296442: EncryptedPrivateKeyInfo can be created with an uninitialized…
Browse files Browse the repository at this point in the history
… AlgorithmParameters

Reviewed-by: xuelei, kdriver, mullan
  • Loading branch information
wangweij committed Nov 16, 2022
1 parent 37848a9 commit 68d3ed5
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,20 @@

public class EncryptedPrivateKeyInfo {

// the "encryptionAlgorithm" field
// The "encryptionAlgorithm" is stored in either the algid or
// the params field. Precisely, if this object is created by
// {@link #EncryptedPrivateKeyInfo(AlgorithmParameters, byte[])}
// with an uninitialized AlgorithmParameters, the AlgorithmParameters
// object is stored in the params field and algid is set to null.
// In all other cases, algid is non null and params is null.
private final AlgorithmId algid;

// the algorithm name of the encrypted private key
private String keyAlg;
private final AlgorithmParameters params;

// the "encryptedData" field
private final byte[] encryptedData;

// the ASN.1 encoded contents of this class
private byte[] encoded;
private final byte[] encoded;

/**
* Constructs (i.e., parses) an {@code EncryptedPrivateKeyInfo} from
Expand Down Expand Up @@ -100,6 +103,7 @@ public EncryptedPrivateKeyInfo(byte[] encoded) throws IOException {
}

this.algid = AlgorithmId.parse(seq[0]);
this.params = null;
if (seq[0].data.available() != 0) {
throw new IOException("encryptionAlgorithm field overrun");
}
Expand Down Expand Up @@ -141,6 +145,7 @@ public EncryptedPrivateKeyInfo(String algName, byte[] encryptedData)
throw new NullPointerException("the algName parameter " +
"must be non-null");
this.algid = AlgorithmId.get(algName);
this.params = null;

if (encryptedData == null) {
throw new NullPointerException("the encryptedData " +
Expand Down Expand Up @@ -181,7 +186,22 @@ public EncryptedPrivateKeyInfo(AlgorithmParameters algParams,
if (algParams == null) {
throw new NullPointerException("algParams must be non-null");
}
this.algid = AlgorithmId.get(algParams);
AlgorithmId tmp;
try {
tmp = AlgorithmId.get(algParams);
} catch (IllegalStateException e) {
// This exception is thrown when algParams.getEncoded() fails.
// While the spec of this constructor requires that
// "getEncoded should return...", in reality people might
// create with an uninitialized algParams first and only
// initialize it before calling getEncoded(). Thus we support
// this case as well.
tmp = null;
}

// one and only one is non null
this.algid = tmp;
this.params = this.algid != null ? null : algParams;

if (encryptedData == null) {
throw new NullPointerException("encryptedData must be non-null");
Expand All @@ -197,7 +217,6 @@ public EncryptedPrivateKeyInfo(AlgorithmParameters algParams,
this.encoded = null;
}


/**
* Returns the encryption algorithm.
* <p>Note: Standard name is returned instead of the specified one
Expand All @@ -209,15 +228,15 @@ public EncryptedPrivateKeyInfo(AlgorithmParameters algParams,
* @return the encryption algorithm name.
*/
public String getAlgName() {
return this.algid.getName();
return algid == null ? params.getAlgorithm() : algid.getName();
}

/**
* Returns the algorithm parameters used by the encryption algorithm.
* @return the algorithm parameters.
*/
public AlgorithmParameters getAlgParameters() {
return this.algid.getParameters();
return algid == null ? params : algid.getParameters();
}

/**
Expand Down Expand Up @@ -252,14 +271,13 @@ public PKCS8EncodedKeySpec getKeySpec(Cipher cipher)
byte[] encoded;
try {
encoded = cipher.doFinal(encryptedData);
checkPKCS8Encoding(encoded);
return pkcs8EncodingToSpec(encoded);
} catch (GeneralSecurityException |
IOException |
IllegalStateException ex) {
throw new InvalidKeySpecException(
"Cannot retrieve the PKCS8EncodedKeySpec", ex);
}
return new PKCS8EncodedKeySpec(encoded, keyAlg);
}

private PKCS8EncodedKeySpec getKeySpecImpl(Key decryptKey,
Expand All @@ -270,21 +288,20 @@ private PKCS8EncodedKeySpec getKeySpecImpl(Key decryptKey,
try {
if (provider == null) {
// use the most preferred one
c = Cipher.getInstance(algid.getName());
c = Cipher.getInstance(getAlgName());
} else {
c = Cipher.getInstance(algid.getName(), provider);
c = Cipher.getInstance(getAlgName(), provider);
}
c.init(Cipher.DECRYPT_MODE, decryptKey, algid.getParameters());
c.init(Cipher.DECRYPT_MODE, decryptKey, getAlgParameters());
encoded = c.doFinal(encryptedData);
checkPKCS8Encoding(encoded);
return pkcs8EncodingToSpec(encoded);
} catch (NoSuchAlgorithmException nsae) {
// rethrow
throw nsae;
} catch (GeneralSecurityException | IOException ex) {
throw new InvalidKeyException(
"Cannot retrieve the PKCS8EncodedKeySpec", ex);
}
return new PKCS8EncodedKeySpec(encoded, keyAlg);
}

/**
Expand Down Expand Up @@ -388,14 +405,23 @@ public byte[] getEncoded() throws IOException {
DerOutputStream tmp = new DerOutputStream();

// encode encryption algorithm
algid.encode(tmp);
if (algid != null) {
algid.encode(tmp);
} else {
try {
// Let's hope params has been initialized by now.
AlgorithmId.get(params).encode(tmp);
} catch (Exception e) {
throw new IOException("not initialized", e);
}
}

// encode encrypted data
tmp.putOctetString(encryptedData);

// wrap everything into a SEQUENCE
out.write(DerValue.tag_Sequence, tmp);
this.encoded = out.toByteArray();
return out.toByteArray();
}
return this.encoded.clone();
}
Expand All @@ -409,7 +435,7 @@ private static void checkTag(DerValue val, byte tag, String valName)
}

@SuppressWarnings("fallthrough")
private void checkPKCS8Encoding(byte[] encodedKey)
private static PKCS8EncodedKeySpec pkcs8EncodingToSpec(byte[] encodedKey)
throws IOException {
DerInputStream in = new DerInputStream(encodedKey);
DerValue[] values = in.getSequence(3);
Expand All @@ -420,9 +446,9 @@ private void checkPKCS8Encoding(byte[] encodedKey)
/* fall through */
case 3:
checkTag(values[0], DerValue.tag_Integer, "version");
keyAlg = AlgorithmId.parse(values[1]).getName();
String keyAlg = AlgorithmId.parse(values[1]).getName();
checkTag(values[2], DerValue.tag_OctetString, "privateKey");
break;
return new PKCS8EncodedKeySpec(encodedKey, keyAlg);
default:
throw new IOException("invalid key encoding");
}
Expand Down
30 changes: 0 additions & 30 deletions src/java.base/share/classes/sun/security/pkcs12/MacData.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,6 @@ class MacData {

}

MacData(AlgorithmParameters algParams, byte[] digest,
byte[] salt, int iterations) throws NoSuchAlgorithmException
{
if (algParams == null)
throw new NullPointerException("the algParams parameter " +
"must be non-null");

AlgorithmId algid = AlgorithmId.get(algParams);
this.digestAlgorithmName = algid.getName();
this.digestAlgorithmParams = algid.getParameters();

if (digest == null) {
throw new NullPointerException("the digest " +
"parameter must be non-null");
} else if (digest.length == 0) {
throw new IllegalArgumentException("the digest " +
"parameter must not be empty");
} else {
this.digest = digest.clone();
}

this.macSalt = salt;
this.iterations = iterations;

// delay the generation of ASN.1 encoding until
// getEncoded() is called
this.encoded = null;

}

String getDigestAlgName() {
return digestAlgorithmName;
}
Expand Down
18 changes: 8 additions & 10 deletions src/java.base/share/classes/sun/security/x509/AlgorithmId.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ public AlgorithmId(ObjectIdentifier oid) {
*
* @param oid the identifier for the algorithm.
* @param algparams the associated algorithm parameters, can be null.
* @exception IllegalStateException if algparams is not initialized
* or cannot be encoded
*/
public AlgorithmId(ObjectIdentifier oid, AlgorithmParameters algparams) {
algid = oid;
Expand All @@ -108,9 +110,9 @@ public AlgorithmId(ObjectIdentifier oid, AlgorithmParameters algparams) {
try {
encodedParams = algParams.getEncoded();
} catch (IOException ioe) {
// Ignore this at the moment. This exception can occur
// if AlgorithmParameters was not initialized yet. Will
// try to re-getEncoded() again later.
throw new IllegalStateException(
"AlgorithmParameters not initialized or cannot be decoded",
ioe);
}
}
}
Expand Down Expand Up @@ -157,17 +159,11 @@ protected void decodeParams() throws IOException {
* @exception IOException on encoding error.
*/
@Override
public void encode (DerOutputStream out) throws IOException {
public void encode(DerOutputStream out) throws IOException {
DerOutputStream bytes = new DerOutputStream();

bytes.putOID(algid);

// Re-getEncoded() from algParams if it was not initialized
if (algParams != null && encodedParams == null) {
encodedParams = algParams.getEncoded();
// If still not initialized. Let the IOE be thrown.
}

if (encodedParams == null) {
// Changes backed out for compatibility with Solaris

Expand Down Expand Up @@ -488,6 +484,8 @@ public static AlgorithmId get(String algname)
*
* @param algparams the associated algorithm parameters.
* @exception NoSuchAlgorithmException on error.
* @exception IllegalStateException if algparams is not initialized
* or cannot be encoded
*/
public static AlgorithmId get(AlgorithmParameters algparams)
throws NoSuchAlgorithmException {
Expand Down
23 changes: 14 additions & 9 deletions test/jdk/javax/crypto/EncryptedPrivateKeyInfo/GetAlgName.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2007, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -23,16 +23,11 @@

/**
* @test
* @bug 4941596
* @bug 4941596 8296442
* @summary Test the EncryptedPrivateKeyInfo.getAlgName(...) methods.
* @author Valerie Peng
*/
import java.util.*;
import java.nio.*;
import java.io.*;
import java.security.*;
import java.util.Arrays;
import java.security.spec.*;
import javax.crypto.*;
import javax.crypto.spec.*;

Expand Down Expand Up @@ -61,8 +56,18 @@ public static void main(String[] argv) throws Exception {
AlgorithmParameters ap = c.getParameters();
epki = new EncryptedPrivateKeyInfo(ap, BYTES);
if (!epki.getAlgName().equalsIgnoreCase(algo)) {
System.out.println("...expect: " + algo);
System.out.println("...got: " + epki.getAlgName());
System.out.println("...expected: " + algo);
System.out.println(" ...got: " + epki.getAlgName());
status = false;
}

// Make sure EncryptedPrivateKeyInfo can be created with an
// uninitialized AlgorithmParameters.
AlgorithmParameters ap2 = AlgorithmParameters.getInstance(ap.getAlgorithm());
epki = new EncryptedPrivateKeyInfo(ap2, BYTES);
if (!epki.getAlgName().equalsIgnoreCase(algo)) {
System.out.println("...expected: " + algo);
System.out.println(" ...got: " + epki.getAlgName());
status = false;
}
}
Expand Down
12 changes: 10 additions & 2 deletions test/jdk/javax/crypto/EncryptedPrivateKeyInfo/GetEncoded.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -23,7 +23,7 @@

/**
* @test
* @bug 8261779
* @bug 8261779 8296442
* @summary Check that EncryptedPrivateKeyInfo.getEncoded() calls
* AlgorithmParameters.getEncoded() when first called
*/
Expand Down Expand Up @@ -56,6 +56,14 @@ public static void main(String[] argv) throws Exception {
AlgorithmParameters ap1 = AlgorithmParameters.getInstance("EC");
EncryptedPrivateKeyInfo epki1 =
new EncryptedPrivateKeyInfo(ap1, new byte[] {1, 2, 3, 4});

try {
epki1.getEncoded();
throw new Exception("Should have thrown IOException");
} catch (IOException ioe) {
// test passed, expected exception
}

ap1.init(new ECGenParameterSpec("secp256r1"));

EncryptedPrivateKeyInfo epki2 =
Expand Down
49 changes: 49 additions & 0 deletions test/jdk/sun/security/x509/AlgorithmId/Uninitialized.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8296442
* @summary EncryptedPrivateKeyInfo can be created with an uninitialized AlgorithmParameters
* @modules java.base/sun.security.x509
*/

import sun.security.x509.AlgorithmId;

import java.security.AlgorithmParameters;

public class Uninitialized {
public static void main(String[] args) throws Exception {
AlgorithmParameters ap = AlgorithmParameters.getInstance("EC");
boolean success;
try {
AlgorithmId.get(ap);
success = true;
} catch (Exception e) {
success = false;
}
if (success) {
throw new RuntimeException();
}
}
}

1 comment on commit 68d3ed5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.