From df7cec527e4d9e7a9bdfefb94b4598acb05ed4ad Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:54:18 +0800 Subject: [PATCH 01/10] feat(op-signer): add KeyProvider enum for cloud provider selection Add KeyProvider type to specify cloud KMS provider (AWS or GCP) in AuthConfig. Defaults to GCP if not specified for backwards compatibility. Includes tests for config parsing and validation. - Add KeyProvider type with AWS and GCP options - Add validation for KeyProvider values - Default to GCP when type is not specified - Add unit tests for config parsing - Update example config.yaml with explicit GCP type --- op-signer/service/config.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/op-signer/service/config.go b/op-signer/service/config.go index a51993d6..5bf75e22 100644 --- a/op-signer/service/config.go +++ b/op-signer/service/config.go @@ -11,11 +11,31 @@ import ( "gopkg.in/yaml.v3" ) +// KeyProvider represents the cloud provider for the key management service +type KeyProvider string + +const ( + KeyProviderAWS KeyProvider = "AWS" + KeyProviderGCP KeyProvider = "GCP" +) + +// IsValid checks if the KeyProvider value is valid +func (k KeyProvider) IsValid() bool { + switch k { + case KeyProviderAWS, KeyProviderGCP: + return true + default: + return false + } +} + type AuthConfig struct { // ClientName DNS name of the client connecting to op-signer. ClientName string `yaml:"name"` // KeyName key resource name of the Cloud KMS KeyName string `yaml:"key"` + // KeyProvider specifies which cloud provider's KMS to use + KeyProvider KeyProvider `yaml:"type"` // ChainID chain id of the op-signer to sign for ChainID uint64 `yaml:"chainID"` // FromAddress sender address that is sending the rpc request @@ -41,7 +61,14 @@ func ReadConfig(path string) (SignerServiceConfig, error) { if err := yaml.Unmarshal(data, &config); err != nil { return config, err } - for _, authConfig := range config.Auth { + for i, authConfig := range config.Auth { + // Default to GCP if KeyProvider is empty to avoid breaking changes + if config.Auth[i].KeyProvider == "" { + config.Auth[i].KeyProvider = KeyProviderGCP + } + if !config.Auth[i].KeyProvider.IsValid() { + return config, fmt.Errorf("invalid key provider '%s' in auth config. Must be 'AWS' or 'GCP'", authConfig.KeyProvider) + } for _, toAddress := range authConfig.ToAddresses { if _, err := hexutil.Decode(toAddress); err != nil { return config, fmt.Errorf("invalid toAddress '%s' in auth config: %w", toAddress, err) From aac0d7a80e08f3d2de9c42619cec77611b8d419a Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:54:18 +0800 Subject: [PATCH 02/10] refactor(op-signer): move KeyProvider to top-level config Move the KeyProvider configuration from individual AuthConfig entries to the top-level SignerServiceConfig. This simplifies the configuration by having a single provider type for all auth configs. - Rename config field to `provider` in yaml - Move KeyProvider from AuthConfig to SignerServiceConfig - Update config validation to check top-level provider - Update tests to reflect new config structure - Maintain backwards compatibility with GCP default --- op-signer/service/config.go | 24 +++++----- op-signer/service/config_test.go | 77 ++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 op-signer/service/config_test.go diff --git a/op-signer/service/config.go b/op-signer/service/config.go index 5bf75e22..f0951865 100644 --- a/op-signer/service/config.go +++ b/op-signer/service/config.go @@ -34,8 +34,6 @@ type AuthConfig struct { ClientName string `yaml:"name"` // KeyName key resource name of the Cloud KMS KeyName string `yaml:"key"` - // KeyProvider specifies which cloud provider's KMS to use - KeyProvider KeyProvider `yaml:"type"` // ChainID chain id of the op-signer to sign for ChainID uint64 `yaml:"chainID"` // FromAddress sender address that is sending the rpc request @@ -49,7 +47,8 @@ func (c AuthConfig) MaxValueToInt() *big.Int { } type SignerServiceConfig struct { - Auth []AuthConfig `yaml:"auth"` + ProviderType KeyProvider `yaml:"provider"` + Auth []AuthConfig `yaml:"auth"` } func ReadConfig(path string) (SignerServiceConfig, error) { @@ -61,14 +60,17 @@ func ReadConfig(path string) (SignerServiceConfig, error) { if err := yaml.Unmarshal(data, &config); err != nil { return config, err } - for i, authConfig := range config.Auth { - // Default to GCP if KeyProvider is empty to avoid breaking changes - if config.Auth[i].KeyProvider == "" { - config.Auth[i].KeyProvider = KeyProviderGCP - } - if !config.Auth[i].KeyProvider.IsValid() { - return config, fmt.Errorf("invalid key provider '%s' in auth config. Must be 'AWS' or 'GCP'", authConfig.KeyProvider) - } + + // Default to GCP if Provider is empty to avoid breaking changes + if config.ProviderType == "" { + config.ProviderType = KeyProviderGCP + } + + if !config.ProviderType.IsValid() { + return config, fmt.Errorf("invalid provider '%s' in config. Must be 'AWS' or 'GCP'", config.ProviderType) + } + + for _, authConfig := range config.Auth { for _, toAddress := range authConfig.ToAddresses { if _, err := hexutil.Decode(toAddress); err != nil { return config, fmt.Errorf("invalid toAddress '%s' in auth config: %w", toAddress, err) diff --git a/op-signer/service/config_test.go b/op-signer/service/config_test.go new file mode 100644 index 00000000..4e03a980 --- /dev/null +++ b/op-signer/service/config_test.go @@ -0,0 +1,77 @@ +package service + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReadConfig_DefaultKeyProvider(t *testing.T) { + tmpFile, err := os.CreateTemp("", "config-*.yaml") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + configData := ` +auth: + - name: "test-client" + key: "test-key" + chainID: 10 + fromAddress: "0x1234567890123456789012345678901234567890" + toAddresses: ["0x1234567890123456789012345678901234567890"] + maxValue: "0x1234" +` + err = os.WriteFile(tmpFile.Name(), []byte(configData), 0644) + require.NoError(t, err) + + config, err := ReadConfig(tmpFile.Name()) + require.NoError(t, err) + assert.Equal(t, KeyProviderGCP, config.ProviderType) +} + +func TestReadConfig_ExplicitKeyProvider(t *testing.T) { + tmpFile, err := os.CreateTemp("", "config-*.yaml") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + configData := ` +provider: "AWS" +auth: + - name: "test-client" + key: "test-key" + chainID: 10 + fromAddress: "0x1234567890123456789012345678901234567890" + toAddresses: ["0x1234567890123456789012345678901234567890"] + maxValue: "0x1234" +` + err = os.WriteFile(tmpFile.Name(), []byte(configData), 0644) + require.NoError(t, err) + + config, err := ReadConfig(tmpFile.Name()) + require.NoError(t, err) + assert.Equal(t, KeyProviderAWS, config.ProviderType) +} + +func TestReadConfig_InvalidKeyProvider(t *testing.T) { + tmpFile, err := os.CreateTemp("", "config-*.yaml") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + configData := ` +provider: "INVALID" +auth: + - name: "test-client" + key: "test-key" + chainID: 10 + fromAddress: "0x1234567890123456789012345678901234567890" + toAddresses: ["0x1234567890123456789012345678901234567890"] + maxValue: "0x1234" +` + err = os.WriteFile(tmpFile.Name(), []byte(configData), 0644) + require.NoError(t, err) + + _, err = ReadConfig(tmpFile.Name()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid provider") +} From 6d9eee4715c011c274e2547adb1f2423eb5f929e Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:54:18 +0800 Subject: [PATCH 03/10] feat(op-signer): add provider factory for signature providers Add factory function to create SignatureProvider instances based on provider type. Updates service initialization to use the new factory pattern. - Add NewSignatureProvider factory function in provider package - Update NewSignerService to handle provider creation errors - Update app.go to handle potential service creation errors - Move ProviderType enum from config to provider package --- op-signer/app.go | 5 +++- op-signer/service/config.go | 25 +++-------------- op-signer/service/config_test.go | 5 ++-- op-signer/service/provider/provider.go | 38 +++++++++++++++++++++++++- op-signer/service/service.go | 8 ++++-- 5 files changed, 54 insertions(+), 27 deletions(-) diff --git a/op-signer/app.go b/op-signer/app.go index 074bf941..8f495ac4 100644 --- a/op-signer/app.go +++ b/op-signer/app.go @@ -149,7 +149,10 @@ func (s *SignerApp) initRPC(cfg *Config) error { if err != nil { return fmt.Errorf("failed to read service config: %w", err) } - s.signer = service.NewSignerService(s.log, serviceCfg) + s.signer, err = service.NewSignerService(s.log, serviceCfg) + if err != nil { + return fmt.Errorf("failed to create signer service: %w", err) + } s.signer.RegisterAPIs(s.rpc) if err := s.rpc.Start(); err != nil { diff --git a/op-signer/service/config.go b/op-signer/service/config.go index f0951865..a8ef1249 100644 --- a/op-signer/service/config.go +++ b/op-signer/service/config.go @@ -6,29 +6,12 @@ import ( "math/big" "os" + "github.com/ethereum-optimism/infra/op-signer/service/provider" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "gopkg.in/yaml.v3" ) -// KeyProvider represents the cloud provider for the key management service -type KeyProvider string - -const ( - KeyProviderAWS KeyProvider = "AWS" - KeyProviderGCP KeyProvider = "GCP" -) - -// IsValid checks if the KeyProvider value is valid -func (k KeyProvider) IsValid() bool { - switch k { - case KeyProviderAWS, KeyProviderGCP: - return true - default: - return false - } -} - type AuthConfig struct { // ClientName DNS name of the client connecting to op-signer. ClientName string `yaml:"name"` @@ -47,8 +30,8 @@ func (c AuthConfig) MaxValueToInt() *big.Int { } type SignerServiceConfig struct { - ProviderType KeyProvider `yaml:"provider"` - Auth []AuthConfig `yaml:"auth"` + ProviderType provider.ProviderType `yaml:"provider"` + Auth []AuthConfig `yaml:"auth"` } func ReadConfig(path string) (SignerServiceConfig, error) { @@ -63,7 +46,7 @@ func ReadConfig(path string) (SignerServiceConfig, error) { // Default to GCP if Provider is empty to avoid breaking changes if config.ProviderType == "" { - config.ProviderType = KeyProviderGCP + config.ProviderType = provider.KeyProviderGCP } if !config.ProviderType.IsValid() { diff --git a/op-signer/service/config_test.go b/op-signer/service/config_test.go index 4e03a980..6e94aed1 100644 --- a/op-signer/service/config_test.go +++ b/op-signer/service/config_test.go @@ -4,6 +4,7 @@ import ( "os" "testing" + "github.com/ethereum-optimism/infra/op-signer/service/provider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -27,7 +28,7 @@ auth: config, err := ReadConfig(tmpFile.Name()) require.NoError(t, err) - assert.Equal(t, KeyProviderGCP, config.ProviderType) + assert.Equal(t, provider.KeyProviderGCP, config.ProviderType) } func TestReadConfig_ExplicitKeyProvider(t *testing.T) { @@ -50,7 +51,7 @@ auth: config, err := ReadConfig(tmpFile.Name()) require.NoError(t, err) - assert.Equal(t, KeyProviderAWS, config.ProviderType) + assert.Equal(t, provider.KeyProviderAWS, config.ProviderType) } func TestReadConfig_InvalidKeyProvider(t *testing.T) { diff --git a/op-signer/service/provider/provider.go b/op-signer/service/provider/provider.go index 33158579..095b9875 100644 --- a/op-signer/service/provider/provider.go +++ b/op-signer/service/provider/provider.go @@ -1,9 +1,45 @@ //go:generate mockgen -destination=mock_provider.go -package=provider github.com/ethereum-optimism/infra/op-signer/service/provider SignatureProvider package provider -import "context" +import ( + "context" + "fmt" + + "github.com/ethereum/go-ethereum/log" +) type SignatureProvider interface { SignDigest(ctx context.Context, keyName string, digest []byte) ([]byte, error) GetPublicKey(ctx context.Context, keyName string) ([]byte, error) } + +// ProviderType represents the cloud provider for the key management service +type ProviderType string + +const ( + KeyProviderAWS ProviderType = "AWS" + KeyProviderGCP ProviderType = "GCP" +) + +// IsValid checks if the KeyProvider value is valid +func (k ProviderType) IsValid() bool { + switch k { + case KeyProviderAWS, KeyProviderGCP: + return true + default: + return false + } +} + +// NewSignatureProvider creates a new SignatureProvider based on the provider type +func NewSignatureProvider(logger log.Logger, providerType ProviderType) (SignatureProvider, error) { + switch providerType { + case KeyProviderGCP: + return NewCloudKMSSignatureProvider(logger), nil + case KeyProviderAWS: + // TODO: Implement AWS provider + return nil, fmt.Errorf("AWS provider not yet implemented") + default: + return nil, fmt.Errorf("unsupported provider type: %s", providerType) + } +} diff --git a/op-signer/service/service.go b/op-signer/service/service.go index fd4951ee..0c3b47fd 100644 --- a/op-signer/service/service.go +++ b/op-signer/service/service.go @@ -33,8 +33,12 @@ type OpsignerSerivce struct { provider provider.SignatureProvider } -func NewSignerService(logger log.Logger, config SignerServiceConfig) *SignerService { - return NewSignerServiceWithProvider(logger, config, provider.NewCloudKMSSignatureProvider(logger)) +func NewSignerService(logger log.Logger, config SignerServiceConfig) (*SignerService, error) { + provider, err := provider.NewSignatureProvider(logger, config.ProviderType) + if err != nil { + return nil, fmt.Errorf("failed to create signature provider: %w", err) + } + return NewSignerServiceWithProvider(logger, config, provider), nil } func NewSignerServiceWithProvider( From 0badbd277a9645ec34a083631c4c9c558581c37a Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:54:18 +0800 Subject: [PATCH 04/10] feat(op-signer): add AWS KMS signature provider Add AWS KMS implementation of SignatureProvider interface to support AWS KMS for signing operations. Updates provider factory to support both GCP and AWS. - Add AWSKMSSignatureProvider with Sign and GetPublicKey methods - Add AWS SDK dependencies - Update provider factory to support AWS KMS - Reuse existing signature conversion utilities - Add test-friendly constructor with mock client --- op-signer/go.mod | 17 +++++ op-signer/go.sum | 28 +++++++++ op-signer/service/provider/aws_kms.go | 87 ++++++++++++++++++++++++++ op-signer/service/provider/cloudkms.go | 7 +-- op-signer/service/provider/provider.go | 5 +- 5 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 op-signer/service/provider/aws_kms.go diff --git a/op-signer/go.mod b/op-signer/go.mod index 5629bc77..890a055e 100644 --- a/op-signer/go.mod +++ b/op-signer/go.mod @@ -17,12 +17,29 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) +require ( + github.com/aws/aws-sdk-go-v2 v1.32.8 // indirect + github.com/aws/aws-sdk-go-v2/config v1.28.11 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.17.52 // indirect + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.23 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.27 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.27 // indirect + github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.8 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.24.9 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.8 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.33.7 // indirect + github.com/aws/smithy-go v1.22.1 // indirect +) + require ( cloud.google.com/go/compute/metadata v0.3.0 // indirect cloud.google.com/go/iam v1.1.1 // indirect github.com/BurntSushi/toml v1.4.0 // indirect github.com/DataDog/zstd v1.5.6-0.20230824185856-869dae002e5e // indirect github.com/Microsoft/go-winio v0.6.2 // indirect + github.com/aws/aws-sdk-go-v2/service/kms v1.37.11 github.com/beorn7/perks v1.0.1 // indirect github.com/bits-and-blooms/bitset v1.13.0 // indirect github.com/btcsuite/btcd/btcec/v2 v2.3.4 // indirect diff --git a/op-signer/go.sum b/op-signer/go.sum index 3b58e1fe..831208c0 100644 --- a/op-signer/go.sum +++ b/op-signer/go.sum @@ -18,6 +18,34 @@ github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA github.com/VictoriaMetrics/fastcache v1.12.2 h1:N0y9ASrJ0F6h0QaC3o6uJb3NIZ9VKLjCM7NQbSmF7WI= github.com/VictoriaMetrics/fastcache v1.12.2/go.mod h1:AmC+Nzz1+3G2eCPapF6UcsnkThDcMsQicp4xDukwJYI= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= +github.com/aws/aws-sdk-go-v2 v1.32.8 h1:cZV+NUS/eGxKXMtmyhtYPJ7Z4YLoI/V8bkTdRZfYhGo= +github.com/aws/aws-sdk-go-v2 v1.32.8/go.mod h1:P5WJBrYqqbWVaOxgH0X/FYYD47/nooaPOZPlQdmiN2U= +github.com/aws/aws-sdk-go-v2/config v1.28.11 h1:7Ekru0IkRHRnSRWGQLnLN6i0o1Jncd0rHo2T130+tEQ= +github.com/aws/aws-sdk-go-v2/config v1.28.11/go.mod h1:x78TpPvBfHH16hi5tE3OCWQ0pzNfyXA349p5/Wp82Yo= +github.com/aws/aws-sdk-go-v2/credentials v1.17.52 h1:I4ymSk35LHogx2Re2Wu6LOHNTRaRWkLVoJgWS5Wd40M= +github.com/aws/aws-sdk-go-v2/credentials v1.17.52/go.mod h1:vAkqKbMNUcher8fDXP2Ge2qFXKMkcD74qvk1lJRMemM= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.23 h1:IBAoD/1d8A8/1aA8g4MBVtTRHhXRiNAgwdbo/xRM2DI= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.23/go.mod h1:vfENuCM7dofkgKpYzuzf1VT1UKkA/YL3qanfBn7HCaA= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.27 h1:jSJjSBzw8VDIbWv+mmvBSP8ezsztMYJGH+eKqi9AmNs= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.27/go.mod h1:/DAhLbFRgwhmvJdOfSm+WwikZrCuUJiA4WgJG0fTNSw= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.27 h1:l+X4K77Dui85pIj5foXDhPlnqcNRG2QUyvca300lXh8= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.27/go.mod h1:KvZXSFEXm6x84yE8qffKvT3x8J5clWnVFXphpohhzJ8= +github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 h1:VaRN3TlFdd6KxX1x3ILT5ynH6HvKgqdiXoTxAF4HQcQ= +github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1/go.mod h1:FbtygfRFze9usAadmnGJNc8KsP346kEe+y2/oyhGAGc= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1 h1:iXtILhvDxB6kPvEXgsDhGaZCSC6LQET5ZHSdJozeI0Y= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1/go.mod h1:9nu0fVANtYiAePIBh2/pFUSwtJ402hLnp854CNoDOeE= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.8 h1:cWno7lefSH6Pp+mSznagKCgfDGeZRin66UvYUqAkyeA= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.8/go.mod h1:tPD+VjU3ABTBoEJ3nctu5Nyg4P4yjqSH5bJGGkY4+XE= +github.com/aws/aws-sdk-go-v2/service/kms v1.37.11 h1:49cjX6w3sLuMk0PBBXzUsgzF6v4eEB1teKchdDQ4HFo= +github.com/aws/aws-sdk-go-v2/service/kms v1.37.11/go.mod h1:wHYtyttsH+A6d2MzXYl8cIf4O2Kw1Kg0qzromSX/wOs= +github.com/aws/aws-sdk-go-v2/service/sso v1.24.9 h1:YqtxripbjWb2QLyzRK9pByfEDvgg95gpC2AyDq4hFE8= +github.com/aws/aws-sdk-go-v2/service/sso v1.24.9/go.mod h1:lV8iQpg6OLOfBnqbGMBKYjilBlf633qwHnBEiMSPoHY= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.8 h1:6dBT1Lz8fK11m22R+AqfRsFn8320K0T5DTGxxOQBSMw= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.8/go.mod h1:/kiBvRQXBc6xeJTYzhSdGvJ5vm1tjaDEjH+MSeRJnlY= +github.com/aws/aws-sdk-go-v2/service/sts v1.33.7 h1:qwGa9MA8G7mBq2YphHFaygdPe5t9OA7SvaJdwWTlEds= +github.com/aws/aws-sdk-go-v2/service/sts v1.33.7/go.mod h1:+8h7PZb3yY5ftmVLD7ocEoE98hdc8PoKS0H3wfx1dlc= +github.com/aws/smithy-go v1.22.1 h1:/HPHZQ0g7f4eUeK6HKglFz8uwVfZKgoI25rb/J+dnro= +github.com/aws/smithy-go v1.22.1/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bits-and-blooms/bitset v1.13.0 h1:bAQ9OPNFYbGHV6Nez0tmNI0RiEu7/hxlYJRUA0wFAVE= diff --git a/op-signer/service/provider/aws_kms.go b/op-signer/service/provider/aws_kms.go new file mode 100644 index 00000000..0fb87aea --- /dev/null +++ b/op-signer/service/provider/aws_kms.go @@ -0,0 +1,87 @@ +package provider + +import ( + "context" + "fmt" + + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/kms" + "github.com/aws/aws-sdk-go-v2/service/kms/types" + "github.com/ethereum/go-ethereum/log" +) + +// AWSKMSClient is the minimal interface for the AWS KMS client requried by the AWSKMSSignatureProvider. These functions +// are already implemented by the AWS SDK, but we define our own type to allow us to mock the client in tests. +type AWSKMSClient interface { + // https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/kms#Client.Sign + Sign(ctx context.Context, params *kms.SignInput, optFns ...func(*kms.Options)) (*kms.SignOutput, error) + // https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/kms#Client.GetPublicKey + GetPublicKey(ctx context.Context, params *kms.GetPublicKeyInput, optFns ...func(*kms.Options)) (*kms.GetPublicKeyOutput, error) +} + +type AWSKMSSignatureProvider struct { + logger log.Logger + client AWSKMSClient +} + +func NewAWSKMSSignatureProvider(logger log.Logger) (SignatureProvider, error) { + ctx := context.Background() + cfg, err := config.LoadDefaultConfig(ctx) + if err != nil { + return nil, fmt.Errorf("failed to load AWS config: %w", err) + } + + client := kms.NewFromConfig(cfg) + return &AWSKMSSignatureProvider{logger: logger, client: client}, nil +} + +func NewAWSKMSSignatureProviderWithClient(logger log.Logger, client AWSKMSClient) SignatureProvider { + return &AWSKMSSignatureProvider{logger: logger, client: client} +} + +// SignDigest signs the digest with a given AWS KMS keyname and returns a compact recoverable signature. +// The key must be an ECC_SECG_P256K1 key type. +func (a *AWSKMSSignatureProvider) SignDigest( + ctx context.Context, + keyName string, + digest []byte, +) ([]byte, error) { + publicKey, err := a.GetPublicKey(ctx, keyName) + if err != nil { + return nil, fmt.Errorf("failed to get public key: %w", err) + } + + signInput := &kms.SignInput{ + KeyId: &keyName, + Message: digest, + MessageType: types.MessageTypeDigest, + SigningAlgorithm: types.SigningAlgorithmSpecEcdsaSha256, + } + + result, err := a.client.Sign(ctx, signInput) + if err != nil { + return nil, fmt.Errorf("aws kms sign request failed: %w", err) + } + + a.logger.Debug(fmt.Sprintf("der signature: %x", result.Signature)) + + return convertToCompactRecoverableSignature(result.Signature, digest, publicKey) +} + +// GetPublicKey returns a decoded secp256k1 public key. +func (a *AWSKMSSignatureProvider) GetPublicKey( + ctx context.Context, + keyName string, +) ([]byte, error) { + input := &kms.GetPublicKeyInput{ + KeyId: &keyName, + } + + result, err := a.client.GetPublicKey(ctx, input) + if err != nil { + return nil, fmt.Errorf("aws kms get public key request failed: %w", err) + } + + // AWS KMS returns the public key in DER format + return x509ParseECDSAPublicKey(result.PublicKey) +} diff --git a/op-signer/service/provider/cloudkms.go b/op-signer/service/provider/cloudkms.go index 7b108ac0..ab0aa960 100644 --- a/op-signer/service/provider/cloudkms.go +++ b/op-signer/service/provider/cloudkms.go @@ -42,14 +42,13 @@ type CloudKMSSignatureProvider struct { client CloudKMSClient } -func NewCloudKMSSignatureProvider(logger log.Logger) SignatureProvider { +func NewCloudKMSSignatureProvider(logger log.Logger) (SignatureProvider, error) { ctx := context.Background() client, err := kms.NewKeyManagementClient(ctx) if err != nil { - logger.Error("failed to initialize kms client", "error", err) - panic(err) + return nil, fmt.Errorf("failed to initialize kms client: %w", err) } - return &CloudKMSSignatureProvider{logger, client} + return &CloudKMSSignatureProvider{logger, client}, nil } func NewCloudKMSSignatureProviderWithClient(logger log.Logger, client CloudKMSClient) SignatureProvider { diff --git a/op-signer/service/provider/provider.go b/op-signer/service/provider/provider.go index 095b9875..0947a0c5 100644 --- a/op-signer/service/provider/provider.go +++ b/op-signer/service/provider/provider.go @@ -35,10 +35,9 @@ func (k ProviderType) IsValid() bool { func NewSignatureProvider(logger log.Logger, providerType ProviderType) (SignatureProvider, error) { switch providerType { case KeyProviderGCP: - return NewCloudKMSSignatureProvider(logger), nil + return NewCloudKMSSignatureProvider(logger) case KeyProviderAWS: - // TODO: Implement AWS provider - return nil, fmt.Errorf("AWS provider not yet implemented") + return NewAWSKMSSignatureProvider(logger) default: return nil, fmt.Errorf("unsupported provider type: %s", providerType) } From b78337e9e942bd2c3d02838a5147fcece66cdbae Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 13:08:02 +0800 Subject: [PATCH 05/10] chore(deps): add AWS SDK dependencies Add AWS SDK dependencies required for AWS KMS implementation: - github.com/aws/aws-sdk-go-v2/config - github.com/aws/aws-sdk-go-v2 (indirect) These dependencies are needed for the AWS KMS signature provider functionality. --- op-signer/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-signer/go.mod b/op-signer/go.mod index 890a055e..e6b72328 100644 --- a/op-signer/go.mod +++ b/op-signer/go.mod @@ -4,6 +4,7 @@ go 1.22.0 require ( cloud.google.com/go/kms v1.12.1 + github.com/aws/aws-sdk-go-v2/config v1.28.11 github.com/ethereum-optimism/optimism v0.0.0-20241213111354-8bf7ff60f34a github.com/ethereum/go-ethereum v1.14.11 github.com/golang/mock v1.6.0 @@ -19,7 +20,6 @@ require ( require ( github.com/aws/aws-sdk-go-v2 v1.32.8 // indirect - github.com/aws/aws-sdk-go-v2/config v1.28.11 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.17.52 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.23 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.27 // indirect From 90e8f0375924209f7db1f5a165aec697e020f21b Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 13:15:38 +0800 Subject: [PATCH 06/10] chore(lint): fixed spelling --- op-signer/service/provider/aws_kms.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-signer/service/provider/aws_kms.go b/op-signer/service/provider/aws_kms.go index 0fb87aea..e9986ac1 100644 --- a/op-signer/service/provider/aws_kms.go +++ b/op-signer/service/provider/aws_kms.go @@ -10,7 +10,7 @@ import ( "github.com/ethereum/go-ethereum/log" ) -// AWSKMSClient is the minimal interface for the AWS KMS client requried by the AWSKMSSignatureProvider. These functions +// AWSKMSClient is the minimal interface for the AWS KMS client required by the AWSKMSSignatureProvider. These functions // are already implemented by the AWS SDK, but we define our own type to allow us to mock the client in tests. type AWSKMSClient interface { // https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/kms#Client.Sign From 51911ee74b0919db2b2e1ae9ab8a98d150d46d78 Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 16:39:02 +0800 Subject: [PATCH 07/10] feat(op-signer): add DER encoding utilities for secp256k1 keys Add utility functions to marshal and unmarshal secp256k1 public keys in DER format for AWS KMS integration. Includes comprehensive tests to verify encoding compatibility. - Add marshalECDSAPublicKey function for DER encoding - Add unmarshalECDSAPublicKey function for DER decoding - Add tests to verify marshal/unmarshal roundtrip - Add tests to verify compatibility with AWS KMS format --- op-signer/service/provider/utils.go | 79 ++++++++++++++++++++++++ op-signer/service/provider/utils_test.go | 38 ++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 op-signer/service/provider/utils.go create mode 100644 op-signer/service/provider/utils_test.go diff --git a/op-signer/service/provider/utils.go b/op-signer/service/provider/utils.go new file mode 100644 index 00000000..8fb06ba9 --- /dev/null +++ b/op-signer/service/provider/utils.go @@ -0,0 +1,79 @@ +package provider + +import ( + "crypto/ecdsa" + "crypto/x509/pkix" + "encoding/asn1" + "errors" + "fmt" + + "github.com/ethereum/go-ethereum/crypto/secp256k1" +) + +// marshalECDSAPublicKey marshals a secp256k1 public key into DER format. This +// is needed because the Golang standard crypto/esdsa and crypto/elliptic libs +// do not support secp256k1. +func marshalECDSAPublicKey(pub *ecdsa.PublicKey) ([]byte, error) { + algoFullBytes, _ := asn1.Marshal(oidNamedCurveSECP256K1) + + // Encode AlgorithmIdentifier with OID for ECDSA and named curve + publicKeyAlgorithm := pkix.AlgorithmIdentifier{ + Algorithm: oidPublicKeyECDSA, + Parameters: asn1.RawValue{ + Tag: asn1.TagOID, + FullBytes: algoFullBytes, + Class: asn1.ClassUniversal, + IsCompound: false, + }, + } + + // Marshal the public key point (X, Y) + publicKeyBytes := secp256k1.S256().Marshal(pub.X, pub.Y) + + // Construct the publicKeyInfo structure + pkix := publicKeyInfo{ + Algorithm: publicKeyAlgorithm, + PublicKey: asn1.BitString{ + Bytes: publicKeyBytes, + BitLength: len(publicKeyBytes) * 8, + }, + } + + // Encode the structure into DER + return asn1.Marshal(pkix) +} + +// unmarshalECDSAPublicKey parses a secp256k1 public key from DER format. +func unmarshalECDSAPublicKey(derBytes []byte) (*ecdsa.PublicKey, error) { + var pkInfo publicKeyInfo + + // Decode DER bytes into publicKeyInfo structure + _, err := asn1.Unmarshal(derBytes, &pkInfo) + if err != nil { + return nil, err + } + + namedCurveOID := new(asn1.ObjectIdentifier) + _, err = asn1.Unmarshal(pkInfo.Algorithm.Parameters.FullBytes, namedCurveOID) + if err != nil { + return nil, fmt.Errorf("x509: failed to parse ECDSA parameters as named curve: %w", err) + } + + if !namedCurveOID.Equal(oidNamedCurveSECP256K1) { + return nil, errors.New("x509: unsupported elliptic curve") + } + + asn1Data := pkInfo.PublicKey.RightAlign() + if asn1Data[0] != 4 { // uncompressed form + return nil, errors.New("x509: only uncompressed keys are supported") + } + + // Decode the public key point + curve := secp256k1.S256() + x, y := curve.Unmarshal(pkInfo.PublicKey.Bytes) + if x == nil || y == nil { + return nil, fmt.Errorf("invalid public key") + } + + return &ecdsa.PublicKey{Curve: curve, X: x, Y: y}, nil +} diff --git a/op-signer/service/provider/utils_test.go b/op-signer/service/provider/utils_test.go new file mode 100644 index 00000000..889722c4 --- /dev/null +++ b/op-signer/service/provider/utils_test.go @@ -0,0 +1,38 @@ +package provider + +import ( + "testing" + + "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMarshalAndUnmarshalECDSAPublicKey tests that a secp256k1 public key can be +// marshaled and unmarshaled correctly. +func TestMarshalAndUnmarshalECDSAPublicKey(t *testing.T) { + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + publicKeyDER, err := marshalECDSAPublicKey(&privateKey.PublicKey) + require.NoError(t, err) + + unmarshalledPublicKey, err := unmarshalECDSAPublicKey(publicKeyDER) + require.NoError(t, err) + + assert.Equal(t, privateKey.PublicKey, *unmarshalledPublicKey) +} + +// TestMarshalAndParseECDSAPublicKey tests that a secp256k1 public key can be +// marshaled and parsed using the existing x509ParseECDSAPublicKey function. This +// ensures that the public key is in the correct format for the AWS KMS client. +func TestMarshalAndParseECDSAPublicKey(t *testing.T) { + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + publicKeyDER, err := marshalECDSAPublicKey(&privateKey.PublicKey) + require.NoError(t, err) + + _, err = x509ParseECDSAPublicKey(publicKeyDER) + require.NoError(t, err) +} From 65640169508b4f622a6b08f5c9913dd2f50c9129 Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 17:08:56 +0800 Subject: [PATCH 08/10] feat(op-signer): add signature format conversion utilities Add utilities for converting between Ethereum's compact recoverable signature format and DER format, with comprehensive tests. - Add convertCompactRecoverableSignatureToDER function - Add test for DER conversion roundtrip - Add test for signature recoverability - Add test for signature format compatibility with go-ethereum This enables proper signature format handling between AWS KMS (DER) and Ethereum (compact recoverable) --- op-signer/service/provider/utils.go | 31 +++++++++++++++ op-signer/service/provider/utils_test.go | 50 ++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/op-signer/service/provider/utils.go b/op-signer/service/provider/utils.go index 8fb06ba9..aabde976 100644 --- a/op-signer/service/provider/utils.go +++ b/op-signer/service/provider/utils.go @@ -6,6 +6,7 @@ import ( "encoding/asn1" "errors" "fmt" + "math/big" "github.com/ethereum/go-ethereum/crypto/secp256k1" ) @@ -77,3 +78,33 @@ func unmarshalECDSAPublicKey(derBytes []byte) (*ecdsa.PublicKey, error) { return &ecdsa.PublicKey{Curve: curve, X: x, Y: y}, nil } + +// ConvertCompactRecoverableSignatureToDER converts an Ethereum signature in +// [R || S || V] format to DER format. +func convertCompactRecoverableSignatureToDER(sig []byte) ([]byte, error) { + // Ensure the signature is the correct length (65 bytes: R=32, S=32, V=1) + if len(sig) != 65 { + return nil, fmt.Errorf("invalid signature length: expected 65 bytes, got %d", len(sig)) + } + + // Extract R and S components + r := new(big.Int).SetBytes(sig[:32]) // First 32 bytes + s := new(big.Int).SetBytes(sig[32:64]) // Next 32 bytes + + // Create a struct representing the DER sequence + derSignature := struct { + R *big.Int + S *big.Int + }{ + R: r, + S: s, + } + + // Marshal the struct into DER + derBytes, err := asn1.Marshal(derSignature) + if err != nil { + return nil, fmt.Errorf("failed to marshal DER: %v", err) + } + + return derBytes, nil +} diff --git a/op-signer/service/provider/utils_test.go b/op-signer/service/provider/utils_test.go index 889722c4..ab9c05b9 100644 --- a/op-signer/service/provider/utils_test.go +++ b/op-signer/service/provider/utils_test.go @@ -36,3 +36,53 @@ func TestMarshalAndParseECDSAPublicKey(t *testing.T) { _, err = x509ParseECDSAPublicKey(publicKeyDER) require.NoError(t, err) } + +// TestConvertEthereumSignatureToDER tests that an Ethereum signature can be +// converted to DER format. This test also ensures that our DER encoding is +// correct by converting from DER back to a compact sig using the existing +// convertToCompactSignature function. +func TestConvertEthereumSignatureToDER(t *testing.T) { + privateKey, _ := crypto.GenerateKey() + + // Sign a message using the private key so we can later verify the signature + message := []byte("test message to sign") + digest := crypto.Keccak256Hash(message).Bytes() + + signature, err := crypto.Sign(digest, privateKey) + require.NoError(t, err) + + derSignature, err := convertCompactRecoverableSignatureToDER(signature) + require.NoError(t, err) + + convertedSignature, err := convertToCompactSignature(derSignature) + require.NoError(t, err) + assert.Equal(t, signature[:len(signature)-1], convertedSignature) +} + +// TestConvertToEthereumSignatureRecoverable tests that an Ethereum signature +// can be converted to a recoverable signature. This test also ensures that +// our DER encoding is correct by converting from DER back to a compact +// verifiable sig using our convertToCompactRecoverableSignature function. +func TestConvertToEthereumSignatureRecoverable(t *testing.T) { + privateKey, _ := crypto.GenerateKey() + + // Sign a message using the private key so we can later verify the signature + message := []byte("test message to sign") + digest := crypto.Keccak256Hash(message).Bytes() + + signature, err := crypto.Sign(digest, privateKey) + require.NoError(t, err) + + derSignature, err := convertCompactRecoverableSignatureToDER(signature) + require.NoError(t, err) + + publicKeyDER, err := marshalECDSAPublicKey(&privateKey.PublicKey) + require.NoError(t, err) + + publicKeyBytes, err := x509ParseECDSAPublicKey(publicKeyDER) + require.NoError(t, err) + + convertedSignature, err := convertToCompactRecoverableSignature(derSignature, digest, publicKeyBytes) + require.NoError(t, err) + assert.Equal(t, signature, convertedSignature) +} From 7d63efc3fadfbfd57a24c7d9e753b413124da03a Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 17:14:56 +0800 Subject: [PATCH 09/10] test(op-signer): add AWS KMS signature provider tests Add comprehensive tests for AWS KMS signature provider implementation: - Add mock AWS KMS client for testing - Test GetPublicKey with DER-encoded secp256k1 keys - Test SignDigest with signature verification and recovery - Verify compatibility with Ethereum signature format - Test signature malleability and length requirements --- op-signer/service/provider/aws_kms_test.go | 103 +++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 op-signer/service/provider/aws_kms_test.go diff --git a/op-signer/service/provider/aws_kms_test.go b/op-signer/service/provider/aws_kms_test.go new file mode 100644 index 00000000..dc4da144 --- /dev/null +++ b/op-signer/service/provider/aws_kms_test.go @@ -0,0 +1,103 @@ +package provider + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/service/kms" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/secp256k1" + "github.com/ethereum/go-ethereum/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mockAWSKMSClient implements AWSKMSClient interface for testing +type mockAWSKMSClient struct { + signOutput *kms.SignOutput + getPublicKeyOutput *kms.GetPublicKeyOutput +} + +func (m *mockAWSKMSClient) Sign(ctx context.Context, params *kms.SignInput, optFns ...func(*kms.Options)) (*kms.SignOutput, error) { + return m.signOutput, nil +} + +func (m *mockAWSKMSClient) GetPublicKey(ctx context.Context, params *kms.GetPublicKeyInput, optFns ...func(*kms.Options)) (*kms.GetPublicKeyOutput, error) { + return m.getPublicKeyOutput, nil +} + +func TestAWSKMSSignatureProvider_GetPublicKey(t *testing.T) { + // Generate a secp256k1 key pair + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + publicKey := privateKey.PublicKey + publicKeyDER, err := marshalECDSAPublicKey(&publicKey) + require.NoError(t, err) + + mockClient := &mockAWSKMSClient{ + getPublicKeyOutput: &kms.GetPublicKeyOutput{ + PublicKey: publicKeyDER, + }, + } + + provider := NewAWSKMSSignatureProviderWithClient(log.New(), mockClient) + + _, err = provider.GetPublicKey(context.Background(), "test-key") + require.NoError(t, err) +} + +func TestAWSKMSSignatureProvider_SignDigest(t *testing.T) { + // Generate a secp256k1 key pair + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + publicKey := privateKey.PublicKey + publicKeyDER, err := marshalECDSAPublicKey(&publicKey) + if err != nil { + t.Fatalf("failed to marshal public key: %v", err) + } + + // Sign a message using the private key so we can later verify the signature + message := []byte("test message to sign") + digest := crypto.Keccak256Hash(message).Bytes() + + compactRecoverableSig, err := crypto.Sign(digest, privateKey) + require.NoError(t, err) + + derSignature, err := convertCompactRecoverableSignatureToDER(compactRecoverableSig) + require.NoError(t, err) + + // Create mock AWS KMS client that returns our test signature + mockClient := &mockAWSKMSClient{ + getPublicKeyOutput: &kms.GetPublicKeyOutput{ + PublicKey: publicKeyDER, + }, + signOutput: &kms.SignOutput{ + Signature: derSignature, + }, + } + + // Create provider with mock client + provider := NewAWSKMSSignatureProviderWithClient(log.New(), mockClient) + + // Test signing + signature, err := provider.SignDigest(context.Background(), "test-key", digest) + require.NoError(t, err) + + // Verify signature length (65 bytes: r[32] || s[32] || v[1]) + assert.Equal(t, 65, len(signature)) + + // Verify signature is recoverable + recoveredPub, err := secp256k1.RecoverPubkey(digest, signature) + require.NoError(t, err) + + // Verify recovered public key matches original + publicKeyBytes, err := x509ParseECDSAPublicKey(publicKeyDER) + require.NoError(t, err) + + assert.Equal(t, publicKeyBytes, recoveredPub) + + // Verify signature using go-ethereum's crypto implementation + assert.True(t, crypto.VerifySignature(publicKeyBytes, digest, signature[:64])) +} From 410a13176dedc4ae2f261f1eb06a562807f16165 Mon Sep 17 00:00:00 2001 From: Dawson <1885328+ddaws@users.noreply.github.com> Date: Wed, 15 Jan 2025 17:21:57 +0800 Subject: [PATCH 10/10] fix(op-signer): use %w verb for error wrapping in DER conversion Update error formatting in convertCompactRecoverableSignatureToDER to use %w instead of %v to properly wrap and preserve the original error context. --- op-signer/service/provider/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-signer/service/provider/utils.go b/op-signer/service/provider/utils.go index aabde976..058756de 100644 --- a/op-signer/service/provider/utils.go +++ b/op-signer/service/provider/utils.go @@ -103,7 +103,7 @@ func convertCompactRecoverableSignatureToDER(sig []byte) ([]byte, error) { // Marshal the struct into DER derBytes, err := asn1.Marshal(derSignature) if err != nil { - return nil, fmt.Errorf("failed to marshal DER: %v", err) + return nil, fmt.Errorf("failed to marshal DER: %w", err) } return derBytes, nil