From dc56846101d4209866db2d39bb8fd6833e523d2a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 4 Jun 2015 11:13:49 -0700 Subject: [PATCH] Check that the RRs passed to Sign and Verify form a valid RFC2181 RRSet Add a sanity check used by RRSig's Sign and Verify functions making sure that the records they operate on form a valid RRSet (same name, type, and class). Add a unit test TestInvalidRRSet that calls RRSig's Sign and Verify methods with invalid RRSets, and makes sure the correct error is returned. --- dnssec.go | 56 ++++++++++++++++++++++++++++++++++++--------- dnssec_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 10 deletions(-) diff --git a/dnssec.go b/dnssec.go index 21ef3775..fd236e3a 100644 --- a/dnssec.go +++ b/dnssec.go @@ -205,11 +205,39 @@ func (d *DS) ToCDS() *CDS { return c } +// isValidRRSet checks if a set of RRs is a valid RRset as defined by RFC 2181. +// This means the RRs need to have the same type, name, and class. Returns true +// if the RR set is valid, otherwise false. +func isValidRRSet(rrset []RR) bool { + if len(rrset) == 0 { + return false + } + if len(rrset) == 1 { + return true + } + rrHeader := rrset[0].Header() + rrType := rrHeader.Rrtype + rrClass := rrHeader.Class + rrName := rrHeader.Name + + for _, rr := range rrset[1:] { + curRRHeader := rr.Header() + if curRRHeader.Rrtype != rrType || curRRHeader.Class != rrClass || curRRHeader.Name != rrName { + // Mismatch between the records, so this is not a valid rrset for + //signing/verifying + return false + } + } + + return true +} + // Sign signs an RRSet. The signature needs to be filled in with // the values: Inception, Expiration, KeyTag, SignerName and Algorithm. // The rest is copied from the RRset. Sign returns true when the signing went OK, // otherwise false. -// There is no check if RRSet is a proper (RFC 2181) RRSet. +// This function checks if RRSet is a proper (RFC 2181) RRSet, and returns +// ErrRRSet if it is not. // If OrigTTL is non zero, it is used as-is, otherwise the TTL of the RRset // is used as the OrigTTL. func (rr *RRSIG) Sign(k PrivateKey, rrset []RR) error { @@ -221,6 +249,10 @@ func (rr *RRSIG) Sign(k PrivateKey, rrset []RR) error { return ErrKey } + if !isValidRRSet(rrset) { + return ErrRRset + } + rr.Hdr.Rrtype = TypeRRSIG rr.Hdr.Name = rrset[0].Header().Name rr.Hdr.Class = rrset[0].Header().Class @@ -296,7 +328,7 @@ func (rr *RRSIG) Sign(k PrivateKey, rrset []RR) error { // This function copies the rdata of some RRs (to lowercase domain names) for the validation to work. func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error { // First the easy checks - if len(rrset) == 0 { + if !isValidRRSet(rrset) { return ErrRRset } if rr.KeyTag != k.KeyTag() { @@ -314,14 +346,17 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error { if k.Protocol != 3 { return ErrKey } - for _, r := range rrset { - if r.Header().Class != rr.Hdr.Class { - return ErrRRset - } - if r.Header().Rrtype != rr.TypeCovered { - return ErrRRset - } + + // isValidRRSet checked that we have at least one RR and that the RRs in + // the set have consistent type, class, and name. Also check that type and + // class matches the RRSIG record. + if rrset[0].Header().Class != rr.Hdr.Class { + return ErrRRset } + if rrset[0].Header().Rrtype != rr.TypeCovered { + return ErrRRset + } + // RFC 4035 5.3.2. Reconstructing the Signed Data // Copy the sig, except the rrsig data sigwire := new(rrsigWireFmt) @@ -409,7 +444,8 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error { // ValidityPeriod uses RFC1982 serial arithmetic to calculate // if a signature period is valid. If t is the zero time, the -// current time is taken other t is. +// current time is taken other t is. Returns true if the signature +// is valid at the given time, otherwise returns false. func (rr *RRSIG) ValidityPeriod(t time.Time) bool { var utc int64 if t.IsZero() { diff --git a/dnssec_test.go b/dnssec_test.go index 48c22362..26282432 100644 --- a/dnssec_test.go +++ b/dnssec_test.go @@ -656,3 +656,64 @@ PrivateKey: WURgWHCcYIYUPWgeLmiPY2DJJk02vgrmTfitxgqcL4vwW7BOrbawVmVe0d9V94SR` t.Fatalf("RRSIG record differs:\n%v\n%v", ourRRSIG, rrRRSIG.(*RRSIG)) } } + +func TestInvalidRRSet(t *testing.T) { + goodRecords := make([]RR, 2) + goodRecords[0] = &TXT{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeTXT, Class: ClassINET, Ttl: 0}, Txt: []string{"Hello world"}} + goodRecords[1] = &TXT{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeTXT, Class: ClassINET, Ttl: 0}, Txt: []string{"_o/"}} + + // Generate key + keyname := "cloudflare.com." + key := &DNSKEY{ + Hdr: RR_Header{Name: keyname, Rrtype: TypeDNSKEY, Class: ClassINET, Ttl: 0}, + Algorithm: ECDSAP256SHA256, + Flags: ZONE, + Protocol: 3, + } + privatekey, err := key.Generate(256) + if err != nil { + t.Fatal(err.Error()) + } + + // Need to fill in: Inception, Expiration, KeyTag, SignerName and Algorithm + curTime := time.Now() + signature := &RRSIG{ + Inception: uint32(curTime.Unix()), + Expiration: uint32(curTime.Add(time.Hour).Unix()), + KeyTag: key.KeyTag(), + SignerName: keyname, + Algorithm: ECDSAP256SHA256, + } + + // Inconsistent name between records + badRecords := make([]RR, 2) + badRecords[0] = &TXT{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeTXT, Class: ClassINET, Ttl: 0}, Txt: []string{"Hello world"}} + badRecords[1] = &TXT{Hdr: RR_Header{Name: "nama.cloudflare.com.", Rrtype: TypeTXT, Class: ClassINET, Ttl: 0}, Txt: []string{"_o/"}} + + if err := signature.Sign(privatekey, badRecords); err != ErrRRset { + t.Fatal("Sign returned no error for record set with inconsistent names") + } + + badRecords[0] = &TXT{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeTXT, Class: ClassINET, Ttl: 0}, Txt: []string{"Hello world"}} + badRecords[1] = &A{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeA, Class: ClassINET, Ttl: 0}} + + if err := signature.Sign(privatekey, badRecords); err != ErrRRset { + t.Fatal("Sign returned no error for record set with inconsistent record types") + } + + badRecords[0] = &TXT{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeTXT, Class: ClassINET, Ttl: 0}, Txt: []string{"Hello world"}} + badRecords[1] = &TXT{Hdr: RR_Header{Name: "name.cloudflare.com.", Rrtype: TypeTXT, Class: ClassCHAOS, Ttl: 0}, Txt: []string{"_o/"}} + + if err := signature.Sign(privatekey, badRecords); err != ErrRRset { + t.Fatal("Sign returned no error for record set with inconsistent record class") + } + + // Sign the good record set and then make sure verification fails on the bad record set + if err := signature.Sign(privatekey, goodRecords); err != nil { + t.Fatal("Signing good records failed") + } + + if err := signature.Verify(key, badRecords); err != ErrRRset { + t.Fatal("Verification did not return ErrRRset with inconsistent records") + } +}