8134364: Add defensive copies to get/set methods for OCSPNonceExtension
Summary: Make OCSPNonceExtension immutable, add defensive copies
Reviewed-by: xuelei, mullan
--- a/jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSPNonceExtension.java Fri Sep 04 15:28:01 2015 +0300
+++ b/jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSPNonceExtension.java Fri Sep 04 09:31:47 2015 -0700
@@ -26,15 +26,13 @@
package sun.security.provider.certpath;
import java.io.IOException;
-import java.io.OutputStream;
-import java.util.Enumeration;
+import java.util.Objects;
import java.security.SecureRandom;
-import sun.security.x509.AttributeNameEnumeration;
-import sun.security.x509.CertAttrSet;
import sun.security.x509.Extension;
import sun.security.x509.PKIXExtensions;
-import sun.security.util.*;
+import sun.security.util.Debug;
+import sun.security.util.DerValue;
/**
* Represent the OCSP Nonce Extension.
@@ -43,252 +41,139 @@
* and help to prevent replay attacks (see RFC 6960, section 4.4.1).
*
* @see Extension
- * @see CertAttrSet
*/
-public class OCSPNonceExtension extends Extension
-implements CertAttrSet<String> {
+public final class OCSPNonceExtension extends Extension {
/**
* Attribute name.
*/
- public static final String NAME = "OCSPNonce";
- public static final String NONCE = "nonce";
-
+ private static final String EXTENSION_NAME = "OCSPNonce";
private byte[] nonceData = null;
- private String extensionName;
-
- /**
- * Encode this extension value to DER and assign it to the
- * {@code extensionName} data member.
- *
- * @throws IOException if any errors occur during DER encoding
- */
- private void encodeInternal() throws IOException {
- if (nonceData == null) {
- this.extensionValue = null;
- return;
- }
- DerOutputStream os = new DerOutputStream();
- os.putOctetString(this.nonceData);
- this.extensionValue = os.toByteArray();
- }
/**
* Create a {@code OCSPNonceExtension} by providing the nonce length.
- * The criticality is set to false. The random bytes will be generated
- * using the SUN provider.
+ * The criticality is set to false, and the OID for the extension will
+ * be the value defined by "id-pkix-ocsp-nonce" from RFC 6960.
*
* @param length the number of random bytes composing the nonce
*
* @throws IOException if any errors happen during encoding of the
* extension.
+ * @throws IllegalArgumentException if length is not a positive integer.
*/
public OCSPNonceExtension(int length) throws IOException {
- this(PKIXExtensions.OCSPNonce_Id, false, length, NAME);
- }
-
- /**
- * Creates the extension (also called by the subclass).
- *
- * @param extensionId the {@code ObjectIdentifier} for the OCSP Nonce
- * extension
- * @param isCritical a boolean flag indicating if the criticality bit
- * is to be set for this extension
- * @param length the length of the nonce in bytes
- * @param extensionName the name of the extension
- *
- * @throws IOException if any errors happen during encoding of the
- * extension.
- */
- protected OCSPNonceExtension(ObjectIdentifier extensionId,
- boolean isCritical, int length, String extensionName)
- throws IOException {
- SecureRandom rng = new SecureRandom();
- this.nonceData = new byte[length];
- rng.nextBytes(nonceData);
- this.extensionId = extensionId;
- this.critical = isCritical;
- this.extensionName = extensionName;
- encodeInternal();
- }
-
- /**
- * Create the extension using the provided criticality bit setting and
- * DER encoding.
- *
- * @param critical true if the extension is to be treated as critical.
- * @param value an array of DER encoded bytes of the extnValue for the
- * extension. It must not include the encapsulating OCTET STRING
- * tag and length. For an {@code OCSPNonceExtension} the data value
- * should be a simple OCTET STRING containing random bytes
- * (see RFC 6960, section 4.4.1).
- *
- * @throws ClassCastException if value is not an array of bytes
- * @throws IOException if any errors happen during encoding of the
- * extension
- */
- public OCSPNonceExtension(Boolean critical, Object value)
- throws IOException {
- this(PKIXExtensions.OCSPNonce_Id, critical, value, NAME);
+ this(false, length);
}
/**
- * Creates the extension (also called by the subclass).
- *
- * @param extensionId the {@code ObjectIdentifier} for the OCSP Nonce
- * extension
- * @param critical a boolean flag indicating if the criticality bit
- * is to be set for this extension
- * @param value an array of DER encoded bytes of the extnValue for the
- * extension. It must not include the encapsulating OCTET STRING
- * tag and length. For an {@code OCSPNonceExtension} the data value
- * should be a simple OCTET STRING containing random bytes
- * (see RFC 6960, section 4.4.1).
- * @param extensionName the name of the extension
+ * Create a {@code OCSPNonceExtension} by providing the nonce length and
+ * criticality setting. The OID for the extension will
+ * be the value defined by "id-pkix-ocsp-nonce" from RFC 6960.
*
- * @throws ClassCastException if value is not an array of bytes
- * @throws IOException if any errors happen during encoding of the
- * extension
- */
- protected OCSPNonceExtension(ObjectIdentifier extensionId,
- Boolean critical, Object value, String extensionName)
- throws IOException {
- this.extensionId = extensionId;
- this.critical = critical;
- this.extensionValue = (byte[]) value;
- DerValue val = new DerValue(this.extensionValue);
- this.nonceData = val.getOctetString();
- this.extensionName = extensionName;
- }
-
- /**
- * Set the attribute value.
- *
- * @param name the name of the attribute.
- * @param obj an array of nonce bytes for the extension. It must not
- * contain any DER tags or length.
+ * @param isCritical a boolean flag indicating whether the criticality bit
+ * is set for this extension
+ * @param length the number of random bytes composing the nonce
*
- * @throws IOException if an unsupported name is provided or the supplied
- * {@code obj} is not a byte array
+ * @throws IOException if any errors happen during encoding of the
+ * extension.
+ * @throws IllegalArgumentException if length is not a positive integer.
*/
- @Override
- public void set(String name, Object obj) throws IOException {
- if (name.equalsIgnoreCase(NONCE)) {
- if (!(obj instanceof byte[])) {
- throw new IOException("Attribute must be of type byte[].");
- }
- nonceData = (byte[])obj;
- } else {
- throw new IOException("Attribute name not recognized by"
- + " CertAttrSet:" + extensionName + ".");
- }
- encodeInternal();
- }
+ public OCSPNonceExtension(boolean isCritical, int length)
+ throws IOException {
+ this.extensionId = PKIXExtensions.OCSPNonce_Id;
+ this.critical = isCritical;
- /**
- * Get the attribute value.
- *
- * @param name the name of the attribute to retrieve. Only "OCSPNonce"
- * is currently supported.
- *
- * @return an array of bytes that are the nonce data. It will not contain
- * any DER tags or length, only the random nonce bytes.
- *
- * @throws IOException if an unsupported name is provided.
- */
- @Override
- public Object get(String name) throws IOException {
- if (name.equalsIgnoreCase(NONCE)) {
- return nonceData;
+ if (length > 0) {
+ SecureRandom rng = new SecureRandom();
+ this.nonceData = new byte[length];
+ rng.nextBytes(nonceData);
+ this.extensionValue = new DerValue(DerValue.tag_OctetString,
+ nonceData).toByteArray();
} else {
- throw new IOException("Attribute name not recognized by"
- + " CertAttrSet:" + extensionName + ".");
+ throw new IllegalArgumentException(
+ "Length must be a positive integer");
}
}
/**
- * Delete the attribute value.
+ * Create a {@code OCSPNonceExtension} by providing a nonce value.
+ * The criticality is set to false, and the OID for the extension will
+ * be the value defined by "id-pkix-ocsp-nonce" from RFC 6960.
*
- * @param name the name of the attribute to retrieve. Only "OCSPNonce"
- * is currently supported.
+ * @param incomingNonce The nonce data to be set for the extension. This
+ * must be a non-null array of at least one byte long.
*
- * @throws IOException if an unsupported name is provided or an error
- * occurs during re-encoding of the extension.
+ * @throws IOException if any errors happen during encoding of the
+ * extension.
+ * @throws IllegalArgumentException if the incomingNonce length is not a
+ * positive integer.
+ * @throws NullPointerException if the incomingNonce is null.
*/
- @Override
- public void delete(String name) throws IOException {
- if (name.equalsIgnoreCase(NONCE)) {
- nonceData = null;
+ public OCSPNonceExtension(byte[] incomingNonce) throws IOException {
+ this(false, incomingNonce);
+ }
+
+ /**
+ * Create a {@code OCSPNonceExtension} by providing a nonce value and
+ * criticality setting. The OID for the extension will
+ * be the value defined by "id-pkix-ocsp-nonce" from RFC 6960.
+ *
+ * @param isCritical a boolean flag indicating whether the criticality bit
+ * is set for this extension
+ * @param incomingNonce The nonce data to be set for the extension. This
+ * must be a non-null array of at least one byte long.
+ *
+ * @throws IOException if any errors happen during encoding of the
+ * extension.
+ * @throws IllegalArgumentException if the incomingNonce length is not a
+ * positive integer.
+ * @throws NullPointerException if the incomingNonce is null.
+ */
+ public OCSPNonceExtension(boolean isCritical, byte[] incomingNonce)
+ throws IOException {
+ this.extensionId = PKIXExtensions.OCSPNonce_Id;
+ this.critical = isCritical;
+
+ Objects.requireNonNull(incomingNonce, "Nonce data must be non-null");
+ if (incomingNonce.length > 0) {
+ this.nonceData = incomingNonce.clone();
+ this.extensionValue = new DerValue(DerValue.tag_OctetString,
+ nonceData).toByteArray();
} else {
- throw new IOException("Attribute name not recognized by"
- + " CertAttrSet:" + extensionName + ".");
+ throw new IllegalArgumentException(
+ "Nonce data must be at least 1 byte in length");
}
- encodeInternal();
+ }
+
+ /**
+ * Return the nonce bytes themselves, without any DER encoding.
+ *
+ * @return A copy of the underlying nonce bytes
+ */
+ public byte[] getNonceValue() {
+ return nonceData.clone();
}
/**
* Returns a printable representation of the {@code OCSPNonceExtension}.
+ *
+ * @return a string representation of the extension.
*/
@Override
public String toString() {
- String s = super.toString() + extensionName + ": " +
- ((nonceData == null) ? "" : Debug.toString(nonceData))
- + "\n";
- return (s);
- }
-
- /**
- * Write the extension to an {@code OutputStream}
- *
- * @param out the {@code OutputStream} to write the extension to.
- *
- * @throws IOException on encoding errors.
- */
- @Override
- public void encode(OutputStream out) throws IOException {
- encode(out, PKIXExtensions.OCSPNonce_Id, this.critical);
+ StringBuilder sb = new StringBuilder();
+ sb.append(super.toString()).append(EXTENSION_NAME).append(": ");
+ sb.append((nonceData == null) ? "" : Debug.toString(nonceData));
+ sb.append("\n");
+ return sb.toString();
}
/**
- * Write the extension to the DerOutputStream.
+ * Return the name of the extension as a {@code String}
*
- * @param out the {@code OutputStream} to write the extension to.
- * @param extensionId the {@code ObjectIdentifier} used for this extension
- * @param isCritical a flag indicating if the criticality bit is set for
- * this extension.
- *
- * @throws IOException on encoding errors.
+ * @return the name of the extension
*/
- protected void encode(OutputStream out, ObjectIdentifier extensionId,
- boolean isCritical) throws IOException {
-
- DerOutputStream tmp = new DerOutputStream();
-
- if (this.extensionValue == null) {
- this.extensionId = extensionId;
- this.critical = isCritical;
- encodeInternal();
- }
- super.encode(tmp);
- out.write(tmp.toByteArray());
- }
-
- /**
- * Return an enumeration of names of attributes existing within this
- * attribute.
- */
- @Override
- public Enumeration<String> getElements() {
- AttributeNameEnumeration elements = new AttributeNameEnumeration();
- elements.addElement(NONCE);
- return (elements.elements());
- }
-
- /**
- * Return the name of this attribute.
- */
- @Override
public String getName() {
- return (extensionName);
+ return EXTENSION_NAME;
}
}
--- a/jdk/test/sun/security/provider/certpath/Extensions/OCSPNonceExtensionTests.java Fri Sep 04 15:28:01 2015 +0300
+++ b/jdk/test/sun/security/provider/certpath/Extensions/OCSPNonceExtensionTests.java Fri Sep 04 09:31:47 2015 -0700
@@ -79,14 +79,11 @@
Map<String, TestCase> testList =
new LinkedHashMap<String, TestCase>() {{
put("CTOR Test (provide length)", testCtorByLength);
+ put("CTOR Test (provide nonce bytes)", testCtorByValue);
+ put("CTOR Test (set criticality forms)", testCtorCritForms);
put("CTOR Test (provide extension DER encoding)",
testCtorSuperByDerValue);
- put("Use set() call to provide random data", testResetValue);
- put("Test get() method", testGet);
- put("test set() method", testSet);
- put("Test getElements() method", testGetElements);
put("Test getName() method", testGetName);
- put("Test delete() method", testDelete);
}};
System.out.println("============ Tests ============");
@@ -179,6 +176,20 @@
Boolean pass = Boolean.FALSE;
String message = null;
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ // Try sending in a negative length
+ try {
+ Extension negLenNonce = new OCSPNonceExtension(-8);
+ throw new RuntimeException(
+ "Accepted a negative length nonce");
+ } catch (IllegalArgumentException iae) { }
+
+ // How about a zero length?
+ try {
+ Extension zeroLenNonce = new OCSPNonceExtension(0);
+ throw new RuntimeException("Accepted a zero length nonce");
+ } catch (IllegalArgumentException iae) { }
+
+ // Valid input to constructor
Extension nonceByLen = new OCSPNonceExtension(32);
// Verify overall encoded extension structure
@@ -216,6 +227,82 @@
}
};
+ public static final TestCase testCtorByValue = new TestCase() {
+ @Override
+ public Map.Entry<Boolean, String> runTest() {
+ Boolean pass = Boolean.FALSE;
+ String message = null;
+ try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+
+ // Try giving a null value for the nonce
+ try {
+ Extension nullNonce = new OCSPNonceExtension(null);
+ throw new RuntimeException("Accepted a null nonce");
+ } catch (NullPointerException npe) { }
+
+ // How about a zero-length byte array?
+ try {
+ Extension zeroLenNonce =
+ new OCSPNonceExtension(new byte[0]);
+ throw new RuntimeException("Accepted a zero length nonce");
+ } catch (IllegalArgumentException iae) { }
+
+ OCSPNonceExtension nonceByValue =
+ new OCSPNonceExtension(DEADBEEF_16);
+
+ // Verify overall encoded extension structure
+ nonceByValue.encode(baos);
+ verifyExtStructure(baos.toByteArray());
+
+ // Verify the name, elements, and data conform to
+ // expected values for this specific object.
+ boolean crit = nonceByValue.isCritical();
+ String oid = nonceByValue.getId();
+ byte[] nonceData = nonceByValue.getNonceValue();
+
+ if (crit) {
+ message = "Extension incorrectly marked critical";
+ } else if (!oid.equals(OCSP_NONCE_OID)) {
+ message = "Incorrect OID (Got " + oid + ", Expected " +
+ OCSP_NONCE_OID + ")";
+ } else if (!Arrays.equals(nonceData, DEADBEEF_16)) {
+ message = "Returned nonce value did not match input";
+ } else {
+ pass = Boolean.TRUE;
+ }
+ } catch (Exception e) {
+ e.printStackTrace(System.out);
+ message = e.getClass().getName();
+ }
+
+ return new AbstractMap.SimpleEntry<>(pass, message);
+ }
+ };
+
+ public static final TestCase testCtorCritForms = new TestCase() {
+ @Override
+ public Map.Entry<Boolean, String> runTest() {
+ Boolean pass = Boolean.FALSE;
+ String message = null;
+ try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ Extension nonceByLength = new OCSPNonceExtension(true, 32);
+ Extension nonceByValue =
+ new OCSPNonceExtension(true, DEADBEEF_16);
+ pass = nonceByLength.isCritical() && nonceByValue.isCritical();
+ if (!pass) {
+ message = "nonceByLength or nonceByValue was not marked " +
+ "critical as expected";
+ }
+ } catch (Exception e) {
+ e.printStackTrace(System.out);
+ message = e.getClass().getName();
+ }
+
+ return new AbstractMap.SimpleEntry<>(pass, message);
+ }
+ };
+
+
public static final TestCase testCtorSuperByDerValue = new TestCase() {
@Override
public Map.Entry<Boolean, String> runTest() {
@@ -260,145 +347,6 @@
}
};
- public static final TestCase testResetValue = new TestCase() {
- @Override
- public Map.Entry<Boolean, String> runTest() {
- Boolean pass = Boolean.FALSE;
- String message = null;
- try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
- OCSPNonceExtension nonce = new OCSPNonceExtension(32);
-
- // Reset the nonce data to reflect 16 bytes of DEADBEEF
- nonce.set(OCSPNonceExtension.NONCE, (Object)DEADBEEF_16);
-
- // Verify overall encoded extension content
- nonce.encode(baos);
- dumpHexBytes(OCSP_NONCE_DB16);
- System.out.println();
- dumpHexBytes(baos.toByteArray());
-
- pass = Arrays.equals(baos.toByteArray(), OCSP_NONCE_DB16);
- } catch (Exception e) {
- e.printStackTrace(System.out);
- message = e.getClass().getName();
- }
-
- return new AbstractMap.SimpleEntry<>(pass, message);
- }
- };
-
- public static final TestCase testSet = new TestCase() {
- @Override
- public Map.Entry<Boolean, String> runTest() {
- Boolean pass = Boolean.FALSE;
- String message = null;
- try {
- OCSPNonceExtension nonceByLen = new OCSPNonceExtension(32);
-
- // Set the nonce data to 16 bytes of DEADBEEF
- nonceByLen.set(ELEMENT_NONCE, DEADBEEF_16);
- byte[] nonceData = (byte[])nonceByLen.get(ELEMENT_NONCE);
- if (!Arrays.equals(nonceData, DEADBEEF_16)) {
- throw new RuntimeException("Retuned nonce data does not " +
- "match expected result");
- }
-
- // Now try to set a value using an object that is not a byte
- // array
- int[] INT_DB_16 = {
- 0xDEADBEEF, 0xDEADBEEF, 0xDEADBEEF, 0xDEADBEEF
- };
- try {
- nonceByLen.set(ELEMENT_NONCE, INT_DB_16);
- throw new RuntimeException("Accepted get() for " +
- "unsupported element name");
- } catch (IOException ioe) { } // Expected result
-
- // And try setting a value using an unknown element name
- try {
- nonceByLen.set("FOO", DEADBEEF_16);
- throw new RuntimeException("Accepted get() for " +
- "unsupported element name");
- } catch (IOException ioe) { } // Expected result
-
- pass = Boolean.TRUE;
- } catch (Exception e) {
- e.printStackTrace(System.out);
- message = e.getClass().getName();
- }
-
- return new AbstractMap.SimpleEntry<>(pass, message);
- }
- };
-
- public static final TestCase testGet = new TestCase() {
- @Override
- public Map.Entry<Boolean, String> runTest() {
- Boolean pass = Boolean.FALSE;
- String message = null;
- try {
- OCSPNonceExtension nonceByLen = new OCSPNonceExtension(32);
-
- // Grab the nonce data by its correct element name
- byte[] nonceData = (byte[])nonceByLen.get(ELEMENT_NONCE);
- if (nonceData == null || nonceData.length != 32) {
- throw new RuntimeException("Unexpected return value from " +
- "get() method: either null or incorrect length");
- }
-
- // Now try to get any kind of data using an element name that
- // doesn't exist for this extension.
- try {
- nonceByLen.get("FOO");
- throw new RuntimeException("Accepted get() for " +
- "unsupported element name");
- } catch (IOException ioe) { } // Expected result
-
- pass = Boolean.TRUE;
- } catch (Exception e) {
- e.printStackTrace(System.out);
- message = e.getClass().getName();
- }
-
- return new AbstractMap.SimpleEntry<>(pass, message);
- }
- };
-
- public static final TestCase testGetElements = new TestCase() {
- @Override
- public Map.Entry<Boolean, String> runTest() {
- Boolean pass = Boolean.FALSE;
- String message = null;
- try {
- OCSPNonceExtension nonceByLen = new OCSPNonceExtension(32);
-
- int elementCount = 0;
- boolean foundElement = false;
-
- // There should be exactly one element and its name should
- // be "nonce"
- for (Enumeration<String> elements = nonceByLen.getElements();
- elements.hasMoreElements(); elementCount++) {
- if (elements.nextElement().equals(ELEMENT_NONCE)) {
- foundElement = true;
- }
- }
-
- if (!foundElement || elementCount != 1) {
- throw new RuntimeException("Unexpected or missing " +
- "Enumeration element");
- }
-
- pass = Boolean.TRUE;
- } catch (Exception e) {
- e.printStackTrace(System.out);
- message = e.getClass().getName();
- }
-
- return new AbstractMap.SimpleEntry<>(pass, message);
- }
- };
-
public static final TestCase testGetName = new TestCase() {
@Override
public Map.Entry<Boolean, String> runTest() {
@@ -415,44 +363,4 @@
return new AbstractMap.SimpleEntry<>(pass, message);
}
};
-
- public static final TestCase testDelete = new TestCase() {
- @Override
- public Map.Entry<Boolean, String> runTest() {
- Boolean pass = Boolean.FALSE;
- String message = null;
- try {
- OCSPNonceExtension nonceByLen = new OCSPNonceExtension(32);
-
- // First verify that there's data to begin with
- byte[] nonceData = (byte[])nonceByLen.get(ELEMENT_NONCE);
- if (nonceData == null || nonceData.length != 32) {
- throw new RuntimeException("Unexpected return value from " +
- "get() method: either null or incorrect length");
- }
-
- // Attempt to delete using an element name that doesn't exist
- // for this extension.
- try {
- nonceByLen.delete("FOO");
- throw new RuntimeException("Accepted delete() for " +
- "unsupported element name");
- } catch (IOException ioe) { } // Expected result
-
- // Now attempt to properly delete the extension data
- nonceByLen.delete(ELEMENT_NONCE);
- nonceData = (byte[])nonceByLen.get(ELEMENT_NONCE);
- if (nonceData != null) {
- throw new RuntimeException("Unexpected non-null return");
- }
-
- pass = Boolean.TRUE;
- } catch (Exception e) {
- e.printStackTrace(System.out);
- message = e.getClass().getName();
- }
-
- return new AbstractMap.SimpleEntry<>(pass, message);
- }
- };
}