-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bring KMS client and multiclient over to chainlink #14484
Merged
connorwstein
merged 17 commits into
develop
from
chore/move-multiclient-and-kmsclient-to-chainlink
Sep 19, 2024
Merged
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
e89847b
Bring KMS client and multiclient over to chainlink
ogtownsend 2381add
Pull seth client into multiclient
ogtownsend 1425b3e
remove toml tags
AnieeG b5f1c48
Keep multiclient simple
connorwstein 188e993
Config validation for kms
connorwstein 7b1592b
Rename to ws, fix test
connorwstein 58b666b
Merge branch 'develop' into chore/move-multiclient-and-kmsclient-to-c…
connorwstein 96e002d
Changeset
connorwstein 7ed0046
Merge branch 'chore/move-multiclient-and-kmsclient-to-chainlink' of g…
connorwstein c0adf8c
Downgrade seth
connorwstein d1ab5b8
Merge branch 'develop' into chore/move-multiclient-and-kmsclient-to-c…
connorwstein 532483d
Lint
connorwstein 2a1d1c3
Merge branch 'chore/move-multiclient-and-kmsclient-to-chainlink' of g…
connorwstein 87883aa
Merge branch 'develop' into chore/move-multiclient-and-kmsclient-to-c…
connorwstein f610a30
More lint
connorwstein 219fb93
Merge branch 'chore/move-multiclient-and-kmsclient-to-chainlink' of g…
connorwstein dc65ea7
Merge branch 'develop' into chore/move-multiclient-and-kmsclient-to-c…
connorwstein File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"chainlink": patch | ||
--- | ||
|
||
#internal KMS client for deployment |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,233 @@ | ||
package deployment | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"crypto/ecdsa" | ||
"encoding/asn1" | ||
"encoding/hex" | ||
"fmt" | ||
"math/big" | ||
|
||
"github.com/aws/aws-sdk-go/aws/session" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/service/kms" | ||
"github.com/ethereum/go-ethereum/accounts/abi/bind" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/core/types" | ||
"github.com/ethereum/go-ethereum/crypto" | ||
"github.com/ethereum/go-ethereum/crypto/secp256k1" | ||
) | ||
|
||
var ( | ||
secp256k1N = crypto.S256().Params().N | ||
secp256k1HalfN = new(big.Int).Div(secp256k1N, big.NewInt(2)) | ||
) | ||
|
||
// See https://docs.aws.amazon.com/kms/latest/APIReference/API_GetPublicKey.html#API_GetPublicKey_ResponseSyntax | ||
// and https://datatracker.ietf.org/doc/html/rfc5280 for why we need to unpack the KMS public key. | ||
type asn1SubjectPublicKeyInfo struct { | ||
AlgorithmIdentifier asn1AlgorithmIdentifier | ||
SubjectPublicKey asn1.BitString | ||
} | ||
|
||
type asn1AlgorithmIdentifier struct { | ||
Algorithm asn1.ObjectIdentifier | ||
Parameters asn1.ObjectIdentifier | ||
} | ||
|
||
// See https://aws.amazon.com/blogs/database/part2-use-aws-kms-to-securely-manage-ethereum-accounts/ for why we | ||
// need to manually prep the signature for Ethereum. | ||
type asn1ECDSASig struct { | ||
R asn1.RawValue | ||
S asn1.RawValue | ||
} | ||
|
||
// TODO: Mockery gen then test with a regular eth key behind the interface. | ||
type KMSClient interface { | ||
GetPublicKey(input *kms.GetPublicKeyInput) (*kms.GetPublicKeyOutput, error) | ||
Sign(input *kms.SignInput) (*kms.SignOutput, error) | ||
} | ||
|
||
type KMS struct { | ||
KmsDeployerKeyId string | ||
KmsDeployerKeyRegion string | ||
AwsProfileName string | ||
} | ||
|
||
func NewKMSClient(config KMS) (KMSClient, error) { | ||
if config.KmsDeployerKeyId == "" { | ||
return nil, fmt.Errorf("KMS key ID is required") | ||
} | ||
if config.KmsDeployerKeyRegion == "" { | ||
return nil, fmt.Errorf("KMS key region is required") | ||
} | ||
var awsSessionFn AwsSessionFn | ||
if config.AwsProfileName != "" { | ||
awsSessionFn = awsSessionFromProfileFn | ||
} else { | ||
awsSessionFn = awsSessionFromEnvVarsFn | ||
} | ||
return kms.New(awsSessionFn(config)), nil | ||
} | ||
|
||
type EVMKMSClient struct { | ||
Client KMSClient | ||
KeyID string | ||
} | ||
|
||
func NewEVMKMSClient(client KMSClient, keyID string) *EVMKMSClient { | ||
return &EVMKMSClient{ | ||
Client: client, | ||
KeyID: keyID, | ||
} | ||
} | ||
|
||
func (c *EVMKMSClient) GetKMSTransactOpts(ctx context.Context, chainID *big.Int) (*bind.TransactOpts, error) { | ||
ecdsaPublicKey, err := c.GetECDSAPublicKey() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
pubKeyBytes := secp256k1.S256().Marshal(ecdsaPublicKey.X, ecdsaPublicKey.Y) | ||
keyAddr := crypto.PubkeyToAddress(*ecdsaPublicKey) | ||
if chainID == nil { | ||
return nil, fmt.Errorf("chainID is required") | ||
} | ||
signer := types.LatestSignerForChainID(chainID) | ||
|
||
signerFn := func(address common.Address, tx *types.Transaction) (*types.Transaction, error) { | ||
if address != keyAddr { | ||
return nil, bind.ErrNotAuthorized | ||
} | ||
|
||
txHashBytes := signer.Hash(tx).Bytes() | ||
|
||
mType := kms.MessageTypeDigest | ||
algo := kms.SigningAlgorithmSpecEcdsaSha256 | ||
signOutput, err := c.Client.Sign( | ||
&kms.SignInput{ | ||
KeyId: &c.KeyID, | ||
SigningAlgorithm: &algo, | ||
MessageType: &mType, | ||
Message: txHashBytes, | ||
}) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to call kms.Sign() on transaction: %v", err) | ||
} | ||
|
||
ethSig, err := kmsToEthSig(signOutput.Signature, pubKeyBytes, txHashBytes) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to convert KMS signature to Ethereum signature: %v", err) | ||
} | ||
|
||
return tx.WithSignature(signer, ethSig) | ||
} | ||
|
||
return &bind.TransactOpts{ | ||
From: keyAddr, | ||
Signer: signerFn, | ||
Context: ctx, | ||
}, nil | ||
} | ||
|
||
// GetECDSAPublicKey retrieves the public key from KMS and converts it to its ECDSA representation. | ||
func (c *EVMKMSClient) GetECDSAPublicKey() (*ecdsa.PublicKey, error) { | ||
getPubKeyOutput, err := c.Client.GetPublicKey(&kms.GetPublicKeyInput{ | ||
KeyId: aws.String(c.KeyID), | ||
}) | ||
if err != nil { | ||
return nil, fmt.Errorf("can not get public key from KMS for KeyId=%s: %s", c.KeyID, err) | ||
} | ||
|
||
var asn1pubKeyInfo asn1SubjectPublicKeyInfo | ||
_, err = asn1.Unmarshal(getPubKeyOutput.PublicKey, &asn1pubKeyInfo) | ||
if err != nil { | ||
return nil, fmt.Errorf("can not parse asn1 public key for KeyId=%s: %s", c.KeyID, err) | ||
} | ||
|
||
pubKey, err := crypto.UnmarshalPubkey(asn1pubKeyInfo.SubjectPublicKey.Bytes) | ||
if err != nil { | ||
return nil, fmt.Errorf("can not unmarshal public key bytes: %s", err) | ||
} | ||
return pubKey, nil | ||
} | ||
|
||
func kmsToEthSig(kmsSig, ecdsaPubKeyBytes, hash []byte) ([]byte, error) { | ||
var asn1Sig asn1ECDSASig | ||
_, err := asn1.Unmarshal(kmsSig, &asn1Sig) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
rBytes := asn1Sig.R.Bytes | ||
sBytes := asn1Sig.S.Bytes | ||
|
||
// Adjust S value from signature to match Eth standard. | ||
// See: https://aws.amazon.com/blogs/database/part2-use-aws-kms-to-securely-manage-ethereum-accounts/ | ||
// "After we extract r and s successfully, we have to test if the value of s is greater than secp256k1n/2 as | ||
// specified in EIP-2 and flip it if required." | ||
sBigInt := new(big.Int).SetBytes(sBytes) | ||
if sBigInt.Cmp(secp256k1HalfN) > 0 { | ||
sBytes = new(big.Int).Sub(secp256k1N, sBigInt).Bytes() | ||
} | ||
|
||
return recoverEthSignature(ecdsaPubKeyBytes, hash, rBytes, sBytes) | ||
} | ||
|
||
// See: https://aws.amazon.com/blogs/database/part2-use-aws-kms-to-securely-manage-ethereum-accounts/ | ||
func recoverEthSignature(expectedPublicKeyBytes, txHash, r, s []byte) ([]byte, error) { | ||
rsSig := append(padTo32Bytes(r), padTo32Bytes(s)...) | ||
ethSig := append(rsSig, []byte{0}...) | ||
|
||
recoveredPublicKeyBytes, err := crypto.Ecrecover(txHash, ethSig) | ||
if err != nil { | ||
return nil, fmt.Errorf("failing to call Ecrecover: %v", err) | ||
} | ||
|
||
if hex.EncodeToString(recoveredPublicKeyBytes) != hex.EncodeToString(expectedPublicKeyBytes) { | ||
ethSig = append(rsSig, []byte{1}...) | ||
recoveredPublicKeyBytes, err = crypto.Ecrecover(txHash, ethSig) | ||
if err != nil { | ||
return nil, fmt.Errorf("failing to call Ecrecover: %v", err) | ||
} | ||
|
||
if hex.EncodeToString(recoveredPublicKeyBytes) != hex.EncodeToString(expectedPublicKeyBytes) { | ||
return nil, fmt.Errorf("can not reconstruct public key from sig") | ||
} | ||
} | ||
|
||
return ethSig, nil | ||
} | ||
|
||
func padTo32Bytes(buffer []byte) []byte { | ||
buffer = bytes.TrimLeft(buffer, "\x00") | ||
for len(buffer) < 32 { | ||
zeroBuf := []byte{0} | ||
buffer = append(zeroBuf, buffer...) | ||
} | ||
return buffer | ||
} | ||
|
||
type AwsSessionFn func(config KMS) *session.Session | ||
|
||
var awsSessionFromEnvVarsFn = func(config KMS) *session.Session { | ||
return session.Must( | ||
session.NewSession(&aws.Config{ | ||
Region: aws.String(config.KmsDeployerKeyRegion), | ||
CredentialsChainVerboseErrors: aws.Bool(true), | ||
})) | ||
} | ||
|
||
var awsSessionFromProfileFn = func(config KMS) *session.Session { | ||
return session.Must( | ||
session.NewSessionWithOptions(session.Options{ | ||
SharedConfigState: session.SharedConfigEnable, | ||
Profile: config.AwsProfileName, | ||
Config: aws.Config{ | ||
Region: aws.String(config.KmsDeployerKeyRegion), | ||
CredentialsChainVerboseErrors: aws.Bool(true), | ||
}, | ||
})) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package deployment | ||
|
||
import ( | ||
"encoding/hex" | ||
"testing" | ||
|
||
"github.com/test-go/testify/require" | ||
) | ||
|
||
func TestKMSToEthSigConversion(t *testing.T) { | ||
kmsSigBytes, err := hex.DecodeString("304402206168865941bafcae3a8cf8b26edbb5693d62222b2e54d962c1aabbeaddf33b6802205edc7f597d2bf2d1eaa14fc514a6202bafcffe52b13ae3fec00674d92a874b73") | ||
require.NoError(t, err) | ||
ecdsaPublicKeyBytes, err := hex.DecodeString("04a735e9e3cb526f83be23b03f1f5ae7788a8654e3f0fcfb4f978290de07ebd47da30eeb72e904fdd4a81b46e320908ff4345e119148f89c1f04674c14a506e24b") | ||
require.NoError(t, err) | ||
txHashBytes, err := hex.DecodeString("a2f037301e90f58c084fe4bec2eef14b26e620d6b6cb46051037d03b29ab7d9a") | ||
require.NoError(t, err) | ||
expectedEthSignBytes, err := hex.DecodeString("6168865941bafcae3a8cf8b26edbb5693d62222b2e54d962c1aabbeaddf33b685edc7f597d2bf2d1eaa14fc514a6202bafcffe52b13ae3fec00674d92a874b7300") | ||
require.NoError(t, err) | ||
|
||
actualEthSig, err := kmsToEthSig( | ||
kmsSigBytes, | ||
ecdsaPublicKeyBytes, | ||
txHashBytes, | ||
) | ||
require.NoError(t, err) | ||
require.Equal(t, expectedEthSignBytes, actualEthSig) | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this and the associated function def'n can be deleted, sorting is no longer a requirement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, not required to do in this PR ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this whole directory could probably deleted no? Thats what I had in mind with CCIP-3049
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these helpers aren't used anywhere and these tests are not needed, yes.