aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--cmd/clef/extapi_changelog.md7
-rw-r--r--cmd/clef/main.go10
-rw-r--r--rpc/http.go6
-rw-r--r--signer/core/api.go120
-rw-r--r--signer/core/api_test.go82
-rw-r--r--signer/core/auditlog.go29
-rw-r--r--signer/core/cliui.go33
-rw-r--r--signer/core/types.go31
-rw-r--r--signer/core/validation.go28
-rw-r--r--signer/core/validation_test.go26
-rw-r--r--signer/storage/aes_gcm_storage.go15
-rw-r--r--signer/storage/aes_gcm_storage_test.go53
12 files changed, 307 insertions, 133 deletions
diff --git a/cmd/clef/extapi_changelog.md b/cmd/clef/extapi_changelog.md
index 2014e90ae..6c2c3e819 100644
--- a/cmd/clef/extapi_changelog.md
+++ b/cmd/clef/extapi_changelog.md
@@ -1,6 +1,13 @@
### Changelog for external API
+#### 4.0.0
+* The external `account_Ecrecover`-method was removed.
+* The external `account_Import`-method was removed.
+
+#### 3.0.0
+
+* The external `account_List`-method was changed to not expose `url`, which contained info about the local filesystem. It now returns only a list of addresses.
#### 2.0.0
diff --git a/cmd/clef/main.go b/cmd/clef/main.go
index f363a86f2..c060285be 100644
--- a/cmd/clef/main.go
+++ b/cmd/clef/main.go
@@ -48,7 +48,7 @@ import (
)
// ExternalAPIVersion -- see extapi_changelog.md
-const ExternalAPIVersion = "2.0.0"
+const ExternalAPIVersion = "3.0.0"
// InternalAPIVersion -- see intapi_changelog.md
const InternalAPIVersion = "2.0.0"
@@ -70,6 +70,10 @@ var (
Value: 4,
Usage: "log level to emit to the screen",
}
+ advancedMode = cli.BoolFlag{
+ Name: "advanced",
+ Usage: "If enabled, issues warnings instead of rejections for suspicious requests. Default off",
+ }
keystoreFlag = cli.StringFlag{
Name: "keystore",
Value: filepath.Join(node.DefaultDataDir(), "keystore"),
@@ -191,6 +195,7 @@ func init() {
ruleFlag,
stdiouiFlag,
testFlag,
+ advancedMode,
}
app.Action = signer
app.Commands = []cli.Command{initCommand, attestCommand, addCredentialCommand}
@@ -384,7 +389,8 @@ func signer(c *cli.Context) error {
c.String(keystoreFlag.Name),
c.Bool(utils.NoUSBFlag.Name),
ui, db,
- c.Bool(utils.LightKDFFlag.Name))
+ c.Bool(utils.LightKDFFlag.Name),
+ c.Bool(advancedMode.Name))
api = apiImpl
diff --git a/rpc/http.go b/rpc/http.go
index 9e4f2b261..af79858e2 100644
--- a/rpc/http.go
+++ b/rpc/http.go
@@ -238,6 +238,12 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
ctx = context.WithValue(ctx, "remote", r.RemoteAddr)
ctx = context.WithValue(ctx, "scheme", r.Proto)
ctx = context.WithValue(ctx, "local", r.Host)
+ if ua := r.Header.Get("User-Agent"); ua != "" {
+ ctx = context.WithValue(ctx, "User-Agent", ua)
+ }
+ if origin := r.Header.Get("Origin"); origin != "" {
+ ctx = context.WithValue(ctx, "Origin", origin)
+ }
body := io.LimitReader(r.Body, maxRequestContentLength)
codec := NewJSONCodec(&httpReadWriteNopCloser{body, w})
diff --git a/signer/core/api.go b/signer/core/api.go
index 4f390c1fd..1da86991e 100644
--- a/signer/core/api.go
+++ b/signer/core/api.go
@@ -39,19 +39,19 @@ import (
// ExternalAPI defines the external API through which signing requests are made.
type ExternalAPI interface {
// List available accounts
- List(ctx context.Context) (Accounts, error)
+ List(ctx context.Context) ([]common.Address, error)
// New request to create a new account
New(ctx context.Context) (accounts.Account, error)
// SignTransaction request to sign the specified transaction
SignTransaction(ctx context.Context, args SendTxArgs, methodSelector *string) (*ethapi.SignTransactionResult, error)
// Sign - request to sign the given data (plus prefix)
Sign(ctx context.Context, addr common.MixedcaseAddress, data hexutil.Bytes) (hexutil.Bytes, error)
- // EcRecover - request to perform ecrecover
- EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error)
// Export - request to export an account
Export(ctx context.Context, addr common.Address) (json.RawMessage, error)
// Import - request to import an account
- Import(ctx context.Context, keyJSON json.RawMessage) (Account, error)
+ // Should be moved to Internal API, in next phase when we have
+ // bi-directional communication
+ //Import(ctx context.Context, keyJSON json.RawMessage) (Account, error)
}
// SignerUI specifies what method a UI needs to implement to be able to be used as a UI for the signer
@@ -83,22 +83,25 @@ type SignerUI interface {
// SignerAPI defines the actual implementation of ExternalAPI
type SignerAPI struct {
- chainID *big.Int
- am *accounts.Manager
- UI SignerUI
- validator *Validator
+ chainID *big.Int
+ am *accounts.Manager
+ UI SignerUI
+ validator *Validator
+ rejectMode bool
}
// Metadata about a request
type Metadata struct {
- Remote string `json:"remote"`
- Local string `json:"local"`
- Scheme string `json:"scheme"`
+ Remote string `json:"remote"`
+ Local string `json:"local"`
+ Scheme string `json:"scheme"`
+ UserAgent string `json:"User-Agent"`
+ Origin string `json:"Origin"`
}
// MetadataFromContext extracts Metadata from a given context.Context
func MetadataFromContext(ctx context.Context) Metadata {
- m := Metadata{"NA", "NA", "NA"} // batman
+ m := Metadata{"NA", "NA", "NA", "", ""} // batman
if v := ctx.Value("remote"); v != nil {
m.Remote = v.(string)
@@ -109,6 +112,12 @@ func MetadataFromContext(ctx context.Context) Metadata {
if v := ctx.Value("local"); v != nil {
m.Local = v.(string)
}
+ if v := ctx.Value("Origin"); v != nil {
+ m.Origin = v.(string)
+ }
+ if v := ctx.Value("User-Agent"); v != nil {
+ m.UserAgent = v.(string)
+ }
return m
}
@@ -194,7 +203,7 @@ var ErrRequestDenied = errors.New("Request denied")
// key that is generated when a new Account is created.
// noUSB disables USB support that is required to support hardware devices such as
// ledger and trezor.
-func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abidb *AbiDb, lightKDF bool) *SignerAPI {
+func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abidb *AbiDb, lightKDF bool, advancedMode bool) *SignerAPI {
var (
backends []accounts.Backend
n, p = keystore.StandardScryptN, keystore.StandardScryptP
@@ -222,12 +231,15 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi
log.Debug("Trezor support enabled")
}
}
- return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, NewValidator(abidb)}
+ if advancedMode {
+ log.Info("Clef is in advanced mode: will warn instead of reject")
+ }
+ return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, NewValidator(abidb), !advancedMode}
}
// List returns the set of wallet this signer manages. Each wallet can contain
// multiple accounts.
-func (api *SignerAPI) List(ctx context.Context) (Accounts, error) {
+func (api *SignerAPI) List(ctx context.Context) ([]common.Address, error) {
var accs []Account
for _, wallet := range api.am.Wallets() {
for _, acc := range wallet.Accounts() {
@@ -243,7 +255,13 @@ func (api *SignerAPI) List(ctx context.Context) (Accounts, error) {
return nil, ErrRequestDenied
}
- return result.Accounts, nil
+
+ addresses := make([]common.Address, 0)
+ for _, acc := range result.Accounts {
+ addresses = append(addresses, acc.Address)
+ }
+
+ return addresses, nil
}
// New creates a new password protected Account. The private key is protected with
@@ -254,15 +272,28 @@ func (api *SignerAPI) New(ctx context.Context) (accounts.Account, error) {
if len(be) == 0 {
return accounts.Account{}, errors.New("password based accounts not supported")
}
- resp, err := api.UI.ApproveNewAccount(&NewAccountRequest{MetadataFromContext(ctx)})
-
- if err != nil {
- return accounts.Account{}, err
- }
- if !resp.Approved {
- return accounts.Account{}, ErrRequestDenied
+ var (
+ resp NewAccountResponse
+ err error
+ )
+ // Three retries to get a valid password
+ for i := 0; i < 3; i++ {
+ resp, err = api.UI.ApproveNewAccount(&NewAccountRequest{MetadataFromContext(ctx)})
+ if err != nil {
+ return accounts.Account{}, err
+ }
+ if !resp.Approved {
+ return accounts.Account{}, ErrRequestDenied
+ }
+ if pwErr := ValidatePasswordFormat(resp.Password); pwErr != nil {
+ api.UI.ShowError(fmt.Sprintf("Account creation attempt #%d failed due to password requirements: %v", (i + 1), pwErr))
+ } else {
+ // No error
+ return be[0].(*keystore.KeyStore).NewAccount(resp.Password)
+ }
}
- return be[0].(*keystore.KeyStore).NewAccount(resp.Password)
+ // Otherwise fail, with generic error message
+ return accounts.Account{}, errors.New("account creation failed")
}
// logDiff logs the difference between the incoming (original) transaction and the one returned from the signer.
@@ -294,10 +325,10 @@ func logDiff(original *SignTxRequest, new *SignTxResponse) bool {
d0s := ""
d1s := ""
if d0 != nil {
- d0s = common.ToHex(*d0)
+ d0s = hexutil.Encode(*d0)
}
if d1 != nil {
- d1s = common.ToHex(*d1)
+ d1s = hexutil.Encode(*d1)
}
if d1s != d0s {
modified = true
@@ -321,6 +352,12 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth
if err != nil {
return nil, err
}
+ // If we are in 'rejectMode', then reject rather than show the user warnings
+ if api.rejectMode {
+ if err := msgs.getWarnings(); err != nil {
+ return nil, err
+ }
+ }
req := SignTxRequest{
Transaction: args,
@@ -404,32 +441,6 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, da
return signature, nil
}
-// EcRecover 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
-// the address of:
-// hash = keccak256("\x19Ethereum Signed Message:\n"${message length}${message})
-// addr = ecrecover(hash, signature)
-//
-// Note, the signature must conform to the secp256k1 curve R, S and V values, where
-// the V value must be 27 or 28 for legacy reasons.
-//
-// https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecover
-func (api *SignerAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error) {
- if len(sig) != 65 {
- return common.Address{}, fmt.Errorf("signature must be 65 bytes long")
- }
- if sig[64] != 27 && sig[64] != 28 {
- return common.Address{}, fmt.Errorf("invalid Ethereum signature (V is not 27 or 28)")
- }
- sig[64] -= 27 // Transform yellow paper V from 27/28 to 0/1
- hash, _ := SignHash(data)
- rpk, err := crypto.SigToPub(hash, sig)
- if err != nil {
- return common.Address{}, err
- }
- return crypto.PubkeyToAddress(*rpk), nil
-}
-
// SignHash is a helper function that calculates a hash for the given message that can be
// safely used to calculate a signature from.
//
@@ -466,6 +477,11 @@ func (api *SignerAPI) Export(ctx context.Context, addr common.Address) (json.Raw
// Import tries to import the given keyJSON in the local keystore. The keyJSON data is expected to be
// in web3 keystore format. It will decrypt the keyJSON with the given passphrase and on successful
// decryption it will encrypt the key with the given newPassphrase and store it in the keystore.
+// OBS! This method is removed from the public API. It should not be exposed on the external API
+// for a couple of reasons:
+// 1. Even though it is encrypted, it should still be seen as sensitive data
+// 2. It can be used to DoS clef, by using malicious data with e.g. extreme large
+// values for the kdfparams.
func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage) (Account, error) {
be := api.am.Backends(keystore.KeyStoreType)
diff --git a/signer/core/api_test.go b/signer/core/api_test.go
index 50ad02198..70aa9aa94 100644
--- a/signer/core/api_test.go
+++ b/signer/core/api_test.go
@@ -45,7 +45,7 @@ func (ui *HeadlessUI) OnSignerStartup(info StartupInfo) {
}
func (ui *HeadlessUI) OnApprovedTx(tx ethapi.SignTransactionResult) {
- fmt.Printf("OnApproved called")
+ fmt.Printf("OnApproved()\n")
}
func (ui *HeadlessUI) ApproveTx(request *SignTxRequest) (SignTxResponse, error) {
@@ -62,26 +62,27 @@ func (ui *HeadlessUI) ApproveTx(request *SignTxRequest) (SignTxResponse, error)
return SignTxResponse{request.Transaction, false, ""}, nil
}
}
+
func (ui *HeadlessUI) ApproveSignData(request *SignDataRequest) (SignDataResponse, error) {
if "Y" == <-ui.controller {
return SignDataResponse{true, <-ui.controller}, nil
}
return SignDataResponse{false, ""}, nil
}
-func (ui *HeadlessUI) ApproveExport(request *ExportRequest) (ExportResponse, error) {
+func (ui *HeadlessUI) ApproveExport(request *ExportRequest) (ExportResponse, error) {
return ExportResponse{<-ui.controller == "Y"}, nil
}
-func (ui *HeadlessUI) ApproveImport(request *ImportRequest) (ImportResponse, error) {
+func (ui *HeadlessUI) ApproveImport(request *ImportRequest) (ImportResponse, error) {
if "Y" == <-ui.controller {
return ImportResponse{true, <-ui.controller, <-ui.controller}, nil
}
return ImportResponse{false, "", ""}, nil
}
-func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error) {
+func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error) {
switch <-ui.controller {
case "A":
return ListResponse{request.Accounts}, nil
@@ -93,20 +94,22 @@ func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error)
return ListResponse{nil}, nil
}
}
-func (ui *HeadlessUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) {
+func (ui *HeadlessUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) {
if "Y" == <-ui.controller {
return NewAccountResponse{true, <-ui.controller}, nil
}
return NewAccountResponse{false, ""}, nil
}
+
func (ui *HeadlessUI) ShowError(message string) {
//stdout is used by communication
- fmt.Fprint(os.Stderr, message)
+ fmt.Fprintln(os.Stderr, message)
}
+
func (ui *HeadlessUI) ShowInfo(message string) {
//stdout is used by communication
- fmt.Fprint(os.Stderr, message)
+ fmt.Fprintln(os.Stderr, message)
}
func tmpDirName(t *testing.T) string {
@@ -123,7 +126,7 @@ func tmpDirName(t *testing.T) string {
func setup(t *testing.T) (*SignerAPI, chan string) {
- controller := make(chan string, 10)
+ controller := make(chan string, 20)
db, err := NewAbiDBFromFile("../../cmd/clef/4byte.json")
if err != nil {
@@ -137,14 +140,14 @@ func setup(t *testing.T) (*SignerAPI, chan string) {
true,
ui,
db,
- true)
+ true, true)
)
return api, controller
}
func createAccount(control chan string, api *SignerAPI, t *testing.T) {
control <- "Y"
- control <- "apassword"
+ control <- "a_long_password"
_, err := api.New(context.Background())
if err != nil {
t.Fatal(err)
@@ -152,6 +155,25 @@ func createAccount(control chan string, api *SignerAPI, t *testing.T) {
// Some time to allow changes to propagate
time.Sleep(250 * time.Millisecond)
}
+
+func failCreateAccountWithPassword(control chan string, api *SignerAPI, password string, t *testing.T) {
+
+ control <- "Y"
+ control <- password
+ control <- "Y"
+ control <- password
+ control <- "Y"
+ control <- password
+
+ acc, err := api.New(context.Background())
+ if err == nil {
+ t.Fatal("Should have returned an error")
+ }
+ if acc.Address != (common.Address{}) {
+ t.Fatal("Empty address should be returned")
+ }
+}
+
func failCreateAccount(control chan string, api *SignerAPI, t *testing.T) {
control <- "N"
acc, err := api.New(context.Background())
@@ -162,7 +184,8 @@ func failCreateAccount(control chan string, api *SignerAPI, t *testing.T) {
t.Fatal("Empty address should be returned")
}
}
-func list(control chan string, api *SignerAPI, t *testing.T) []Account {
+
+func list(control chan string, api *SignerAPI, t *testing.T) []common.Address {
control <- "A"
list, err := api.List(context.Background())
if err != nil {
@@ -172,7 +195,6 @@ func list(control chan string, api *SignerAPI, t *testing.T) []Account {
}
func TestNewAcc(t *testing.T) {
-
api, control := setup(t)
verifyNum := func(num int) {
if list := list(control, api, t); len(list) != num {
@@ -188,6 +210,13 @@ func TestNewAcc(t *testing.T) {
failCreateAccount(control, api, t)
createAccount(control, api, t)
failCreateAccount(control, api, t)
+
+ verifyNum(4)
+
+ // Fail to create this, due to bad password
+ failCreateAccountWithPassword(control, api, "short", t)
+ failCreateAccountWithPassword(control, api, "longerbutbad\rfoo", t)
+
verifyNum(4)
// Testing listing:
@@ -212,7 +241,6 @@ func TestNewAcc(t *testing.T) {
}
func TestSignData(t *testing.T) {
-
api, control := setup(t)
//Create two accounts
createAccount(control, api, t)
@@ -222,7 +250,7 @@ func TestSignData(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- a := common.NewMixedcaseAddress(list[0].Address)
+ a := common.NewMixedcaseAddress(list[0])
control <- "Y"
control <- "wrongpassword"
@@ -233,7 +261,6 @@ func TestSignData(t *testing.T) {
if err != keystore.ErrDecrypt {
t.Errorf("Expected ErrLocked! %v", err)
}
-
control <- "No way"
h, err = api.Sign(context.Background(), a, []byte("EHLO world"))
if h != nil {
@@ -242,11 +269,9 @@ func TestSignData(t *testing.T) {
if err != ErrRequestDenied {
t.Errorf("Expected ErrRequestDenied! %v", err)
}
-
control <- "Y"
- control <- "apassword"
+ control <- "a_long_password"
h, err = api.Sign(context.Background(), a, []byte("EHLO world"))
-
if err != nil {
t.Fatal(err)
}
@@ -273,9 +298,8 @@ func mkTestTx(from common.MixedcaseAddress) SendTxArgs {
}
func TestSignTx(t *testing.T) {
-
var (
- list Accounts
+ list []common.Address
res, res2 *ethapi.SignTransactionResult
err error
)
@@ -287,7 +311,7 @@ func TestSignTx(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- a := common.NewMixedcaseAddress(list[0].Address)
+ a := common.NewMixedcaseAddress(list[0])
methodSig := "test(uint)"
tx := mkTestTx(a)
@@ -301,7 +325,6 @@ func TestSignTx(t *testing.T) {
if err != keystore.ErrDecrypt {
t.Errorf("Expected ErrLocked! %v", err)
}
-
control <- "No way"
res, err = api.SignTransaction(context.Background(), tx, &methodSig)
if res != nil {
@@ -310,9 +333,8 @@ func TestSignTx(t *testing.T) {
if err != ErrRequestDenied {
t.Errorf("Expected ErrRequestDenied! %v", err)
}
-
control <- "Y"
- control <- "apassword"
+ control <- "a_long_password"
res, err = api.SignTransaction(context.Background(), tx, &methodSig)
if err != nil {
@@ -320,12 +342,13 @@ func TestSignTx(t *testing.T) {
}
parsedTx := &types.Transaction{}
rlp.Decode(bytes.NewReader(res.Raw), parsedTx)
+
//The tx should NOT be modified by the UI
if parsedTx.Value().Cmp(tx.Value.ToInt()) != 0 {
t.Errorf("Expected value to be unchanged, expected %v got %v", tx.Value, parsedTx.Value())
}
control <- "Y"
- control <- "apassword"
+ control <- "a_long_password"
res2, err = api.SignTransaction(context.Background(), tx, &methodSig)
if err != nil {
@@ -337,20 +360,19 @@ func TestSignTx(t *testing.T) {
//The tx is modified by the UI
control <- "M"
- control <- "apassword"
+ control <- "a_long_password"
res2, err = api.SignTransaction(context.Background(), tx, &methodSig)
if err != nil {
t.Fatal(err)
}
-
parsedTx2 := &types.Transaction{}
rlp.Decode(bytes.NewReader(res.Raw), parsedTx2)
+
//The tx should be modified by the UI
if parsedTx2.Value().Cmp(tx.Value.ToInt()) != 0 {
t.Errorf("Expected value to be unchanged, got %v", parsedTx.Value())
}
-
if bytes.Equal(res.Raw, res2.Raw) {
t.Error("Expected tx to be modified by UI")
}
@@ -372,9 +394,9 @@ func TestAsyncronousResponses(t *testing.T){
control <- "W" //wait
control <- "Y" //
- control <- "apassword"
+ control <- "a_long_password"
control <- "Y" //
- control <- "apassword"
+ control <- "a_long_password"
var err error
diff --git a/signer/core/auditlog.go b/signer/core/auditlog.go
index d0ba733d2..1f9c90918 100644
--- a/signer/core/auditlog.go
+++ b/signer/core/auditlog.go
@@ -33,11 +33,10 @@ type AuditLogger struct {
api ExternalAPI
}
-func (l *AuditLogger) List(ctx context.Context) (Accounts, error) {
+func (l *AuditLogger) List(ctx context.Context) ([]common.Address, error) {
l.log.Info("List", "type", "request", "metadata", MetadataFromContext(ctx).String())
res, e := l.api.List(ctx)
-
- l.log.Info("List", "type", "response", "data", res.String())
+ l.log.Info("List", "type", "response", "data", res)
return res, e
}
@@ -72,14 +71,6 @@ func (l *AuditLogger) Sign(ctx context.Context, addr common.MixedcaseAddress, da
return b, e
}
-func (l *AuditLogger) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error) {
- l.log.Info("EcRecover", "type", "request", "metadata", MetadataFromContext(ctx).String(),
- "data", common.Bytes2Hex(data))
- a, e := l.api.EcRecover(ctx, data, sig)
- l.log.Info("EcRecover", "type", "response", "addr", a.String(), "error", e)
- return a, e
-}
-
func (l *AuditLogger) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) {
l.log.Info("Export", "type", "request", "metadata", MetadataFromContext(ctx).String(),
"addr", addr.Hex())
@@ -89,14 +80,14 @@ func (l *AuditLogger) Export(ctx context.Context, addr common.Address) (json.Raw
return j, e
}
-func (l *AuditLogger) Import(ctx context.Context, keyJSON json.RawMessage) (Account, error) {
- // Don't actually log the json contents
- l.log.Info("Import", "type", "request", "metadata", MetadataFromContext(ctx).String(),
- "keyJSON size", len(keyJSON))
- a, e := l.api.Import(ctx, keyJSON)
- l.log.Info("Import", "type", "response", "addr", a.String(), "error", e)
- return a, e
-}
+//func (l *AuditLogger) Import(ctx context.Context, keyJSON json.RawMessage) (Account, error) {
+// // Don't actually log the json contents
+// l.log.Info("Import", "type", "request", "metadata", MetadataFromContext(ctx).String(),
+// "keyJSON size", len(keyJSON))
+// a, e := l.api.Import(ctx, keyJSON)
+// l.log.Info("Import", "type", "response", "addr", a.String(), "error", e)
+// return a, e
+//}
func NewAuditLogger(path string, api ExternalAPI) (*AuditLogger, error) {
l := log.New("api", "signer")
diff --git a/signer/core/cliui.go b/signer/core/cliui.go
index 2f969669c..cc237612e 100644
--- a/signer/core/cliui.go
+++ b/signer/core/cliui.go
@@ -25,7 +25,7 @@ import (
"sync"
"github.com/davecgh/go-spew/spew"
- "github.com/ethereum/go-ethereum/common"
+ "github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/internal/ethapi"
"github.com/ethereum/go-ethereum/log"
"golang.org/x/crypto/ssh/terminal"
@@ -95,6 +95,8 @@ func (ui *CommandlineUI) confirm() bool {
func showMetadata(metadata Metadata) {
fmt.Printf("Request context:\n\t%v -> %v -> %v\n", metadata.Remote, metadata.Scheme, metadata.Local)
+ fmt.Printf("\nAdditional HTTP header data, provided by the external caller:\n")
+ fmt.Printf("\tUser-Agent: %v\n\tOrigin: %v\n", metadata.UserAgent, metadata.Origin)
}
// ApproveTx prompt the user for confirmation to request to sign Transaction
@@ -111,18 +113,22 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro
} else {
fmt.Printf("to: <contact creation>\n")
}
- fmt.Printf("from: %v\n", request.Transaction.From.String())
- fmt.Printf("value: %v wei\n", weival)
+ fmt.Printf("from: %v\n", request.Transaction.From.String())
+ fmt.Printf("value: %v wei\n", weival)
+ fmt.Printf("gas: %v (%v)\n", request.Transaction.Gas, uint64(request.Transaction.Gas))
+ fmt.Printf("gasprice: %v wei\n", request.Transaction.GasPrice.ToInt())
+ fmt.Printf("nonce: %v (%v)\n", request.Transaction.Nonce, uint64(request.Transaction.Nonce))
if request.Transaction.Data != nil {
d := *request.Transaction.Data
if len(d) > 0 {
- fmt.Printf("data: %v\n", common.Bytes2Hex(d))
+
+ fmt.Printf("data: %v\n", hexutil.Encode(d))
}
}
if request.Callinfo != nil {
fmt.Printf("\nTransaction validation:\n")
for _, m := range request.Callinfo {
- fmt.Printf(" * %s : %s", m.Typ, m.Message)
+ fmt.Printf(" * %s : %s\n", m.Typ, m.Message)
}
fmt.Println()
@@ -196,7 +202,9 @@ func (ui *CommandlineUI) ApproveListing(request *ListRequest) (ListResponse, err
fmt.Printf("A request has been made to list all accounts. \n")
fmt.Printf("You can select which accounts the caller can see\n")
for _, account := range request.Accounts {
- fmt.Printf("\t[x] %v\n", account.Address.Hex())
+ fmt.Printf(" [x] %v\n", account.Address.Hex())
+ fmt.Printf(" URL: %v\n", account.URL)
+ fmt.Printf(" Type: %v\n", account.Typ)
}
fmt.Printf("-------------------------------------------\n")
showMetadata(request.Meta)
@@ -212,10 +220,10 @@ func (ui *CommandlineUI) ApproveNewAccount(request *NewAccountRequest) (NewAccou
ui.mu.Lock()
defer ui.mu.Unlock()
- fmt.Printf("-------- New Account request--------------\n")
- fmt.Printf("A request has been made to create a new. \n")
- fmt.Printf("Approving this operation means that a new Account is created,\n")
- fmt.Printf("and the address show to the caller\n")
+ fmt.Printf("-------- New Account request--------------\n\n")
+ fmt.Printf("A request has been made to create a new account. \n")
+ fmt.Printf("Approving this operation means that a new account is created,\n")
+ fmt.Printf("and the address is returned to the external caller\n\n")
showMetadata(request.Meta)
if !ui.confirm() {
return NewAccountResponse{false, ""}, nil
@@ -225,8 +233,9 @@ func (ui *CommandlineUI) ApproveNewAccount(request *NewAccountRequest) (NewAccou
// ShowError displays error message to user
func (ui *CommandlineUI) ShowError(message string) {
-
- fmt.Printf("ERROR: %v\n", message)
+ fmt.Printf("-------- Error message from Clef-----------\n")
+ fmt.Println(message)
+ fmt.Printf("-------------------------------------------\n")
}
// ShowInfo displays info message to user
diff --git a/signer/core/types.go b/signer/core/types.go
index 2acc0a4f4..128055774 100644
--- a/signer/core/types.go
+++ b/signer/core/types.go
@@ -18,6 +18,7 @@ package core
import (
"encoding/json"
+ "fmt"
"strings"
"math/big"
@@ -60,6 +61,36 @@ type ValidationMessages struct {
Messages []ValidationInfo
}
+const (
+ WARN = "WARNING"
+ CRIT = "CRITICAL"
+ INFO = "Info"
+)
+
+func (vs *ValidationMessages) crit(msg string) {
+ vs.Messages = append(vs.Messages, ValidationInfo{CRIT, msg})
+}
+func (vs *ValidationMessages) warn(msg string) {
+ vs.Messages = append(vs.Messages, ValidationInfo{WARN, msg})
+}
+func (vs *ValidationMessages) info(msg string) {
+ vs.Messages = append(vs.Messages, ValidationInfo{INFO, msg})
+}
+
+/// getWarnings returns an error with all messages of type WARN of above, or nil if no warnings were present
+func (v *ValidationMessages) getWarnings() error {
+ var messages []string
+ for _, msg := range v.Messages {
+ if msg.Typ == WARN || msg.Typ == CRIT {
+ messages = append(messages, msg.Message)
+ }
+ }
+ if len(messages) > 0 {
+ return fmt.Errorf("Validation failed: %s", strings.Join(messages, ","))
+ }
+ return nil
+}
+
// SendTxArgs represents the arguments to submit a transaction
type SendTxArgs struct {
From common.MixedcaseAddress `json:"from"`
diff --git a/signer/core/validation.go b/signer/core/validation.go
index 288456df8..7c3ec4274 100644
--- a/signer/core/validation.go
+++ b/signer/core/validation.go
@@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"math/big"
+ "regexp"
"github.com/ethereum/go-ethereum/common"
)
@@ -30,16 +31,6 @@ import (
// - Transaction semantics validation
// The package provides warnings for typical pitfalls
-func (vs *ValidationMessages) crit(msg string) {
- vs.Messages = append(vs.Messages, ValidationInfo{"CRITICAL", msg})
-}
-func (vs *ValidationMessages) warn(msg string) {
- vs.Messages = append(vs.Messages, ValidationInfo{"WARNING", msg})
-}
-func (vs *ValidationMessages) info(msg string) {
- vs.Messages = append(vs.Messages, ValidationInfo{"Info", msg})
-}
-
type Validator struct {
db *AbiDb
}
@@ -72,6 +63,9 @@ func (v *Validator) validateCallData(msgs *ValidationMessages, data []byte, meth
msgs.warn("Tx contains data which is not valid ABI")
return
}
+ if arglen := len(data) - 4; arglen%32 != 0 {
+ msgs.warn(fmt.Sprintf("Not ABI-encoded data; length should be a multiple of 32 (was %d)", arglen))
+ }
var (
info *decodedCallData
err error
@@ -161,3 +155,17 @@ func (v *Validator) ValidateTransaction(txArgs *SendTxArgs, methodSelector *stri
msgs := &ValidationMessages{}
return msgs, v.validate(msgs, txArgs, methodSelector)
}
+
+var Printable7BitAscii = regexp.MustCompile("^[A-Za-z0-9!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~ ]+$")
+
+// ValidatePasswordFormat returns an error if the password is too short, or consists of characters
+// outside the range of the printable 7bit ascii set
+func ValidatePasswordFormat(password string) error {
+ if len(password) < 10 {
+ return errors.New("password too short (<10 characters)")
+ }
+ if !Printable7BitAscii.MatchString(password) {
+ return errors.New("password contains invalid characters - only 7bit printable ascii allowed")
+ }
+ return nil
+}
diff --git a/signer/core/validation_test.go b/signer/core/validation_test.go
index 2b33a8630..155b25e92 100644
--- a/signer/core/validation_test.go
+++ b/signer/core/validation_test.go
@@ -137,3 +137,29 @@ func TestValidator(t *testing.T) {
}
}
}
+
+func TestPasswordValidation(t *testing.T) {
+ testcases := []struct {
+ pw string
+ shouldFail bool
+ }{
+ {"test", true},
+ {"testtest\xbd\xb2\x3d\xbc\x20\xe2\x8c\x98", true},
+ {"placeOfInterest⌘", true},
+ {"password\nwith\nlinebreak", true},
+ {"password\twith\vtabs", true},
+ // Ok passwords
+ {"password WhichIsOk", false},
+ {"passwordOk!@#$%^&*()", false},
+ {"12301203123012301230123012", false},
+ }
+ for _, test := range testcases {
+ err := ValidatePasswordFormat(test.pw)
+ if err == nil && test.shouldFail {
+ t.Errorf("password '%v' should fail validation", test.pw)
+ } else if err != nil && !test.shouldFail {
+
+ t.Errorf("password '%v' shound not fail validation, but did: %v", test.pw, err)
+ }
+ }
+}
diff --git a/signer/storage/aes_gcm_storage.go b/signer/storage/aes_gcm_storage.go
index 399637a44..900831867 100644
--- a/signer/storage/aes_gcm_storage.go
+++ b/signer/storage/aes_gcm_storage.go
@@ -63,7 +63,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))
+ ciphertext, iv, err := encrypt(s.key, []byte(value), []byte(key))
if err != nil {
log.Warn("Failed to encrypt entry", "err", err)
return
@@ -90,7 +90,7 @@ func (s *AESEncryptedStorage) Get(key string) string {
log.Warn("Key does not exist", "key", key)
return ""
}
- entry, err := decrypt(s.key, encrypted.Iv, encrypted.CipherText)
+ entry, err := decrypt(s.key, encrypted.Iv, encrypted.CipherText, []byte(key))
if err != nil {
log.Warn("Failed to decrypt key", "key", key)
return ""
@@ -129,7 +129,10 @@ func (s *AESEncryptedStorage) writeEncryptedStorage(creds map[string]storedCrede
return nil
}
-func encrypt(key []byte, plaintext []byte) ([]byte, []byte, error) {
+// 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 eachother.
+func encrypt(key []byte, plaintext []byte, additionalData []byte) ([]byte, []byte, error) {
block, err := aes.NewCipher(key)
if err != nil {
return nil, nil, err
@@ -142,11 +145,11 @@ func encrypt(key []byte, plaintext []byte) ([]byte, []byte, error) {
if err != nil {
return nil, nil, err
}
- ciphertext := aesgcm.Seal(nil, nonce, plaintext, nil)
+ ciphertext := aesgcm.Seal(nil, nonce, plaintext, additionalData)
return ciphertext, nonce, nil
}
-func decrypt(key []byte, nonce []byte, ciphertext []byte) ([]byte, error) {
+func decrypt(key []byte, nonce []byte, ciphertext []byte, additionalData []byte) ([]byte, error) {
block, err := aes.NewCipher(key)
if err != nil {
return nil, err
@@ -155,7 +158,7 @@ func decrypt(key []byte, nonce []byte, ciphertext []byte) ([]byte, error) {
if err != nil {
return nil, err
}
- plaintext, err := aesgcm.Open(nil, nonce, ciphertext, nil)
+ plaintext, err := aesgcm.Open(nil, nonce, ciphertext, additionalData)
if err != nil {
return nil, err
}
diff --git a/signer/storage/aes_gcm_storage_test.go b/signer/storage/aes_gcm_storage_test.go
index 77804905a..a421a8449 100644
--- a/signer/storage/aes_gcm_storage_test.go
+++ b/signer/storage/aes_gcm_storage_test.go
@@ -18,6 +18,7 @@ package storage
import (
"bytes"
+ "encoding/json"
"fmt"
"io/ioutil"
"testing"
@@ -33,13 +34,13 @@ func TestEncryption(t *testing.T) {
key := []byte("AES256Key-32Characters1234567890")
plaintext := []byte("exampleplaintext")
- c, iv, err := encrypt(key, plaintext)
+ c, iv, err := encrypt(key, plaintext, nil)
if err != nil {
t.Fatal(err)
}
fmt.Printf("Ciphertext %x, nonce %x\n", c, iv)
- p, err := decrypt(key, iv, c)
+ p, err := decrypt(key, iv, c, nil)
if err != nil {
t.Fatal(err)
}
@@ -113,3 +114,51 @@ func TestEnd2End(t *testing.T) {
t.Errorf("Expected bazonk->foobar, got '%v'", v)
}
}
+
+func TestSwappedKeys(t *testing.T) {
+ // It should not be possible to swap the keys/values, so that
+ // K1:V1, K2:V2 can be swapped into K1:V2, K2:V1
+ log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(3), log.StreamHandler(colorable.NewColorableStderr(), log.TerminalFormat(true))))
+
+ d, err := ioutil.TempDir("", "eth-encrypted-storage-test")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ s1 := &AESEncryptedStorage{
+ filename: fmt.Sprintf("%v/vault.json", d),
+ key: []byte("AES256Key-32Characters1234567890"),
+ }
+ s1.Put("k1", "v1")
+ s1.Put("k2", "v2")
+ // Now make a modified copy
+
+ creds := make(map[string]storedCredential)
+ raw, err := ioutil.ReadFile(s1.filename)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = json.Unmarshal(raw, &creds); err != nil {
+ t.Fatal(err)
+ }
+ swap := func() {
+ // Turn it into K1:V2, K2:V2
+ v1, v2 := creds["k1"], creds["k2"]
+ creds["k2"], creds["k1"] = v1, v2
+ raw, err = json.Marshal(creds)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = ioutil.WriteFile(s1.filename, raw, 0600); err != nil {
+ t.Fatal(err)
+ }
+ }
+ swap()
+ if v := s1.Get("k1"); v != "" {
+ t.Errorf("swapped value should return empty")
+ }
+ swap()
+ if v := s1.Get("k1"); v != "v1" {
+ t.Errorf("double-swapped value should work fine")
+ }
+}
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-17 10:20:58 +0800