Mitigate XSW in SMD verification (#3148)

There's a bit of a mismatch between the bit that parses the Java object
and the bit that validates the XML. The parser parses the Java root
node, however the validator follows the reference in a (valid) signature
to *any* node, which can be hidden elsewhere.

To fix this robustly:
- Enforce that the XML signature Reference URI matches the root element ID precisely
- Assert that exactly one <smd:signedMark> element exists in the DOM
- Don't allow additional signed marks elsewhere in the XML just in case

core/src/main/java/google/registry/tmch/TmchXmlSignature.java

@@ -51,6 +51,7 @@ import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.validation.Schema;
 import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
 
@@ -84,6 +85,27 @@ public class TmchXmlSignature {
     checkArgument(smdXml.length > 0);
     Document doc = parseSmdDocument(new ByteArrayInputStream(smdXml));
 
+    // Verify root element is <smd:signedMark> in correct namespace
+    Element rootElement = doc.getDocumentElement();
+    if (!"signedMark".equals(rootElement.getLocalName())
+        || !"urn:ietf:params:xml:ns:signedMark-1.0".equals(rootElement.getNamespaceURI())) {
+      throw new XMLSignatureException(
+          "Root element must be signedMark in namespace urn:ietf:params:xml:ns:signedMark-1.0");
+    }
+
+    // Assert exactly one <smd:signedMark> element exists in the DOM to prevent wrapping/nesting
+    NodeList signedMarkNodes =
+        doc.getElementsByTagNameNS("urn:ietf:params:xml:ns:signedMark-1.0", "signedMark");
+    if (signedMarkNodes.getLength() != 1) {
+      throw new XMLSignatureException(
+          "Expected exactly one <smd:signedMark> element in the document.");
+    }
+
+    String rootId = rootElement.getAttribute("id");
+    if (rootId.isEmpty()) {
+      throw new XMLSignatureException("Root signedMark element must have an id attribute.");
+    }
+
     NodeList signatureNodes = doc.getElementsByTagNameNS(XMLSignature.XMLNS, "Signature");
     if (signatureNodes.getLength() != 1) {
       throw new XMLSignatureException("Expected exactly one <ds:Signature> element.");
@@ -91,8 +113,20 @@ public class TmchXmlSignature {
     XMLSignatureFactory factory = XMLSignatureFactory.getInstance("DOM");
     KeyValueKeySelector selector = new KeyValueKeySelector(tmchCertificateAuthority);
     DOMValidateContext context = new DOMValidateContext(selector, signatureNodes.item(0));
+
+    // Explicitly register the root ID attribute in the validation context just in case
+    context.setIdAttributeNS(rootElement, null, "id");
+
     XMLSignature signature = factory.unmarshalXMLSignature(context);
 
+    // Verify that the signature Reference URI matches the root element ID ("#" + rootId)
+    String expectedUri = String.format("#%s", rootId);
+    List<Reference> references = signature.getSignedInfo().getReferences();
+    if (references.stream().noneMatch(ref -> expectedUri.equals(ref.getURI()))) {
+      throw new XMLSignatureException(
+          "Signature Reference URI does not match the root element ID.");
+    }
+
     boolean isValid;
     try {
       isValid = signature.validate(context);

core/src/test/java/google/registry/tmch/TmchXmlSignatureTest.java

@@ -16,6 +16,7 @@ package google.registry.tmch;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.tmch.TmchTestData.loadSmd;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import google.registry.config.RegistryConfig.ConfigModule.TmchCaMode;
@@ -139,4 +140,139 @@ class TmchXmlSignatureTest {
         assertThrows(CertificateRevokedException.class, () -> tmchXmlSignature.verify(smdData));
     assertThat(e).hasMessageThat().contains("Certificate has been revoked");
   }
+
+  // These tests check the structure of the decoded XML (unrelated to the decoding itself)
+  @Test
+  void testVerify_rootElementNotSignedMark_fails() {
+    String xml =
+        """
+        <?xml version="1.0" encoding="UTF-8"?>
+        <container>
+           <smd:signedMark xmlns:smd="urn:ietf:params:xml:ns:signedMark-1.0" id="id-1"/>
+        </container>
+        """;
+    XMLSignatureException e =
+        assertThrows(
+            XMLSignatureException.class, () -> tmchXmlSignature.verify(xml.getBytes(UTF_8)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "Root element must be signedMark in namespace urn:ietf:params:xml:ns:signedMark-1.0");
+  }
+
+  @Test
+  void testVerify_xswWrapping_fails() {
+    // By default, the verifier follows the reference from a valid signature wherever it goes. This
+    // could be a second signedMark object hidden elsewhere in the XML (say, inside a ds:Object,
+    // which can contain anything). The SignedMark parser, however, uses the root node. We need to
+    // make sure that the valid signature points to the root node, and not anything else.
+    String xswXml =
+        """
+        <?xml version="1.0" encoding="UTF-8"?>
+        <smd:signedMark xmlns:smd="urn:ietf:params:xml:ns:signedMark-1.0" id="fake-id">
+           <smd:id>fake-id</smd:id>
+           <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+              <ds:SignedInfo>
+                 <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+                 <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
+                 <ds:Reference URI="#real-id">
+                    <ds:Transforms>
+                       <ds:Transform Algorithm=\
+                       "http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
+                    </ds:Transforms>
+                    <ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
+                    <ds:DigestValue>dGVzdA==</ds:DigestValue>
+                 </ds:Reference>
+              </ds:SignedInfo>
+              <ds:SignatureValue>dGVzdA==</ds:SignatureValue>
+              <ds:Object>
+                 <smd:signedMark id="real-id">
+                    <smd:id>real-id</smd:id>
+                 </smd:signedMark>
+              </ds:Object>
+           </ds:Signature>
+        </smd:signedMark>
+        """;
+
+    XMLSignatureException e =
+        assertThrows(
+            XMLSignatureException.class, () -> tmchXmlSignature.verify(xswXml.getBytes(UTF_8)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains("Expected exactly one <smd:signedMark> element in the document");
+  }
+
+  @Test
+  void testVerify_signatureDoesNotSignRoot_fails() {
+    // The internal signature reference URI must match the root signed mark ID
+    String xml =
+        """
+        <?xml version="1.0" encoding="UTF-8"?>
+        <smd:signedMark xmlns:smd="urn:ietf:params:xml:ns:signedMark-1.0" id="modified-id">
+           <smd:id>modified-id</smd:id>
+           <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+              <ds:SignedInfo>
+                 <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+                 <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
+                 <ds:Reference URI="#original-id">
+                    <ds:Transforms>
+                       <ds:Transform Algorithm=\
+                       "http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
+                    </ds:Transforms>
+                    <ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
+                    <ds:DigestValue>dGVzdA==</ds:DigestValue>
+                 </ds:Reference>
+              </ds:SignedInfo>
+              <ds:SignatureValue>dGVzdA==</ds:SignatureValue>
+           </ds:Signature>
+        </smd:signedMark>
+        """;
+
+    XMLSignatureException e =
+        assertThrows(
+            XMLSignatureException.class, () -> tmchXmlSignature.verify(xml.getBytes(UTF_8)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains("Signature Reference URI does not match the root element ID");
+  }
+
+  @Test
+  void testVerify_multipleSignedMarks_fails() {
+    // Even if the signature does validate the root signed mark, it's sketchy at best to include
+    // another signed mark hidden in the XML. Don't allow it.
+    String xml =
+        """
+        <?xml version="1.0" encoding="UTF-8"?>
+        <smd:signedMark xmlns:smd="urn:ietf:params:xml:ns:signedMark-1.0" id="id-1">
+           <smd:id>id-1</smd:id>
+           <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+              <ds:SignedInfo>
+                 <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+                 <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
+                 <ds:Reference URI="#id-1">
+                    <ds:Transforms>
+                       <ds:Transform Algorithm=\
+                       "http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
+                    </ds:Transforms>
+                    <ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
+                    <ds:DigestValue>dGVzdA==</ds:DigestValue>
+                 </ds:Reference>
+              </ds:SignedInfo>
+              <ds:SignatureValue>dGVzdA==</ds:SignatureValue>
+              <ds:Object>
+                 <smd:signedMark id="id-2">
+                    <smd:id>id-2</smd:id>
+                 </smd:signedMark>
+              </ds:Object>
+           </ds:Signature>
+        </smd:signedMark>
+        """;
+
+    XMLSignatureException e =
+        assertThrows(
+            XMLSignatureException.class, () -> tmchXmlSignature.verify(xml.getBytes(UTF_8)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains("Expected exactly one <smd:signedMark> element in the document");
+  }
 }