From dd4fee88ef6e4e0e5cf8eab0bf5a3e078df355c9 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 4 May 2022 11:08:26 -0700 Subject: [PATCH] keymigrate: improve filtering for legacy transaction hashes (#8466) This is a follow-up to #8352. The check for legacy evidence keys is only based on the prefix of the key. Hashes, which are unprefixed, could easily have this form and be misdiagnosed. Because the conversion for evidence checks the key structure, this should not cause corruption. The probability that a hash is a syntactically valid evidence key is negligible. The tool will report an error rather than storing bad data. But this does mean that such transaction hashes could cause the migration to stop and report an error before it is complete. To ensure we convert all the data, refine the legacy key check to filter these keys more precisely. Update the test cases to exercise this condition. * Update upgrading instructions. --- UPGRADING.md | 21 ++++++----- scripts/keymigrate/migrate.go | 60 +++++++++++++++++++++++------- scripts/keymigrate/migrate_test.go | 16 ++++++-- 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 28e44e58c..93cd6c20f 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -212,22 +212,25 @@ and one function have moved to the Tendermint `crypto` package: The format of all tendermint on-disk database keys changes in 0.35. Upgrading nodes must either re-sync all data or run a migration -script provided in this release. The script located in -`github.com/tendermint/tendermint/scripts/keymigrate/migrate.go` -provides the function `Migrate(context.Context, db.DB)` which you can -operationalize as makes sense for your deployment. +script provided in this release. + +The script located in +`github.com/tendermint/tendermint/scripts/keymigrate/migrate.go` provides the +function `Migrate(context.Context, db.DB)` which you can operationalize as +makes sense for your deployment. For ease of use the `tendermint` command includes a CLI version of the migration script, which you can invoke, as in: tendermint key-migrate -This reads the configuration file as normal and allows the -`--db-backend` and `--db-dir` flags to change database operations as -needed. +This reads the configuration file as normal and allows the `--db-backend` and +`--db-dir` flags to override the database location as needed. -The migration operation is idempotent and can be run more than once, -if needed. +The migration operation is intended to be idempotent, and should be safe to +rerun on the same database multiple times. As a safety measure, however, we +recommend that operators test out the migration on a copy of the database +first, if it is practical to do so, before applying it to the production data. ### CLI Changes diff --git a/scripts/keymigrate/migrate.go b/scripts/keymigrate/migrate.go index ca2c528e2..a0b43aef6 100644 --- a/scripts/keymigrate/migrate.go +++ b/scripts/keymigrate/migrate.go @@ -86,27 +86,30 @@ const ( var prefixes = []struct { prefix []byte ktype keyType + check func(keyID) bool }{ - {[]byte("consensusParamsKey:"), consensusParamsKey}, - {[]byte("abciResponsesKey:"), abciResponsesKey}, - {[]byte("validatorsKey:"), validatorsKey}, - {[]byte("stateKey"), stateStoreKey}, - {[]byte("H:"), blockMetaKey}, - {[]byte("P:"), blockPartKey}, - {[]byte("C:"), commitKey}, - {[]byte("SC:"), seenCommitKey}, - {[]byte("BH:"), blockHashKey}, - {[]byte("size"), lightSizeKey}, - {[]byte("lb/"), lightBlockKey}, - {[]byte("\x00"), evidenceCommittedKey}, - {[]byte("\x01"), evidencePendingKey}, + {[]byte("consensusParamsKey:"), consensusParamsKey, nil}, + {[]byte("abciResponsesKey:"), abciResponsesKey, nil}, + {[]byte("validatorsKey:"), validatorsKey, nil}, + {[]byte("stateKey"), stateStoreKey, nil}, + {[]byte("H:"), blockMetaKey, nil}, + {[]byte("P:"), blockPartKey, nil}, + {[]byte("C:"), commitKey, nil}, + {[]byte("SC:"), seenCommitKey, nil}, + {[]byte("BH:"), blockHashKey, nil}, + {[]byte("size"), lightSizeKey, nil}, + {[]byte("lb/"), lightBlockKey, nil}, + {[]byte("\x00"), evidenceCommittedKey, checkEvidenceKey}, + {[]byte("\x01"), evidencePendingKey, checkEvidenceKey}, } // checkKeyType classifies a candidate key based on its structure. func checkKeyType(key keyID) keyType { for _, p := range prefixes { if bytes.HasPrefix(key, p.prefix) { - return p.ktype + if p.check == nil || p.check(key) { + return p.ktype + } } } @@ -342,6 +345,35 @@ func convertEvidence(key keyID, newPrefix int64) ([]byte, error) { return orderedcode.Append(nil, newPrefix, binary.BigEndian.Uint64(hb), string(evidenceHash)) } +// checkEvidenceKey reports whether a candidate key with one of the legacy +// evidence prefixes has the correct structure for a legacy evidence key. +// +// This check is needed because transaction hashes are stored without a prefix, +// so checking the one-byte prefix alone is not enough to distinguish them. +// Legacy evidence keys are suffixed with a string of the format: +// +// "%0.16X/%X" +// +// where the first element is the height and the second is the hash. Thus, we +// check +func checkEvidenceKey(key keyID) bool { + parts := bytes.SplitN(key[1:], []byte("/"), 2) + if len(parts) != 2 || len(parts[0]) != 16 || !isHex(parts[0]) || !isHex(parts[1]) { + return false + } + return true +} + +func isHex(data []byte) bool { + for _, b := range data { + if ('0' <= b && b <= '9') || ('a' <= b && b <= 'f') || ('A' <= b && b <= 'F') { + continue + } + return false + } + return len(data) != 0 +} + func replaceKey(db dbm.DB, key keyID, gooseFn migrateFunc) error { exists, err := db.Has(key) if err != nil { diff --git a/scripts/keymigrate/migrate_test.go b/scripts/keymigrate/migrate_test.go index b2727a5df..f7322b352 100644 --- a/scripts/keymigrate/migrate_test.go +++ b/scripts/keymigrate/migrate_test.go @@ -1,11 +1,11 @@ package keymigrate import ( - "bytes" "context" "errors" "fmt" "math" + "strings" "testing" "github.com/google/orderedcode" @@ -21,6 +21,7 @@ func makeKey(t *testing.T, elems ...interface{}) []byte { } func getLegacyPrefixKeys(val int) map[string][]byte { + vstr := fmt.Sprintf("%02x", byte(val)) return map[string][]byte{ "Height": []byte(fmt.Sprintf("H:%d", val)), "BlockPart": []byte(fmt.Sprintf("P:%d:%d", val, val)), @@ -40,14 +41,19 @@ func getLegacyPrefixKeys(val int) map[string][]byte { "UserKey1": []byte(fmt.Sprintf("foo/bar/baz/%d/%d", val, val)), "TxHeight": []byte(fmt.Sprintf("tx.height/%s/%d/%d", fmt.Sprint(val), val, val)), "TxHash": append( - bytes.Repeat([]byte{fmt.Sprint(val)[0]}, 16), - bytes.Repeat([]byte{fmt.Sprint(val)[len([]byte(fmt.Sprint(val)))-1]}, 16)..., + []byte(strings.Repeat(vstr[:1], 16)), + []byte(strings.Repeat(vstr[1:], 16))..., ), + + // Transaction hashes that could be mistaken for evidence keys. + "TxHashMimic0": append([]byte{0}, []byte(strings.Repeat(vstr, 16)[:31])...), + "TxHashMimic1": append([]byte{1}, []byte(strings.Repeat(vstr, 16)[:31])...), } } func getNewPrefixKeys(t *testing.T, val int) map[string][]byte { t.Helper() + vstr := fmt.Sprintf("%02x", byte(val)) return map[string][]byte{ "Height": makeKey(t, int64(0), int64(val)), "BlockPart": makeKey(t, int64(1), int64(val), int64(val)), @@ -66,7 +72,9 @@ func getNewPrefixKeys(t *testing.T, val int) map[string][]byte { "UserKey0": makeKey(t, "foo", "bar", int64(val), int64(val)), "UserKey1": makeKey(t, "foo", "bar/baz", int64(val), int64(val)), "TxHeight": makeKey(t, "tx.height", fmt.Sprint(val), int64(val), int64(val+2), int64(val+val)), - "TxHash": makeKey(t, "tx.hash", string(bytes.Repeat([]byte{[]byte(fmt.Sprint(val))[0]}, 32))), + "TxHash": makeKey(t, "tx.hash", strings.Repeat(vstr, 16)), + "TxHashMimic0": makeKey(t, "tx.hash", "\x00"+strings.Repeat(vstr, 16)[:31]), + "TxHashMimic1": makeKey(t, "tx.hash", "\x01"+strings.Repeat(vstr, 16)[:31]), } }