From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 2 Sep 2022 10:39:32 +0000 (+0200)
Subject: More strict ENABLE_GSS_TSIG #ifdefs and checking of g_doGSSTSIG.
X-Git-Tag: rec-4.8.0-alpha1~31^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c113acc3dccb96fdd9487258d609ba3022669193;p=thirdparty%2Fpdns.git

More strict ENABLE_GSS_TSIG #ifdefs and checking of g_doGSSTSIG.

In some (low level) code, g_doGSSTSIG cannot be used as the code is shared betwen various executables,
but the functionality should be conditional in the higher level processing.

This is a bit tricky to get right, so carefull checks needed.
---

diff --git a/pdns/auth-main.cc b/pdns/auth-main.cc
index 0d7f13335d..0d52c79328 100644
--- a/pdns/auth-main.cc
+++ b/pdns/auth-main.cc
@@ -109,7 +109,9 @@ int g_luaRecordExecLimit;
 time_t g_luaHealthChecksInterval{5};
 time_t g_luaHealthChecksExpireDelay{3600};
 #endif
+#ifdef ENABLE_GSS_TSIG
 bool g_doGssTSIG;
+#endif
 typedef Distributor<DNSPacket, DNSPacket, PacketHandler> DNSDistributor;
 
 ArgvMap theArg;
@@ -326,7 +328,9 @@ void declareArguments()
   ::arg().setSwitch("consistent-backends", "Assume individual zones are not divided over backends. Send only ANY lookup operations to the backend to reduce the number of lookups") = "yes";
 
   ::arg().set("rng", "Specify the random number generator to use. Valid values are auto,sodium,openssl,getrandom,arc4random,urandom.") = "auto";
+#ifdef ENABLE_GSS_TSIG
   ::arg().setSwitch("enable-gss-tsig", "Enable GSS TSIG processing") = "no";
+#endif
   ::arg().setDefaults();
 }
 
@@ -699,7 +703,9 @@ void mainthread()
   g_luaHealthChecksInterval = ::arg().asNum("lua-health-checks-interval");
   g_luaHealthChecksExpireDelay = ::arg().asNum("lua-health-checks-expire-delay");
 #endif
+#ifdef ENABLE_GSS_TSIG
   g_doGssTSIG = ::arg().mustDo("enable-gss-tsig");
+#endif
 
   DNSPacket::s_udpTruncationThreshold = std::max(512, ::arg().asNum("udp-truncation-threshold"));
   DNSPacket::s_doEDNSSubnetProcessing = ::arg().mustDo("edns-subnet-processing");
diff --git a/pdns/auth-main.hh b/pdns/auth-main.hh
index de840cc0d4..83fb0c8f4a 100644
--- a/pdns/auth-main.hh
+++ b/pdns/auth-main.hh
@@ -60,5 +60,4 @@ extern bool g_doLuaRecord;
 extern bool g_LuaRecordSharedState;
 extern time_t g_luaHealthChecksInterval;
 extern time_t g_luaHealthChecksExpireDelay;
-extern bool g_doGssTSIG;
 #endif // HAVE_LUA_RECORDS
diff --git a/pdns/distributor.hh b/pdns/distributor.hh
index bba617cbca..2f52454ac4 100644
--- a/pdns/distributor.hh
+++ b/pdns/distributor.hh
@@ -38,6 +38,7 @@
 #include "arguments.hh"
 #include <atomic>
 #include "statbag.hh"
+#include "gss_context.hh"
 
 extern StatBag S;
 
@@ -249,7 +250,7 @@ retry:
 
       QD->callback(a, QD->start);
 #ifdef ENABLE_GSS_TSIG
-      if (a != nullptr) {
+      if (g_doGssTSIG && a != nullptr) {
         QD->Q.cleanupGSS(a->d.rcode);
       }
 #endif
@@ -315,7 +316,7 @@ retry:
   }
   callback(a, start);
 #ifdef ENABLE_GSS_TSIG
-  if (a != nullptr) {
+  if (g_doGssTSIG && a != nullptr) {
     q.cleanupGSS(a->d.rcode);
   }
 #endif
diff --git a/pdns/dnspacket.cc b/pdns/dnspacket.cc
index aa43d337e1..1a7de51ed5 100644
--- a/pdns/dnspacket.cc
+++ b/pdns/dnspacket.cc
@@ -763,6 +763,8 @@ const DNSName& DNSPacket::getTSIGKeyname() const {
 #ifdef ENABLE_GSS_TSIG
 void DNSPacket::cleanupGSS(int rcode)
 {
+  // We cannot check g_doGssTSIG here, as this code is also included in other executables
+  // than pdns_server.
   if (rcode != RCode::NoError && d_tsig_algo == TSIG_GSS && !getTSIGKeyname().empty()) {
     GssContext ctx(getTSIGKeyname());
     ctx.destroy();
diff --git a/pdns/gss_context.hh b/pdns/gss_context.hh
index 659df9d2f0..4c746befab 100644
--- a/pdns/gss_context.hh
+++ b/pdns/gss_context.hh
@@ -32,6 +32,7 @@
 #ifdef ENABLE_GSS_TSIG
 #include <gssapi/gssapi.h>
 #include <gssapi/gssapi_krb5.h>
+extern bool g_doGssTSIG;
 #endif
 
 //! Generic errors
diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index d74efbd1dd..f24f7f3988 100644
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -1379,12 +1379,14 @@ std::unique_ptr<DNSPacket> PacketHandler::doQuestion(DNSPacket& p)
       return r;
     } else {
       getTSIGHashEnum(trc.d_algoName, p.d_tsig_algo);
+#ifdef ENABLE_GSS_TSIG
       if (g_doGssTSIG && p.d_tsig_algo == TSIG_GSS) {
         GssContext gssctx(keyname);
         if (!gssctx.getPeerPrincipal(p.d_peer_principal)) {
           g_log<<Logger::Warning<<"Failed to extract peer principal from GSS context with keyname '"<<keyname<<"'"<<endl;
         }
       }
+#endif
     }
     p.setTSIGDetails(trc, keyname, secret, trc.d_mac); // this will get copied by replyPacket()
     noCache=true;
diff --git a/pdns/rfc2136handler.cc b/pdns/rfc2136handler.cc
index 3692857e85..80b8fa3081 100644
--- a/pdns/rfc2136handler.cc
+++ b/pdns/rfc2136handler.cc
@@ -695,7 +695,7 @@ int PacketHandler::processUpdate(DNSPacket& p) {
         g_log<<Logger::Error<<msgPrefix<<"TSIG key required, but packet does not contain key. Sending REFUSED"<<endl;
         return RCode::Refused;
       }
-
+#ifdef ENABLE_GSS_TSIG
       if (g_doGssTSIG && p.d_tsig_algo == TSIG_GSS) {
         GssName inputname(p.d_peer_principal); // match against principal since GSS requires that
         for(const auto& key: tsigKeys) {
@@ -704,7 +704,10 @@ int PacketHandler::processUpdate(DNSPacket& p) {
             break;
           }
         }
-      } else {
+      }
+      else
+#endif
+        {
         for(const auto& key: tsigKeys) {
           if (inputkey == DNSName(key)) { // because checkForCorrectTSIG has already been performed earlier on, if the name of the key matches with the domain given it is valid.
             validKey=true;
diff --git a/pdns/tcpreceiver.cc b/pdns/tcpreceiver.cc
index 81fb0097f2..47a1c29ba1 100644
--- a/pdns/tcpreceiver.cc
+++ b/pdns/tcpreceiver.cc
@@ -411,7 +411,9 @@ void TCPNameserver::doConnection(int fd)
 
       sendPacket(reply, fd);
 #ifdef ENABLE_GSS_TSIG
-      packet->cleanupGSS(reply->d.rcode);
+      if (g_doGssTSIG) {
+        packet->cleanupGSS(reply->d.rcode);
+      }
 #endif
     }
   }
@@ -459,15 +461,18 @@ bool TCPNameserver::canDoAXFR(std::unique_ptr<DNSPacket>& q, bool isAXFR, std::u
       return false;
     } else {
       getTSIGHashEnum(trc.d_algoName, q->d_tsig_algo);
+#ifdef ENABLE_GSS_TSIG
       if (g_doGssTSIG && q->d_tsig_algo == TSIG_GSS) {
         GssContext gssctx(keyname);
         if (!gssctx.getPeerPrincipal(q->d_peer_principal)) {
           g_log<<Logger::Warning<<"Failed to extract peer principal from GSS context with keyname '"<<keyname<<"'"<<endl;
         }
       }
+#endif
     }
 
     DNSSECKeeper dk(packetHandler->getBackend());
+#ifdef ENABLE_GSS_TSIG
     if (g_doGssTSIG && q->d_tsig_algo == TSIG_GSS) {
       vector<string> princs;
       packetHandler->getBackend()->getDomainMetadata(q->qdomain, "GSS-ALLOW-AXFR-PRINCIPAL", princs);
@@ -480,6 +485,7 @@ bool TCPNameserver::canDoAXFR(std::unique_ptr<DNSPacket>& q, bool isAXFR, std::u
       g_log<<Logger::Warning<<"AXFR of domain '"<<q->qdomain<<"' denied: TSIG signed request with principal '"<<q->d_peer_principal<<"' and algorithm 'gss-tsig' is not permitted"<<endl;
       return false;
     }
+#endif
     if(!dk.TSIGGrantsAccess(q->qdomain, keyname)) {
       g_log<<Logger::Warning<<logPrefix<<"denied: key with name '"<<keyname<<"' and algorithm '"<<getTSIGAlgoName(q->d_tsig_algo)<<"' does not grant access"<<endl;
       return false;
diff --git a/pdns/test-distributor_hh.cc b/pdns/test-distributor_hh.cc
index 61cdf43a32..d094668c39 100644
--- a/pdns/test-distributor_hh.cc
+++ b/pdns/test-distributor_hh.cc
@@ -8,7 +8,9 @@
 #include <boost/test/unit_test.hpp>
 #include "distributor.hh"
 #include "dnspacket.hh"
-#include "namespaces.hh" 
+#include "namespaces.hh"
+
+bool g_doGssTSIG = false;
 
 BOOST_AUTO_TEST_SUITE(test_distributor_hh)
 
diff --git a/pdns/tkey.cc b/pdns/tkey.cc
index 143ff69ae9..f7f439afa4 100644
--- a/pdns/tkey.cc
+++ b/pdns/tkey.cc
@@ -32,6 +32,7 @@ void PacketHandler::tkeyHandler(const DNSPacket& p, std::unique_ptr<DNSPacket>&
   tkey_out->d_expiration = tkey_out->d_inception+15;
 
   if (tkey_in.d_mode == 3) { // establish context
+#ifdef ENABLE_GSS_TSIG
     if (g_doGssTSIG) {
       if (tkey_in.d_algo == DNSName("gss-tsig.")) {
         std::vector<std::string> meta;
@@ -59,7 +60,9 @@ void PacketHandler::tkeyHandler(const DNSPacket& p, std::unique_ptr<DNSPacket>&
       } else {
         tkey_out->d_error = 21; // BADALGO
       }
-    } else {
+    } else
+#endif
+      {
       tkey_out->d_error = 21; // BADALGO
 #ifdef ENABLE_GSS_TSIG
       g_log<<Logger::Error<<"GSS-TSIG request but feature not enabled by enable-gss-tsigs setting"<<endl;