Skip to content

Commit

Permalink
Add length checks
Browse files Browse the repository at this point in the history
  • Loading branch information
job committed Jul 28, 2021
1 parent f82e338 commit 2d8fda6
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
17 changes: 16 additions & 1 deletion cmd/stayrtr/stayrtr.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"flag"
"fmt"
"io/ioutil"
"net"
"net/http"
"os"
"os/signal"
Expand Down Expand Up @@ -180,6 +181,16 @@ func decodeJSON(data []byte) (*prefixfile.VRPList, error) {
return &vrplistjson, err
}

func checkPrefixLengths(prefix *net.IPNet, maxLength uint8) (bool) {
plen, max := net.IPMask.Size(prefix.Mask)

if (uint8(plen) > maxLength || maxLength > uint8(max)) {
log.Errorf("%s Maxlength wrong: %d - %d", prefix, plen, maxLength)
return false
}
return true
}

func processData(vrplistjson []prefixfile.VRPJson) ([]rtr.VRP, int, int, int) {
filterDuplicates := make(map[string]bool)

Expand All @@ -200,12 +211,16 @@ func processData(vrplistjson []prefixfile.VRPJson) ([]rtr.VRP, int, int, int) {
continue
}

count++
if !checkPrefixLengths(prefix, v.Length) {

This comment has been minimized.

Copy link
@ties

ties Jul 28, 2021

Collaborator

Quick reply here (busy day/quick reply from mobile, so bound to be incorrect): I think I read there were some issues with routers receiving packers via rtr with a host part that is non-zero.

This pr fixes the 'prefix length invalid, causing a router to have issues' case but it may be good to double check that the parsing does make sure that the host part is zero. I think rejecting the file is likely better then masking the host part to zero.

This comment has been minimized.

Copy link
@job

job Aug 1, 2021

Author Member

agreed! rejecting the file is the safest thing to do

continue
}

if prefix.IP.To4() != nil {
countv4++
} else if prefix.IP.To16() != nil {
countv6++
}
count++

key := fmt.Sprintf("%s,%d,%d", prefix, asn, v.Length)
_, exists := filterDuplicates[key]
Expand Down
8 changes: 7 additions & 1 deletion prefixfile/slurm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ func TestFilterOnVRPs(t *testing.T) {
Prefix: "10.0.0.0/24",
Length: 24,
},
VRPJson{
ASN: uint32(65005),
Prefix: "10.1.0.0/24",
Length: 16, // this VRP is broken, maxlength can't be smaller than plen
},
}

slurm := SlurmValidationOutputFilters{
Expand All @@ -115,8 +120,9 @@ func TestFilterOnVRPs(t *testing.T) {
}
added, removed := slurm.FilterOnVRPs(vrps)
assert.Len(t, added, 1)
assert.Len(t, removed, 3)
assert.Len(t, removed, 4)
assert.Equal(t, uint32(65001), removed[0].GetASN())
assert.Equal(t, uint32(65005), removed[3].GetASN())
}

func TestAssertVRPs(t *testing.T) {
Expand Down

0 comments on commit 2d8fda6

Please sign in to comment.