davsclaus commented on code in PR #26726:
URL: https://github.com/apache/camel/pull/26726#discussion_r4112510335
##########
components/camel-xmlsecurity/src/main/java/org/apache/camel/component/xmlsecurity/api/DefaultXmlSignature2Message.java:
##########
@@ -314,6 +359,111 @@ protected Node
getNodeForMessageBodyInEnvelopingCase(Input input) throws Excepti
return node;
}
+ /**
+ * Checks that a validated Reference actually covered the document element
the default search is about to emit.
+ * <p>
+ * Core signature validation only proves that each Reference's digest
matches the content that Reference resolves
+ * to. It says nothing about the rest of the document. So an attacker can
take a legitimately signed fragment, embed
+ * it unchanged inside a larger document of their own, and validation
still passes - the same-document URI resolves
+ * to that fragment exactly as before - while this method would hand the
whole attacker document downstream as
+ * verified content. That is XML signature wrapping.
+ * <p>
+ * The check is deliberately narrow, so that it rejects that shape and
nothing else. It only complains when the
+ * signature carries same-document references and none of them covers the
document element. A Reference with an
+ * empty URI covers the whole document, and a signature whose References
are all external says nothing about this
+ * document either way, so both are left alone.
+ *
+ * @param input the verification input, carrying the validated
References
+ * @param documentElement the element the default search would emit
+ */
+ protected void checkDocumentElementIsCoveredByAReference(Input input,
Element documentElement) throws Exception {
+ List<Reference> references = getReferencesForMessageMapping(input);
+ if (references == null || references.isEmpty()) {
+ return;
+ }
+
+ boolean sameDocumentReferenceSeen = false;
+ for (Reference reference : references) {
+ String uri = reference.getURI();
+ if (uri == null) {
+ // Absent URI (getURI() == null per JSR-105) identifies the
whole document per the XML Signature
+ // spec, the same as URI="". However, treating it as
whole-document coverage here would let an
+ // attacker bypass the check by attaching a null-URI
reference, so we skip it conservatively:
+ // a lone absent-URI reference leaves
sameDocumentReferenceSeen false and the document is rejected.
Review Comment:
Two inaccuracies here. (1) XMLDSig does not give an omitted URI a referent —
it is the "application knows the object" case; only `URI=""` is the whole
document. (2) A lone absent-URI reference leaves `sameDocumentReferenceSeen`
false, so the method returns without throwing — the document is left alone
(like the all-external case), not rejected.
```suggestion
// An absent URI tells us nothing about this document:
XMLDSig leaves its referent to the application,
// unlike URI="", which is the whole document. Like an
external reference below, it must not
// short-circuit the check for the references that follow
it. A signature whose references are all
// absent or external leaves sameDocumentReferenceSeen false
and is left alone, as described above.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
