Add crypto.Signer support for KMS/HSM keys#6
Conversation
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
2 times, most recently
from
3b7da22 to
4a22109
Compare
|
I removed integration with Reviewable on this repo |
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
3 times, most recently
from
504881c to
d878cf8
Compare
There was a problem hiding this comment.
Pull request overview
This PR enables support for hardware security modules (HSM) and cloud Key Management Systems (KMS) by allowing the use of crypto.Signer interface implementations instead of requiring concrete *rsa.PrivateKey or *ecdsa.PrivateKey types.
Changes:
- Updated key type validation to check public key type instead of private key type, enabling KMS/HSM signer support
- Implemented custom JWT signing fallback using
crypto.Signerinterface for keys that aren't concrete private key types - Added ECDSA signature format conversion from ASN.1 DER to JWT's raw R||S format
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| service_provider.go | Updated key validation to check public key type instead of private key type |
| samlsp/session_jwt.go | Added custom JWT signing for crypto.Signer implementations with ECDSA signature conversion |
| samlsp/request_tracker_jwt.go | Added fallback to custom signing for crypto.Signer implementations |
| samlsp/new.go | Updated default signing method selection to check public key type |
| samlsp/middleware_test.go | Added comprehensive end-to-end tests using mock crypto.Signer |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
d878cf8 to
9ad3a32
Compare
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
2 times, most recently
from
9f655f0 to
9399db6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
6afae23 to
ed2dbd3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
f390c93 to
f3654e5
Compare
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
f3654e5 to
5065e71
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
5065e71 to
c1e940b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
11de088 to
e89fe6c
Compare
Using `go-version: stable` resolved to Go 1.26, but go.mod declares go 1.24.0. golangci-lint was picking up a file from the Go 1.26 toolchain's own vendor directory: golang.org/x/crypto/chacha20poly1305/fips140only_go1.26.go This file has a `//go:build go1.26` constraint, which causes a typecheck failure when the module is built with go 1.24. That failure cascades into false-positive errors across the codebase. Switching to `go-version-file: go.mod` pins CI to the Go version declared in go.mod, ensuring toolchain and module version stay in sync.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
56c7fd4 to
8894041
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
8894041 to
4db9789
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
4db9789 to
3a9fb97
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
3a9fb97 to
d1718fa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
d1718fa to
efc1bd1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Check public key type instead of private key type to support crypto.Signer implementations (e.g. GCP KMS, AWS KMS, HSM) that aren't concrete *rsa.PrivateKey or *ecdsa.PrivateKey types. Supports RSA (RS256/RS384/RS512), RSA-PSS (PS256/PS384/PS512), ECDSA (ES256/ES384/ES512), and EdDSA signing methods via crypto.Signer for JWT session and tracked request signing.
dineshudayakumar
force-pushed
the
crypto-signer-support
branch
from
efc1bd1 to
91213ee
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Add crypto.Signer support for KMS/HSM
Check public key type instead of private key type to support
crypto.Signer implementations (e.g. GCP KMS, AWS KMS, HSM)
that aren't concrete *rsa.PrivateKey or *ecdsa.PrivateKey types.
Supports RSA (RS256/RS384/RS512), RSA-PSS (PS256/PS384/PS512),
ECDSA (ES256/ES384/ES512), and EdDSA signing methods via
crypto.Signer for JWT session and tracked request signing.