From e5eaf412877fb936955042545db48a4fe9fa5844 Mon Sep 17 00:00:00 2001
From: Matthieu Tanon <matthieu.tanon@belledonne-communications.com>
Date: Tue, 14 Aug 2018 14:52:18 +0200
Subject: [PATCH] Improve security alerts to avoid duplicates

---
 src/chat/chat-room/client-group-chat-room.cpp |  2 +-
 src/chat/encryption/lime-v2.cpp               | 21 +++++++++++++++----
 src/conference/session/media-session.cpp      |  7 ++++---
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/chat/chat-room/client-group-chat-room.cpp b/src/chat/chat-room/client-group-chat-room.cpp
index 9c1c3eadc..067e06b5f 100644
--- a/src/chat/chat-room/client-group-chat-room.cpp
+++ b/src/chat/chat-room/client-group-chat-room.cpp
@@ -725,7 +725,7 @@ void ClientGroupChatRoom::onSecurityAlert (const shared_ptr<ConferenceSecurityEv
 	_linphone_chat_room_notify_security_alert(cr, L_GET_C_BACK_PTR(event));
 
 	// Try to set the faulty device PeerDeviceStatus to unsafe
-	if (getCore()->limeV2Enabled()) {
+	if (getCore()->limeV2Enabled() && event->getFaultyDevice().isValid()) {
 		LimeV2 *limeV2Engine = static_cast<LimeV2 *>(getCore()->getEncryptionEngine());
 
 		// TODO has no effect if faulty device is unkown to LIMEv2
diff --git a/src/chat/encryption/lime-v2.cpp b/src/chat/encryption/lime-v2.cpp
index 0b7a46794..301393c18 100644
--- a/src/chat/encryption/lime-v2.cpp
+++ b/src/chat/encryption/lime-v2.cpp
@@ -169,15 +169,28 @@ ChatMessageModifier::Result LimeV2::processOutgoingMessage (const shared_ptr<Cha
 		}
 	}
 
+	// If too many devices for a participant, throw a local security alert event
 	if (tooManyDevices) {
-		// If too many devices for a participant, throw a local security alert event
 		lWarning() << "Sending encrypted message to multidevice participant, message rejected";
 
-		ConferenceSecurityEvent::SecurityAlertType securityAlertType = ConferenceSecurityEvent::SecurityAlertType::MultideviceParticipant;
-		shared_ptr<ConferenceSecurityEvent> securityEvent = make_shared<ConferenceSecurityEvent>(time(nullptr), chatRoom->getConferenceId(), securityAlertType);
+		// Check the last 2 events for security alerts before sending a new security event
+		bool recentSecurityAlert = false;
 		shared_ptr<ClientGroupChatRoom> confListener = static_pointer_cast<ClientGroupChatRoom>(chatRoom);
-		confListener->onSecurityAlert(securityEvent);
+		list<shared_ptr<EventLog>> eventList = chatRoom->getHistory(2);
 
+		// If there is at least one security alert don't send a new one
+		for (const auto &event : eventList) {
+			if (event->getType() == ConferenceEvent::Type::ConferenceSecurityAlert) {
+				recentSecurityAlert = true;
+			}
+		}
+
+		// If there is no recent security alert send a new one
+		if (!recentSecurityAlert) {
+			ConferenceSecurityEvent::SecurityAlertType securityAlertType = ConferenceSecurityEvent::SecurityAlertType::MultideviceParticipant;
+			shared_ptr<ConferenceSecurityEvent> securityEvent = make_shared<ConferenceSecurityEvent>(time(nullptr), chatRoom->getConferenceId(), securityAlertType);
+			confListener->onSecurityAlert(securityEvent);
+		}
 		return ChatMessageModifier::Result::Error;
 	}
 
diff --git a/src/conference/session/media-session.cpp b/src/conference/session/media-session.cpp
index bcc430a76..a3e228857 100644
--- a/src/conference/session/media-session.cpp
+++ b/src/conference/session/media-session.cpp
@@ -3523,7 +3523,7 @@ void MediaSessionPrivate::propagateEncryptionChanged () {
 				char *peerDeviceId = sal_address_as_string_uri_only(remoteAddress);
 
 				// TODO If mismatch = 0 set this peer as trusted with this Ik
-				// TODO If mismatch = 1 it means that the stored Ik was corrupted (identity theft)
+				// TODO If mismatch = 1 the Ik exchange went wrong (possible identity theft)
 				if (ms_zrtp_getAuxiliarySharedSecretMismatch(audioStream->ms.sessions.zrtp_context) == 0) {
 					if (limeV2Engine) {
 						try {
@@ -3532,15 +3532,16 @@ void MediaSessionPrivate::propagateEncryptionChanged () {
 							limeV2Engine->getLimeManager()->set_peerDeviceStatus(peerDeviceId, remoteIk_vector, peerDeviceStatus);
 							lInfo() << "LIMEv2 peer device " << peerDeviceId << " is now trusted";
 						} catch (const exception &e) {
+							// The stored IK doesn't correspond to the Ik we are trying to use here
 							// TODO Report the security issue to application level (chatroom event)
 							lError() << "LIMEv2 identity theft detected from " << peerDeviceId << " (" << e.what() << ")";
 						}
 					} else {
-						lError() << "Unable to get LIMEv2 context, unable to set peer identity verified status";
+						lError() << "Unable to get LIMEv2 context, unable to set peer device status";
 					}
 				} else {
 					// TODO Report the security issue to application level (chatroom event)
-					lError() << "LIMEv2 identity theft detected from " << peerDeviceId;
+					lError() << "LIMEv2 auxiliary secret mismatch: possible identity theft detected from " << peerDeviceId;
 				}
 				ms_free(peerDeviceId);
 			}
-- 
GitLab