From 45121c84b1c0bce9ee075af3df6762670095fd91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= <sebastien.blin@savoirfairelinux.com> Date: Tue, 25 Oct 2022 16:37:23 -0400 Subject: [PATCH] Revert "sip: fix incoming TLS calls - default disable secure dialog" This reverts commit 7bce5bb7658f8bad4e2bef8abc4ccd86bb32e506. Reason for revert: Break config Change-Id: I7e21df8f0fc80ceea3088f891992b1ae19693e33 --- bin/dbus/cx.ring.Ring.ConfigurationManager.xml | 1 - src/account_schema.h | 1 - src/jami/account_const.h | 1 - src/jamidht/jamiaccount.cpp | 1 - src/manager.cpp | 6 ++++++ src/preferences.cpp | 6 +++++- src/preferences.h | 4 ++++ src/sip/sipaccount.cpp | 9 --------- src/sip/sipaccount.h | 1 - src/sip/sipaccountbase.h | 1 - src/sip/siptransport.cpp | 2 +- src/sip/sipvoiplink.cpp | 4 ++-- 12 files changed, 18 insertions(+), 19 deletions(-) diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml index 56c3e8137f..469048349e 100644 --- a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml +++ b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml @@ -66,7 +66,6 @@ <li>TLS_VERIFY_SERVER</li> <li>TLS_VERIFY_CLIENT</li> <li>TLS_REQUIRE_CLIENT_CERTIFICATE</li> - <li>TLS_DISABLE_SECURE_DLG_CHECK</li> <li>TLS_NEGOTIATION_TIMEOUT_SEC</li> </ul> </tp:docstring> diff --git a/src/account_schema.h b/src/account_schema.h index 6491a54c11..3ff6ef0b60 100644 --- a/src/account_schema.h +++ b/src/account_schema.h @@ -101,7 +101,6 @@ static const char* const CONFIG_TLS_SERVER_NAME = "TLS.serverName"; static const char* const CONFIG_TLS_VERIFY_SERVER = "TLS.verifyServer"; static const char* const CONFIG_TLS_VERIFY_CLIENT = "TLS.verifyClient"; static const char* const CONFIG_TLS_REQUIRE_CLIENT_CERTIFICATE = "TLS.requireClientCertificate"; -static const char* const CONFIG_TLS_DISABLE_SECURE_DLG_CHECK = "TLS.disableSecureDlgCheck"; static const char* const CONFIG_TLS_NEGOTIATION_TIMEOUT_SEC = "TLS.negotiationTimeoutSec"; // DHT specific parameters diff --git a/src/jami/account_const.h b/src/jami/account_const.h index 08eef98dd4..756d487489 100644 --- a/src/jami/account_const.h +++ b/src/jami/account_const.h @@ -230,7 +230,6 @@ constexpr static const char SERVER_NAME[] = "TLS.serverName"; constexpr static const char VERIFY_SERVER[] = "TLS.verifyServer"; constexpr static const char VERIFY_CLIENT[] = "TLS.verifyClient"; constexpr static const char REQUIRE_CLIENT_CERTIFICATE[] = "TLS.requireClientCertificate"; -constexpr static const char DISABLE_SECURE_DLG_CHECK[] = "TLS.disableSecureDlgCheck"; constexpr static const char NEGOTIATION_TIMEOUT_SEC[] = "TLS.negotiationTimeoutSec"; } // namespace TLS diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index 8b65f50597..c98ce1c301 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1493,7 +1493,6 @@ JamiAccount::getAccountDetails() const a.emplace(Conf::CONFIG_TLS_VERIFY_SERVER, TRUE_STR); a.emplace(Conf::CONFIG_TLS_VERIFY_CLIENT, TRUE_STR); a.emplace(Conf::CONFIG_TLS_REQUIRE_CLIENT_CERTIFICATE, TRUE_STR); - a.emplace(Conf::CONFIG_TLS_DISABLE_SECURE_DLG_CHECK, TRUE_STR); a.emplace(DRing::Account::ConfProperties::ALLOW_CERT_FROM_HISTORY, allowPeersFromHistory_ ? TRUE_STR : FALSE_STR); a.emplace(DRing::Account::ConfProperties::ALLOW_CERT_FROM_CONTACT, diff --git a/src/manager.cpp b/src/manager.cpp index c5b1f8e705..c8e894dd5e 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -823,6 +823,12 @@ Manager::init(const std::string& config_file) JAMI_ERR("%s", e.what()); no_errors = false; } + // Some VoIP services support SIP/TLS and SRTP, but do not set the + // correct schema in the INVITE request. For more details, see: + // https://trac.pjsip.org/repos/ticket/1735 + if (voipPreferences.getDisableSecureDlgCheck()) { + pjsip_cfg()->endpt.disable_secure_dlg_check = PJ_TRUE; + } // always back up last error-free configuration if (no_errors) { diff --git a/src/preferences.cpp b/src/preferences.cpp index 86dcea4f65..6e14b79554 100644 --- a/src/preferences.cpp +++ b/src/preferences.cpp @@ -93,6 +93,7 @@ static constexpr const char* MD5_HASH_KEY {"md5Hash"}; // voip preferences constexpr const char* const VoipPreference::CONFIG_LABEL; +static constexpr const char* DISABLE_SECURE_DLG_CHECK_KEY {"disableSecureDlgCheck"}; static constexpr const char* PLAY_DTMF_KEY {"playDtmf"}; static constexpr const char* PLAY_TONES_KEY {"playTones"}; static constexpr const char* PULSE_LENGTH_KEY {"pulseLength"}; @@ -239,7 +240,8 @@ Preferences::unserialize(const YAML::Node& in) } VoipPreference::VoipPreference() - : playDtmf_(true) + : disableSecureDlgCheck_(false) + , playDtmf_(true) , playTones_(true) , pulseLength_(PULSE_LENGTH_DEFAULT) , symmetricRtp_(true) @@ -249,6 +251,7 @@ void VoipPreference::serialize(YAML::Emitter& out) const { out << YAML::Key << CONFIG_LABEL << YAML::Value << YAML::BeginMap; + out << YAML::Key << DISABLE_SECURE_DLG_CHECK_KEY << YAML::Value << disableSecureDlgCheck_; out << YAML::Key << PLAY_DTMF_KEY << YAML::Value << playDtmf_; out << YAML::Key << PLAY_TONES_KEY << YAML::Value << playTones_; out << YAML::Key << PULSE_LENGTH_KEY << YAML::Value << pulseLength_; @@ -261,6 +264,7 @@ void VoipPreference::unserialize(const YAML::Node& in) { const auto& node = in[CONFIG_LABEL]; + parseValue(node, DISABLE_SECURE_DLG_CHECK_KEY, disableSecureDlgCheck_); parseValue(node, PLAY_DTMF_KEY, playDtmf_); parseValue(node, PLAY_TONES_KEY, playTones_); parseValue(node, PULSE_LENGTH_KEY, pulseLength_); diff --git a/src/preferences.h b/src/preferences.h index 2ab1e581bc..2a2985f0ff 100644 --- a/src/preferences.h +++ b/src/preferences.h @@ -111,6 +111,9 @@ public: void serialize(YAML::Emitter& out) const override; void unserialize(const YAML::Node& in) override; + bool getDisableSecureDlgCheck() const { return disableSecureDlgCheck_; } + void setDisableSecureDlgCheck(bool disable) { disableSecureDlgCheck_ = disable; } + bool getPlayDtmf() const { return playDtmf_; } void setPlayDtmf(bool dtmf) { playDtmf_ = dtmf; } @@ -130,6 +133,7 @@ public: void setZidFile(const std::string& file) { zidFile_ = file; } private: + bool disableSecureDlgCheck_; bool playDtmf_; bool playTones_; int pulseLength_; diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp index 344ab80ba8..fc8455d4cf 100644 --- a/src/sip/sipaccount.cpp +++ b/src/sip/sipaccount.cpp @@ -140,7 +140,6 @@ SIPAccount::SIPAccount(const std::string& accountID, bool presenceEnabled) , tlsVerifyServer_(false) , tlsVerifyClient_(true) , tlsRequireClientCertificate_(true) - , tlsDisableSecureDlgCheck_(true) , tlsNegotiationTimeoutSec_("2") , registrationStateDetailed_() , registrationRefreshEnabled_(true) @@ -466,7 +465,6 @@ SIPAccount::serialize(YAML::Emitter& out) const out << YAML::Key << Conf::VERIFY_CLIENT_KEY << YAML::Value << tlsVerifyClient_; out << YAML::Key << Conf::VERIFY_SERVER_KEY << YAML::Value << tlsVerifyServer_; out << YAML::Key << Conf::REQUIRE_CERTIF_KEY << YAML::Value << tlsRequireClientCertificate_; - out << YAML::Key << Conf::DISABLE_SECURE_DLG_CHECK << YAML::Value << tlsDisableSecureDlgCheck_; out << YAML::Key << Conf::TIMEOUT_KEY << YAML::Value << tlsNegotiationTimeoutSec_; out << YAML::Key << Conf::CIPHERS_KEY << YAML::Value << tlsCiphers_; out << YAML::Key << Conf::METHOD_KEY << YAML::Value << tlsMethod_; @@ -583,7 +581,6 @@ SIPAccount::unserialize(const YAML::Node& node) parseValue(tlsMap, Conf::SERVER_KEY, tlsServerName_); parseValue(tlsMap, Conf::REQUIRE_CERTIF_KEY, tlsRequireClientCertificate_); - parseValue(tlsMap, Conf::DISABLE_SECURE_DLG_CHECK, tlsDisableSecureDlgCheck_); parseValue(tlsMap, Conf::VERIFY_CLIENT_KEY, tlsVerifyClient_); parseValue(tlsMap, Conf::VERIFY_SERVER_KEY, tlsVerifyServer_); // FIXME @@ -638,7 +635,6 @@ SIPAccount::setAccountDetails(const std::map<std::string, std::string>& details) parseBool(details, Conf::CONFIG_TLS_VERIFY_SERVER, tlsVerifyServer_); parseBool(details, Conf::CONFIG_TLS_VERIFY_CLIENT, tlsVerifyClient_); parseBool(details, Conf::CONFIG_TLS_REQUIRE_CLIENT_CERTIFICATE, tlsRequireClientCertificate_); - parseBool(details, Conf::CONFIG_TLS_DISABLE_SECURE_DLG_CHECK, tlsDisableSecureDlgCheck_); parseString(details, Conf::CONFIG_TLS_NEGOTIATION_TIMEOUT_SEC, tlsNegotiationTimeoutSec_); parseBool(details, Conf::CONFIG_TLS_VERIFY_SERVER, tlsVerifyServer_); parseBool(details, Conf::CONFIG_TLS_VERIFY_CLIENT, tlsVerifyClient_); @@ -1313,9 +1309,6 @@ SIPAccount::initTlsConfiguration() tlsSetting_.verify_client = tlsVerifyClient_; tlsSetting_.require_client_cert = tlsRequireClientCertificate_; - pjsip_cfg()->endpt.disable_secure_dlg_check = tlsDisableSecureDlgCheck_; - JAMI_DEBUG("Secure check dialog disabled ? {}", tlsDisableSecureDlgCheck_); - tlsSetting_.timeout.sec = atol(tlsNegotiationTimeoutSec_.c_str()); tlsSetting_.qos_type = PJ_QOS_TYPE_BEST_EFFORT; @@ -1801,8 +1794,6 @@ SIPAccount::getTlsSettings() const {Conf::CONFIG_TLS_VERIFY_CLIENT, tlsVerifyClient_ ? TRUE_STR : FALSE_STR}, {Conf::CONFIG_TLS_REQUIRE_CLIENT_CERTIFICATE, tlsRequireClientCertificate_ ? TRUE_STR : FALSE_STR}, - {Conf::CONFIG_TLS_DISABLE_SECURE_DLG_CHECK, - tlsDisableSecureDlgCheck_ ? TRUE_STR : FALSE_STR}, {Conf::CONFIG_TLS_NEGOTIATION_TIMEOUT_SEC, tlsNegotiationTimeoutSec_}}; } diff --git a/src/sip/sipaccount.h b/src/sip/sipaccount.h index 33ff31e6a3..525cda54d9 100644 --- a/src/sip/sipaccount.h +++ b/src/sip/sipaccount.h @@ -680,7 +680,6 @@ private: bool tlsVerifyServer_; bool tlsVerifyClient_; bool tlsRequireClientCertificate_; - bool tlsDisableSecureDlgCheck_; std::string tlsNegotiationTimeoutSec_; /** diff --git a/src/sip/sipaccountbase.h b/src/sip/sipaccountbase.h index bc9a1521e5..0d9c4a459a 100644 --- a/src/sip/sipaccountbase.h +++ b/src/sip/sipaccountbase.h @@ -87,7 +87,6 @@ const char* const TIMEOUT_KEY = "timeout"; const char* const TLS_PASSWORD_KEY = "password"; const char* const PRIVATE_KEY_KEY = "privateKey"; const char* const REQUIRE_CERTIF_KEY = "requireCertif"; -const char* const DISABLE_SECURE_DLG_CHECK = "disableSecureDlgCheck"; const char* const SERVER_KEY = "server"; const char* const VERIFY_CLIENT_KEY = "verifyClient"; const char* const VERIFY_SERVER_KEY = "verifyServer"; diff --git a/src/sip/siptransport.cpp b/src/sip/siptransport.cpp index 2776331f0d..adcf658702 100644 --- a/src/sip/siptransport.cpp +++ b/src/sip/siptransport.cpp @@ -337,7 +337,7 @@ SipTransportBroker::getTlsListener(const IpAddr& ipAddress, const pjsip_tls_sett { RETURN_IF_FAIL(settings, nullptr, "TLS settings not specified"); RETURN_IF_FAIL(ipAddress, nullptr, "Could not determine IP address for this transport"); - JAMI_DEBUG("Creating TLS listener on {:s}...", ipAddress.toString(true)); + JAMI_DBG("Creating TLS listener on %s...", ipAddress.toString(true).c_str()); #if 0 JAMI_DBG(" ca_list_file : %s", settings->ca_list_file.ptr); JAMI_DBG(" cert_file : %s", settings->cert_file.ptr); diff --git a/src/sip/sipvoiplink.cpp b/src/sip/sipvoiplink.cpp index fc89b3e1fa..954009c57a 100644 --- a/src/sip/sipvoiplink.cpp +++ b/src/sip/sipvoiplink.cpp @@ -376,6 +376,7 @@ transaction_request_cb(pjsip_rx_data* rdata) } pjmedia_sdp_session* r_sdp {nullptr}; + if (body) { if (pjmedia_sdp_parse(rdata->tp_info.pool, (char*) body->data, body->len, &r_sdp) != PJ_SUCCESS) { @@ -386,6 +387,7 @@ transaction_request_cb(pjsip_rx_data* rdata) if (not account->hasActiveCodec(MEDIA_AUDIO)) { try_respond_stateless(endpt_, rdata, PJSIP_SC_NOT_ACCEPTABLE_HERE, NULL, NULL, NULL); + return PJ_FALSE; } @@ -393,8 +395,6 @@ transaction_request_cb(pjsip_rx_data* rdata) unsigned options = 0; if (pjsip_inv_verify_request(rdata, &options, NULL, NULL, endpt_, NULL) != PJ_SUCCESS) { - - JAMI_ERR("Couldn't verify INVITE request in secure dialog."); try_respond_stateless(endpt_, rdata, PJSIP_SC_METHOD_NOT_ALLOWED, NULL, NULL, NULL); return PJ_FALSE; } -- GitLab