Applied changes for strict encryption (#495)

* Applied changes for strict encryption

apps/socketoptions.hpp

@@ -222,7 +222,8 @@ const SocketOption srt_options [] {
     { "payloadsize", 0, SRTO_PAYLOADSIZE, SocketOption::PRE, SocketOption::INT, nullptr},
     { "transtype", 0, SRTO_TRANSTYPE, SocketOption::PRE, SocketOption::ENUM, &enummap_transtype },
     { "kmrefreshrate", 0, SRTO_KMREFRESHRATE, SocketOption::PRE, SocketOption::INT, nullptr },
-    { "kmpreannounce", 0, SRTO_KMPREANNOUNCE, SocketOption::PRE, SocketOption::INT, nullptr }
+    { "kmpreannounce", 0, SRTO_KMPREANNOUNCE, SocketOption::PRE, SocketOption::INT, nullptr },
+    { "strictenc", 0, SRTO_STRICTENC, SocketOption::PRE, SocketOption::BOOL, nullptr }
 };
 }
 

srtcore/api.cpp

@@ -439,15 +439,6 @@ int CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr* peer, CHan
    }
    catch (...)
    {
-       // The mapped socket should be now unmapped to preserve the situation that
-       // was in the original UDT code.
-       // In SRT additionally the acceptAndRespond() function (it was called probably
-       // connect() in UDT code) may fail, in which case this socket should not be
-       // further processed and should be removed.
-       {
-           CGuard cg(m_ControlLock);
-           m_Sockets.erase(ns->m_SocketID);
-       }
        error = 1;
        goto ERR_ROLLBACK;
    }
@@ -497,9 +488,20 @@ int CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr* peer, CHan
        static const char* why [] = {"?", "ACCEPT ERROR", "IPE when mapping a socket", "IPE when inserting a socket" };
        LOGC(mglog.Error, log << CONID(ns->m_SocketID) << "newConnection: connection rejected due to: " << why[error]);
 #endif
+      SRTSOCKET id = ns->m_SocketID;
       ns->m_pUDT->close();
       ns->m_Status = SRTS_CLOSED;
       ns->m_TimeStamp = CTimer::getTime();
+      // The mapped socket should be now unmapped to preserve the situation that
+      // was in the original UDT code.
+      // In SRT additionally the acceptAndRespond() function (it was called probably
+      // connect() in UDT code) may fail, in which case this socket should not be
+      // further processed and should be removed.
+      {
+          CGuard cg(m_ControlLock);
+          m_Sockets.erase(id);
+          m_ClosedSockets[id] = ns;
+      }
 
       return -1;
    }

srtcore/core.cpp

@@ -244,6 +244,7 @@ CUDT::CUDT()
    m_iOPT_PeerTsbPdDelay = 0;       //Peer's TsbPd delay as receiver (here is its minimum value, if used)
    m_bOPT_TLPktDrop = true;
    m_iOPT_SndDropDelay = 0;
+   m_bOPT_StrictEncryption = true;
    m_bTLPktDrop = true;         //Too-late Packet Drop
    m_bMessageAPI = true;
    m_zOPT_ExpPayloadSize = SRT_LIVE_DEF_PLSIZE;
@@ -305,6 +306,7 @@ CUDT::CUDT(const CUDT& ancestor)
    m_iOPT_PeerTsbPdDelay = ancestor.m_iOPT_PeerTsbPdDelay;
    m_bOPT_TLPktDrop = ancestor.m_bOPT_TLPktDrop;
    m_iOPT_SndDropDelay = ancestor.m_iOPT_SndDropDelay;
+   m_bOPT_StrictEncryption = ancestor.m_bOPT_StrictEncryption;
    m_zOPT_ExpPayloadSize = ancestor.m_zOPT_ExpPayloadSize;
    m_bTLPktDrop = ancestor.m_bTLPktDrop;
    m_bMessageAPI = ancestor.m_bMessageAPI;
@@ -819,6 +821,13 @@ void CUDT::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
       }
       break;
 
+   case SRTO_STRICTENC:
+      if (m_bConnected)
+          throw CUDTException(MJ_NOTSUP, MN_ISCONNECTED, 0);
+
+        m_bOPT_StrictEncryption = bool_int_value(optval, optlen);
+        break;
+
     default:
         throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
     }
@@ -1086,6 +1095,11 @@ void CUDT::getOpt(SRT_SOCKOPT optName, void* optval, int& optlen)
       *(int*)optval = m_zOPT_ExpPayloadSize;
       break;
 
+   case SRTO_STRICTENC:
+      optlen = sizeof (int32_t); // also with TSBPDMODE and SENDER
+      *(int32_t*)optval = m_bOPT_StrictEncryption;
+      break;
+
    default:
       throw CUDTException(MJ_NOTSUP, MN_NONE, 0);
    }
@@ -1951,6 +1965,12 @@ bool CUDT::processSrtMsg(const CPacket *ctrlpkt)
             {
                 if (len_out == 1)
                 {
+                    if (m_bOPT_StrictEncryption)
+                    {
+                        LOGC(mglog.Error, log << "KMREQ FAILURE: " << KmStateStr(SRT_KM_STATE(srtdata_out[0]))
+                                << " - rejecting per strict encryption");
+                        return false;
+                    }
                     HLOGC(mglog.Debug, log << "MKREQ -> KMRSP FAILURE state: " << KmStateStr(SRT_KM_STATE(srtdata_out[0])));
                 }
                 else
@@ -1959,6 +1979,8 @@ bool CUDT::processSrtMsg(const CPacket *ctrlpkt)
                 }
                 sendSrtMsg(SRT_CMD_KMRSP, srtdata_out, len_out);
             }
+            // XXX Dead code. processSrtMsg_KMREQ now doesn't return any other value now.
+            // Please review later.
             else
             {
                 LOGC(mglog.Error, log << "KMREQ failed to process the request - ignoring");
@@ -2445,7 +2467,13 @@ bool CUDT::interpretSrtHandshake(const CHandShake& hs, const CPacket& hspkt, uin
 
         if (!m_pCryptoControl->hasPassphrase())
         {
-            LOGC(mglog.Error, log << "HS KMREQ: Peer declares encryption, but agent does not.");
+            if (m_bOPT_StrictEncryption)
+            {
+                LOGC(mglog.Error, log << "HS KMREQ: Peer declares encryption, but agent does not - rejecting per strict requirement");
+                return false;
+            }
+
+            LOGC(mglog.Error, log << "HS KMREQ: Peer declares encryption, but agent does not - still allowing connection.");
 
             // Still allow for connection, and allow Agent to send unencrypted stream to the peer.
             // Also normally allow the key to be processed; worst case it will send the failure response.
@@ -2478,12 +2506,26 @@ bool CUDT::interpretSrtHandshake(const CHandShake& hs, const CPacket& hspkt, uin
                     HLOGC(mglog.Debug, log << "interpretSrtHandshake: KMREQ processing failed - returned " << res);
                     return false;
                 }
+                if (*out_len == 1)
+                {
+                    // This means that there was an abnormal encryption situation occurred.
+                    // This is inacceptable in case of strict encryption.
+                    if (m_bOPT_StrictEncryption)
+                    {
+                        LOGC(mglog.Error, log << "interpretSrtHandshake: KMREQ result abnornal - rejecting per strict encryption");
+                        return false;
+                    }
+                }
                 encrypted = true;
             }
             else if ( cmd == SRT_CMD_KMRSP )
             {
-                m_pCryptoControl->processSrtMsg_KMRSP(begin+1, bytelen, HS_VERSION_SRT1);
-                // XXX Possible to check status?
+                int res = m_pCryptoControl->processSrtMsg_KMRSP(begin+1, bytelen, HS_VERSION_SRT1);
+                if (m_bOPT_StrictEncryption && res == -1)
+                {
+                    LOGC(mglog.Error, log << "KMRSP failed - rejecting connection as per strict encryption.");
+                    return false;
+                }
                 encrypted = true;
             }
             else if ( cmd == SRT_CMD_NONE )
@@ -2587,6 +2629,12 @@ bool CUDT::interpretSrtHandshake(const CHandShake& hs, const CPacket& hspkt, uin
     // Check if peer declared encryption
     if (!encrypted && m_CryptoSecret.len > 0)
     {
+        if (m_bOPT_StrictEncryption)
+        {
+            LOGC(mglog.Error, log << "HS EXT: Agent declares encryption, but Peer does not - rejecting connection per strict requirement.");
+            return false;
+        }
+
         LOGC(mglog.Error, log << "HS EXT: Agent declares encryption, but Peer does not (Agent can still receive unencrypted packets from Peer).");
 
         // This is required so that the sender is still allowed to send data, when encryption is required,

srtcore/core.h

@@ -559,6 +559,7 @@ class CUDT
     int m_iOPT_PeerTsbPdDelay;       // Peer's Rx latency for the traffic made by Agent's Tx.
     bool m_bOPT_TLPktDrop;           // Whether Agent WILL DO TLPKTDROP on Rx.
     int m_iOPT_SndDropDelay;         // Extra delay when deciding to snd-drop for TLPKTDROP, -1 to off
+    bool m_bOPT_StrictEncryption;    // Off by default. When on, any connection other than nopw-nopw & pw1-pw1 is rejected.
     std::string m_sStreamName;
 
     int m_iTsbPdDelay_ms;                           // Rx delay to absorb burst in milliseconds

srtcore/crypto.cpp

@@ -286,6 +286,7 @@ int CCryptoControl::processSrtMsg_KMREQ(const uint32_t* srtdata, size_t bytelen,
     return SRT_CMD_KMRSP;
 
 HSv4_ErrorReport:
+    srtlen = 1;
 
     if (bidirectional && hasPassphrase())
     {
@@ -311,6 +312,8 @@ int CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int
     size_t srtlen = len/sizeof(uint32_t);
     HtoNLA(srtd, srtdata, srtlen);
 
+    int retstatus = -1;
+
     // Unused?
     //bool bidirectional = hsv > CUDT::HS_VERSION_UDT4;
 
@@ -328,6 +331,7 @@ int CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int
         {
         case SRT_KM_S_BADSECRET:
             m_SndKmState = m_RcvKmState = SRT_KM_S_BADSECRET;
+            retstatus = -1;
             break;
 
             // Default embraces two cases:
@@ -338,6 +342,7 @@ int CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int
             // This means that the peer did not set the password, while Agent did.
             m_RcvKmState = SRT_KM_S_UNSECURED;
             m_SndKmState = SRT_KM_S_NOSECRET;
+            retstatus = -1;
             break;
 
         case SRT_KM_S_UNSECURED:
@@ -346,13 +351,15 @@ int CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int
             // but can't decrypt what Peer would send.
             m_RcvKmState = SRT_KM_S_NOSECRET;
             m_SndKmState = SRT_KM_S_UNSECURED;
+            retstatus = 0;
             break;
 
         default:
             LOGC(mglog.Fatal, log << "processSrtMsg_KMRSP: IPE: unknown peer error state: "
                     << KmStateStr(peerstate) << " (" << int(peerstate) << ")");
             m_RcvKmState = SRT_KM_S_NOSECRET;
             m_SndKmState = SRT_KM_S_NOSECRET;
+            retstatus = -1; //This is IPE
             break;
         }
 
@@ -371,9 +378,11 @@ int CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int
         {
             m_SndKmState = m_RcvKmState = SRT_KM_S_SECURED;
             HLOGC(mglog.Debug, log << "processSrtMsg_KMRSP: KM response matches " << (key1 ? "EVEN" : "ODD") << " key");
+            retstatus = 1;
         }
         else
         {
+            retstatus = -1;
             LOGC(mglog.Error, log << "processSrtMsg_KMRSP: IPE??? KM response key matches no key");
             /* XXX INSECURE
             LOGC(mglog.Error, log << "processSrtMsg_KMRSP: KM response: [" << FormatBinaryString((uint8_t*)srtd, len)
@@ -389,7 +398,7 @@ int CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int
 
     LOGP(mglog.Note, FormatKmMessage("processSrtMsg_KMRSP", SRT_CMD_KMRSP, len));
 
-    return SRT_CMD_NONE;
+    return retstatus;
 }
 
 void CCryptoControl::sendKeysToPeer(Whether2RegenKm regen)

srtcore/crypto.h

@@ -115,6 +115,11 @@ class CCryptoControl
 
     // Detailed processing
     int processSrtMsg_KMREQ(const uint32_t* srtdata, size_t len, uint32_t* srtdata_out, ref_t<size_t> r_srtlen, int hsv);
+
+    // This returns:
+    // 1 - the given payload is the same as the currently used key
+    // 0 - there's no key in agent or the payload is error message with agent NOSECRET.
+    // -1 - the payload is error message with other state or it doesn't match the key
     int processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, int hsv);
     void createFakeSndContext();
 

srtcore/srt.h

@@ -172,13 +172,14 @@ typedef enum SRT_SOCKOPT {
     SRTO_RCVLATENCY,      // TsbPd receiver delay (mSec) to absorb burst of missed packet retransmission
     SRTO_PEERLATENCY,     // Minimum value of the TsbPd receiver delay (mSec) for the opposite side (peer)
     SRTO_MINVERSION,      // Minimum SRT version needed for the peer (peers with less version will get connection reject)
-    SRTO_STREAMID,         // A string set to a socket and passed to the listener's accepted socket
-    SRTO_SMOOTHER,         // Smoother selection (congestion control algorithm)
-    SRTO_MESSAGEAPI,
-    SRTO_PAYLOADSIZE,
-    SRTO_TRANSTYPE,         // Transmission type (set of options required for given transmission type)
-    SRTO_KMREFRESHRATE,
-    SRTO_KMPREANNOUNCE
+    SRTO_STREAMID,        // A string set to a socket and passed to the listener's accepted socket
+    SRTO_SMOOTHER,        // Smoother selection (congestion control algorithm)
+    SRTO_MESSAGEAPI,      // In File mode, use message API (portions of data with boundaries)
+    SRTO_PAYLOADSIZE,     // Maximum payload size sent in one UDP packet (0 if unlimited)
+    SRTO_TRANSTYPE,       // Transmission type (set of options required for given transmission type)
+    SRTO_KMREFRESHRATE,   // After sending how many packets the encryption key should be flipped to the new key
+    SRTO_KMPREANNOUNCE,   // How many packets before key flip the new key is annnounced and after key flip the old one decommissioned
+    SRTO_STRICTENC,       // Connection to be rejected or quickly broken when one side encryption set or bad password
 } SRT_SOCKOPT;
 
 // DEPRECATED OPTIONS: