jami/jami-daemon
1
0
Fork 0
mirror of https://git.jami.net/savoirfairelinux/jami-daemon.git synced 2025-08-12 22:09:25 +08:00
Code Issues Projects Releases Wiki Activity

jamiaccount: check if uri is valid before adding contact

Browse Source 
Else, if a client calls addContact with an invalid uri, it will
create a conversation with an invalid contact and the client will
be in a bugguy state.

git.jami.net/savoirfairelinux/jami-client-android/-/issues/1681
Change-Id: Id6227c45c279c78aac0a191b6ae688ebe0d3d1c4
This commit is contained in:
Sébastien Blin
2024-05-31 11:18:56 -04:00
parent 90c9ae390e
commit 239d136788
7 changed files with 54 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
src/jamidht/account_manager.cpp
View File

@ -480,19 +480,14 @@ AccountManager::onPeerCertificate(const std::shared_ptr<dht::crypto::Certificate
}
bool
AccountManager::addContact(const std::string& uri, bool confirmed, const std::string& conversationId)
AccountManager::addContact(const dht::InfoHash& uri, bool confirmed, const std::string& conversationId)
{
JAMI_WARN("AccountManager::addContact %d", confirmed);
dht::InfoHash h(uri);
if (not h) {
JAMI_ERR("addContact: invalid contact URI");
return false;
}
JAMI_WARNING("AccountManager::addContact {}", confirmed);
if (not info_) {
JAMI_ERR("addContact(): account not loaded");
JAMI_ERROR("addContact(): account not loaded");
return false;
}
if (info_->contacts->addContact(h, confirmed, conversationId)) {
if (info_->contacts->addContact(uri, confirmed, conversationId)) {
syncDevices();
return true;
}
@ -504,11 +499,11 @@ AccountManager::removeContact(const std::string& uri, bool banned)
{
dht::InfoHash h(uri);
if (not h) {
JAMI_ERR("removeContact: invalid contact URI");
JAMI_ERROR("removeContact: invalid contact URI");
return;
}
if (not info_) {
JAMI_ERR("addContact(): account not loaded");
JAMI_ERROR("addContact(): account not loaded");
return;
}
if (info_->contacts->removeContact(h, banned))

2
src/jamidht/account_manager.h
View File

@ -219,7 +219,7 @@ public:
* Add contact to the account contact list.
* Set confirmed if we know the contact also added us.
*/
bool addContact(const std::string& uri,
bool addContact(const dht::InfoHash& uri,
bool confirmed = false,
const std::string& conversationId = "");
void removeContact(const std::string& uri, bool banned = true);

8
src/jamidht/conversation_module.cpp
View File

@ -2024,7 +2024,7 @@ ConversationModule::declineConversationRequest(const std::string& conversationId
}
std::string
ConversationModule::startConversation(ConversationMode mode, const std::string& otherMember)
ConversationModule::startConversation(ConversationMode mode, const dht::InfoHash& otherMember)
{
auto acc = pimpl_->account_.lock();
if (!acc)
@ -2035,7 +2035,7 @@ ConversationModule::startConversation(ConversationMode mode, const std::string&
// Create the conversation object
std::shared_ptr<Conversation> conversation;
try {
conversation = std::make_shared<Conversation>(acc, mode, otherMember);
conversation = std::make_shared<Conversation>(acc, mode, otherMember.toString());
auto conversationId = conversation->id();
conversation->onMessageStatusChanged([this, conversationId](const auto& status) {
auto msg = std::make_shared<SyncMsg>();
@ -2068,8 +2068,8 @@ ConversationModule::startConversation(ConversationMode mode, const std::string&
std::unique_lock lk(conv->mtx);
conv->info.created = std::time(nullptr);
conv->info.members.emplace(pimpl_->username_);
if (!otherMember.empty())
conv->info.members.emplace(otherMember);
if (!otherMember)
conv->info.members.emplace(otherMember.toString());
conv->conversation = conversation;
addConvInfo(conv->info);
lk.unlock();

2
src/jamidht/conversation_module.h
View File

@ -193,7 +193,7 @@ public:
* @return conversation's id
*/
std::string startConversation(ConversationMode mode = ConversationMode::INVITES_ONLY,
const std::string& otherMember = "");
const dht::InfoHash& otherMember = {});
// Message send/load
void sendMessage(const std::string& conversationId,

35
src/jamidht/jamiaccount.cpp
View File

@ -2782,14 +2782,19 @@ JamiAccount::getContactHeader(const std::shared_ptr<SipTransport>& sipTransport)
void
JamiAccount::addContact(const std::string& uri, bool confirmed)
{
dht::InfoHash h(uri);
if (not h) {
JAMI_ERROR("addContact: invalid contact URI");
return;
}
auto conversation = convModule()->getOneToOneConversation(uri);
if (!confirmed && conversation.empty())
conversation = convModule()->startConversation(ConversationMode::ONE_TO_ONE, uri);
conversation = convModule()->startConversation(ConversationMode::ONE_TO_ONE, h);
std::unique_lock<std::recursive_mutex> lock(configurationMutex_);
if (accountManager_)
accountManager_->addContact(uri, confirmed, conversation);
accountManager_->addContact(h, confirmed, conversation);
else
JAMI_WARN("[Account %s] addContact: account not loaded", getAccountID().c_str());
JAMI_WARNING("[Account {}] addContact: account not loaded", getAccountID());
}
void
@ -2799,7 +2804,7 @@ JamiAccount::removeContact(const std::string& uri, bool ban)
if (accountManager_)
accountManager_->removeContact(uri, ban);
else
JAMI_WARN("[Account %s] removeContact: account not loaded", getAccountID().c_str());
JAMI_WARNING("[Account {}] removeContact: account not loaded", getAccountID());
}
bool
@ -2852,16 +2857,21 @@ JamiAccount::getTrustRequests() const
bool
JamiAccount::acceptTrustRequest(const std::string& from, bool includeConversation)
{
dht::InfoHash h(from);
if (not h) {
JAMI_ERROR("addContact: invalid contact URI");
return false;
}
std::unique_lock<std::recursive_mutex> lock(configurationMutex_);
if (accountManager_) {
if (!accountManager_->acceptTrustRequest(from, includeConversation)) {
// Note: unused for swarm
// Typically the case where the trust request doesn't exists, only incoming DHT messages
return accountManager_->addContact(from, true);
return accountManager_->addContact(h, true);
}
return true;
}
JAMI_WARN("[Account %s] acceptTrustRequest: account not loaded", getAccountID().c_str());
JAMI_WARNING("[Account {}] acceptTrustRequest: account not loaded", getAccountID());
return false;
}
@ -2909,13 +2919,18 @@ JamiAccount::declineConversationRequest(const std::string& conversationId)
void
JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>& payload)
{
dht::InfoHash h(to);
if (not h) {
JAMI_ERROR("addContact: invalid contact URI");
return;
}
// Here we cache payload sent by the client
auto requestPath = cachePath_ / "requests";
dhtnet::fileutils::recursive_mkdir(requestPath, 0700);
auto cachedFile = requestPath / to;
std::ofstream req(cachedFile, std::ios::trunc | std::ios::binary);
if (!req.is_open()) {
JAMI_ERR("Could not write data to %s", cachedFile.c_str());
JAMI_ERROR("Could not write data to {}", cachedFile);
return;
}
@ -2929,7 +2944,7 @@ JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>&
auto conversation = convModule()->getOneToOneConversation(to);
if (conversation.empty())
conversation = convModule()->startConversation(ConversationMode::ONE_TO_ONE, to);
conversation = convModule()->startConversation(ConversationMode::ONE_TO_ONE, h);
if (not conversation.empty()) {
std::lock_guard lock(configurationMutex_);
if (accountManager_)
@ -2938,9 +2953,9 @@ JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>&
payload.size() >= 64000 ? std::vector<uint8_t> {}
: payload);
else
JAMI_WARN("[Account %s] sendTrustRequest: account not loaded", getAccountID().c_str());
JAMI_WARNING("[Account {}] sendTrustRequest: account not loaded", getAccountID());
} else
JAMI_WARN("[Account %s] sendTrustRequest: account not loaded", getAccountID().c_str());
JAMI_WARNING("[Account {}] sendTrustRequest: account not loaded", getAccountID());
}
void

2
test/unitTest/conversation/conversation.cpp
View File

@ -2175,7 +2175,7 @@ ConversationTest::testRemoveOneToOneNotInDetails()
auto firstConv = aliceData.conversationId;
// Create a duplicate
std::this_thread::sleep_for(2s); // Avoid to get same id
aliceAccount->convModule()->startConversation(ConversationMode::ONE_TO_ONE, bobUri);
aliceAccount->convModule()->startConversation(ConversationMode::ONE_TO_ONE, dht::InfoHash(bobUri));
CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return firstConv != aliceData.conversationId; }));
// Assert that repository exists

17
test/unitTest/conversation/conversationMembersEvent.cpp
View File

@ -73,6 +73,7 @@ public:
const std::string& convId,
const std::string& uri);
void testAddInvalidUri();
void testRemoveConversationNoMember();
void testRemoveConversationWithMember();
void testAddMember();
@ -125,6 +126,7 @@ public:
private:
CPPUNIT_TEST_SUITE(ConversationMembersEventTest);
CPPUNIT_TEST(testAddInvalidUri);
CPPUNIT_TEST(testRemoveConversationNoMember);
CPPUNIT_TEST(testRemoveConversationWithMember);
CPPUNIT_TEST(testAddMember);
@ -389,7 +391,9 @@ ConversationMembersEventTest::connectSignals()
}));
confHandlers.insert(libjami::exportable_callback<libjami::ConfigurationSignal::ContactAdded>(
[&](const std::string& accountId, const std::string&, bool) {
if (accountId == bobId) {
if (accountId == aliceId) {
aliceData.contactAdded = true;
} else if (accountId == bobId) {
bobData.contactAdded = true;
} else if (accountId == bob2Id) {
bob2Data.contactAdded = true;
@ -445,6 +449,17 @@ ConversationMembersEventTest::generateFakeInvite(std::shared_ptr<JamiAccount> ac
"");
}
void
ConversationMembersEventTest::testAddInvalidUri()
{
connectSignals();
auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
aliceAccount->addContact("Shitty/Uri");
CPPUNIT_ASSERT(!cv.wait_for(lk, 5s, [&]() { return aliceData.contactAdded; }));
CPPUNIT_ASSERT(aliceData.conversationId.empty());
}
void
ConversationMembersEventTest::testRemoveConversationNoMember()
{