From 44992fb486dd7b54ff135b3bda13eeeec6c4d2ce Mon Sep 17 00:00:00 2001 From: "aleksey.mochalov" Date: Tue, 13 Dec 2022 17:35:20 +0300 Subject: [PATCH 1/2] Fix resetting and accidental key damaging/deleting (#7415) This should add key correctness check in case of resetting encryption key on Classic server with multiple attachments, and hanging Super server in case of accidental key damaging/deleting --- src/jrd/CryptoManager.cpp | 91 ++++++++++++++++++++++++++++++++++----- 1 file changed, 80 insertions(+), 11 deletions(-) diff --git a/src/jrd/CryptoManager.cpp b/src/jrd/CryptoManager.cpp index a54a14f953c..dbf34de6aeb 100644 --- a/src/jrd/CryptoManager.cpp +++ b/src/jrd/CryptoManager.cpp @@ -368,7 +368,7 @@ namespace Jrd { // tdbb w/o attachment comes when database is shutting down in the end of detachDatabase() // the only needed here page is header, i.e. we can live w/o cryptPlugin - if ((crypt || process) && (!cryptPlugin) && tdbb->getAttachment()) + if ((crypt || process) && tdbb->getAttachment()) { ClumpletWriter hc(ClumpletWriter::UnTagged, hdr->hdr_page_size); hdr.getClumplets(hc); @@ -377,18 +377,58 @@ namespace Jrd { else keyName = ""; - loadPlugin(tdbb, hdr->hdr_crypt_plugin); - - string valid; - calcValidation(valid, cryptPlugin); - if (hc.find(Ods::HDR_crypt_hash)) + if (!cryptPlugin) { - hc.getString(hash); - if (hash != valid) - (Arg::Gds(isc_bad_crypt_key) << keyName).raise(); + loadPlugin(tdbb, hdr->hdr_crypt_plugin); + string valid; + calcValidation(valid, cryptPlugin); + if (hc.find(Ods::HDR_crypt_hash)) + { + hc.getString(hash); + if (hash != valid) + (Arg::Gds(isc_bad_crypt_key) << keyName).raise(); + } + else + hash = valid; } else - hash = valid; + { + if (!keyName.isEmpty()) + { + for (GetPlugins keyControl(IPluginManager::TYPE_KEY_HOLDER, dbb.dbb_config); + keyControl.hasData(); keyControl.next()) + { + // check does keyHolder want to provide a key for us + IKeyHolderPlugin* keyHolder = keyControl.plugin(); + + FbLocalStatus st; + int keyCallbackRc = keyHolder->keyCallback(&st, tdbb->getAttachment()->att_crypt_callback); + st.check(); + if (!keyCallbackRc) + continue; + + // validate a key + AutoPlugin crypt(checkFactory->makeInstance()); + setDbInfo(crypt); + crypt->setKey(&st, 1, &keyHolder, keyName.c_str()); + + + string valid; + calcValidation(valid, crypt); + if (hc.find(Ods::HDR_crypt_hash)) + { + hc.getString(hash); + if (hash == valid) + { + // unload old plugin and set new one + PluginManagerInterfacePtr()->releasePlugin(cryptPlugin); + cryptPlugin = NULL; + cryptPlugin = crypt.release(); + } + } + } + } + } } if (cryptPlugin && (flags & CRYPT_HDR_INIT)) @@ -495,7 +535,7 @@ namespace Jrd { checkFactory = NULL; // store new one - if (dbb.dbb_config->getServerMode() == MODE_SUPER && !holderLess) + if (!holderLess) checkFactory = cryptControl.release(); } @@ -670,7 +710,36 @@ namespace Jrd { } } else + { + for (GetPlugins keyControl(IPluginManager::TYPE_KEY_HOLDER, dbb.dbb_config); + keyControl.hasData(); keyControl.next()) + { + // check does keyHolder want to provide a key for us + IKeyHolderPlugin* keyHolder = keyControl.plugin(); + + FbLocalStatus st; + int keyCallbackRc = keyHolder->keyCallback(&st, tdbb->getAttachment()->att_crypt_callback); + st.check(); + if (!keyCallbackRc) + continue; + + // validate a key + AutoPlugin crypt(checkFactory->makeInstance()); + setDbInfo(crypt); + crypt->setKey(&st, 1, &keyHolder, keyName.c_str()); + + + string valid; + calcValidation(valid, crypt); + if (hc.find(Ods::HDR_crypt_hash)) + { + hc.getString(hash); + if (hash != valid) + (Arg::Gds(isc_bad_crypt_key) << keyName).raise(); + } + } header->hdr_flags &= ~Ods::hdr_encrypted; + } hdr.setClumplets(hc); From 337a45d3ef810c90bf6dda7e074ec873dd9b5c2e Mon Sep 17 00:00:00 2001 From: "aleksey.mochalov" Date: Tue, 20 Dec 2022 17:49:32 +0300 Subject: [PATCH 2/2] Remove redundant key name existence check. KeyHolder can change even unnamed key --- src/jrd/CryptoManager.cpp | 49 ++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/jrd/CryptoManager.cpp b/src/jrd/CryptoManager.cpp index dbf34de6aeb..aaec8b0b2ee 100644 --- a/src/jrd/CryptoManager.cpp +++ b/src/jrd/CryptoManager.cpp @@ -393,38 +393,35 @@ namespace Jrd { } else { - if (!keyName.isEmpty()) + for (GetPlugins keyControl(IPluginManager::TYPE_KEY_HOLDER, dbb.dbb_config); + keyControl.hasData(); keyControl.next()) { - for (GetPlugins keyControl(IPluginManager::TYPE_KEY_HOLDER, dbb.dbb_config); - keyControl.hasData(); keyControl.next()) - { - // check does keyHolder want to provide a key for us - IKeyHolderPlugin* keyHolder = keyControl.plugin(); + // check does keyHolder want to provide a key for us + IKeyHolderPlugin* keyHolder = keyControl.plugin(); - FbLocalStatus st; - int keyCallbackRc = keyHolder->keyCallback(&st, tdbb->getAttachment()->att_crypt_callback); - st.check(); - if (!keyCallbackRc) - continue; + FbLocalStatus st; + int keyCallbackRc = keyHolder->keyCallback(&st, tdbb->getAttachment()->att_crypt_callback); + st.check(); + if (!keyCallbackRc) + continue; - // validate a key - AutoPlugin crypt(checkFactory->makeInstance()); - setDbInfo(crypt); - crypt->setKey(&st, 1, &keyHolder, keyName.c_str()); + // validate a key + AutoPlugin crypt(checkFactory->makeInstance()); + setDbInfo(crypt); + crypt->setKey(&st, 1, &keyHolder, keyName.c_str()); - string valid; - calcValidation(valid, crypt); - if (hc.find(Ods::HDR_crypt_hash)) + string valid; + calcValidation(valid, crypt); + if (hc.find(Ods::HDR_crypt_hash)) + { + hc.getString(hash); + if (hash == valid) { - hc.getString(hash); - if (hash == valid) - { - // unload old plugin and set new one - PluginManagerInterfacePtr()->releasePlugin(cryptPlugin); - cryptPlugin = NULL; - cryptPlugin = crypt.release(); - } + // unload old plugin and set new one + PluginManagerInterfacePtr()->releasePlugin(cryptPlugin); + cryptPlugin = NULL; + cryptPlugin = crypt.release(); } } }