Skip to content

fix: respect namespace config for default namespace C14N#283

Merged
kislyuk merged 1 commit intoXML-Security:mainfrom
pitrafa:fix/default-namespace-c14n
Jan 15, 2026
Merged

fix: respect namespace config for default namespace C14N#283
kislyuk merged 1 commit intoXML-Security:mainfrom
pitrafa:fix/default-namespace-c14n

Conversation

@pitrafa
Copy link
Contributor

@pitrafa pitrafa commented Jan 13, 2026

Fixes #275

Problem

When signer.namespaces = {None: namespaces.ds}, the C14N output contained spurious xmlns="" undeclarations on elements inside <Reference>, changing the signature.

Root Cause

ds_tag() always returns QName(namespaces.ds, tag), creating elements with explicit namespace regardless of how signer.namespaces is configured. When lxml canonicalizes these elements with C14N 1.0, it adds xmlns="" to undeclare the namespace on nested elements.

Solution

Added _ds_tag() method that respects namespace configuration - returns QName(None, tag) when default namespace is configured, allowing elements to inherit from nsmap context.

Test

Added test_default_namespace_c14n_no_xmlns_undeclarations to verify:

  • Round-trip signing/verification works with default namespace
  • No xmlns="" undeclarations in SignedInfo C14N output

When signer.namespaces = {None: namespaces.ds} (default namespace),
elements were created with explicit namespace via QName(namespaces.ds, tag).
This caused lxml C14N to add xmlns="" undeclarations on child elements
inside <Reference>, changing the SignedInfo digest and signature.

Added _ds_tag() method that returns QName(None, tag) when default
namespace is configured, allowing elements to inherit from nsmap context.

Fixes XML-Security#275
@kislyuk kislyuk merged commit 65eeb0e into XML-Security:main Jan 15, 2026
1 of 22 checks passed
@kislyuk
Copy link
Member

kislyuk commented Jan 19, 2026

Hi there, I'm going to have to revert this PR because the test case does not pass. I thought that it passed and CI was failing for unrelated reasons, but that's not the case. I'm going to have to ask you to provide a test case that works for round-tripping a signature with these changes.

@pitrafa
Copy link
Contributor Author

pitrafa commented Jan 19, 2026

Hi, apologies for pushing a failing test. I was confused by a pre-existing failure (test_xades_roundtrip) and incorrectly assumed mine was passing.

The updated test now uses excise_empty_xmlns_declarations=True on the verifier for round-trip to work.

The reason: after serialize/parse, lxml normalizes namespace handling, which reintroduces xmlns="" during verification C14N. I noticed this flag is marked as "incorrect legacy behavior".

Is this a valid approach, or is there a better way?

@pitrafa
Copy link
Contributor Author

pitrafa commented Jan 19, 2026

I've opened #286 with a complete fix that includes:

  1. The original signer fix (_ds_tag() method)
  2. A verifier fix that auto-detects documents using default namespace and handles the C14N consistently
  3. A clean round-trip test (test_sign_verify_default_ns_roundtrip) that passes without requiring the legacy
    excise_empty_xmlns_declarations flag

The verifier fix detects when SignedInfo uses only default namespace (no ds: prefix) and automatically applies the same xmlns="" removal, ensuring sign/verify round-trip works correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SignXML Verifier Fails When XML Signature Uses Default Namespace Without ds: Prefix

2 participants