plugin/sign: track zone file's mtime (#4431)

* plugin/sign: track zone file's mtime

Resign if the original zone's mtime is change in some way.

Closes #4407

Signed-off-by: Miek Gieben <miek@miek.nl>

* Update plugin/sign/README.md

Co-authored-by: Chris O'Haver <cohaver@infoblox.com>

Co-authored-by: Yong Tang <yong.tang.github@outlook.com>
Co-authored-by: Chris O'Haver <cohaver@infoblox.com>

3 files changed, 73 insertions, 4 deletions

diff --git a/plugin/sign/README.md b/plugin/sign/README.md
index 6eb4ba8af..64cb7e7af 100644
--- a/plugin/sign/README.md
+++ b/plugin/sign/README.md
@@ -7,9 +7,9 @@
 ## Description
 
 The *sign* plugin is used to sign (see RFC 6781) zones. In this process DNSSEC resource records are
-added. The signatures that sign the resource records sets have an expiration date, this means the
-signing process must be repeated before this expiration data is reached. Otherwise the zone's data
-will go BAD (RFC 4035, Section 5.5). The *sign* plugin takes care of this.
+added to the zone. The signatures that sign the resource records sets have an expiration date. This
+means the signing process must be repeated before this expiration data is reached. Otherwise the
+zone's data will go BAD (RFC 4035, Section 5.5). The *sign* plugin takes care of this.
 
 Only NSEC is supported, *sign* does *not* support NSEC3.
 
@@ -29,7 +29,12 @@ it do key or algorithm rollovers - it just signs.
 
      -  the signature only has 14 days left before expiring.
 
-    Both these dates are only checked on the SOA's signature(s).
+    Both these dates are only checked on the SOA's signature(s). This concerns the DNSSEC data, the
+    *sign* plugin will also take into account and resign if:
+
+     - the **mtime** of the zone file has changed, since the last time it was checked.
+
+     - the signed zone file doesn't exist on disk.
 
  *  Create RRSIGs that have an inception of -3 hours (minus a jitter between 0 and 18 hours)
     and a expiration of +32 (plus a jitter between 0 and 5 days) days for every given DNSKEY.
diff --git a/plugin/sign/resign_test.go b/plugin/sign/resign_test.go
index 2f67f52aa..af8d22904 100644
--- a/plugin/sign/resign_test.go
+++ b/plugin/sign/resign_test.go
@@ -1,9 +1,12 @@
 package sign
 
 import (
+	"os"
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/coredns/caddy"
 )
 
 func TestResignInception(t *testing.T) {
@@ -38,3 +41,52 @@ func TestResignExpire(t *testing.T) {
 		t.Errorf("Expected RRSIG to be invalid for %s, got valid", then.Format(timeFmt))
 	}
 }
+
+func TestResignModTime(t *testing.T) {
+	input := `sign testdata/db.miek.nl miek.nl {
+               key file testdata/Kmiek.nl.+013+59725
+               directory testdata
+       }`
+	c := caddy.NewTestController("dns", input)
+	sign, err := parse(c)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove("testdata/db.miek.nl.signed")
+
+	if len(sign.signers) != 1 {
+		t.Fatalf("Expected 1 signer, got %d", len(sign.signers))
+	}
+	signer := sign.signers[0]
+
+	why := signer.resign()
+	if !strings.Contains(why.Error(), "no such file or directory") {
+		t.Fatalf("Expected %q, got: %s", "no such file or directory", why.Error())
+	}
+
+	// Slightly harder to properly test this, as we need to pull in the zone writing as well.
+	z, err := signer.Sign(time.Now().UTC())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := signer.write(z); err != nil {
+		t.Fatal(err)
+	}
+	if x := signer.modTime; x.IsZero() {
+		t.Errorf("Expected non zero modification time: got: %s", x.Format(timeFmt))
+	}
+
+	why = signer.resign()
+	if why != nil {
+		t.Errorf("Expected not to have to resign the zone, got: %s", why)
+	}
+
+	// set mtime on original zone file and see if we pick it up as a cue to resign
+	if err := os.Chtimes("testdata/db.miek.nl", time.Now(), time.Now()); err != nil {
+		t.Fatal(err)
+	}
+	why = signer.resign()
+	if !strings.Contains(why.Error(), "differs from last seen modification") {
+		t.Errorf("Expecting to resign the zone, but got: %s", why.Error())
+	}
+}
diff --git a/plugin/sign/signer.go b/plugin/sign/signer.go
index d1ea22356..5753077c8 100644
--- a/plugin/sign/signer.go
+++ b/plugin/sign/signer.go
@@ -22,6 +22,7 @@ type Signer struct {
 	origin      string
 	dbfile      string
 	directory   string
+	modTime     time.Time
 	jitterIncep time.Duration
 	jitterExpir time.Duration
 
@@ -41,6 +42,11 @@ func (s *Signer) Sign(now time.Time) (*file.Zone, error) {
 		return nil, err
 	}
 
+	// s.dbfile is a parseable zone file, track the mtime
+	if fi, err := os.Stat(s.dbfile); err == nil {
+		s.modTime = fi.ModTime()
+	}
+
 	mttl := z.Apex.SOA.Minttl
 	ttl := z.Apex.SOA.Header().Ttl
 	inception, expiration := lifetime(now, s.jitterIncep, s.jitterExpir)
@@ -115,6 +121,12 @@ func (s *Signer) resign() error {
 	if err != nil && os.IsNotExist(err) {
 		return err
 	}
+	// if modtime of the input zone file has changed, we will also resign.
+	if fi, err := os.Stat(s.dbfile); err == nil {
+		if !s.modTime.IsZero() && fi.ModTime() != s.modTime {
+			return fmt.Errorf("zone's modification time %s; differs from last seen modification time: %s", fi.ModTime().Format(timeFmt), s.modTime.Format(timeFmt))
+		}
+	}
 
 	now := time.Now().UTC()
 	return resign(rd, now)