From 65da3cf340e035254dcb3955fb069467d509301d Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 22 Jul 2017 05:25:59 -0400 Subject: [PATCH 1/5] Add crc16 support --- glide.lock | 24 +++++++++--------- glide.yaml | 1 + keys/ecc.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ keys/ecc_test.go | 6 +++++ 4 files changed, 83 insertions(+), 11 deletions(-) diff --git a/glide.lock b/glide.lock index 71292ce11..4cd7c5670 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 3bcee9fbccf29d21217b24b6a83ec51e1514f37b2ae5d8718cf6c5df80f4fb2c -updated: 2017-06-19T17:16:58.037568333+02:00 +hash: c0a2db1b80c6b1b8aab31c526ce43e22e49b23c893c78b8fdb8546aa2e7b7cc6 +updated: 2017-07-22T05:24:42.82932575-04:00 imports: - name: github.com/bgentry/speakeasy version: 4aabc24848ce5fd31929f7d1e4ea74d3709c14cd @@ -38,11 +38,11 @@ imports: - name: github.com/gorilla/context version: 08b5f424b9271eedf6f9f0ce86cb9396ed337a42 - name: github.com/gorilla/handlers - version: 3a5767ca75ece5f7f1440b1d16975247f8d8b221 + version: a4043c62cc2329bacda331d33fc908ab11ef0ec3 - name: github.com/gorilla/mux - version: 392c28fe23e1c45ddba891b0320b3b5df220beea + version: bcd8bc72b08df0f70df986b97f95590779502d31 - name: github.com/hashicorp/hcl - version: a4b07c25de5ff55ad3b8936cea69a79a3d95a855 + version: 392dba7d905ed5d04a5794ba89f558b27e2ba1ca subpackages: - hcl/ast - hcl/parser @@ -52,6 +52,8 @@ imports: - json/parser - json/scanner - json/token +- name: github.com/howeyc/crc16 + version: 58da63c846043d0bea709c8d47039df06577d6d9 - name: github.com/inconshreveable/mousetrap version: 76626ae9c91c4f2a10f34cad8ce83ea42c93bb75 - name: github.com/kr/logfmt @@ -67,7 +69,7 @@ imports: - name: github.com/pelletier/go-toml version: 13d49d4606eb801b8f01ae542b4afc4c6ee3d84a - name: github.com/pkg/errors - version: ff09b135c25aae272398c51a07235b90a75aa4f0 + version: 645ef00459ed84a119197bfb8d8205042c6df63d - name: github.com/spf13/afero version: 9be650865eab0c12963d8753212f4f9c66cdcf12 subpackages: @@ -75,11 +77,11 @@ imports: - name: github.com/spf13/cast version: acbeb36b902d72a7a4c18e8f3241075e7ab763e4 - name: github.com/spf13/cobra - version: db6b9a8b3f3f400c8ecb4a4d7d02245b8facad66 + version: 4cdb38c072b86bf795d2c81de50784d9fdd6eb77 - name: github.com/spf13/jwalterweatherman - version: fa7ca7e836cf3a8bb4ebf799f472c12d7e903d66 + version: 8f07c835e5cc1450c082fe3a439cf87b0cbb2d99 - name: github.com/spf13/pflag - version: 80fe0fb4eba54167e2ccae1c6c950e72abf61b73 + version: e57e3eeb33f795204c1ca35f56c44f83227c6e66 - name: github.com/spf13/viper version: 0967fc9aceab2ce9da34061253ac10fb99bba5b2 - name: github.com/tendermint/ed25519 @@ -93,7 +95,7 @@ imports: - data - data/base58 - name: github.com/tendermint/tmlibs - version: bd9d0d1637dadf1330e167189d5e5031aadcda6f + version: 2f6f3e6aa70bb19b70a6e73210273fa127041070 subpackages: - cli - common @@ -111,7 +113,7 @@ imports: - ripemd160 - salsa20/salsa - name: golang.org/x/sys - version: 9ccfe848b9db8435a24c424abbc07a921adf1df5 + version: e62c3de784db939836898e5c19ffd41bece347da subpackages: - unix - name: golang.org/x/text diff --git a/glide.yaml b/glide.yaml index 58e3aecca..a99d3b63f 100644 --- a/glide.yaml +++ b/glide.yaml @@ -29,6 +29,7 @@ import: - package: github.com/spf13/cobra - package: github.com/spf13/viper - package: gopkg.in/go-playground/validator.v9 +- package: github.com/howeyc/crc16 testImport: - package: github.com/mndrix/btcutil - package: github.com/stretchr/testify diff --git a/keys/ecc.go b/keys/ecc.go index 96590cd91..b94bf1fb9 100644 --- a/keys/ecc.go +++ b/keys/ecc.go @@ -5,6 +5,8 @@ import ( "errors" "hash/crc32" "hash/crc64" + + "github.com/howeyc/crc16" ) // ECC is used for anything that calculates an error-correcting code @@ -26,6 +28,67 @@ var _ ECC = NoECC{} func (_ NoECC) AddECC(input []byte) []byte { return input } func (_ NoECC) CheckECC(input []byte) ([]byte, error) { return input, nil } +// CRC16 does the ieee crc16 polynomial check +type CRC16 struct { + Poly uint16 + table *crc16.Table +} + +var _ ECC = &CRC16{} + +const crc16Size = 2 + +func NewIBMCRC16() *CRC16 { + return &CRC16{Poly: crc16.IBM} +} + +func NewSCSICRC16() *CRC16 { + return &CRC16{Poly: crc16.SCSI} +} + +func NewCCITTCRC16() *CRC16 { + return &CRC16{Poly: crc16.CCITT} +} + +func (c *CRC16) AddECC(input []byte) []byte { + table := c.getTable() + + // get crc and convert to some bytes... + crc := crc16.Checksum(input, table) + check := make([]byte, crc16Size) + binary.BigEndian.PutUint16(check, crc) + + // append it to the input + output := append(input, check...) + return output +} + +func (c *CRC16) CheckECC(input []byte) ([]byte, error) { + table := c.getTable() + + if len(input) <= crc16Size { + return nil, errors.New("input too short, no checksum present") + } + cut := len(input) - crc16Size + data, check := input[:cut], input[cut:] + crc := binary.BigEndian.Uint16(check) + calc := crc16.Checksum(data, table) + if crc != calc { + return nil, errors.New("Checksum does not match") + } + return data, nil +} + +func (c *CRC16) getTable() *crc16.Table { + if c.table == nil { + if c.Poly == 0 { + c.Poly = crc16.IBM + } + c.table = crc16.MakeTable(c.Poly) + } + return c.table +} + // CRC32 does the ieee crc32 polynomial check type CRC32 struct { Poly uint32 diff --git a/keys/ecc_test.go b/keys/ecc_test.go index 334c49423..930422b98 100644 --- a/keys/ecc_test.go +++ b/keys/ecc_test.go @@ -14,6 +14,9 @@ func TestECCPasses(t *testing.T) { checks := []ECC{ NoECC{}, + NewIBMCRC16(), + NewSCSICRC16(), + NewCCITTCRC16(), NewIEEECRC32(), NewCastagnoliCRC32(), NewKoopmanCRC32(), @@ -40,6 +43,9 @@ func TestECCFails(t *testing.T) { assert := assert.New(t) checks := []ECC{ + NewIBMCRC16(), + NewSCSICRC16(), + NewCCITTCRC16(), NewIEEECRC32(), NewCastagnoliCRC32(), NewKoopmanCRC32(), From 4ff889a236daa068db5666115637d74761a55a33 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 22 Jul 2017 05:44:09 -0400 Subject: [PATCH 2/5] Use 16 random bytes for seed and key, crc16 by default --- keys/cryptostore/encoder_test.go | 9 ++++++--- keys/cryptostore/generator.go | 16 ++++++++-------- keys/cryptostore/holder.go | 21 ++++++++++++++------- keys/cryptostore/storage_test.go | 6 +++++- keys/wordcodec.go | 3 ++- keys/wordcodec_test.go | 2 +- 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/keys/cryptostore/encoder_test.go b/keys/cryptostore/encoder_test.go index e5ea21111..945e19865 100644 --- a/keys/cryptostore/encoder_test.go +++ b/keys/cryptostore/encoder_test.go @@ -5,6 +5,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + cmn "github.com/tendermint/tmlibs/common" + "github.com/tendermint/go-crypto/keys/cryptostore" ) @@ -12,8 +15,8 @@ func TestNoopEncoder(t *testing.T) { assert, require := assert.New(t), require.New(t) noop := cryptostore.Noop - key := cryptostore.GenEd25519.Generate() - key2 := cryptostore.GenSecp256k1.Generate() + key := cryptostore.GenEd25519.Generate(cmn.RandBytes(16)) + key2 := cryptostore.GenSecp256k1.Generate(cmn.RandBytes(16)) b, err := noop.Encrypt(key, "encode") require.Nil(err) @@ -40,7 +43,7 @@ func TestSecretBox(t *testing.T) { assert, require := assert.New(t), require.New(t) enc := cryptostore.SecretBox - key := cryptostore.GenEd25519.Generate() + key := cryptostore.GenEd25519.Generate(cmn.RandBytes(16)) pass := "some-special-secret" b, err := enc.Encrypt(key, pass) diff --git a/keys/cryptostore/generator.go b/keys/cryptostore/generator.go index 6bbdb6441..307a0ae86 100644 --- a/keys/cryptostore/generator.go +++ b/keys/cryptostore/generator.go @@ -14,22 +14,22 @@ var ( // Generator determines the type of private key the keystore creates type Generator interface { - Generate() crypto.PrivKey + Generate(secret []byte) crypto.PrivKey } // GenFunc is a helper to transform a function into a Generator -type GenFunc func() crypto.PrivKey +type GenFunc func(secret []byte) crypto.PrivKey -func (f GenFunc) Generate() crypto.PrivKey { - return f() +func (f GenFunc) Generate(secret []byte) crypto.PrivKey { + return f(secret) } -func genEd25519() crypto.PrivKey { - return crypto.GenPrivKeyEd25519().Wrap() +func genEd25519(secret []byte) crypto.PrivKey { + return crypto.GenPrivKeyEd25519FromSecret(secret).Wrap() } -func genSecp256() crypto.PrivKey { - return crypto.GenPrivKeySecp256k1().Wrap() +func genSecp256(secret []byte) crypto.PrivKey { + return crypto.GenPrivKeySecp256k1FromSecret(secret).Wrap() } func getGenerator(algo string) (Generator, error) { diff --git a/keys/cryptostore/holder.go b/keys/cryptostore/holder.go index 0e4fde042..a3b5d2f61 100644 --- a/keys/cryptostore/holder.go +++ b/keys/cryptostore/holder.go @@ -43,12 +43,16 @@ func (s Manager) Create(name, passphrase, algo string) (keys.Info, string, error if err != nil { return keys.Info{}, "", err } - key := gen.Generate() + + // 128-bits the the all the randomness we can make use of + secret := crypto.CRandBytes(16) + key := gen.Generate(secret) err = s.es.Put(name, passphrase, key) if err != nil { return keys.Info{}, "", err } - seed, err := s.codec.BytesToWords(key.Bytes()) + + seed, err := s.codec.BytesToWords(secret) phrase := strings.Join(seed, " ") return info(name, key), phrase, err } @@ -61,15 +65,18 @@ func (s Manager) Create(name, passphrase, algo string) (keys.Info, string, error // Result similar to New(), except it doesn't return the seed again... func (s Manager) Recover(name, passphrase, seedphrase string) (keys.Info, error) { words := strings.Split(strings.TrimSpace(seedphrase), " ") - data, err := s.codec.WordsToBytes(words) + secret, err := s.codec.WordsToBytes(words) if err != nil { return keys.Info{}, err } - key, err := crypto.PrivKeyFromBytes(data) - if err != nil { - return keys.Info{}, err - } + // TODO: flag this??? + gen := GenEd25519 + // gen, err := getGenerator(algo) + // if err != nil { + // return keys.Info{}, "", err + // } + key := gen.Generate(secret) // d00d, it worked! create the bugger.... err = s.es.Put(name, passphrase, key) diff --git a/keys/cryptostore/storage_test.go b/keys/cryptostore/storage_test.go index b109c44e8..907a19f11 100644 --- a/keys/cryptostore/storage_test.go +++ b/keys/cryptostore/storage_test.go @@ -4,13 +4,17 @@ import ( "testing" "github.com/stretchr/testify/assert" + + crypto "github.com/tendermint/go-crypto" + cmn "github.com/tendermint/tmlibs/common" + keys "github.com/tendermint/go-crypto/keys" ) func TestSortKeys(t *testing.T) { assert := assert.New(t) - gen := GenEd25519.Generate + gen := func() crypto.PrivKey { return GenEd25519.Generate(cmn.RandBytes(16)) } assert.NotEqual(gen(), gen()) // alphabetical order is n3, n1, n2 diff --git a/keys/wordcodec.go b/keys/wordcodec.go index ee1374644..214fe7e0b 100644 --- a/keys/wordcodec.go +++ b/keys/wordcodec.go @@ -34,7 +34,8 @@ func NewCodec(words []string) (codec *WordCodec, err error) { res := &WordCodec{ words: words, // TODO: configure this outside??? - check: NewIEEECRC32(), + // check: NewIEEECRC32(), + check: NewIBMCRC16(), } return res, nil diff --git a/keys/wordcodec_test.go b/keys/wordcodec_test.go index 1ae97d8a7..25c5439a6 100644 --- a/keys/wordcodec_test.go +++ b/keys/wordcodec_test.go @@ -152,7 +152,7 @@ func TestCheckTypoDetection(t *testing.T) { codec, err := LoadCodec(bank) require.Nil(err, "%s: %+v", bank, err) for i := 0; i < 1000; i++ { - numBytes := cmn.RandInt()%60 + 1 + numBytes := cmn.RandInt()%60 + 4 data := cmn.RandBytes(numBytes) words, err := codec.BytesToWords(data) From c20e83565c959438f00c029195345e23485b9e50 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 22 Jul 2017 05:53:46 -0400 Subject: [PATCH 3/5] Recovery also works with secp256 keys --- keys/cryptostore/generator.go | 11 +++++++++++ keys/cryptostore/holder.go | 17 ++++++++++------ tests/keys.sh | 37 ++++++++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/keys/cryptostore/generator.go b/keys/cryptostore/generator.go index 307a0ae86..0a2bb55c2 100644 --- a/keys/cryptostore/generator.go +++ b/keys/cryptostore/generator.go @@ -42,3 +42,14 @@ func getGenerator(algo string) (Generator, error) { return nil, errors.Errorf("Cannot generate keys for algorithm: %s", algo) } } + +func getGeneratorByType(typ byte) (Generator, error) { + switch typ { + case crypto.TypeEd25519: + return GenEd25519, nil + case crypto.TypeSecp256k1: + return GenSecp256k1, nil + default: + return nil, errors.Errorf("Cannot generate keys for algorithm: %X", typ) + } +} diff --git a/keys/cryptostore/holder.go b/keys/cryptostore/holder.go index a3b5d2f61..202e55973 100644 --- a/keys/cryptostore/holder.go +++ b/keys/cryptostore/holder.go @@ -52,6 +52,10 @@ func (s Manager) Create(name, passphrase, algo string) (keys.Info, string, error return keys.Info{}, "", err } + // TODO: clean up type/kind handling with go-data + typ := key.Bytes()[0] + secret = append(secret, typ) + seed, err := s.codec.BytesToWords(secret) phrase := strings.Join(seed, " ") return info(name, key), phrase, err @@ -70,12 +74,13 @@ func (s Manager) Recover(name, passphrase, seedphrase string) (keys.Info, error) return keys.Info{}, err } - // TODO: flag this??? - gen := GenEd25519 - // gen, err := getGenerator(algo) - // if err != nil { - // return keys.Info{}, "", err - // } + l := len(secret) + secret, typ := secret[:l-1], secret[l-1] + + gen, err := getGeneratorByType(typ) + if err != nil { + return keys.Info{}, err + } key := gen.Generate(secret) // d00d, it worked! create the bugger.... diff --git a/tests/keys.sh b/tests/keys.sh index c848f8dd2..599f3d743 100755 --- a/tests/keys.sh +++ b/tests/keys.sh @@ -68,7 +68,6 @@ test03recoverKeys() { PASS2=1234567890 # make a user and check they exist - echo "create..." KEY=$(echo $PASS1 | ${EXE} new $USER -o json) if ! assertTrue "created $USER" $?; then return 1; fi if [ -n "$DEBUG" ]; then echo $KEY; echo; fi @@ -79,13 +78,11 @@ test03recoverKeys() { assertTrue "${EXE} get $USER > /dev/null" # let's delete this key - echo "delete..." assertFalse "echo foo | ${EXE} delete $USER > /dev/null" assertTrue "echo $PASS1 | ${EXE} delete $USER > /dev/null" assertFalse "${EXE} get $USER > /dev/null" # fails on short password - echo "recover..." assertFalse "echo foo; echo $SEED | ${EXE} recover $USER2 -o json > /dev/null" # fails on bad seed assertFalse "echo $PASS2; echo \"silly white whale tower bongo\" | ${EXE} recover $USER2 -o json > /dev/null" @@ -106,6 +103,40 @@ test03recoverKeys() { assertTrue "${EXE} get $USER2 > /dev/null" } +# try recovery with secp256k1 keys +test03recoverSecp() { + USER=dings + PASS1=Sbub-U9byS7hso + + USER2=booms + PASS2=1234567890 + + KEY=$(echo $PASS1 | ${EXE} new $USER -o json -t secp256k1) + if ! assertTrue "created $USER" $?; then return 1; fi + if [ -n "$DEBUG" ]; then echo $KEY; echo; fi + + SEED=$(echo $KEY | jq .seed | tr -d \") + ADDR=$(echo $KEY | jq .key.address | tr -d \") + PUBKEY=$(echo $KEY | jq .key.pubkey | tr -d \") + assertTrue "${EXE} get $USER > /dev/null" + + # now we got it + KEY2=$((echo $PASS2; echo $SEED) | ${EXE} recover $USER2 -o json) + if ! assertTrue "recovery failed: $KEY2" $?; then return 1; fi + if [ -n "$DEBUG" ]; then echo $KEY2; echo; fi + + # make sure it looks the same + NAME2=$(echo $KEY2 | jq .name | tr -d \") + ADDR2=$(echo $KEY2 | jq .address | tr -d \") + PUBKEY2=$(echo $KEY2 | jq .pubkey | tr -d \") + assertEquals "wrong username" "$USER2" "$NAME2" + assertEquals "address doesn't match" "$ADDR" "$ADDR2" + assertEquals "pubkey doesn't match" "$PUBKEY" "$PUBKEY2" + + # and we can find the info + assertTrue "${EXE} get $USER2 > /dev/null" +} + # load and run these tests with shunit2! DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" #get this files directory . $DIR/shunit2 From 10222adaf1dc58a0d844caafce36fdd001c1e0be Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 26 Jul 2017 16:12:25 -0400 Subject: [PATCH 4/5] Remove deprecated code. Now in basecoin/weave --- keys/tx/docs.go | 10 ------ keys/tx/multi.go | 67 ------------------------------------ keys/tx/multi_test.go | 78 ------------------------------------------ keys/tx/one.go | 57 ------------------------------ keys/tx/one_test.go | 74 --------------------------------------- keys/tx/reader.go | 76 ---------------------------------------- keys/tx/reader_test.go | 72 -------------------------------------- 7 files changed, 434 deletions(-) delete mode 100644 keys/tx/docs.go delete mode 100644 keys/tx/multi.go delete mode 100644 keys/tx/multi_test.go delete mode 100644 keys/tx/one.go delete mode 100644 keys/tx/one_test.go delete mode 100644 keys/tx/reader.go delete mode 100644 keys/tx/reader_test.go diff --git a/keys/tx/docs.go b/keys/tx/docs.go deleted file mode 100644 index 6a5ea3ce3..000000000 --- a/keys/tx/docs.go +++ /dev/null @@ -1,10 +0,0 @@ -/* -package tx contains generic Signable implementations that can be used -by your application or tests to handle authentication needs. - -It currently supports transaction data as opaque bytes and either single -or multiple private key signatures using straightforward algorithms. -It currently does not support N-of-M key share signing of other more -complex algorithms (although it would be great to add them) -*/ -package tx diff --git a/keys/tx/multi.go b/keys/tx/multi.go deleted file mode 100644 index f069fb273..000000000 --- a/keys/tx/multi.go +++ /dev/null @@ -1,67 +0,0 @@ -package tx - -import ( - "github.com/pkg/errors" - crypto "github.com/tendermint/go-crypto" - data "github.com/tendermint/go-wire/data" -) - -// MultiSig lets us wrap arbitrary data with a go-crypto signature -// -// TODO: rethink how we want to integrate this with KeyStore so it makes -// more sense (particularly the verify method) -type MultiSig struct { - Data data.Bytes - Sigs []Signed -} - -type Signed struct { - Sig crypto.Signature - Pubkey crypto.PubKey -} - -var _ SigInner = &MultiSig{} - -func NewMulti(data []byte) Sig { - return Sig{&MultiSig{Data: data}} -} - -// SignBytes returns the original data passed into `NewSig` -func (s *MultiSig) SignBytes() []byte { - return s.Data -} - -// Sign will add a signature and pubkey. -// -// Depending on the Signable, one may be able to call this multiple times for multisig -// Returns error if called with invalid data or too many times -func (s *MultiSig) Sign(pubkey crypto.PubKey, sig crypto.Signature) error { - if pubkey.Empty() || sig.Empty() { - return errors.New("Signature or Key missing") - } - - // set the value once we are happy - x := Signed{sig, pubkey} - s.Sigs = append(s.Sigs, x) - return nil -} - -// Signers will return the public key(s) that signed if the signature -// is valid, or an error if there is any issue with the signature, -// including if there are no signatures -func (s *MultiSig) Signers() ([]crypto.PubKey, error) { - if len(s.Sigs) == 0 { - return nil, errors.New("Never signed") - } - - keys := make([]crypto.PubKey, len(s.Sigs)) - for i := range s.Sigs { - ms := s.Sigs[i] - if !ms.Pubkey.VerifyBytes(s.Data, ms.Sig) { - return nil, errors.Errorf("Signature %d doesn't match (key: %X)", i, ms.Pubkey.Bytes()) - } - keys[i] = ms.Pubkey - } - - return keys, nil -} diff --git a/keys/tx/multi_test.go b/keys/tx/multi_test.go deleted file mode 100644 index 22fb0ed75..000000000 --- a/keys/tx/multi_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package tx - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - crypto "github.com/tendermint/go-crypto" - keys "github.com/tendermint/go-crypto/keys" - "github.com/tendermint/go-crypto/keys/cryptostore" - "github.com/tendermint/go-crypto/keys/storage/memstorage" -) - -func TestMultiSig(t *testing.T) { - assert, require := assert.New(t), require.New(t) - - algo := crypto.NameEd25519 - cstore := cryptostore.New( - cryptostore.SecretBox, - memstorage.New(), - keys.MustLoadCodec("english"), - ) - n, p := "foo", "bar" - n2, p2 := "other", "thing" - - acct, _, err := cstore.Create(n, p, algo) - require.Nil(err, "%+v", err) - acct2, _, err := cstore.Create(n2, p2, algo) - require.Nil(err, "%+v", err) - - type signer struct { - key keys.Info - name, pass string - } - cases := []struct { - data string - signers []signer - }{ - {"one", []signer{{acct, n, p}}}, - {"two", []signer{{acct2, n2, p2}}}, - {"both", []signer{{acct, n, p}, {acct2, n2, p2}}}, - } - - for _, tc := range cases { - tx := NewMulti([]byte(tc.data)) - // unsigned version - _, err = tx.Signers() - assert.NotNil(err) - orig, err := tx.TxBytes() - require.Nil(err, "%+v", err) - data := tx.SignBytes() - assert.Equal(tc.data, string(data)) - - // sign it - for _, s := range tc.signers { - err = cstore.Sign(s.name, s.pass, tx) - require.Nil(err, "%+v", err) - } - - // make sure it is proper now - sigs, err := tx.Signers() - require.Nil(err, "%+v", err) - if assert.Equal(len(tc.signers), len(sigs)) { - for i := range sigs { - // This must be refactored... - assert.Equal(tc.signers[i].key.PubKey, sigs[i]) - } - } - // the tx bytes should change after this - after, err := tx.TxBytes() - require.Nil(err, "%+v", err) - assert.NotEqual(orig, after, "%X != %X", orig, after) - - // sign bytes are the same - data = tx.SignBytes() - assert.Equal(tc.data, string(data)) - } -} diff --git a/keys/tx/one.go b/keys/tx/one.go deleted file mode 100644 index af468cc2c..000000000 --- a/keys/tx/one.go +++ /dev/null @@ -1,57 +0,0 @@ -package tx - -import ( - "github.com/pkg/errors" - crypto "github.com/tendermint/go-crypto" - data "github.com/tendermint/go-wire/data" -) - -// OneSig lets us wrap arbitrary data with a go-crypto signature -// -// TODO: rethink how we want to integrate this with KeyStore so it makes -// more sense (particularly the verify method) -type OneSig struct { - Data data.Bytes - Signed -} - -var _ SigInner = &OneSig{} - -func New(data []byte) Sig { - return WrapSig(&OneSig{Data: data}) -} - -// SignBytes returns the original data passed into `NewSig` -func (s *OneSig) SignBytes() []byte { - return s.Data -} - -// Sign will add a signature and pubkey. -// -// Depending on the Signable, one may be able to call this multiple times for multisig -// Returns error if called with invalid data or too many times -func (s *OneSig) Sign(pubkey crypto.PubKey, sig crypto.Signature) error { - if pubkey.Empty() || sig.Empty() { - return errors.New("Signature or Key missing") - } - if !s.Sig.Empty() { - return errors.New("Transaction can only be signed once") - } - - // set the value once we are happy - s.Signed = Signed{sig, pubkey} - return nil -} - -// Signers will return the public key(s) that signed if the signature -// is valid, or an error if there is any issue with the signature, -// including if there are no signatures -func (s *OneSig) Signers() ([]crypto.PubKey, error) { - if s.Pubkey.Empty() || s.Sig.Empty() { - return nil, errors.New("Never signed") - } - if !s.Pubkey.VerifyBytes(s.Data, s.Sig) { - return nil, errors.New("Signature doesn't match") - } - return []crypto.PubKey{s.Pubkey}, nil -} diff --git a/keys/tx/one_test.go b/keys/tx/one_test.go deleted file mode 100644 index e0a3f1056..000000000 --- a/keys/tx/one_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package tx - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - crypto "github.com/tendermint/go-crypto" - keys "github.com/tendermint/go-crypto/keys" - "github.com/tendermint/go-crypto/keys/cryptostore" - "github.com/tendermint/go-crypto/keys/storage/memstorage" -) - -func TestOneSig(t *testing.T) { - assert, require := assert.New(t), require.New(t) - - algo := crypto.NameEd25519 - cstore := cryptostore.New( - cryptostore.SecretBox, - memstorage.New(), - keys.MustLoadCodec("english"), - ) - n, p := "foo", "bar" - n2, p2 := "other", "thing" - - acct, _, err := cstore.Create(n, p, algo) - require.Nil(err, "%+v", err) - acct2, _, err := cstore.Create(n2, p2, algo) - require.Nil(err, "%+v", err) - - cases := []struct { - data string - key keys.Info - name, pass string - }{ - {"first", acct, n, p}, - {"kehfkhefy8y", acct, n, p}, - {"second", acct2, n2, p2}, - } - - for _, tc := range cases { - tx := New([]byte(tc.data)) - // unsigned version - _, err = tx.Signers() - assert.NotNil(err) - orig, err := tx.TxBytes() - require.Nil(err, "%+v", err) - data := tx.SignBytes() - assert.Equal(tc.data, string(data)) - - // sign it - err = cstore.Sign(tc.name, tc.pass, tx) - require.Nil(err, "%+v", err) - // but not twice - err = cstore.Sign(tc.name, tc.pass, tx) - require.NotNil(err) - - // make sure it is proper now - sigs, err := tx.Signers() - require.Nil(err, "%+v", err) - if assert.Equal(1, len(sigs)) { - // This must be refactored... - assert.Equal(tc.key.PubKey, sigs[0]) - } - // the tx bytes should change after this - after, err := tx.TxBytes() - require.Nil(err, "%+v", err) - assert.NotEqual(orig, after, "%X != %X", orig, after) - - // sign bytes are the same - data = tx.SignBytes() - assert.Equal(tc.data, string(data)) - } -} diff --git a/keys/tx/reader.go b/keys/tx/reader.go deleted file mode 100644 index 265e88b46..000000000 --- a/keys/tx/reader.go +++ /dev/null @@ -1,76 +0,0 @@ -package tx - -import ( - crypto "github.com/tendermint/go-crypto" - keys "github.com/tendermint/go-crypto/keys" - data "github.com/tendermint/go-wire/data" -) - -const ( - typeOneSig = byte(0x01) - typeMultiSig = byte(0x02) - nameOneSig = "sig" - nameMultiSig = "multi" -) - -var _ keys.Signable = Sig{} -var TxMapper data.Mapper - -func init() { - TxMapper = data.NewMapper(Sig{}). - RegisterImplementation(&OneSig{}, nameOneSig, typeOneSig). - RegisterImplementation(&MultiSig{}, nameMultiSig, typeMultiSig) -} - -/* -DO NOT USE this interface. - -It is public by necessity but should never be used directly -outside of this package. - -Only use Sig, never SigInner -*/ -type SigInner interface { - SignBytes() []byte - Sign(pubkey crypto.PubKey, sig crypto.Signature) error - Signers() ([]crypto.PubKey, error) -} - -// Sig is what is exported, and handles serialization -type Sig struct { - SigInner -} - -// TxBytes -func (s Sig) TxBytes() ([]byte, error) { - return data.ToWire(s) -} - -// WrapSig goes from concrete implementation to "interface" struct -func WrapSig(pk SigInner) Sig { - if wrap, ok := pk.(Sig); ok { - pk = wrap.Unwrap() - } - return Sig{pk} -} - -// Unwrap recovers the concrete interface safely (regardless of levels of embeds) -func (p Sig) Unwrap() SigInner { - pk := p.SigInner - for wrap, ok := pk.(Sig); ok; wrap, ok = pk.(Sig) { - pk = wrap.SigInner - } - return pk -} - -func (p Sig) MarshalJSON() ([]byte, error) { - return TxMapper.ToJSON(p.Unwrap()) -} - -func (p *Sig) UnmarshalJSON(data []byte) (err error) { - parsed, err := TxMapper.FromJSON(data) - if err == nil && parsed != nil { - p.SigInner = parsed.(SigInner) - } - return -} diff --git a/keys/tx/reader_test.go b/keys/tx/reader_test.go deleted file mode 100644 index f0561301f..000000000 --- a/keys/tx/reader_test.go +++ /dev/null @@ -1,72 +0,0 @@ -package tx - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - crypto "github.com/tendermint/go-crypto" - "github.com/tendermint/go-crypto/keys" - "github.com/tendermint/go-crypto/keys/cryptostore" - "github.com/tendermint/go-crypto/keys/storage/memstorage" - data "github.com/tendermint/go-wire/data" -) - -func TestReader(t *testing.T) { - assert, require := assert.New(t), require.New(t) - - algo := crypto.NameEd25519 - cstore := cryptostore.New( - cryptostore.SecretBox, - memstorage.New(), - keys.MustLoadCodec("english"), - ) - type sigs struct{ name, pass string } - u := sigs{"alice", "1234"} - u2 := sigs{"bob", "foobar"} - - _, _, err := cstore.Create(u.name, u.pass, algo) - require.Nil(err, "%+v", err) - _, _, err = cstore.Create(u2.name, u2.pass, algo) - require.Nil(err, "%+v", err) - - cases := []struct { - tx Sig - sigs []sigs - }{ - {New([]byte("first")), nil}, - {New([]byte("second")), []sigs{u}}, - {New([]byte("other")), []sigs{u2}}, - {NewMulti([]byte("m-first")), nil}, - {NewMulti([]byte("m-second")), []sigs{u}}, - {NewMulti([]byte("m-other")), []sigs{u, u2}}, - } - - for _, tc := range cases { - tx := tc.tx - - // make sure json serialization and loading works w/o sigs - var pre Sig - pjs, err := data.ToJSON(tx) - require.Nil(err, "%+v", err) - err = data.FromJSON(pjs, &pre) - require.Nil(err, "%+v", err) - assert.Equal(tx, pre) - - for _, s := range tc.sigs { - err = cstore.Sign(s.name, s.pass, tx) - require.Nil(err, "%+v", err) - } - - var post Sig - sjs, err := data.ToJSON(tx) - require.Nil(err, "%+v", err) - err = data.FromJSON(sjs, &post) - require.Nil(err, "%+v\n%s", err, string(sjs)) - assert.Equal(tx, post) - - if len(tc.sigs) > 0 { - assert.NotEqual(pjs, sjs, "%s\n ------ %s", string(pjs), string(sjs)) - } - } -} From 050b9657083e3c2835dc6807291bd83b90345b61 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 27 Jul 2017 15:59:59 -0400 Subject: [PATCH 5/5] Code cleanup ala Emmanuel --- keys/cryptostore/holder.go | 7 +++++-- keys/ecc.go | 40 +++++++++++++++++++++----------------- keys/ecc_test.go | 35 +++++++++++++-------------------- 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/keys/cryptostore/holder.go b/keys/cryptostore/holder.go index 202e55973..18437a9b5 100644 --- a/keys/cryptostore/holder.go +++ b/keys/cryptostore/holder.go @@ -44,7 +44,7 @@ func (s Manager) Create(name, passphrase, algo string) (keys.Info, string, error return keys.Info{}, "", err } - // 128-bits the the all the randomness we can make use of + // 128-bits are the all the randomness we can make use of secret := crypto.CRandBytes(16) key := gen.Generate(secret) err = s.es.Put(name, passphrase, key) @@ -52,7 +52,8 @@ func (s Manager) Create(name, passphrase, algo string) (keys.Info, string, error return keys.Info{}, "", err } - // TODO: clean up type/kind handling with go-data + // we append the type byte to the serialized secret to help with recovery + // ie [secret] = [secret] + [type] typ := key.Bytes()[0] secret = append(secret, typ) @@ -74,6 +75,8 @@ func (s Manager) Recover(name, passphrase, seedphrase string) (keys.Info, error) return keys.Info{}, err } + // secret is comprised of the actual secret with the type appended + // ie [secret] = [secret] + [type] l := len(secret) secret, typ := secret[:l-1], secret[l-1] diff --git a/keys/ecc.go b/keys/ecc.go index b94bf1fb9..c1ac258fe 100644 --- a/keys/ecc.go +++ b/keys/ecc.go @@ -20,6 +20,9 @@ type ECC interface { CheckECC([]byte) ([]byte, error) } +var errInputTooShort = errors.New("input too short, no checksum present") +var errChecksumDoesntMatch = errors.New("checksum does not match") + // NoECC is a no-op placeholder, kind of useless... except for tests type NoECC struct{} @@ -34,9 +37,9 @@ type CRC16 struct { table *crc16.Table } -var _ ECC = &CRC16{} +var _ ECC = (*CRC16)(nil) -const crc16Size = 2 +const crc16ByteCount = 2 func NewIBMCRC16() *CRC16 { return &CRC16{Poly: crc16.IBM} @@ -55,7 +58,7 @@ func (c *CRC16) AddECC(input []byte) []byte { // get crc and convert to some bytes... crc := crc16.Checksum(input, table) - check := make([]byte, crc16Size) + check := make([]byte, crc16ByteCount) binary.BigEndian.PutUint16(check, crc) // append it to the input @@ -66,26 +69,27 @@ func (c *CRC16) AddECC(input []byte) []byte { func (c *CRC16) CheckECC(input []byte) ([]byte, error) { table := c.getTable() - if len(input) <= crc16Size { - return nil, errors.New("input too short, no checksum present") + if len(input) <= crc16ByteCount { + return nil, errInputTooShort } - cut := len(input) - crc16Size + cut := len(input) - crc16ByteCount data, check := input[:cut], input[cut:] crc := binary.BigEndian.Uint16(check) calc := crc16.Checksum(data, table) if crc != calc { - return nil, errors.New("Checksum does not match") + return nil, errChecksumDoesntMatch } return data, nil } func (c *CRC16) getTable() *crc16.Table { - if c.table == nil { - if c.Poly == 0 { - c.Poly = crc16.IBM - } - c.table = crc16.MakeTable(c.Poly) + if c.table != nil { + return c.table + } + if c.Poly == 0 { + c.Poly = crc16.IBM } + c.table = crc16.MakeTable(c.Poly) return c.table } @@ -95,7 +99,7 @@ type CRC32 struct { table *crc32.Table } -var _ ECC = &CRC32{} +var _ ECC = (*CRC32)(nil) func NewIEEECRC32() *CRC32 { return &CRC32{Poly: crc32.IEEE} @@ -126,14 +130,14 @@ func (c *CRC32) CheckECC(input []byte) ([]byte, error) { table := c.getTable() if len(input) <= crc32.Size { - return nil, errors.New("input too short, no checksum present") + return nil, errInputTooShort } cut := len(input) - crc32.Size data, check := input[:cut], input[cut:] crc := binary.BigEndian.Uint32(check) calc := crc32.Checksum(data, table) if crc != calc { - return nil, errors.New("Checksum does not match") + return nil, errChecksumDoesntMatch } return data, nil } @@ -154,7 +158,7 @@ type CRC64 struct { table *crc64.Table } -var _ ECC = &CRC64{} +var _ ECC = (*CRC64)(nil) func NewISOCRC64() *CRC64 { return &CRC64{Poly: crc64.ISO} @@ -181,14 +185,14 @@ func (c *CRC64) CheckECC(input []byte) ([]byte, error) { table := c.getTable() if len(input) <= crc64.Size { - return nil, errors.New("input too short, no checksum present") + return nil, errInputTooShort } cut := len(input) - crc64.Size data, check := input[:cut], input[cut:] crc := binary.BigEndian.Uint64(check) calc := crc64.Checksum(data, table) if crc != calc { - return nil, errors.New("Checksum does not match") + return nil, errChecksumDoesntMatch } return data, nil } diff --git a/keys/ecc_test.go b/keys/ecc_test.go index 930422b98..d6b536aaa 100644 --- a/keys/ecc_test.go +++ b/keys/ecc_test.go @@ -8,21 +8,22 @@ import ( cmn "github.com/tendermint/tmlibs/common" ) +var codecs = []ECC{ + NewIBMCRC16(), + NewSCSICRC16(), + NewCCITTCRC16(), + NewIEEECRC32(), + NewCastagnoliCRC32(), + NewKoopmanCRC32(), + NewISOCRC64(), + NewECMACRC64(), +} + // TestECCPasses makes sure that the AddECC/CheckECC methods are symetric func TestECCPasses(t *testing.T) { assert := assert.New(t) - checks := []ECC{ - NoECC{}, - NewIBMCRC16(), - NewSCSICRC16(), - NewCCITTCRC16(), - NewIEEECRC32(), - NewCastagnoliCRC32(), - NewKoopmanCRC32(), - NewISOCRC64(), - NewECMACRC64(), - } + checks := append(codecs, NoECC{}) for _, check := range checks { for i := 0; i < 2000; i++ { @@ -42,17 +43,7 @@ func TestECCPasses(t *testing.T) { func TestECCFails(t *testing.T) { assert := assert.New(t) - checks := []ECC{ - NewIBMCRC16(), - NewSCSICRC16(), - NewCCITTCRC16(), - NewIEEECRC32(), - NewCastagnoliCRC32(), - NewKoopmanCRC32(), - NewISOCRC64(), - NewECMACRC64(), - } - + checks := codecs attempts := 2000 for _, check := range checks {