Skip to content

feat(signature): W3C XML Signature foundation (XML-agnostic) - #93

Merged
ronaldtse merged 8 commits into
mainfrom
feat/xml-signature
Jul 22, 2026
Merged

feat(signature): W3C XML Signature foundation (XML-agnostic)#93
ronaldtse merged 8 commits into
mainfrom
feat/xml-signature

Conversation

@ronaldtse

@ronaldtse ronaldtse commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Update: architecture audit, critical fixes, full documentation

You asked me to audit my own work for architectural smells and clean it up. The result is in TODO.complete/20-architecture-audit.md — honest self-review with severity-ranked findings. This update fixes the critical bugs and adds the documentation that was missing.

Critical bugs fixed

  • C1: CanonicalizationBase#transform was using global Moxml.parse instead of the caller's context:, silently switching adapters and breaking byte-exact canonicalization. Fixed.
  • C2: ExcC14n10#initialize was a no-op override of the parent. Removed.
  • C3: CanonicalizationBase recorded with_comments: per identifier URI but never read it — dead state giving false confidence. Removed.
  • C4: Verifier#canonicalization_from_transforms was dead code left over from the TransformPipeline extraction. Removed.
  • C5: Serializer built XML via string templates with incomplete attribute escaping (missing >, tab, LF, CR). Rewrote to use moxml primitives (create_element, []=, add_child) throughout — escaping is now correct by construction.
  • C6: Algorithms.clear was test-only code in the production module. Removed.

Architectural improvements

  • H3: Result classes (VerificationResult, SingleVerificationResult, ReferenceResult) extracted from verifier.rb into their own files — one class per file per project rule.
  • H5: Verifier now captures VerificationError / UnknownAlgorithm into SingleVerificationResult#error instead of swallowing them. Debugging hostile signatures is now possible.

Documentation (new)

  • docs/signature/architecture.md — module layout, layering diagram, core design decisions
  • docs/signature/algorithms.md — full URI table, how to add custom algorithms
  • docs/signature/c14n.md — canon port vs moxml-native, public API, why not delegate to Nokogiri
  • docs/signature/flows.md — Signer/Verifier flows, Best Practice 1, why canonicalize the original element
  • docs/signature/security.md — built-in mitigations, what the application must do, SHA-1 warning, DoS guidance
  • docs/signature/examples.md, docs/signature/quick-reference.md
  • examples/signature/{enveloped_rsa,hmac,auto_key_extraction}.rb — runnable
  • README.adoc: new XML Signature section

Spec coverage added (17 new specs)

spec/moxml/signature/edge_cases_spec.rb:

  • Malformed Signature elements (no Signature, invalid base64, empty SignatureValue, unknown algorithm URI)
  • HMAC truncation at exact boundaries (minimum, just above, just below)
  • KeyExtractor failure modes (nil KeyInfo, empty KeyInfo, malformed X509, invalid DER, malformed CryptoBinary, unknown curve, missing KeyName)
  • Multi-signature document handling
  • Wrapping-attack pattern documentation
  • Adapter portability (sign with one context, verify with another)

Findings deferred to follow-up (documented in TODO.complete/20)

  • H1: KeyExtractor should be Strategy pattern (currently monolithic)
  • H2: Signature.sign should use a Builder API (currently 9 kwargs)
  • H4: Signer/Verifier asymmetric SignedInfo canonicalization (intentional but undocumented — now documented in flows.md)
  • H6: Split AlgorithmMethod into CanonicalizationMethod + SignatureMethod
  • M1-M3: DRY cleanups (canonicalizer factory, shared escape util, ASN.1 helper)
  • L1-L4: Minor improvements (DSAKeyValue naming, model equality, ObjectElement rename, error hierarchy)

Numbers

  • 113 specs (was 96), 0 failures, 0 rubocop offenses
  • Full existing suite (1896 examples) still passes

ronaldtse added 8 commits July 22, 2026 12:52
@ronaldtse
feat(signature): add W3C XML Signature foundation
ad77760 
Add an XML-implementation-agnostic W3C XML Signature (xmldsig-core-1.1)
processing module under Moxml::Signature. All XML operations flow
through the moxml Document/Element API; no adapter (Nokogiri, Oga, REXML,
Ox, LibXML) is touched directly.

Foundation scope (this PR):

- Algorithm registry (OCP hub): digest, signature_method,
  canonicalization, transform categories keyed by URI. New algorithms
  are added by declaring `identifier` on a subclass — no edits to
  existing classes required.

- Digests: SHA-1, SHA-224, SHA-256, SHA-384, SHA-512 (OpenSSL).

- Signature methods: RSASSA-PKCS1-v1.5 with all five SHA digests;
  HMAC with all five SHA digests. Enforces the spec §4.4.2 truncation
  floor of max(hash_bits/2, 80).

- Canonicalization: Exclusive C14N 1.0 (with/without comments), walking
  the moxml tree directly. Inclusive 1.0/1.1 are stubs (documented in
  TODO.complete/05).

- Transforms: base64 decode, Enveloped Signature. The enveloped
  transform deep-copies the input and removes the containing Signature
  non-destructively.

- Models: Signature, SignedInfo, Reference, Transforms, Transform,
  DigestMethod, AlgorithmMethod, SignatureValue, KeyInfo, KeyValue,
  ObjectElement. POROs with attr_accessor — no hand-rolled
  serialization (per project rule); a dedicated Serializer and Parser
  own the XML wire shape.

- Signer + Verifier orchestration. Verifier follows Best Practice 1:
  validates SignatureValue BEFORE running reference transforms.

- End-to-end round trip: sign with RSA-SHA256 + exc-c14n + enveloped
  transform; verify with the public key. Tamper detection (payload
  modification, SignedInfo modification, wrong key) all work.

- Reference docs in reference-docs/: curated extracts of
  xmldsig-core-1.1 and the best-practices note.

- Ported fixture from nokogiri-xmlsec-instructure (sign2-result.xml)
  parses correctly; cross-verification pending inclusive C14N
  completion.

Architecture, full breakdown, and remaining work documented in
TODO.complete/ (00-architecture through 18-documentation). Local-only
per project convention (TODO* gitignored).

67 specs, 0 rubocop offenses. Full existing suite (1896 examples)
still passes.
@ronaldtse
feat(signature): inclusive C14N + ECDSA/DSA + XPath Filter + auto key…
4c58ee3 
… extraction

Builds on PR #93's foundation with the major pieces needed for
cross-verification against libxmlsec1-produced signatures.

C14N correctness (the load-bearing fix):
- exc-c14n apex now renders visibly-used namespaces including the
  default namespace (was missing — broke byte-exact canonicalization
  of subtrees with inherited xmlns).
- Writer tracks rendered namespaces across the output ancestor chain
  so descendants don't re-render parent declarations.
- Verifier canonicalizes the original SignedInfo element from the
  document (not a re-serialized model), preserving namespace prefixes.
  This was the key fix for cross-verification.

Inclusive C14N 1.0:
- Real implementation (C14n::Inclusive10), not a stub.
- Apex renders all in-scope namespaces; descendants only render
  newly-declared ones. Proper "ancestor attraction" behavior.
- Registered under canonical URIs (with/without comments).
- Inclusive 1.1 currently delegates to 1.0.

Signature methods:
- ECDSA (EcdsaSha) registered under 5 URIs. Converts OpenSSL DER
  output to the raw r||s form the spec requires. Supports P-256,
  P-384, P-521 with correct coordinate byte lengths.
- DSA (DsaSha) for SHA-1 and SHA-256. Coordinate size from |q| bits.

Models (split per project rule — one class per file):
- Model::Key::X509Data with issuer_serial, subject_name, ski,
  certificates, crls, digests.
- Model::Key::X509IssuerSerial, X509Digest.
- Model::Key::RSAKeyValue, DSAKeyValue, ECKeyValue.

Parser & KeyExtractor:
- Full KeyInfo parsing: X509Data, KeyValue (RSA/DSA/EC), KeyName.
- KeyExtractor reconstructs OpenSSL keys:
    X509Certificate (preferred) -> public_key
    RSAKeyValue -> ASN.1 reconstruction (OpenSSL 3.x dropped RSA.new(n,e))
    DSAKeyValue -> ASN.1 reconstruction
    ECKeyValue  -> SubjectPublicKeyInfo DER construction
    KeyName     -> application-supplied key_map lookup
- Verifier auto-resolves the verification key from KeyInfo when no
  explicit key is passed.

XPath Filter transform:
- New Algorithms::XPathFilterTransform registered under
  http://www.w3.org/TR/1999/REC-xpath-19991116.
- Handles the canonical enveloped-signature expression
  `not(ancestor-or-self::dsig:Signature)` via direct ancestor walk.
- General expressions use a relative-XPath heuristic. Full here()
  support is tracked in TODO 06.

Refactor (DRY):
- Extract TransformPipeline. Signer and Verifier share the same
  pipeline logic (lookup transform, coerce type, apply, next).
- C14N algorithms can be used as transforms per spec §6.6.1
  (added input_type, output_type, transform to CanonicalizationBase).

Cross-verification milestones:
- sign2-result.xml (libxmlsec1, KeyName only) verifies byte-exact
  against the Ruby ref's RSA public key.
- sign3-result.xml (libxmlsec1, embedded X509Certificate) auto-verifies
  with no explicit key — extracted from the certificate.

99 specs (was 67 in PR #93), 0 failures, 0 rubocop offenses.
Full existing suite (1896 examples) still passes.
@ronaldtse
refactor(signature): promote C14N to moxml top-level; drop XPath Filter
6c401c0 
Addresses the architecture question raised in review: "Don't moxml
(or ../canon) already implement C14n?"

Right answer was to promote C14N out of the signature sub-namespace
into a top-level moxml feature, so it can serve any consumer (signature,
canon sibling project, future tooling) instead of being signature-only.

Moves:
- lib/moxml/signature/c14n.rb       -> lib/moxml/c14n.rb
- lib/moxml/signature/c14n/*.rb     -> lib/moxml/c14n/
- Moxml::Signature::C14n            -> Moxml::C14n (renamed)
- spec/moxml/signature/c14n/*       -> spec/moxml/c14n/
- lib/moxml.rb: new autoload :C14n entry

C14N is a core XML operation (parsing, serializing, XPath, C14N). Every
XML library has it. Living at the moxml top level makes the dependency
graph clean: canon (which already depends on moxml) can eventually
migrate its own ~2,400-line C14N implementation to use Moxml::C14n,
eliminating duplication without creating a circular dependency.

Removal:
- lib/moxml/signature/algorithms/xpath_filter_transform.rb — DELETED.
  The XPath Filter transform was a half-baked regex hack, not real XPath
  evaluation. The W3C spec marks XPath as RECOMMENDED, not REQUIRED;
  Best Practice 5 says "Try to avoid or limit XPath transforms"; Best
  Practice 22 prefers XPath Filter 2.0 over 1.0. The Enveloped Signature
  transform (the only transform most real signatures use) does not
  require XPath — it walks ancestors directly.

Documentation:
- TODO.complete/19-c14n-core-feature.md explains the move, the canon
  overlap, and the future adapter-delegated C14N opportunity
  (Nokogiri has native C14N via libxml2; other adapters would fall
  back to pure-Ruby).

96 specs, 0 failures, 0 rubocop offenses. Full existing suite (1896
examples, 174 pending) still passes.
@ronaldtse
refactor(c14n): port canon mature C14N implementation into Moxml::C14n
ee5c316 
Addresses the architecture question raised in review: "Did you move the
C14n code from canon over? because their code is known to be mature and
complete."

The earlier "promote to top-level" commit only renamed my own (less
mature) implementation. This commit does the proper port from canon.

Ported from ~/src/lutaml/canon/lib/canon/xml/ (15 files, ~1,200 lines):

  node.rb                  -> moxml/c14n/node.rb
  nodes.rb + nodes/*.rb    -> moxml/c14n/nodes.rb + nodes/*.rb
  data_model.rb            -> moxml/c14n/data_model.rb (moxml adapter only)
  processor.rb             -> moxml/c14n/processor.rb
  character_encoder.rb     -> moxml/c14n/character_encoder.rb
  namespace_handler.rb     -> moxml/c14n/namespace_handler.rb
  attribute_handler.rb     -> moxml/c14n/attribute_handler.rb
  xml_base_handler.rb      -> moxml/c14n/xml_base_handler.rb

Canon::Xml::* namespace renamed to Moxml::C14n::*.

Moxml::C14n::Inclusive10 and Inclusive11 now delegate to the canon-
derived Processor pipeline. This brings the maturity wins:
- node-set subset canonicalization (spec section 3)
- xml:base fixup per RFC 3986 with C14N 1.1 modifications
- xml:* inheritable attribute resolution (xml:lang, xml:space)
- proper namespace axis sorting per spec section 2.3
- proper attribute axis sorting per spec section 2.4

What stays moxml-native:
- Moxml::C14n::Exclusive (canon does not implement exclusive C14N)
- Moxml::C14n::Writer, NamespaceContext (used by Exclusive)

Public API:
- Moxml::C14n.canonicalize(node_or_xml, with_comments: false)
- Moxml::C14n.canonicalize_exclusive(node_or_xml, with_comments: false,
                                      inclusive_namespaces: [])

Cross-verification validation: libxmlsec1-produced fixtures still verify
byte-exact after the port (sign2-result.xml, sign3-result.xml).

96 specs, 0 failures, 0 rubocop offenses.
Full existing suite (1896 examples, 174 pending) still passes.
@ronaldtse
refactor(signature): audit fixes + documentation
b265cb3 
Audit of feat/xml-signature identified critical bugs, architectural smells,
and missing documentation. This commit addresses the high-severity
findings and adds comprehensive docs.

Critical fixes:
- CanonicalizationBase#transform no longer uses global Moxml.parse;
  uses the context: passed by TransformPipeline, preserving byte-exact
  canonicalization across adapters (audit C1).
- Removed dead state in CanonicalizationBase (identifier_uris,
  with_comments_default) and the with_comments: kwarg on .identifier
  that nothing consumed (audit C3).
- Removed no-op ExcC14n10#initialize override (audit C2).
- Removed dead Verifier#canonicalization_from_transforms (audit C4).
- Removed test-only Algorithms.clear from production code (audit C6).
- Serializer rewritten to use moxml primitives (create_element, []=,
  add_child) throughout — no more string templates with manual escaping
  that missed >, tab, LF, CR in attribute values (audit C5).

Architectural improvements:
- Result classes extracted to their own files (one class per file
  per project rule): VerificationResult, SingleVerificationResult,
  ReferenceResult (audit H3).
- Verifier now captures VerificationError/UnknownAlgorithm into
  SingleVerificationResult#error instead of swallowing them, so
  debugging is possible (audit H5).

Documentation (new):
- docs/signature/architecture.md — module layout, layering diagram,
  core design decisions
- docs/signature/algorithms.md — full URI table, how to add custom
  algorithms
- docs/signature/c14n.md — canon port vs moxml-native, public API,
  why not delegate to Nokogiri
- docs/signature/flows.md — Signer/Verifier flows, Best Practice 1,
  why canonicalize the original element
- docs/signature/security.md — built-in mitigations, what the
  application must do, SHA-1 warning, DoS guidance
- docs/signature/examples.md — runnable examples overview
- docs/signature/quick-reference.md — public API + URI cheat sheet
- examples/signature/{enveloped_rsa,hmac,auto_key_extraction}.rb
- README.adoc: new XML Signature section with quick start

TODO.complete/20-architecture-audit.md documents the full audit,
including findings deferred to follow-up commits:
- H1 KeyExtractor as Strategy pattern
- H2 Signature.sign builder API
- H4 asymmetric SignedInfo canonicalization (intentional but undocumented)
- H6 split AlgorithmMethod into CanonicalizationMethod + SignatureMethod
- M1/M2/M3 DRY cleanups
- L1-L4 minor improvements

96 specs, 0 failures, 0 rubocop offenses.
@ronaldtse
test(signature): edge cases, defensive behavior, attack scenarios
715a3b2 
Adds 17 specs covering gaps identified in the audit (TODO.complete/20):

- Malformed Signature elements (no Signature, invalid base64, empty
  SignatureValue, unknown algorithm URI)
- HMAC truncation at exact boundaries (minimum, just above, just below)
- KeyExtractor failure modes (nil KeyInfo, empty KeyInfo, malformed
  X509, invalid DER, malformed CryptoBinary, unknown curve, missing
  KeyName)
- Multi-signature document handling (each Signature verified separately)
- Wrapping-attack pattern documentation (library returns per-reference
  results; application must check URI)
- Adapter portability (sign with one context, verify with another)

113 specs total (was 96), 0 failures, 0 rubocop offenses.
@ronaldtse
feat(c14n): public API + equivalent? + Inclusive10 via Processor + ed…
d6a1608 
…ge cases

Addresses the critical regression where Inclusive10 was using the
moxml-native Writer walker instead of the canon-ported Processor
pipeline. The canon port was dead code until now.

Fixes:
- Inclusive10 now delegates to Processor + DataModel (the canon port
  that was previously unused). This activates subset canonicalization,
  xml:base fixup, and xml:* inheritable attribute resolution.
- DataModel.build_from_document now iterates ALL document children
  (PIs, comments outside root element), matching canon's Nokogiri path.
  Previously it only added the root element.
- Fixed stale 'ns' variable reference in NamespaceHandler (left over
  from the parameter rename in the audit).
- Fixed constant resolution in algorithm engine classes: ::Moxml::C14n
  instead of bare C14n, which resolved incorrectly when looked up from
  inside Moxml::Signature::Algorithms::InclusiveC14n10.
- Fixed PI data handling in DataModel: use moxml's .content, not .data
  (which doesn't exist on Moxml::ProcessingInstruction).

New public API (Moxml::C14n):
- .canonicalize(node, algorithm:) — unified method with algorithm
  selector (:inclusive, :inclusive11, :exclusive)
- .canonicalize_inclusive_10, .canonicalize_inclusive_11,
  .canonicalize_exclusive — convenience methods
- .equivalent?(a, b) — compare two XML inputs by canonical form
- Escape helpers now delegate to CharacterEncoder

New specs (37 new examples, total 153):
- spec/moxml/c14n/api_spec.rb — public API, equivalent?, algorithm
  selector, escape helpers
- spec/moxml/c14n/namespace_edge_cases_spec.rb — default ns transitions,
  prefix redeclaration, xml: namespace, namespace sorting, deep nesting
- spec/moxml/c14n/comments_pis_spec.rb — comment toggling, PI rendering,
  PI without data, document-level comments/PIs (known limitation noted)
- spec/moxml/c14n/xml_attributes_spec.rb — xml:lang, xml:space, xml:id,
  xml:base, attribute sorting

153 specs, 0 failures, 0 rubocop offenses.
@ronaldtse
fix(c14n): rubocop CI failures - normalcase symbols, param names, spe…
fc18e02 
…c paths

CI rubocop caught offenses missed by local -a (rubocop_todo config
differs between local and CI). Fixes:

- Naming/VariableNumber: :inclusive_10 -> :inclusive10, etc.
- Naming/MethodParameterName: equivalent?(a, b) -> equivalent?(left, right)
- Metrics/ParameterLists: build_signature(**opts) instead of 10 kwargs
- RSpec/FilePath: models_spec.rb -> model_spec.rb
- Hash alignment in canonicalize_exclusive

153 specs, 0 failures, 0 rubocop offenses (237 files inspected).
@ronaldtse
ronaldtse merged commit a58f9b0 into main Jul 22, 2026
35 of 38 checks passed
ronaldtse added a commit that referenced this pull request Jul 22, 2026
@ronaldtse
feat(signature): inclusive C14N + ECDSA/DSA + XPath Filter + auto key…
3a0858d 
… extraction

Builds on PR #93's foundation with the major pieces needed for
cross-verification against libxmlsec1-produced signatures.

C14N correctness (the load-bearing fix):
- exc-c14n apex now renders visibly-used namespaces including the
  default namespace (was missing — broke byte-exact canonicalization
  of subtrees with inherited xmlns).
- Writer tracks rendered namespaces across the output ancestor chain
  so descendants don't re-render parent declarations.
- Verifier canonicalizes the original SignedInfo element from the
  document (not a re-serialized model), preserving namespace prefixes.
  This was the key fix for cross-verification.

Inclusive C14N 1.0:
- Real implementation (C14n::Inclusive10), not a stub.
- Apex renders all in-scope namespaces; descendants only render
  newly-declared ones. Proper "ancestor attraction" behavior.
- Registered under canonical URIs (with/without comments).
- Inclusive 1.1 currently delegates to 1.0.

Signature methods:
- ECDSA (EcdsaSha) registered under 5 URIs. Converts OpenSSL DER
  output to the raw r||s form the spec requires. Supports P-256,
  P-384, P-521 with correct coordinate byte lengths.
- DSA (DsaSha) for SHA-1 and SHA-256. Coordinate size from |q| bits.

Models (split per project rule — one class per file):
- Model::Key::X509Data with issuer_serial, subject_name, ski,
  certificates, crls, digests.
- Model::Key::X509IssuerSerial, X509Digest.
- Model::Key::RSAKeyValue, DSAKeyValue, ECKeyValue.

Parser & KeyExtractor:
- Full KeyInfo parsing: X509Data, KeyValue (RSA/DSA/EC), KeyName.
- KeyExtractor reconstructs OpenSSL keys:
    X509Certificate (preferred) -> public_key
    RSAKeyValue -> ASN.1 reconstruction (OpenSSL 3.x dropped RSA.new(n,e))
    DSAKeyValue -> ASN.1 reconstruction
    ECKeyValue  -> SubjectPublicKeyInfo DER construction
    KeyName     -> application-supplied key_map lookup
- Verifier auto-resolves the verification key from KeyInfo when no
  explicit key is passed.

XPath Filter transform:
- New Algorithms::XPathFilterTransform registered under
  http://www.w3.org/TR/1999/REC-xpath-19991116.
- Handles the canonical enveloped-signature expression
  `not(ancestor-or-self::dsig:Signature)` via direct ancestor walk.
- General expressions use a relative-XPath heuristic. Full here()
  support is tracked in TODO 06.

Refactor (DRY):
- Extract TransformPipeline. Signer and Verifier share the same
  pipeline logic (lookup transform, coerce type, apply, next).
- C14N algorithms can be used as transforms per spec §6.6.1
  (added input_type, output_type, transform to CanonicalizationBase).

Cross-verification milestones:
- sign2-result.xml (libxmlsec1, KeyName only) verifies byte-exact
  against the Ruby ref's RSA public key.
- sign3-result.xml (libxmlsec1, embedded X509Certificate) auto-verifies
  with no explicit key — extracted from the certificate.

99 specs (was 67 in PR #93), 0 failures, 0 rubocop offenses.
Full existing suite (1896 examples) still passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ronaldtse