From 9967a6a03ddb4fa5844952349c6dd579621e2731 Mon Sep 17 00:00:00 2001 From: AKalinich-Luxoft Date: Fri, 25 May 2018 13:18:50 +0300 Subject: Fix nonfunctional CertificatePath and KeyPath parameters in INI Currently, SDL Core ignores both the CertificatePath and KeyPath keywords that would allow the system integrator to specify certificates for their environment, instead SDL Core only processes the certificate provided via the policy table. This fix makes these keywords functional. Following changes were done: - Added getters for CertificatePath and KeyPath parameters in SecurityManagerSettings class to provide another components an access to these properties - Added methods for loading certificate and private key data from the files specified by CertificatePath and KeyPath keywords - CryptoManager component implementation was updated. Now this component also read certificate data from files (if they are present and accessible) on its own initialization --- .../include/security_manager/crypto_manager_impl.h | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/components/security_manager/include/security_manager/crypto_manager_impl.h b/src/components/security_manager/include/security_manager/crypto_manager_impl.h index f4a13198da..cba1a1d1d0 100644 --- a/src/components/security_manager/include/security_manager/crypto_manager_impl.h +++ b/src/components/security_manager/include/security_manager/crypto_manager_impl.h @@ -177,6 +177,29 @@ class CryptoManagerImpl : public CryptoManager { */ EVP_PKEY* LoadModulePrivateKeyFromFile(); + /** + * @brief Updates certificate and private key for the current SSL context + * @param certificate new certificate to update + * @param key new private key to update + * @return true if certificate and private key were updated successfully, + * otherwise returns false + */ + bool UpdateModuleCertificateData(X509* certificate, EVP_PKEY* key); + + /** + * @brief Loads X509 certificate from file specified in CryptoManagerSettings + * @return returns pointer to the loaded X509 certificate in case of success + * otherwise returns NULL + */ + X509* LoadModuleCertificateFromFile(); + + /** + * @brief Loads private key from file specified in CryptoManagerSettings + * @return returns pointer to the loaded private key in case of success + * otherwise returns NULL + */ + EVP_PKEY* LoadModulePrivateKeyFromFile(); + const utils::SharedPtr settings_; SSL_CTX* context_; static uint32_t instance_count_; -- cgit v1.2.1 From 8d2379ca0b62c97118293e24d9ae097f1a556c6a Mon Sep 17 00:00:00 2001 From: AKalinich-Luxoft Date: Fri, 25 May 2018 15:02:52 +0300 Subject: Fix cert processing and module saving after policy table update SDL Core should update the module certificate in the local file system when a policy table update occurs. Currently SDL core is retrieving its certificate directly out of the policy table. This fix provides functionality for saving module certificate to the file system. Following changes were done: - Added getters for CertificatePath and KeyPath parameters in SecurityManagerSettings class to provide another components an access to these properties - Added methods for saving certificate and private key data to the files specified by CertificatePath and KeyPath keywords - CryptoManager component implementation was updated. Now this component also saves certificate data to files (if write permission allowed) after PTU --- .../include/security_manager/crypto_manager_impl.h | 26 +++++++ .../security_manager/src/crypto_manager_impl.cc | 86 ++++++++++++++-------- 2 files changed, 81 insertions(+), 31 deletions(-) diff --git a/src/components/security_manager/include/security_manager/crypto_manager_impl.h b/src/components/security_manager/include/security_manager/crypto_manager_impl.h index cba1a1d1d0..770dfa102d 100644 --- a/src/components/security_manager/include/security_manager/crypto_manager_impl.h +++ b/src/components/security_manager/include/security_manager/crypto_manager_impl.h @@ -154,6 +154,14 @@ class CryptoManagerImpl : public CryptoManager { bool AreForceProtectionSettingsCorrect() const; bool set_certificate(const std::string& cert_data); + /** + * @brief Saves new certificate data on the file system + * @param cert_data certificate data in PEM format + * @return true if new certificate data was successfully saved on the file + * system, otherwise returns false + */ + bool SaveCertificateData(const std::string& cert_data); + /** * @brief Updates certificate and private key for the current SSL context * @param certificate new certificate to update @@ -200,6 +208,24 @@ class CryptoManagerImpl : public CryptoManager { */ EVP_PKEY* LoadModulePrivateKeyFromFile(); + /** + * @brief Saves new X509 certificate data to file specified in + * CryptoManagerSettings + * @param certificate new X509 certificate data + * @return true if certificate data was saved to the file system otherwise + * returns false + */ + bool SaveModuleCertificateToFile(X509* certificate) const; + + /** + * @brief Saves new private key data to file specified in + * CryptoManagerSettings + * @param key new private key data + * @return true if private key data was saved to the file system otherwise + * returns false + */ + bool SaveModuleKeyToFile(EVP_PKEY* key) const; + const utils::SharedPtr settings_; SSL_CTX* context_; static uint32_t instance_count_; diff --git a/src/components/security_manager/src/crypto_manager_impl.cc b/src/components/security_manager/src/crypto_manager_impl.cc index 34727fedf9..48acd61614 100644 --- a/src/components/security_manager/src/crypto_manager_impl.cc +++ b/src/components/security_manager/src/crypto_manager_impl.cc @@ -221,7 +221,7 @@ bool CryptoManagerImpl::Init() { // Disable SSL2 as deprecated SSL_CTX_set_options(context_, SSL_OP_NO_SSLv2); - set_certificate(get_settings().certificate_data()); + SaveCertificateData(get_settings().certificate_data()); if (get_settings().ciphers_list().empty()) { LOG4CXX_WARN(logger_, "Empty ciphers list"); @@ -288,7 +288,7 @@ bool CryptoManagerImpl::OnCertificateUpdated(const std::string& data) { return false; } - if (!set_certificate(data)) { + if (!SaveCertificateData(data)) { LOG4CXX_ERROR(logger_, "Failed to save certificate data"); return false; } @@ -362,7 +362,7 @@ const CryptoManagerSettings& CryptoManagerImpl::get_settings() const { return *settings_; } -bool CryptoManagerImpl::set_certificate(const std::string& cert_data) { +bool CryptoManagerImpl::SaveCertificateData(const std::string& cert_data) { LOG4CXX_AUTO_TRACE(logger_); if (cert_data.empty()) { @@ -393,35 +393,10 @@ bool CryptoManagerImpl::set_certificate(const std::string& cert_data) { return false; } - if (!SSL_CTX_use_certificate(context_, cert)) { - LOG4CXX_WARN(logger_, "Could not use certificate: " << LastError()); - return false; - } - - if (!SSL_CTX_use_PrivateKey(context_, pkey)) { - LOG4CXX_ERROR(logger_, "Could not use key: " << LastError()); - return false; - } - - if (!SSL_CTX_check_private_key(context_)) { - LOG4CXX_ERROR(logger_, "Could not use certificate: " << LastError()); - return false; - } - - X509_STORE* store = SSL_CTX_get_cert_store(context_); - if (store) { - X509* extra_cert = NULL; - while ((extra_cert = PEM_read_bio_X509(bio_cert, NULL, 0, 0))) { - if (extra_cert != cert) { - LOG4CXX_DEBUG(logger_, - "Added new certificate to store: " << extra_cert); - X509_STORE_add_cert(store, extra_cert); - } - } - } + utils::ScopeGuard key_guard = utils::MakeGuard(EVP_PKEY_free, pkey); + UNUSED(key_guard); - LOG4CXX_DEBUG(logger_, "Certificate and key successfully updated"); - return true; + return SaveModuleCertificateToFile(cert) && SaveModuleKeyToFile(pkey); } bool CryptoManagerImpl::UpdateModuleCertificateData(X509* certificate, @@ -501,4 +476,53 @@ EVP_PKEY* CryptoManagerImpl::LoadModulePrivateKeyFromFile() { return module_key; } +bool CryptoManagerImpl::SaveModuleCertificateToFile(X509* certificate) const { + LOG4CXX_AUTO_TRACE(logger_); + + if (NULL == certificate) { + LOG4CXX_WARN(logger_, "Empty certificate. Saving will be skipped"); + return false; + } + + const std::string cert_path = get_settings().module_cert_path(); + BIO* bio_cert = BIO_new_file(cert_path.c_str(), "w"); + if (NULL == bio_cert) { + LOG4CXX_ERROR(logger_, + "Failed to open " << cert_path << " file: " << LastError()); + return false; + } + + if (0 == PEM_write_bio_X509(bio_cert, certificate)) { + LOG4CXX_ERROR(logger_, + "Failed to write certificate to file: " << LastError()); + return false; + } + + return true; +} + +bool CryptoManagerImpl::SaveModuleKeyToFile(EVP_PKEY* key) const { + LOG4CXX_AUTO_TRACE(logger_); + + if (NULL == key) { + LOG4CXX_WARN(logger_, "Empty private key. Saving will be skipped"); + return false; + } + + const std::string key_path = get_settings().module_key_path(); + BIO* bio_key = BIO_new_file(key_path.c_str(), "w"); + if (NULL == bio_key) { + LOG4CXX_ERROR(logger_, + "Failed to open " << key_path << " file: " << LastError()); + return false; + } + + if (0 == PEM_write_bio_PrivateKey(bio_key, key, NULL, NULL, 0, NULL, NULL)) { + LOG4CXX_ERROR(logger_, "Failed to write key to file: " << LastError()); + return false; + } + + return true; +} + } // namespace security_manager -- cgit v1.2.1 From 6c33fcaf72bf184b2daa22a9abde3ca36cea000b Mon Sep 17 00:00:00 2001 From: AKalinich-Luxoft Date: Fri, 25 May 2018 15:10:06 +0300 Subject: Fixed affected mocks and UT's Added new expectations for a security tests due to some changes in the security flow --- .../test/crypto_manager_impl_test.cc | 31 +++++++++------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/components/security_manager/test/crypto_manager_impl_test.cc b/src/components/security_manager/test/crypto_manager_impl_test.cc index 47e63dc194..5569325b96 100644 --- a/src/components/security_manager/test/crypto_manager_impl_test.cc +++ b/src/components/security_manager/test/crypto_manager_impl_test.cc @@ -145,26 +145,21 @@ TEST_F(CryptoManagerTest, UsingBeforeInit) { } TEST_F(CryptoManagerTest, WrongInit) { - forced_protected_services_.push_back(kServiceNumber); - forced_unprotected_services_.push_back(kServiceNumber); - EXPECT_CALL(*mock_security_manager_settings_, security_manager_mode()) - .WillOnce(Return(security_manager::SERVER)); - EXPECT_CALL(*mock_security_manager_settings_, force_unprotected_service()) - .WillOnce(ReturnRef(forced_unprotected_services_)); - EXPECT_CALL(*mock_security_manager_settings_, force_protected_service()) - .WillOnce(ReturnRef(forced_protected_services_)); - EXPECT_FALSE(crypto_manager_->Init()); - forced_protected_services_.pop_back(); - forced_unprotected_services_.pop_back(); - EXPECT_NE(std::string(), crypto_manager_->LastError()); + // We have to cast (-1) to security_manager::Protocol Enum to be accepted by + // crypto_manager_->Init(...) + // Unknown protocol version + security_manager::Protocol UNKNOWN = + static_cast(-1); // Unexistent cipher value const std::string invalid_cipher = "INVALID_UNKNOWN_CIPHER"; - EXPECT_CALL(*mock_security_manager_settings_, security_manager_mode()) - .WillOnce(Return(security_manager::SERVER)); - EXPECT_CALL(*mock_security_manager_settings_, force_unprotected_service()) - .WillOnce(ReturnRef(forced_unprotected_services_)); - EXPECT_CALL(*mock_security_manager_settings_, force_protected_service()) - .WillOnce(ReturnRef(forced_protected_services_)); + const security_manager::Mode mode = security_manager::SERVER; + + SetInitialValues(mode, UNKNOWN, invalid_cipher); + + EXPECT_FALSE(crypto_manager_->Init()); + EXPECT_NE(std::string(), crypto_manager_->LastError()); + +>>>>>>> Fixed affected mocks and UT's EXPECT_CALL(*mock_security_manager_settings_, security_manager_protocol_name()) .WillOnce(Return(security_manager::TLSv1_2)); -- cgit v1.2.1 From fbfc18dae7e58c7c74cae7b918e013f3e2b78ef8 Mon Sep 17 00:00:00 2001 From: Andrii Kalinich Date: Sat, 26 May 2018 00:18:08 +0300 Subject: Fix leaked objects and add const Conflicts: src/components/security_manager/src/crypto_manager_impl.cc --- .../include/security_manager/crypto_manager_impl.h | 2 +- .../security_manager/src/crypto_manager_impl.cc | 34 +++++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/components/security_manager/include/security_manager/crypto_manager_impl.h b/src/components/security_manager/include/security_manager/crypto_manager_impl.h index 770dfa102d..95b44edf7d 100644 --- a/src/components/security_manager/include/security_manager/crypto_manager_impl.h +++ b/src/components/security_manager/include/security_manager/crypto_manager_impl.h @@ -160,7 +160,7 @@ class CryptoManagerImpl : public CryptoManager { * @return true if new certificate data was successfully saved on the file * system, otherwise returns false */ - bool SaveCertificateData(const std::string& cert_data); + bool SaveCertificateData(const std::string& cert_data) const; /** * @brief Updates certificate and private key for the current SSL context diff --git a/src/components/security_manager/src/crypto_manager_impl.cc b/src/components/security_manager/src/crypto_manager_impl.cc index 48acd61614..51dff4e90f 100644 --- a/src/components/security_manager/src/crypto_manager_impl.cc +++ b/src/components/security_manager/src/crypto_manager_impl.cc @@ -362,7 +362,8 @@ const CryptoManagerSettings& CryptoManagerImpl::get_settings() const { return *settings_; } -bool CryptoManagerImpl::SaveCertificateData(const std::string& cert_data) { +bool CryptoManagerImpl::SaveCertificateData( + const std::string& cert_data) const { LOG4CXX_AUTO_TRACE(logger_); if (cert_data.empty()) { @@ -377,7 +378,13 @@ bool CryptoManagerImpl::SaveCertificateData(const std::string& cert_data) { UNUSED(bio_guard) X509* cert = NULL; - PEM_read_bio_X509(bio_cert, &cert, 0, 0); + if (!PEM_read_bio_X509(bio_cert, &cert, 0, 0)) { + LOG4CXX_WARN(logger_, "Could not read certificate data: " << LastError()); + return false; + } + + utils::ScopeGuard cert_guard = utils::MakeGuard(X509_free, cert); + UNUSED(cert_guard); EVP_PKEY* pkey = NULL; if (1 == BIO_reset(bio_cert)) { @@ -388,8 +395,9 @@ bool CryptoManagerImpl::SaveCertificateData(const std::string& cert_data) { << LastError()); } - if (NULL == cert || NULL == pkey) { - LOG4CXX_WARN(logger_, "Either certificate or key not valid."); + EVP_PKEY* pkey = NULL; + if (!PEM_read_bio_PrivateKey(bio_cert, &pkey, 0, 0)) { + LOG4CXX_WARN(logger_, "Could not read private key data: " << LastError()); return false; } @@ -479,20 +487,23 @@ EVP_PKEY* CryptoManagerImpl::LoadModulePrivateKeyFromFile() { bool CryptoManagerImpl::SaveModuleCertificateToFile(X509* certificate) const { LOG4CXX_AUTO_TRACE(logger_); - if (NULL == certificate) { + if (!certificate) { LOG4CXX_WARN(logger_, "Empty certificate. Saving will be skipped"); return false; } const std::string cert_path = get_settings().module_cert_path(); BIO* bio_cert = BIO_new_file(cert_path.c_str(), "w"); - if (NULL == bio_cert) { + if (!bio_cert) { LOG4CXX_ERROR(logger_, "Failed to open " << cert_path << " file: " << LastError()); return false; } - if (0 == PEM_write_bio_X509(bio_cert, certificate)) { + utils::ScopeGuard bio_guard = utils::MakeGuard(BIO_free, bio_cert); + UNUSED(bio_guard); + + if (!PEM_write_bio_X509(bio_cert, certificate)) { LOG4CXX_ERROR(logger_, "Failed to write certificate to file: " << LastError()); return false; @@ -504,20 +515,23 @@ bool CryptoManagerImpl::SaveModuleCertificateToFile(X509* certificate) const { bool CryptoManagerImpl::SaveModuleKeyToFile(EVP_PKEY* key) const { LOG4CXX_AUTO_TRACE(logger_); - if (NULL == key) { + if (!key) { LOG4CXX_WARN(logger_, "Empty private key. Saving will be skipped"); return false; } const std::string key_path = get_settings().module_key_path(); BIO* bio_key = BIO_new_file(key_path.c_str(), "w"); - if (NULL == bio_key) { + if (!bio_key) { LOG4CXX_ERROR(logger_, "Failed to open " << key_path << " file: " << LastError()); return false; } - if (0 == PEM_write_bio_PrivateKey(bio_key, key, NULL, NULL, 0, NULL, NULL)) { + utils::ScopeGuard bio_guard = utils::MakeGuard(BIO_free, bio_key); + UNUSED(bio_guard); + + if (!PEM_write_bio_PrivateKey(bio_key, key, NULL, NULL, 0, NULL, NULL)) { LOG4CXX_ERROR(logger_, "Failed to write key to file: " << LastError()); return false; } -- cgit v1.2.1 From 0a7317dda7adb28fc5a15234ab2bc09428153015 Mon Sep 17 00:00:00 2001 From: AKalinich-Luxoft Date: Mon, 4 Jun 2018 18:55:13 +0300 Subject: Added logic related to certificate saving Also was removed redundant logic --- .../src/connection_handler_impl.cc | 8 -- .../test/connection_handler_impl_test.cc | 10 --- .../include/protocol_handler/session_observer.h | 7 +- .../policy_external/src/policy_manager_impl.cc | 5 -- .../policy_regular/src/policy_manager_impl.cc | 11 +-- .../protocol_handler/protocol_handler_impl.h | 14 ---- .../protocol_handler/src/protocol_handler_impl.cc | 93 +--------------------- .../include/security_manager/crypto_manager_impl.h | 23 ------ .../security_manager/src/crypto_manager_impl.cc | 5 +- .../test/crypto_manager_impl_test.cc | 1 - 10 files changed, 11 insertions(+), 166 deletions(-) diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc index b97c6eacd4..59865ac4b2 100644 --- a/src/components/connection_handler/src/connection_handler_impl.cc +++ b/src/components/connection_handler/src/connection_handler_impl.cc @@ -467,14 +467,6 @@ void ConnectionHandlerImpl::OnSessionStartedCallback( const uint32_t session_key = KeyFromPair(connection_handle, context.new_session_id_); - uint32_t app_id = 0; - GetDataOnSessionKey( - session_key, &app_id, NULL, static_cast(NULL)); - if (app_id > 0) { - context.is_ptu_required_ = - !connection_handler_observer_->CheckAppIsNavi(app_id); - } - { sync_primitives::AutoLock auto_lock(start_service_context_map_lock_); start_service_context_map_[session_key] = context; diff --git a/src/components/connection_handler/test/connection_handler_impl_test.cc b/src/components/connection_handler/test/connection_handler_impl_test.cc index b231ee625f..56dbf6b9de 100644 --- a/src/components/connection_handler/test/connection_handler_impl_test.cc +++ b/src/components/connection_handler/test/connection_handler_impl_test.cc @@ -1273,9 +1273,6 @@ TEST_F(ConnectionHandlerTest, SessionStarted_WithRpc) { true, ByRef(empty))); - EXPECT_CALL(mock_connection_handler_observer, CheckAppIsNavi(_)) - .WillOnce(Return(true)); - connection_handler_->set_protocol_handler(&mock_protocol_handler_); EXPECT_CALL(mock_protocol_handler_, NotifySessionStarted(_, _)) .WillOnce(SaveArg<0>(&out_context_)); @@ -1312,8 +1309,6 @@ TEST_F(ConnectionHandlerTest, ServiceStarted_Video_SUCCESS) { session_key, true, ByRef(empty))); - EXPECT_CALL(mock_connection_handler_observer, CheckAppIsNavi(_)) - .WillOnce(Return(true)); // confirm that NotifySessionStarted() is called connection_handler_->set_protocol_handler(&mock_protocol_handler_); @@ -1354,8 +1349,6 @@ TEST_F(ConnectionHandlerTest, ServiceStarted_Video_FAILURE) { session_key, false, ByRef(empty))); - EXPECT_CALL(mock_connection_handler_observer, CheckAppIsNavi(_)) - .WillOnce(Return(true)); // confirm that NotifySessionStarted() is called connection_handler_->set_protocol_handler(&mock_protocol_handler_); @@ -1446,9 +1439,6 @@ TEST_F(ConnectionHandlerTest, ServiceStarted_Video_Multiple) { session_key1, true, ByRef(empty)))); - EXPECT_CALL(mock_connection_handler_observer, CheckAppIsNavi(_)) - .Times(2) - .WillOnce(Return(true)); // verify that connection handler will not mix up the two results SessionContext new_context_first, new_context_second; diff --git a/src/components/include/protocol_handler/session_observer.h b/src/components/include/protocol_handler/session_observer.h index 3482c6569c..242775bf25 100644 --- a/src/components/include/protocol_handler/session_observer.h +++ b/src/components/include/protocol_handler/session_observer.h @@ -66,7 +66,6 @@ struct SessionContext { uint32_t hash_id_; bool is_protected_; bool is_new_service_; - bool is_ptu_required_; /** * @brief Constructor @@ -78,8 +77,7 @@ struct SessionContext { , service_type_(protocol_handler::kInvalidServiceType) , hash_id_(0) , is_protected_(false) - , is_new_service_(false) - , is_ptu_required_(false) {} + , is_new_service_(false) {} /** * @brief Constructor @@ -105,8 +103,7 @@ struct SessionContext { , service_type_(service_type) , hash_id_(hash_id) , is_protected_(is_protected) - , is_new_service_(false) - , is_ptu_required_(false) {} + , is_new_service_(false) {} }; /** diff --git a/src/components/policy/policy_external/src/policy_manager_impl.cc b/src/components/policy/policy_external/src/policy_manager_impl.cc index 56de26af64..172e218e0e 100644 --- a/src/components/policy/policy_external/src/policy_manager_impl.cc +++ b/src/components/policy/policy_external/src/policy_manager_impl.cc @@ -1790,11 +1790,6 @@ StatusNotifier PolicyManagerImpl::AddApplication( device_consent); } else { PromoteExistedApplication(application_id, device_consent); - if (helpers::in_range(hmi_types, policy_table::AHT_NAVIGATION) && - !HasCertificate()) { - LOG4CXX_DEBUG(logger_, "Certificate does not exist, scheduling update."); - update_status_manager_.ScheduleUpdate(); - } return utils::MakeShared(); } } diff --git a/src/components/policy/policy_regular/src/policy_manager_impl.cc b/src/components/policy/policy_regular/src/policy_manager_impl.cc index 84d4ac853a..8687501b2f 100644 --- a/src/components/policy/policy_regular/src/policy_manager_impl.cc +++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc @@ -1073,13 +1073,6 @@ StatusNotifier PolicyManagerImpl::AddApplication( device_consent); } else { PromoteExistedApplication(application_id, device_consent); - const policy_table::AppHMIType type = policy_table::AHT_NAVIGATION; - if (helpers::in_range(hmi_types, - (rpc::Enum)type) && - !HasCertificate()) { - LOG4CXX_DEBUG(logger_, "Certificate does not exist, scheduling update."); - update_status_manager_.ScheduleUpdate(); - } return utils::MakeShared(); } } @@ -1157,6 +1150,10 @@ bool PolicyManagerImpl::InitPT(const std::string& file_name, if (ret) { RefreshRetrySequence(); update_status_manager_.OnPolicyInit(cache_->UpdateRequired()); + const std::string certificate_data = cache_->GetCertificate(); + if (!certificate_data.empty()) { + listener_->OnCertificateUpdated(certificate_data); + } } return ret; } diff --git a/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h b/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h index 4d86a78688..99f03b1c04 100644 --- a/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h +++ b/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h @@ -474,14 +474,6 @@ class ProtocolHandlerImpl void OnConnectionClosed( const transport_manager::ConnectionUID connection_id) OVERRIDE; - /** - * @brief OnPTUFinished the callback which signals PTU has finished - * - * @param ptu_result the result from the PTU - true if successful, - * otherwise false. - */ - void OnPTUFinished(const bool ptu_result) OVERRIDE; - /** * @brief Notifies subscribers about message * received from mobile device. @@ -685,12 +677,6 @@ class ProtocolHandlerImpl #ifdef ENABLE_SECURITY security_manager::SecurityManager* security_manager_; - - bool is_ptu_triggered_; - std::list > ptu_pending_handlers_; - std::list > handshake_handlers_; - sync_primitives::Lock ptu_handlers_lock_; - sync_primitives::Lock handshake_handlers_lock_; #endif // ENABLE_SECURITY // Thread that pumps non-parsed messages coming from mobile side. diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc index 5516af81c1..86dbb604fa 100644 --- a/src/components/protocol_handler/src/protocol_handler_impl.cc +++ b/src/components/protocol_handler/src/protocol_handler_impl.cc @@ -75,7 +75,6 @@ ProtocolHandlerImpl::ProtocolHandlerImpl( , #ifdef ENABLE_SECURITY security_manager_(NULL) - , is_ptu_triggered_(false) , #endif // ENABLE_SECURITY raw_ford_messages_from_mobile_( @@ -149,7 +148,6 @@ ProtocolHandlerImpl::~ProtocolHandlerImpl() { "Not all observers have unsubscribed" " from ProtocolHandlerImpl"); } - handshake_handlers_.clear(); } void ProtocolHandlerImpl::AddProtocolObserver(ProtocolObserver* observer) { @@ -845,61 +843,6 @@ void ProtocolHandlerImpl::NotifyOnFailedHandshake() { security_manager_->NotifyListenersOnHandshakeFailed(); } -void ProtocolHandlerImpl::OnPTUFinished(const bool ptu_result) { - LOG4CXX_AUTO_TRACE(logger_); - -#ifdef ENABLE_SECURITY - sync_primitives::AutoLock lock(ptu_handlers_lock_); - - if (!is_ptu_triggered_) { - LOG4CXX_ERROR(logger_, - "PTU was not triggered by service starting. Ignored"); - return; - } - - for (auto handler : ptu_pending_handlers_) { - const bool is_cert_expired = security_manager_->IsCertificateUpdateRequired( - handler->connection_key()); - security_manager::SSLContext* ssl_context = - is_cert_expired ? NULL - : security_manager_->CreateSSLContext( - handler->connection_key(), - security_manager::SecurityManager::kUseExisting); - - if (!ssl_context) { - const std::string error("CreateSSLContext failed"); - LOG4CXX_ERROR(logger_, error); - security_manager_->SendInternalError( - handler->connection_key(), - security_manager::SecurityManager::ERROR_INTERNAL, - error); - - handler->OnHandshakeDone( - handler->connection_key(), - security_manager::SSLContext::Handshake_Result_Fail); - - continue; - } - - if (ssl_context->IsInitCompleted()) { - handler->OnHandshakeDone( - handler->connection_key(), - security_manager::SSLContext::Handshake_Result_Success); - } else { - security_manager_->AddListener(new HandshakeHandler(*handler)); - if (!ssl_context->IsHandshakePending()) { - // Start handshake process - security_manager_->StartHandshake(handler->connection_key()); - } - } - } - - LOG4CXX_DEBUG(logger_, "Handshake handlers were notified"); - ptu_pending_handlers_.clear(); - is_ptu_triggered_ = false; -#endif // ENABLE_SECURITY -} - RESULT_CODE ProtocolHandlerImpl::SendFrame(const ProtocolFramePtr packet) { LOG4CXX_AUTO_TRACE(logger_); if (!packet) { @@ -1572,40 +1515,12 @@ void ProtocolHandlerImpl::NotifySessionStarted( context, packet->protocol_version(), bson_object_bytes); - handshake_handlers_.push_back(handler); - - const bool is_certificate_empty = - security_manager_->IsPolicyCertificateDataEmpty(); - - if (context.is_ptu_required_ && is_certificate_empty) { - LOG4CXX_DEBUG(logger_, - "PTU for StartSessionHandler " - << handler.get() - << " is required and certificate data is empty"); - - sync_primitives::AutoLock lock(ptu_handlers_lock_); - if (!is_ptu_triggered_) { - LOG4CXX_DEBUG(logger_, - "PTU is not triggered yet. " - << "Starting PTU and postponing SSL handshake"); - - ptu_pending_handlers_.push_back(handler); - is_ptu_triggered_ = true; - security_manager_->NotifyOnCertificateUpdateRequired(); - security_manager_->PostponeHandshake(connection_key); - } else { - LOG4CXX_DEBUG(logger_, "PTU has been triggered. Added to pending."); - ptu_pending_handlers_.push_back(handler); - } - return; - } security_manager::SSLContext* ssl_context = - is_certificate_empty - ? NULL - : security_manager_->CreateSSLContext( - connection_key, - security_manager::SecurityManager::kUseExisting); + security_manager_->CreateSSLContext( + connection_key, + security_manager::SecurityManager::ContextCreationStrategy:: + kUseExisting); if (!ssl_context) { const std::string error("CreateSSLContext failed"); LOG4CXX_ERROR(logger_, error); diff --git a/src/components/security_manager/include/security_manager/crypto_manager_impl.h b/src/components/security_manager/include/security_manager/crypto_manager_impl.h index 95b44edf7d..aa3be0f430 100644 --- a/src/components/security_manager/include/security_manager/crypto_manager_impl.h +++ b/src/components/security_manager/include/security_manager/crypto_manager_impl.h @@ -185,29 +185,6 @@ class CryptoManagerImpl : public CryptoManager { */ EVP_PKEY* LoadModulePrivateKeyFromFile(); - /** - * @brief Updates certificate and private key for the current SSL context - * @param certificate new certificate to update - * @param key new private key to update - * @return true if certificate and private key were updated successfully, - * otherwise returns false - */ - bool UpdateModuleCertificateData(X509* certificate, EVP_PKEY* key); - - /** - * @brief Loads X509 certificate from file specified in CryptoManagerSettings - * @return returns pointer to the loaded X509 certificate in case of success - * otherwise returns NULL - */ - X509* LoadModuleCertificateFromFile(); - - /** - * @brief Loads private key from file specified in CryptoManagerSettings - * @return returns pointer to the loaded private key in case of success - * otherwise returns NULL - */ - EVP_PKEY* LoadModulePrivateKeyFromFile(); - /** * @brief Saves new X509 certificate data to file specified in * CryptoManagerSettings diff --git a/src/components/security_manager/src/crypto_manager_impl.cc b/src/components/security_manager/src/crypto_manager_impl.cc index 51dff4e90f..84c5db7c0e 100644 --- a/src/components/security_manager/src/crypto_manager_impl.cc +++ b/src/components/security_manager/src/crypto_manager_impl.cc @@ -386,10 +386,7 @@ bool CryptoManagerImpl::SaveCertificateData( utils::ScopeGuard cert_guard = utils::MakeGuard(X509_free, cert); UNUSED(cert_guard); - EVP_PKEY* pkey = NULL; - if (1 == BIO_reset(bio_cert)) { - PEM_read_bio_PrivateKey(bio_cert, &pkey, 0, 0); - } else { + if (1 != BIO_reset(bio_cert)) { LOG4CXX_WARN(logger_, "Unabled to reset BIO in order to read private key, " << LastError()); diff --git a/src/components/security_manager/test/crypto_manager_impl_test.cc b/src/components/security_manager/test/crypto_manager_impl_test.cc index 5569325b96..74b071793d 100644 --- a/src/components/security_manager/test/crypto_manager_impl_test.cc +++ b/src/components/security_manager/test/crypto_manager_impl_test.cc @@ -159,7 +159,6 @@ TEST_F(CryptoManagerTest, WrongInit) { EXPECT_FALSE(crypto_manager_->Init()); EXPECT_NE(std::string(), crypto_manager_->LastError()); ->>>>>>> Fixed affected mocks and UT's EXPECT_CALL(*mock_security_manager_settings_, security_manager_protocol_name()) .WillOnce(Return(security_manager::TLSv1_2)); -- cgit v1.2.1