siptransport: fix tlsInfos_

sipTransport was only true on destroy, causing tlsInfos_ to be
always incorrect.
Moreover, if the SIPTransport is generated from a channeled socket,
we do not need to copy the peerCertificate.
Finally, update conference to use the tlsInfos_ to get the URI
instead of playing with peerNumber's

Change-Id: If8b93a4614e790e6ffbfd1bcc0aa68f6407344ce
This commit is contained in:
Sébastien Blin
2021-12-10 15:13:03 -05:00
parent 578036c496
commit 405ea9ee59
5 changed files with 125 additions and 47 deletions

View File

@ -686,9 +686,8 @@ Conference::addParticipant(const std::string& participant_id)
}
// Check for allModeratorEnabled preference
if (account->isAllModerators()) {
moderators_.emplace(string_remove_suffix(call->getPeerNumber(), '@'));
}
if (account->isAllModerators())
moderators_.emplace(getRemoteId(call));
}
}
#ifdef ENABLE_VIDEO
@ -857,7 +856,7 @@ Conference::removeParticipant(const std::string& participant_id)
return;
}
if (auto call = std::dynamic_pointer_cast<SIPCall>(getCall(participant_id))) {
auto peerId = std::string(string_remove_suffix(call->getPeerNumber(), '@'));
const auto& peerId = getRemoteId(call);
participantsMuted_.erase(call->getCallId());
if (auto* transport = call->getTransport())
handsRaised_.erase(std::string(transport->deviceId()));
@ -1155,16 +1154,13 @@ Conference::onConfOrder(const std::string& callId, const std::string& confOrder)
{
// Check if the peer is a master
if (auto call = getCall(callId)) {
auto peerId = string_remove_suffix(call->getPeerNumber(), '@');
const auto& peerId = getRemoteId(call);
std::string err;
Json::Value root;
Json::CharReaderBuilder rbuilder;
auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader());
if (!reader->parse(confOrder.c_str(), confOrder.c_str() + confOrder.size(), &root, &err)) {
JAMI_WARN("Couldn't parse conference order from %.*s",
(int) peerId.size(),
peerId.data());
JAMI_WARN("Couldn't parse conference order from %s", peerId.c_str());
return;
}
@ -1237,7 +1233,7 @@ Conference::setModerator(const std::string& participant_id, const bool& state)
for (const auto& p : getParticipantList()) {
if (auto call = getCall(p)) {
auto isPeerModerator = isModerator(participant_id);
if (participant_id == string_remove_suffix(call->getPeerNumber(), '@')) {
if (participant_id == getRemoteId(call)) {
if (state and not isPeerModerator) {
JAMI_DBG("Add %s as moderator", participant_id.c_str());
moderators_.emplace(participant_id);
@ -1650,7 +1646,7 @@ Conference::getCallFromPeerID(std::string_view peerID)
{
for (const auto& p : getParticipantList()) {
auto call = getCall(p);
if (call && string_remove_suffix(call->getPeerNumber(), '@') == peerID) {
if (call && getRemoteId(call) == peerID) {
return call;
}
}
@ -1672,4 +1668,14 @@ Conference::getCallWith(const std::string& accountUri, const std::string& device
return {};
}
std::string
Conference::getRemoteId(const std::shared_ptr<jami::Call>& call) const
{
if (auto* transport = std::dynamic_pointer_cast<SIPCall>(call)->getTransport())
if (auto cert = transport->getTlsInfos().peerCert)
if (cert->issuer)
return cert->issuer->getId().toString();
return {};
}
} // namespace jami

View File

@ -488,6 +488,7 @@ private:
#endif // ENABLE_PLUGIN
ConfProtocolParser parser_;
std::string getRemoteId(const std::shared_ptr<jami::Call>& call) const;
};
} // namespace jami

View File

@ -25,6 +25,7 @@
#include "ice_transport.h"
#include "security/tls_session.h"
#include "jamidht/abstract_sip_transport.h"
#include "jamidht/channeled_transport.h"
#include "jamidht/multiplexed_socket.h"
@ -89,6 +90,13 @@ SipTransport::SipTransport(pjsip_transport* t, const std::shared_ptr<TlsListener
tlsListener_ = l;
}
SipTransport::SipTransport(pjsip_transport* t,
const std::shared_ptr<dht::crypto::Certificate>& peerCertficate)
: SipTransport(t)
{
tlsInfos_.peerCert = peerCertficate;
}
SipTransport::~SipTransport()
{
JAMI_DBG("~SipTransport@%p {tr=%p {rc=%ld}}",
@ -121,17 +129,19 @@ SipTransport::stateCallback(pjsip_transport_state state, const pjsip_transport_s
tlsInfos_.proto = (pj_ssl_sock_proto) tlsInfo->proto;
tlsInfos_.cipher = tlsInfo->cipher;
tlsInfos_.verifyStatus = (pj_ssl_cert_verify_flag_t) tlsInfo->verify_status;
const auto& peers = tlsInfo->remote_cert_info->raw_chain;
std::vector<std::pair<const uint8_t*, const uint8_t*>> bits;
bits.resize(peers.cnt);
std::transform(peers.cert_raw,
peers.cert_raw + peers.cnt,
std::begin(bits),
[](const pj_str_t& crt) {
return std::make_pair((uint8_t*) crt.ptr,
(uint8_t*) (crt.ptr + crt.slen));
});
tlsInfos_.peerCert = std::make_shared<dht::crypto::Certificate>(bits);
if (!tlsInfos_.peerCert) {
const auto& peers = tlsInfo->remote_cert_info->raw_chain;
std::vector<std::pair<const uint8_t*, const uint8_t*>> bits;
bits.resize(peers.cnt);
std::transform(peers.cert_raw,
peers.cert_raw + peers.cnt,
std::begin(bits),
[](const pj_str_t& crt) {
return std::make_pair((uint8_t*) crt.ptr,
(uint8_t*) (crt.ptr + crt.slen));
});
tlsInfos_.peerCert = std::make_shared<dht::crypto::Certificate>(bits);
}
} else {
tlsInfos_ = {};
}
@ -204,34 +214,28 @@ SipTransportBroker::transportStateChanged(pjsip_transport* tp,
// and remove it from any mapping if destroy pending or done.
std::shared_ptr<SipTransport> sipTransport;
std::lock_guard<std::mutex> lock(transportMapMutex_);
auto key = transports_.find(tp);
if (key == transports_.end())
return;
if (!isDestroying_) {
std::lock_guard<std::mutex> lock(transportMapMutex_);
auto key = transports_.find(tp);
if (key == transports_.end())
return;
sipTransport = key->second.lock();
bool destroyed = state == PJSIP_TP_STATE_DESTROY;
sipTransport = key->second.lock();
if (!isDestroying_ && state == PJSIP_TP_STATE_DESTROY) {
// maps cleanup
if (destroyed) {
JAMI_DBG("unmap pjsip transport@%p {SipTransport@%p}", tp, sipTransport.get());
transports_.erase(key);
JAMI_DBG("unmap pjsip transport@%p {SipTransport@%p}", tp, sipTransport.get());
transports_.erase(key);
// If UDP
const auto type = tp->key.type;
if (type == PJSIP_TRANSPORT_UDP or type == PJSIP_TRANSPORT_UDP6) {
const auto updKey
= std::find_if(udpTransports_.cbegin(),
udpTransports_.cend(),
[tp](const std::pair<IpAddr, pjsip_transport*>& pair) {
return pair.second == tp;
});
if (updKey != udpTransports_.cend())
udpTransports_.erase(updKey);
}
// If UDP
const auto type = tp->key.type;
if (type == PJSIP_TRANSPORT_UDP or type == PJSIP_TRANSPORT_UDP6) {
const auto updKey = std::find_if(udpTransports_.cbegin(),
udpTransports_.cend(),
[tp](const std::pair<IpAddr, pjsip_transport*>& pair) {
return pair.second == tp;
});
if (updKey != udpTransports_.cend())
udpTransports_.erase(updKey);
}
}
@ -414,7 +418,7 @@ SipTransportBroker::getChanneledTransport(const std::shared_ptr<SIPAccountBase>&
remote,
std::move(cb));
auto tr = sips_tr->getTransportBase();
auto sip_tr = std::make_shared<SipTransport>(tr);
auto sip_tr = std::make_shared<SipTransport>(tr, socket->peerCertificate());
sip_tr->setDeviceId(socket->deviceId().toString());
sip_tr->setAccount(account);

View File

@ -92,6 +92,10 @@ class SipTransport
public:
SipTransport(pjsip_transport*);
SipTransport(pjsip_transport*, const std::shared_ptr<TlsListener>&);
// If the SipTransport is a channeled transport, we are already connected to the peer,
// so, we can directly set tlsInfos_.peerCert and avoid any copy
SipTransport(pjsip_transport* t,
const std::shared_ptr<dht::crypto::Certificate>& peerCertficate);
~SipTransport();

View File

@ -27,6 +27,8 @@
#include "manager.h"
#include "jamidht/connectionmanager.h"
#include "jamidht/jamiaccount.h"
#include "sip/sipcall.h"
#include "sip/siptransport.h"
#include "../../test_runner.h"
#include "jami.h"
#include "account_const.h"
@ -62,12 +64,14 @@ private:
void testCachedCall();
void testStopSearching();
void testDeclineMultiDevice();
void testTlsInfosPeerCertificate();
CPPUNIT_TEST_SUITE(CallTest);
CPPUNIT_TEST(testCall);
CPPUNIT_TEST(testCachedCall);
CPPUNIT_TEST(testStopSearching);
CPPUNIT_TEST(testDeclineMultiDevice);
CPPUNIT_TEST(testTlsInfosPeerCertificate);
CPPUNIT_TEST_SUITE_END();
};
@ -296,6 +300,65 @@ CallTest::testDeclineMultiDevice()
}));
}
void
CallTest::testTlsInfosPeerCertificate()
{
auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
auto bobUri = bobAccount->getUsername();
auto aliceUri = aliceAccount->getUsername();
std::mutex mtx;
std::unique_lock<std::mutex> lk {mtx};
std::condition_variable cv;
std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers;
std::atomic<int> callStopped {0};
std::string bobCallId;
std::string aliceCallState;
// Watch signals
confHandlers.insert(DRing::exportable_callback<DRing::CallSignal::IncomingCallWithMedia>(
[&](const std::string& accountId,
const std::string& callId,
const std::string&,
const std::vector<std::map<std::string, std::string>>&) {
if (accountId == bobId)
bobCallId = callId;
cv.notify_one();
}));
confHandlers.insert(DRing::exportable_callback<DRing::CallSignal::StateChange>(
[&](const std::string& accountId, const std::string&, const std::string& state, signed) {
if (accountId == aliceId)
aliceCallState = state;
if (state == "OVER") {
callStopped += 1;
if (callStopped == 2)
cv.notify_one();
}
}));
DRing::registerSignalHandlers(confHandlers);
JAMI_INFO("Start call between alice and Bob");
auto callId = DRing::placeCallWithMedia(aliceId, bobUri, {});
CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&] { return !bobCallId.empty(); }));
Manager::instance().answerCall(bobId, bobCallId);
CPPUNIT_ASSERT(
cv.wait_for(lk, std::chrono::seconds(30), [&] { return aliceCallState == "CURRENT"; }));
auto call = std::dynamic_pointer_cast<SIPCall>(aliceAccount->getCall(callId));
auto* transport = call->getTransport();
CPPUNIT_ASSERT(transport);
auto cert = transport->getTlsInfos().peerCert;
CPPUNIT_ASSERT(cert && cert->issuer);
CPPUNIT_ASSERT(cert->issuer->getId().toString() == bobAccount->getUsername());
JAMI_INFO("Stop call between alice and Bob");
callStopped = 0;
Manager::instance().hangupCall(aliceId, callId);
CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&] { return callStopped == 2; }));
}
} // namespace test
} // namespace jami