Skip to content

Use cryptobyte inside cryptobackend ASN.1 paths#2372

Merged
qmuntal merged 1 commit into
microsoft/mainfrom
qmuntal/cryptobackend-cryptobyte-asn1
Jun 16, 2026
Merged

Use cryptobyte inside cryptobackend ASN.1 paths#2372
qmuntal merged 1 commit into
microsoft/mainfrom
qmuntal/cryptobackend-cryptobyte-asn1

Conversation

@qmuntal

@qmuntal qmuntal commented Jun 16, 2026

Copy link
Copy Markdown
Member

This change moves DER/ASN.1 handling into the cryptobackend algorithm packages that actually need it, instead of keeping that parsing and encoding in the standard-library integration layer or passing parser callbacks across the backend boundary. RSA Darwin key marshaling now lives in cryptobackend/rsa, Windows ECDSA signs and verifies ASN.1 signatures directly in cryptobackend/ecdsa, and Linux DSA hides the OpenSSL ASN.1 signature format inside cryptobackend/dsa.

This is also why cryptobackend now has an explicit golang.org/x/crypto dependency pinned to the version recorded in patches/0001-Vendor-external-dependencies.patch, which owns the go/src/go.mod dependency hunk. The new go generate hook in cryptobackend reads that patch as the source of truth, runs go mod tidy, and the matching test verifies that the generated module state is clean. Using the patch instead of the checked-out go submodule keeps the check working in CI jobs that do not initialize submodules.

Why this is reasonable now: before the backend package was more monolithic, importing cryptobyte in the backend would have expanded the shared backend dependency surface. That would have meant algorithms that do not parse or emit DER, such as AES or hash backends, would still conceptually carry the ASN.1 helper dependency. The backend is now split into algorithm-specific packages, so cryptobackend/dsa, cryptobackend/ecdsa, and cryptobackend/rsa can use cryptobyte without pulling it into unrelated backend packages.

The main benefit is a cleaner ownership boundary. Native backend packages now translate between native signature/key formats and Go's expected DER shapes themselves, so crypto/dsa, crypto/ecdsa, and crypto/rsa do less backend-specific adapter work. This also removes the raw ECDSA backend Sign/Verify surface and lets crypto/ecdsa call the ASN.1 backend operations directly.

Copilot AI review requested due to automatic review settings June 16, 2026 13:25
@qmuntal qmuntal requested a review from a team as a code owner June 16, 2026 13:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR moves DER/ASN.1 parsing and encoding into the per-algorithm cryptobackend/* packages that actually need it (DSA/ECDSA/RSA), reducing adapter logic in the stdlib integration layer and avoiding callback-based parsing across the backend boundary. It also introduces an explicit golang.org/x/crypto dependency for cryptobyte, plus a go generate-driven sync check to keep that version aligned with go/src.

Changes:

  • Shift DSA and ECDSA signature ASN.1 handling into cryptobackend/dsa (Linux) and cryptobackend/ecdsa (Windows), and move Darwin RSA key marshaling into cryptobackend/rsa.
  • Add golang.org/x/crypto to cryptobackend/go.mod (for cryptobyte) and record checksums in cryptobackend/go.sum.
  • Add a go generate hook + test to keep cryptobackend’s x/crypto version synced with go/src and to enforce a tidy module state.
Show a summary per file
File Description
patches/0002-Add-crypto-backends.patch Updates stdlib integration to call backend ASN.1-aware APIs; also includes additional backend-related patch hunks.
patches/0001-Vendor-external-dependencies.patch Updates vendored cryptobackend/* code used by go/src, including new cryptobyte usage paths.
cryptobackend/rsa/rsa_darwin.go Implements RSA key DER parsing/encoding in the Darwin backend using cryptobyte.
cryptobackend/go.mod Adds an explicit golang.org/x/crypto requirement for cryptobyte.
cryptobackend/go.sum Adds checksums for the newly required golang.org/x/crypto version.
cryptobackend/dsa/dsa_linux.go Hides OpenSSL’s DSA ASN.1 signature format behind backend parsing/encoding helpers.
cryptobackend/dsa/dsa_windows.go Simplifies DSA backend API by removing callback-based ASN.1 plumbing.
cryptobackend/dsa/dsa_darwin.go Keeps DSA stub backend signatures aligned with the updated API shape.
cryptobackend/dsa/nobackend.go Updates no-backend stubs to match the simplified DSA API.
cryptobackend/ecdsa/ecdsa_windows.go Implements ASN.1 signature encoding/decoding in the Windows ECDSA backend using cryptobyte.
cryptobackend/ecdsa/ecdsa_linux.go Removes unsupported raw (r,s) ECDSA surface in favor of ASN.1 operations.
cryptobackend/ecdsa/ecdsa_darwin.go Removes unsupported raw (r,s) ECDSA surface in favor of ASN.1 operations.
cryptobackend/ecdsa/nobackend.go Updates no-backend stubs to expose only ASN.1 ECDSA operations.
cryptobackend/backend_test.go Adds go generate + test enforcement for syncing and tidiness of the x/crypto dependency.

Copilot's findings

  • Files reviewed: 13/14 changed files
  • Comments generated: 4

Comment thread cryptobackend/backend_test.go
Comment thread cryptobackend/rsa/rsa_darwin.go
Comment thread patches/0002-Add-crypto-backends.patch Outdated
Comment thread patches/0001-Vendor-external-dependencies.patch
@qmuntal qmuntal force-pushed the qmuntal/cryptobackend-cryptobyte-asn1 branch 4 times, most recently from de0ebbb to 25357b0 Compare June 16, 2026 14:19
@qmuntal qmuntal force-pushed the qmuntal/cryptobackend-cryptobyte-asn1 branch from 25357b0 to 60b4f9b Compare June 16, 2026 14:22
@qmuntal qmuntal enabled auto-merge June 16, 2026 14:57
@qmuntal qmuntal merged commit 8ffe8a3 into microsoft/main Jun 16, 2026
57 checks passed
@qmuntal qmuntal deleted the qmuntal/cryptobackend-cryptobyte-asn1 branch June 16, 2026 15:09
@qmuntal qmuntal linked an issue Jun 16, 2026 that may be closed by this pull request
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.

Clean up crypto backend patches

3 participants