[
https://issues.apache.org/jira/browse/WSS-727?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101667#comment-18101667
]
Colm O hEigeartaigh commented on WSS-727:
-----------------------------------------
Hi,
Can you attach the patch please as a pull request to
[https://github.com/apache/ws-wss4j/pulls] ?
> SignatureProcessor retains the entire attachment in heap when verifying an
> SwA signed attachment (cacheReference)
> -----------------------------------------------------------------------------------------------------------------
>
> Key: WSS-727
> URL: https://issues.apache.org/jira/browse/WSS-727
> Project: WSS4J
> Issue Type: Bug
> Components: WSS4J Core
> Affects Versions: 4.0.1
> Reporter: Ivan Novak
> Assignee: Colm O hEigeartaigh
> Priority: Major
> Attachments: LargeAttachmentRepro-1.java, wss4j-patch-1.diff
>
>
> Verifying a SOAP message with a signed SwA attachment requires roughly *2.3x
> the attachment size* in heap, regardless of whether the attachment is backed
> by disk and regardless of whether the caller supplies a properly streaming,
> mark-capable source stream. Signing the same message is streaming and needs a
> constant ~20 MB.
> The cause is {{SignatureProcessor.verifyXMLSignature()}} (ws-security-dom,
> line 353):
> {code:java}
> XMLValidateContext context = new DOMValidateContext(key, elem);
> context.setProperty("javax.xml.crypto.dsig.cacheReference", Boolean.TRUE);
> {code}
> In Santuario that single property controls two unrelated caches
> ({{{}DOMReference.transform(){}}}, xmlsec 4.0.4, DOMReference.java:474-476
> and :566):
> * the dereferenced {{Data}} - cheap: for a {{cid:}} URI the resolver returns
> a zero-length input, and it is the half WSS4J actually consumes
> ({{{}buildProtectedRefs(){}}} uses it to work out what each Reference
> covered, and for an attachment to synthesise the {{<attachment>}} marker
> element and set {{{}WSDataRef.setAttachment(true){}}});
> * the pre-digested input - {{DigesterOutputStream}} retains every octet fed
> to the digest in an {{UnsyncByteArrayOutputStream}} that grows by doubling,
> plus one more full array copy in {{{}getInputStream(){}}}.
> For an SwA reference the last transform
> ({{{}AttachmentContentSignatureTransform{}}}) writes the attachment straight
> into that stream, so a 200 MB attachment costs a 256 MB backing array plus a
> 200 MB copy - ~456 MB live at once. The buffered copy is dead weight: its
> only consumer is {{{}Reference.getDigestInputStream(){}}}, which nothing in
> WSS4J calls.
> h3. Reproduction
> Attached: {{LargeAttachmentRepro.java}} (self-contained, no test-jar
> dependencies).
> {code:java}
> java -Xmx<N> -cp <ws-security-dom test classpath> LargeAttachmentRepro
> sign|verify 200 <workdir>
> {code}
> It signs / verifies a 200 MB {{application/octet-stream}} attachment read
> from a file. The attachment's source stream implements
> {{{}mark(){}}}/{{{}reset(){}}} by re-opening the file, so it contributes zero
> heap - any growth observed is the digest-side cache alone.
> Environment: WSS4J 4.0.1, xmlsec 4.0.4, Temurin JDK 26.0.1, Linux.
> ||scenario||-Xmx||result||
> |sign, 200 MB attachment|256m|OK, peak used heap 22 MB|
> |verify, 200 MB attachment|256m|OutOfMemoryError|
> |verify, 200 MB attachment|400m|OutOfMemoryError (peak 394 MB)|
> |verify, 200 MB attachment|512m|OK, peak used heap *466 MB*|
> |verify, with attached patch|256m|OK, peak used heap *14 MB*|
> {code:java}
> java.lang.OutOfMemoryError: Java heap space
> at
> org.apache.xml.security.utils.UnsyncByteArrayOutputStream.expandSize(UnsyncByteArrayOutputStream.java:113)
> at
> org.apache.xml.security.utils.UnsyncByteArrayOutputStream.write(UnsyncByteArrayOutputStream.java:63)
> at
> org.apache.jcp.xml.dsig.internal.DigesterOutputStream.write(DigesterOutputStream.java:83)
> at
> org.apache.xml.security.utils.UnsyncBufferedOutputStream.write(UnsyncBufferedOutputStream.java:52)
> at
> org.apache.wss4j.dom.transform.AttachmentContentSignatureTransform.processAttachment(AttachmentContentSignatureTransform.java:218)
> at
> org.apache.wss4j.dom.transform.AttachmentContentSignatureTransform.transform(AttachmentContentSignatureTransform.java:122)
> at
> org.apache.jcp.xml.dsig.internal.dom.DOMTransform.transform(DOMTransform.java:170)
> at
> org.apache.jcp.xml.dsig.internal.dom.DOMReference.transform(DOMReference.java:488)
> at
> org.apache.jcp.xml.dsig.internal.dom.DOMReference.validate(DOMReference.java:415)
> at
> org.apache.jcp.xml.dsig.internal.dom.DOMXMLSignature.validate(DOMXMLSignature.java:295)
> at
> org.apache.wss4j.dom.processor.SignatureProcessor.verifyXMLSignature(SignatureProcessor.java:381)
> ...
> {code}
> h3. Proposed fix (attached: {{wss4j-patch.diff}} - WSS4J-only, no Santuario
> change required)
> The outer {{xmlSignature.validate(context)}} is replaced by the equivalent
> explicit loop - {{SignatureValue}} check plus per-Reference
> {{Reference.validate()}} - with {{cacheReference}} toggled off for SwA
> attachment References. This is behaviour-preserving: the property is read per
> Reference at transform time, {{Reference.validate()}} is public JSR-105 and
> runs at most once per Reference, short-circuit semantics are kept, and
> {{DOMXMLSignature.validate()}} does nothing else WSS4J relies on (Manifest
> validation is gated on {{{}org.jcp.xml.dsig.validateManifests{}}}, which
> WSS4J never sets - and Manifests are rejected by {{checkBSPCompliance}}
> anyway).
> The second half of the patch is required, not optional: with caching off,
> {{getDereferencedData()}} returns null, so {{buildProtectedRefs()}} must
> recognise attachment References by their Transform algorithm instead -
> toggling the flag without it makes every signed attachment fail verification
> with {{{}FAILED_CHECK{}}}. The resulting {{WSDataRef}} is byte-for-byte what
> the old code produced (same synthesised {{<attachment>}} element, same
> {{{}setAttachment(true){}}}).
> Non-attachment References keep {{{}cacheReference=true{}}}, so element
> recovery, the STR-dereference path (WSS-222) and the anti-wrapping checks
> that depend on it are untouched. The Santuario {{Reference}} objects never
> escape {{{}SignatureProcessor{}}}, and ws-security-stax does not use the
> property.
> Test results with the patch applied: {{mvn -pl ws-security-dom test}} - 563
> tests, 0 failures, 0 errors, 5 skipped (unchanged from baseline), including
> {{AttachmentTest}} (17), {{XOPAttachmentTest}} (13) and both negative
> attachment-signature tests; {{mvn -pl integration test}} - 15 tests, 0
> failures. The 200 MB verify above passes under {{-Xmx256m}} with a 14 MB peak.
> h3. Notes
> * This is WSS4J's defect, not Santuario's: the {{XMLValidateContext}}
> javadoc specifies {{cacheReference}} as one switch for both caches, so
> {{DOMReference}} cannot drop the digest-input half without breaking its
> contract. WSS4J asks for the full cache and then consumes only half of it.
> (Decoupling the caches upstream fixes the symptom too, but would have to be a
> new opt-in property plus a Santuario release; the WSS4J patch needs neither.)
> * Distinct from WSS-638, which is about {{processAttachment()}} buffering a
> non-mark-capable *source* stream via
> {{BufferedInputStream.mark(Integer.MAX_VALUE)}} and is still reproducible
> here on both the sign and verify paths. That one has a caller-side workaround
> (a disk-backed, mark-capable stream); this one has none, because the
> digest-side cache retains the attachment independently of the source stream.
> Both need fixing for large attachments to verify in bounded memory.
> * The {{cacheReference}} call has been present since the
> {{org.apache.wss4j}} rename (WSS4J 2.0, commit f647a91bd), so 2.x and 3.x are
> affected too; measured on 4.0.1 only.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
