diff --git a/core/src/main/java/google/registry/flows/domain/DomainFlowTmchUtils.java b/core/src/main/java/google/registry/flows/domain/DomainFlowTmchUtils.java index bbeae85d54c..d1c4649cbc2 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowTmchUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowTmchUtils.java @@ -101,8 +101,9 @@ public SignedMark verifyEncodedSignedMark(EncodedSignedMark encodedSignedMark, I throw new SignedMarkRevokedErrorException(); } + String signedElementId; try { - tmchXmlSignature.verify(signedMarkData); + signedElementId = tmchXmlSignature.verify(signedMarkData); } catch (CertificateExpiredException e) { throw new SignedMarkCertificateExpiredException(e); } catch (CertificateNotYetValidException e) { @@ -122,6 +123,10 @@ public SignedMark verifyEncodedSignedMark(EncodedSignedMark encodedSignedMark, I throw new SignedMarkParsingErrorException(e); } + if (!signedElementId.equals(signedMark.getXsdId())) { + throw new SignedMarkSignatureException(new Exception("Signature reference ID mismatch.")); + } + if (now.isBefore(signedMark.getCreationTime())) { throw new FoundMarkNotYetValidException(); } diff --git a/core/src/main/java/google/registry/model/smd/SignedMark.java b/core/src/main/java/google/registry/model/smd/SignedMark.java index d67a51027c2..662cb9b0f23 100644 --- a/core/src/main/java/google/registry/model/smd/SignedMark.java +++ b/core/src/main/java/google/registry/model/smd/SignedMark.java @@ -78,6 +78,10 @@ public Mark getMark() { return mark; } + public String getXsdId() { + return xsdId; + } + public boolean hasSignature() { return xmlSignature != null; } diff --git a/core/src/main/java/google/registry/tmch/TmchXmlSignature.java b/core/src/main/java/google/registry/tmch/TmchXmlSignature.java index 56a27db5918..3a51115525c 100644 --- a/core/src/main/java/google/registry/tmch/TmchXmlSignature.java +++ b/core/src/main/java/google/registry/tmch/TmchXmlSignature.java @@ -51,6 +51,7 @@ 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; @@ -78,12 +79,33 @@ public TmchXmlSignature(TmchCertificateAuthority tmchCertificateAuthority) { * @throws GeneralSecurityException for unsupported protocols, certs not signed by the TMCH, * incorrect keys, and for invalid, old, not-yet-valid or revoked certificates. */ - public void verify(byte[] smdXml) - throws GeneralSecurityException, IOException, MarshalException, ParserConfigurationException, - SAXException, XMLSignatureException { + public String verify(byte[] smdXml) + throws GeneralSecurityException, + IOException, + MarshalException, + ParserConfigurationException, + SAXException, + XMLSignatureException { checkArgument(smdXml.length > 0); Document doc = parseSmdDocument(new ByteArrayInputStream(smdXml)); + Element rootElement = doc.getDocumentElement(); + if (!"signedMark".equals(rootElement.getLocalName()) + || !"urn:ietf:params:xml:ns:signedMark-1.0".equals(rootElement.getNamespaceURI())) { + throw new XMLSignatureException("Invalid root element name or namespace."); + } + String rootId = rootElement.getAttribute("id"); + if (rootId.isEmpty()) { + throw new XMLSignatureException("Root element missing id attribute."); + } + + // Verify that exactly one element exists in the DOM + NodeList smdNodes = + doc.getElementsByTagNameNS("urn:ietf:params:xml:ns:signedMark-1.0", "signedMark"); + if (smdNodes.getLength() != 1) { + throw new XMLSignatureException("Expected exactly one element."); + } + NodeList signatureNodes = doc.getElementsByTagNameNS(XMLSignature.XMLNS, "Signature"); if (signatureNodes.getLength() != 1) { throw new XMLSignatureException("Expected exactly one element."); @@ -103,6 +125,23 @@ public void verify(byte[] smdXml) if (!isValid) { throw new XMLSignatureException(explainValidationProblem(context, signature)); } + + @SuppressWarnings("unchecked") + List references = signature.getSignedInfo().getReferences(); + if (references.isEmpty()) { + throw new XMLSignatureException("No references found in signature."); + } + Reference reference = references.get(0); + String uri = reference.getURI(); + if (uri == null || !uri.startsWith("#")) { + throw new XMLSignatureException("Invalid signature reference URI: " + uri); + } + String signedElementId = uri.substring(1); + if (!signedElementId.equals(rootId)) { + throw new XMLSignatureException( + String.format("Signature reference ID mismatch: expected #%s, got %s", rootId, uri)); + } + return signedElementId; } private static Document parseSmdDocument(InputStream input) diff --git a/core/src/test/java/google/registry/tmch/TmchXmlSignatureTest.java b/core/src/test/java/google/registry/tmch/TmchXmlSignatureTest.java index 417ad6ecd41..48ea8d10d5d 100644 --- a/core/src/test/java/google/registry/tmch/TmchXmlSignatureTest.java +++ b/core/src/test/java/google/registry/tmch/TmchXmlSignatureTest.java @@ -23,17 +23,26 @@ import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.testing.FakeClock; import google.registry.tmch.TmchXmlSignature.CertificateSignatureException; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.security.cert.CertificateRevokedException; import java.time.Instant; import javax.xml.crypto.dsig.XMLSignatureException; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.stream.StreamResult; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.junitpioneer.jupiter.cartesian.CartesianTest; import org.junitpioneer.jupiter.cartesian.CartesianTest.Values; +import org.w3c.dom.Document; +import org.w3c.dom.Element; /** * Unit tests for {@link TmchXmlSignature}. @@ -139,4 +148,37 @@ void testIndTmvRevoked(String language) { assertThrows(CertificateRevokedException.class, () -> tmchXmlSignature.verify(smdData)); assertThat(e).hasMessageThat().contains("Certificate has been revoked"); } + + @Test + void testVerify_signatureReferenceIdMismatch() throws Exception { + smdData = loadSmd("smd/active.smd"); + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + Document originalDoc = dbf.newDocumentBuilder().parse(new ByteArrayInputStream(smdData)); + Element originalRoot = originalDoc.getDocumentElement(); + + // Create a new document to construct the XML Signature Wrapping (XSW) payload + Document forgedDoc = dbf.newDocumentBuilder().newDocument(); + Element forgedRoot = + forgedDoc.createElementNS("urn:ietf:params:xml:ns:signedMark-1.0", "smd:signedMark"); + forgedRoot.setAttribute("id", "forged-id"); + forgedRoot.setIdAttribute("id", true); + forgedDoc.appendChild(forgedRoot); + + // Import the original valid signedMark element as a child of the forged root + Element importedChild = (Element) forgedDoc.importNode(originalRoot, true); + forgedRoot.appendChild(importedChild); + // Ensure the DOM resolver treats the original ID attribute correctly + importedChild.setIdAttribute("id", true); + + // Serialize the XSW DOM back to bytes + Transformer transformer = TransformerFactory.newInstance().newTransformer(); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + transformer.transform(new DOMSource(forgedDoc), new StreamResult(bos)); + byte[] manipulatedData = bos.toByteArray(); + + XMLSignatureException e = + assertThrows(XMLSignatureException.class, () -> tmchXmlSignature.verify(manipulatedData)); + assertThat(e).hasMessageThat().contains("Expected exactly one element"); + } }