From cd40d41a52c4be00184127dc35d8bc1b950ff7e4 Mon Sep 17 00:00:00 2001 From: iseki Date: Sat, 2 Mar 2024 16:51:32 +0800 Subject: [PATCH] rewrite the GCM encryption function, altering more error comparison methods. --- signer/core/api.go | 8 +-- signer/core/signed_data.go | 2 +- signer/core/uiapi.go | 10 ++-- signer/storage/aes_gcm_storage.go | 71 +++++++++++++++----------- signer/storage/aes_gcm_storage_test.go | 6 ++- 5 files changed, 54 insertions(+), 43 deletions(-) diff --git a/signer/core/api.go b/signer/core/api.go index a32f24cb18..3b0e11b049 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -332,7 +332,7 @@ func (api *SignerAPI) startUSBListener() { for _, wallet := range am.Wallets() { if err := wallet.Open(""); err != nil { log.Warn("Failed to open wallet", "url", wallet.URL(), "err", err) - if err == usbwallet.ErrTrezorPINNeeded { + if errors.Is(err, usbwallet.ErrTrezorPINNeeded) { go api.openTrezor(wallet.URL()) } } @@ -348,7 +348,7 @@ func (api *SignerAPI) derivationLoop(events chan accounts.WalletEvent) { case accounts.WalletArrived: if err := event.Wallet.Open(""); err != nil { log.Warn("New wallet appeared, failed to open", "url", event.Wallet.URL(), "err", err) - if err == usbwallet.ErrTrezorPINNeeded { + if errors.Is(err, usbwallet.ErrTrezorPINNeeded) { go api.openTrezor(event.Wallet.URL()) } } @@ -663,8 +663,8 @@ func (api *SignerAPI) SignGnosisSafeTx(ctx context.Context, signerAddress common return &gnosisTx, nil } -// Returns the external api version. This method does not require user acceptance. Available methods are +// Version Returns the external api version. This method does not require user acceptance. Available methods are // available via enumeration anyway, and this info does not contain user-specific data -func (api *SignerAPI) Version(ctx context.Context) (string, error) { +func (api *SignerAPI) Version(_ context.Context) (string, error) { return ExternalAPIVersion, nil } diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index f8b3c9d86d..9dfdca3653 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -291,7 +291,7 @@ func typedDataRequest(data any) (*SignDataRequest, error) { // EcRecover recovers the address associated with the given sig. // Only compatible with `text/plain` -func (api *SignerAPI) EcRecover(ctx context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) { +func (api *SignerAPI) EcRecover(_ context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) { // Returns the address for the Account that was used to create the signature. // // Note, this function is compatible with eth_sign and personal_sign. As such it recovers diff --git a/signer/core/uiapi.go b/signer/core/uiapi.go index b8c3acfb4d..76c8b7d06c 100644 --- a/signer/core/uiapi.go +++ b/signer/core/uiapi.go @@ -48,11 +48,11 @@ func NewUIServerAPI(extapi *SignerAPI) *UIServerAPI { return &UIServerAPI{extapi, extapi.am} } -// List available accounts. As opposed to the external API definition, this method delivers +// ListAccounts List available accounts. As opposed to the external API definition, this method delivers // the full Account object and not only Address. // Example call // {"jsonrpc":"2.0","method":"clef_listAccounts","params":[], "id":4} -func (s *UIServerAPI) ListAccounts(ctx context.Context) ([]accounts.Account, error) { +func (s *UIServerAPI) ListAccounts(_ context.Context) ([]accounts.Account, error) { var accs []accounts.Account for _, wallet := range s.am.Wallets() { accs = append(accs, wallet.Accounts()...) @@ -170,7 +170,7 @@ func (s *UIServerAPI) SetChainId(id math.HexOrDecimal64) math.HexOrDecimal64 { // Export returns encrypted private key associated with the given address in web3 keystore format. // Example // {"jsonrpc":"2.0","method":"clef_export","params":["0x19e7e376e7c213b7e7e7e46cc70a5dd086daff2a"], "id":4} -func (s *UIServerAPI) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) { +func (s *UIServerAPI) Export(_ context.Context, addr common.Address) (json.RawMessage, error) { // Look up the wallet containing the requested signer wallet, err := s.am.Find(accounts.Account{Address: addr}) if err != nil { @@ -187,7 +187,7 @@ func (s *UIServerAPI) Export(ctx context.Context, addr common.Address) (json.Raw // decryption it will encrypt the key with the given newPassphrase and store it in the keystore. // Example (the address in question has privkey `11...11`): // {"jsonrpc":"2.0","method":"clef_import","params":[{"address":"19e7e376e7c213b7e7e7e46cc70a5dd086daff2a","crypto":{"cipher":"aes-128-ctr","ciphertext":"33e4cd3756091d037862bb7295e9552424a391a6e003272180a455ca2a9fb332","cipherparams":{"iv":"b54b263e8f89c42bb219b6279fba5cce"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"e4ca94644fd30569c1b1afbbc851729953c92637b7fe4bb9840bbb31ffbc64a5"},"mac":"f4092a445c2b21c0ef34f17c9cd0d873702b2869ec5df4439a0c2505823217e7"},"id":"216c7eac-e8c1-49af-a215-fa0036f29141","version":3},"test","yaddayadda"], "id":4} -func (api *UIServerAPI) Import(ctx context.Context, keyJSON json.RawMessage, oldPassphrase, newPassphrase string) (accounts.Account, error) { +func (api *UIServerAPI) Import(_ context.Context, keyJSON json.RawMessage, oldPassphrase, newPassphrase string) (accounts.Account, error) { be := api.am.Backends(keystore.KeyStoreType) if len(be) == 0 { @@ -205,7 +205,7 @@ func (api *UIServerAPI) Import(ctx context.Context, keyJSON json.RawMessage, old // This method is the same as New on the external API, the difference being that // this implementation does not ask for confirmation, since it's initiated by // the user -func (api *UIServerAPI) New(ctx context.Context) (common.Address, error) { +func (api *UIServerAPI) New(_ context.Context) (common.Address, error) { return api.extApi.newAccount() } diff --git a/signer/storage/aes_gcm_storage.go b/signer/storage/aes_gcm_storage.go index 928d643dd6..913a5f0091 100644 --- a/signer/storage/aes_gcm_storage.go +++ b/signer/storage/aes_gcm_storage.go @@ -21,7 +21,7 @@ import ( "crypto/cipher" "crypto/rand" "encoding/json" - "io" + "errors" "os" "github.com/ethereum/go-ethereum/log" @@ -41,14 +41,32 @@ type AESEncryptedStorage struct { filename string // Key stored in base64 key []byte + gcm cipher.AEAD + + gcmNS int // NonceSize } // NewAESEncryptedStorage creates a new encrypted storage backed by the given file/key func NewAESEncryptedStorage(filename string, key []byte) *AESEncryptedStorage { - return &AESEncryptedStorage{ + aesStore := &AESEncryptedStorage{ filename: filename, key: key, } + + blk, err := aes.NewCipher(key) + if err != nil { + log.Warn("reading AES key", "err", err) + return nil + } + + aesStore.gcm, err = cipher.NewGCM(blk) + if err != nil { + log.Warn("initializing AES AEAD", "err", err) + return nil + } + + aesStore.gcmNS = aesStore.gcm.NonceSize() + return aesStore } // Put stores a value by key. 0-length keys results in noop. @@ -61,7 +79,7 @@ func (s *AESEncryptedStorage) Put(key, value string) { log.Warn("Failed to read encrypted storage", "err", err, "file", s.filename) return } - ciphertext, iv, err := encrypt(s.key, []byte(value), []byte(key)) + ciphertext, iv, err := s.encrypt([]byte(value), []byte(key)) if err != nil { log.Warn("Failed to encrypt entry", "err", err) return @@ -89,7 +107,7 @@ func (s *AESEncryptedStorage) Get(key string) (string, error) { log.Warn("Key does not exist", "key", key) return "", ErrNotFound } - entry, err := decrypt(s.key, encrypted.Iv, encrypted.CipherText, []byte(key)) + entry, err := s.decrypt(encrypted.CipherText, []byte(key)) if err != nil { log.Warn("Failed to decrypt key", "key", key) return "", err @@ -141,38 +159,29 @@ func (s *AESEncryptedStorage) writeEncryptedStorage(creds map[string]storedCrede return nil } +func randBytes(size int) (blk []byte, err error) { + blk = make([]byte, size) + _, err = rand.Read(blk) + return +} + // encrypt encrypts plaintext with the given key, with additional data // The 'additionalData' is used to place the (plaintext) KV-store key into the V, // to prevent the possibility to alter a K, or swap two entries in the KV store with each other. -func encrypt(key []byte, plaintext []byte, additionalData []byte) ([]byte, []byte, error) { - block, err := aes.NewCipher(key) - if err != nil { - return nil, nil, err +func (s *AESEncryptedStorage) encrypt(plaintext []byte, additionalData []byte) (_ []byte, _ []byte, err error) { + // Never use more than 2^32 random nonce's with a given + // key because of the risk of a repeat. + var n []byte + + if n, err = randBytes(s.gcmNS); err != nil { + return } - aesgcm, err := cipher.NewGCM(block) - if err != nil { - return nil, nil, err - } - nonce := make([]byte, aesgcm.NonceSize()) - if _, err := io.ReadFull(rand.Reader, nonce); err != nil { - return nil, nil, err - } - ciphertext := aesgcm.Seal(nil, nonce, plaintext, additionalData) - return ciphertext, nonce, nil + return append(n, s.gcm.Seal(nil, n, plaintext, additionalData)...), n, nil } -func decrypt(key []byte, nonce []byte, ciphertext []byte, additionalData []byte) ([]byte, error) { - block, err := aes.NewCipher(key) - if err != nil { - return nil, err +func (s *AESEncryptedStorage) decrypt(ciphertext []byte, additionalData []byte) ([]byte, error) { + if len(ciphertext) < s.gcmNS { + return nil, errors.New("cipher data too short") } - aesgcm, err := cipher.NewGCM(block) - if err != nil { - return nil, err - } - plaintext, err := aesgcm.Open(nil, nonce, ciphertext, additionalData) - if err != nil { - return nil, err - } - return plaintext, nil + return s.gcm.Open(nil, ciphertext[0:s.gcmNS], ciphertext[s.gcmNS:], additionalData) } diff --git a/signer/storage/aes_gcm_storage_test.go b/signer/storage/aes_gcm_storage_test.go index a223b1a6b4..b2eca91739 100644 --- a/signer/storage/aes_gcm_storage_test.go +++ b/signer/storage/aes_gcm_storage_test.go @@ -36,13 +36,15 @@ func TestEncryption(t *testing.T) { key := []byte("AES256Key-32Characters1234567890") plaintext := []byte("exampleplaintext") - c, iv, err := encrypt(key, plaintext, nil) + aesStore := NewAESEncryptedStorage("", key) + + c, iv, err := aesStore.encrypt(plaintext, nil) if err != nil { t.Fatal(err) } t.Logf("Ciphertext %x, nonce %x\n", c, iv) - p, err := decrypt(key, iv, c, nil) + p, err := aesStore.decrypt(c, nil) if err != nil { t.Fatal(err) }