Merge pull request #230 from G10h4ck/ss_port_invalid_family

Check sockaddr_storage is not null before using it and call sockaddr_storage_port

libretroshare/src/libretroshare.pro

@@ -494,7 +494,8 @@ HEADERS +=	util/folderiterator.h \
 			util/rsmemcache.h \
 			util/rstickevent.h \
 			util/rsrecogn.h \
-			util/rsscopetimer.h
+			util/rsscopetimer.h \
+			util/stacktrace.h
 
 SOURCES +=	dbase/cachestrapper.cc \
 			dbase/fimonitor.cc \

libretroshare/src/pqi/p3linkmgr.cc

@@ -1571,140 +1571,90 @@ bool   p3LinkMgrIMPL::tryConnectUDP(const RsPeerId &id, const struct sockaddr_st
 
 
 
-
-bool   p3LinkMgrIMPL::retryConnectTCP(const RsPeerId &id)
-{
-	/* Check if we should retry first */
-	{
-		RsStackMutex stack(mLinkMtx); /****** STACK LOCK MUTEX *******/
-	
 /* push all available addresses onto the connect addr stack...
  * with the following exceptions:
-	   	 *   - check local address, see if it is the same network as us
+ *  - id is our own
-		     - check address age. don't add old ones
+ *  - id is not our friend
+ *  - id is already connected
+ *  - id is hidden but of an unkown type
+ *  - we are hidden but id is not
  */
-	
+bool p3LinkMgrIMPL::retryConnectTCP(const RsPeerId &id)
+{
 #ifdef LINKMGR_DEBUG
 		std::cerr << "p3LinkMgrIMPL::retryConnectTCP() id: " << id << std::endl;
 #endif
 
-		if (id == getOwnId()) 
+	if (id == getOwnId()) return false;
+
 	{
-#ifdef LINKMGR_DEBUG
+		RS_STACK_MUTEX(mLinkMtx);
-			rslog(RSL_WARNING, p3connectzone, "p3LinkMgrIMPL::retryConnectTCP() Failed, connecting to own id: ");
+		std::map<RsPeerId, peerConnectState>::iterator it = mFriendList.find(id);
-#endif
+		if ( it == mFriendList.end() ) return false;
-			return false;
+		if ( it->second.state & RS_PEER_S_CONNECTED ) return false;
 	}
 
-	        /* look up the id */
+	// Extract the required info from p3PeerMgr
-	        std::map<RsPeerId, peerConnectState>::iterator it;
-		if (mFriendList.end() == (it = mFriendList.find(id)))
-		{
-#ifdef LINKMGR_DEBUG
-			std::cerr << "p3LinkMgrIMPL::retryConnectTCP() Peer is not Friend" << std::endl;
-#endif
-			return false;
-		}
 
-		/* if already connected -> done */
+	// first possibility - is it a hidden peer
-		if (it->second.state & RS_PEER_S_CONNECTED)
-		{
-#ifdef LINKMGR_DEBUG
-			std::cerr << "p3LinkMgrIMPL::retryConnectTCP() Peer Already Connected" << std::endl;
-#endif
-			return false;
-		}
-	} /****** END of LOCKED ******/
-
-#ifdef LINKMGR_DEBUG
-	std::cerr << "p3LinkMgrIMPL::retryConnectTCP() Getting Address from PeerMgr for : " << id;
-	std::cerr << std::endl;
-#endif
-	/* If we reach here, must retry .... extract the required info from p3PeerMgr */
-
-	/* first possibility - is it a hidden peer */
 	if (mPeerMgr->isHiddenPeer(id))
 	{
 		/* check for valid hidden type */
 		uint32_t type = mPeerMgr->getHiddenType(id);
-		if (type & (~RS_HIDDEN_TYPE_MASK))
+		if ( type & (~RS_HIDDEN_TYPE_MASK) ) return false;
-		{
-#ifdef LINKMGR_DEBUG
-			std::cerr << "p3LinkMgrIMPL::retryConnectTCP() invalid hidden type (" << type << ") -> return false";
-			std::cerr << std::endl;
-#endif
-			return false;
-		}
 
+		/* then we just have one connect attempt via the Proxy */
 		struct sockaddr_storage proxy_addr;
 		std::string domain_addr;
 		uint16_t domain_port;
-
-		/* then we just have one connect attempt via the Proxy */
 		if ( mPeerMgr->getProxyAddress(id, proxy_addr, domain_addr, domain_port) )
 		{
-			RsStackMutex stack(mLinkMtx); /****** STACK LOCK MUTEX *******/
+			RS_STACK_MUTEX(mLinkMtx);
-
+			std::map<RsPeerId, peerConnectState>::iterator it = mFriendList.find(id);
-	        	std::map<RsPeerId, peerConnectState>::iterator it;
+			if (it != mFriendList.end())
-			if (mFriendList.end() != (it = mFriendList.find(id)))
 			{
 				locked_ConnectAttempt_ProxyAddress(&(it->second), type, proxy_addr, domain_addr, domain_port);
 				return locked_ConnectAttempt_Complete(&(it->second));
 			}
 		}
+
 		return false;
 	}
 
-	if (mPeerMgr->isHidden())
+	if (mPeerMgr->isHidden()) return false;
-	{
-#ifdef LINKMGR_DEBUG
-		std::cerr << "p3LinkMgrIMPL::retryConnectTCP() isHidden(): no connection attempts for : " << id;
-		std::cerr << std::endl;
-#endif
-		return false;
-	}
 
 	struct sockaddr_storage lAddr;
 	struct sockaddr_storage eAddr;
 	pqiIpAddrSet histAddrs;
 	std::string dyndns;
-
 	if (mPeerMgr->getConnectAddresses(id, lAddr, eAddr, histAddrs, dyndns))
 	{
-		RsStackMutex stack(mLinkMtx); /****** STACK LOCK MUTEX *******/
+		RS_STACK_MUTEX(mLinkMtx);
 
-	        std::map<RsPeerId, peerConnectState>::iterator it;
+		std::map<RsPeerId, peerConnectState>::iterator it = mFriendList.find(id);
-		if (mFriendList.end() != (it = mFriendList.find(id)))
+		if ( it != mFriendList.end() )
 		{
 			locked_ConnectAttempt_CurrentAddresses(&(it->second), lAddr, eAddr);
 
-			uint16_t dynPort = sockaddr_storage_port(eAddr);
+			uint16_t dynPort = 0;
-			if (!dynPort)
+			if (!sockaddr_storage_isnull(eAddr)) dynPort = sockaddr_storage_port(eAddr);
+			if (!dynPort && !sockaddr_storage_isnull(lAddr))
 				dynPort = sockaddr_storage_port(lAddr);
 			if (dynPort)
-			{
 				locked_ConnectAttempt_AddDynDNS(&(it->second), dyndns, dynPort);
-			}
 
 			locked_ConnectAttempt_HistoricalAddresses(&(it->second), histAddrs);
 
-			/* finish it off */
+			// finish it off
 			return locked_ConnectAttempt_Complete(&(it->second));
 		}
 		else
-		{
+			std::cerr << "p3LinkMgrIMPL::retryConnectTCP() ERROR failed to find friend data : " << id << std::endl;
-			std::cerr << "p3LinkMgrIMPL::retryConnectTCP() ERROR failed to find friend data : " << id;
-			std::cerr << std::endl;
-		}
 	}
 	else
-	{
+		std::cerr << "p3LinkMgrIMPL::retryConnectTCP() ERROR failed to get addresses from PeerMgr for: " << id << std::endl;
-		std::cerr << "p3LinkMgrIMPL::retryConnectTCP() ERROR failed to addresses from PeerMgr for: " << id;
-		std::cerr << std::endl;
-	}
 
 	return false;
-
 }
 
 

libretroshare/src/pqi/p3peermgr.cc

@@ -381,7 +381,7 @@ bool    p3PeerMgrIMPL::getGpgId(const RsPeerId &ssl_id, RsPgpId &gpgId)
 
 bool p3PeerMgrIMPL::isHidden()
 {
-	RsStackMutex stack(mPeerMtx); /****** STACK LOCK MUTEX *******/
+	RS_STACK_MUTEX(mPeerMtx);
 	return mOwnState.hiddenNode;
 }
 

libretroshare/src/retroshare/rspeers.h

@@ -42,6 +42,10 @@
 class RsPeers;
 extern RsPeers *rsPeers;
 
+/* TODO: 2015/12/31 As for type safetyness all those constant must be declared as enum!
+ * C++ now supports typed enum so there is no ambiguity in serialization size
+ */
+
 /* Trust Levels. Should be the same values than what is declared in PGPHandler.h */
 
 const uint32_t RS_TRUST_LVL_UNDEFINED  = 0;
@@ -216,7 +220,7 @@ class RsPeerDetails
 	RsPgpId issuer;
 
 	PGPFingerprintType fpr; /* pgp fingerprint */
-	std::string authcode; 	// (cyril) what is this used for ?????
+	std::string authcode; 	// TODO: 2015/12/31 (cyril) what is this used for ?????
 	std::list<RsPgpId> gpgSigners;
 
 	uint32_t trustLvl;
@@ -234,6 +238,8 @@ class RsPeerDetails
 	uint32_t state;
 	bool actAsServer;
 
+	// TODO: 2015/12/31 to take advantage of multiple connection this must be
+	// replaced by a set of addresses
 	std::string connectAddr ; // current address if connected.
 	uint16_t connectPort ;
 
@@ -274,7 +280,6 @@ class RsPeerDetails
 
 // This class is used to get info about crytographic algorithms used with a
 // particular peer.
-//
 class RsPeerCryptoParams
 {
 public:
@@ -299,13 +304,17 @@ public:
 
 std::ostream &operator<<(std::ostream &out, const RsPeerDetails &detail);
 
+/* TODO: 2015/12/31 this class seems foundamental for RetroShare code
+ * understanding must document it as soon as possible
+ */
 class RsPeers 
 {
 public:
 
-		RsPeers()  { return; }
+	RsPeers() {}
-		virtual ~RsPeers() { return; }
+	virtual ~RsPeers() {}
 
+	// TODO: 2015/12/31 is this dead code?
 	/* Updates ... */
 	// not implemented
 	//virtual bool FriendsChanged() 					= 0;
@@ -322,7 +331,7 @@ class RsPeers
 
 	virtual bool isOnline(const RsPeerId &ssl_id) = 0;
 	virtual bool isFriend(const RsPeerId &ssl_id) = 0;
-		virtual bool    isGPGAccepted(const RsPgpId &gpg_id_is_friend)			= 0; //
+	virtual bool isGPGAccepted(const RsPgpId &gpg_id_is_friend) = 0;
 	virtual std::string getPeerName(const RsPeerId &ssl_id) = 0;
 	virtual std::string getGPGName(const RsPgpId& gpg_id) = 0;
 	virtual bool getPeerDetails(const RsPeerId& ssl_id, RsPeerDetails &d) = 0;
@@ -410,7 +419,9 @@ class RsPeers
 	// ... computes the sharing file permission hint flags set for this peer, that is a combination of
 	// 		RS_FILE_HINTS_NETWORK_WIDE and RS_FILE_HINTS_BROWSABLE.
 	//
-		virtual FileSearchFlags computePeerPermissionFlags(const RsPeerId& peer_id,FileStorageFlags file_sharing_flags,const std::list<std::string>& file_parent_groups) = 0;
+	virtual FileSearchFlags computePeerPermissionFlags(
+			const RsPeerId& peer_id, FileStorageFlags file_sharing_flags,
+			const std::list<std::string>& file_parent_groups) = 0;
 
 	/* Service permission flags */
 

libretroshare/src/rsserver/p3peers.cc

@@ -256,8 +256,6 @@ bool	p3Peers::getPeerDetails(const RsPeerId& id, RsPeerDetails &d)
 	std::cerr << "p3Peers::getPeerDetails() called for id : " << id << std::endl;
 #endif
 
-	// NOW Only for SSL Details.
-
 	RsPeerId sOwnId = AuthSSL::getAuthSSL()->OwnId();
 	peerState ps;
 
@@ -274,24 +272,8 @@ bool	p3Peers::getPeerDetails(const RsPeerId& id, RsPeerDetails &d)
 		return false;
 	}
 
-//			bool res = getGPGDetails(id, d);
-//
-//			d.isOnlyGPGdetail = true;
-//
-//			if(id.length() == 16)
-//				d.service_perm_flags = mPeerMgr->servicePermissionFlags(id) ;
-//			else if(id.length() == 32)
-//				d.service_perm_flags = mPeerMgr->servicePermissionFlags(id) ;
-//			else
-//			{
-//				std::cerr << "p3Peers::getPeerDetails() ERROR not an correct Id: " << id << std::endl;
-//				d.service_perm_flags = RS_SERVICE_PERM_NONE ;
-//			}
-//
-//			return res ;
-
 	/* get from gpg (first), to fill in the sign and trust details */
-	/* don't retrun now, we've got fill in the ssl and connection info */
+	/* don't return now, we've got fill in the ssl and connection info */
 	getGPGDetails(ps.gpg_id, d);
 	d.isOnlyGPGdetail = false;
 
@@ -327,10 +309,28 @@ bool	p3Peers::getPeerDetails(const RsPeerId& id, RsPeerDetails &d)
 		d.hiddenNodePort = 0;
 		d.hiddenType = RS_HIDDEN_TYPE_NONE;
 
+		if (sockaddr_storage_isnull(ps.localaddr))
+		{
+			d.localAddr	= "INVALID_IP";
+			d.localPort	= 0;
+		}
+		else
+		{
 			d.localAddr	= sockaddr_storage_iptostring(ps.localaddr);
 			d.localPort	= sockaddr_storage_port(ps.localaddr);
+		}
+
+		if (sockaddr_storage_isnull(ps.serveraddr))
+		{
+			d.extAddr = "INVALID_IP";
+			d.extPort = 0;
+		}
+		else
+		{
 			d.extAddr = sockaddr_storage_iptostring(ps.serveraddr);
 			d.extPort = sockaddr_storage_port(ps.serveraddr);
+		}
+
 		d.dyndns        = ps.dyndns;
 	
 		std::list<pqiIpAddress>::iterator it;

libretroshare/src/rsserver/p3peers.h

@@ -38,7 +38,7 @@ class p3Peers: public RsPeers
 public:
 
 	p3Peers(p3LinkMgr *lm, p3PeerMgr *pm, p3NetMgr *nm);
-virtual ~p3Peers() { return; }
+	virtual ~p3Peers() {}
 
 	/* Updates ... */
 	virtual bool FriendsChanged();
@@ -49,15 +49,13 @@ virtual const RsPeerId& getOwnId();
 
 	virtual bool haveSecretKey(const RsPgpId& gpg_id) ;
 
-
 	virtual bool getOnlineList(std::list<RsPeerId> &ids);
 	virtual bool getFriendList(std::list<RsPeerId> &ids);
-//virtual bool	getOthersList(std::list<std::string> &ids);
 	virtual bool getPeerCount (unsigned int *friendCount, unsigned int *onlineCount, bool ssl);
 
 	virtual bool isOnline(const RsPeerId &id);
 	virtual bool isFriend(const RsPeerId &id);
-virtual bool    isGPGAccepted(const RsPgpId &gpg_id_is_friend); //
+	virtual bool isGPGAccepted(const RsPgpId &gpg_id_is_friend);
 	virtual std::string getGPGName(const RsPgpId &gpg_id);
 	virtual std::string getPeerName(const RsPeerId& ssl_or_gpg_id);
 	virtual bool getPeerDetails(const RsPeerId& ssl_or_gpg_id, RsPeerDetails &d);
@@ -96,7 +94,7 @@ virtual bool setVisState(const RsPeerId &id, uint16_t vs_disc, uint16_t vs_dht);
 
 	virtual bool getProxyServer(const uint32_t type, std::string &addr, uint16_t &port,uint32_t& status);
 	virtual bool setProxyServer(const uint32_t type, const std::string &addr, const uint16_t port);
-virtual bool isProxyAddress(const uint32_t type,const sockaddr_storage&);
+	virtual bool isProxyAddress(const uint32_t type, const sockaddr_storage &addr);
 
 	virtual void getIPServersList(std::list<std::string>& ip_servers);
 	virtual void allowServerIPDetermination(bool);
@@ -146,7 +144,6 @@ virtual FileSearchFlags computePeerPermissionFlags(const RsPeerId& peer_id,FileS
 	p3LinkMgr *mLinkMgr;
 	p3PeerMgr *mPeerMgr;
 	p3NetMgr *mNetMgr;
-	
 };
 
 #endif

libretroshare/src/util/rsnet_ss.cc

@@ -29,6 +29,7 @@
 #include "util/rsnet.h"
 #include "util/rsstring.h"
 #include "pqi/pqinetwork.h"
+#include "util/stacktrace.h"
 
 /***************************** Internal Helper Fns ******************************/
 
@@ -186,13 +187,12 @@ uint16_t sockaddr_storage_port(const struct sockaddr_storage &addr)
 	{
 		case AF_INET:
 			return sockaddr_storage_ipv4_port(addr);
-			break;
 		case AF_INET6:
 			return sockaddr_storage_ipv6_port(addr);
-			break;
 		default:
 			std::cerr << "sockaddr_storage_port() invalid addr.ss_family" << std::endl;
 			sockaddr_storage_dump(addr);
+			print_stacktrace();
 			break;
 	}
 	return 0;
@@ -508,22 +508,19 @@ bool sockaddr_storage_isnull(const struct sockaddr_storage &addr)
 bool sockaddr_storage_isValidNet(const struct sockaddr_storage &addr)
 {
 #ifdef SS_DEBUG
-	std::cerr << "sockaddr_storage_isValidNet()";
+	std::cerr << "sockaddr_storage_isValidNet()" << std::endl;
-	std::cerr << std::endl;
 #endif
 
 	switch(addr.ss_family)
 	{
 		case AF_INET:
 			return sockaddr_storage_ipv4_isValidNet(addr);
-			break;
 		case AF_INET6:
 			return sockaddr_storage_ipv6_isValidNet(addr);
-			break;
 		default:
 #ifdef SS_DEBUG
-            std::cerr << "sockaddr_storage_isValidNet() INVALID Family - error: " << sockaddr_storage_iptostring(addr);
+			std::cerr << "sockaddr_storage_isValidNet() INVALID Family" << std::endl;
-            std::cerr << std::endl;
+			sockaddr_storage_dump(addr);
 #endif
 			break;
 	}

libretroshare/src/util/stacktrace.h

@@ -0,0 +1,93 @@
+// stacktrace.h (c) 2008, Timo Bingmann from http://idlebox.net/
+// published under the WTFPL v2.0
+
+#ifndef _STACKTRACE_H_
+#define _STACKTRACE_H_
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <execinfo.h>
+#include <cxxabi.h>
+
+/** Print a demangled stack backtrace of the caller function to FILE* out. */
+static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames = 63)
+{
+    fprintf(out, "stack trace:\n");
+
+    // storage array for stack trace address data
+    void* addrlist[max_frames+1];
+
+    // retrieve current stack addresses
+    int addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*));
+
+    if (addrlen == 0) {
+	fprintf(out, "  <empty, possibly corrupt>\n");
+	return;
+    }
+
+    // resolve addresses into strings containing "filename(function+address)",
+    // this array must be free()-ed
+    char** symbollist = backtrace_symbols(addrlist, addrlen);
+
+    // allocate string which will be filled with the demangled function name
+    size_t funcnamesize = 256;
+    char* funcname = (char*)malloc(funcnamesize);
+
+    // iterate over the returned symbol lines. skip the first, it is the
+    // address of this function.
+    for (int i = 1; i < addrlen; i++)
+    {
+	char *begin_name = 0, *begin_offset = 0, *end_offset = 0;
+
+	// find parentheses and +address offset surrounding the mangled name:
+	// ./module(function+0x15c) [0x8048a6d]
+	for (char *p = symbollist[i]; *p; ++p)
+	{
+	    if (*p == '(')
+		begin_name = p;
+	    else if (*p == '+')
+		begin_offset = p;
+	    else if (*p == ')' && begin_offset) {
+		end_offset = p;
+		break;
+	    }
+	}
+
+	if (begin_name && begin_offset && end_offset
+	    && begin_name < begin_offset)
+	{
+	    *begin_name++ = '\0';
+	    *begin_offset++ = '\0';
+	    *end_offset = '\0';
+
+	    // mangled name is now in [begin_name, begin_offset) and caller
+	    // offset in [begin_offset, end_offset). now apply
+	    // __cxa_demangle():
+
+	    int status;
+	    char* ret = abi::__cxa_demangle(begin_name,
+					    funcname, &funcnamesize, &status);
+	    if (status == 0) {
+		funcname = ret; // use possibly realloc()-ed string
+		fprintf(out, "  %s : %s+%s\n",
+			symbollist[i], funcname, begin_offset);
+	    }
+	    else {
+		// demangling failed. Output function name as a C function with
+		// no arguments.
+		fprintf(out, "  %s : %s()+%s\n",
+			symbollist[i], begin_name, begin_offset);
+	    }
+	}
+	else
+	{
+	    // couldn't parse the line? print the whole line.
+	    fprintf(out, "  %s\n", symbollist[i]);
+	}
+    }
+
+    free(funcname);
+    free(symbollist);
+}
+
+#endif // _STACKTRACE_H_