Merge pull request #1249

dird: fix tls reload crash

CHANGELOG.md

@@ -52,6 +52,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https:
 - build: run a build and test with sanitizers enabled [PR #1244]
 
 ### Fixed
+- dird: fix possible crash in tls context on configuration reload [PR #1249]
 - dird: RunScript fixes [PR #1217]
   - fix show command output for RunScript RunsOnClient
   - fix show verbose for RunScripts

core/src/dird/reload.cc

@@ -255,7 +255,7 @@ bool DoReloadConfig()
 
   DbSqlPoolFlush();
 
-  auto backup_table = my_config->BackupResourceTable();
+  auto backup_container = my_config->BackupResourcesContainer();
   Dmsg0(100, "Reloading config file\n");
 
 
@@ -281,7 +281,7 @@ bool DoReloadConfig()
          my_config->get_base_config_path().c_str());
 
     Jmsg(nullptr, M_ERROR, 0, _("Resetting to previous configuration.\n"));
-    my_config->RestoreResourceTable(std::move(backup_table));
+    my_config->RestoreResourcesContainer(std::move(backup_container));
     // me is changed above by CheckResources()
     me = (DirectorResource*)my_config->GetNextRes(R_DIRECTOR, nullptr);
     assert(me);

core/src/lib/parse_conf.cc

@@ -487,26 +487,29 @@ bool ConfigurationParser::FindConfigPath(PoolMem& full_path)
   return found;
 }
 
-// swap the previously saved configuration_resources_previous_ with
-// configuration_resources_ and release the configuration_resources_previous_
-void ConfigurationParser::RestoreResourceTable(
+void ConfigurationParser::RestoreResourcesContainer(
     std::shared_ptr<ConfigResourcesContainer>&& backup_table)
 {
   std::swap(config_resources_container_, backup_table);
   backup_table.reset();
 }
 
-// copy the current resource table to configuration_resources_backup_
-// and create a new empty config_resources_container_
 std::shared_ptr<ConfigResourcesContainer>
-ConfigurationParser::BackupResourceTable()
+ConfigurationParser::BackupResourcesContainer()
 {
   auto backup_table = config_resources_container_;
   config_resources_container_
       = std::make_shared<ConfigResourcesContainer>(this);
   return backup_table;
 }
 
+std::shared_ptr<ConfigResourcesContainer>
+ConfigurationParser::GetResourcesContainer()
+{
+  return config_resources_container_;
+}
+
+
 bool ConfigurationParser::RemoveResource(int rcode, const char* name)
 {
   int rindex = rcode;

core/src/lib/parse_conf.h

@@ -256,8 +256,9 @@ class ConfigurationParser {
   const std::string& get_base_config_path() const { return used_config_path_; }
   void FreeResources();
 
-  std::shared_ptr<ConfigResourcesContainer> BackupResourceTable();
-  void RestoreResourceTable(std::shared_ptr<ConfigResourcesContainer>&&);
+  std::shared_ptr<ConfigResourcesContainer> GetResourcesContainer();
+  std::shared_ptr<ConfigResourcesContainer> BackupResourcesContainer();
+  void RestoreResourcesContainer(std::shared_ptr<ConfigResourcesContainer>&&);
 
   void InitResource(int rcode,
                     ResourceItem items[],

core/src/lib/tls_openssl.cc

@@ -75,7 +75,9 @@ void TlsOpenSsl::SetTlsPskServerContext(ConfigurationParser* config)
   } else if (!config) {
     Dmsg0(50, "Could not prepare TLS_PSK SERVER callback (no config)\n");
   } else {
-    Dmsg0(50, "Preparing TLS_PSK SERVER callback\n");
+    // keep a shared_ptr to the current config, so a reload won't
+    // free the memory we're going to use in the private context
+    d_->config_table_ = config->GetResourcesContainer();
     SSL_CTX_set_ex_data(
         d_->openssl_ctx_,
         TlsOpenSslPrivate::SslCtxExDataIndex::kConfigurationParserPtr,

core/src/lib/tls_openssl.h

@@ -1,7 +1,7 @@
 /*
    BAREOS® - Backup Archiving REcovery Open Sourced
 
-   Copyright (C) 2018-2020 Bareos GmbH & Co. KG
+   Copyright (C) 2018-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -27,6 +27,7 @@
 #include <memory>
 
 class TlsOpenSslPrivate;
+class ConfigResourcesContainer;
 
 class TlsOpenSsl : public Tls {
  public:

core/src/lib/tls_openssl_private.cc

@@ -2,7 +2,7 @@
    BAREOS® - Backup Archiving REcovery Open Sourced
 
    Copyright (C) 2005-2010 Free Software Foundation Europe e.V.
-   Copyright (C) 2018-2020 Bareos GmbH & Co. KG
+   Copyright (C) 2018-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -54,13 +54,6 @@ const std::string TlsOpenSslPrivate::tls_default_ciphers_{
 
 
 TlsOpenSslPrivate::TlsOpenSslPrivate()
-    : openssl_(nullptr)
-    , openssl_ctx_(nullptr)
-    , openssl_conf_ctx_(nullptr)
-    , tcp_file_descriptor_(0)
-    , pem_callback_(nullptr)
-    , pem_userdata_(nullptr)
-    , verify_peer_(false)
 {
   Dmsg0(100, "Construct TlsOpenSslPrivate\n");
 

core/src/lib/tls_openssl_private.h

@@ -2,7 +2,7 @@
    BAREOS® - Backup Archiving REcovery Open Sourced
 
    Copyright (C) 2005-2010 Free Software Foundation Europe e.V.
-   Copyright (C) 2018-2021 Bareos GmbH & Co. KG
+   Copyright (C) 2018-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -68,9 +68,9 @@ class TlsOpenSslPrivate {
                                     unsigned int max_psk_len);
 
   /* each TCP connection has its own SSL_CTX object and SSL object */
-  SSL* openssl_;
-  SSL_CTX* openssl_ctx_;
-  SSL_CONF_CTX* openssl_conf_ctx_;
+  SSL* openssl_{};
+  SSL_CTX* openssl_ctx_{};
+  SSL_CONF_CTX* openssl_conf_ctx_{};
 
   /* PskCredentials lookup map for all connections */
   static std::map<const SSL_CTX*, PskCredentials> psk_client_credentials_;
@@ -84,18 +84,19 @@ class TlsOpenSslPrivate {
   std::string protocol_;
 
   /* cert attributes */
-  int tcp_file_descriptor_;
+  int tcp_file_descriptor_{};
   std::string ca_certfile_;
   std::string ca_certdir_;
   std::string crlfile_;
   std::string certfile_;
   std::string keyfile_;
-  CRYPTO_PEM_PASSWD_CB* pem_callback_;
-  void* pem_userdata_;
+  CRYPTO_PEM_PASSWD_CB* pem_callback_{};
+  void* pem_userdata_{};
   std::string dhfile_;
   std::string cipherlist_;
-  bool verify_peer_;
-  /* *************** */
+  bool verify_peer_{};
+  std::shared_ptr<ConfigResourcesContainer>
+      config_table_{};  // config table being used
 };
 
 #endif  // BAREOS_LIB_TLS_OPENSSL_PRIVATE_H_

core/src/tests/test_config_parser_dir.cc

@@ -100,28 +100,31 @@ TEST(ConfigParser_Dir, bareos_configparser_tests)
   my_config->ParseConfig();
   my_config->DumpResources(PrintMessage, NULL);
 
-  auto backup = my_config->BackupResourceTable();
-  my_config->ParseConfig();
-
-  me = (DirectorResource*)my_config->GetNextRes(R_DIRECTOR, nullptr);
-  my_config->own_resource_ = me;
-  ASSERT_NE(nullptr, me);
-  my_config->RestoreResourceTable(std::move(backup));
-  ASSERT_NE(nullptr, me);
-
-  // If a config already exists, BackupResourceTable() needs to be called before
-  // ParseConfig(), otherwise memory of the existing config is not freed
+  {
+    auto backup = my_config->BackupResourcesContainer();
+    my_config->ParseConfig();
+
+    me = (DirectorResource*)my_config->GetNextRes(R_DIRECTOR, nullptr);
+    my_config->own_resource_ = me;
+    ASSERT_NE(nullptr, me);
+    my_config->RestoreResourcesContainer(std::move(backup));
+    ASSERT_NE(nullptr, me);
+  }
+  // If a config already exists, BackupResourcesContainer() needs to be called
+  // before ParseConfig(), otherwise memory of the existing config is not freed
   // completely
-  auto backup2 = my_config->BackupResourceTable();
-  my_config->ParseConfig();
+  {
+    auto backup = my_config->BackupResourcesContainer();
+    my_config->ParseConfig();
 
-  me = (DirectorResource*)my_config->GetNextRes(R_DIRECTOR, nullptr);
-  my_config->own_resource_ = me;
-  assert(me);
+    me = (DirectorResource*)my_config->GetNextRes(R_DIRECTOR, nullptr);
+    my_config->own_resource_ = me;
+    assert(me);
 
-  ASSERT_NE(nullptr, me);
-  my_config->DumpResources(PrintMessage, NULL);
-  ASSERT_NE(nullptr, me);
+    ASSERT_NE(nullptr, me);
+    my_config->DumpResources(PrintMessage, NULL);
+    ASSERT_NE(nullptr, me);
+  }
   delete my_config;
 }