Fix regression - closing dnstap stream on bind 9 very slow (#472)

* patch for #461
dmachard · Nov 21, 2023 · 9a19545 · 9a19545
1 parent fb9aeb1
commit 9a19545
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 10 deletions.
diff --git a/Makefile b/Makefile
@@ -1,10 +1,10 @@
 BINARY_NAME := go-dnscollector
 
 GO_VERSION := 1.21
-GO_LOGGER := 0.3.0
+GO_LOGGER := 0.4.0
 GO_POWERDNS_PROTOBUF := 0.2.0
 GO_DNSTAP_PROTOBUF := 0.6.0
-GO_FRAMESTREAM := 0.6.0
+GO_FRAMESTREAM := 0.7.0
 GO_CLIENTSYSLOG := 0.3.0
 
 BUILD_TIME := $(shell LANG=en_US date +"%F_%T_%z")

diff --git a/collectors/dnstap.go b/collectors/dnstap.go
@@ -174,6 +174,18 @@ func (c *Dnstap) HandleConn(conn net.Conn) {
 			break
 		}
 
+		if frame.IsControl() {
+			if err := fs.ResetReceiver(frame); err != nil {
+				if errors.Is(err, io.EOF) {
+					c.LogConnInfo(connID, "framestream reseted by sender")
+				} else {
+					c.LogConnError(connID, "unexpected control framestream - %s", err)
+				}
+
+			}
+			break
+		}
+
 		// send payload to the channel
 		select {
 		case dnstapProcessor.GetChannel() <- frame.Data(): // Successful send to channel

diff --git a/collectors/dnstap_test.go b/collectors/dnstap_test.go
@@ -2,8 +2,10 @@ package collectors
 
 import (
 	"bufio"
+	"fmt"
 	"log"
 	"net"
+	"regexp"
 	"testing"
 	"time"
 
@@ -111,3 +113,57 @@ func Test_DnstapCollector(t *testing.T) {
 		})
 	}
 }
+
+// Testcase for https://github.com/dmachard/go-dnscollector/issues/461
+// Support Bind9 with dnstap closing.
+func Test_DnstapCollector_CloseFrameStream(t *testing.T) {
+	// redirect stdout output to bytes buffer
+	logsChan := make(chan logger.LogEntry, 10)
+	lg := logger.New(true)
+	lg.SetOutputChannel((logsChan))
+
+	config := dnsutils.GetFakeConfig()
+	config.Collectors.Dnstap.SockPath = "/tmp/dnscollector.sock"
+
+	// start the collector in unix mode
+	g := loggers.NewFakeLogger()
+	c := NewDnstap([]dnsutils.Worker{g}, config, lg, "test")
+	if err := c.Listen(); err != nil {
+		log.Fatal("collector listening  error: ", err)
+	}
+
+	go c.Run()
+
+	// simulate dns server connection to collector
+	conn, err := net.Dial(dnsutils.SocketUnix, "/tmp/dnscollector.sock")
+	if err != nil {
+		t.Error("could not connect: ", err)
+	}
+	defer conn.Close()
+
+	r := bufio.NewReader(conn)
+	w := bufio.NewWriter(conn)
+	fs := framestream.NewFstrm(r, w, conn, 5*time.Second, []byte("protobuf:dnstap.Dnstap"), true)
+	if err := fs.InitSender(); err != nil {
+		t.Fatalf("framestream init error: %s", err)
+	}
+
+	// checking reset
+	errClose := fs.ResetSender()
+	if errClose != nil {
+		t.Errorf("reset sender error: %s", errClose)
+	}
+
+	regxp := ".*framestream reseted by sender.*"
+	for entry := range logsChan {
+		fmt.Println(entry)
+		pattern := regexp.MustCompile(regxp)
+		if pattern.MatchString(entry.Message) {
+			break
+		}
+	}
+
+	// cleanup
+	c.Stop()
+
+}
diff --git a/config.yml b/config.yml
@@ -70,7 +70,6 @@ multiplexer:
       dnstap:
         listen-ip: 0.0.0.0
         listen-port: 6000
-        add-dns-payload: false
       transforms:
         normalize:
           qname-lowercase: false

diff --git a/dnsutils/dns_parser.go b/dnsutils/dns_parser.go
@@ -294,7 +294,8 @@ func DecodePayload(dm *DNSMessage, header *DNSHeader, config *Config) error {
 
 	// decode authoritative answers
 	if header.Nscount > 0 {
-		if answers, offsetrr, err := DecodeAnswer(header.Nscount, payloadOffset, dm.DNS.Payload); err == nil {
+		answers, offsetrr, err := DecodeAnswer(header.Nscount, payloadOffset, dm.DNS.Payload)
+		if err == nil { // nolint
 			dm.DNS.DNSRRs.Nameservers = answers
 			payloadOffset = offsetrr
 		} else if dm.DNS.Flags.TC && (errors.Is(err, ErrDecodeDNSAnswerTooShort) || errors.Is(err, ErrDecodeDNSAnswerRdataTooShort) || errors.Is(err, ErrDecodeDNSLabelTooShort)) {

diff --git a/go.mod b/go.mod
@@ -7,8 +7,8 @@ require (
 	github.com/cilium/ebpf v0.12.3
 	github.com/dmachard/go-clientsyslog v0.3.0
 	github.com/dmachard/go-dnstap-protobuf v0.6.0
-	github.com/dmachard/go-framestream v0.6.0
-	github.com/dmachard/go-logger v0.3.0
+	github.com/dmachard/go-framestream v0.7.0
+	github.com/dmachard/go-logger v0.4.0
 	github.com/dmachard/go-powerdns-protobuf v0.2.0
 	github.com/dmachard/go-topmap v0.5.0
 	github.com/farsightsec/golang-framestream v0.3.0

diff --git a/go.sum b/go.sum
@@ -437,10 +437,10 @@ github.com/dmachard/go-clientsyslog v0.3.0 h1:CV6PlG6mr6nYoKjcEZP1RFJudw+Lr7D3ab
 github.com/dmachard/go-clientsyslog v0.3.0/go.mod h1:llRfIIzxlTNsEQbVF6GKUzxORDWTiSvld3ElJcUtyCo=
 github.com/dmachard/go-dnstap-protobuf v0.6.0 h1:L3t+IdUTehkt+Rste7f2QRf5PX9ECHGh1QIvtDsHzeg=
 github.com/dmachard/go-dnstap-protobuf v0.6.0/go.mod h1:GUKK5vMZTZW1gEtpSAdPeg8Egyn1p7pA1WE+FW+VW8I=
-github.com/dmachard/go-framestream v0.6.0 h1:H2DtbkXNIpM/PSuMN/DA1EDGGO5NhN/OBB2K9SkbA8c=
-github.com/dmachard/go-framestream v0.6.0/go.mod h1:f0LF2Npbe4plNgzVJX1rUfoIUjTWZIpLLyhuG04RTo4=
-github.com/dmachard/go-logger v0.3.0 h1:Q7RnOLFCU9V5RSiuFs5cEwsEXTQ4HbvFDjF2H5GPNuQ=
-github.com/dmachard/go-logger v0.3.0/go.mod h1:Gf6Au3CX5l3rZ+Tb3yX31u6h4lwVeZQSBklUI3h8gCA=
+github.com/dmachard/go-framestream v0.7.0 h1:fLwBLIBnG10wwGcS5y6qCoLg6WTD2yAxyezcAvpf61o=
+github.com/dmachard/go-framestream v0.7.0/go.mod h1:f0LF2Npbe4plNgzVJX1rUfoIUjTWZIpLLyhuG04RTo4=
+github.com/dmachard/go-logger v0.4.0 h1:JJJW8C5Ri6OaWIECAE6dUNqLs4ym1+WX3xD6h5MxLI4=
+github.com/dmachard/go-logger v0.4.0/go.mod h1:Gf6Au3CX5l3rZ+Tb3yX31u6h4lwVeZQSBklUI3h8gCA=
 github.com/dmachard/go-powerdns-protobuf v0.2.0 h1:YK3xcTxAe97BynwAv75Mcfvu+VSiwQEWoawfNI3bdnc=
 github.com/dmachard/go-powerdns-protobuf v0.2.0/go.mod h1:jnC3qwk4XifYvT1cBa88sR/XDf0gsTcLofqrdr+0oP4=
 github.com/dmachard/go-topmap v0.5.0 h1:zGWTFTwgsOsCIG7NxoJpWBAV7pGqxqUFdLZ+6A+UFrw=