diff --git a/src/cluster.c b/src/cluster.c index b8316100c5..9165a6f031 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -166,11 +166,7 @@ int verifyDumpPayload(unsigned char *p, size_t len, uint16_t *rdbver_ptr) { if (rdbver_ptr) { *rdbver_ptr = rdbver; } - if ((rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) || - (rdbver > RDB_VERSION && server.rdb_version_check == RDB_VERSION_CHECK_STRICT)) { - return C_ERR; - } - + if (!rdbIsVersionAccepted(rdbver, false, false)) return C_ERR; if (server.skip_checksum_validation) return C_OK; /* Verify CRC64 */ @@ -203,6 +199,7 @@ void dumpCommand(client *c) { /* RESTORE key ttl serialized-value [REPLACE] [ABSTTL] [IDLETIME seconds] [FREQ frequency] */ void restoreCommand(client *c) { long long ttl, lfu_freq = -1, lru_idle = -1, lru_clock = -1; + uint16_t rdbver = 0; rio payload; int j, type, replace = 0, absttl = 0; robj *obj; @@ -251,14 +248,27 @@ void restoreCommand(client *c) { } /* Verify RDB version and data checksum. */ - if (verifyDumpPayload(c->argv[3]->ptr, sdslen(c->argv[3]->ptr), NULL) == C_ERR) { + if (verifyDumpPayload(c->argv[3]->ptr, sdslen(c->argv[3]->ptr), &rdbver) == C_ERR) { addReplyError(c, "DUMP payload version or checksum are wrong"); return; } rioInitWithBuffer(&payload, c->argv[3]->ptr); - if (((type = rdbLoadObjectType(&payload)) == -1) || - ((obj = rdbLoadObject(type, &payload, key->ptr, c->db->id, NULL)) == NULL)) { + type = rdbLoadObjectType(&payload); + if (type == -1) { + addReplyError(c, "Bad data format"); + return; + } + + /* If it's a foreign RDB format, only accept old data types that we know + * existed in the past and that don't clash with new types added later. */ + if (rdbIsForeignVersion(rdbver) && type >= RDB_FOREIGN_TYPE_MIN) { + addReplyErrorFormat(c, "Unsupported foreign data type: %d", type); + return; + } + + obj = rdbLoadObject(type, &payload, key->ptr, c->db->id, NULL); + if (obj == NULL) { addReplyError(c, "Bad data format"); return; } diff --git a/src/rdb.c b/src/rdb.c index 55d6e95f6f..b04c01949f 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -76,6 +76,22 @@ void rdbCheckError(const char *fmt, ...); void rdbCheckSetError(const char *fmt, ...); int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadingCtx *rdb_loading_ctx); +/* Returns true if the RDB version is valid and accepted, false otherwise. This + * function takes configuration into account. The parameter `is_valkey_magic` + * indicates that an RDB file with the VALKEY magic string was parsed. + * `is_redis_magic` indicates a legacy RDB file with the REDIS magic string. + * When there is no magic string such as in DUMP/RESTORE, set both to false. */ +bool rdbIsVersionAccepted(int rdbver, bool is_valkey_magic, bool is_redis_magic) { + if (rdbver < 1) return false; + if (is_valkey_magic && rdbver <= RDB_FOREIGN_VERSION_MAX) return false; + if (is_redis_magic && rdbver > RDB_FOREIGN_VERSION_MAX) return false; + if (server.rdb_version_check == RDB_VERSION_CHECK_STRICT) { + if (rdbver > RDB_VERSION) return false; /* future version */ + if (rdbIsForeignVersion(rdbver)) return false; + } + return true; +} + #ifdef __GNUC__ void rdbReportError(int corruption_error, int linenum, char *reason, ...) __attribute__((format(printf, 3, 4))); #endif @@ -2875,6 +2891,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { return NULL; } o = createModuleObject(mt, ptr); + } else if (server.rdb_version_check == RDB_VERSION_CHECK_RELAXED) { + /* Future or foreign type. Don't report it as an internal error. */ + if (error) *error = RDB_LOAD_ERR_UNKNOWN_TYPE; + return NULL; } else { rdbReportReadError("Unknown RDB encoding type %d", rdbtype); return NULL; @@ -3084,10 +3104,7 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin return C_ERR; } rdbver = atoi(buf + 6); - if (rdbver < 1 || (rdbver < RDB_FOREIGN_VERSION_MIN && !is_redis_magic) || - (rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) || - (rdbver > RDB_FOREIGN_VERSION_MAX && !is_valkey_magic) || - (rdbver > RDB_VERSION && server.rdb_version_check == RDB_VERSION_CHECK_STRICT)) { + if (!rdbIsVersionAccepted(rdbver, is_valkey_magic, is_redis_magic)) { serverLog(LL_WARNING, "Can't handle RDB format version %d", rdbver); return C_ERR; } @@ -3103,6 +3120,13 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin /* Read type. */ if ((type = rdbLoadType(rdb)) == -1) goto eoferr; + /* Safeguard for unknown foreign opcode interpretations. */ + if (is_redis_magic && type >= RDB_FOREIGN_TYPE_MIN && type <= RDB_FOREIGN_TYPE_MAX) { + serverLog(LL_WARNING, "Can't handle foreign type or opcode %d in RDB with version %d", + type, rdbver); + return C_ERR; + } + /* Handle special types. */ if (type == RDB_OPCODE_EXPIRETIME) { /* EXPIRETIME: load an expire associated with the next key @@ -3155,6 +3179,19 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin if ((expires_size = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr; should_expand_db = 1; continue; /* Read next opcode. */ + } else if (type == RDB_OPCODE_SLOT_INFO) { + /* RDB slot info size annotations used in pre-8.0 and foreign RDB. + * See the aux field "slot-info". */ + uint64_t slot_id, slot_size, expires_slot_size; + if ((slot_id = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr; + if ((slot_size = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr; + if ((expires_slot_size = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr; + if (server.cluster_enabled && slot_id < CLUSTER_SLOTS) { + if (slot_size) kvstoreHashtableExpand(db->keys, slot_id, slot_size); + if (expires_slot_size) kvstoreHashtableExpand(db->expires, slot_id, expires_slot_size); + should_expand_db = 0; + } + continue; /* Read next opcode. */ } else if (type == RDB_OPCODE_AUX) { /* AUX: generic string-string fields. Use to add state to RDB * which is backward compatible. Implementations of RDB loading @@ -3215,7 +3252,7 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin goto eoferr; } - if (server.cluster_enabled) { + if (server.cluster_enabled && slot_id >= 0 && slot_id < CLUSTER_SLOTS) { /* In cluster mode we resize individual slot specific dictionaries based on the number of keys that * slot holds. */ if (slot_size) kvstoreHashtableExpand(db->keys, slot_id, slot_size); @@ -3352,6 +3389,10 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin if (error == RDB_LOAD_ERR_EMPTY_KEY) { if (empty_keys_skipped++ < 10) serverLog(LL_NOTICE, "rdbLoadObject skipping empty key: %s", key); sdsfree(key); + } else if (error == RDB_LOAD_ERR_UNKNOWN_TYPE) { + sdsfree(key); + serverLog(LL_WARNING, "Unknown type or opcode when loading DB. Unrecoverable error, aborting now."); + return C_ERR; } else { sdsfree(key); goto eoferr; diff --git a/src/rdb.h b/src/rdb.h index 56bb74e9f4..0b76b1a416 100644 --- a/src/rdb.h +++ b/src/rdb.h @@ -57,6 +57,10 @@ static_assert(RDB_VERSION < RDB_FOREIGN_VERSION_MIN || RDB_VERSION > RDB_FOREIGN_VERSION_MAX, "RDB version in foreign version range"); +static inline bool rdbIsForeignVersion(int rdbver) { + return rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX; +} + /* Defines related to the dump file format. To store 32 bits lengths for short * keys requires a lot of space, so we check the most significant 2 bits of * the first byte to interpreter the length: @@ -112,11 +116,16 @@ enum RdbType { RDB_TYPE_STREAM_LISTPACKS_2 = 19, RDB_TYPE_SET_LISTPACK = 20, RDB_TYPE_STREAM_LISTPACKS_3 = 21, - RDB_TYPE_HASH_2 = 22, + RDB_TYPE_HASH_2 = 22, /* Hash with field-level expiration (Valkey 9.0) */ RDB_TYPE_LAST }; /* NOTE: WHEN ADDING NEW RDB TYPE, UPDATE rdb_type_string[] */ +/* When our RDB format diverges, we need to reject types/opcodes for which we + * may have assigned a different meaning compared to other implementations. */ +#define RDB_FOREIGN_TYPE_MIN 22 +#define RDB_FOREIGN_TYPE_MAX 243 + typedef int (*ChildSnapshotFunc)(int req, rio *rdb, void *privdata); typedef struct rdbSnapshotOptions { int connsnum; /* Number of connections. */ @@ -131,7 +140,9 @@ typedef struct rdbSnapshotOptions { /* Test if a type is an object type. */ #define rdbIsObjectType(t) (((t) >= 0 && (t) <= 7) || ((t) >= 9 && (t) < RDB_TYPE_LAST)) -/* Special RDB opcodes (saved/loaded with rdbSaveType/rdbLoadType). */ +/* Special RDB opcodes (saved/loaded with rdbSaveType/rdbLoadType). + * These are special RDB types, but they start from 255 and grow down. */ +#define RDB_OPCODE_SLOT_INFO 244 /* Foreign slot info, safe to ignore. */ #define RDB_OPCODE_FUNCTION2 245 /* function library data */ #define RDB_OPCODE_FUNCTION_PRE_GA 246 /* old function library data for 7.0 rc1 and rc2 */ #define RDB_OPCODE_MODULE_AUX 247 /* Module auxiliary data. */ @@ -168,9 +179,11 @@ typedef struct rdbSnapshotOptions { /* When rdbLoadObject() returns NULL, the err flag is * set to hold the type of error that occurred */ -#define RDB_LOAD_ERR_EMPTY_KEY 1 /* Error of empty key */ -#define RDB_LOAD_ERR_OTHER 2 /* Any other errors */ +#define RDB_LOAD_ERR_EMPTY_KEY 1 /* Error of empty key */ +#define RDB_LOAD_ERR_UNKNOWN_TYPE 2 /* Unknown type in file */ +#define RDB_LOAD_ERR_OTHER 3 /* Any other errors */ +bool rdbIsVersionAccepted(int rdbver, bool is_valkey_magic, bool is_redis_magic); ssize_t rdbWriteRaw(rio *rdb, void *p, size_t len); int rdbSaveType(rio *rdb, unsigned char type); int rdbLoadType(rio *rdb); diff --git a/src/valkey-check-rdb.c b/src/valkey-check-rdb.c index 7a6cd97658..60a33f2427 100644 --- a/src/valkey-check-rdb.c +++ b/src/valkey-check-rdb.c @@ -76,6 +76,7 @@ struct { rio *rio; robj *key; /* Current key we are reading. */ int key_type; /* Current key type if != -1. */ + int rdbver; /* RDB version. */ unsigned long keys; /* Number of keys processed. */ unsigned long expires; /* Number of keys with an expire. */ unsigned long already_expired; /* Number of keys already expired. */ @@ -625,13 +626,17 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) { goto err; } rdbver = atoi(buf + 6); - if (rdbver < 1 || rdbver > RDB_VERSION || + if (rdbver < 1 || (rdbver < RDB_FOREIGN_VERSION_MIN && !is_redis_magic) || - (rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) || (rdbver > RDB_FOREIGN_VERSION_MAX && !is_valkey_magic)) { rdbCheckError("Can't handle RDB format version %d", rdbver); goto err; + } else if (rdbver > RDB_VERSION) { + rdbCheckInfo("Future RDB version %d detected", rdbver); + } else if (rdbIsForeignVersion(rdbver)) { + rdbCheckInfo("Foreign RDB version %d detected", rdbver); } + rdbstate.rdbver = rdbver; expiretime = -1; while (1) { @@ -689,6 +694,12 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) { if ((db_size = rdbLoadLen(&rdb, NULL)) == RDB_LENERR) goto eoferr; if ((expires_size = rdbLoadLen(&rdb, NULL)) == RDB_LENERR) goto eoferr; continue; /* Read type again. */ + } else if (type == RDB_OPCODE_SLOT_INFO) { + /* Hint used in foreign RDB versions. */ + if (rdbLoadLen(&rdb, NULL) == RDB_LENERR) goto eoferr; + if (rdbLoadLen(&rdb, NULL) == RDB_LENERR) goto eoferr; + if (rdbLoadLen(&rdb, NULL) == RDB_LENERR) goto eoferr; + continue; /* Read type again. */ } else if (type == RDB_OPCODE_AUX) { /* AUX: generic string-string fields. Use to add state to RDB * which is backward compatible. Implementations of RDB loading @@ -742,11 +753,19 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) { } rdbstate.functions_num++; continue; - } else { - if (!rdbIsObjectType(type)) { + } else if (rdbIsForeignVersion(rdbver) && + type >= RDB_FOREIGN_TYPE_MIN && + type <= RDB_FOREIGN_TYPE_MAX) { + rdbCheckError("Unknown object type %d in RDB file with foreign version %d", type, rdbver); + goto err; + } else if (!rdbIsObjectType(type)) { + if (rdbver > RDB_VERSION) { + rdbCheckError("Unknown object type %d in RDB file with future version %d", type, rdbver); + } else { rdbCheckError("Invalid object type: %d", type); - goto err; } + goto err; + } else { rdbstate.key_type = type; } @@ -894,8 +913,13 @@ int redis_check_rdb_main(int argc, char **argv, FILE *fp) { rdbCheckInfo("Checking RDB file %s", argv[1]); rdbCheckSetupSignals(); int retval = redis_check_rdb(argv[1], fp); + rdbCheckInfo("Check RDB returned error code %d (0 means success)", retval); if (retval == 0) { - rdbCheckInfo("\\o/ RDB looks OK! \\o/"); + if (rdbIsForeignVersion(rdbstate.rdbver) || rdbstate.rdbver > RDB_VERSION) { + rdbCheckInfo("\\o/ RDB looks OK, but loading requires config 'rdb-version-check relaxed'"); + } else { + rdbCheckInfo("\\o/ RDB looks OK! \\o/"); + } rdbShowGenericInfo(); } if (fp) return (retval == 0) ? C_OK : C_ERR; diff --git a/tests/assets/encodings-rdb12.rdb b/tests/assets/encodings-rdb12.rdb new file mode 100644 index 0000000000..9988c1fbb5 Binary files /dev/null and b/tests/assets/encodings-rdb12.rdb differ diff --git a/tests/assets/encodings-rdb75-unknown-types.rdb b/tests/assets/encodings-rdb75-unknown-types.rdb new file mode 100644 index 0000000000..12f8c8969f Binary files /dev/null and b/tests/assets/encodings-rdb75-unknown-types.rdb differ diff --git a/tests/assets/encodings-rdb987-unknown-types.rdb b/tests/assets/encodings-rdb987-unknown-types.rdb new file mode 100644 index 0000000000..516c83dc75 Binary files /dev/null and b/tests/assets/encodings-rdb987-unknown-types.rdb differ diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index cd7ef0674c..efc304f284 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -15,7 +15,10 @@ set server_path [tmpdir "server.rdb-encoding-test"] # Copy RDB with different encodings in server path exec cp tests/assets/encodings.rdb $server_path +exec cp tests/assets/encodings-rdb12.rdb $server_path +exec cp tests/assets/encodings-rdb75-unknown-types.rdb $server_path exec cp tests/assets/encodings-rdb987.rdb $server_path +exec cp tests/assets/encodings-rdb987-unknown-types.rdb $server_path exec cp tests/assets/list-quicklist.rdb $server_path start_server [list overrides [list "dir" $server_path "dbfilename" "list-quicklist.rdb" save ""]] { @@ -55,7 +58,7 @@ start_server_and_kill_it [list "dir" $server_path "dbfilename" "encodings-rdb987 [string match {*Fatal error loading*} \ [exec tail -1 < [dict get $srv stdout]]] } else { - fail "Server started even if RDB version check failed" + fail "Server started even though RDB version is unsupported" } } } @@ -69,6 +72,42 @@ start_server [list overrides [list "dir" $server_path \ } $csv_dump } +start_server_and_kill_it [list dir $server_path \ + dbfilename "encodings-rdb987-unknown-types.rdb" \ + rdb-version-check relaxed] { + test "RDB future version loading with unknown types, relaxed version check" { + wait_for_condition 50 100 { + [string match {*Unknown type or opcode when loading DB. Unrecoverable error, aborting now.*} \ + [exec tail -2 < [dict get $srv stdout]]] + } else { + fail "Server started even though RDB contains unknown types" + } + } +} + +start_server [list overrides [list dir $server_path \ + dbfilename "encodings-rdb12.rdb" \ + rdb-version-check relaxed]] { + test "RDB foreign version loading, relaxed version check" { + r select 0 + assert_equal foo [r keys *] + assert_equal bar [r get foo] + } +} + +start_server_and_kill_it [list dir $server_path \ + dbfilename "encodings-rdb75-unknown-types.rdb" \ + rdb-version-check relaxed] { + test "RDB foreign version loading with unknown types, relaxed version check" { + wait_for_condition 50 100 { + [string match {*Can't handle foreign type or opcode 150 in RDB with version 75*} \ + [exec tail -2 < [dict get $srv stdout]]] + } else { + fail "Server started even though RDB contains unknown types" + } + } +} + set server_path [tmpdir "server.rdb-startup-test"] start_server [list overrides [list "dir" $server_path] keep_persistence true] { diff --git a/tests/integration/valkey-check-rdb.tcl b/tests/integration/valkey-check-rdb.tcl index e52eaf64f4..ee5783e33e 100644 --- a/tests/integration/valkey-check-rdb.tcl +++ b/tests/integration/valkey-check-rdb.tcl @@ -2,6 +2,51 @@ proc get_function_code {args} { return [format "#!%s name=%s\nserver.register_function('%s', function(KEYS, ARGV)\n %s \nend)" [lindex $args 0] [lindex $args 1] [lindex $args 2] [lindex $args 3]] } +tags {"check-rdb external:skip logreqres:skip"} { + test {Check old valid RDB} { + catch { + exec src/valkey-check-rdb tests/assets/encodings.rdb + } result + assert_match {*\[offset ???\] \\o/ RDB looks OK! \\o/*} $result + } + + test {Check foreign RDB without unknown data} { + catch { + exec src/valkey-check-rdb tests/assets/encodings-rdb12.rdb + } result + assert_match {*\[offset ?\] Foreign RDB version 12 detected*} $result + assert_match {*\[offset ???\] \\o/ RDB looks OK, but loading requires config 'rdb-version-check relaxed'*} $result + } + + test {Check foreign RDB with unknown data} { + catch { + exec src/valkey-check-rdb tests/assets/encodings-rdb75-unknown-types.rdb + } result + assert_match {*\[offset ?\] Foreign RDB version 75 detected*} $result + assert_match {*--- RDB ERROR DETECTED ---*} $result + assert_match {*\[offset ??\] Unknown object type 150 in RDB file with foreign version 75*} $result + assert_no_match {*RDB looks OK*} $result + } + + test {Check future RDB without unknown data} { + catch { + exec src/valkey-check-rdb tests/assets/encodings-rdb987.rdb + } result + assert_match {*\[offset ?\] Future RDB version 987 detected*} $result + assert_match {*\[offset ???\] \\o/ RDB looks OK, but loading requires config 'rdb-version-check relaxed'*} $result + } + + test {Check future RDB with unknown data} { + catch { + exec src/valkey-check-rdb tests/assets/encodings-rdb987-unknown-types.rdb + } result + assert_match {*\[offset ?\] Future RDB version 987 detected*} $result + assert_match {*--- RDB ERROR DETECTED ---*} $result + assert_match {*\[offset ??\] Unknown object type 150 in RDB file with future version 987*} $result + assert_no_match {*RDB looks OK*} $result + } +} + tags {"check-rdb network external:skip logreqres:skip"} { start_server {} { test "test valkey-check-rdb stats with empty RDB" { @@ -96,4 +141,4 @@ tags {"check-rdb network external:skip logreqres:skip"} { assert_match "*db.5.type.stream.keys.total:10*" $result } } -} \ No newline at end of file +}