From 3a22fc96a90b165a4c56d1304122e6ab7566f70c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Mon, 15 Jun 2026 21:37:17 +0200 Subject: [PATCH] feat(signer): sign txs via KMS --- common/version/version.go | 2 +- rollup/go.mod | 1 + rollup/go.sum | 2 + rollup/internal/config/relayer.go | 17 +- .../internal/controller/relayer/l2_relayer.go | 8 + rollup/internal/controller/sender/README.md | 92 ++++++ .../internal/controller/sender/kms_signer.go | 198 +++++++++++++ .../controller/sender/kms_signer_test.go | 270 ++++++++++++++++++ rollup/internal/controller/sender/sender.go | 2 +- .../controller/sender/transaction_signer.go | 41 ++- .../sender/transaction_signer_test.go | 4 +- 11 files changed, 626 insertions(+), 11 deletions(-) create mode 100644 rollup/internal/controller/sender/README.md create mode 100644 rollup/internal/controller/sender/kms_signer.go create mode 100644 rollup/internal/controller/sender/kms_signer_test.go diff --git a/common/version/version.go b/common/version/version.go index 1703340026..b2f531715c 100644 --- a/common/version/version.go +++ b/common/version/version.go @@ -5,7 +5,7 @@ import ( "runtime/debug" ) -var tag = "v4.7.13" +var tag = "v4.7.14" var commit = func() string { if info, ok := debug.ReadBuildInfo(); ok { diff --git a/rollup/go.mod b/rollup/go.mod index dce906fd97..1206b35cb2 100644 --- a/rollup/go.mod +++ b/rollup/go.mod @@ -7,6 +7,7 @@ require ( github.com/aws/aws-sdk-go-v2 v1.36.3 github.com/aws/aws-sdk-go-v2/config v1.29.14 github.com/aws/aws-sdk-go-v2/credentials v1.17.67 + github.com/aws/aws-sdk-go-v2/service/kms v1.38.3 github.com/aws/aws-sdk-go-v2/service/s3 v1.80.0 github.com/consensys/gnark-crypto v0.16.0 github.com/crate-crypto/go-kzg-4844 v1.1.0 diff --git a/rollup/go.sum b/rollup/go.sum index 4d9dca034b..2c04347c51 100644 --- a/rollup/go.sum +++ b/rollup/go.sum @@ -34,6 +34,8 @@ github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 h1:dM9/92u2 github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15/go.mod h1:SwFBy2vjtA0vZbjjaFtfN045boopadnoVPhu4Fv66vY= github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.15 h1:moLQUoVq91LiqT1nbvzDukyqAlCv89ZmwaHw/ZFlFZg= github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.15/go.mod h1:ZH34PJUc8ApjBIfgQCFvkWcUDBtl/WTD+uiYHjd8igA= +github.com/aws/aws-sdk-go-v2/service/kms v1.38.3 h1:RivOtUH3eEu6SWnUMFHKAW4MqDOzWn1vGQ3S38Y5QMg= +github.com/aws/aws-sdk-go-v2/service/kms v1.38.3/go.mod h1:cQn6tAF77Di6m4huxovNM7NVAozWTZLsDRp9t8Z/WYk= github.com/aws/aws-sdk-go-v2/service/s3 v1.80.0 h1:fV4XIU5sn/x8gjRouoJpDVHj+ExJaUk4prYF+eb6qTs= github.com/aws/aws-sdk-go-v2/service/s3 v1.80.0/go.mod h1:qbn305Je/IofWBJ4bJz/Q7pDEtnnoInw/dGt71v6rHE= github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 h1:1Gw+9ajCV1jogloEv1RRnvfRFia2cL6c9cuKV2Ps+G8= diff --git a/rollup/internal/config/relayer.go b/rollup/internal/config/relayer.go index 2e50969ada..8c0db9550a 100644 --- a/rollup/internal/config/relayer.go +++ b/rollup/internal/config/relayer.go @@ -123,9 +123,10 @@ type GasOracleConfig struct { // SignerConfig - config of signer, contains type and config corresponding to type type SignerConfig struct { - SignerType string `json:"signer_type"` // type of signer can be PrivateKey or RemoteSigner + SignerType string `json:"signer_type"` // type of signer can be PrivateKey, RemoteSigner or AWSKMS PrivateKeySignerConfig *PrivateKeySignerConfig `json:"private_key_signer_config"` RemoteSignerConfig *RemoteSignerConfig `json:"remote_signer_config"` + AWSKMSSignerConfig *AWSKMSSignerConfig `json:"aws_kms_signer_config"` } // PrivateKeySignerConfig - config of private signer, contains private key @@ -138,3 +139,17 @@ type RemoteSignerConfig struct { RemoteSignerUrl string `json:"remote_signer_url"` // remote signer url (web3signer) in case of RemoteSigner signerType SignerAddress string `json:"signer_address"` // address of signer } + +// AWSKMSSignerConfig - config of an AWS KMS backed signer. The private key never +// leaves KMS; the service only requests signatures over transaction hashes. +type AWSKMSSignerConfig struct { + // KeyID is the KMS key id, alias or ARN of an asymmetric ECC_SECG_P256K1 / SIGN_VERIFY key. + KeyID string `json:"key_id"` + // Region is the AWS region of the key. Optional; falls back to the ambient AWS config + // (AWS_REGION, shared config, instance/IRSA role) when empty. + Region string `json:"region,omitempty"` + // SignerAddress is the expected Ethereum address of the key. Required: it is validated + // against the address derived from the KMS public key at startup so a misconfigured + // key id fails fast instead of signing from an unexpected account. + SignerAddress string `json:"signer_address"` +} diff --git a/rollup/internal/controller/relayer/l2_relayer.go b/rollup/internal/controller/relayer/l2_relayer.go index e57b6ca73f..8864c59730 100644 --- a/rollup/internal/controller/relayer/l2_relayer.go +++ b/rollup/internal/controller/relayer/l2_relayer.go @@ -1289,6 +1289,14 @@ func addrFromSignerConfig(config *config.SignerConfig) (common.Address, error) { return common.Address{}, fmt.Errorf("signer address is empty") } return common.HexToAddress(config.RemoteSignerConfig.SignerAddress), nil + case sender.AWSKMSSignerType: + if config.AWSKMSSignerConfig == nil || config.AWSKMSSignerConfig.SignerAddress == "" { + return common.Address{}, fmt.Errorf("aws kms signer address is empty") + } + if !common.IsHexAddress(config.AWSKMSSignerConfig.SignerAddress) { + return common.Address{}, fmt.Errorf("aws kms signer address %q is not a valid hex address", config.AWSKMSSignerConfig.SignerAddress) + } + return common.HexToAddress(config.AWSKMSSignerConfig.SignerAddress), nil default: return common.Address{}, fmt.Errorf("failed to determine signer address, unknown signer type: %v", config.SignerType) } diff --git a/rollup/internal/controller/sender/README.md b/rollup/internal/controller/sender/README.md new file mode 100644 index 0000000000..46bb8463ce --- /dev/null +++ b/rollup/internal/controller/sender/README.md @@ -0,0 +1,92 @@ +# Transaction signers + +Each rollup sender (`gas_oracle_sender`, `commit_sender`, `finalize_sender`) is +configured with its own `*_signer_config` block in the relayer config. The +`signer_type` field selects how transactions are signed: + +| `signer_type` | Key location | Blob tx (EIP-4844) support | +| ------------- | ------------ | -------------------------- | +| `PrivateKey` | In process / mounted secret | yes | +| `RemoteSigner`| web3signer (`eth_signTransaction`) | no | +| `AWSKMS` | AWS KMS (never leaves KMS) | yes | + +`commit_sender` submits batches as blob transactions, so it must use +`PrivateKey` or `AWSKMS`. + +## AWSKMS + +The `AWSKMS` signer keeps the private key inside AWS KMS. The service only sends +the 32-byte transaction signing hash to KMS and assembles the signature locally, +so all transaction types — including blob transactions — are supported. + +```json +"commit_sender_signer_config": { + "signer_type": "AWSKMS", + "aws_kms_signer_config": { + "key_id": "arn:aws:kms:us-east-1:123456789012:key/abcd-...", + "region": "us-east-1", + "signer_address": "0x1C5A77d9FA7eF466951B2F01F724BCa3A5820b63" + } +} +``` + +- **`key_id`** — id, alias, or ARN of an asymmetric KMS key with key spec + `ECC_SECG_P256K1` and key usage `SIGN_VERIFY`. +- **`region`** — optional; falls back to the ambient AWS config + (`AWS_REGION`, shared config, instance/IRSA role) when empty. +- **`signer_address`** — **required**. The expected Ethereum address of the key. + It is validated at startup against the address derived from the KMS public key, + so a misconfigured `key_id` fails fast instead of signing from an unexpected + account. + +AWS credentials are resolved from the standard AWS SDK chain (IRSA, instance +role, or environment) — never put credentials in the config file. + +### Recommended IAM setup + +- Use a **separate KMS key per sender** so `kms:Sign` can be scoped and audited + (via CloudTrail) per workload. +- Grant each workload only `kms:Sign` and `kms:GetPublicKey` on its key. +- KMS protects the key material, not the spending policy: keep the hot-wallet + pattern (low balances, alerting, rotation). A compromised service can still + request signatures, so consider transaction value/rate guards as a follow-up. + +### Provisioning the key + +There are two ways to get a key into KMS. Whichever you use, the signer never +needs the raw private key — it derives the Ethereum address from the KMS public +key (`keccak256(pubkey)[12:]`) at startup, and you put that address in +`signer_address`. Get the address from `GetPublicKey` (the relayer logs it on +startup, or derive it yourself from the returned point) and **fund it** before +the sender goes live. + +**Option A — generate in KMS (recommended).** The private key is created inside +the KMS HSM and is non-exportable: it provably never exists outside KMS. + +```bash +aws kms create-key \ + --key-spec ECC_SECG_P256K1 \ + --key-usage SIGN_VERIFY \ + --origin AWS_KMS \ + --description "scroll commit_sender" +# optional friendlier reference: +aws kms create-alias --alias-name alias/scroll-commit-sender --target-key-id +``` + +Use this for new senders — fund the freshly derived address. + +**Option B — import an existing private key (`Origin=EXTERNAL`).** Use this only +when you must preserve an already-funded address. KMS supports importing key +material for asymmetric keys, but the key existed in plaintext outside KMS at +least once, which is exactly the exposure the KMS signer otherwise eliminates — +so prefer Option A unless address continuity is a hard requirement. Create the +key with `--origin EXTERNAL`, then `get-parameters-for-import` / +`import-key-material` to upload the wrapped secp256k1 private key. + +**Rotation.** KMS does not auto-rotate asymmetric keys, and a key's address +cannot change in place. Rotating means creating a new key (Option A), pointing +`signer_address`/`key_id` at it, and sweeping the old balance to the new address +— the old KMS key signs that sweep itself while still enabled, so no key access +is needed; disable/schedule-delete it only after the sweep confirms. Because a +non-exportable key can't leak, rotation is primarily an IAM-compromise response +rather than routine hygiene. diff --git a/rollup/internal/controller/sender/kms_signer.go b/rollup/internal/controller/sender/kms_signer.go new file mode 100644 index 0000000000..19b7be545d --- /dev/null +++ b/rollup/internal/controller/sender/kms_signer.go @@ -0,0 +1,198 @@ +package sender + +import ( + "context" + "crypto/ecdsa" + "encoding/asn1" + "fmt" + "math/big" + + awsconfig "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/kms" + kmstypes "github.com/aws/aws-sdk-go-v2/service/kms/types" + + "github.com/scroll-tech/go-ethereum/common" + "github.com/scroll-tech/go-ethereum/crypto" + + "scroll-tech/rollup/internal/config" +) + +// kmsAPI captures the subset of the AWS KMS client used by the signer. It is an +// interface so the signer can be unit-tested with a mock in place of a live KMS. +type kmsAPI interface { + GetPublicKey(ctx context.Context, params *kms.GetPublicKeyInput, optFns ...func(*kms.Options)) (*kms.GetPublicKeyOutput, error) + Sign(ctx context.Context, params *kms.SignInput, optFns ...func(*kms.Options)) (*kms.SignOutput, error) +} + +// kmsSigner produces ECDSA secp256k1 signatures over transaction hashes using an +// AWS KMS asymmetric key. The private key never leaves KMS: only the 32-byte +// signing hash is sent, and KMS returns a DER-encoded signature which we turn +// into Ethereum's 65-byte [R || S || V] form locally. +type kmsSigner struct { + client kmsAPI + keyID string + addr common.Address +} + +// asn1Spki mirrors the SubjectPublicKeyInfo DER structure returned by KMS +// GetPublicKey for an ECC_SECG_P256K1 key. PublicKey holds the uncompressed +// point (0x04 || X || Y). +type asn1Spki struct { + Algorithm struct { + Algorithm asn1.ObjectIdentifier + Parameters asn1.ObjectIdentifier + } + PublicKey asn1.BitString +} + +// asn1EcSig mirrors the DER ECDSA signature KMS returns: SEQUENCE { r, s }. +type asn1EcSig struct { + R *big.Int + S *big.Int +} + +// OIDs expected in the SPKI AlgorithmIdentifier of a KMS ECC_SECG_P256K1 key. +var ( + oidPublicKeyECDSA = asn1.ObjectIdentifier{1, 2, 840, 10045, 2, 1} // id-ecPublicKey + oidNamedCurveS256 = asn1.ObjectIdentifier{1, 3, 132, 0, 10} // secp256k1 +) + +// secp256k1N is the secp256k1 curve order; secp256k1HalfN is N/2. Signatures with +// s above N/2 are normalized to N-s (EIP-2 low-s). +var ( + secp256k1N = crypto.S256().Params().N + secp256k1HalfN = new(big.Int).Rsh(secp256k1N, 1) +) + +// newKMSSigner constructs a signer backed by a live AWS KMS key. cfg.SignerAddress +// is required and validated against the address derived from the KMS public key, +// so a wrong key id fails fast instead of signing from an unexpected account. +func newKMSSigner(ctx context.Context, cfg *config.AWSKMSSignerConfig) (*kmsSigner, error) { + if cfg == nil { + return nil, fmt.Errorf("aws_kms_signer_config is nil") + } + if cfg.KeyID == "" { + return nil, fmt.Errorf("aws kms signer: key_id is empty") + } + if cfg.SignerAddress == "" { + return nil, fmt.Errorf("aws kms signer: signer_address is required") + } + if !common.IsHexAddress(cfg.SignerAddress) { + return nil, fmt.Errorf("aws kms signer: signer_address %q is not a valid hex address", cfg.SignerAddress) + } + + var optFns []func(*awsconfig.LoadOptions) error + if cfg.Region != "" { + optFns = append(optFns, awsconfig.WithRegion(cfg.Region)) + } + awsCfg, err := awsconfig.LoadDefaultConfig(ctx, optFns...) + if err != nil { + return nil, fmt.Errorf("aws kms signer: failed to load aws config: %w", err) + } + + return newKMSSignerWithClient(ctx, kms.NewFromConfig(awsCfg), cfg.KeyID, common.HexToAddress(cfg.SignerAddress)) +} + +// newKMSSignerWithClient is the testable core: it derives the key's address from +// the KMS public key and asserts it matches expectedAddr. +func newKMSSignerWithClient(ctx context.Context, client kmsAPI, keyID string, expectedAddr common.Address) (*kmsSigner, error) { + pub, err := publicKeyFromKMS(ctx, client, keyID) + if err != nil { + return nil, err + } + derivedAddr := crypto.PubkeyToAddress(*pub) + if derivedAddr != expectedAddr { + return nil, fmt.Errorf("aws kms signer: configured signer_address %s does not match address %s derived from KMS key %s", expectedAddr.Hex(), derivedAddr.Hex(), keyID) + } + return &kmsSigner{client: client, keyID: keyID, addr: derivedAddr}, nil +} + +func publicKeyFromKMS(ctx context.Context, client kmsAPI, keyID string) (*ecdsa.PublicKey, error) { + out, err := client.GetPublicKey(ctx, &kms.GetPublicKeyInput{KeyId: &keyID}) + if err != nil { + return nil, fmt.Errorf("aws kms signer: GetPublicKey failed: %w", err) + } + var spki asn1Spki + rest, err := asn1.Unmarshal(out.PublicKey, &spki) + if err != nil { + return nil, fmt.Errorf("aws kms signer: failed to parse public key DER: %w", err) + } + if len(rest) != 0 { + return nil, fmt.Errorf("aws kms signer: trailing bytes after public key DER") + } + if !spki.Algorithm.Algorithm.Equal(oidPublicKeyECDSA) || !spki.Algorithm.Parameters.Equal(oidNamedCurveS256) { + return nil, fmt.Errorf("aws kms signer: unexpected public key algorithm/curve, want id-ecPublicKey/secp256k1") + } + if spki.PublicKey.BitLength != len(spki.PublicKey.Bytes)*8 { + return nil, fmt.Errorf("aws kms signer: public key bit string has unused bits") + } + // crypto.UnmarshalPubkey requires the 65-byte uncompressed form (0x04 || X || Y) + // and verifies the point lies on the curve. + pub, err := crypto.UnmarshalPubkey(spki.PublicKey.Bytes) + if err != nil { + return nil, fmt.Errorf("aws kms signer: failed to unmarshal secp256k1 public key: %w", err) + } + return pub, nil +} + +// address returns the Ethereum address of the KMS key. +func (k *kmsSigner) address() common.Address { + return k.addr +} + +// sign returns the 65-byte [R || S || V] Ethereum signature over the given 32-byte hash. +func (k *kmsSigner) sign(ctx context.Context, hash []byte) ([]byte, error) { + out, err := k.client.Sign(ctx, &kms.SignInput{ + KeyId: &k.keyID, + Message: hash, + MessageType: kmstypes.MessageTypeDigest, + SigningAlgorithm: kmstypes.SigningAlgorithmSpecEcdsaSha256, + }) + if err != nil { + return nil, fmt.Errorf("aws kms signer: Sign failed: %w", err) + } + + var sig asn1EcSig + rest, err := asn1.Unmarshal(out.Signature, &sig) + if err != nil { + return nil, fmt.Errorf("aws kms signer: failed to parse DER signature: %w", err) + } + if len(rest) != 0 { + return nil, fmt.Errorf("aws kms signer: trailing bytes after DER signature") + } + if sig.R == nil || sig.S == nil { + return nil, fmt.Errorf("aws kms signer: DER signature missing r or s") + } + + r, s := sig.R, sig.S + // Guard against a malformed DER signature: r and s must be in [1, N-1]. + // Besides being the valid ECDSA range, this keeps them positive and <32 bytes, + // so FillBytes below cannot panic. + if r.Sign() <= 0 || s.Sign() <= 0 || r.Cmp(secp256k1N) >= 0 || s.Cmp(secp256k1N) >= 0 { + return nil, fmt.Errorf("aws kms signer: signature (r,s) out of range [1, N-1]") + } + // EIP-2: enforce low-s to keep signatures canonical. + if s.Cmp(secp256k1HalfN) > 0 { + s = new(big.Int).Sub(secp256k1N, s) + } + + rsSig := make([]byte, 65) + r.FillBytes(rsSig[0:32]) + s.FillBytes(rsSig[32:64]) + + // Recover the recovery id (v). KMS returns only (r, s), so we try the two valid + // Ethereum parities and keep the one that recovers our address. Recovery ids 2/3 + // require R.x >= N (probability ~2^-128) and are not valid Ethereum yParity values, + // so an unrecoverable signature returns the error below rather than a bad signature. + for v := byte(0); v <= 1; v++ { + rsSig[64] = v + pub, err := crypto.SigToPub(hash, rsSig) + if err != nil { + continue + } + if crypto.PubkeyToAddress(*pub) == k.addr { + return rsSig, nil + } + } + return nil, fmt.Errorf("aws kms signer: failed to recover a valid recovery id for the signature") +} diff --git a/rollup/internal/controller/sender/kms_signer_test.go b/rollup/internal/controller/sender/kms_signer_test.go new file mode 100644 index 0000000000..c85f985bc1 --- /dev/null +++ b/rollup/internal/controller/sender/kms_signer_test.go @@ -0,0 +1,270 @@ +package sender + +import ( + "context" + "crypto/ecdsa" + "encoding/asn1" + "math/big" + "testing" + + "github.com/aws/aws-sdk-go-v2/service/kms" + "github.com/holiman/uint256" + "github.com/scroll-tech/go-ethereum/common" + gethTypes "github.com/scroll-tech/go-ethereum/core/types" + "github.com/scroll-tech/go-ethereum/crypto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "scroll-tech/rollup/internal/config" +) + +var ( + ecPublicKeyOID = asn1.ObjectIdentifier{1, 2, 840, 10045, 2, 1} + secp256k1OID = asn1.ObjectIdentifier{1, 3, 132, 0, 10} +) + +// fakeKMS implements kmsAPI backed by a local secp256k1 key, mimicking the DER +// encodings a real AWS KMS ECC_SECG_P256K1 key returns. +type fakeKMS struct { + priv *ecdsa.PrivateKey + forceHi bool // emit a high-s signature to exercise EIP-2 normalization + signErr error +} + +func (f *fakeKMS) GetPublicKey(_ context.Context, _ *kms.GetPublicKeyInput, _ ...func(*kms.Options)) (*kms.GetPublicKeyOutput, error) { + pubBytes := crypto.FromECDSAPub(&f.priv.PublicKey) // 0x04 || X || Y + der, err := asn1.Marshal(asn1Spki{ + Algorithm: struct { + Algorithm asn1.ObjectIdentifier + Parameters asn1.ObjectIdentifier + }{Algorithm: ecPublicKeyOID, Parameters: secp256k1OID}, + PublicKey: asn1.BitString{Bytes: pubBytes, BitLength: len(pubBytes) * 8}, + }) + if err != nil { + return nil, err + } + return &kms.GetPublicKeyOutput{PublicKey: der}, nil +} + +func (f *fakeKMS) Sign(_ context.Context, in *kms.SignInput, _ ...func(*kms.Options)) (*kms.SignOutput, error) { + if f.signErr != nil { + return nil, f.signErr + } + sig, err := crypto.Sign(in.Message, f.priv) // canonical low-s [R||S||V] + if err != nil { + return nil, err + } + r := new(big.Int).SetBytes(sig[0:32]) + s := new(big.Int).SetBytes(sig[32:64]) + if f.forceHi { + s = new(big.Int).Sub(crypto.S256().Params().N, s) // make it high-s + } + der, err := asn1.Marshal(asn1EcSig{R: r, S: s}) + if err != nil { + return nil, err + } + return &kms.SignOutput{Signature: der}, nil +} + +func newTestKMSSigner(t *testing.T, fake *fakeKMS) *kmsSigner { + t.Helper() + expected := crypto.PubkeyToAddress(fake.priv.PublicKey) + ks, err := newKMSSignerWithClient(context.Background(), fake, "test-key-id", expected) + require.NoError(t, err) + assert.Equal(t, expected, ks.address()) + return ks +} + +func TestKMSSigner_AddressValidation(t *testing.T) { + priv, err := crypto.GenerateKey() + require.NoError(t, err) + fake := &fakeKMS{priv: priv} + + // matching address succeeds + newTestKMSSigner(t, fake) + + // mismatching address fails fast + _, err = newKMSSignerWithClient(context.Background(), fake, "test-key-id", common.HexToAddress("0xdeadbeef00000000000000000000000000000000")) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match") +} + +func TestKMSSigner_SignAllTxTypes(t *testing.T) { + priv, err := crypto.GenerateKey() + require.NoError(t, err) + + chainID := big.NewInt(534352) + to := common.HexToAddress("0x000000000000000000000000000000000000dEaD") + + cases := []struct { + name string + forceHi bool + txData gethTypes.TxData + }{ + { + name: "legacy", + txData: &gethTypes.LegacyTx{Nonce: 1, GasPrice: big.NewInt(1e9), Gas: 21000, To: &to, Value: big.NewInt(1)}, + }, + { + name: "dynamic_fee", + txData: &gethTypes.DynamicFeeTx{ChainID: chainID, Nonce: 2, GasTipCap: big.NewInt(1e9), GasFeeCap: big.NewInt(2e9), Gas: 21000, To: &to, Value: big.NewInt(1)}, + }, + { + name: "blob", + txData: &gethTypes.BlobTx{ + ChainID: uint256.MustFromBig(chainID), + Nonce: 3, + GasTipCap: uint256.NewInt(1e9), + GasFeeCap: uint256.NewInt(2e9), + Gas: 21000, + To: to, + BlobFeeCap: uint256.NewInt(1e9), + BlobHashes: []common.Hash{{0x01}}, + // A sidecar must survive signing — it carries the blobs/commitments + // the node needs to accept the tx. Production blob txs always set it. + Sidecar: &gethTypes.BlobTxSidecar{}, + }, + }, + { + name: "dynamic_fee_high_s", + forceHi: true, + txData: &gethTypes.DynamicFeeTx{ChainID: chainID, Nonce: 4, GasTipCap: big.NewInt(1e9), GasFeeCap: big.NewInt(2e9), Gas: 21000, To: &to, Value: big.NewInt(1)}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ks := newTestKMSSigner(t, &fakeKMS{priv: priv, forceHi: tc.forceHi}) + ts := &TransactionSigner{ + config: &config.SignerConfig{SignerType: AWSKMSSignerType}, + kmsSigner: ks, + kmsTxSigner: gethTypes.LatestSignerForChainID(chainID), + addr: ks.address(), + } + + tx := gethTypes.NewTx(tc.txData) + signedTx, err := ts.SignTransaction(context.Background(), tx) + require.NoError(t, err) + + // recovered sender must match the KMS key address + signer := gethTypes.LatestSignerForChainID(chainID) + from, err := gethTypes.Sender(signer, signedTx) + require.NoError(t, err) + assert.Equal(t, ks.address(), from) + + // output signature must be canonical low-s regardless of what KMS returned. + // RawSignatureValues returns (v, r, s) — the third value is s. + _, _, s := signedTx.RawSignatureValues() + assert.True(t, s.Cmp(secp256k1HalfN) <= 0, "signature s must be low-s") + + // a blob tx's sidecar must survive signing, otherwise the node rejects it. + if tx.BlobTxSidecar() != nil { + assert.NotNil(t, signedTx.BlobTxSidecar(), "blob sidecar must survive signing") + } + }) + } +} + +// rawKMS returns operator-supplied raw bytes, used to exercise malformed +// GetPublicKey / Sign responses that the address-deriving fakeKMS can't produce. +type rawKMS struct { + pub []byte + sig []byte +} + +func (f *rawKMS) GetPublicKey(_ context.Context, _ *kms.GetPublicKeyInput, _ ...func(*kms.Options)) (*kms.GetPublicKeyOutput, error) { + return &kms.GetPublicKeyOutput{PublicKey: f.pub}, nil +} + +func (f *rawKMS) Sign(_ context.Context, _ *kms.SignInput, _ ...func(*kms.Options)) (*kms.SignOutput, error) { + return &kms.SignOutput{Signature: f.sig}, nil +} + +func marshalSPKI(t *testing.T, algo, curve asn1.ObjectIdentifier, point []byte, bitLen int) []byte { + t.Helper() + der, err := asn1.Marshal(asn1Spki{ + Algorithm: struct { + Algorithm asn1.ObjectIdentifier + Parameters asn1.ObjectIdentifier + }{Algorithm: algo, Parameters: curve}, + PublicKey: asn1.BitString{Bytes: point, BitLength: bitLen}, + }) + require.NoError(t, err) + return der +} + +func TestKMSSigner_MalformedPublicKey(t *testing.T) { + priv, err := crypto.GenerateKey() + require.NoError(t, err) + point := crypto.FromECDSAPub(&priv.PublicKey) // 65-byte uncompressed + addr := crypto.PubkeyToAddress(priv.PublicKey) + full := len(point) * 8 + + // For the unused-bits case we need valid DER padding (the discarded low bits + // must be zero) so that asn1.Unmarshal accepts it and our explicit BitLength + // check is the thing that rejects it, deterministically. + evenPoint := append([]byte(nil), point...) + evenPoint[len(evenPoint)-1] &^= 1 + + // control: a well-formed SPKI must still be accepted. + _, err = newKMSSignerWithClient(context.Background(), &rawKMS{pub: marshalSPKI(t, ecPublicKeyOID, secp256k1OID, point, full)}, "k", addr) + require.NoError(t, err) + + cases := []struct { + name string + pub []byte + want string + }{ + {"wrong_algorithm_oid", marshalSPKI(t, secp256k1OID, secp256k1OID, point, full), "algorithm/curve"}, + {"wrong_curve_oid", marshalSPKI(t, ecPublicKeyOID, ecPublicKeyOID, point, full), "algorithm/curve"}, + {"trailing_bytes", append(marshalSPKI(t, ecPublicKeyOID, secp256k1OID, point, full), 0x00), "trailing bytes"}, + {"unused_bits", marshalSPKI(t, ecPublicKeyOID, secp256k1OID, evenPoint, full-1), "unused bits"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := newKMSSignerWithClient(context.Background(), &rawKMS{pub: tc.pub}, "k", addr) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.want) + }) + } +} + +func TestKMSSigner_MalformedSignature(t *testing.T) { + addr := common.HexToAddress("0x1C5A77d9FA7eF466951B2F01F724BCa3A5820b63") + hash := crypto.Keccak256([]byte("tx")) + + derOf := func(r, s *big.Int) []byte { + b, err := asn1.Marshal(asn1EcSig{R: r, S: s}) + require.NoError(t, err) + return b + } + + cases := []struct { + name string + sig []byte + want string + }{ + {"not_der", []byte{0x05, 0x00}, "failed to parse DER signature"}, + {"trailing_bytes", append(derOf(big.NewInt(1), big.NewInt(1)), 0x00), "trailing bytes"}, + {"zero_r", derOf(big.NewInt(0), big.NewInt(1)), "out of range"}, + {"negative_s", derOf(big.NewInt(1), big.NewInt(-1)), "out of range"}, + {"r_equals_n", derOf(new(big.Int).Set(secp256k1N), big.NewInt(1)), "out of range"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ks := &kmsSigner{client: &rawKMS{sig: tc.sig}, keyID: "k", addr: addr} + _, err := ks.sign(context.Background(), hash) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.want) + }) + } +} + +func TestKMSSigner_InvalidSignerAddress(t *testing.T) { + _, err := newKMSSigner(context.Background(), &config.AWSKMSSignerConfig{ + KeyID: "some-key-id", + SignerAddress: "not-a-hex-address", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid hex address") +} diff --git a/rollup/internal/controller/sender/sender.go b/rollup/internal/controller/sender/sender.go index 5b37473596..2f29372e4d 100644 --- a/rollup/internal/controller/sender/sender.go +++ b/rollup/internal/controller/sender/sender.go @@ -104,7 +104,7 @@ func NewSender(ctx context.Context, config *config.SenderConfig, signerConfig *c if err != nil { return nil, fmt.Errorf("failed to get chain ID, err: %w", err) } - transactionSigner, err := NewTransactionSigner(signerConfig, chainID) + transactionSigner, err := NewTransactionSigner(ctx, signerConfig, chainID) if err != nil { return nil, fmt.Errorf("failed to create transaction signer, err: %w", err) } diff --git a/rollup/internal/controller/sender/transaction_signer.go b/rollup/internal/controller/sender/transaction_signer.go index 68305afb36..0023aa8b87 100644 --- a/rollup/internal/controller/sender/transaction_signer.go +++ b/rollup/internal/controller/sender/transaction_signer.go @@ -22,18 +22,23 @@ const ( // RemoteSignerType is the type of signer that uses a remote signer to sign transactions RemoteSignerType = "RemoteSigner" + + // AWSKMSSignerType is the type of signer that uses an AWS KMS key to sign transactions + AWSKMSSignerType = "AWSKMS" ) // TransactionSigner signs given transactions type TransactionSigner struct { - config *config.SignerConfig - auth *bind.TransactOpts - rpcClient *rpc.Client - nonce uint64 - addr common.Address + config *config.SignerConfig + auth *bind.TransactOpts + rpcClient *rpc.Client + kmsSigner *kmsSigner + kmsTxSigner gethTypes.Signer + nonce uint64 + addr common.Address } -func NewTransactionSigner(config *config.SignerConfig, chainID *big.Int) (*TransactionSigner, error) { +func NewTransactionSigner(ctx context.Context, config *config.SignerConfig, chainID *big.Int) (*TransactionSigner, error) { switch config.SignerType { case PrivateKeySignerType: privKey, err := crypto.ToECDSA(common.FromHex(config.PrivateKeySignerConfig.PrivateKey)) @@ -62,6 +67,17 @@ func NewTransactionSigner(config *config.SignerConfig, chainID *big.Int) (*Trans rpcClient: rpcClient, addr: common.HexToAddress(config.RemoteSignerConfig.SignerAddress), }, nil + case AWSKMSSignerType: + ks, err := newKMSSigner(ctx, config.AWSKMSSignerConfig) + if err != nil { + return nil, fmt.Errorf("failed to create AWS KMS signer, err: %w", err) + } + return &TransactionSigner{ + config: config, + kmsSigner: ks, + kmsTxSigner: gethTypes.LatestSignerForChainID(chainID), + addr: ks.address(), + }, nil default: return nil, fmt.Errorf("failed to create new transaction signer, unknown type: %v", config.SignerType) } @@ -92,6 +108,19 @@ func (ts *TransactionSigner) SignTransaction(ctx context.Context, tx *gethTypes. return nil, err } return signedTx, nil + case AWSKMSSignerType: + // KMS signs the transaction hash, so we construct the signed tx locally. + // This supports every tx type the sender builds, including BlobTx. + sig, err := ts.kmsSigner.sign(ctx, ts.kmsTxSigner.Hash(tx).Bytes()) + if err != nil { + log.Info("failed to sign tx with AWS KMS", "address", ts.addr.String(), "err", err) + return nil, err + } + signedTx, err := tx.WithSignature(ts.kmsTxSigner, sig) + if err != nil { + return nil, fmt.Errorf("failed to apply KMS signature to tx, err: %w", err) + } + return signedTx, nil default: // this shouldn't happen, because SignerType is checked during creation return nil, fmt.Errorf("shouldn't happen, unknown signer type") diff --git a/rollup/internal/controller/sender/transaction_signer_test.go b/rollup/internal/controller/sender/transaction_signer_test.go index 5937cb8c8c..8bcf993ab2 100644 --- a/rollup/internal/controller/sender/transaction_signer_test.go +++ b/rollup/internal/controller/sender/transaction_signer_test.go @@ -49,7 +49,7 @@ func testBothSignerTypes(t *testing.T) { RemoteSignerUrl: endpoint, }, } - remoteSigner, err := NewTransactionSigner(remoteSignerConf, big.NewInt(int64(chainId))) + remoteSigner, err := NewTransactionSigner(context.Background(), remoteSignerConf, big.NewInt(int64(chainId))) assert.NoError(t, err) remoteSigner.SetNonce(2) @@ -60,7 +60,7 @@ func testBothSignerTypes(t *testing.T) { PrivateKey: "1212121212121212121212121212121212121212121212121212121212121212", }, } - privateKeySigner, err := NewTransactionSigner(privateKeySignerConf, big.NewInt(int64(chainId))) + privateKeySigner, err := NewTransactionSigner(context.Background(), privateKeySignerConf, big.NewInt(int64(chainId))) assert.NoError(t, err) privateKeySigner.SetNonce(2)