shunkica opened a new pull request, #640:
URL: https://github.com/apache/ws-wss4j/pull/640

   JIRA: https://issues.apache.org/jira/browse/WSS-727
   
   ## The Problem
   
   When you verify a SOAP message with a signed SwA attachment, it needs about 
2.3x the attachment's size in heap memory. This happens even if the attachment 
is already on disk or if you provide a streaming source. Compare that to 
*signing* the same message, which only needs ~20 MB consistently. That's a huge 
difference.
   
   The culprit is a single flag in `SignatureProcessor.verifyXMLSignature()`:
   
   ```java
   XMLValidateContext context = new DOMValidateContext(key, elem);
   context.setProperty("javax.xml.crypto.dsig.cacheReference", Boolean.TRUE);
   ```
   
   This flag tells Santuario to cache stuff, but it's a blunt instrument. In 
`DOMReference.transform()` (xmlsec 4.0.4), it flips on *two* separate caches 
that have nothing to do with each other:
   
   ```java
   Boolean cache = (Boolean) 
context.getProperty("javax.xml.crypto.dsig.cacheReference");
   if (cache != null && cache) {
       this.derefData = copyDerefData(dereferencedData);
       dos = new DigesterOutputStream(md, true);   // <-- keeps every byte fed 
to the digest
   } else {
       dos = new DigesterOutputStream(md);
   }
   ```
   
   The two caches:
   1. **Dereferenced data** - cheap, and WSS4J actually needs this. It's used 
by `buildProtectedRefs()` to figure out what each Reference covered.
   2. **Pre-digested input** - the problem. The `DigesterOutputStream` buffers 
it all in an `UnsyncByteArrayOutputStream`, which doubles in size as it grows, 
then copies the whole thing again in `getInputStream()`.
   
   For attachments, `AttachmentContentSignatureTransform` feeds the attachment 
directly into that stream. So a 200 MB attachment ends up consuming a 256 MB 
backing array *plus* a 200 MB copy - ~456 MB live at once. This buffered copy 
is useless: the only thing that reads it is `Reference.getDigestInputStream()`, 
and WSS4J never calls that.
   
   ```
   java.lang.OutOfMemoryError: Java heap space
       at 
org.apache.xml.security.utils.UnsyncByteArrayOutputStream.expandSize(UnsyncByteArrayOutputStream.java:113)
       at 
org.apache.jcp.xml.dsig.internal.DigesterOutputStream.write(DigesterOutputStream.java:83)
       at 
org.apache.wss4j.dom.transform.AttachmentContentSignatureTransform.processAttachment(AttachmentContentSignatureTransform.java:218)
       ...
   ```
   
   ## The Fix
   
   The solution has two parts:
   
   1. **Replace `xmlSignature.validate(context)` with an explicit loop.** 
Instead of one call, we do:
      - Check the `SignatureValue`
      - Validate each `Reference` individually
      - For attachment References (identified by their Transform algorithm), 
toggle off `cacheReference`
      
   2. **Update `buildProtectedRefs()` to recognize attachments by their 
Transform algorithm.** This is essential. With caching off, 
`getDereferencedData()` returns `null`, so toggling the flag alone would break 
every signed attachment with `FAILED_CHECK`. The fix keeps 
`buildProtectedRefs()` working: it still produces the same `WSDataRef` with the 
synthesised `<attachment>` element and `setAttachment(true)`.
   
   ### Why this preserves the existing behavior:
   
   - **The flag is read per-Reference at transform time**, so we can toggle it 
between `Reference.validate()` calls without issues.
   
   - **`Reference.validate()` caches its result**, so when debug logging 
re-validates a Reference we already checked, it just uses the cached status, it 
doesn't re-transform. References the loop hasn't reached yet get validated 
there, and the caching rule applies to them too. This matters more than it 
sounds: if the SignatureValue check fails (a common failure), we bail out 
before validating any References. Without this, turning on debug logging to 
diagnose the failure would buffer every signed attachment on that message, 
leading to the very OutOfMemoryError we're fixing. The old code did exactly 
this.
   
   - **Non-attachment References still cache**, so element recovery, the 
STR-dereference path (WSS-222), and the anti-wrapping checks all keep working. 
We restore the flag to `TRUE` when we're done.
   
   - **The explicit loop is fully equivalent to `DOMXMLSignature.validate()`**: 
Manifest validation only runs if `org.jcp.xml.dsig.validateManifests` is set 
(WSS4J never sets it), and the XMLSignature's cached validation status is never 
read because the object doesn't escape `SignatureProcessor`.
   
   - **The short-circuit semantics match the old behavior** - still bail on 
first failure.
   
   - **`ws-security-stax` doesn't use this property**, so no impact there.
   
   ## Testing
   
   A new test, `AttachmentTest.testXMLAttachmentContentSignatureDataRef`, 
verifies that the `WSDataRef` for a signed attachment still has 
`isAttachment()` set and still carries the synthesised SwA `attachment` 
element. This ensures the `buildProtectedRefs()` change does what it's supposed 
to.
   
   ### Memory measurements
   
   200 MB attachment from the WSS-727 repro; source stream is 
mark/reset-capable so it contributes no heap
   
   | Scenario | Heap limit | Result |
   |----------|-----------|---------|
   | Sign 200 MB attachment | 256m | OK, peak 17 MB |
   | Verify 200 MB attachment (before fix) | 256m | OutOfMemoryError |
   | Verify 200 MB attachment (before fix) | 512m | OK, peak 466 MB |
   | Verify 200 MB attachment (after fix) | 256m | OK, peak 22 MB |
   
   ## Notes
   
   **History:** The `cacheReference` flag has been there since the WSS4J 2.0 
rename (commit f647a91bd), so the bug affects 2.x and 3.x as well. Tested on 
4.0.1 and current master.
   
   **The bigger picture:** The digest input buffer also costs ~3.2x the signed 
content for regular element References (minimum -Xmx: 160m vs 96m for a 20 MB 
Body, or 288m vs 160m for 40 MB). The key difference is asymptotic: element 
content is already in the DOM, so the cache is a constant-factor overhead you 
can outrun by raising heap; attachment content is streaming and otherwise never 
in memory, so the cache turns constant-heap streaming into O(attachment-size) 
that no fixed heap covers. This fix is narrowly scoped to SwA attachments - the 
only case with no caller-side workaround.
   
   For element and STR References, the dereferenced data is the authoritative 
record of what was digested and is essential for `WSDataRef` and the 
anti-wrapping checks. You can't just disable caching for those without losing 
that data. A proper fix for all Reference types would require Santuario to 
expose *separate* properties for the two caches instead of one toggle for both 
- that would let `buildProtectedRefs()` fetch the dereferenced data without 
buffering the digest input. That's a design change upstream that would need a 
new Santuario release.
   
   **Related:** WSS-638 is about `processAttachment()` buffering 
non-mark-capable source streams via 
`BufferedInputStream.mark(Integer.MAX_VALUE)`. That one still happens on both 
sign and verify, and it's not fixed here. The workaround is to supply a 
disk-backed, mark-capable stream. The bug we're fixing had no workaround 
because the digest-side cache retained the attachment regardless of how the 
stream behaved.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to