8296736: Some PKCS9Attribute can be created but cannot be encoded

Reviewed-by: xuelei, valeriep

src/java.base/share/classes/sun/security/pkcs/PKCS9Attribute.java

@@ -378,6 +378,12 @@ public class PKCS9Attribute implements DerEncoder {
         this.oid = oid;
         index = indexOf(oid, PKCS9_OIDS, 1);
         Class<?> clazz = index == -1 ? BYTE_ARRAY_CLASS: VALUE_CLASSES[index];
+        if (clazz == null) {
+            throw new IllegalArgumentException(
+                    "No value class supported " +
+                            " for attribute " + oid +
+                            " constructing PKCS9Attribute");
+        }
         if (!clazz.isInstance(value)) {
                 throw new IllegalArgumentException(
                            "Wrong value class " +
@@ -597,20 +603,20 @@ public class PKCS9Attribute implements DerEncoder {
             break;
 
         case 9:     // extended-certificate attribute -- not supported
-            throw new IOException("PKCS9 extended-certificate " +
+            throw new IllegalArgumentException("PKCS9 extended-certificate " +
                                   "attribute not supported.");
             // break unnecessary
         case 10:    // issuerAndserialNumber attribute -- not supported
-            throw new IOException("PKCS9 IssuerAndSerialNumber " +
+            throw new IllegalArgumentException("PKCS9 IssuerAndSerialNumber " +
                                   "attribute not supported.");
             // break unnecessary
         case 11:    // RSA DSI proprietary
         case 12:    // RSA DSI proprietary
-            throw new IOException("PKCS9 RSA DSI attributes " +
+            throw new IllegalArgumentException("PKCS9 RSA DSI attributes " +
                                   "11 and 12, not supported.");
             // break unnecessary
         case 13:    // S/MIME unused attribute
-            throw new IOException("PKCS9 attribute #13 not supported.");
+            throw new IllegalArgumentException("PKCS9 attribute #13 not supported.");
             // break unnecessary
 
         case 14:     // ExtensionRequest
@@ -622,14 +628,17 @@ public class PKCS9Attribute implements DerEncoder {
             }
             break;
         case 15:    // SMIMECapability
-            throw new IOException("PKCS9 attribute #15 not supported.");
+            throw new IllegalArgumentException("PKCS9 attribute #15 not supported.");
             // break unnecessary
 
         case 16:    // SigningCertificate
-            throw new IOException(
+            {
-                "PKCS9 SigningCertificate attribute not supported.");
+                DerOutputStream temp2 = new DerOutputStream();
-            // break unnecessary
+                SigningCertificateInfo info = (SigningCertificateInfo)value;
-
+                temp2.writeBytes(info.toByteArray());
+                temp.write(DerValue.tag_Set, temp2.toByteArray());
+            }
+            break;
         case 17:    // SignatureTimestampToken
         case 18:    // CMSAlgorithmProtection
             temp.write(DerValue.tag_Set, (byte[])value);

src/java.base/share/classes/sun/security/pkcs/SigningCertificateInfo.java

@@ -79,14 +79,21 @@ import sun.security.x509.SerialNumber;
  * @since 1.5
  * @author Vincent Ryan
  */
-public class SigningCertificateInfo {
+class SigningCertificateInfo {
 
+    private byte[] ber;
     private ESSCertId[] certId = null;
 
-    public SigningCertificateInfo(byte[] ber) throws IOException {
+    SigningCertificateInfo(byte[] ber) throws IOException {
         parse(ber);
+        this.ber = ber;
     }
 
+    byte[] toByteArray() {
+        return ber;
+    }
+
+    @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();
         sb.append("[\n");
@@ -99,7 +106,7 @@ public class SigningCertificateInfo {
         return sb.toString();
     }
 
-    public void parse(byte[] bytes) throws IOException {
+    private void parse(byte[] bytes) throws IOException {
 
         // Parse signingCertificate
         DerValue derValue = new DerValue(bytes);
@@ -122,45 +129,46 @@ public class SigningCertificateInfo {
             }
         }
     }
-}
 
-class ESSCertId {
+    static class ESSCertId {
 
-    private static volatile HexDumpEncoder hexDumper;
+        private static volatile HexDumpEncoder hexDumper;
 
-    private final byte[] certHash;
+        private final byte[] certHash;
-    private final GeneralNames issuer;
+        private final GeneralNames issuer;
-    private final SerialNumber serialNumber;
+        private final SerialNumber serialNumber;
 
-    ESSCertId(DerValue certId) throws IOException {
+        ESSCertId(DerValue certId) throws IOException {
-        // Parse certHash
+            // Parse certHash
-        certHash = certId.data.getDerValue().toByteArray();
+            certHash = certId.data.getDerValue().toByteArray();
 
-        // Parse issuerSerial, if present
+            // Parse issuerSerial, if present
-        if (certId.data.available() > 0) {
+            if (certId.data.available() > 0) {
-            DerValue issuerSerial = certId.data.getDerValue();
+                DerValue issuerSerial = certId.data.getDerValue();
-            // Parse issuer
+                // Parse issuer
-            issuer = new GeneralNames(issuerSerial.data.getDerValue());
+                issuer = new GeneralNames(issuerSerial.data.getDerValue());
-            // Parse serialNumber
+                // Parse serialNumber
-            serialNumber = new SerialNumber(issuerSerial.data.getDerValue());
+                serialNumber = new SerialNumber(issuerSerial.data.getDerValue());
-        } else {
+            } else {
-            issuer = null;
+                issuer = null;
-            serialNumber = null;
+                serialNumber = null;
+            }
+        }
+
+        @Override
+        public String toString() {
+            StringBuilder sb = new StringBuilder();
+            sb.append("[\n\tCertificate hash (SHA-1):\n");
+            if (hexDumper == null) {
+                hexDumper = new HexDumpEncoder();
+            }
+            sb.append(hexDumper.encode(certHash));
+            if (issuer != null && serialNumber != null) {
+                sb.append("\n\tIssuer: " + issuer + "\n");
+                sb.append("\t" + serialNumber);
+            }
+            sb.append("\n]");
+            return sb.toString();
         }
     }
-
-    public String toString() {
-        StringBuilder sb = new StringBuilder();
-        sb.append("[\n\tCertificate hash (SHA-1):\n");
-        if (hexDumper == null) {
-            hexDumper = new HexDumpEncoder();
-        }
-        sb.append(hexDumper.encode(certHash));
-        if (issuer != null && serialNumber != null) {
-            sb.append("\n\tIssuer: " + issuer + "\n");
-            sb.append("\t" + serialNumber);
-        }
-        sb.append("\n]");
-        return sb.toString();
-    }
 }

test/jdk/sun/security/pkcs/pkcs9/PKCS9AttrTypeTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8239950
+ * @bug 8239950 8296736
  * @summary Update PKCS9 Attributes to PKCS#9 v2.0 Encodings
  * @library /test/lib
  * @modules java.base/sun.security.pkcs
@@ -33,6 +33,7 @@
 import java.io.IOException;
 import java.util.*;
 import sun.security.pkcs.PKCS9Attribute;
+import sun.security.util.DerOutputStream;
 import sun.security.util.DerValue;
 import jdk.test.lib.Utils;
 
@@ -123,6 +124,9 @@ public class PKCS9AttrTypeTests {
                 put("signingTime as GeneralizedTime",
                     "301e06092a864886f70d010905311118" +
                     "0f32303530303533313132303030305a");
+
+                put("SigningCertificateInfo",
+                    "3018060b2a864886f70d010910020c3109300730053003040100");
             }};
 
     static final Map<String, String> TEST_INPUT_BAD =
@@ -162,10 +166,20 @@ public class PKCS9AttrTypeTests {
             try {
                 System.out.print("Test - " + entry.getKey() + ": ");
 
-                // Decode each Base64 test vector into DER and place into
+                // Decode each HEX test vector into DER and place into
                 // a DerValue object to be consumed by PKCS9Attribute.
                 PKCS9Attribute p9Attr = new PKCS9Attribute(
                         new DerValue(Utils.toByteArray(entry.getValue())));
+
+                // There is a value inside
+                if (p9Attr.getValue() == null) {
+                    throw new IOException("Empty attribute");
+                }
+
+                // Encoding is supported
+                DerOutputStream dos = new DerOutputStream();
+                p9Attr.encode(dos);
+
                 System.out.println("PASS");
                 System.out.println("---------------");
                 System.out.println(p9Attr);