MB-65568: Compaction removes 'Unencrypted' key

When running "compaction" and encryption is turned off it'll
remove the "unencrypted" key from the internal cache. If one
enable encryption at a later time we start reporting the
current active key, but given we've removed the unencrypted
key we would only return the active key to ns_server and it
would think that all data is encrypted with that key

Change-Id: Ie70ee7b58658177f2bbe0cbc5b9eaa1eece70be2
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/224537
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dan Owen <owend@couchbase.com>

Author: trondn

engines/ep/src/kvstore/couch-kvstore/couch-kvstore.cc

@@ -1556,7 +1556,10 @@ CompactDBStatus CouchKVStore::compactDBInternal(
                     if (ret) {
                         vbucketEncryptionKeysManager.setNextKey(vbid, ret->id);
                     } else {
-                        vbucketEncryptionKeysManager.removeNextKey(vbid);
+                        vbucketEncryptionKeysManager.setNextKey(
+                                vbid,
+                                cb::crypto::DataEncryptionKey::
+                                        UnencryptedKeyId);
                     }
                     return ret;
                 },

engines/ewouldblock_engine/ewouldblock_engine.cc

@@ -249,6 +249,8 @@ class EWB_Engine : public EngineIface,
     cb::engine_errc wait_for_seqno_persistence(CookieIface& cookie,
                                                uint64_t seqno,
                                                Vbid vbid) override;
+    cb::engine_errc set_active_encryption_keys(
+            const nlohmann::json& json) override;
     cb::engine_errc prepare_snapshot(
             CookieIface& cookie,
             Vbid vbid,
@@ -1414,6 +1416,12 @@ cb::engine_errc EWB_Engine::wait_for_seqno_persistence(CookieIface& cookie,
                                                        Vbid vbid) {
     return real_engine->wait_for_seqno_persistence(cookie, seqno, vbid);
 }
+
+cb::engine_errc EWB_Engine::set_active_encryption_keys(
+        const nlohmann::json& json) {
+    return real_engine->set_active_encryption_keys(json);
+}
+
 cb::engine_errc EWB_Engine::prepare_snapshot(
         CookieIface& cookie,
         Vbid vbid,

tests/testapp/testapp_encryption.cc

@@ -210,3 +210,41 @@ TEST_P(EncryptionTest, TestPruneDeks) {
     ASSERT_EQ(stats["@logs"].front().get<std::string>(),
               dekManager.lookup(cb::dek::Entity::Logs)->getId());
 }
+
+TEST_P(EncryptionTest, TestDisableEncryption) {
+    // Verify that the bucket is encrypted
+    auto connection = adminConnection->clone();
+    connection->authenticate("@admin");
+    connection->selectBucket(bucketName);
+
+    nlohmann::json stats;
+    connection->stats(
+            [&stats](auto& k, auto& v) { stats = nlohmann::json::parse(v); },
+            "encryption-key-ids");
+
+    ASSERT_EQ(1, stats.size()) << stats.dump();
+    ASSERT_NE("unencrypted", stats.front().get<std::string>());
+
+    // Disable encryption
+    mcd_env->getTestBucket().keystore.setActiveKey({});
+    std::string config =
+            nlohmann::json(mcd_env->getTestBucket().keystore).dump();
+
+    auto rsp = connection->execute(BinprotGenericCommand{
+            cb::mcbp::ClientOpcode::SetActiveEncryptionKeys,
+            bucketName,
+            config});
+    ASSERT_EQ(cb::mcbp::Status::Success, rsp.getStatus());
+
+    // Compact the vbucket
+    rsp = connection->execute(BinprotCompactDbCommand{});
+    ASSERT_EQ(cb::mcbp::Status::Success, rsp.getStatus());
+
+    // fetch the stats and it should be "unencrypted"
+    connection->stats(
+            [&stats](auto& k, auto& v) { stats = nlohmann::json::parse(v); },
+            "encryption-key-ids");
+
+    ASSERT_EQ(1, stats.size()) << stats.dump();
+    ASSERT_EQ("unencrypted", stats.front().get<std::string>());
+}

tests/testapp/testapp_environment.cc

@@ -122,7 +122,7 @@ void TestBucketImpl::setParam(
 TestBucketImpl::TestBucketImpl(const std::filesystem::path& test_directory,
                                std::string extraConfig)
     : dbPath(test_directory / "dbase"), extraConfig(std::move(extraConfig)) {
-    encryption_keys.emplace_back(cb::crypto::DataEncryptionKey::generate());
+    keystore.setActiveKey(cb::crypto::DataEncryptionKey::generate());
 }
 
 void TestBucketImpl::createBucket(const std::string& name,
@@ -211,13 +211,7 @@ bool TestBucketImpl::isFullEviction() const {
 }
 
 std::string TestBucketImpl::getEncryptionConfig() const {
-    nlohmann::json encryption = {{"active", encryption_keys.front()->id}};
-    auto keys = nlohmann::json::array();
-    for (const auto& key : encryption_keys) {
-        keys.push_back(*key);
-    }
-    encryption["keys"] = std::move(keys);
-    return encryption.dump();
+    return nlohmann::json(keystore).dump();
 }
 
 McdEnvironment::McdEnvironment(std::string engineConfig)

tests/testapp/testapp_environment.h

@@ -144,6 +144,9 @@ class TestBucketImpl {
         return extraConfig;
     }
 
+    /// The keystore used to keep track of the encrypt keys for the bucket
+    cb::crypto::KeyStore keystore;
+
 protected:
     static void createEwbBucket(const std::string& name,
                                 BucketType type,
@@ -159,9 +162,6 @@ class TestBucketImpl {
 
     std::string extraConfig;
 
-    /// The key to use for encryption@rest
-    std::vector<std::shared_ptr<cb::crypto::DataEncryptionKey>> encryption_keys;
-
     BucketCreateMode bucketCreateMode = BucketCreateMode::Clean;
 };