From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 4 Aug 2017 09:49:22 +0000 (+0200)
Subject: dnsdist: Copy the DNS header before encrypting it in place
X-Git-Tag: dnsdist-1.2.0~30^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=57847d6518525db98487275e20d49a3595a374ed;p=pdns

dnsdist: Copy the DNS header before encrypting it in place

When DNSCrypt is enabled, we encrypt the answer in place, but we
need to keep a copy of the DNS header of the original answer to
be able to populate the responses ring buffer.
---

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index 70696d3de..b5fe2fd5b 100644
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -233,6 +233,11 @@ void* tcpClientThread(int pipefd)
 #ifdef HAVE_PROTOBUF
   boost::uuids::random_generator uuidGenerator;
 #endif
+#ifdef HAVE_DNSCRYPT
+  /* when the answer is encrypted in place, we need to get a copy
+     of the original header before encryption to fill the ring buffer */
+  dnsheader dhCopy;
+#endif
 
   map<ComboAddress,int> sockets;
   for(;;) {
@@ -365,7 +370,7 @@ void* tcpClientThread(int pipefd)
 	if(dq.dh->qr) { // something turned it into a response
           restoreFlags(dh, origFlags);
 #ifdef HAVE_DNSCRYPT
-          if (!encryptResponse(queryBuffer, &dq.len, dq.size, true, dnsCryptQuery)) {
+          if (!encryptResponse(queryBuffer, &dq.len, dq.size, true, dnsCryptQuery, nullptr, nullptr)) {
             goto drop;
           }
 #endif
@@ -413,7 +418,7 @@ void* tcpClientThread(int pipefd)
             }
 
 #ifdef HAVE_DNSCRYPT
-            if (!encryptResponse(cachedResponse, &cachedResponseSize, sizeof cachedResponse, true, dnsCryptQuery)) {
+            if (!encryptResponse(cachedResponse, &cachedResponseSize, sizeof cachedResponse, true, dnsCryptQuery, nullptr, nullptr)) {
               goto drop;
             }
 #endif
@@ -433,7 +438,7 @@ void* tcpClientThread(int pipefd)
             dq.dh->qr = true;
 
 #ifdef HAVE_DNSCRYPT
-            if (!encryptResponse(queryBuffer, &dq.len, dq.size, true, dnsCryptQuery)) {
+            if (!encryptResponse(queryBuffer, &dq.len, dq.size, true, dnsCryptQuery, nullptr, nullptr)) {
               goto drop;
             }
 #endif
@@ -569,7 +574,7 @@ void* tcpClientThread(int pipefd)
 	}
 
 #ifdef HAVE_DNSCRYPT
-        if (!encryptResponse(response, &responseLen, responseSize, true, dnsCryptQuery)) {
+        if (!encryptResponse(response, &responseLen, responseSize, true, dnsCryptQuery, &dh, &dhCopy)) {
           goto drop;
         }
 #endif
diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index a5cb24ce1..bf54ddbf6 100644
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -331,10 +331,17 @@ bool fixUpResponse(char** response, uint16_t* responseLen, size_t* responseSize,
 }
 
 #ifdef HAVE_DNSCRYPT
-bool encryptResponse(char* response, uint16_t* responseLen, size_t responseSize, bool tcp, std::shared_ptr<DnsCryptQuery> dnsCryptQuery)
+bool encryptResponse(char* response, uint16_t* responseLen, size_t responseSize, bool tcp, std::shared_ptr<DnsCryptQuery> dnsCryptQuery, dnsheader** dh, dnsheader* dhCopy)
 {
   if (dnsCryptQuery) {
     uint16_t encryptedResponseLen = 0;
+
+    /* save the original header before encrypting it in place */
+    if (dh != nullptr && *dh != nullptr && dhCopy != nullptr) {
+      memcpy(dhCopy, *dh, sizeof(dnsheader));
+      *dh = dhCopy;
+    }
+
     int res = dnsCryptQuery->ctx->encryptResponse(response, *responseLen, responseSize, dnsCryptQuery, tcp, &encryptedResponseLen);
     if (res == 0) {
       *responseLen = encryptedResponseLen;
@@ -377,15 +384,18 @@ try {
   auto localRespRulactions = g_resprulactions.getLocal();
 #ifdef HAVE_DNSCRYPT
   char packet[4096 + DNSCRYPT_MAX_RESPONSE_PADDING_AND_MAC_SIZE];
+  /* when the answer is encrypted in place, we need to get a copy
+     of the original header before encryption to fill the ring buffer */
+  dnsheader dhCopy;
 #else
   char packet[4096];
 #endif
   static_assert(sizeof(packet) <= UINT16_MAX, "Packet size should fit in a uint16_t");
   vector<uint8_t> rewrittenResponse;
 
-  struct dnsheader* dh = (struct dnsheader*)packet;
   uint16_t queryId = 0;
   for(;;) {
+    dnsheader* dh = reinterpret_cast<struct dnsheader*>(packet);
     try {
       ssize_t got = recv(state->fd, packet, sizeof(packet), 0);
       char * response = packet;
@@ -449,7 +459,7 @@ try {
 
       if (ids->cs && !ids->cs->muted) {
 #ifdef HAVE_DNSCRYPT
-        if (!encryptResponse(response, &responseLen, responseSize, false, ids->dnsCryptQuery)) {
+        if (!encryptResponse(response, &responseLen, responseSize, false, ids->dnsCryptQuery, &dh, &dhCopy)) {
           continue;
         }
 #endif
@@ -1174,7 +1184,7 @@ try
 
         if (!cs->muted) {
 #ifdef HAVE_DNSCRYPT
-          if (!encryptResponse(response, &responseLen, dq.size, false, dnsCryptQuery)) {
+          if (!encryptResponse(response, &responseLen, dq.size, false, dnsCryptQuery, nullptr, nullptr)) {
             continue;
           }
 #endif
@@ -1219,7 +1229,7 @@ try
 
           if (!cs->muted) {
 #ifdef HAVE_DNSCRYPT
-            if (!encryptResponse(cachedResponse, &cachedResponseSize, sizeof cachedResponse, false, dnsCryptQuery)) {
+            if (!encryptResponse(cachedResponse, &cachedResponseSize, sizeof cachedResponse, false, dnsCryptQuery, nullptr, nullptr)) {
               continue;
             }
 #endif
@@ -1246,7 +1256,7 @@ try
           dq.dh->qr = true;
 
 #ifdef HAVE_DNSCRYPT
-          if (!encryptResponse(response, &responseLen, dq.size, false, dnsCryptQuery)) {
+          if (!encryptResponse(response, &responseLen, dq.size, false, dnsCryptQuery, nullptr, nullptr)) {
             continue;
           }
 #endif
diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index e92a72ecf..bfe0bf7a8 100644
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -825,7 +825,7 @@ void restoreFlags(struct dnsheader* dh, uint16_t origFlags);
 extern std::vector<std::tuple<ComboAddress,DnsCryptContext,bool,int, std::string>> g_dnsCryptLocals;
 
 int handleDnsCryptQuery(DnsCryptContext* ctx, char* packet, uint16_t len, std::shared_ptr<DnsCryptQuery>& query, uint16_t* decryptedQueryLen, bool tcp, std::vector<uint8_t>& response);
-bool encryptResponse(char* response, uint16_t* responseLen, size_t responseSize, bool tcp, std::shared_ptr<DnsCryptQuery> dnsCryptQuery);
+bool encryptResponse(char* response, uint16_t* responseLen, size_t responseSize, bool tcp, std::shared_ptr<DnsCryptQuery> dnsCryptQuery, dnsheader** dh, dnsheader* dhCopy);
 #endif
 
 #include "dnsdist-snmp.hh"