* #28529: sip: ensure that ports are unique

daemon/src/sip/sdp.cpp

@@ -39,6 +39,7 @@
 #include "manager.h"
 
 #include <algorithm>
+#include "sipaccount.h"
 
 #ifdef HAVE_OPUS
 #include "audio/codecs/opus.h"
@@ -77,6 +78,14 @@ Sdp::Sdp(pj_pool_t *pool)
     , telephoneEventPayload_(101) // same as asterisk
 {}
 
+Sdp::~Sdp()
+{
+    SIPAccount::releasePort(localAudioDataPort_);
+#ifdef SFL_VIDEO
+    SIPAccount::releasePort(localVideoDataPort_);
+#endif
+}
+
 namespace {
     bool hasPayload(const std::vector<sfl::AudioCodec*> &codecs, int pt)
     {

daemon/src/sip/sdp.h

@@ -69,6 +69,8 @@ class Sdp {
          */
         Sdp(pj_pool_t *pool);
 
+        ~Sdp();
+
         /**
          * Accessor for the internal memory pool
          */

daemon/src/sip/sipaccount.cpp

@@ -64,6 +64,8 @@ const char *const TRUE_STR = "true";
 const char *const FALSE_STR = "false";
 }
 
+bool SIPAccount::portsInUse_[UINT16_MAX];
+
 SIPAccount::SIPAccount(const std::string& accountID)
     : Account(accountID)
     , transport_(NULL)
@@ -1304,24 +1306,35 @@ bool SIPAccount::matches(const std::string &userName, const std::string &server,
         return false;
 }
 
-namespace {
-    // returns even number in range [lower, upper]
-    unsigned int getRandomEvenNumber(const std::pair<unsigned, unsigned> &range)
-    {
-        const unsigned halfUpper = range.second * 0.5;
-        const unsigned halfLower = range.first * 0.5;
-        return 2 * (halfLower + rand() % (halfUpper - halfLower + 1));
-    }
+// returns even number in range [lower, upper]
+uint16_t
+SIPAccount::getRandomEvenNumber(const std::pair<uint16_t, uint16_t> &range)
+{
+    const uint16_t halfUpper = range.second * 0.5;
+    const uint16_t halfLower = range.first * 0.5;
+    uint16_t result;
+    do {
+        result = 2 * (halfLower + rand() % (halfUpper - halfLower + 1));
+    } while (portsInUse_[result]);
+
+    portsInUse_[result] = true;
+    return result;
+}
+
+void
+SIPAccount::releasePort(uint16_t port)
+{
+    portsInUse_[port] = false;
 }
 
-unsigned
+uint16_t
 SIPAccount::generateAudioPort() const
 {
     return getRandomEvenNumber(audioPortRange_);
 }
 
 #ifdef SFL_VIDEO
-unsigned
+uint16_t
 SIPAccount::generateVideoPort() const
 {
     return getRandomEvenNumber(videoPortRange_);

daemon/src/sip/sipaccount.h

@@ -36,6 +36,7 @@
 #define SIPACCOUNT_H
 
 #include <vector>
+#include  <set>
 #include <map>
 #include "account.h"
 #include "pjsip/sip_transport_tls.h"
@@ -124,6 +125,9 @@ class SIPAccount : public Account {
          */
         bool isIP2IP() const;
 
+        static void
+        releasePort(uint16_t port);
+
         /**
          * Serialize internal state of this account for configuration
          * @param YamlEmitter the configuration engine which generate the configuration file
@@ -515,9 +519,9 @@ class SIPAccount : public Account {
         /* Returns true if the username and/or hostname match this account */
         bool matches(const std::string &username, const std::string &hostname, pjsip_endpoint *endpt, pj_pool_t *pool) const;
 
-        unsigned generateAudioPort() const;
+        uint16_t generateAudioPort() const;
 #ifdef SFL_VIDEO
-        unsigned generateVideoPort() const;
+        uint16_t generateVideoPort() const;
 #endif
 
     private:
@@ -771,14 +775,17 @@ class SIPAccount : public Account {
         /**
          * Port range for audio RTP ports
          */
-        std::pair<unsigned, unsigned> audioPortRange_;
+        std::pair<uint16_t, uint16_t> audioPortRange_;
 
 #ifdef SFL_VIDEO
         /**
          * Port range for video RTP ports
          */
-        std::pair<unsigned, unsigned> videoPortRange_;
+        std::pair<uint16_t, uint16_t> videoPortRange_;
 #endif
+        static bool portsInUse_[UINT16_MAX];
+        static uint16_t getRandomEvenNumber(const std::pair<uint16_t, uint16_t> &range);
+
 };
 
 #endif

daemon/src/sip/sipvoiplink.cpp

@@ -2369,10 +2369,9 @@ void setCallMediaLocal(SIPCall* call, const std::string &localIP)
 #ifdef SFL_VIDEO
     if (call->getLocalVideoPort() == 0) {
         // https://projects.savoirfairelinux.com/issues/17498
-        unsigned int callLocalVideoPort;
-        do
-            callLocalVideoPort = account->generateVideoPort();
-        while (call->getLocalAudioPort() == callLocalVideoPort);
+        const unsigned int callLocalVideoPort = account->generateVideoPort();
+        // this should already be guaranteed by SIPAccount
+        assert(call->getLocalAudioPort() != callLocalVideoPort);
 
         call->setLocalVideoPort(callLocalVideoPort);
         call->getLocalSDP()->setLocalPublishedVideoPort(callLocalVideoPort);