[core] Fixed default reject reason for a listener callback (#2605).

It also fixes the wrong handling of a listener's rejection response.
Haivision · Jan 10, 2023 · 04e8dbc · 04e8dbc
1 parent 746551d
commit 04e8dbc
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 4 deletions.
diff --git a/srtcore/core.cpp b/srtcore/core.cpp
@@ -62,6 +62,7 @@ modified by
 #include <algorithm>
 #include <iterator>
 #include "srt.h"
+#include "access_control.h" // Required for SRT_REJX_FALLBACK
 #include "queue.h"
 #include "api.h"
 #include "core.h"
@@ -4440,9 +4441,12 @@ EConnectStatus srt::CUDT::processConnectResponse(const CPacket& response, CUDTEx
     }
 
     HLOGC(cnlog.Debug, log << CONID() << "processConnectResponse: HS RECEIVED: " << m_ConnRes.show());
-    if (m_ConnRes.m_iReqType > URQ_FAILURE_TYPES)
+    if (m_ConnRes.m_iReqType >= URQ_FAILURE_TYPES)
     {
         m_RejectReason = RejectReasonForURQ(m_ConnRes.m_iReqType);
+        LOGC(cnlog.Warn,
+                log << CONID() << "processConnectResponse: rejecting per reception of a rejection HS response: "
+                    << RequestTypeStr(m_ConnRes.m_iReqType));
         return CONN_REJECT;
     }
 
@@ -4625,6 +4629,7 @@ EConnectStatus srt::CUDT::postConnect(const CPacket* pResponse, bool rendezvous,
     // in rendezvous it's completed before calling this function.
     if (!rendezvous)
     {
+        HLOGC(cnlog.Debug, log << CONID() << boolalpha << "postConnect: packet:" << bool(pResponse) << " rendezvous:" << rendezvous);
         // The "local storage depleted" case shouldn't happen here, but
         // this is a theoretical path that needs prevention.
         bool ok = pResponse;
@@ -11592,6 +11597,8 @@ bool srt::CUDT::runAcceptHook(CUDT *acore, const sockaddr* peer, const CHandShak
     acore->m_HSGroupType = gt;
 #endif
 
+    // Set the default value
+    acore->m_RejectReason = SRT_REJX_FALLBACK;
     try
     {
         int result = CALLBACK_CALL(m_cbAcceptHook, acore->m_SocketID, hs.m_iVersion, peer, target);
@@ -11604,6 +11611,7 @@ bool srt::CUDT::runAcceptHook(CUDT *acore, const sockaddr* peer, const CHandShak
         return false;
     }
 
+    acore->m_RejectReason = SRT_REJ_UNKNOWN;
     return true;
 }
 

diff --git a/test/test_listen_callback.cpp b/test/test_listen_callback.cpp
@@ -9,6 +9,7 @@
 #endif
 
 #include "srt.h"
+#include "access_control.h"
 #include "utilities.h"
 
 srt_listen_callback_fn SrtTestListenCallback;
@@ -98,8 +99,8 @@ class ListenerCallback
             {
                 if (results[0].events == SRT_EPOLL_IN)
                 {
-                    int acp = srt_accept(server_sock, NULL, NULL);
-                    if (acp == SRT_ERROR)
+                    SRTSOCKET acp = srt_accept(server_sock, NULL, NULL);
+                    if (acp == SRT_INVALID_SOCK)
                     {
                         std::cout << "[T] Accept failed, so exitting\n";
                         break;
@@ -140,7 +141,7 @@ class ListenerCallback
 
 };
 
-int SrtTestListenCallback(void* opaq, SRTSOCKET ns SRT_ATR_UNUSED, int hsversion, const struct sockaddr* peeraddr, const char* streamid)
+int SrtTestListenCallback(void* opaq, SRTSOCKET ns, int hsversion, const struct sockaddr* peeraddr, const char* streamid)
 {
     using namespace std;
 
@@ -192,6 +193,7 @@ int SrtTestListenCallback(void* opaq, SRTSOCKET ns SRT_ATR_UNUSED, int hsversion
 
         if (!found)
         {
+            srt_setrejectreason(ns, SRT_REJX_UNAUTHORIZED);
             cerr << "TEST: USER NOT FOUND, returning false.\n";
             return -1;
         }
@@ -246,6 +248,8 @@ TEST_F(ListenerCallback, SecureSuccess)
 
     // EXPECTED RESULT: connected successfully
     EXPECT_NE(srt_connect(client_sock, psa, sizeof sa), SRT_ERROR);
+
+    EXPECT_EQ(srt_getrejectreason(client_sock), SRT_REJ_UNKNOWN);
 }
 
 #if SRT_ENABLE_ENCRYPTION
@@ -259,6 +263,8 @@ TEST_F(ListenerCallback, FauxPass)
 
     // EXPECTED RESULT: connection rejected
     EXPECT_EQ(srt_connect(client_sock, psa, sizeof sa), SRT_ERROR);
+
+    EXPECT_EQ(srt_getrejectreason(client_sock), SRT_REJ_BADSECRET);
 }
 #endif
 
@@ -274,7 +280,24 @@ TEST_F(ListenerCallback, FauxUser)
 
     // EXPECTED RESULT: connection rejected
     EXPECT_EQ(srt_connect(client_sock, psa, sizeof sa), SRT_ERROR);
+
+    EXPECT_EQ(srt_getrejectreason(client_sock), SRT_REJX_FALLBACK);
 }
 
+TEST_F(ListenerCallback, FauxSyntax)
+{
+    string username_spec = "#!::r=mystream,t=publish"; // No 'u' key specified
+    string password = "thelocalmanager";
+
+    ASSERT_NE(srt_setsockflag(client_sock, SRTO_STREAMID, username_spec.c_str(), username_spec.size()), -1);
+#if SRT_ENABLE_ENCRYPTION
+    ASSERT_NE(srt_setsockflag(client_sock, SRTO_PASSPHRASE, password.c_str(), password.size()), -1);
+#endif
+
+    // EXPECTED RESULT: connection rejected
+    EXPECT_EQ(srt_connect(client_sock, psa, sizeof sa), SRT_ERROR);
+
+    EXPECT_EQ(srt_getrejectreason(client_sock), SRT_REJX_UNAUTHORIZED);
+}