fix: Correct C14N 1.1 handling and improve ECDSA format compatibility#1
Open
atulpatildbz wants to merge 2 commits intomainfrom
Open
fix: Correct C14N 1.1 handling and improve ECDSA format compatibility#1atulpatildbz wants to merge 2 commits intomainfrom
atulpatildbz wants to merge 2 commits intomainfrom
Conversation
C14N 1.1 canonicalizer was missing namespace inheritance from parent elements, causing XML signature validation failures when signed elements had namespace prefixes defined in ancestor elements. The issue was already correctly handled in C14N 1.0 REC canonicalizer which uses getParentNamespaceAndXmlAttributes() and enhanceNamespaceAttributes() to inherit namespaces from parent elements before canonicalization. This commit applies the same approach to c14N11Canonicalizer: - Collect parent namespace declarations using getParentNamespaceAndXmlAttributes() - Copy input element to avoid mutations - Enhance element with inherited namespaces using enhanceNamespaceAttributes() - Proceed with standard canonicalization Added TestC14N11NamespaceInheritance to verify the fix works correctly. The test demonstrates that canonicalized output now includes proper xmlns declarations for inherited namespaces (e.g., xmlns:ds="..."). Fixes XML signature validation for documents using C14N 1.1 canonicalization with namespace prefixes defined in parent elements
This commit addresses two critical issues: 1. **C14N 1.1 Integration Fix**: Modified getCanonicalSignedInfo() to respect the CanonicalizationMethod specified in XML signatures. Previously, the function ignored the XML-specified algorithm and always used basic canonicalization, preventing the C14N 1.1 namespace inheritance fix (b650795) from being applied during signature verification. 2. **ECDSA Signature Format Conversion**: Added automatic conversion from XML DSig IEEE P1363 format to ASN.1 DER format for ECDSA signatures. XML Digital Signature standard uses IEEE P1363 (r||s concatenated), while Go's x509.CheckSignature expects ASN.1 DER (structured sequence). Changes: - Modified getCanonicalSignedInfo() to extract and respect CanonicalizationMethod - Added switch statement to apply correct canonicalizer based on algorithm - Implemented convertXMLDSigECDSASignature() for IEEE P1363 → ASN.1 DER conversion - Added isECDSAAlgorithm() helper for signature type detection - Added graceful fallback when format conversion fails Testing: - All 27 existing tests pass (zero regressions) - Handles both IEEE P1363 and ASN.1 DER ECDSA formats gracefully
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
russellhaering#170