rewrite the GCM encryption function, altering more error comparison methods.

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
 }

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

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()
 }
 

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) {
+func (s *AESEncryptedStorage) encrypt(plaintext []byte, additionalData []byte) (_ []byte, _ []byte, err error) {
-	block, err := aes.NewCipher(key)
+	// Never use more than 2^32 random nonce's with a given
-	if err != nil {
+	// key because of the risk of a repeat.
-		return nil, nil, err
+	var n []byte
+
+	if n, err = randBytes(s.gcmNS); err != nil {
+		return
 	}
-	aesgcm, err := cipher.NewGCM(block)
+	return append(n, s.gcm.Seal(nil, n, plaintext, additionalData)...), n, nil
-	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
 }
 
-func decrypt(key []byte, nonce []byte, ciphertext []byte, additionalData []byte) ([]byte, error) {
+func (s *AESEncryptedStorage) decrypt(ciphertext []byte, additionalData []byte) ([]byte, error) {
-	block, err := aes.NewCipher(key)
+	if len(ciphertext) < s.gcmNS {
-	if err != nil {
+		return nil, errors.New("cipher data too short")
-		return nil, err
 	}
-	aesgcm, err := cipher.NewGCM(block)
+	return s.gcm.Open(nil, ciphertext[0:s.gcmNS], ciphertext[s.gcmNS:], additionalData)
-	if err != nil {
-		return nil, err
-	}
-	plaintext, err := aesgcm.Open(nil, nonce, ciphertext, additionalData)
-	if err != nil {
-		return nil, err
-	}
-	return plaintext, nil
 }

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)
 	}