fix(dnssec): return nil sigs on sign error (#7999)

This commit is contained in:
Ville Vesilehto
2026-04-04 20:40:29 +03:00
committed by GitHub
parent ce9da6fa41
commit cb40d84c85
2 changed files with 54 additions and 2 deletions

View File

@@ -139,7 +139,7 @@ func (d Dnssec) sign(rrs []dns.RR, signerName string, ttl, incep, expir uint32,
} }
sig := k.newRRSIG(signerName, ttl, incep, expir) sig := k.newRRSIG(signerName, ttl, incep, expir)
if e := sig.Sign(k.s, rrs); e != nil { if e := sig.Sign(k.s, rrs); e != nil {
return sigs, e return nil, e
} }
sigs = append(sigs, sig) 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, 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) } func (d Dnssec) set(key uint64, sigs []dns.RR) { d.cache.Add(key, sigs) }

View File

@@ -1,6 +1,9 @@
package dnssec package dnssec
import ( import (
"crypto"
"errors"
"io"
"testing" "testing"
"time" "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()) { func newDnssec(t *testing.T, zones []string) (Dnssec, func(), func()) {
t.Helper() t.Helper()
k, rm1, rm2 := newKey(t) k, rm1, rm2 := newKey(t)