Introduce makeUNsockaddr and have everybody use it

makeUNsockaddr does the length check in a central place, and calling
code should be a bit more readable now.

diff --git a/modules/pipebackend/coprocess.cc b/modules/pipebackend/coprocess.cc
index 110c5033d562ec4e8aaba2d4907abac72813a3a3..8357781209484f420be4fe49b123c5076c98fb26 100644 (file)
--- a/modules/pipebackend/coprocess.cc
+++ b/modules/pipebackend/coprocess.cc
@@ -171,10 +171,8 @@ UnixRemote::UnixRemote(const string& path, int timeout)
     throw PDNSException("Unable to create UNIX domain socket: "+string(strerror(errno)));
 
   struct sockaddr_un remote;
-  memset(&remote, 0, sizeof(remote));
-  remote.sun_family = AF_UNIX;
-  memset(remote.sun_path, 0, sizeof(remote.sun_path));
-  path.copy(remote.sun_path, sizeof(remote.sun_path), 0);
+  if (makeUNsockaddr(path, &remote))
+    throw PDNSException("Unable to create UNIX domain socket: Path '"+path+"' is not a valid UNIX socket path.");
 
   // fcntl(fd, F_SETFL, O_NONBLOCK, &sock);
 

diff --git a/modules/remotebackend/unixconnector.cc b/modules/remotebackend/unixconnector.cc
index e0871a741285c8ef8202b3f350213188e701194c..1b68353703e5308c49d153ff850a94f2f28c6fa1 100644 (file)
--- a/modules/remotebackend/unixconnector.cc
+++ b/modules/remotebackend/unixconnector.cc
@@ -133,9 +133,12 @@ void UnixsocketConnector::reconnect() {
       L<<Logger::Error<<"Cannot create socket: " << strerror(errno) << std::endl;;
       return;
    }
-   sock.sun_family = AF_UNIX;
-   memset(sock.sun_path, 0, UNIX_PATH_MAX);
-   path.copy(sock.sun_path, UNIX_PATH_MAX, 0);
+
+   if (makeUNsockaddr(path, &sock)) {
+      L<<Logger::Error<<"Unable to create UNIX domain socket: Path '"<<path<<"' is not a valid UNIX socket path."<<std::endl;
+      return;
+   }
+
    if (fcntl(fd, F_SETFL, O_NONBLOCK, &fd)) {
       connected = false;
       L<<Logger::Error<<"Cannot manipulate socket: " << strerror(errno) << std::endl;;

diff --git a/pdns/dynlistener.cc b/pdns/dynlistener.cc
index 117c8b5e8bee4ff4425ff9ab3139c1e826266263..0e80ab15bde04f0baba1ebee699b2d237d9653c5 100644 (file)
--- a/pdns/dynlistener.cc
+++ b/pdns/dynlistener.cc
@@ -94,13 +94,10 @@ bool DynListener::testLive(const string& fname)
     return false;
   }
 
-  memset(&addr, 0, sizeof(addr));
-  addr.sun_family = AF_UNIX;
-  if(fname.length()+1 > sizeof(addr.sun_path)) {
-    L<<Logger::Critical<<"Unable to open controlsocket, path '"<<fname<<"' too long."<<endl;
+  if (makeUNsockaddr(fname, &addr)) {
+    L<<Logger::Critical<<"Unable to open controlsocket, path '"<<fname<<"' is not a valid UNIX socket path."<<endl;
     exit(1);
   }
-  strcpy(addr.sun_path, fname.c_str());
 
   int status = connect(fd, (struct sockaddr*)&addr, sizeof(addr));
   close(fd);
@@ -120,13 +117,10 @@ void DynListener::listenOnUnixDomain(const string& fname)
   }
 
   struct sockaddr_un local;
-  memset(&local,0,sizeof(local));
-  local.sun_family=AF_UNIX;
-  if(fname.length()+1 > sizeof(local.sun_path)) {
-    L<<Logger::Critical<<"Unable to bind to controlsocket, path '"<<fname<<"' too long."<<endl;
+  if (makeUNsockaddr(fname, &local)) {
+    L<<Logger::Critical<<"Unable to bind to controlsocket, path '"<<fname<<"' is not a valid UNIX socket path."<<endl;
     exit(1);
   }
-  strcpy(local.sun_path, fname.c_str());
   
   createSocketAndBind(AF_UNIX, (struct sockaddr*)& local, sizeof(local));
   d_socketname=fname;

diff --git a/pdns/dynlistener.hh b/pdns/dynlistener.hh
index 827ecd27037d1f312790268d0d679ba0dd8113a8..bcec327b3e5e88fb3735d5f07e859dd6b98a9260 100644 (file)
--- a/pdns/dynlistener.hh
+++ b/pdns/dynlistener.hh
@@ -65,12 +65,10 @@ private:
   void listenOnTCP(const ComboAddress&);
   void createSocketAndBind(int family, struct sockaddr*local, size_t len);
 
-#ifndef WIN32
-  struct sockaddr_un d_remote;
-#else
+#ifdef WIN32
   HANDLE m_pipeHandle;
 #endif // WIN32
-  
+
   Utility::socklen_t d_addrlen;
   NetmaskGroup d_tcprange;
   int d_s;

diff --git a/pdns/dynmessenger.cc b/pdns/dynmessenger.cc
index eced8f229b3688d957249577e42f35e875908bb2..ba746bf1a95402d80b7088472e02aaa3acaa5442 100644 (file)
--- a/pdns/dynmessenger.cc
+++ b/pdns/dynmessenger.cc
@@ -33,40 +33,38 @@ DynMessenger::DynMessenger(const string &localdir, const string &fname)
   if(d_s<0) {
     throw PDNSException(string("socket")+strerror(errno));
   }
-  
-  memset(&d_local,0,sizeof(d_local));
 
   string localname=localdir;
 
   localname+="/lsockXXXXXX";
-  d_local.sun_family=AF_UNIX;
-  strcpy(d_local.sun_path,localname.c_str());
+  if (makeUNsockaddr(localname, &d_local))
+    throw PDNSException("Unable to bind to local temporary file, path '"+localname+"' is not a valid UNIX socket path.");
 
   if(mkstemp(d_local.sun_path)<0)
-    throw PDNSException("Unable to generate local temporary file: "+string(strerror(errno)));
+    throw PDNSException("Unable to generate local temporary file: "+stringerror());
   
   unlink(d_local.sun_path);
-  
-  if(bind(d_s, (sockaddr*)&d_local,sizeof(d_local))<0) {
-    unlink(d_local.sun_path);
-    throw PDNSException("Unable to bind to local temporary file: "+string(strerror(errno)));
-  }
-  
-  if(chmod(d_local.sun_path,0666)<0) { // make sure that pdns can reply!
-    unlink(d_local.sun_path);
-    perror("fchmod");
-    exit(1);
-  }
 
-  memset(&d_remote,0,sizeof(d_remote));
-  
-  d_remote.sun_family=AF_UNIX;
-  strcpy(d_remote.sun_path,fname.c_str());
-  if(connect(d_s,(sockaddr*)&d_remote,sizeof(d_remote))<0) {
+  try {
+    if(bind(d_s, (sockaddr*)&d_local,sizeof(d_local))<0)
+      throw PDNSException("Unable to bind to local temporary file: "+stringerror());
+
+    // make sure that pdns can reply!
+    if(chmod(d_local.sun_path,0666)<0)
+      throw PDNSException("Unable to chmod local temporary file: "+stringerror());
+
+    if(makeUNsockaddr(fname, &d_remote))
+      throw PDNSException("Unable to connect to remote '"+fname+"': Path is not a valid UNIX socket path.");
+
+    if(connect(d_s,(sockaddr*)&d_remote,sizeof(d_remote))<0)
+      throw PDNSException("Unable to connect to remote '"+fname+"': "+stringerror());
+
+  } catch(...) {
+    close(d_s);
+    d_s=-1;
     unlink(d_local.sun_path);
-    throw PDNSException("Unable to connect to remote '"+fname+"': "+string(strerror(errno)));
+    throw;
   }
-  
 }
 
 DynMessenger::DynMessenger(const ComboAddress& remote, const string &secret)
@@ -81,6 +79,7 @@ DynMessenger::DynMessenger(const ComboAddress& remote, const string &secret)
   
   if(connect(d_s, (sockaddr*)&remote, remote.getSocklen())<0) {
     close(d_s);
+    d_s=-1;
     throw PDNSException("Unable to connect to remote '"+remote.toStringWithPort()+"': "+string(strerror(errno)));
   }
 
@@ -90,9 +89,10 @@ DynMessenger::DynMessenger(const ComboAddress& remote, const string &secret)
 
 DynMessenger::~DynMessenger()
 {
+  if (d_s > 0)
+    close(d_s);
   if(*d_local.sun_path && unlink(d_local.sun_path)<0)
     cerr<<"Warning: unable to unlink local unix domain endpoint: "<<strerror(errno)<<endl;
-  close(d_s);
 }   
 
 int DynMessenger::send(const string &msg) const

diff --git a/pdns/misc.cc b/pdns/misc.cc
index 866711052b8be116977c1b59f8ed34574d69852e..874a9561da7a8eb434790ffbedb1bc490b0d781c 100644 (file)
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -23,6 +23,7 @@
 #include <sys/time.h>
 #include <time.h>
 #include <netinet/in.h>
+#include <sys/un.h>
 #include <unistd.h>
 #endif // WIN32
 
@@ -713,7 +714,7 @@ int makeIPv6sockaddr(const std::string& addr, struct sockaddr_in6* ret)
   return 0;
 }
 
-int makeIPv4sockaddr(const string &str, struct sockaddr_in* ret)
+int makeIPv4sockaddr(const std::string& str, struct sockaddr_in* ret)
 {
   if(str.empty()) {
     return -1;
@@ -744,6 +745,19 @@ int makeIPv4sockaddr(const string &str, struct sockaddr_in* ret)
   return -1;
 }
 
+int makeUNsockaddr(const std::string& path, struct sockaddr_un* ret)
+{
+  if (path.empty())
+    return -1;
+
+  memset(ret, 0, sizeof(struct sockaddr_un));
+  ret->sun_family = AF_UNIX;
+  if (path.length() >= sizeof(ret->sun_path))
+    return -1;
+
+  path.copy(ret->sun_path, sizeof(ret->sun_path), 0);
+  return 0;
+}
 
 //! read a line of text from a FILE* to a std::string, returns false on 'no data'
 bool stringfgets(FILE* fp, std::string& line)

diff --git a/pdns/misc.hh b/pdns/misc.hh
index 1742d695a7143712c04c511b899cd0ec2546e109..505db2982b8e4ea63ee42ec980113ce20572307b 100644 (file)
--- a/pdns/misc.hh
+++ b/pdns/misc.hh
@@ -476,7 +476,8 @@ string makeRelative(const std::string& fqdn, const std::string& zone);
 string labelReverse(const std::string& qname);
 std::string dotConcat(const std::string& a, const std::string &b);
 int makeIPv6sockaddr(const std::string& addr, struct sockaddr_in6* ret);
-int makeIPv4sockaddr(const string &str, struct sockaddr_in* ret);
+int makeIPv4sockaddr(const std::string& str, struct sockaddr_in* ret);
+int makeUNsockaddr(const std::string& path, struct sockaddr_un* ret);
 bool stringfgets(FILE* fp, std::string& line);
 
 template<typename Index>

diff --git a/pdns/rec_channel.cc b/pdns/rec_channel.cc
index 3c36438517ba15ad96575566deedfb2e443a229a..1b28b1edcd1d18e9e6c0e825ab42e998014b44d9 100644 (file)
--- a/pdns/rec_channel.cc
+++ b/pdns/rec_channel.cc
@@ -33,24 +33,21 @@ int RecursorControlChannel::listen(const string& fname)
   Utility::setCloseOnExec(d_fd);
 
   if(d_fd < 0) 
-    throw PDNSException("Creating UNIX domain socket: "+string(strerror(errno)));
+    throw PDNSException("Creating UNIX domain socket: "+stringerror());
   
   int tmp=1;
   if(setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0)
-    throw PDNSException(string("Setsockopt failed: ")+strerror(errno));
+    throw PDNSException("Setsockopt failed: "+stringerror());
   
   int err=unlink(fname.c_str());
   if(err < 0 && errno!=ENOENT)
-    throw PDNSException("Can't remove (previous) controlsocket '"+fname+"': "+string(strerror(errno)) + " (try --socket-dir)");
+    throw PDNSException("Can't remove (previous) controlsocket '"+fname+"': "+stringerror() + " (try --socket-dir)");
 
-  memset(&d_local,0,sizeof(d_local));
-  d_local.sun_family=AF_UNIX;
-  if(fname.length()+1 > sizeof(d_local.sun_path))
-    throw PDNSException("Unable to bind to controlsocket, path '"+fname+"' too long.");
-  strcpy(d_local.sun_path, fname.c_str());
+  if(makeUNsockaddr(fname, &d_local))
+    throw PDNSException("Unable to bind to controlsocket, path '"+fname+"' is not a valid UNIX socket path.");
     
   if(bind(d_fd, (sockaddr*)&d_local,sizeof(d_local))<0) 
-    throw PDNSException("Unable to bind to controlsocket '"+fname+"': "+string(strerror(errno)));
+    throw PDNSException("Unable to bind to controlsocket '"+fname+"': "+stringerror());
 
   return d_fd;
 }
@@ -64,49 +61,41 @@ void RecursorControlChannel::connect(const string& path, const string& fname)
 
   if(d_fd < 0) 
     throw PDNSException("Creating UNIX domain socket: "+string(strerror(errno)));
+
+  try {
+    int tmp=1;
+    if(setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0)
+      throw PDNSException("Setsockopt failed: "+stringerror());
   
-  int tmp=1;
-  if(setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0) {
-    close(d_fd);
-    d_fd=-1;
-    throw PDNSException(string("Setsockopt failed: ")+strerror(errno));
-  }
-  
-  string localname=path+"/lsockXXXXXX";
-  strcpy(d_local.sun_path, localname.c_str());
+    string localname=path+"/lsockXXXXXX";
+    if (makeUNsockaddr(localname, &d_local))
+      throw PDNSException("Unable to bind to local temporary file, path '"+localname+"' is not a valid UNIX socket path.");
 
-  if(mkstemp(d_local.sun_path) < 0) {
-    close(d_fd);
-    d_fd=-1;
-    d_local.sun_path[0]=0;
-    throw PDNSException("Unable to generate local temporary file in directory '"+path+"': "+string(strerror(errno)));
-  }
+    if(mkstemp(d_local.sun_path) < 0)
+      throw PDNSException("Unable to generate local temporary file in directory '"+path+"': "+stringerror());
 
-  d_local.sun_family=AF_UNIX;
+    int err=unlink(d_local.sun_path);
+    if(err < 0 && errno!=ENOENT)
+      throw PDNSException("Unable to remove local controlsocket: "+stringerror());
 
-  int err=unlink(d_local.sun_path);
-  if(err < 0 && errno!=ENOENT)
-    throw PDNSException("Unable to remove local controlsocket: "+string(strerror(errno)));
+    if(bind(d_fd, (sockaddr*)&d_local,sizeof(d_local))<0)
+      throw PDNSException("Unable to bind to local temporary file: "+stringerror());
 
-  if(bind(d_fd, (sockaddr*)&d_local,sizeof(d_local))<0) {
-    unlink(d_local.sun_path);
-    close(d_fd);
-    d_fd=-1;
-    throw PDNSException("Unable to bind to local temporary file: "+string(strerror(errno)));
-  }
+    if(chmod(d_local.sun_path,0666)<0) // make sure that pdns can reply!
+      throw PDNSException("Unable to chmod local temporary socket: "+stringerror());
 
-  if(chmod(d_local.sun_path,0666)<0) { // make sure that pdns can reply!
-    unlink(d_local.sun_path);
-    throw PDNSException("Unable to chmnod local temporary socket: "+string(strerror(errno)));
-  }
+    string remotename=path+"/"+fname;
+    if (makeUNsockaddr(remotename, &remote))
+      throw PDNSException("Unable to connect to controlsocket, path '"+remotename+"' is not a valid UNIX socket path.");
 
-  memset(&remote,0,sizeof(remote));
-  
-  remote.sun_family=AF_UNIX;
-  strcpy(remote.sun_path,(path+"/"+fname).c_str());
-  if(::connect(d_fd, (sockaddr*)&remote, sizeof(remote)) < 0) {
-    unlink(d_local.sun_path);
-    throw PDNSException("Unable to connect to remote '"+string(remote.sun_path)+"': "+string(strerror(errno)));
+    if(::connect(d_fd, (sockaddr*)&remote, sizeof(remote)) < 0)
+      throw PDNSException("Unable to connect to remote '"+string(remote.sun_path)+"': "+stringerror());
+
+  } catch (...) {
+    close(d_fd);
+    d_fd=-1;
+    d_local.sun_path[0]=0;
+    throw;
   }
 }