]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Merge pull request #7576 from cmouse/remotebackend
authorRemi Gacogne <rgacogne@users.noreply.github.com>
Thu, 14 Mar 2019 10:48:10 +0000 (11:48 +0100)
committerGitHub <noreply@github.com>
Thu, 14 Mar 2019 10:48:10 +0000 (11:48 +0100)
remotebackend: http connector - Properly escape parameters

modules/remotebackend/httpconnector.cc
modules/remotebackend/pipeconnector.cc
modules/remotebackend/remotebackend.cc
modules/remotebackend/remotebackend.hh
modules/remotebackend/zmqconnector.cc

index 950aa27878bf728e34b6cca7afe8e180b44ca11e..bf74849590e59fffc1905c427732fee045db66f2 100644 (file)
 #define UNIX_PATH_MAX 108
 #endif
 
-HTTPConnector::HTTPConnector(std::map<std::string,std::string> options) {
+HTTPConnector::HTTPConnector(std::map<std::string,std::string> options): d_socket(nullptr) {
+
+    if (options.find("url") == options.end()) {
+      throw PDNSException("Cannot find 'url' option in the remote backend HTTP connector's parameters");
+    }
+
     this->d_url = options.find("url")->second;
+
+    try {
+      YaHTTP::URL url(d_url);
+      d_host = url.host;
+      d_port = url.port;
+    }
+    catch(const std::exception& e) {
+      throw PDNSException("Error parsing the 'url' option provided to the remote backend HTTP connector: " + std::string(e.what()));
+    }
+
     if (options.find("url-suffix") != options.end()) {
       this->d_url_suffix = options.find("url-suffix")->second;
     } else {
@@ -44,7 +59,6 @@ HTTPConnector::HTTPConnector(std::map<std::string,std::string> options) {
     this->timeout = 2;
     this->d_post = false;
     this->d_post_json = false;
-    this->d_socket = NULL;
 
     if (options.find("timeout") != options.end()) {
       this->timeout = std::stoi(options.find("timeout")->second)/1000;
@@ -63,15 +77,12 @@ HTTPConnector::HTTPConnector(std::map<std::string,std::string> options) {
     }
 }
 
-HTTPConnector::~HTTPConnector() {
-    if (d_socket != NULL)
-      delete d_socket;
-}
+HTTPConnector::~HTTPConnector() { }
 
 void HTTPConnector::addUrlComponent(const Json &parameters, const string& element, std::stringstream& ss) {
     std::string sparam;
     if (parameters[element] != Json())
-       ss << "/" << asString(parameters[element]);
+       ss << "/" << YaHTTP::Utility::encodeURL(asString(parameters[element]), false);
 }
 
 std::string HTTPConnector::buildMemberListArgs(std::string prefix, const Json& args) {
@@ -81,9 +92,9 @@ std::string HTTPConnector::buildMemberListArgs(std::string prefix, const Json& a
         if (pair.second.is_bool()) {
           stream << (pair.second.bool_value()?"1":"0");
         } else if (pair.second.is_null()) {
-          stream << prefix << "[" << pair.first << "]=";
+          stream << prefix << "[" << YaHTTP::Utility::encodeURL(pair.first, false) << "]=";
         } else {
-          stream << prefix << "[" << pair.first << "]=" << this->asString(pair.second);
+          stream << prefix << "[" << YaHTTP::Utility::encodeURL(pair.first, false) << "]=" << YaHTTP::Utility::encodeURL(this->asString(pair.second), false);
         }
         stream << "&";
     }
@@ -305,7 +316,7 @@ int HTTPConnector::send_message(const Json& input) {
     out << req;
 
     // try sending with current socket, if it fails retry with new socket
-    if (this->d_socket != NULL) {
+    if (this->d_socket != nullptr) {
       fd = this->d_socket->getHandle();
       // there should be no data waiting
       if (waitForRWData(fd, true, 0, 1000) < 1) {
@@ -322,48 +333,41 @@ int HTTPConnector::send_message(const Json& input) {
 
     if (rv == 1) return rv;
 
-    delete this->d_socket;
-    this->d_socket = NULL;
-
-    if (req.url.protocol == "unix") {
-      // connect using unix socket
-    } else {
-      // connect using tcp
-      struct addrinfo *gAddr, *gAddrPtr, hints;
-      std::string sPort = std::to_string(req.url.port);
-      memset(&hints,0,sizeof hints);
-      hints.ai_family = AF_UNSPEC;
-      hints.ai_flags = AI_ADDRCONFIG; 
-      hints.ai_socktype = SOCK_STREAM;
-      hints.ai_protocol = 6; // tcp
-      if ((ec = getaddrinfo(req.url.host.c_str(), sPort.c_str(), &hints, &gAddr)) == 0) {
-        // try to connect to each address. 
-        gAddrPtr = gAddr;
-  
-        while(gAddrPtr) {
-          try {
-            d_socket = new Socket(gAddrPtr->ai_family, gAddrPtr->ai_socktype, gAddrPtr->ai_protocol);
-            d_addr.setSockaddr(gAddrPtr->ai_addr, gAddrPtr->ai_addrlen);
-            d_socket->connect(d_addr);
-            d_socket->setNonBlocking();
-            d_socket->writenWithTimeout(out.str().c_str(), out.str().size(), timeout);
-            rv = 1;
-          } catch (NetworkError& ne) {
-            g_log<<Logger::Error<<"While writing to HTTP endpoint "<<d_addr.toStringWithPort()<<": "<<ne.what()<<std::endl;
-          } catch (...) {
-            g_log<<Logger::Error<<"While writing to HTTP endpoint "<<d_addr.toStringWithPort()<<": exception caught"<<std::endl;
-          }
-
-          if (rv > -1) break;
-          delete d_socket;
-          d_socket = NULL;
-          gAddrPtr = gAddrPtr->ai_next;
-          
+    this->d_socket.reset();
+
+    // connect using tcp
+    struct addrinfo *gAddr, *gAddrPtr, hints;
+    std::string sPort = std::to_string(d_port);
+    memset(&hints,0,sizeof hints);
+    hints.ai_family = AF_UNSPEC;
+    hints.ai_flags = AI_ADDRCONFIG;
+    hints.ai_socktype = SOCK_STREAM;
+    hints.ai_protocol = IPPROTO_TCP;
+    if ((ec = getaddrinfo(d_host.c_str(), sPort.c_str(), &hints, &gAddr)) == 0) {
+      // try to connect to each address.
+      gAddrPtr = gAddr;
+
+      while(gAddrPtr) {
+        try {
+          d_socket = std::unique_ptr<Socket>(new Socket(gAddrPtr->ai_family, gAddrPtr->ai_socktype, gAddrPtr->ai_protocol));
+          d_addr.setSockaddr(gAddrPtr->ai_addr, gAddrPtr->ai_addrlen);
+          d_socket->connect(d_addr);
+          d_socket->setNonBlocking();
+          d_socket->writenWithTimeout(out.str().c_str(), out.str().size(), timeout);
+          rv = 1;
+        } catch (NetworkError& ne) {
+          g_log<<Logger::Error<<"While writing to HTTP endpoint "<<d_addr.toStringWithPort()<<": "<<ne.what()<<std::endl;
+        } catch (...) {
+          g_log<<Logger::Error<<"While writing to HTTP endpoint "<<d_addr.toStringWithPort()<<": exception caught"<<std::endl;
         }
-        freeaddrinfo(gAddr);
-      } else {
-        g_log<<Logger::Error<<"Unable to resolve " << req.url.host << ": " << gai_strerror(ec) << std::endl;
+
+        if (rv > -1) break;
+        d_socket.reset();
+        gAddrPtr = gAddrPtr->ai_next;
       }
+      freeaddrinfo(gAddr);
+    } else {
+      g_log<<Logger::Error<<"Unable to resolve " << d_host << ": " << gai_strerror(ec) << std::endl;
     }
 
     return rv;
@@ -373,7 +377,7 @@ int HTTPConnector::recv_message(Json& output) {
     YaHTTP::AsyncResponseLoader arl;
     YaHTTP::Response resp;
 
-    if (d_socket == NULL ) return -1; // cannot receive :(
+    if (d_socket == nullptr ) return -1; // cannot receive :(
     char buffer[4096];
     int rd = -1;
     bool fail = false;
@@ -396,12 +400,11 @@ int HTTPConnector::recv_message(Json& output) {
         throw NetworkError("timeout");
     } catch (NetworkError &ne) {
       g_log<<Logger::Error<<"While reading from HTTP endpoint "<<d_addr.toStringWithPort()<<": "<<ne.what()<<std::endl; 
-      delete d_socket;
-      d_socket = NULL;
+      d_socket.reset();
       fail = true;
     } catch (...) {
       g_log<<Logger::Error<<"While reading from HTTP endpoint "<<d_addr.toStringWithPort()<<": exception caught"<<std::endl;
-      delete d_socket;
+      d_socket.reset();
       fail = true;
     }
 
index 53721271dc42d9bba2c54837d78e50998eb8d206..a919c553de8cae24649f25d04918fbdb40542452 100644 (file)
@@ -24,7 +24,7 @@
 #endif
 #include "remotebackend.hh"
 
-PipeConnector::PipeConnector(std::map<std::string,std::string> optionsMap) {
+PipeConnector::PipeConnector(std::map<std::string,std::string> optionsMap): d_pid(-1)  {
   if (optionsMap.count("command") == 0) {
     g_log<<Logger::Error<<"Cannot find 'command' option in connection string"<<endl;
     throw PDNSException();
@@ -37,8 +37,6 @@ PipeConnector::PipeConnector(std::map<std::string,std::string> optionsMap) {
      d_timeout = std::stoi(optionsMap.find("timeout")->second);
   }
 
-  d_pid = -1;
-  d_fp = NULL;
   d_fd1[0] = d_fd1[1] = -1;
   d_fd2[0] = d_fd2[1] = -1;
 }
@@ -53,8 +51,9 @@ PipeConnector::~PipeConnector(){
     waitpid(d_pid, &status, 0);
   }
 
-  close(d_fd1[1]);
-  if (d_fp != NULL) fclose(d_fp);
+  if (d_fd1[1]) {
+    close(d_fd1[1]);
+  }
 }
 
 void PipeConnector::launch() {
@@ -85,10 +84,10 @@ void PipeConnector::launch() {
     setCloseOnExec(d_fd1[1]);
     close(d_fd2[1]);
     setCloseOnExec(d_fd2[0]);
-    if(!(d_fp=fdopen(d_fd2[0],"r")))
+    if(!(d_fp=std::unique_ptr<FILE, int(*)(FILE*)>(fdopen(d_fd2[0],"r"), fclose)))
       throw PDNSException("Unable to associate a file pointer with pipe: "+stringerror());
-    if (d_timeout) 
-      setbuf(d_fp,0); // no buffering please, confuses select
+    if (d_timeout)
+      setbuf(d_fp.get(),0); // no buffering please, confuses select
   }
   else if(!d_pid) { // child
     signal(SIGCHLD, SIG_DFL); // silence a warning from perl
@@ -163,15 +162,15 @@ int PipeConnector::recv_message(Json& output)
        tv.tv_usec = (d_timeout % 1000) * 1000;
        fd_set rds;
        FD_ZERO(&rds);
-       FD_SET(fileno(d_fp),&rds);
-       int ret=select(fileno(d_fp)+1,&rds,0,0,&tv);
+       FD_SET(fileno(d_fp.get()),&rds);
+       int ret=select(fileno(d_fp.get())+1,&rds,0,0,&tv);
        if(ret<0) 
          throw PDNSException("Error waiting on data from coprocess: "+stringerror());
        if(!ret)
          throw PDNSException("Timeout waiting for data from coprocess");
      }
 
-     if(!stringfgets(d_fp, receive))
+     if(!stringfgets(d_fp.get(), receive))
        throw PDNSException("Child closed pipe");
   
       s_output.append(receive);
index f3a35157250c6d48807a40e978053a595ef8b1de..c77728a68b62c59e73cb405a1c5b334f19116834 100644 (file)
@@ -71,11 +71,7 @@ RemoteBackend::RemoteBackend(const std::string &suffix)
       build();
 }
 
-RemoteBackend::~RemoteBackend() {
-    if (connector != NULL) {
-       delete connector;
-     }
-}
+RemoteBackend::~RemoteBackend() { }
 
 bool RemoteBackend::send(Json& value) {
    try {
@@ -84,7 +80,7 @@ bool RemoteBackend::send(Json& value) {
      g_log<<Logger::Error<<"Exception caught when sending: "<<ex.reason<<std::endl;
    }
 
-   delete this->connector;
+   this->connector.reset();
    build();
    return false;
 }
@@ -98,7 +94,7 @@ bool RemoteBackend::recv(Json& value) {
      g_log<<Logger::Error<<"Exception caught when receiving"<<std::endl;;
    }
 
-   delete this->connector;
+   this->connector.reset();
    build();
    return false;
 }
@@ -147,17 +143,17 @@ int RemoteBackend::build() {
 
       // connectors know what they are doing
       if (type == "unix") {
-        this->connector = new UnixsocketConnector(options);
+        this->connector = std::unique_ptr<Connector>(new UnixsocketConnector(options));
       } else if (type == "http") {
-        this->connector = new HTTPConnector(options);
+        this->connector = std::unique_ptr<Connector>(new HTTPConnector(options));
       } else if (type == "zeromq") {
 #ifdef REMOTEBACKEND_ZEROMQ
-        this->connector = new ZeroMQConnector(options);
+        this->connector = std::unique_ptr<Connector>(new ZeroMQConnector(options));
 #else
         throw PDNSException("Invalid connection string: zeromq connector support not enabled. Recompile with --enable-remotebackend-zeromq");
 #endif
       } else if (type == "pipe") {
-        this->connector = new PipeConnector(options);
+        this->connector = std::unique_ptr<Connector>(new PipeConnector(options));
       } else {
         throw PDNSException("Invalid connection string: unknown connector");
       }
index 0c64a7deedf4a25a2ad667129a0969ce93af1daf..a794dd1a3b8109cec1d090f96ad71ecc55e2198e 100644 (file)
@@ -102,8 +102,10 @@ class HTTPConnector: public Connector {
     void post_requestbuilder(const Json &input, YaHTTP::Request& req);
     void addUrlComponent(const Json &parameters, const string& element, std::stringstream& ss);
     std::string buildMemberListArgs(std::string prefix, const Json& args);
-    Socket* d_socket;
+    std::unique_ptr<Socket> d_socket;
     ComboAddress d_addr;
+    std::string d_host;
+    uint16_t d_port;
 };
 
 #ifdef REMOTEBACKEND_ZEROMQ
@@ -119,8 +121,8 @@ class ZeroMQConnector: public Connector {
     int d_timeout;
     int d_timespent;
     std::map<std::string,std::string> d_options;
-    void *d_ctx;
-    void *d_sock; 
+    std::unique_ptr<void, int(*)(void*)> d_ctx;
+    std::unique_ptr<void, int(*)(void*)> d_sock;
 };
 #endif
 
@@ -144,7 +146,7 @@ class PipeConnector: public Connector {
   int d_fd1[2], d_fd2[2];
   int d_pid;
   int d_timeout;
-  FILE *d_fp;
+  std::unique_ptr<FILE, int(*)(FILE*)> d_fp{nullptr, fclose};
 };
 
 class RemoteBackend : public DNSBackend
@@ -192,7 +194,7 @@ class RemoteBackend : public DNSBackend
 
   private:
     int build();
-    Connector *connector;
+    std::unique_ptr<Connector> connector;
     bool d_dnssec;
     Json d_result;
     int d_index;
index a525d3365c8f5d4d664d700f1c603c495cbd7828..1c5ae093f67042be834f6aa04f8a945ecf8cdf13 100644 (file)
@@ -25,7 +25,7 @@
 #include "remotebackend.hh"
 #ifdef REMOTEBACKEND_ZEROMQ
 
-ZeroMQConnector::ZeroMQConnector(std::map<std::string,std::string> options) {
+ZeroMQConnector::ZeroMQConnector(std::map<std::string,std::string> options): d_ctx(std::unique_ptr<void, int(*)(void*)>(zmq_init(2), zmq_close)), d_sock(std::unique_ptr<void, int(*)(void*)>(zmq_socket(d_ctx.get(), ZMQ_REQ), zmq_close)) {
   int opt=0;
 
   // lookup timeout, target and stuff
@@ -41,11 +41,9 @@ ZeroMQConnector::ZeroMQConnector(std::map<std::string,std::string> options) {
      this->d_timeout = std::stoi(options.find("timeout")->second);
   }
 
-  d_ctx = zmq_init(2);
-  d_sock = zmq_socket(this->d_ctx, ZMQ_REQ);
-  zmq_setsockopt(d_sock, ZMQ_LINGER, &opt, sizeof(opt));
+  zmq_setsockopt(d_sock.get(), ZMQ_LINGER, &opt, sizeof(opt));
 
-  if(zmq_connect(this->d_sock, this->d_endpoint.c_str()) < 0)
+  if(zmq_connect(this->d_sock.get(), this->d_endpoint.c_str()) < 0)
   {
     g_log<<Logger::Error<<"zmq_connect() failed"<< zmq_strerror(errno)<<std::endl;;
     throw PDNSException("Cannot find 'endpoint' option in connection string");
@@ -65,10 +63,7 @@ ZeroMQConnector::ZeroMQConnector(std::map<std::string,std::string> options) {
   } 
 };
 
-ZeroMQConnector::~ZeroMQConnector() {
-  zmq_close(this->d_sock);
-  zmq_term(this->d_ctx);
-};
+ZeroMQConnector::~ZeroMQConnector() {}
 
 int ZeroMQConnector::send_message(const Json& input) {
    auto line = input.dump();
@@ -80,13 +75,13 @@ int ZeroMQConnector::send_message(const Json& input) {
 
    try {
      zmq_pollitem_t item;
-     item.socket = d_sock;
+     item.socket = d_sock.get();
      item.events = ZMQ_POLLOUT;
      // poll until it's sent or timeout is spent. try to leave 
      // leave few cycles for read. just in case. 
      for(d_timespent = 0; d_timespent < d_timeout-5; d_timespent++) {
        if (zmq_poll(&item, 1, 1)>0) {
-         if(zmq_msg_send(&message, this->d_sock, 0) == -1) {
+         if(zmq_msg_send(&message, this->d_sock.get(), 0) == -1) {
            // message was not sent
            g_log<<Logger::Error<<"Cannot send to " << this->d_endpoint << ": " << zmq_strerror(errno)<<std::endl;
          } else
@@ -107,7 +102,7 @@ int ZeroMQConnector::recv_message(Json& output) {
    zmq_pollitem_t item;
    zmq_msg_t message;
 
-   item.socket = d_sock;
+   item.socket = d_sock.get();
    item.events = ZMQ_POLLIN;
 
    try {
@@ -122,7 +117,7 @@ int ZeroMQConnector::recv_message(Json& output) {
            size_t msg_size;
            zmq_msg_init(&message);
            // read something
-           if(zmq_msg_recv(&message, this->d_sock, ZMQ_NOBLOCK)>0) {
+           if(zmq_msg_recv(&message, this->d_sock.get(), ZMQ_NOBLOCK)>0) {
                string err;
                msg_size = zmq_msg_size(&message);
                data.assign(reinterpret_cast<const char*>(zmq_msg_data(&message)), msg_size);