Add SA specific implementation by migueltorresvalls · Pull Request #19 · invopop/xmldsig

migueltorresvalls · 2026-05-06T17:31:50Z

Add ZATCA (Saudi Arabia) signing profile

Adds profiles/zatca/ and the core extensions needed to produce ZATCA-compliant XML signatures. ZATCA diverges from standard W3C XML DSig in several ways that the existing config surface couldn't express.

Core changes

ECDSA DER format: new ECDSARawDER bool — when true, ECDSA signatures keep raw ASN.1 DER encoding instead of the default W3C concatenated r||s format. No breaking changes to Certificate.Sign().
XPath filter transforms: DocumentTransforms config field with XPath support, replacing the hardcoded enveloped-signature transform (defaults preserve existing behavior).
Pre-hash transforms: PreHashTransforms callback for arbitrary byte transforms before canonicalization.
Sign document digest: SignDocumentDigest flag signs the first Reference DigestValue instead of canonicalized SignedInfo.
Hex-encoded digests: base64(hex(hash)) encoding via HexEncodeDigests flag.
PEM fingerprint: FingerprintPEM() + HashPEMText flag to hash base64 PEM text instead of DER bytes.
NewCertificate() constructor: create a Certificate from a parsed x509.Certificate + crypto.Signer without PKCS12.

Test plan

Existing unit tests updated and passing
ZATCA onboarding validated end-to-end via apps/gov-sa

…hash transform

Copilot

Pull request overview

Adds a Saudi Arabia (ZATCA) signing profile and extends the library’s signing configuration surface to support ZATCA-specific deviations from standard W3C XML DSig/XAdES.

Changes:

Added profiles/zatca with ZATCA-specific XMLDSig/XAdES configs, XPath-based document transforms, and a pre-hash transform hook.
Extended signing capabilities: configurable DocumentTransforms, PreHashTransforms, ECDSA signature output format control, optional hex-encoded digests, and PEM-text certificate fingerprinting.
Introduced NewCertificate constructor to build a Certificate from an x509.Certificate + crypto.Signer (non-PKCS12).

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
xades.go	Adds `HashPEMText` behavior when computing XAdES signing certificate digest.
signature.go	Adds XPath transform support in transform serialization, pre-hash transforms hook, hex-digest option for SignedProperties, and passes ECDSA DER-format flag into signing.
profiles/zatca/zatca.go	New ZATCA profile providing preset XMLDSig/XAdES configs and ZATCA-specific pre-hash transforms.
options.go	Extends XMLDSig/XAdES config structs with new flags/hooks and defaults for `DocumentTransforms`.
digest.go	Adds `digestBytesHex` helper to support base64(hex(hash)) encoding.
certificates.go	Adds `NewCertificate`, adds `FingerprintPEM`, and extends `Certificate.Sign` to optionally return DER ECDSA signatures.
certificates_test.go	Updates tests to match the new `Certificate.Sign` signature.
go.mod / go.sum	Updates dependencies (notably `gobl`) and Go version metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


+// NewCertificate creates a Certificate from a parsed x509.Certificate and
+// a crypto.Signer
+func NewCertificate(cert *x509.Certificate, key crypto.Signer) (*Certificate, error) {


 	// RSA and ECDSA certificates require different signing code
 	// (even though both implement crypto.Signer)
 	var signature []byte
 	var signingErr error
 	switch cert.privateKey.(type) {
 	case *rsa.PrivateKey:
 		signature, signingErr = cert.privateKey.Sign(rand.Reader, digest, hash)
 	case *ecdsa.PrivateKey:
-		// When using ECDSA, privateKey.Sign returns signature in DER format, but XML DSig
-		// requires the signature to be in the concatenated format (r || s)
-		signature, signingErr = signECDSA(cert.privateKey.(*ecdsa.PrivateKey), digest, hash)
+		if ecdsaFormatDER {
+			// Keep the raw DER encoding from privateKey.Sign (required by ZATCA).
+			signature, signingErr = cert.privateKey.Sign(rand.Reader, digest, hash)
+		} else {
+			// Convert DER to concatenated r||s (W3C XML DSig standard).
+			signature, signingErr = signECDSA(cert.privateKey.(*ecdsa.PrivateKey), digest, hash)
+		}
 	default:
 		return "", fmt.Errorf("unsupported key type: %T", cert.privateKey)
 	}


 	certHash := s.opts.xadesConfig.SigningCertificateHash
 	fingerprint, err := cert.Fingerprint(certHash)
+	if s.opts.xadesConfig.HashPEMText {
+		fingerprint, err = cert.FingerprintPEM(certHash)
+	}


+// digestBytesHex creates a base64(hex(hash)) encoded digest.
+func digestBytesHex(data []byte, hash crypto.Hash) (string, error) {
+	if !hash.Available() {
+		return "", fmt.Errorf("hash %v not available", hash)
+	}
+
+	hasher := hash.New()
+	if _, err := hasher.Write(data); err != nil {
+		return "", err
+	}
+
+	hexStr := hex.EncodeToString(hasher.Sum(nil))
+	return base64.StdEncoding.EncodeToString([]byte(hexStr)), nil
+}


+func zatcaTimestampFormatter(t time.Time) string {
+	return t.UTC().Format("2006-01-02T15:04:05Z")
+}
+
+func zatcaPreHashTransforms(xmlData []byte) ([]byte, error) {
+	doc := etree.NewDocument()
+	doc.ReadSettings.PreserveCData = true
+	if err := doc.ReadFromBytes(xmlData); err != nil {
+		return nil, fmt.Errorf("parse xml: %w", err)
+	}
+
+	invoice := doc.Root()
+	if invoice == nil {
+		return xmlData, nil
+	}
+
+	// Insert residual newlines that match ZATCA's XSLT-based element stripping.
+
+	// 1. ext:UBLExtensions — first child element
+	if invoice.SelectElement("UBLExtensions") == nil {
+		if idx := indexOfFirstChildElement(invoice); idx >= 0 {
+			invoice.InsertChildAt(idx, &etree.CharData{Data: "\n  "})
+		}
+	}
+
+	// 2. QR AdditionalDocumentReference — after the last existing ref
+	if !hasQRRef(invoice) {
+		if idx := indexOfLastChildElementByTag(invoice, "AdditionalDocumentReference"); idx >= 0 {
+			invoice.InsertChildAt(idx+1, &etree.CharData{Data: "\n  "})
+		}
+	}
+
+	// 3. cac:Signature — before AccountingSupplierParty
+	if invoice.SelectElement("Signature") == nil {
+		if idx := indexOfChildElementByTag(invoice, "AccountingSupplierParty"); idx >= 0 {
+			invoice.InsertChildAt(idx, &etree.CharData{Data: "\n  "})
+		}
+	}
+
+	return doc.WriteToBytes()
+}


pmenendz

Looks good. I am just concerned with the ECDSAFormatDER. Feels too specific or something that could be known from the key

pmenendz · 2026-05-11T10:11:27Z

 	KeyInfoCanonicalizer              dsig.Canonicalizer
 	SignedInfoCanonicalizer           dsig.Canonicalizer
 	SignedInfoHash                    crypto.Hash
+	ECDSAFormatDER                    bool


Can't this be known from the private key?

pmenendz · 2026-05-11T10:12:43Z

+// digest using the configured private key. For ECDSA keys, ecdsaFormat controls
+// whether the signature is returned in concatenated r||s format (W3C XML DSig
+// standard) or raw DER encoding (required by ZATCA).
+func (cert *Certificate) Sign(data string, hash crypto.Hash, ecdsaFormatDER bool) (string, error) {


Feels too specific for me to include this argument here. Isn't there a way to know this directly from the certificate or the key?

This isn't a property of the key or the certificate (all ecdsa keys and x509 certificates are serialized the same way). The difference here is the format that ZATCA expects the signature to be in. Can't really think of another way of knowing this

migueltorresvalls added 6 commits April 23, 2026 15:10

zatca first commit

4f5186f

added option for profil-specific ecdsa serialization

5ff1b43

claude's fix

8a9fdca

refactored claude's fixes and remove unnecessary functions

c77933c

removed unnecessary options, zatca onboarding working

16062b4

made ecdsaFormat bool for easier backwards compatibility

8d33941

migueltorresvalls changed the title ~~Sa profile~~ Add SA specific implementation May 6, 2026

migueltorresvalls requested a review from pmenendz May 6, 2026 17:42

made ecdsakey logic a bool and removed unnecessary logic in zatca pre…

d9b84af

…hash transform

pmenendz requested review from cavalle and Copilot May 11, 2026 10:02

Copilot started reviewing on behalf of pmenendz May 11, 2026 10:02 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

pmenendz requested changes May 11, 2026

View reviewed changes

migueltorresvalls added 3 commits May 13, 2026 18:59

zatca support for dom4j serializatin

59cd19b

dom4j signature covered in tests

bb579ff

using xadesSignedProperties

18e5fda

migueltorresvalls requested a review from pmenendz May 14, 2026 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SA specific implementation#19

Add SA specific implementation#19
migueltorresvalls wants to merge 10 commits into
mainfrom
sa-profile

migueltorresvalls commented May 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

pmenendz left a comment

Uh oh!

pmenendz May 11, 2026

Uh oh!

pmenendz May 11, 2026

Uh oh!

migueltorresvalls May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

migueltorresvalls commented May 6, 2026

Add ZATCA (Saudi Arabia) signing profile

Core changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

pmenendz left a comment

Choose a reason for hiding this comment

Uh oh!

pmenendz May 11, 2026

Choose a reason for hiding this comment

Uh oh!

pmenendz May 11, 2026

Choose a reason for hiding this comment

Uh oh!

migueltorresvalls May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants