Summary
Make removesNodes required on CanonicalizationOrTransformationAlgorithm. It shipped as optional in 6.x because it could not be required without breaking every custom algorithm; 7.0 is where that gets fixed.
Background
computeSignature() rejects a reference that encloses the signature when every transform on it provably cannot remove the Signature — otherwise the digest covers a Signature whose SignatureValue is still empty, and the result can never verify. Each algorithm declares that about itself:
readonly removesNodes = false; // canonicalization: renders the node-set, drops nothing
readonly removesNodes = true; // enveloped-signature: removes the Signature
The problem with optional
Three states exist today, and the third is the bad one:
removesNodes |
Meaning |
Check |
false |
provably preserves the Signature |
applies |
true |
may remove it |
defers |
| undefined |
we assume it may remove it |
defers |
That third row is a default making a security-relevant decision for the implementer. Someone writing a pure canonicalization transform gets no protection and is never told — they did not choose to opt out, they just did not know the field existed. This is the shape AGENTS.md warns about: "Don't add a default for anything security-relevant. Throw instead."
Requiring it removes the row. Every algorithm states what it does, and the check is exact for custom algorithms and built-ins alike.
Scope
- Change
removesNodes?: boolean to removesNodes: boolean in src/types.ts.
- Drop the
algo != null && / === false defensiveness in SignedXml.preservesEveryNode(); the value is then always present.
- Note the break in the changelog with the migration below.
Migration
Add one line to any custom canonicalization or transform:
class MyTransform implements CanonicalizationOrTransformationAlgorithm {
readonly removesNodes = false; // or true, if it drops nodes from the node-set
process(node: Node) { /* ... */ }
getAlgorithmName() { return "http://MyTransformation"; }
}
Pick false only if the algorithm returns the same node-set it was given. If it filters nodes at all — an XPath transform, say — use true, which is also the conservative choice: it keeps the current deferring behaviour.
TypeScript implementors get a compile error naming the missing property, so the break is loud and mechanical. JavaScript implementors get no such signal, which is worth a prominent changelog entry: for them the field is silently undefined, and the runtime behaviour is unchanged from 6.x rather than newly broken.
Worth deciding alongside
Whether preservesEveryNode() should throw on an unregistered algorithm rather than returning false. Today an unknown URI defers here and then fails later in findCanonicalizationAlgorithm() with "canonicalization algorithm is not supported". That ordering is fine, but if the errors are ever reworked, the two paths should agree on which one reports first.
Related
Summary
Make
removesNodesrequired onCanonicalizationOrTransformationAlgorithm. It shipped as optional in 6.x because it could not be required without breaking every custom algorithm; 7.0 is where that gets fixed.Background
computeSignature()rejects a reference that encloses the signature when every transform on it provably cannot remove theSignature— otherwise the digest covers aSignaturewhoseSignatureValueis still empty, and the result can never verify. Each algorithm declares that about itself:The problem with optional
Three states exist today, and the third is the bad one:
removesNodesfalseSignaturetrueThat third row is a default making a security-relevant decision for the implementer. Someone writing a pure canonicalization transform gets no protection and is never told — they did not choose to opt out, they just did not know the field existed. This is the shape AGENTS.md warns about: "Don't add a default for anything security-relevant. Throw instead."
Requiring it removes the row. Every algorithm states what it does, and the check is exact for custom algorithms and built-ins alike.
Scope
removesNodes?: booleantoremovesNodes: booleaninsrc/types.ts.algo != null &&/=== falsedefensiveness inSignedXml.preservesEveryNode(); the value is then always present.Migration
Add one line to any custom canonicalization or transform:
Pick
falseonly if the algorithm returns the same node-set it was given. If it filters nodes at all — an XPath transform, say — usetrue, which is also the conservative choice: it keeps the current deferring behaviour.TypeScript implementors get a compile error naming the missing property, so the break is loud and mechanical. JavaScript implementors get no such signal, which is worth a prominent changelog entry: for them the field is silently
undefined, and the runtime behaviour is unchanged from 6.x rather than newly broken.Worth deciding alongside
Whether
preservesEveryNode()should throw on an unregistered algorithm rather than returningfalse. Today an unknown URI defers here and then fails later infindCanonicalizationAlgorithm()with "canonicalization algorithm is not supported". That ordering is fine, but if the errors are ever reworked, the two paths should agree on which one reports first.Related
fix/omit-empty-transforms-cleanbranch (PR fix: omit <Transforms> element when no transforms are specified #542).