plugin/dnssec: Add support for KSK/ZSK split key setups (#2196)

* plugin/dnssec: Add support for KSK/ZSK split key setups

* plugin/dnssec: Update README to document split ZSK/KSK operation

plugin/dnssec/README.md

@@ -21,8 +21,13 @@ dnssec [ZONES... ] {
 }
 ~~~
 
-The specified key is used for all signing operations. The DNSSEC signing will treat this key as a
+The signing behavior depends on the keys specified. If multiple keys are specified of which there is
-CSK (common signing key), forgoing the ZSK/KSK split. All signing operations are done online.
+at least one key with the SEP bit set and at least one key with the SEP bit unset, signing will happen
+in split ZSK/KSK mode. DNSKEY records will be signed with all keys that have the SEP bit set. All other
+records will be signed with all keys that do not have the SEP bit set.
+
+In any other case, each specified key will be treated as a CSK (common signing key), forgoing the
+ZSK/KSK split. All signing operations are done online.
 Authenticated denial of existence is implemented with NSEC black lies. Using ECDSA as an algorithm
 is preferred as this leads to smaller signatures (compared to RSA). NSEC3 is *not* supported.
 

plugin/dnssec/cache_test.go

@@ -24,7 +24,7 @@ func TestCacheSet(t *testing.T) {
 	m := testMsg()
 	state := request.Request{Req: m, Zone: "miek.nl."}
 	k := hash(m.Answer) // calculate *before* we add the sig
-	d := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, nil, c)
+	d := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, false, nil, c)
 	d.Sign(state, time.Now().UTC(), server)
 
 	_, ok := d.get(k, server)
@@ -48,7 +48,7 @@ func TestCacheNotValidExpired(t *testing.T) {
 	m := testMsg()
 	state := request.Request{Req: m, Zone: "miek.nl."}
 	k := hash(m.Answer) // calculate *before* we add the sig
-	d := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, nil, c)
+	d := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, false, nil, c)
 	d.Sign(state, time.Now().UTC().AddDate(0, 0, -9), server)
 
 	_, ok := d.get(k, server)
@@ -72,7 +72,7 @@ func TestCacheNotValidYet(t *testing.T) {
 	m := testMsg()
 	state := request.Request{Req: m, Zone: "miek.nl."}
 	k := hash(m.Answer) // calculate *before* we add the sig
-	d := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, nil, c)
+	d := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, false, nil, c)
 	d.Sign(state, time.Now().UTC().AddDate(0, 0, +9), server)
 
 	_, ok := d.get(k, server)

plugin/dnssec/dnskey.go

@@ -28,6 +28,7 @@ func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) {
 	if e != nil {
 		return nil, e
 	}
+	defer f.Close()
 	k, e := dns.ReadRR(f, pubFile)
 	if e != nil {
 		return nil, e
@@ -37,6 +38,7 @@ func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) {
 	if e != nil {
 		return nil, e
 	}
+	defer f.Close()
 
 	dk, ok := k.(*dns.DNSKEY)
 	if !ok {
@@ -76,3 +78,13 @@ func (d Dnssec) getDNSKEY(state request.Request, zone string, do bool, server st
 	}
 	return m
 }
+
+// Return true iff this is a zone key with the SEP bit unset. This implies a ZSK (rfc4034 2.1.1).
+func (k DNSKEY) isZSK() bool {
+	return k.K.Flags & (1<<8) == (1<<8) && k.K.Flags & 1 == 0
+}
+
+// Return true iff this is a zone key with the SEP bit set. This implies a KSK (rfc4034 2.1.1).
+func (k DNSKEY) isKSK() bool {
+	return k.K.Flags & (1<<8) == (1<<8) && k.K.Flags & 1 == 1
+}

plugin/dnssec/dnssec.go

@@ -20,15 +20,17 @@ type Dnssec struct {
 
 	zones     []string
 	keys      []*DNSKEY
+	splitkeys bool
 	inflight  *singleflight.Group
 	cache     *cache.Cache
 }
 
 // New returns a new Dnssec.
-func New(zones []string, keys []*DNSKEY, next plugin.Handler, c *cache.Cache) Dnssec {
+func New(zones []string, keys []*DNSKEY, splitkeys bool, next plugin.Handler, c *cache.Cache) Dnssec {
 	return Dnssec{Next: next,
 		zones:     zones,
 		keys:      keys,
+		splitkeys: splitkeys,
 		cache:     c,
 		inflight:  new(singleflight.Group),
 	}
@@ -97,15 +99,29 @@ func (d Dnssec) sign(rrs []dns.RR, signerName string, ttl, incep, expir uint32,
 	}
 
 	sigs, err := d.inflight.Do(k, func() (interface{}, error) {
-		sigs := make([]dns.RR, len(d.keys))
+		var sigs []dns.RR
-		var e error
+		for _, k := range d.keys {
-		for i, k := range d.keys {
+			if d.splitkeys {
+				if len(rrs) > 0 && rrs[0].Header().Rrtype == dns.TypeDNSKEY {
+					// We are signing a DNSKEY RRSet. With split keys, we need to use a KSK here.
+					if !k.isKSK() {
+						continue
+					}
+				} else {
+					// For non-DNSKEY RRSets, we want to use a ZSK.
+					if !k.isZSK() {
+						continue
+					}
+				}
+			}
 			sig := k.newRRSIG(signerName, ttl, incep, expir)
-			e = sig.Sign(k.s, rrs)
+			if e := sig.Sign(k.s, rrs); e != nil {
-			sigs[i] = sig
+				return sigs, e
+			}
+			sigs = append(sigs, sig)
 		}
 		d.set(k, sigs)
-		return sigs, e
+		return sigs, nil
 	})
 	return sigs.([]dns.RR), err
 }

plugin/dnssec/dnssec_test.go

@@ -70,7 +70,7 @@ func TestSigningDifferentZone(t *testing.T) {
 	m := testMsgEx()
 	state := request.Request{Req: m, Zone: "example.org."}
 	c := cache.New(defaultCap)
-	d := New([]string{"example.org."}, []*DNSKEY{key}, nil, c)
+	d := New([]string{"example.org."}, []*DNSKEY{key}, false, nil, c)
 	m = d.Sign(state, time.Now().UTC(), server)
 	if !section(m.Answer, 1) {
 		t.Errorf("Answer section should have 1 RRSIG")
@@ -218,7 +218,7 @@ func testEmptyMsg() *dns.Msg {
 func newDnssec(t *testing.T, zones []string) (Dnssec, func(), func()) {
 	k, rm1, rm2 := newKey(t)
 	c := cache.New(defaultCap)
-	d := New(zones, []*DNSKEY{k}, nil, c)
+	d := New(zones, []*DNSKEY{k}, false, nil, c)
 	return d, rm1, rm2
 }
 

plugin/dnssec/handler_test.go

@@ -104,7 +104,7 @@ func TestLookupZone(t *testing.T) {
 	defer rm1()
 	defer rm2()
 	c := cache.New(defaultCap)
-	dh := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, fm, c)
+	dh := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, false, fm, c)
 
 	for _, tc := range dnsTestCases {
 		m := tc.Msg()
@@ -125,7 +125,7 @@ func TestLookupDNSKEY(t *testing.T) {
 	defer rm1()
 	defer rm2()
 	c := cache.New(defaultCap)
-	dh := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, test.ErrorHandler(), c)
+	dh := New([]string{"miek.nl."}, []*DNSKEY{dnskey}, false, test.ErrorHandler(), c)
 
 	for _, tc := range dnssecTestCases {
 		m := tc.Msg()

plugin/dnssec/setup.go

@@ -25,14 +25,14 @@ func init() {
 }
 
 func setup(c *caddy.Controller) error {
-	zones, keys, capacity, err := dnssecParse(c)
+	zones, keys, capacity, splitkeys, err := dnssecParse(c)
 	if err != nil {
 		return plugin.Error("dnssec", err)
 	}
 
 	ca := cache.New(capacity)
 	dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
-		return New(zones, keys, next, ca)
+		return New(zones, keys, splitkeys, next, ca)
 	})
 
 	c.OnStartup(func() error {
@@ -43,7 +43,7 @@ func setup(c *caddy.Controller) error {
 	return nil
 }
 
-func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) {
+func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, bool, error) {
 	zones := []string{}
 
 	keys := []*DNSKEY{}
@@ -53,7 +53,7 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) {
 	i := 0
 	for c.Next() {
 		if i > 0 {
-			return nil, nil, 0, plugin.ErrOnce
+			return nil, nil, 0, false, plugin.ErrOnce
 		}
 		i++
 
@@ -71,21 +71,21 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) {
 			case "key":
 				k, e := keyParse(c)
 				if e != nil {
-					return nil, nil, 0, e
+					return nil, nil, 0, false, e
 				}
 				keys = append(keys, k...)
 			case "cache_capacity":
 				if !c.NextArg() {
-					return nil, nil, 0, c.ArgErr()
+					return nil, nil, 0, false, c.ArgErr()
 				}
 				value := c.Val()
 				cacheCap, err := strconv.Atoi(value)
 				if err != nil {
-					return nil, nil, 0, err
+					return nil, nil, 0, false, err
 				}
 				capacity = cacheCap
 			default:
-				return nil, nil, 0, c.Errf("unknown property '%s'", x)
+				return nil, nil, 0, false, c.Errf("unknown property '%s'", x)
 			}
 
 		}
@@ -94,6 +94,17 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) {
 		zones[i] = plugin.Host(zones[i]).Normalize()
 	}
 
+	// Check if we have both KSKs and ZSKs.
+	zsk, ksk := 0, 0
+	for _, k := range keys {
+		if k.isKSK() {
+			ksk++
+		} else if k.isZSK() {
+			zsk++
+		}
+	}
+	splitkeys := zsk > 0 && ksk > 0
+
 	// Check if each keys owner name can actually sign the zones we want them to sign.
 	for _, k := range keys {
 		kname := plugin.Name(k.K.Header().Name)
@@ -105,11 +116,11 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) {
 			}
 		}
 		if !ok {
-			return zones, keys, capacity, fmt.Errorf("key %s (keyid: %d) can not sign any of the zones", string(kname), k.tag)
+			return zones, keys, capacity, splitkeys, fmt.Errorf("key %s (keyid: %d) can not sign any of the zones", string(kname), k.tag)
 		}
 	}
 
-	return zones, keys, capacity, nil
+	return zones, keys, capacity, splitkeys, nil
 }
 
 func keyParse(c *caddy.Controller) ([]*DNSKEY, error) {

plugin/dnssec/setup_test.go

@@ -18,56 +18,71 @@ func TestSetupDnssec(t *testing.T) {
 		t.Fatalf("Failed to write private key file: %s", err)
 	}
 	defer func() { os.Remove("Kcluster.local.private") }()
+	if err := ioutil.WriteFile("ksk_Kcluster.local.key", []byte(kskpub), 0644); err != nil {
+		t.Fatalf("Failed to write pub key file: %s", err)
+	}
+	defer func() { os.Remove("ksk_Kcluster.local.key") }()
+	if err := ioutil.WriteFile("ksk_Kcluster.local.private", []byte(kskpriv), 0644); err != nil {
+		t.Fatalf("Failed to write private key file: %s", err)
+	}
+	defer func() { os.Remove("ksk_Kcluster.local.private") }()
 
 	tests := []struct {
 		input              string
 		shouldErr          bool
 		expectedZones      []string
 		expectedKeys       []string
+		expectedSplitkeys  bool
 		expectedCapacity   int
 		expectedErrContent string
 	}{
-		{`dnssec`, false, nil, nil, defaultCap, ""},
+		{`dnssec`, false, nil, nil, false, defaultCap, ""},
-		{`dnssec example.org`, false, []string{"example.org."}, nil, defaultCap, ""},
+		{`dnssec example.org`, false, []string{"example.org."}, nil, false, defaultCap, ""},
-		{`dnssec 10.0.0.0/8`, false, []string{"10.in-addr.arpa."}, nil, defaultCap, ""},
+		{`dnssec 10.0.0.0/8`, false, []string{"10.in-addr.arpa."}, nil, false, defaultCap, ""},
 		{
 			`dnssec example.org {
 				cache_capacity 100
-			}`, false, []string{"example.org."}, nil, 100, "",
+			}`, false, []string{"example.org."}, nil, false, 100, "",
 		},
 		{
 			`dnssec cluster.local {
 				key file Kcluster.local
-			}`, false, []string{"cluster.local."}, nil, defaultCap, "",
+			}`, false, []string{"cluster.local."}, nil, false, defaultCap, "",
 		},
 		{
 			`dnssec example.org cluster.local {
 				key file Kcluster.local
-			}`, false, []string{"example.org.", "cluster.local."}, nil, defaultCap, "",
+			}`, false, []string{"example.org.", "cluster.local."}, nil, false, defaultCap, "",
 		},
 		// fails
 		{
 			`dnssec example.org {
 				key file Kcluster.local
-			}`, true, []string{"example.org."}, nil, defaultCap, "can not sign any",
+			}`, true, []string{"example.org."}, nil, false, defaultCap, "can not sign any",
 		},
 		{
 			`dnssec example.org {
 				key
-			}`, true, []string{"example.org."}, nil, defaultCap, "argument count",
+			}`, true, []string{"example.org."}, nil, false, defaultCap, "argument count",
 		},
 		{
 			`dnssec example.org {
 				key file
-			}`, true, []string{"example.org."}, nil, defaultCap, "argument count",
+			}`, true, []string{"example.org."}, nil, false, defaultCap, "argument count",
 		},
 		{`dnssec
-		  dnssec`, true, nil, nil, defaultCap, ""},
+		  dnssec`, true, nil, nil, false, defaultCap, ""},
+		{
+			`dnssec cluster.local {
+				key file Kcluster.local
+				key file ksk_Kcluster.local
+			}`, false, []string{"cluster.local."}, nil, true, defaultCap, "",
+		},
 	}
 
 	for i, test := range tests {
 		c := caddy.NewTestController("dns", test.input)
-		zones, keys, capacity, err := dnssecParse(c)
+		zones, keys, capacity, splitkeys, err := dnssecParse(c)
 
 		if test.shouldErr && err == nil {
 			t.Errorf("Test %d: Expected error but found %s for input %s", i, err, test.input)
@@ -93,6 +108,9 @@ func TestSetupDnssec(t *testing.T) {
 					t.Errorf("Dnssec not correctly set for input %s. Expected: '%s', actual: '%s'", test.input, k, keys[i].K.Header().Name)
 				}
 			}
+			if splitkeys != test.expectedSplitkeys {
+				t.Errorf("Detected split keys does not match. Expected: %t, actual %t", test.expectedSplitkeys, splitkeys)
+			}
 			if capacity != test.expectedCapacity {
 				t.Errorf("Dnssec not correctly set capacity for input '%s' Expected: '%d', actual: '%d'", test.input, capacity, test.expectedCapacity)
 			}
@@ -120,3 +138,24 @@ Created: 20170901060531
 Publish: 20170901060531
 Activate: 20170901060531
 `
+
+const kskpub = `; This is a zone-signing key, keyid 45330, for cluster.local.
+; Created: 20170901060531 (Fri Sep  1 08:05:31 2017)
+; Publish: 20170901060531 (Fri Sep  1 08:05:31 2017)
+; Activate: 20170901060531 (Fri Sep  1 08:05:31 2017)
+cluster.local. IN DNSKEY 257 3 5 AwEAAcFpDv+Cb23kFJowu+VU++b2N1uEHi6Ll9H0BzLasFOdJjEEclCO q/KlD4682vOMXxJNN8ZwOyiCa7Y0TEYqSwWvhHyn3bHCwuy4I6fss4Wd 7Y9dU+6QTgJ8LimGG40Iizjc9zqoU8Q+q81vIukpYWOHioHoY7hsWBvS RSlzDJk3`
+
+const kskpriv = `Private-key-format: v1.3
+Algorithm: 5 (RSASHA1)
+Modulus: wWkO/4JvbeQUmjC75VT75vY3W4QeLouX0fQHMtqwU50mMQRyUI6r8qUPjrza84xfEk03xnA7KIJrtjRMRipLBa+EfKfdscLC7Lgjp+yzhZ3tj11T7pBOAnwuKYYbjQiLONz3OqhTxD6rzW8i6SlhY4eKgehjuGxYG9JFKXMMmTc=
+PublicExponent: AQAB
+PrivateExponent: K5XyZFBPrjMVFX5gCZlyPyVDamNGrfSVXSIiMSqpS96BSdCXtmHAjCj4bZFPwkzi6+vs4tJN8p4ZifEVM0a6qwPZyENBrc2qbsweOXE6l8BaPVWFX30xvVRzGXuNtXxlBXE17zoHty5r5mRyRou1bc2HUS5otdkEjE30RiocQVk=
+Prime1: 7RRFUxaZkVNVH1DaT/SV5Sb8kABB389qLwU++argeDCVf+Wm9BBlTrsz2U6bKlfpaUmYZKtCCd+CVxqzMyuu0w==
+Prime2: 0NiY3d7Fa08IGY9L4TaFc02A721YcDNBBf95BP31qGvwnYsLFM/1xZwaEsIjohg8g+m/GpyIlvNMbK6pywIVjQ==
+Exponent1: XjXO8pype9mMmvwrNNix9DTQ6nxfsQugW30PMHGZ78kGr6NX++bEC0xS50jYWjRDGcbYGzD+9iNujSScD3qNZw==
+Exponent2: wkoOhLIfhUIj7etikyUup2Ld5WAbW15DSrotstg0NrgcQ+Q7reP96BXeJ79WeREFE09cyvv/EjdLzPv81/CbbQ==
+Coefficient: ah4LL0KLTO8kSKHK+X9Ud8grYi94QSNdbX11ge/eFcS/41QhDuZRTAFv4y0+IG+VWd+XzojLsQs+jzLe5GzINg==
+Created: 20170901060531
+Publish: 20170901060531
+Activate: 20170901060531
+`