From 5cf8571b01a96a430ee666ca65679353316d0206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 08:55:38 +0100 Subject: [PATCH 01/11] Adding icmp tests. --- include/stuntrace.h | 6 ++++++ src/stuntrace.c | 12 ++++++------ test/stuntrace_test.c | 29 ++++++++++++++++++++++++++--- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/include/stuntrace.h b/include/stuntrace.h index 366a941..d83f4ea 100644 --- a/include/stuntrace.h +++ b/include/stuntrace.h @@ -76,7 +76,13 @@ struct hiutResult { STUN_TRACECB traceCb; }; +bool +isDstUnreachable(const int32_t ICMPtype, + const u_int16_t addrFamily); +bool +isTimeExceeded(const int32_t ICMPtype, + const u_int16_t addrFamily); int StunTrace_startTrace(STUN_CLIENT_DATA* clientData, diff --git a/src/stuntrace.c b/src/stuntrace.c index adfb772..735cb8d 100644 --- a/src/stuntrace.c +++ b/src/stuntrace.c @@ -12,9 +12,9 @@ void StunStatusCallBack(void* userCtx, StunCallBackData_T* stunCbData); -static bool -isDstUnreachable(int32_t ICMPtype, - u_int16_t addrFamily) +bool +isDstUnreachable(const int32_t ICMPtype, + const u_int16_t addrFamily) { if ( ( (ICMPtype == 3) && (addrFamily == AF_INET) ) || ( (ICMPtype == 1) && (addrFamily == AF_INET6) ) ) @@ -24,9 +24,9 @@ isDstUnreachable(int32_t ICMPtype, return false; } -static bool -isTimeExceeded(int32_t ICMPtype, - u_int16_t addrFamily) +bool +isTimeExceeded(const int32_t ICMPtype, + const u_int16_t addrFamily) { if ( ( (ICMPtype == 11) && (addrFamily == AF_INET) ) || ( (ICMPtype == 3) && (addrFamily == AF_INET6) ) ) diff --git a/test/stuntrace_test.c b/test/stuntrace_test.c index ec04ca1..2dfc1e7 100644 --- a/test/stuntrace_test.c +++ b/test/stuntrace_test.c @@ -515,12 +515,35 @@ CTEST(stuntrace, run_IPv4_Stunresp_end) ASSERT_TRUE( LastTTL == 2); - memcpy( &m.msgHdr.id, &LastTransId, STUN_MSG_ID_SIZE); + memcpy(&m.msgHdr.id, &LastTransId, STUN_MSG_ID_SIZE); StunClient_HandleIncResp(clientData, &m, (struct sockaddr*)&remoteAddr); - ASSERT_TRUE( Done); - ASSERT_TRUE( EndOfTrace); + ASSERT_TRUE(Done); + ASSERT_TRUE(EndOfTrace); + +} + +CTEST(stuntrace, isDstUnreachable) +{ + ASSERT_TRUE( isDstUnreachable(3, AF_INET) ); + + ASSERT_FALSE( isDstUnreachable(5,AF_INET) ); + + ASSERT_TRUE( isDstUnreachable(1, AF_INET6) ); + ASSERT_FALSE( isDstUnreachable(3, AF_INET6) ); + +} + +CTEST(stuntrace, isTimeExceeded) +{ + ASSERT_TRUE( isTimeExceeded(11, AF_INET) ); + ASSERT_FALSE( isTimeExceeded(3, AF_INET) ); + ASSERT_FALSE( isTimeExceeded(5, AF_INET) ); + + ASSERT_TRUE( isTimeExceeded(3, AF_INET6) ); + ASSERT_FALSE( isTimeExceeded(11, AF_INET6) ); + ASSERT_FALSE( isTimeExceeded(5, AF_INET6) ); } From 7dd4deeaf43e0622cf19758dbb65252087b9634d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 09:17:23 +0100 Subject: [PATCH 02/11] Behave correctly when far end is "dead". --- src/stuntrace.c | 32 +++++++++++++++--- test/stuntrace_test.c | 77 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/stuntrace.c b/src/stuntrace.c index 735cb8d..9dba51e 100644 --- a/src/stuntrace.c +++ b/src/stuntrace.c @@ -118,9 +118,33 @@ void handleStunNoAnswer(struct hiutResult* result) { result->pathElement[result->currentTTL].inactive = true; + if (result->currentTTL == MAX_TTL) + { + /* Part of far end alive test */ + result->remoteAlive = false; + result->currentTTL = 1; + + stunlib_createId(&result->currStunMsgId, + rand(), result->currentTTL); + + StunClient_startSTUNTrace( (STUN_CLIENT_DATA*)result->stunCtx, + result, + (struct sockaddr*)&result->remoteAddr, + (struct sockaddr*)&result->localAddr, + false, + result->username, + result->password, + result->currentTTL, + result->currStunMsgId, + result->sockfd, + result->sendFunc, + StunStatusCallBack, + NULL ); + return; + } /* Hov many no answer in a row? */ - if (numConcecutiveInactiveNodes(result) >= MAX_CONCECUTIVE_INACTIVE && - !result->remoteAlive) + if ( (numConcecutiveInactiveNodes(result) >= MAX_CONCECUTIVE_INACTIVE) && + !result->remoteAlive ) { bool done = result->num_traces < result->max_recuring ? false : true; result->path_max_ttl = result->currentTTL - MAX_CONCECUTIVE_INACTIVE; @@ -143,7 +167,7 @@ handleStunNoAnswer(struct hiutResult* result) false, false); - if ( result->currentTTL < result->user_max_ttl ) + if (result->currentTTL < result->user_max_ttl) { while (result->pathElement[result->currentTTL].inactive && result->currentTTL < result->path_max_ttl) @@ -327,7 +351,7 @@ handleStunRespSucsessfull(struct hiutResult* result, result->sendFunc, StunStatusCallBack, NULL ); - return; + return; } bool done = result->num_traces < result->max_recuring ? false : true; diff --git a/test/stuntrace_test.c b/test/stuntrace_test.c index 2dfc1e7..5bbff22 100644 --- a/test/stuntrace_test.c +++ b/test/stuntrace_test.c @@ -282,6 +282,8 @@ CTEST(stuntrace, no_answer_IPv4) } + + CTEST(stuntrace, no_answer_recurring_IPv4) { int someData = 3; @@ -459,6 +461,81 @@ CTEST(stuntrace, run_IPv4_Stunresp) } +CTEST(stuntrace, run_IPv4_Stunresp_dead) +{ + int someData = 3; + STUN_CLIENT_DATA* clientData; + + struct sockaddr_storage localAddr, remoteAddr, hop1Addr; + int sockfd = 4; + + sockaddr_initFromString( (struct sockaddr*)&remoteAddr, + "193.200.93.152:45674" ); + + sockaddr_initFromString( (struct sockaddr*)&localAddr, + "192.168.1.34:45674" ); + + StunClient_Alloc(&clientData); + + + StunTrace_startTrace(clientData, + &someData, + (const struct sockaddr*)&remoteAddr, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + sendPacket); + + /*Timeout is roughtly 160*50 ms*/ + for (int i = 0; i < 160; i++) + { + StunClient_HandleTick(clientData, 50); + } + + /* First hop.. */ + ASSERT_TRUE(LastTTL == 1); + sockaddr_initFromString( (struct sockaddr*)&hop1Addr, + "192.168.1.1:45674" ); + StunClient_HandleICMP(clientData, + (struct sockaddr*)&hop1Addr, + 11); + ASSERT_TRUE( sockaddr_alike( (struct sockaddr*)&LastHopAddr, + (struct sockaddr*)&hop1Addr ) ); + + ASSERT_TRUE( LastTTL == 2); + + for (int i = 0; i < 160; i++) + { + StunClient_HandleTick(clientData, 50); + } + ASSERT_TRUE( LastTTL == 3); + + for (int i = 0; i < 160; i++) + { + StunClient_HandleTick(clientData, 50); + } + ASSERT_TRUE( LastTTL == 4); + + for (int i = 0; i < 160; i++) + { + StunClient_HandleTick(clientData, 50); + } + ASSERT_TRUE( LastTTL == 5); + + for (int i = 0; i < 160; i++) + { + StunClient_HandleTick(clientData, 50); + } + ASSERT_TRUE( LastTTL == 5); + + ASSERT_TRUE( Done); + ASSERT_TRUE( EndOfTrace); +} + + CTEST(stuntrace, run_IPv4_Stunresp_end) { int someData = 3; From 588f0e164edf6b02686df3410fca53f6b2198e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 10:02:29 +0100 Subject: [PATCH 03/11] Better handling of max_ttl. --- src/stuntrace.c | 30 ++++++-------- test/stuntrace_test.c | 95 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 93 insertions(+), 32 deletions(-) diff --git a/src/stuntrace.c b/src/stuntrace.c index 9dba51e..7c27a03 100644 --- a/src/stuntrace.c +++ b/src/stuntrace.c @@ -236,7 +236,7 @@ handleStunRespIcmp(struct hiutResult* result, } if ( isTimeExceeded(ICMPtype, srcAddr->sa_family) ) { - if (result->currentTTL < result->user_max_ttl) + if (result->currentTTL < result->user_max_ttl - 1) { result->currentTTL++; while (result->pathElement[result->currentTTL].inactive && @@ -270,24 +270,20 @@ handleStunRespIcmp(struct hiutResult* result, result->sendFunc, StunStatusCallBack, NULL ); - } - else - { - bool done = result->num_traces < result->max_recuring ? false : true; - sendCallback(result, - srcAddr, - ttl, - rtt, - retransmits, - true, - done); - resartIfNotDone(result); + return; } } - else - { - /* do nothing */ - } + + bool done = result->num_traces < result->max_recuring ? false : true; + sendCallback(result, + srcAddr, + ttl, + rtt, + retransmits, + true, + done); + resartIfNotDone(result); + } else if ( isDstUnreachable(ICMPtype,srcAddr->sa_family) ) { diff --git a/test/stuntrace_test.c b/test/stuntrace_test.c index 5bbff22..e823226 100644 --- a/test/stuntrace_test.c +++ b/test/stuntrace_test.c @@ -479,15 +479,15 @@ CTEST(stuntrace, run_IPv4_Stunresp_dead) StunTrace_startTrace(clientData, - &someData, - (const struct sockaddr*)&remoteAddr, - (const struct sockaddr*)&localAddr, - sockfd, - "test", - "tset", - 1, - StunTraceCallBack, - sendPacket); + &someData, + (const struct sockaddr*)&remoteAddr, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + sendPacket); /*Timeout is roughtly 160*50 ms*/ for (int i = 0; i < 160; i++) @@ -511,28 +511,28 @@ CTEST(stuntrace, run_IPv4_Stunresp_dead) { StunClient_HandleTick(clientData, 50); } - ASSERT_TRUE( LastTTL == 3); + ASSERT_TRUE(LastTTL == 3); for (int i = 0; i < 160; i++) { StunClient_HandleTick(clientData, 50); } - ASSERT_TRUE( LastTTL == 4); + ASSERT_TRUE(LastTTL == 4); for (int i = 0; i < 160; i++) { StunClient_HandleTick(clientData, 50); } - ASSERT_TRUE( LastTTL == 5); + ASSERT_TRUE(LastTTL == 5); for (int i = 0; i < 160; i++) { StunClient_HandleTick(clientData, 50); } - ASSERT_TRUE( LastTTL == 5); + ASSERT_TRUE(LastTTL == 5); - ASSERT_TRUE( Done); - ASSERT_TRUE( EndOfTrace); + ASSERT_TRUE(Done); + ASSERT_TRUE(EndOfTrace); } @@ -602,6 +602,71 @@ CTEST(stuntrace, run_IPv4_Stunresp_end) } +CTEST(stuntrace, run_IPv4_Stunresp_max_ttl) +{ + int someData = 3; + STUN_CLIENT_DATA* clientData; + + struct sockaddr_storage localAddr, remoteAddr, hop1Addr; + int sockfd = 4; + + sockaddr_initFromString( (struct sockaddr*)&remoteAddr, + "193.200.93.152:45674" ); + + sockaddr_initFromString( (struct sockaddr*)&localAddr, + "192.168.1.34:45674" ); + + StunClient_Alloc(&clientData); + + + int len = StunTrace_startTrace(clientData, + &someData, + (const struct sockaddr*)&remoteAddr, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + sendPacket); + /* First alive probe */ + ASSERT_TRUE(len == 224); + ASSERT_TRUE(LastTTL == 40); + StunMessage m; + memset( &m, 0, sizeof(m) ); + memcpy( &m.msgHdr.id, &LastTransId, STUN_MSG_ID_SIZE); + memcpy( &m.msgHdr.cookie, StunCookie, sizeof(m.msgHdr.cookie) ); + m.msgHdr.msgType = STUN_MSG_BindResponseMsg; + m.hasXorMappedAddress = true; + m.xorMappedAddress.familyType = STUN_ADDR_IPv4Family; + m.xorMappedAddress.addr.v4.addr = test_addr_ipv4; + m.xorMappedAddress.addr.v4.port = test_port_ipv4; + + StunClient_HandleIncResp(clientData, + &m, + NULL); + + /*Timeout is roughtly 160*50 ms*/ + for (int i = 0; i < 160 * 38; i++) + { + StunClient_HandleTick(clientData, 50); + } + + /* First hop.. */ + ASSERT_TRUE(LastTTL == 39); + sockaddr_initFromString( (struct sockaddr*)&hop1Addr, + "192.168.1.1:45674" ); + StunClient_HandleICMP(clientData, + (struct sockaddr*)&hop1Addr, + 11); + ASSERT_TRUE( sockaddr_alike( (struct sockaddr*)&LastHopAddr, + (struct sockaddr*)&hop1Addr ) ); + + ASSERT_TRUE( Done); + ASSERT_TRUE( EndOfTrace); + +} + CTEST(stuntrace, isDstUnreachable) { ASSERT_TRUE( isDstUnreachable(3, AF_INET) ); From 316651e2c5bfdd710af5f8aaf930b9b4def37625 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 10:26:21 +0100 Subject: [PATCH 04/11] Removing unused code path. Exeption handled earlier. --- src/stuntrace.c | 19 ------------ test/stuntrace_test.c | 72 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/stuntrace.c b/src/stuntrace.c index 7c27a03..750336c 100644 --- a/src/stuntrace.c +++ b/src/stuntrace.c @@ -190,13 +190,6 @@ handleStunNoAnswer(struct hiutResult* result) StunStatusCallBack, NULL ); } - else - { - /* TODO: Callabck here */ - /* - * stopAndExit(result); - */ - } } void @@ -207,8 +200,6 @@ handleStunRespIcmp(struct hiutResult* result, int rtt, int retransmits) { - STUN_CLIENT_DATA* clientData = (STUN_CLIENT_DATA*) result->stunCtx; - if ( (ttl == MAX_TTL) && isDstUnreachable(ICMPtype, srcAddr->sa_family) ) { @@ -273,7 +264,6 @@ handleStunRespIcmp(struct hiutResult* result, return; } } - bool done = result->num_traces < result->max_recuring ? false : true; sendCallback(result, srcAddr, @@ -304,15 +294,6 @@ handleStunRespIcmp(struct hiutResult* result, resartIfNotDone(result); } } - else - { - StunPrint(clientData->logUserData, - clientData->Log_cb, - StunInfoCategory_Trace, - " handleStunRespIcmp: Ignoring ICMP type: %i\n ", - ICMPtype); - - } } void diff --git a/test/stuntrace_test.c b/test/stuntrace_test.c index e823226..be27f63 100644 --- a/test/stuntrace_test.c +++ b/test/stuntrace_test.c @@ -129,6 +129,76 @@ CTEST(stuntrace, run_IPv4) } +CTEST(stuntrace, run_IPv4_unhandled_ICMP) +{ + int someData = 3; + STUN_CLIENT_DATA* clientData; + + Done = false; + EndOfTrace = false; + struct sockaddr_storage localAddr, remoteAddr, hop1Addr, hop2Addr; + int sockfd = 4; + + sockaddr_initFromString( (struct sockaddr*)&remoteAddr, + "193.200.93.152:45674" ); + + sockaddr_initFromString( (struct sockaddr*)&localAddr, + "192.168.1.34:45674" ); + + StunClient_Alloc(&clientData); + + + int len = StunTrace_startTrace(clientData, + &someData, + (const struct sockaddr*)&remoteAddr, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + sendPacket); + /* First alive probe */ + ASSERT_TRUE(len == 224); + ASSERT_TRUE(LastTTL == 40); + + + StunClient_HandleICMP(clientData, + (struct sockaddr*)&remoteAddr, + 3); + ASSERT_FALSE(Done); + ASSERT_FALSE(EndOfTrace); + + /* First hop.. */ + ASSERT_TRUE(LastTTL == 1); + sockaddr_initFromString( (struct sockaddr*)&hop1Addr, + "192.168.1.1:45674" ); + + StunClient_HandleICMP(clientData, + (struct sockaddr*)&hop1Addr, + 11); + ASSERT_TRUE( sockaddr_alike( (struct sockaddr*)&LastHopAddr, + (struct sockaddr*)&hop1Addr ) ); + + ASSERT_TRUE( LastTTL == 2); + StunClient_HandleICMP(clientData, + (struct sockaddr*)&hop1Addr, + 5); + + sockaddr_initFromString( (struct sockaddr*)&hop2Addr, + "193.200.93.152:45674" ); + + StunClient_HandleICMP(clientData, + (struct sockaddr*)&hop2Addr, + 3); + ASSERT_TRUE( sockaddr_alike( (struct sockaddr*)&LastHopAddr, + (struct sockaddr*)&hop2Addr ) ); + ASSERT_TRUE( Done); + ASSERT_TRUE( EndOfTrace); + +} + + CTEST(stuntrace, recurring_IPv4) { int someData = 3; @@ -651,7 +721,7 @@ CTEST(stuntrace, run_IPv4_Stunresp_max_ttl) { StunClient_HandleTick(clientData, 50); } - + /* First hop.. */ ASSERT_TRUE(LastTTL == 39); sockaddr_initFromString( (struct sockaddr*)&hop1Addr, From 4bab871d236a35f8bf3837e52b6ff8bed44030b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 11:11:04 +0100 Subject: [PATCH 05/11] Removing unused code path. --- src/stuntrace.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/stuntrace.c b/src/stuntrace.c index 750336c..bce2791 100644 --- a/src/stuntrace.c +++ b/src/stuntrace.c @@ -359,11 +359,6 @@ StunStatusCallBack(void* userCtx, { struct hiutResult* result = (struct hiutResult*)userCtx; - if (result->pathElement[stunCbData->ttl].gotAnswer) - { - printf("Got this one already! Ignoring (%i)\n", stunCbData->ttl); - return; - } result->pathElement[stunCbData->ttl].gotAnswer = true; switch (stunCbData->stunResult) From 0c7b9b6bb9cd4fd62d15a8f8eba6ac110d2e2abc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 11:32:22 +0100 Subject: [PATCH 06/11] Sanity check of inout at the right place. --- src/stunclient.c | 9 +------ src/stuntrace.c | 8 ++++++ test/stuntrace_test.c | 57 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/src/stunclient.c b/src/stunclient.c index fab7a7c..c435391 100644 --- a/src/stunclient.c +++ b/src/stunclient.c @@ -397,14 +397,7 @@ StunClient_startSTUNTrace(STUN_CLIENT_DATA* clientData, StunMessage stunMsg; uint8_t stunBuff[STUN_MAX_PACKET_SIZE]; uint32_t len; - if (clientData == NULL) - { - StunPrint(clientData->logUserData, - clientData->Log_cb, - StunInfoCategory_Error, - " startStuntrace() failed, Not initialised or no memory"); - return 0; - } + memset( &m, 0, sizeof(m) ); m.userCtx = userCtx; sockaddr_copy( (struct sockaddr*)&m.serverAddr, serverAddr ); diff --git a/src/stuntrace.c b/src/stuntrace.c index bce2791..9f76650 100644 --- a/src/stuntrace.c +++ b/src/stuntrace.c @@ -399,6 +399,14 @@ StunTrace_startTrace(STUN_CLIENT_DATA* clientData, STUN_TRACECB traceCbFunc, STUN_SENDFUNC sendFunc) { + if (clientData == NULL) + { + return 0; + } + if (!sendFunc || !toAddr) + { + return 0; + } struct hiutResult* result; uint32_t len; diff --git a/test/stuntrace_test.c b/test/stuntrace_test.c index be27f63..47bc1f0 100644 --- a/test/stuntrace_test.c +++ b/test/stuntrace_test.c @@ -64,8 +64,63 @@ StunTraceCallBack(void* userCtx, EndOfTrace = data->traceEnd; } +CTEST(stuntrace, null_ptr) +{ + int someData = 3; + STUN_CLIENT_DATA* clientData = NULL; + + struct sockaddr_storage localAddr, remoteAddr; + int sockfd = 4; + + sockaddr_initFromString( (struct sockaddr*)&remoteAddr, + "193.200.93.152:45674" ); + + sockaddr_initFromString( (struct sockaddr*)&localAddr, + "192.168.1.34:45674" ); + + + int len = StunTrace_startTrace(clientData, + &someData, + (const struct sockaddr*)&remoteAddr, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + sendPacket); + + ASSERT_TRUE(len == 0); + + StunClient_Alloc(&clientData); + + len = StunTrace_startTrace(clientData, + &someData, + (const struct sockaddr*)&remoteAddr, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + NULL); + ASSERT_TRUE(len == 0); + + len = StunTrace_startTrace(clientData, + &someData, + NULL, + (const struct sockaddr*)&localAddr, + sockfd, + "test", + "tset", + 1, + StunTraceCallBack, + sendPacket); + ASSERT_TRUE(len == 0); +} + CTEST(stuntrace, run_IPv4) { int someData = 3; @@ -134,7 +189,7 @@ CTEST(stuntrace, run_IPv4_unhandled_ICMP) int someData = 3; STUN_CLIENT_DATA* clientData; - Done = false; + Done = false; EndOfTrace = false; struct sockaddr_storage localAddr, remoteAddr, hop1Addr, hop2Addr; int sockfd = 4; From a52b3460133c377812d0dcba612624fedff1a6c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 12:07:09 +0100 Subject: [PATCH 07/11] More tests. And please no more printing of NULL pinters.. --- src/stunclient.c | 29 ++++++++++++---------------- test/stunclient_test.c | 43 +++++++++++++++++++++++++++++++++++++++++- test/stuntrace_test.c | 4 ++++ 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/stunclient.c b/src/stunclient.c index c435391..a3d8c3e 100644 --- a/src/stunclient.c +++ b/src/stunclient.c @@ -454,7 +454,6 @@ StunClient_HandleIncResp(STUN_CLIENT_DATA* clientData, return; } } - StunPrint(clientData->logUserData, clientData->Log_cb, StunInfoCategory_Trace, @@ -471,14 +470,15 @@ StunClient_HandleICMP(STUN_CLIENT_DATA* clientData, { return; } - /* Todo: Test if this is fr me.. */ + /* Todo: Test if this is for me.. */ StunPrint(clientData->logUserData, - clientData->Log_cb, - StunInfoCategory_Trace, - " StunClient_HandleICMP: Got ICMP type: %i\n ", ICMPtype); + clientData->Log_cb, + StunInfoCategory_Trace, + " StunClient_HandleICMP: Got ICMP type: %i\n ", + ICMPtype); - if ( ( (ICMPtype == 11 || ICMPtype == 3) && (srcAddr->sa_family == AF_INET) ) || - ( (ICMPtype == 3 || ICMPtype == 1) && (srcAddr->sa_family == AF_INET6) ) ) + if ( isTimeExceeded(ICMPtype, srcAddr->sa_family) || + isDstUnreachable(ICMPtype,srcAddr->sa_family) ) { for (int i = 0; i < MAX_STUN_TRANSACTIONS; i++) { @@ -498,15 +498,14 @@ StunClient_HandleICMP(STUN_CLIENT_DATA* clientData, } } + } + else + { StunPrint(clientData->logUserData, clientData->Log_cb, StunInfoCategory_Trace, - " no instance with transId, discarding, ICMP message\n "); - }else{ - StunPrint(clientData->logUserData, - clientData->Log_cb, - StunInfoCategory_Trace, - " StunClient_HandleICMP: Ignoring ICMP Type, nothing to do\n ", ICMPtype); + " StunClient_HandleICMP: Ignoring ICMP Type, nothing to do\n ", + ICMPtype); } } @@ -523,10 +522,6 @@ StunClient_cancelBindingTransaction(STUN_CLIENT_DATA* clientData, { if (clientData == NULL) { - StunPrint(clientData->logUserData, - clientData->Log_cb, - StunInfoCategory_Error, - " cancelBindingTransaction() failed, Not initialised or no memory"); return STUNCLIENT_CTX_UNKNOWN; } diff --git a/test/stunclient_test.c b/test/stunclient_test.c index 1751eff..3464f58 100644 --- a/test/stunclient_test.c +++ b/test/stunclient_test.c @@ -229,6 +229,41 @@ SimBindSuccessResp(bool IPv6, } +static void +SimBindSuccessRespWrongId(bool IPv6, + bool success) +{ + StunMessage m; + memset( &m, 0, sizeof(m) ); + memcpy( &m.msgHdr.id, &m.msgHdr.cookie, STUN_MSG_ID_SIZE); + memcpy( &m.msgHdr.cookie, StunCookie, sizeof(m.msgHdr.cookie) ); + if (success) + { + m.msgHdr.msgType = STUN_MSG_BindResponseMsg; + } + else + { + m.msgHdr.msgType = STUN_MSG_BindErrorResponseMsg; + } + + m.hasXorMappedAddress = true; + + if (IPv6) + { + stunlib_setIP6Address(&m.xorMappedAddress, test_addr_ipv6, 0x4200); + + } + else + { + m.xorMappedAddress.familyType = STUN_ADDR_IPv4Family; + m.xorMappedAddress.addr.v4.addr = test_addr_ipv4; + m.xorMappedAddress.addr.v4.port = test_port_ipv4; + } + + StunClient_HandleIncResp(stunInstance, &m, NULL); + +} + CTEST_SETUP(data) { data->a = 1; @@ -360,7 +395,9 @@ CTEST(stunclient, WaitBindRespNotAut_BindSuccess) StartBindTransaction(0); StunClient_HandleTick(stunInstance, STUN_TICK_INTERVAL_MS); - + /*Just quick sanity if NULL ihandled propperly */ + StunClient_HandleIncResp(NULL, NULL, NULL); + SimBindSuccessRespWrongId(runningAsIPv6, true); SimBindSuccessResp(runningAsIPv6, true); ASSERT_TRUE(stunResult == StunResult_BindOk); StunClient_free(stunInstance); @@ -388,6 +425,10 @@ CTEST(stunclient, CancelTrans_BindResp) ctx = StartBindTransaction(0); StunClient_HandleTick(stunInstance, STUN_TICK_INTERVAL_MS); + /* NULL check first.. */ + ASSERT_FALSE(StunClient_cancelBindingTransaction(NULL, + LastTransId) == ctx); + ASSERT_TRUE(StunClient_cancelBindingTransaction(stunInstance, LastTransId) == ctx); diff --git a/test/stuntrace_test.c b/test/stuntrace_test.c index 47bc1f0..e67e573 100644 --- a/test/stuntrace_test.c +++ b/test/stuntrace_test.c @@ -153,6 +153,10 @@ CTEST(stuntrace, run_IPv4) ASSERT_TRUE(LastTTL == 40); + StunClient_HandleICMP(NULL, + (struct sockaddr*)&remoteAddr, + 3); + StunClient_HandleICMP(clientData, (struct sockaddr*)&remoteAddr, 3); From 8450d430132d7d56a39d4a74fdc937d54e95626e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 12:13:56 +0100 Subject: [PATCH 08/11] Removing unused code. --- src/stunlib.c | 52 ------------------------------------------ test/stunclient_test.c | 9 ++++---- 2 files changed, 5 insertions(+), 56 deletions(-) diff --git a/src/stunlib.c b/src/stunlib.c index bcaa73e..e0eafe4 100644 --- a/src/stunlib.c +++ b/src/stunlib.c @@ -3464,58 +3464,6 @@ stunlib_setIP6Address(StunIPAddress* pIpAddr, } } -int -stunlib_compareIPAddresses(const StunIPAddress* pS1, - const StunIPAddress* pS2) -{ - int res; - if (!pS1 && !pS2) - { - return 0; - } - if (!pS1) - { - return -1; - } - if (!pS2) - { - return 1; - } - - if ( 0 != ( res = (pS1->familyType - pS2->familyType) ) ) - { - return res; - } - if (pS1->familyType == STUN_ADDR_IPv4Family) - { - if ( 0 != ( res = (pS1->addr.v4.port - pS2->addr.v4.port) ) ) - { - return res; - } - if ( 0 != ( res = (pS1->addr.v4.addr - pS2->addr.v4.addr) ) ) - { - return res; - } - } - else - { - int i; - if ( 0 != ( res = (pS1->addr.v6.port - pS2->addr.v6.port) ) ) - { - return res; - } - for (i = 0; i < 4; i++) - { - if ( 0 != (res = pS1->addr.v6.addr[i] - pS2->addr.v6.addr[i]) ) - { - return res; - } - } - } - return 0; -} - - uint32_t stunlib_calculateFingerprint(const uint8_t* buf, size_t len) diff --git a/test/stunclient_test.c b/test/stunclient_test.c index 3464f58..96bd530 100644 --- a/test/stunclient_test.c +++ b/test/stunclient_test.c @@ -140,6 +140,7 @@ StartBindTransaction(int n) false, false, 4567, /* uint64_t + * * * *0x932FF9B151263B36LL @@ -231,12 +232,12 @@ SimBindSuccessResp(bool IPv6, static void SimBindSuccessRespWrongId(bool IPv6, - bool success) + bool success) { StunMessage m; memset( &m, 0, sizeof(m) ); - memcpy( &m.msgHdr.id, &m.msgHdr.cookie, STUN_MSG_ID_SIZE); - memcpy( &m.msgHdr.cookie, StunCookie, sizeof(m.msgHdr.cookie) ); + memcpy( &m.msgHdr.id, &m.msgHdr.cookie, STUN_MSG_ID_SIZE); + memcpy( &m.msgHdr.cookie, StunCookie, sizeof(m.msgHdr.cookie) ); if (success) { m.msgHdr.msgType = STUN_MSG_BindResponseMsg; @@ -427,7 +428,7 @@ CTEST(stunclient, CancelTrans_BindResp) /* NULL check first.. */ ASSERT_FALSE(StunClient_cancelBindingTransaction(NULL, - LastTransId) == ctx); + LastTransId) == ctx); ASSERT_TRUE(StunClient_cancelBindingTransaction(stunInstance, LastTransId) == ctx); From 930876c9010dbd55e0fba611d3e407f8c01b6f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 14:05:31 +0100 Subject: [PATCH 09/11] Fixed unknown attribut decode error and added tests. --- src/stunlib.c | 4 +- test/testvector_test.c | 89 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/stunlib.c b/src/stunlib.c index e0eafe4..d105e23 100644 --- a/src/stunlib.c +++ b/src/stunlib.c @@ -1438,6 +1438,7 @@ stunDecodeUnknownAtr(StunAtrUnknown* pUnk, int* nBufLen, int atrLen) { + uint32_t padLen = calcPadLen(atrLen, 4); int i; if (*nBufLen < atrLen) { @@ -1448,7 +1449,8 @@ stunDecodeUnknownAtr(StunAtrUnknown* pUnk, read_16(pBuf, &pUnk->attrType[i]); } pUnk->numAttributes = i; - *nBufLen -= atrLen; + *nBufLen -= ( atrLen + padLen ); + *pBuf += padLen; if ( i < (atrLen / 2) ) { *nBufLen -= (atrLen - 2 * i); diff --git a/test/testvector_test.c b/test/testvector_test.c index 18bcb19..4f7465d 100644 --- a/test/testvector_test.c +++ b/test/testvector_test.c @@ -89,6 +89,36 @@ static unsigned char respv6[] = "\xc8\xfb\x0b\x4c"; /* CRC32 fingerprint */ + static unsigned char unknwn[] = + "\x00\x01\x00\x58" /* Request type and message length */ + "\x21\x12\xa4\x42" /* Magic cookie */ + "\xb7\xe7\xa7\x01" /* } */ + "\xbc\x34\xd6\x86" /* } Transaction ID */ + "\xfa\x87\xdf\xae" /* } */ + "\x80\x22\x00\x10" /* SOFTWARE attribute header */ + "\x53\x54\x55\x4e" /* } */ + "\x20\x74\x65\x73" /* } User-agent... */ + "\x74\x20\x63\x6c" /* } ...name */ + "\x69\x65\x6e\x74" /* } */ + "\x00\x26\x00\x04" /* Unkown attribute header */ + "\x6e\x00\x01\xff" /* some value */ + "\x80\x29\x00\x08" /* ICE-CONTROLLED attribute header */ + "\x93\x2f\xf9\xb1" /* } Pseudo-random tie breaker... */ + "\x51\x26\x3b\x36" /* } ...for ICE control */ + "\x00\x06\x00\x09" /* USERNAME attribute header */ + "\x65\x76\x74\x6a" /* } */ + "\x3a\x68\x36\x76" /* } Username (9 bytes) and padding (3 bytes) */ + "\x59\x20\x20\x20" /* } */ + "\x00\x08\x00\x14" /* MESSAGE-INTEGRITY attribute header */ + "\x9a\xea\xa7\x0c" /* } */ + "\xbf\xd8\xcb\x56" /* } */ + "\x78\x1e\xf2\xb5" /* } HMAC-SHA1 fingerprint */ + "\xb2\xd3\xf2\x49" /* } */ + "\xc1\xb5\x71\xa2" /* } */ + "\x80\x28\x00\x04" /* FINGERPRINT attribute header */ + "\xe5\x7a\x3b\xcf"; /* CRC32 fingerprint */ + + static const char username[] = "evtj:h6vY"; char password[] = "VOkJxbRl1RmTxUk/WvJxBt"; @@ -999,3 +1029,62 @@ CTEST(testvector, dont_crash_if_atrLen_bogus_on_errors_messages) NULL, NULL) ); } + +CTEST(testvector, unkowns_encode_decode) +{ + StunMessage stunMsg; + StunAtrUnknown unknowns; + unsigned char stunBuf[STUN_MAX_PACKET_SIZE]; + ASSERT_TRUE( stunlib_DecodeMessage(unknwn, + 108, + &stunMsg, + &unknowns, + NULL) ); + + + ASSERT_TRUE( stunMsg.msgHdr.msgType == STUN_MSG_BindRequestMsg); + + ASSERT_TRUE( 0 == memcmp(&stunMsg.msgHdr.id.octet,&idOctet,12) ); + + ASSERT_TRUE( stunMsg.hasUsername); + ASSERT_TRUE( 0 == + memcmp(&stunMsg.username.value, username, + stunMsg.username.sizeValue) ); + + ASSERT_TRUE( stunMsg.hasSoftware); + ASSERT_TRUE( 0 == + memcmp(&stunMsg.software.value, software, + stunMsg.software.sizeValue) ); + + ASSERT_TRUE(unknowns.numAttributes == 1); + + ASSERT_TRUE(stunMsg.hasControlled); + ASSERT_TRUE(stunMsg.controlled.value == tieBreaker); + + + memset( &stunMsg, 0, sizeof(StunMessage) ); + stunMsg.msgHdr.msgType = STUN_MSG_AllocateResponseMsg; + memcpy(&stunMsg.msgHdr.id.octet,&idOctet,12); + stunlib_addError(&stunMsg, "UNKNOWN-ATTRIBUTE", 420, ' '); + stunMsg.hasUnknownAttributes = true; + memcpy(&stunMsg.unknownAttributes, &unknowns, sizeof unknowns); + + memset(stunBuf, 0, sizeof(stunBuf)); + ASSERT_TRUE( stunlib_encodeMessage(&stunMsg, + stunBuf, + sizeof(stunBuf), + (unsigned char*)password, + strlen(password), + NULL) ); + memset(&stunMsg, 0, sizeof stunMsg); + + ASSERT_TRUE( stunlib_DecodeMessage(stunBuf, + 88, + &stunMsg, + NULL, + NULL) ); + ASSERT_TRUE(stunMsg.hasUnknownAttributes); + + + +} From 9663878c84327f5891b1303b5bc5ff15ebce8cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 14:18:05 +0100 Subject: [PATCH 10/11] stun message length test. --- test/testvector_test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/testvector_test.c b/test/testvector_test.c index 4f7465d..ae631ba 100644 --- a/test/testvector_test.c +++ b/test/testvector_test.c @@ -1084,7 +1084,9 @@ CTEST(testvector, unkowns_encode_decode) NULL, NULL) ); ASSERT_TRUE(stunMsg.hasUnknownAttributes); +} - - +CTEST(testvector, stun_msg_len) +{ + ASSERT_TRUE( stunlib_StunMsgLen(unknwn) == 88 ); } From de16ce14bd8946791929445b6faf6d1d343b0792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l-Erik=20Martinsen?= Date: Fri, 22 Jan 2016 14:32:54 +0100 Subject: [PATCH 11/11] quick test to see if redericting to /dev/null works --- test/testvector_test.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/testvector_test.c b/test/testvector_test.c index ae631ba..2aad0ad 100644 --- a/test/testvector_test.c +++ b/test/testvector_test.c @@ -1035,11 +1035,12 @@ CTEST(testvector, unkowns_encode_decode) StunMessage stunMsg; StunAtrUnknown unknowns; unsigned char stunBuf[STUN_MAX_PACKET_SIZE]; + FILE *f = fopen("/dev/null", "w+"); ASSERT_TRUE( stunlib_DecodeMessage(unknwn, 108, &stunMsg, &unknowns, - NULL) ); + f) ); ASSERT_TRUE( stunMsg.msgHdr.msgType == STUN_MSG_BindRequestMsg); @@ -1075,7 +1076,7 @@ CTEST(testvector, unkowns_encode_decode) sizeof(stunBuf), (unsigned char*)password, strlen(password), - NULL) ); + f) ); memset(&stunMsg, 0, sizeof stunMsg); ASSERT_TRUE( stunlib_DecodeMessage(stunBuf,