client code change - Filtering test comes befor Behavior test and separate modes on command line

client/clientmain.cpp

@@ -235,9 +235,17 @@ HRESULT CreateConfigFromCommandLine(ClientCmdLineArgs& args, StunClientLogicConf
             config.fBehaviorTest = true;
             config.fFilteringTest = (fTCP == false); // impossible to to a filtering test in TCP
         }
+        else if (args.strMode == "behavior")
+        {
+            config.fBehaviorTest = true;
+        }
+        else if (args.strMode == "filtering")
+        {
+            config.fFilteringTest = true;
+        }
         else
         {
-            Logging::LogMsg(LL_ALWAYS, "Mode option must be 'full' or 'basic'");
+            Logging::LogMsg(LL_ALWAYS, "Mode option must be 'full', 'basic', 'behavior', or 'filtering'");
         }
     }
 

resources/Makefile

@@ -6,7 +6,7 @@ include ../common.inc
 	sh makecodefile.sh $^ $@ $(*)_text
 
 %.txt: %.md
-	pandoc --no-wrap -s -w plain -o $@ $^
+	cat $^ | sed s/\*\*//g | pandoc --no-wrap -s -w plain -o $@ 
 
 %.1: %.md
 	pandoc -s -w man -o $@ $^

resources/stunclient.1

+.\" Automatically generated by Pandoc 1.16.0.2
+.\"
 .TH "STUNCLIENT" "1" "" "January 22, 2012" "User Manual"
+.hy
 .SH NAME
 .PP
 stunclient \- command line app for the STUN protocol
@@ -47,12 +50,23 @@ The default is 3478 for UDP and TCP.
 .PP
 \f[B]\-\-mode\f[] MODE
 .PP
-Where MODE is either "basic" or "full".
+Where MODE is either "basic", "behavior", or "filtering".
+.PP
 "basic" mode is the default and indicates that the client should perform
 a STUN binding test only.
-"full" mode indicates that the client should attempt to diagnose NAT
-behavior and filtering methodologies if the server supports this mode.
+.PP
+"behavior" mode indicates that the client should attempt to diagnose NAT
+behavior and port mapping methodologies if the server supports this
+mode.
+.PP
+"filtering" mode indicates that the client should attempt to diagnose
+NAT filtering methodologies if the server supports this mode.
 The NAT filtering test is only supported for UDP.
+.PP
+"full" mode is a deprecated mode.
+It performs both a filtering and a behavior test together.
+Users are encouraged to run these tests separately and to avoid using
+the same local port.
 .PP
    *   *   *   *   *
 .PP
@@ -111,9 +125,15 @@ Performs a simple binding test request with the server listening at
 .RS
 .RE
 .TP
-.B stunclient \-\-mode full \-\-localport 9999 12.34.56.78
-Performs a full set of UDP NAT behavior tests from local port 9999 to
-the server listening at IP Address 12.34.56.78 (port 3478)
+.B stunclient \-\-mode filtering \-\-localport 9999 12.34.56.78
+Performs the NAT filtering tests from local port 9999 to the server
+listening at IP Address 12.34.56.78 (port 3478)
+.RS
+.RE
+.TP
+.B stunclient \-\-mode behavior 12.34.56.78
+Performs the NAT behavior and port mapping tests from a random local
+port to the server listening at IP Address 12.34.56.78 (port 3478)
 .RS
 .RE
 .TP

resources/stunclient.md

@@ -43,10 +43,19 @@ _____
 
 **--mode** MODE
 
-Where MODE is either "basic" or "full". "basic" mode is the default and indicates that the
-client should perform a STUN binding test only. "full" mode indicates that the client should
-attempt to diagnose NAT behavior and filtering methodologies if the server supports this mode.
-The NAT filtering test is only supported for UDP.
+Where MODE is either "basic", "behavior", or "filtering".
+
+"basic" mode is the default and indicates that the
+client should perform a STUN binding test only.
+
+"behavior" mode indicates that the client should
+attempt to diagnose NAT behavior and port mapping methodologies if the server supports this mode.
+
+"filtering" mode indicates that the client should
+attempt to diagnose NAT filtering methodologies if the server supports this mode.  The NAT filtering test is only supported for UDP.
+
+"full" mode is a deprecated mode. It performs both a filtering and a behavior test together. Users
+are encouraged to run these tests separately and to avoid using the same local port.
 
 ____
 
@@ -96,8 +105,12 @@ Prints this help page
 stunclient stunserver.org 3478
 :    Performs a simple binding test request with the server listening at "stunserver.org"
 
-stunclient --mode full --localport 9999 12.34.56.78
-:    Performs a full set of UDP NAT behavior tests from local port 9999 to the server listening at IP
+stunclient --mode filtering --localport 9999 12.34.56.78
+:    Performs the NAT filtering tests from local port 9999 to the server listening at IP
+     Address 12.34.56.78 (port 3478)
+
+stunclient --mode behavior 12.34.56.78
+:    Performs the NAT behavior and port mapping tests from a random local port to the server listening at IP
      Address 12.34.56.78 (port 3478)
 
 stunclient --protocol tcp stun.selbie.com

resources/stunserver.1

+.\" Automatically generated by Pandoc 1.16.0.2
+.\"
 .TH "STUNSERVER" "1" "" "January 22, 2012" "User Manual"
+.hy
 .SH NAME
 .PP
 stunserver \- STUN protocol service (RFCs: 3489, 5389, 5789, 5780)
@@ -212,8 +215,7 @@ With no options, starts a basic STUN binding service on UDP port 3478.
 .RS
 .RE
 .TP
-.B stunserver \-\-mode full \-\-primaryinterface 128.34.56.78
-\-\-altinterface 128.34.56.79
+.B stunserver \-\-mode full \-\-primaryinterface 128.34.56.78 \-\-altinterface 128.34.56.79
 Above example starts a dual\-host STUN service on the the interfaces
 identified by the IP address "128.34.56.78" and "128.34.56.79".
 There are four UDP socket listeners
@@ -227,8 +229,7 @@ IP, Alternate Port) 128.34.56.79:3478 (Primary IP, Primary Port)
 An error occurs if the addresses specified do not exist on the local
 host running the service.
 .TP
-.B stunserver \-\-mode full \-\-primaryinterface eth0 \-\-altinterface
-eth1
+.B stunserver \-\-mode full \-\-primaryinterface eth0 \-\-altinterface eth1
 Same as above, except the interfaces are specified by their names as
 enumerated by the system.
 The "ifconfig" or "ipconfig" command will enumerate available interface

stuncore/stunclientlogic.cpp

@@ -108,16 +108,6 @@ HRESULT CStunClientLogic::Initialize(StunClientLogicConfig& config)
     _testlist.push_back(&_test1);
 
 
-    if (_config.fBehaviorTest)
-    {
-        _testBehavior2.Init(&_config, &_results);
-        _testlist.push_back(&_testBehavior2);
-
-        _testBehavior3.Init(&_config, &_results);
-        _testBehavior3.RunAsTest3(true);
-        _testlist.push_back(&_testBehavior3);
-    }
-
     if (_config.fFilteringTest)
     {
         _testFiltering2.Init(&_config, &_results);
@@ -127,6 +117,16 @@ HRESULT CStunClientLogic::Initialize(StunClientLogicConfig& config)
         _testFiltering3.RunAsTest3(true);
         _testlist.push_back(&_testFiltering3);
     }
+    
+    if (_config.fBehaviorTest)
+    {
+        _testBehavior2.Init(&_config, &_results);
+        _testlist.push_back(&_testBehavior2);
+
+        _testBehavior3.Init(&_config, &_results);
+        _testBehavior3.RunAsTest3(true);
+        _testlist.push_back(&_testBehavior3);
+    }
 
     _fPreCheckRunOnTest = false;
 

stuncore/stunclienttests.cpp

@@ -74,13 +74,23 @@ HRESULT CStunClientTestBase::BasicReaderValidation(CRefCountedBuffer& spMsg, CSt
     int cmp = 0;
 
     readerstate = reader.AddBytes(spMsg->GetData(), spMsg->GetSize());
-    ChkIf(readerstate != CStunMessageReader::BodyValidated, E_FAIL);
-
-    reader.GetTransactionId(&transid);
-
-    cmp = memcmp(transid.id, _transid.id, sizeof(_transid));
+    
+    hr = (readerstate == CStunMessageReader::BodyValidated) ? S_OK : E_FAIL;
+    if (FAILED(hr))
+    {
+        Logging::LogMsg(LL_DEBUG, "BasicReaderValidation - body parsing failed");
+    }
+    else
+    {
+        reader.GetTransactionId(&transid);
+        cmp = memcmp(transid.id, _transid.id, sizeof(_transid));
+        hr = (cmp == 0) ? S_OK : E_FAIL;
+        if (FAILED(hr))
+        {
+            Logging::LogMsg(LL_DEBUG, "BasicReaderValidation - transaction id comparison failed");
+        }
+    }
 
-    ChkIf(cmp!=0, E_FAIL);
 
 Cleanup:
     return hr;
@@ -144,7 +154,6 @@ HRESULT CBasicBindingTest::ProcessResponse(CRefCountedBuffer& spMsg, CSocketAddr
     CSocketAddress addrOther;
     bool fHasOtherAddress = false;
 
-    // todo - figure out a way to make buffering TCP fragments work
     Chk(BasicReaderValidation(spMsg, reader));
 
     hr = reader.GetXorMappedAddress(&addrMapped);
@@ -252,10 +261,12 @@ HRESULT CBehaviorTest::GetMessage(CRefCountedBuffer& spMsg, CSocketAddress* pAdd
 
     if (_fIsTest3 == false)
     {
+        Logging::LogMsg(LL_DEBUG, "Preparing message for behavior test #2 (destination=AP)");
         *pAddrDest = _pResults->addrAP;
     }
     else
     {
+        Logging::LogMsg(LL_DEBUG, "Preparing message for behavior test #2 (destination=AA)");
         *pAddrDest = _pResults->addrAA;
     }
 
@@ -369,12 +380,14 @@ HRESULT CFilteringTest::GetMessage(CRefCountedBuffer& spMsg, CSocketAddress* pAd
 
     if (_fIsTest3 == false)
     {
+        Logging::LogMsg(LL_DEBUG, "Preparing message for filtering test #2 (ChangeRequest=AA)");
         change.fChangeIP = true;
         change.fChangePort = true;
         builder.AddChangeRequest(change);
     }
     else
     {
+        Logging::LogMsg(LL_DEBUG, "Preparing message for filtering test #3 (ChangeRequest=PA)");
         change.fChangeIP = false;
         change.fChangePort = true;
         builder.AddChangeRequest(change);