From 525c12878f74b359917d313f3a61a088af301601 Mon Sep 17 00:00:00 2001 From: Antoine Noreau <antoine.noreau@savoirfairelinux.com> Date: Mon, 24 Oct 2022 16:49:28 -0400 Subject: [PATCH] sip: fix incoming TLS calls - default disable secure dialog Change-Id: I834f27f66836be3ece3feac7ead662802d89da2d Gitlab: https://git.jami.net/savoirfairelinux/jami-client-android/-/issues/1111 --- 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, 19 insertions(+), 18 deletions(-) diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml index 469048349e..56c3e8137f 100644 --- a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml +++ b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml @@ -66,6 +66,7 @@ <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 3ff6ef0b60..6491a54c11 100644 --- a/src/account_schema.h +++ b/src/account_schema.h @@ -101,6 +101,7 @@ 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 756d487489..08eef98dd4 100644 --- a/src/jami/account_const.h +++ b/src/jami/account_const.h @@ -230,6 +230,7 @@ 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 c98ce1c301..8b65f50597 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1493,6 +1493,7 @@ 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 c8e894dd5e..c5b1f8e705 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -823,12 +823,6 @@ 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 6e14b79554..86dcea4f65 100644 --- a/src/preferences.cpp +++ b/src/preferences.cpp @@ -93,7 +93,6 @@ 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"}; @@ -240,8 +239,7 @@ Preferences::unserialize(const YAML::Node& in) } VoipPreference::VoipPreference() - : disableSecureDlgCheck_(false) - , playDtmf_(true) + : playDtmf_(true) , playTones_(true) , pulseLength_(PULSE_LENGTH_DEFAULT) , symmetricRtp_(true) @@ -251,7 +249,6 @@ 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_; @@ -264,7 +261,6 @@ 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 2a2985f0ff..2ab1e581bc 100644 --- a/src/preferences.h +++ b/src/preferences.h @@ -111,9 +111,6 @@ 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; } @@ -133,7 +130,6 @@ 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 fc8455d4cf..0cf2992a78 100644 --- a/src/sip/sipaccount.cpp +++ b/src/sip/sipaccount.cpp @@ -140,6 +140,7 @@ SIPAccount::SIPAccount(const std::string& accountID, bool presenceEnabled) , tlsVerifyServer_(false) , tlsVerifyClient_(true) , tlsRequireClientCertificate_(true) + , tlsDisableSecureDlgCheck_(true) , tlsNegotiationTimeoutSec_("2") , registrationStateDetailed_() , registrationRefreshEnabled_(true) @@ -465,6 +466,7 @@ 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_; @@ -581,6 +583,7 @@ SIPAccount::unserialize(const YAML::Node& node) parseValue(tlsMap, Conf::SERVER_KEY, tlsServerName_); parseValue(tlsMap, Conf::REQUIRE_CERTIF_KEY, tlsRequireClientCertificate_); + parseValueOptional(tlsMap, Conf::DISABLE_SECURE_DLG_CHECK, tlsDisableSecureDlgCheck_); parseValue(tlsMap, Conf::VERIFY_CLIENT_KEY, tlsVerifyClient_); parseValue(tlsMap, Conf::VERIFY_SERVER_KEY, tlsVerifyServer_); // FIXME @@ -635,6 +638,7 @@ 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_); @@ -1309,6 +1313,9 @@ 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; @@ -1794,6 +1801,8 @@ 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 525cda54d9..33ff31e6a3 100644 --- a/src/sip/sipaccount.h +++ b/src/sip/sipaccount.h @@ -680,6 +680,7 @@ 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 0d9c4a459a..bc9a1521e5 100644 --- a/src/sip/sipaccountbase.h +++ b/src/sip/sipaccountbase.h @@ -87,6 +87,7 @@ 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 adcf658702..2776331f0d 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_DBG("Creating TLS listener on %s...", ipAddress.toString(true).c_str()); + JAMI_DEBUG("Creating TLS listener on {:s}...", ipAddress.toString(true)); #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 954009c57a..fc89b3e1fa 100644 --- a/src/sip/sipvoiplink.cpp +++ b/src/sip/sipvoiplink.cpp @@ -376,7 +376,6 @@ 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) { @@ -387,7 +386,6 @@ 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; } @@ -395,6 +393,8 @@ 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