signer: implement audit logging

cmd/signer/abihelper_test.go

@@ -108,7 +108,7 @@ func TestReflect(t *testing.T) {
 		t.Fatalf("Nope, %v != %v", a, b)
 	}
 }
- */
+*/
 
 func TestCalldataDecoding(t *testing.T) {
 

cmd/signer/api.go

@@ -53,6 +53,29 @@ type Metadata struct {
 	Scheme string `json:"scheme"`
 }
 
+func MetadataFromContext(ctx context.Context) Metadata {
+	m := Metadata{"NA", "NA", "NA"}
+
+	if v := ctx.Value("remote"); v != nil {
+		m.Remote = v.(string)
+	}
+	if v := ctx.Value("scheme"); v != nil {
+		m.Scheme = v.(string)
+	}
+	if v := ctx.Value("local"); v != nil {
+		m.Local = v.(string)
+	}
+	return m
+}
+
+func (m Metadata) String() string {
+	s, err := json.Marshal(m)
+	if err == nil {
+		return string(s)
+	}
+	return err.Error()
+}
+
 // types for the requests/response types
 type (
 	// SignTxRequest contains info about a Transaction to sign
@@ -129,6 +152,16 @@ func (ew errorWrapper) String() string {
 	return fmt.Sprintf("%s\n%s", ew.msg, ew.err)
 }
 
+type ExternalAPI interface {
+	List(ctx context.Context) (Accounts, error)
+	New(ctx context.Context) (accounts.Account, error)
+	SignTransaction(ctx context.Context, from common.MixedcaseAddress, args TransactionArg, methodSelector *string) (hexutil.Bytes, error)
+	Sign(ctx context.Context, addr common.MixedcaseAddress, data hexutil.Bytes) (hexutil.Bytes, error)
+	EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error)
+	Export(ctx context.Context, addr common.Address) (json.RawMessage, error)
+	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
 type SignerUI interface {
@@ -188,21 +221,6 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi
 	return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, *abidb}
 }
 
-func metaData(ctx context.Context) Metadata {
-	m := Metadata{"NA", "NA", "NA"}
-
-	if v := ctx.Value("remote"); v != nil {
-		m.Remote = v.(string)
-	}
-	if v := ctx.Value("scheme"); v != nil {
-		m.Scheme = v.(string)
-	}
-	if v := ctx.Value("local"); v != nil {
-		m.Local = v.(string)
-	}
-	return m
-}
-
 // List returns the set of wallet this signer manages. Each wallet can contain
 // multiple accounts.
 func (api *SignerAPI) List(ctx context.Context) (Accounts, error) {
@@ -215,7 +233,7 @@ func (api *SignerAPI) List(ctx context.Context) (Accounts, error) {
 		}
 	}
 
-	result, err := api.ui.ApproveListing(&ListRequest{Accounts: accs, Meta: metaData(ctx)})
+	result, err := api.ui.ApproveListing(&ListRequest{Accounts: accs, Meta: MetadataFromContext(ctx)})
 	if err != nil {
 		return nil, err
 	}
@@ -234,7 +252,7 @@ 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{metaData(ctx)})
+	resp, err := api.ui.ApproveNewAccount(&NewAccountRequest{MetadataFromContext(ctx)})
 
 	if err != nil {
 		return accounts.Account{}, err
@@ -307,7 +325,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Mixedcase
 		result SignTxResponse
 	)
 
-	req := SignTxRequest{Transaction: args, From: from, Meta: metaData(ctx)}
+	req := SignTxRequest{Transaction: args, From: from, Meta: MetadataFromContext(ctx)}
 
 	data := args.Data
 	if len(data) > 3 {
@@ -383,7 +401,7 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, da
 
 	// We make the request prior to looking up if we actually have the account, to prevent
 	// account-enumeration via the API
-	req := &SignDataRequest{Address: addr, Rawdata: data, Message: msg, Hash: sighash, Meta: metaData(ctx)}
+	req := &SignDataRequest{Address: addr, Rawdata: data, Message: msg, Hash: sighash, Meta: MetadataFromContext(ctx)}
 	res, err := api.ui.ApproveSignData(req)
 
 	if err != nil {
@@ -454,7 +472,7 @@ func signHash(data []byte) ([]byte, string) {
 // Export returns encrypted private key associated with the given address in web3 keystore format.
 func (api *SignerAPI) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) {
 
-	res, err := api.ui.ApproveExport(&ExportRequest{Address: addr, Meta: metaData(ctx)})
+	res, err := api.ui.ApproveExport(&ExportRequest{Address: addr, Meta: MetadataFromContext(ctx)})
 
 	if err != nil {
 		return nil, err
@@ -487,7 +505,7 @@ func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage) (Acco
 		return Account{}, errors.New("password based accounts not supported")
 	}
 
-	res, err := api.ui.ApproveImport(&ImportRequest{Meta: metaData(ctx)})
+	res, err := api.ui.ApproveImport(&ImportRequest{Meta: MetadataFromContext(ctx)})
 
 	if err != nil {
 		return Account{}, err

cmd/signer/auditlog.go

@@ -1,27 +1,82 @@
 package main
 
 import (
-	"bufio"
-	"fmt"
-	"io"
-	"time"
+	"context"
 
-	"github.com/ethereum/go-ethereum/rpc"
+	"encoding/json"
+	"github.com/ethereum/go-ethereum/accounts"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/log"
 )
 
 type AuditLogger struct {
-	writer *bufio.Writer
+	log log.Logger
+	api ExternalAPI
 }
 
-func (l AuditLogger) Store(record *rpc.RPCInvocationRecord) {
-	l.writer.WriteString(fmt.Sprintf("%v\n%v\n", time.Now().Format(time.RFC3339), record.Method))
-	for i, arg := range record.Args {
-		l.writer.WriteString(fmt.Sprintf("\t%d: %v\n", i, arg))
+func (l *AuditLogger) List(ctx context.Context) (Accounts, error) {
+
+	l.log.Info("Called list", "interface", "http")
+	return l.api.List(ctx)
+}
+
+func (l *AuditLogger) New(ctx context.Context) (accounts.Account, error) {
+	return l.api.New(ctx)
+}
+
+func (l *AuditLogger) SignTransaction(ctx context.Context, from common.MixedcaseAddress, args TransactionArg, methodSelector *string) (hexutil.Bytes, error) {
+	l.log.Info("SignTransaction", "type", "request", "metadata", MetadataFromContext(ctx).String(),
+		"from", from.String(), "tx", args.String(),
+		"methodSelector", methodSelector)
+	b, e := l.api.SignTransaction(ctx, from, args, methodSelector)
+
+	l.log.Info("SignTransaction", "type", "response", "data", common.Bytes2Hex(b), "error", e)
+
+	return b, e
+}
+
+func (l *AuditLogger) Sign(ctx context.Context, addr common.MixedcaseAddress, data hexutil.Bytes) (hexutil.Bytes, error) {
+	l.log.Info("Sign", "type", "request", "metadata", MetadataFromContext(ctx).String(),
+		"addr", addr.String(), "data", common.Bytes2Hex(data))
+	b, e := l.api.Sign(ctx, addr, data)
+	l.log.Info("Sign", "type", "response", "data", common.Bytes2Hex(b), "error", e)
+	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())
+	j, e := l.api.Export(ctx, addr)
+	// In this case, we don't actually log the json-response, which may be extra sensitive
+	l.log.Info("Export", "type", "response", "json response size", len(j), "error", e)
+	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 NewAuditLogger(path string, api ExternalAPI) (*AuditLogger, error) {
+	l := log.New("api", "signer")
+	handler, err := log.FileHandler(path, log.LogfmtFormat())
+	if err != nil {
+		return nil, err
 	}
-	l.writer.WriteString(fmt.Sprintf("%v\n", record.Response))
-	l.writer.Flush()
-}
-
-func NewAuditLogger(writer io.Writer) *AuditLogger {
-	return &AuditLogger{bufio.NewWriter(writer)}
+	l.SetHandler(handler)
+	l.Info("Configured", "audit log", path)
+	return &AuditLogger{l, api}, nil
 }

cmd/signer/main.go

@@ -115,23 +115,26 @@ func main() {
 		log.Info("Loaded 4byte db", "signatures", db.Size(), "file", c.String("4bytedb"))
 
 		var (
+			api      ExternalAPI
+			listener net.Listener
 			server   = rpc.NewServer()
+		)
 
-			api = NewSignerAPI(
+		api_impl := NewSignerAPI(
 			c.Int64(utils.NetworkIdFlag.Name),
 			c.String("keystore"),
 			c.Bool(utils.NoUSBFlag.Name),
 			ui, db,
 			c.Bool(utils.LightKDFFlag.Name))
-			listener net.Listener
-		)
+
+		api = api_impl
+
 		// Audit logging
 		if logfile := c.String("auditlog"); logfile != "" {
-			f, err := os.OpenFile(logfile, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
+			api, err = NewAuditLogger(logfile, api_impl)
 			if err != nil {
-				utils.Fatalf("Could not open %v for audit logging", logfile)
+				utils.Fatalf(err.Error())
 			}
-			server.SetAuditLogger(NewAuditLogger(f))
 			log.Info("Audit logs configured", "file", logfile)
 		}
 		// register signer API with server
@@ -156,7 +159,7 @@ func main() {
 
 		if c.Bool("stdio-ui-test") {
 			log.Info("Performing UI test")
-			go testExternalUI(api)
+			go testExternalUI(api_impl)
 		}
 
 		rpc.NewHTTPServer(cors, server).Serve(listener)