diff --git a/plugin/dnssec/dnssec.go b/plugin/dnssec/dnssec.go index 2d9003f93..adcf1d391 100644 --- a/plugin/dnssec/dnssec.go +++ b/plugin/dnssec/dnssec.go @@ -139,7 +139,7 @@ func (d Dnssec) sign(rrs []dns.RR, signerName string, ttl, incep, expir uint32, } sig := k.newRRSIG(signerName, ttl, incep, expir) if e := sig.Sign(k.s, rrs); e != nil { - return sigs, e + return nil, e } sigs = append(sigs, sig) } @@ -148,7 +148,10 @@ func (d Dnssec) sign(rrs []dns.RR, signerName string, ttl, incep, expir uint32, } return sigs, nil }) - return sigs.([]dns.RR), err + if err != nil { + return nil, err + } + return sigs.([]dns.RR), nil } func (d Dnssec) set(key uint64, sigs []dns.RR) { d.cache.Add(key, sigs) } diff --git a/plugin/dnssec/dnssec_test.go b/plugin/dnssec/dnssec_test.go index ec79ce5b1..2764398de 100644 --- a/plugin/dnssec/dnssec_test.go +++ b/plugin/dnssec/dnssec_test.go @@ -1,6 +1,9 @@ package dnssec import ( + "crypto" + "errors" + "io" "testing" "time" @@ -247,6 +250,52 @@ func testEmptyMsg() *dns.Msg { } } +// errSigner is a crypto.Signer that always returns an error. Used to simulate +// signing failures in tests without panicking on nil keys. +type errSigner struct { + pub crypto.PublicKey +} + +func (e *errSigner) Public() crypto.PublicKey { return e.pub } +func (e *errSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, error) { + return nil, errors.New("simulated signing failure") +} + +// TestSignReturnsNilOnError verifies that when a signing operation fails mid-way +// through multiple keys, sign() returns (nil, error) rather than (partial_sigs, error). +// Before the fix, the inflight function returned the partially-accumulated sigs slice +// alongside the error. While callers checked err before using the sigs, returning +// partial results from an error path is incorrect and could cause a nil-assertion +// panic if the error guard were ever removed. +func TestSignReturnsNilOnError(t *testing.T) { + // Get a valid key that will sign successfully. + k1, rm1, rm2 := newKey(t) + defer rm1() + defer rm2() + + // Create a second key that will fail during signing. + brokenKey := &DNSKEY{ + K: dns.Copy(k1.K).(*dns.DNSKEY), + s: &errSigner{pub: k1.s.Public()}, + tag: k1.tag + 1, + } + + c := cache.New[[]dns.RR](defaultCap) + // k1 succeeds, brokenKey fails — sign() should return nil, not k1's partial sig. + d := New([]string{"miek.nl."}, []*DNSKEY{k1, brokenKey}, false, nil, c) + + m := testMsg() + incep, expir := incepExpir(time.Now().UTC()) + sigs, err := d.sign(m.Answer, "miek.nl.", 1703, incep, expir, server) + + if err == nil { + t.Fatal("Expected error from broken key, got nil") + } + if sigs != nil { + t.Errorf("Expected nil sigs on signing error, got %d sig(s)", len(sigs)) + } +} + func newDnssec(t *testing.T, zones []string) (Dnssec, func(), func()) { t.Helper() k, rm1, rm2 := newKey(t)