8012288: XML DSig API allows wrong tag names and extra elements in SignedInfo
Reviewed-by: xuelei
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMKeyValue.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMKeyValue.java Thu Jul 25 20:12:14 2013 -0400
@@ -218,9 +218,11 @@
("unable to create RSA KeyFactory: " + e.getMessage());
}
}
- Element modulusElem = DOMUtils.getFirstChildElement(kvtElem);
+ Element modulusElem = DOMUtils.getFirstChildElement(kvtElem,
+ "Modulus");
modulus = new DOMCryptoBinary(modulusElem.getFirstChild());
- Element exponentElem = DOMUtils.getNextSiblingElement(modulusElem);
+ Element exponentElem = DOMUtils.getNextSiblingElement(modulusElem,
+ "Exponent");
exponent = new DOMCryptoBinary(exponentElem.getFirstChild());
RSAPublicKeySpec spec = new RSAPublicKeySpec(modulus.getBigNum(),
exponent.getBigNum());
@@ -289,13 +291,13 @@
// check for P and Q
if (curElem.getLocalName().equals("P")) {
p = new DOMCryptoBinary(curElem.getFirstChild());
- curElem = DOMUtils.getNextSiblingElement(curElem);
+ curElem = DOMUtils.getNextSiblingElement(curElem, "Q");
q = new DOMCryptoBinary(curElem.getFirstChild());
curElem = DOMUtils.getNextSiblingElement(curElem);
}
if (curElem.getLocalName().equals("G")) {
g = new DOMCryptoBinary(curElem.getFirstChild());
- curElem = DOMUtils.getNextSiblingElement(curElem);
+ curElem = DOMUtils.getNextSiblingElement(curElem, "Y");
}
y = new DOMCryptoBinary(curElem.getFirstChild());
curElem = DOMUtils.getNextSiblingElement(curElem);
@@ -460,7 +462,7 @@
} else {
throw new MarshalException("Invalid ECKeyValue");
}
- curElem = DOMUtils.getNextSiblingElement(curElem);
+ curElem = DOMUtils.getNextSiblingElement(curElem, "PublicKey");
ECPoint ecPoint = null;
try {
Object[] args = new Object[] { Base64.decode(curElem),
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMManifest.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMManifest.java Thu Jul 25 20:12:14 2013 -0400
@@ -101,20 +101,24 @@
boolean secVal = Utils.secureValidation(context);
- Element refElem = DOMUtils.getFirstChildElement(manElem);
+ Element refElem = DOMUtils.getFirstChildElement(manElem, "Reference");
List<Reference> refs = new ArrayList<Reference>();
+ refs.add(new DOMReference(refElem, context, provider));
- int refCount = 0;
+ refElem = DOMUtils.getNextSiblingElement(refElem);
while (refElem != null) {
+ String localName = refElem.getLocalName();
+ if (!localName.equals("Reference")) {
+ throw new MarshalException("Invalid element name: " +
+ localName + ", expected Reference");
+ }
refs.add(new DOMReference(refElem, context, provider));
- refElem = DOMUtils.getNextSiblingElement(refElem);
-
- refCount++;
- if (secVal && (refCount > DOMSignedInfo.MAXIMUM_REFERENCE_COUNT)) {
+ if (secVal && (refs.size() > DOMSignedInfo.MAXIMUM_REFERENCE_COUNT)) {
String error = "A maxiumum of " + DOMSignedInfo.MAXIMUM_REFERENCE_COUNT + " "
+ "references per Manifest are allowed with secure validation";
throw new MarshalException(error);
}
+ refElem = DOMUtils.getNextSiblingElement(refElem);
}
this.references = Collections.unmodifiableList(refs);
}
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMReference.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMReference.java Thu Jul 25 20:12:14 2013 -0400
@@ -204,23 +204,33 @@
Element nextSibling = DOMUtils.getFirstChildElement(refElem);
List<Transform> transforms = new ArrayList<Transform>(5);
if (nextSibling.getLocalName().equals("Transforms")) {
- Element transformElem = DOMUtils.getFirstChildElement(nextSibling);
-
- int transformCount = 0;
+ Element transformElem = DOMUtils.getFirstChildElement(nextSibling,
+ "Transform");
+ transforms.add(new DOMTransform(transformElem, context, provider));
+ transformElem = DOMUtils.getNextSiblingElement(transformElem);
while (transformElem != null) {
+ String localName = transformElem.getLocalName();
+ if (!localName.equals("Transform")) {
+ throw new MarshalException(
+ "Invalid element name: " + localName +
+ ", expected Transform");
+ }
transforms.add
(new DOMTransform(transformElem, context, provider));
- transformElem = DOMUtils.getNextSiblingElement(transformElem);
-
- transformCount++;
- if (secVal && (transformCount > MAXIMUM_TRANSFORM_COUNT)) {
+ if (secVal && (transforms.size() > MAXIMUM_TRANSFORM_COUNT)) {
String error = "A maxiumum of " + MAXIMUM_TRANSFORM_COUNT + " "
+ "transforms per Reference are allowed with secure validation";
throw new MarshalException(error);
}
+ transformElem = DOMUtils.getNextSiblingElement(transformElem);
}
nextSibling = DOMUtils.getNextSiblingElement(nextSibling);
}
+ if (!nextSibling.getLocalName().equals("DigestMethod")) {
+ throw new MarshalException("Invalid element name: " +
+ nextSibling.getLocalName() +
+ ", expected DigestMethod");
+ }
// unmarshal DigestMethod
Element dmElem = nextSibling;
@@ -234,13 +244,19 @@
}
// unmarshal DigestValue
+ Element dvElem = DOMUtils.getNextSiblingElement(dmElem, "DigestValue");
try {
- Element dvElem = DOMUtils.getNextSiblingElement(dmElem);
this.digestValue = Base64.decode(dvElem);
} catch (Base64DecodingException bde) {
throw new MarshalException(bde);
}
+ // check for extra elements
+ if (DOMUtils.getNextSiblingElement(dvElem) != null) {
+ throw new MarshalException(
+ "Unexpected element after DigestValue element");
+ }
+
// unmarshal attributes
this.uri = DOMUtils.getAttributeValue(refElem, "URI");
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMRetrievalMethod.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMRetrievalMethod.java Thu Jul 25 20:12:14 2013 -0400
@@ -136,21 +136,30 @@
List<Transform> transforms = new ArrayList<Transform>();
Element transformsElem = DOMUtils.getFirstChildElement(rmElem);
- int transformCount = 0;
if (transformsElem != null) {
+ String localName = transformsElem.getLocalName();
+ if (!localName.equals("Transforms")) {
+ throw new MarshalException("Invalid element name: " +
+ localName + ", expected Transforms");
+ }
Element transformElem =
- DOMUtils.getFirstChildElement(transformsElem);
+ DOMUtils.getFirstChildElement(transformsElem, "Transform");
+ transforms.add(new DOMTransform(transformElem, context, provider));
+ transformElem = DOMUtils.getNextSiblingElement(transformElem);
while (transformElem != null) {
+ String name = transformElem.getLocalName();
+ if (!name.equals("Transform")) {
+ throw new MarshalException("Invalid element name: " +
+ name + ", expected Transform");
+ }
transforms.add
(new DOMTransform(transformElem, context, provider));
- transformElem = DOMUtils.getNextSiblingElement(transformElem);
-
- transformCount++;
- if (secVal && (transformCount > DOMReference.MAXIMUM_TRANSFORM_COUNT)) {
+ if (secVal && (transforms.size() > DOMReference.MAXIMUM_TRANSFORM_COUNT)) {
String error = "A maxiumum of " + DOMReference.MAXIMUM_TRANSFORM_COUNT + " "
+ "transforms per Reference are allowed with secure validation";
throw new MarshalException(error);
}
+ transformElem = DOMUtils.getNextSiblingElement(transformElem);
}
}
if (transforms.isEmpty()) {
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureProperties.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureProperties.java Thu Jul 25 20:12:14 2013 -0400
@@ -109,6 +109,11 @@
for (int i = 0; i < length; i++) {
Node child = nodes.item(i);
if (child.getNodeType() == Node.ELEMENT_NODE) {
+ String name = child.getLocalName();
+ if (!name.equals("SignatureProperty")) {
+ throw new MarshalException("Invalid element name: " + name +
+ ", expected SignatureProperty");
+ }
properties.add(new DOMSignatureProperty((Element)child,
context));
}
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignedInfo.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignedInfo.java Thu Jul 25 20:12:14 2013 -0400
@@ -150,11 +150,14 @@
id = DOMUtils.getAttributeValue(siElem, "Id");
// unmarshal CanonicalizationMethod
- Element cmElem = DOMUtils.getFirstChildElement(siElem);
- canonicalizationMethod = new DOMCanonicalizationMethod(cmElem, context, provider);
+ Element cmElem = DOMUtils.getFirstChildElement(siElem,
+ "CanonicalizationMethod");
+ canonicalizationMethod = new DOMCanonicalizationMethod(cmElem, context,
+ provider);
// unmarshal SignatureMethod
- Element smElem = DOMUtils.getNextSiblingElement(cmElem);
+ Element smElem = DOMUtils.getNextSiblingElement(cmElem,
+ "SignatureMethod");
signatureMethod = DOMSignatureMethod.unmarshal(smElem);
boolean secVal = Utils.secureValidation(context);
@@ -169,19 +172,24 @@
// unmarshal References
ArrayList<Reference> refList = new ArrayList<Reference>(5);
- Element refElem = DOMUtils.getNextSiblingElement(smElem);
+ Element refElem = DOMUtils.getNextSiblingElement(smElem, "Reference");
+ refList.add(new DOMReference(refElem, context, provider));
- int refCount = 0;
+ refElem = DOMUtils.getNextSiblingElement(refElem);
while (refElem != null) {
+ String name = refElem.getLocalName();
+ if (!name.equals("Reference")) {
+ throw new MarshalException("Invalid element name: " +
+ name + ", expected Reference");
+ }
refList.add(new DOMReference(refElem, context, provider));
- refElem = DOMUtils.getNextSiblingElement(refElem);
- refCount++;
- if (secVal && (refCount > MAXIMUM_REFERENCE_COUNT)) {
+ if (secVal && (refList.size() > MAXIMUM_REFERENCE_COUNT)) {
String error = "A maxiumum of " + MAXIMUM_REFERENCE_COUNT + " "
+ "references per Manifest are allowed with secure validation";
throw new MarshalException(error);
}
+ refElem = DOMUtils.getNextSiblingElement(refElem);
}
references = Collections.unmodifiableList(refList);
}
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMUtils.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMUtils.java Thu Jul 25 20:12:14 2013 -0400
@@ -132,6 +132,36 @@
}
/**
+ * Returns the first child element of the specified node and checks that
+ * the local name is equal to {@code localName}.
+ *
+ * @param node the node
+ * @return the first child element of the specified node
+ * @throws NullPointerException if {@code node == null}
+ * @throws MarshalException if no such element or the local name is not
+ * equal to {@code localName}
+ */
+ public static Element getFirstChildElement(Node node, String localName)
+ throws MarshalException
+ {
+ return verifyElement(getFirstChildElement(node), localName);
+ }
+
+ private static Element verifyElement(Element elem, String localName)
+ throws MarshalException
+ {
+ if (elem == null) {
+ throw new MarshalException("Missing " + localName + " element");
+ }
+ String name = elem.getLocalName();
+ if (!name.equals(localName)) {
+ throw new MarshalException("Invalid element name: " +
+ name + ", expected " + localName);
+ }
+ return elem;
+ }
+
+ /**
* Returns the last child element of the specified node, or null if there
* is no such element.
*
@@ -166,6 +196,22 @@
}
/**
+ * Returns the next sibling element of the specified node and checks that
+ * the local name is equal to {@code localName}.
+ *
+ * @param node the node
+ * @return the next sibling element of the specified node
+ * @throws NullPointerException if {@code node == null}
+ * @throws MarshalException if no such element or the local name is not
+ * equal to {@code localName}
+ */
+ public static Element getNextSiblingElement(Node node, String localName)
+ throws MarshalException
+ {
+ return verifyElement(getNextSiblingElement(node), localName);
+ }
+
+ /**
* Returns the attribute value for the attribute with the specified name.
* Returns null if there is no such attribute, or
* the empty string if the attribute value is empty.
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMX509IssuerSerial.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMX509IssuerSerial.java Thu Jul 25 20:12:14 2013 -0400
@@ -80,9 +80,11 @@
*
* @param isElem an X509IssuerSerial element
*/
- public DOMX509IssuerSerial(Element isElem) {
- Element iNElem = DOMUtils.getFirstChildElement(isElem);
- Element sNElem = DOMUtils.getNextSiblingElement(iNElem);
+ public DOMX509IssuerSerial(Element isElem) throws MarshalException {
+ Element iNElem = DOMUtils.getFirstChildElement(isElem,
+ "X509IssuerName");
+ Element sNElem = DOMUtils.getNextSiblingElement(iNElem,
+ "X509SerialNumber");
issuerName = iNElem.getFirstChild().getNodeValue();
serialNumber = new BigInteger(sNElem.getFirstChild().getNodeValue());
}
--- a/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMXMLSignature.java Wed Jul 17 00:34:39 2013 -0700
+++ b/jdk/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMXMLSignature.java Thu Jul 25 20:12:14 2013 -0400
@@ -141,11 +141,13 @@
id = DOMUtils.getAttributeValue(localSigElem, "Id");
// unmarshal SignedInfo
- Element siElem = DOMUtils.getFirstChildElement(localSigElem);
+ Element siElem = DOMUtils.getFirstChildElement(localSigElem,
+ "SignedInfo");
si = new DOMSignedInfo(siElem, context, provider);
// unmarshal SignatureValue
- Element sigValElem = DOMUtils.getNextSiblingElement(siElem);
+ Element sigValElem = DOMUtils.getNextSiblingElement(siElem,
+ "SignatureValue");
sv = new DOMSignatureValue(sigValElem, context);
// unmarshal KeyInfo, if specified
@@ -161,6 +163,11 @@
} else {
List<XMLObject> tempObjects = new ArrayList<XMLObject>();
while (nextSibling != null) {
+ String name = nextSibling.getLocalName();
+ if (!name.equals("Object")) {
+ throw new MarshalException("Invalid element name: " + name +
+ ", expected KeyInfo or Object");
+ }
tempObjects.add(new DOMXMLObject(nextSibling,
context, provider));
nextSibling = DOMUtils.getNextSiblingElement(nextSibling);