clef: external signing fixes + signing data (#19003)

* signer/clef: make use of json-rpc notification

* signer: tidy up output of OnApprovedTx

* accounts/external, signer: implement remote signing of text, make accounts_sign take hexdata

* clef: added basic testscript

* signer, external, api: add clique signing test to debug rpc, fix clique signing in clef

* signer: fix clique interoperability between geth and clef

* clef: rename networkid switch to chainid

* clef: enable chainid flag

* clef, signer: minor changes from review

* clef: more tests for signer
This commit is contained in:
Martin Holst Swende 2019-02-12 14:00:02 +01:00 committed by GitHub
parent edf976ee8e
commit 75d292bcf6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 240 additions and 73 deletions

@ -26,7 +26,6 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/event"
"github.com/ethereum/go-ethereum/internal/ethapi"
"github.com/ethereum/go-ethereum/log"
@ -154,13 +153,31 @@ func (api *ExternalSigner) signHash(account accounts.Account, hash []byte) ([]by
// SignData signs keccak256(data). The mimetype parameter describes the type of data being signed
func (api *ExternalSigner) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) {
// TODO! Replace this with a call to clef SignData with correct mime-type for Clique, once we
// have that in place
return api.signHash(account, crypto.Keccak256(data))
var res hexutil.Bytes
var signAddress = common.NewMixedcaseAddress(account.Address)
if err := api.client.Call(&res, "account_signData",
mimeType,
&signAddress, // Need to use the pointer here, because of how MarshalJSON is defined
hexutil.Encode(data)); err != nil {
return nil, err
}
// If V is on 27/28-form, convert to to 0/1 for Clique
if mimeType == accounts.MimetypeClique && (res[64] == 27 || res[64] == 28) {
res[64] -= 27 // Transform V from 27/28 to 0/1 for Clique use
}
return res, nil
}
func (api *ExternalSigner) SignText(account accounts.Account, text []byte) ([]byte, error) {
return api.signHash(account, accounts.TextHash(text))
var res hexutil.Bytes
var signAddress = common.NewMixedcaseAddress(account.Address)
if err := api.client.Call(&res, "account_signData",
accounts.MimetypeTextPlain,
&signAddress, // Need to use the pointer here, because of how MarshalJSON is defined
hexutil.Encode(text)); err != nil {
return nil, err
}
return res, nil
}
func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
@ -202,18 +219,6 @@ func (api *ExternalSigner) listAccounts() ([]common.Address, error) {
return res, nil
}
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", core.ApplicationClique.Mime, a, rlpBlock); err != nil {
return nil, err
}
if sig[64] != 27 && sig[64] != 28 {
return nil, fmt.Errorf("invalid Ethereum signature (V is not 27 or 28)")
}
sig[64] -= 27 // Transform V from 27/28 to 0/1 for Clique use
return sig, nil
}
func (api *ExternalSigner) pingVersion() (string, error) {
var v string
if err := api.client.Call(&v, "account_version"); err != nil {

@ -1,8 +1,15 @@
### Changelog for internal API (ui-api)
### 3.2.0
* Make `ShowError`, `OnApprovedTx`, `OnSignerStartup` be json-rpc [notifications](https://www.jsonrpc.org/specification#notification):
> A Notification is a Request object without an "id" member. A Request object that is a Notification signifies the Client's lack of interest in the corresponding Response object, and as such no Response object needs to be returned to the client. The Server MUST NOT reply to a Notification, including those that are within a batch request.
>
> Notifications are not confirmable by definition, since they do not have a Response object to be returned. As such, the Client would not be aware of any errors (like e.g. "Invalid params","Internal error"
### 3.1.0
* Add `ContentType string` to `SignDataRequest` to accommodate the latest EIP-191 and EIP-712 implementations.
* Add `ContentType` `string` to `SignDataRequest` to accommodate the latest EIP-191 and EIP-712 implementations.
### 3.0.0

@ -44,6 +44,7 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/node"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/rpc"
"github.com/ethereum/go-ethereum/signer/core"
@ -83,6 +84,11 @@ var (
Value: DefaultConfigDir(),
Usage: "Directory for Clef configuration",
}
chainIdFlag = cli.Int64Flag{
Name: "chainid",
Value: params.MainnetChainConfig.ChainID.Int64(),
Usage: "Chain id to use for signing (1=mainnet, 3=ropsten, 4=rinkeby, 5=Goerli)",
}
rpcPortFlag = cli.IntFlag{
Name: "rpcport",
Usage: "HTTP-RPC server listening port",
@ -178,7 +184,7 @@ func init() {
logLevelFlag,
keystoreFlag,
configdirFlag,
utils.NetworkIdFlag,
chainIdFlag,
utils.LightKDFFlag,
utils.NoUSBFlag,
utils.RPCListenAddrFlag,
@ -402,9 +408,13 @@ func signer(c *cli.Context) error {
}
}
}
log.Info("Starting signer", "chainid", c.GlobalInt64(chainIdFlag.Name),
"keystore", c.GlobalString(keystoreFlag.Name),
"light-kdf", c.GlobalBool(utils.LightKDFFlag.Name),
"advanced", c.GlobalBool(advancedMode.Name))
apiImpl := core.NewSignerAPI(
c.GlobalInt64(utils.NetworkIdFlag.Name),
c.GlobalInt64(chainIdFlag.Name),
c.GlobalString(keystoreFlag.Name),
c.GlobalBool(utils.NoUSBFlag.Name),
ui, db,

@ -0,0 +1,73 @@
// This file is a test-utility for testing clef-functionality
//
// Start clef with
//
// build/bin/clef --4bytedb=./cmd/clef/4byte.json --rpc
//
// Start geth with
//
// build/bin/geth --nodiscover --maxpeers 0 --signer http://localhost:8550 console --preload=cmd/clef/tests/testsigner.js
//
// and in the console simply invoke
//
// > test()
//
// You can reload the file via `reload()`
function reload(){
loadScript("./cmd/clef/tests/testsigner.js");
}
function init(){
if (typeof accts == 'undefined' || accts.length == 0){
accts = eth.accounts
console.log("Got accounts ", accts);
}
}
init()
function testTx(){
if( accts && accts.length > 0) {
var a = accts[0]
var txdata = eth.signTransaction({from: a, to: a, value: 1, nonce: 1, gas: 1, gasPrice: 1})
var v = parseInt(txdata.tx.v)
console.log("V value: ", v)
if (v == 37 || v == 38){
console.log("Mainnet 155-protected chainid was used")
}
if (v == 27 || v == 28){
throw new Error("Mainnet chainid was used, but without replay protection!")
}
}
}
function testSignText(){
if( accts && accts.length > 0){
var a = accts[0]
var r = eth.sign(a, "0x68656c6c6f20776f726c64"); //hello world
console.log("signing response", r)
}
}
function testClique(){
if( accts && accts.length > 0){
var a = accts[0]
var r = debug.testSignCliqueBlock(a, 0); // Sign genesis
console.log("signing response", r)
if( a != r){
throw new Error("Requested signing by "+a+ " but got sealer "+r)
}
}
}
function test(){
var tests = [
testTx,
testSignText,
testClique,
]
for( i in tests){
try{
tests[i]()
}catch(err){
console.log(err)
}
}
}

@ -31,6 +31,7 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/common/math"
"github.com/ethereum/go-ethereum/consensus/clique"
"github.com/ethereum/go-ethereum/consensus/ethash"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/rawdb"
@ -1469,6 +1470,45 @@ func (api *PublicDebugAPI) GetBlockRlp(ctx context.Context, number uint64) (stri
return fmt.Sprintf("%x", encoded), nil
}
// TestSignCliqueBlock fetches the given block number, and attempts to sign it as a clique header with the
// given address, returning the address of the recovered signature
//
// This is a temporary method to debug the externalsigner integration,
// TODO: Remove this method when the integration is mature
func (api *PublicDebugAPI) TestSignCliqueBlock(ctx context.Context, address common.Address, number uint64) (common.Address, error) {
block, _ := api.b.BlockByNumber(ctx, rpc.BlockNumber(number))
if block == nil {
return common.Address{}, fmt.Errorf("block #%d not found", number)
}
header := block.Header()
header.Extra = make([]byte, 32+65)
encoded := clique.CliqueRLP(header)
// Look up the wallet containing the requested signer
account := accounts.Account{Address: address}
wallet, err := api.b.AccountManager().Find(account)
if err != nil {
return common.Address{}, err
}
signature, err := wallet.SignData(account, accounts.MimetypeClique, encoded)
if err != nil {
return common.Address{}, err
}
sealHash := clique.SealHash(header).Bytes()
log.Info("test signing of clique block",
"Sealhash", fmt.Sprintf("%x", sealHash),
"signature", fmt.Sprintf("%x", signature))
pubkey, err := crypto.Ecrecover(sealHash, signature)
if err != nil {
return common.Address{}, err
}
var signer common.Address
copy(signer[:], crypto.Keccak256(pubkey[1:])[12:])
return signer, nil
}
// PrintBlock retrieves a block and returns its pretty printed form.
func (api *PublicDebugAPI) PrintBlock(ctx context.Context, number uint64) (string, error) {
block, _ := api.b.BlockByNumber(ctx, rpc.BlockNumber(number))

@ -231,6 +231,12 @@ web3._extend({
call: 'debug_getBlockRlp',
params: 1
}),
new web3._extend.Method({
name: 'testSignCliqueBlock',
call: 'debug_testSignCliqueBlock',
params: 2,
inputFormatters: [web3._extend.formatters.inputAddressFormatter, null],
}),
new web3._extend.Method({
name: 'setHead',
call: 'debug_setHead',

@ -41,7 +41,7 @@ const (
// ExternalAPIVersion -- see extapi_changelog.md
ExternalAPIVersion = "5.0.0"
// InternalAPIVersion -- see intapi_changelog.md
InternalAPIVersion = "3.1.0"
InternalAPIVersion = "3.2.0"
)
// ExternalAPI defines the external API through which signing requests are made.

@ -18,12 +18,12 @@ package core
import (
"bufio"
"encoding/json"
"fmt"
"os"
"strings"
"sync"
"github.com/davecgh/go-spew/spew"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/internal/ethapi"
"github.com/ethereum/go-ethereum/log"
@ -264,7 +264,11 @@ func (ui *CommandlineUI) ShowInfo(message string) {
func (ui *CommandlineUI) OnApprovedTx(tx ethapi.SignTransactionResult) {
fmt.Printf("Transaction signed:\n ")
spew.Dump(tx.Tx)
if jsn, err := json.MarshalIndent(tx.Tx, " ", " "); err != nil {
fmt.Printf("WARN: marshalling error %v\n", err)
} else {
fmt.Println(string(jsn))
}
}
func (ui *CommandlineUI) OnSignerStartup(info StartupInfo) {

@ -121,8 +121,8 @@ var typedDataReferenceTypeRegexp = regexp.MustCompile(`^[A-Z](\w*)(\[\])?$`)
// 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(addr common.MixedcaseAddress, req *SignDataRequest) (hexutil.Bytes, error) {
// where the V value will be 27 or 28 for legacy reasons, if legacyV==true.
func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest, legacyV bool) (hexutil.Bytes, error) {
// We make the request prior to looking up if we actually have the account, to prevent
// account-enumeration via the API
@ -140,11 +140,13 @@ func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest) (
return nil, err
}
// Sign the data with the wallet
signature, err := wallet.SignDataWithPassphrase(account, res.Password, req.ContentType, req.Hash)
signature, err := wallet.SignDataWithPassphrase(account, res.Password, req.ContentType, req.Rawdata)
if err != nil {
return nil, err
}
signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
if legacyV {
signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
}
return signature, nil
}
@ -153,17 +155,16 @@ func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest) (
//
// 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(ctx, contentType, addr, data)
var req, transformV, err = api.determineSignatureFormat(ctx, contentType, addr, data)
if err != nil {
return nil, err
}
signature, err := api.sign(addr, req)
signature, err := api.sign(addr, req, transformV)
if err != nil {
api.UI.ShowError(err.Error())
return nil, err
}
return signature, nil
}
@ -173,12 +174,14 @@ func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr com
// charset, ok := params["charset"]
// As it is now, we accept any charset and just treat it as 'raw'.
// 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
func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (*SignDataRequest, bool, error) {
var (
req *SignDataRequest
useEthereumV = true // Default to use V = 27 or 28, the legacy Ethereum format
)
mediaType, _, err := mime.ParseMediaType(contentType)
if err != nil {
return nil, err
return nil, useEthereumV, err
}
switch mediaType {
@ -186,7 +189,7 @@ func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType
// Data with an intended validator
validatorData, err := UnmarshalValidatorData(data)
if err != nil {
return nil, err
return nil, useEthereumV, err
}
sighash, msg := SignTextValidator(validatorData)
message := []*NameValueType{
@ -201,55 +204,63 @@ func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType
// Clique is the Ethereum PoA standard
stringData, ok := data.(string)
if !ok {
return nil, fmt.Errorf("input for %v plain must be an hex-encoded string", ApplicationClique.Mime)
return nil, useEthereumV, fmt.Errorf("input for %v must be an hex-encoded string", ApplicationClique.Mime)
}
cliqueData, err := hexutil.Decode(stringData)
if err != nil {
return nil, err
return nil, useEthereumV, err
}
header := &types.Header{}
if err := rlp.DecodeBytes(cliqueData, header); err != nil {
return nil, err
return nil, useEthereumV, err
}
// The incoming clique header is already truncated, sent to us with a extradata already shortened
if len(header.Extra) < 65 {
// Need to add it back, to get a suitable length for hashing
newExtra := make([]byte, len(header.Extra)+65)
copy(newExtra, header.Extra)
header.Extra = newExtra
}
// Get back the rlp data, encoded by us
cliqueData = clique.CliqueRLP(header)
sighash, err := SignCliqueHeader(header)
sighash, cliqueRlp, err := cliqueHeaderHashAndRlp(header)
if err != nil {
return nil, err
return nil, useEthereumV, err
}
message := []*NameValueType{
{
Name: "Clique block",
Name: "Clique header",
Typ: "clique",
Value: fmt.Sprintf("clique block %d [0x%x]", header.Number, header.Hash()),
Value: fmt.Sprintf("clique header %d [0x%x]", header.Number, header.Hash()),
},
}
req = &SignDataRequest{ContentType: mediaType, Rawdata: cliqueData, Message: message, Hash: sighash}
// Clique uses V on the form 0 or 1
useEthereumV = false
req = &SignDataRequest{ContentType: mediaType, Rawdata: cliqueRlp, Message: message, Hash: sighash}
default: // also case TextPlain.Mime:
// Calculates an Ethereum ECDSA signature for:
// hash = keccak256("\x19${byteVersion}Ethereum Signed Message:\n${message length}${message}")
// We expect it to be a string
stringData, ok := data.(string)
if !ok {
return nil, fmt.Errorf("input for text/plain must be a string")
if stringData, ok := data.(string); !ok {
return nil, useEthereumV, fmt.Errorf("input for text/plain must be an hex-encoded string")
} else {
if textData, err := hexutil.Decode(stringData); err != nil {
return nil, useEthereumV, err
} else {
sighash, msg := accounts.TextAndHash(textData)
message := []*NameValueType{
{
Name: "message",
Typ: "text/plain",
Value: msg,
},
}
req = &SignDataRequest{ContentType: mediaType, Rawdata: []byte(msg), Message: message, Hash: sighash}
}
}
//plainData, err := hexutil.Decode(stringdata)
//if err != nil {
// return nil, err
//}
sighash, msg := accounts.TextAndHash([]byte(stringData))
message := []*NameValueType{
{
Name: "message",
Typ: "text/plain",
Value: msg,
},
}
req = &SignDataRequest{ContentType: mediaType, Rawdata: []byte(msg), Message: message, Hash: sighash}
}
req.Address = addr
req.Meta = MetadataFromContext(ctx)
return req, nil
return req, useEthereumV, nil
}
@ -262,20 +273,21 @@ func SignTextValidator(validatorData ValidatorData) (hexutil.Bytes, string) {
return crypto.Keccak256([]byte(msg)), msg
}
// SignCliqueHeader returns the hash which is used as input for the proof-of-authority
// cliqueHeaderHashAndRlp returns the hash which is used as input for the proof-of-authority
// signing. It is the hash of the entire header apart from the 65 byte signature
// contained at the end of the extra data.
//
// The method requires the extra data to be at least 65 bytes -- the original implementation
// 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{}
func cliqueHeaderHashAndRlp(header *types.Header) (hash, rlp []byte, err error) {
if len(header.Extra) < 65 {
return nil, fmt.Errorf("clique header extradata too short, %d < 65", len(header.Extra))
err = fmt.Errorf("clique header extradata too short, %d < 65", len(header.Extra))
return
}
hash := clique.SealHash(header)
return hash.Bytes(), nil
rlp = clique.CliqueRLP(header)
hash = clique.SealHash(header).Bytes()
return hash, rlp, err
}
// SignTypedData signs EIP-712 conformant typed data
@ -293,7 +305,7 @@ func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAd
sighash := crypto.Keccak256(rawData)
message := typedData.Format()
req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Message: message, Hash: sighash}
signature, err := api.sign(addr, req)
signature, err := api.sign(addr, req, true)
if err != nil {
api.UI.ShowError(err.Error())
return nil, err
@ -722,7 +734,7 @@ func (nvt *NameValueType) Pprint(depth int) string {
output.WriteString(sublevel)
}
} else {
output.WriteString(fmt.Sprintf("%s\n", nvt.Value))
output.WriteString(fmt.Sprintf("%q\n", nvt.Value))
}
return output.String()
}

@ -49,6 +49,16 @@ func (ui *StdIOUI) dispatch(serviceMethod string, args interface{}, reply interf
return err
}
// notify sends a request over the stdio, and does not listen for a response
func (ui *StdIOUI) notify(serviceMethod string, args interface{}) error {
ctx := context.Background()
err := ui.client.Notify(ctx, serviceMethod, args)
if err != nil {
log.Info("Error", "exc", err.Error())
}
return err
}
func (ui *StdIOUI) ApproveTx(request *SignTxRequest) (SignTxResponse, error) {
var result SignTxResponse
err := ui.dispatch("ApproveTx", request, &result)
@ -86,27 +96,27 @@ func (ui *StdIOUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResp
}
func (ui *StdIOUI) ShowError(message string) {
err := ui.dispatch("ShowError", &Message{message}, nil)
err := ui.notify("ShowError", &Message{message})
if err != nil {
log.Info("Error calling 'ShowError'", "exc", err.Error(), "msg", message)
}
}
func (ui *StdIOUI) ShowInfo(message string) {
err := ui.dispatch("ShowInfo", Message{message}, nil)
err := ui.notify("ShowInfo", Message{message})
if err != nil {
log.Info("Error calling 'ShowInfo'", "exc", err.Error(), "msg", message)
}
}
func (ui *StdIOUI) OnApprovedTx(tx ethapi.SignTransactionResult) {
err := ui.dispatch("OnApprovedTx", tx, nil)
err := ui.notify("OnApprovedTx", tx)
if err != nil {
log.Info("Error calling 'OnApprovedTx'", "exc", err.Error(), "tx", tx)
}
}
func (ui *StdIOUI) OnSignerStartup(info StartupInfo) {
err := ui.dispatch("OnSignerStartup", info, nil)
err := ui.notify("OnSignerStartup", info)
if err != nil {
log.Info("Error calling 'OnSignerStartup'", "exc", err.Error(), "info", info)
}