Skip to content

Commit

Permalink
plugin/sign: fix signing of authoritative data
Browse files Browse the repository at this point in the history
Don't sign data we are not authoritative for. This adds an AuthWalk
which skips names we should not authoritative for. Adds a few tests to
check this is the case. Generates zones have been compared to
dnssec-signzone.

A number of changes have been made:

* don't add DS records to the apex
* NSEC TTL is the SOA's minttl value (copying bind9)
* Various cleanups
* signer struct was cleaned up: doesn't need ttl, nor expiration or
  inception.
* plugin/sign: remove apex stuff from names()
  This is never used because we will always have other types in the
  apex, because we *ADD* them ourselves, before we sign (DNSKEY, CDS and
  CDNSKEY).

Signed-off-by: Miek Gieben <miek@miek.nl>
  • Loading branch information
miekg committed Nov 25, 2019
1 parent d56dbff commit 6aed527
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 70 deletions.
58 changes: 58 additions & 0 deletions plugin/file/tree/auth_walk.go
@@ -0,0 +1,58 @@
package tree

import (
"github.com/miekg/dns"
)

// AuthWalk performs fn on all authoritative values stored in the tree in
// pre-order depth first. If a non-nil error is returned the AuthWalk was interrupted
// by an fn returning that error. If fn alters stored values' sort
// relationships, future tree operation behaviors are undefined.
//
// The fn function will be called with 3 arguments, the current element, a map containing all
// the RRs for this element and a boolean if this name is considered authoritative.
func (t *Tree) AuthWalk(fn func(*Elem, map[uint16][]dns.RR, bool) error) error {
if t.Root == nil {
return nil
}
return t.Root.authwalk(make(map[string]struct{}), fn)
}

func (n *Node) authwalk(ns map[string]struct{}, fn func(*Elem, map[uint16][]dns.RR, bool) error) error {
if n.Left != nil {
if err := n.Left.authwalk(ns, fn); err != nil {
return err
}
}

// Check if the current name is a subdomain of *any* of the delegated names we've seen, if so, skip this name.
// The ordering of the tree and how we walk if guarantees we see parents first.
if n.Elem.Type(dns.TypeNS) != nil {
ns[n.Elem.Name()] = struct{}{}
}

auth := true
i := 0
for {
j, end := dns.NextLabel(n.Elem.Name(), i)
if end {
break
}
if _, ok := ns[n.Elem.Name()[j:]]; ok {
auth = false
break
}
i++
}

if err := fn(n.Elem, n.Elem.m, auth); err != nil {
return err
}

if n.Right != nil {
if err := n.Right.authwalk(ns, fn); err != nil {
return err
}
}
return nil
}
11 changes: 7 additions & 4 deletions plugin/file/tree/walk.go
Expand Up @@ -2,25 +2,28 @@ package tree

import "github.com/miekg/dns"

// Walk performs fn on all values stored in the tree. If a non-nil error is returned the
// Walk was interrupted by an fn returning that error. If fn alters stored values' sort
// Walk performs fn on all authoritative values stored in the tree in
// pre-order depth first. If a non-nil error is returned the Walk was interrupted
// by an fn returning that error. If fn alters stored values' sort
// relationships, future tree operation behaviors are undefined.
func (t *Tree) Walk(fn func(e *Elem, rrs map[uint16][]dns.RR) error) error {
func (t *Tree) Walk(fn func(*Elem, map[uint16][]dns.RR) error) error {
if t.Root == nil {
return nil
}
return t.Root.walk(fn)
}

func (n *Node) walk(fn func(e *Elem, rrs map[uint16][]dns.RR) error) error {
func (n *Node) walk(fn func(*Elem, map[uint16][]dns.RR) error) error {
if n.Left != nil {
if err := n.Left.walk(fn); err != nil {
return err
}
}

if err := fn(n.Elem, n.Elem.m); err != nil {
return err
}

if n.Right != nil {
if err := n.Right.walk(fn); err != nil {
return err
Expand Down
27 changes: 17 additions & 10 deletions plugin/sign/README.md
Expand Up @@ -9,8 +9,7 @@
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. *Sign* works, but has
a couple of limitations, see the "Bugs" section.
will go BAD (RFC 4035, Section 5.5). The *sign* plugin takes care of this.

Only NSEC is supported, *sign* does not support NSEC3.

Expand All @@ -32,17 +31,21 @@ it do key or algorithm rollovers - it just signs.

Both these dates are only checked on the SOA's signature(s).

* Create signatures that have an inception of -3 hours (minus a jitter between 0 and 18 hours)
* Create RRSIGs that have an inception of -3 hours (minus a jitter between 0 and 18 hours)
and a expiration of +32 days for every given DNSKEY.

* Add NSEC records for all names in the zone. The TTL for these is the negative cache TTL from the
SOA record.

* Add or replace *all* apex CDS/CDNSKEY records with the ones derived from the given keys. For
each key two CDS are created one with SHA1 and another with SHA256.

* Update the SOA's serial number to the *Unix epoch* of when the signing happens. This will
overwrite *any* previous serial number.

Thus there are two ways that dictate when a zone is signed. Normally every 6 days (plus jitter) it
will be resigned. If for some reason we fail this check, the 14 days before expiring kicks in.

There are two ways that dictate when a zone is signed. Normally every 6 days (plus jitter) it will
be resigned. If for some reason we fail this check, the 14 days before expiring kicks in.

Keys are named (following BIND9): `K<name>+<alg>+<id>.key` and `K<name>+<alg>+<id>.private`.
The keys **must not** be included in your zone; they will be added by *sign*. These keys can be
Expand Down Expand Up @@ -144,18 +147,22 @@ example.org example.net {
This will lead to `db.example.org` be signed *twice*, as this entire section is parsed twice because
you have specified the origins `example.org` and `example.net` in the server block.

Forcibly resigning a zone can be accomplished by removing the signed zone file (CoreDNS will keep on
serving it from memory), and sending SIGUSR1 to the process to make it reload and resign the zone
Forcibly resigning a zone can be accomplished by removing the signed zone file (CoreDNS will keep
on serving it from memory), and sending SIGUSR1 to the process to make it reload and resign the zone
file.

## Also See

The DNSSEC RFCs: RFC 4033, RFC 4034 and RFC 4035. And the BCP on DNSSEC, RFC 6781. Further more the
manual pages coredns-keygen(1) and dnssec-keygen(8). And the *file* plugin's documentation.

Coredns-keygen can be found at <https://github.com/coredns/coredns-utils> in the coredns-keygen directory.
Coredns-keygen can be found at
[https://github.com/coredns/coredns-utils](https://github.com/coredns/coredns-utils) in the
coredns-keygen directory.

Other useful DNSSEC tools can be found in [ldns](https://nlnetlabs.nl/projects/ldns/about/), e.g.
`ldns-key2ds` to create DS records from DNSKEYs.

## Bugs

`keys directory` is not implemented. Glue records are currently signed, and no DS records are added
for child zones.
`keys directory` is not implemented.
23 changes: 9 additions & 14 deletions plugin/sign/nsec.go
Expand Up @@ -9,24 +9,19 @@ import (
"github.com/miekg/dns"
)

// names returns the elements of the zone in nsec order. If the returned boolean is true there were
// no other apex records than SOA and NS, which are stored separately.
func names(origin string, z *file.Zone) ([]string, bool) {
// if there are no apex records other than NS and SOA we'll miss the origin
// in this list. Check the first element and if not origin prepend it.
// names returns the elements of the zone in nsec order.
func names(origin string, z *file.Zone) []string {
// There will also be apex records other than NS and SOA (who are kept separate), as we
// are adding DNSKEY and CDS/CDNSKEY records in the apex *before* we sign.
n := []string{}
z.Walk(func(e *tree.Elem, _ map[uint16][]dns.RR) error {
z.AuthWalk(func(e *tree.Elem, _ map[uint16][]dns.RR, auth bool) error {
if !auth {
return nil
}
n = append(n, e.Name())
return nil
})
if len(n) == 0 {
return nil, false
}
if n[0] != origin {
n = append([]string{origin}, n...)
return n, true
}
return n, false
return n
}

// NSEC returns an NSEC record according to name, next, ttl and bitmap. Note that the bitmap is sorted before use.
Expand Down
27 changes: 27 additions & 0 deletions plugin/sign/nsec_test.go
@@ -0,0 +1,27 @@
package sign

import (
"os"
"testing"

"github.com/coredns/coredns/plugin/file"
)

func TestNames(t *testing.T) {
f, err := os.Open("testdata/db.miek.nl_ns")
if err != nil {
t.Error(err)
}
z, err := file.Parse(f, "db.miek.nl_ns", "miek.nl", 0)
if err != nil {
t.Error(err)
}

names := names("miek.nl.", z)
expected := []string{"miek.nl.", "child.miek.nl.", "www.miek.nl."}
for i := range names {
if names[i] != expected[i] {
t.Errorf("Expected %s, got %s", expected[i], names[i])
}
}
}
2 changes: 0 additions & 2 deletions plugin/sign/resign_test.go
Expand Up @@ -13,7 +13,6 @@ func TestResignInception(t *testing.T) {
if x := resign(zr, then); x != nil {
t.Errorf("Expected RRSIG to be valid for %s, got invalid: %s", then.Format(timeFmt), x)
}

// inception starts after this date.
zr = strings.NewReader(`miek.nl. 1800 IN RRSIG SOA 13 2 1800 20190808191936 20190731161936 59725 miek.nl. eU6gI1OkSEbyt`)
if x := resign(zr, then); x == nil {
Expand All @@ -33,7 +32,6 @@ func TestResignExpire(t *testing.T) {
if x := resign(zr, then); x != nil {
t.Errorf("Expected RRSIG to be valid for %s, got invalid: %s", then.Format(timeFmt), x)
}

// expired yesterday
zr = strings.NewReader(`miek.nl. 1800 IN RRSIG SOA 13 2 1800 20190721191936 20190717161936 59725 miek.nl. eU6gI1OkSEbyt`)
if x := resign(zr, then); x == nil {
Expand Down
51 changes: 19 additions & 32 deletions plugin/sign/signer.go
Expand Up @@ -26,10 +26,6 @@ type Signer struct {

signedfile string
stop chan struct{}

expiration uint32
inception uint32
ttl uint32
}

// Sign signs a zone file according to the parameters in s.
Expand All @@ -44,46 +40,31 @@ func (s *Signer) Sign(now time.Time) (*file.Zone, error) {
return nil, err
}

s.inception, s.expiration = lifetime(now, s.jitter)

s.ttl = z.Apex.SOA.Header().Ttl
mttl := z.Apex.SOA.Minttl
ttl := z.Apex.SOA.Header().Ttl
inception, expiration := lifetime(now, s.jitter)
z.Apex.SOA.Serial = uint32(now.Unix())

for _, pair := range s.keys {
pair.Public.Header().Ttl = s.ttl // set TTL on key so it matches the RRSIG.
pair.Public.Header().Ttl = ttl // set TTL on key so it matches the RRSIG.
z.Insert(pair.Public)
z.Insert(pair.Public.ToDS(dns.SHA1))
z.Insert(pair.Public.ToDS(dns.SHA256))
z.Insert(pair.Public.ToDS(dns.SHA1).ToCDS())
z.Insert(pair.Public.ToDS(dns.SHA256).ToCDS())
z.Insert(pair.Public.ToCDNSKEY())
}

names, apex := names(s.origin, z)
names := names(s.origin, z)
ln := len(names)

var nsec *dns.NSEC
if apex {
nsec = NSEC(s.origin, names[(ln+1)%ln], s.ttl, []uint16{dns.TypeSOA, dns.TypeNS, dns.TypeRRSIG, dns.TypeNSEC})
z.Insert(nsec)
}

for _, pair := range s.keys {
rrsig, err := pair.signRRs([]dns.RR{z.Apex.SOA}, s.origin, s.ttl, s.inception, s.expiration)
rrsig, err := pair.signRRs([]dns.RR{z.Apex.SOA}, s.origin, ttl, inception, expiration)
if err != nil {
return nil, err
}
z.Insert(rrsig)
// NS apex may not be set if RR's have been discarded because the origin doesn't match.
if len(z.Apex.NS) > 0 {
rrsig, err = pair.signRRs(z.Apex.NS, s.origin, s.ttl, s.inception, s.expiration)
if err != nil {
return nil, err
}
z.Insert(rrsig)
}
if apex {
rrsig, err = pair.signRRs([]dns.RR{nsec}, s.origin, s.ttl, s.inception, s.expiration)
rrsig, err = pair.signRRs(z.Apex.NS, s.origin, ttl, inception, expiration)
if err != nil {
return nil, err
}
Expand All @@ -93,21 +74,27 @@ func (s *Signer) Sign(now time.Time) (*file.Zone, error) {

// We are walking the tree in the same direction, so names[] can be used here to indicated the next element.
i := 1
err = z.Walk(func(e *tree.Elem, zrrs map[uint16][]dns.RR) error {
if !apex && e.Name() == s.origin {
nsec := NSEC(e.Name(), names[(ln+i)%ln], s.ttl, append(e.Types(), dns.TypeNS, dns.TypeSOA, dns.TypeNSEC, dns.TypeRRSIG))
err = z.AuthWalk(func(e *tree.Elem, zrrs map[uint16][]dns.RR, auth bool) error {
if !auth {
return nil
}

if e.Name() == s.origin {
nsec := NSEC(e.Name(), names[(ln+i)%ln], mttl, append(e.Types(), dns.TypeNS, dns.TypeSOA, dns.TypeRRSIG, dns.TypeNSEC))
z.Insert(nsec)
} else {
nsec := NSEC(e.Name(), names[(ln+i)%ln], s.ttl, append(e.Types(), dns.TypeNSEC, dns.TypeRRSIG))
nsec := NSEC(e.Name(), names[(ln+i)%ln], mttl, append(e.Types(), dns.TypeRRSIG, dns.TypeNSEC))
z.Insert(nsec)
}

for t, rrs := range zrrs {
if t == dns.TypeRRSIG {
// RRSIGs are not signed and NS records are not signed because we are never authoratiative for them.
// The zone's apex nameservers records are not kept in this tree and are signed separately.
if t == dns.TypeRRSIG || t == dns.TypeNS {
continue
}
for _, pair := range s.keys {
rrsig, err := pair.signRRs(rrs, s.origin, s.ttl, s.inception, s.expiration)
rrsig, err := pair.signRRs(rrs, s.origin, rrs[0].Header().Ttl, inception, expiration)
if err != nil {
return err
}
Expand Down

0 comments on commit 6aed527

Please sign in to comment.