From a51bea0cb95c81500ee8f1ecd722ce3d40acf21e Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 5 Feb 2019 13:18:53 +0100 Subject: [PATCH] accounts, signer: fix mimetypes, add interface to sign data with passphrase --- accounts/accounts.go | 29 ++++++++------ accounts/external/backend.go | 9 +++-- accounts/keystore/wallet.go | 32 ++++++++++------ accounts/usbwallet/wallet.go | 7 ++++ consensus/clique/clique.go | 2 +- signer/core/api.go | 50 ------------------------ signer/core/signed_data.go | 74 ++++++++++++++++++------------------ 7 files changed, 90 insertions(+), 113 deletions(-) diff --git a/accounts/accounts.go b/accounts/accounts.go index 11232b19a0..93ecf179b5 100644 --- a/accounts/accounts.go +++ b/accounts/accounts.go @@ -35,6 +35,13 @@ type Account struct { URL URL `json:"url"` // Optional resource locator within a backend } +const ( + MimetypeTextWithValidator = "text/validator" + MimetypeTypedData = "data/typed" + MimetypeClique = "application/x-clique-header" + MimetypeTextPlain = "text/plain" +) + // Wallet represents a software or hardware wallet that might contain one or more // accounts (derived from the same seed). type Wallet interface { @@ -101,6 +108,12 @@ type Wallet interface { // the account in a keystore). SignData(account Account, mimeType string, data []byte) ([]byte, error) + // SignDataWithPassphrase is identical to SignData, but also takes a password + // NOTE: there's an chance that an erroneous call might mistake the two strings, and + // supply password in the mimetype field, or vice versa. Thus, an implementation + // should never echo the mimetype or return the mimetype in the error-response + SignDataWithPassphrase(account Account, passphrase, mimeType string, data []byte) ([]byte, error) + // Signtext requests the wallet to sign the hash of a given piece of data, prefixed // by the Ethereum prefix scheme // It looks up the account specified either solely via its address contained within, @@ -114,6 +127,9 @@ type Wallet interface { // the account in a keystore). SignText(account Account, text []byte) ([]byte, error) + // SignTextWithPassphrase is identical to Signtext, but also takes a password + SignTextWithPassphrase(account Account, passphrase string, hash []byte) ([]byte, error) + // SignTx requests the wallet to sign the given transaction. // // It looks up the account specified either solely via its address contained within, @@ -127,18 +143,7 @@ type Wallet interface { // the account in a keystore). SignTx(account Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) - // SignTextWithPassphrase requests the wallet to sign the given text with the - // given passphrase as extra authentication information. - // - // It looks up the account specified either solely via its address contained within, - // or optionally with the aid of any location metadata from the embedded URL field. - SignTextWithPassphrase(account Account, passphrase string, hash []byte) ([]byte, error) - - // SignTxWithPassphrase requests the wallet to sign the given transaction, with the - // given passphrase as extra authentication information. - // - // It looks up the account specified either solely via its address contained within, - // or optionally with the aid of any location metadata from the embedded URL field. + // SignTxWithPassphrase is identical to SignTx, but also takes a password SignTxWithPassphrase(account Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) } diff --git a/accounts/external/backend.go b/accounts/external/backend.go index 35b9c276d1..3b8d50f1b6 100644 --- a/accounts/external/backend.go +++ b/accounts/external/backend.go @@ -184,11 +184,14 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio } func (api *ExternalSigner) SignTextWithPassphrase(account accounts.Account, passphrase string, text []byte) ([]byte, error) { - return []byte{}, fmt.Errorf("operation not supported on external signers") + return []byte{}, fmt.Errorf("passphrase-operations not supported on external signers") } func (api *ExternalSigner) SignTxWithPassphrase(account accounts.Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) { - return nil, fmt.Errorf("operation not supported on external signers") + return nil, fmt.Errorf("passphrase-operations not supported on external signers") +} +func (api *ExternalSigner) SignDataWithPassphrase(account accounts.Account, passphrase, mimeType string, data []byte) ([]byte, error) { + return nil, fmt.Errorf("passphrase-operations not supported on external signers") } func (api *ExternalSigner) listAccounts() ([]common.Address, error) { @@ -201,7 +204,7 @@ func (api *ExternalSigner) listAccounts() ([]common.Address, error) { func (api *ExternalSigner) signCliqueBlock(a common.Address, rlpBlock hexutil.Bytes) (hexutil.Bytes, error) { var sig hexutil.Bytes - if err := api.client.Call(&sig, "account_signData", "application/clique", a, rlpBlock); err != nil { + if err := api.client.Call(&sig, "account_signData", core.ApplicationClique.Mime, a, rlpBlock); err != nil { return nil, err } if sig[64] != 27 && sig[64] != 28 { diff --git a/accounts/keystore/wallet.go b/accounts/keystore/wallet.go index 0490f39ff5..632620ead9 100644 --- a/accounts/keystore/wallet.go +++ b/accounts/keystore/wallet.go @@ -97,10 +97,31 @@ func (w *keystoreWallet) SignData(account accounts.Account, mimeType string, dat return w.signHash(account, crypto.Keccak256(data)) } +// SignDataWithPassphrase signs keccak256(data). The mimetype parameter describes the type of data being signed +func (w *keystoreWallet) SignDataWithPassphrase(account accounts.Account, passphrase, mimeType string, data []byte) ([]byte, error) { + // Make sure the requested account is contained within + if !w.Contains(account) { + return nil, accounts.ErrUnknownAccount + } + // Account seems valid, request the keystore to sign + return w.keystore.SignHashWithPassphrase(account, passphrase, crypto.Keccak256(data)) +} + func (w *keystoreWallet) SignText(account accounts.Account, text []byte) ([]byte, error) { return w.signHash(account, accounts.TextHash(text)) } +// SignHashWithPassphrase implements accounts.Wallet, attempting to sign the +// given hash with the given account using passphrase as extra authentication. +func (w *keystoreWallet) SignTextWithPassphrase(account accounts.Account, passphrase string, text []byte) ([]byte, error) { + // Make sure the requested account is contained within + if !w.Contains(account) { + return nil, accounts.ErrUnknownAccount + } + // Account seems valid, request the keystore to sign + return w.keystore.SignHashWithPassphrase(account, passphrase, accounts.TextHash(text)) +} + // SignTx implements accounts.Wallet, attempting to sign the given transaction // with the given account. If the wallet does not wrap this particular account, // an error is returned to avoid account leakage (even though in theory we may @@ -114,17 +135,6 @@ func (w *keystoreWallet) SignTx(account accounts.Account, tx *types.Transaction, return w.keystore.SignTx(account, tx, chainID) } -// SignHashWithPassphrase implements accounts.Wallet, attempting to sign the -// given hash with the given account using passphrase as extra authentication. -func (w *keystoreWallet) SignTextWithPassphrase(account accounts.Account, passphrase string, text []byte) ([]byte, error) { - // Make sure the requested account is contained within - if !w.Contains(account) { - return nil, accounts.ErrUnknownAccount - } - // Account seems valid, request the keystore to sign - return w.keystore.SignHashWithPassphrase(account, passphrase, accounts.TextHash(text)) -} - // SignTxWithPassphrase implements accounts.Wallet, attempting to sign the given // transaction with the given account using passphrase as extra authentication. func (w *keystoreWallet) SignTxWithPassphrase(account accounts.Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) { diff --git a/accounts/usbwallet/wallet.go b/accounts/usbwallet/wallet.go index a99dcd0f5a..feab505c9b 100644 --- a/accounts/usbwallet/wallet.go +++ b/accounts/usbwallet/wallet.go @@ -507,6 +507,13 @@ func (w *wallet) SignData(account accounts.Account, mimeType string, data []byte return w.signHash(account, crypto.Keccak256(data)) } +// SignDataWithPassphrase implements accounts.Wallet, attempting to sign the given +// data with the given account using passphrase as extra authentication. +// Since USB wallets don't rely on passphrases, these are silently ignored. +func (w *wallet) SignDataWithPassphrase(account accounts.Account, passphrase, mimeType string, data []byte) ([]byte, error) { + return w.SignData(account, mimeType, data) +} + func (w *wallet) SignText(account accounts.Account, text []byte) ([]byte, error) { return w.signHash(account, accounts.TextHash(text)) } diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index c0f78ce655..967a843dec 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -616,7 +616,7 @@ func (c *Clique) Seal(chain consensus.ChainReader, block *types.Block, results c log.Trace("Out-of-turn signing requested", "wiggle", common.PrettyDuration(wiggle)) } // Sign all the things! - sighash, err := signFn(accounts.Account{Address: signer}, "application/x-clique-header", CliqueRLP(header)) + sighash, err := signFn(accounts.Account{Address: signer}, accounts.MimetypeClique, CliqueRLP(header)) if err != nil { return err } diff --git a/signer/core/api.go b/signer/core/api.go index f9937abac4..754aab11cb 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -521,56 +521,6 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth } -// Sign calculates an Ethereum ECDSA signature for: -// keccack256("\x19Ethereum Signed Message:\n" + len(message) + message)) -// -// Note, the produced signature conforms to the secp256k1 curve R, S and V values, -// where the V value will be 27 or 28 for legacy reasons. -// -// The key used to calculate the signature is decrypted with the given password. -// -// https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_sign -func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, data hexutil.Bytes) (hexutil.Bytes, error) { - sighash, msg := SignHash(data) - // 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: MetadataFromContext(ctx)} - res, err := api.UI.ApproveSignData(req) - - if err != nil { - return nil, err - } - if !res.Approved { - return nil, ErrRequestDenied - } - // Look up the wallet containing the requested signer - account := accounts.Account{Address: addr.Address()} - wallet, err := api.am.Find(account) - if err != nil { - return nil, err - } - // Assemble sign the data with the wallet - signature, err := wallet.SignTextWithPassphrase(account, res.Password, data) - if err != nil { - api.UI.ShowError(err.Error()) - return nil, err - } - signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper - return signature, nil -} - -// SignHash is a helper function that calculates a hash for the given message that can be -// safely used to calculate a signature from. -// -// The hash is calculated as -// keccak256("\x19Ethereum Signed Message:\n"${message length}${message}). -// -// This gives context to the signed message and prevents signing of transactions. -func SignHash(data []byte) ([]byte, string) { - msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data) - return crypto.Keccak256([]byte(msg)), msg -} - // 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: MetadataFromContext(ctx)}) diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 3298d4b71c..92bf9db9d4 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -21,6 +21,7 @@ import ( "context" "errors" "fmt" + "github.com/ethereum/go-ethereum/consensus/clique" "math/big" "mime" "regexp" @@ -37,7 +38,6 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rlp" - "golang.org/x/crypto/sha3" ) type SigFormat struct { @@ -47,19 +47,19 @@ type SigFormat struct { var ( TextValidator = SigFormat{ - "text/validator", + accounts.MimetypeTextWithValidator, 0x00, } DataTyped = SigFormat{ - "data/typed", + accounts.MimetypeTypedData, 0x01, } ApplicationClique = SigFormat{ - "application/clique", + accounts.MimetypeClique, 0x02, } TextPlain = SigFormat{ - "text/plain", + accounts.MimetypeTextPlain, 0x45, } ) @@ -118,13 +118,11 @@ type TypedDataDomain struct { var typedDataReferenceTypeRegexp = regexp.MustCompile(`^[A-Z](\w*)(\[\])?$`) -// Sign receives a request and produces a signature +// sign receives a request and produces a signature // Note, the produced signature conforms to the secp256k1 curve R, S and V values, // where the V value will be 27 or 28 for legacy reasons. -func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, req *SignDataRequest) (hexutil.Bytes, error) { - req.Address = addr - req.Meta = MetadataFromContext(ctx) +func (api *SignerAPI) sign(ctx context.Context, addr common.MixedcaseAddress, req *SignDataRequest) (hexutil.Bytes, error) { // We make the request prior to looking up if we actually have the account, to prevent // account-enumeration via the API @@ -142,7 +140,7 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, re return nil, err } // Sign the data with the wallet - signature, err := wallet.SignHashWithPassphrase(account, res.Password, req.Hash) + signature, err := wallet.SignDataWithPassphrase(account, res.Password, req.ContentType, req.Hash) if err != nil { return nil, err } @@ -155,12 +153,12 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, re // // Different types of validation occur. func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (hexutil.Bytes, error) { - var req, err = api.determineSignatureFormat(contentType, addr, data) + var req, err = api.determineSignatureFormat(ctx, contentType, addr, data) if err != nil { return nil, err } - signature, err := api.Sign(ctx, addr, req) + signature, err := api.sign(ctx, addr, req) if err != nil { api.UI.ShowError(err.Error()) return nil, err @@ -174,8 +172,12 @@ func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr com // resides in the 'params' returned as the second returnvalue from mime.ParseMediaType // charset, ok := params["charset"] // As it is now, we accept any charset and just treat it as 'raw'. -func (api *SignerAPI) determineSignatureFormat(contentType string, addr common.MixedcaseAddress, data interface{}) (*SignDataRequest, error) { +// This method returns the mimetype for signing along with the request +func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (*SignDataRequest, error) { var req *SignDataRequest + req.Address = addr + req.Meta = MetadataFromContext(ctx) + mediaType, _, err := mime.ParseMediaType(contentType) if err != nil { return nil, err @@ -234,7 +236,6 @@ func (api *SignerAPI) determineSignatureFormat(contentType string, addr common.M Value: msg, }, } - req = &SignDataRequest{ContentType: mediaType, Rawdata: plainData, Message: message, Hash: sighash} } return req, nil @@ -258,29 +259,30 @@ func SignTextValidator(validatorData ValidatorData) (hexutil.Bytes, string) { // in clique.go panics if this is the case, thus it's been reimplemented here to avoid the panic // and simply return an error instead func SignCliqueHeader(header *types.Header) (hexutil.Bytes, error) { - hash := common.Hash{} + //hash := common.Hash{} if len(header.Extra) < 65 { - return hash.Bytes(), fmt.Errorf("clique header extradata too short, %d < 65", len(header.Extra)) + return nil, fmt.Errorf("clique header extradata too short, %d < 65", len(header.Extra)) } - hasher := sha3.NewLegacyKeccak256() - rlp.Encode(hasher, []interface{}{ - header.ParentHash, - header.UncleHash, - header.Coinbase, - header.Root, - header.TxHash, - header.ReceiptHash, - header.Bloom, - header.Difficulty, - header.Number, - header.GasLimit, - header.GasUsed, - header.Time, - header.Extra[:len(header.Extra)-65], - header.MixDigest, - header.Nonce, - }) - hasher.Sum(hash[:0]) + //hasher := sha3.NewLegacyKeccak256() + hash := clique.SealHash(header) + //rlp.Encode(hasher, []interface{}{ + // header.ParentHash, + // header.UncleHash, + // header.Coinbase, + // header.Root, + // header.TxHash, + // header.ReceiptHash, + // header.Bloom, + // header.Difficulty, + // header.Number, + // header.GasLimit, + // header.GasUsed, + // header.Time, + // header.Extra[:len(header.Extra)-65], + // header.MixDigest, + // header.Nonce, + //}) + //hasher.Sum(hash[:0]) return hash.Bytes(), nil } @@ -309,7 +311,7 @@ func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAd sighash := crypto.Keccak256([]byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash)))) message := typedData.Format() req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: typedData.Map(), Message: message, Hash: sighash} - signature, err := api.Sign(ctx, addr, req) + signature, err := api.sign(ctx, addr, req) if err != nil { api.UI.ShowError(err.Error()) return nil, err