Panic with invalid SLURM file
I did some testing and found that goRTR either panics or has undesirable behavior if invalid data is provided. Maybe do some sanity checking before accepting data?
When an invalid prefix (v4 or v6, either the address portion or mask portion) is provided in the prefix assertion section, goRTR panics with:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x6af31e]
goroutine 1 [running]:
github.com/cloudflare/gortr/prefixfile.(*SlurmLocallyAddedAssertions).AssertROAs(0xc00014a460, 0xc009b72000, 0x24de6, 0x32a16)
/home/agallo/go/src/github.com/cloudflare/gortr/prefixfile/slurm.go:138 +0x12e
github.com/cloudflare/gortr/prefixfile.(*SlurmConfig).AssertROAs(...)
/home/agallo/go/src/github.com/cloudflare/gortr/prefixfile/slurm.go:154
main.(*state).updateFile(0xc0000c0000, 0x7ffc1078324d, 0x21, 0x0, 0x0)
/home/agallo/go/src/github.com/cloudflare/gortr/cmd/gortr/gortr.go:403 +0xdba
main.(*state).routineUpdate(0xc0000c0000, 0x7ffc1078324d, 0x21, 0x258, 0x7ffc10783296, 0x24)
/home/agallo/go/src/github.com/cloudflare/gortr/cmd/gortr/gortr.go:507 +0x2c0
main.main()
/home/agallo/go/src/github.com/cloudflare/gortr/cmd/gortr/gortr.go:861 +0x666
example entry used to cause this:
{
"asn": 65039,
"prefix": "198.51.100.0/355",
"comment": "I trust the TESTNET-2"
},
If an invalid ASN is provided, there is no panic, this message is logged:
ERRO[0002] Slurm: json: cannot unmarshal number 4294967296 into Go struct field SlurmPrefixAssertion.LocallyAddedAssertions.PrefixAssertions.ASN of type uint32
And goRTR ignores all entries in the Prefix Assertion section
code used to generate this:
{
"asn": 4294967296,
"prefix": "198.51.100.0/24",
"comment": "I trust the TESTNET-2"
},
For the local assertions, things got a bit worse- An invalid address specification, such as
{
"prefix": "198.51.100.0/322",
"comment": "local something"
},
results in goRTR filtering everything:
INFO[1204] Slurm filtering: 0 kept, 151101 removed, 2 asserted
INFO[1204] New update (2 uniques, 2 total prefixes). 0 bytes. Updating sha256 hash 06580ab7a3b1872323ef1f513acdd402e901de75f6c13304101bbb37b9c66408 -> b56168b0fac51993de909cfaf6c1c294a76513bd64649fd45e6a9b4ee2624f00
INFO[1206] Updated added, new serial 1
All entries from the upstream validator (I'm using octoRPKI in this test) are filtered. I confirmed that only 2 prefixes made it to the router I was testing with.
Maybe provide some sanity checking for the input via SLURM?
Thank you.
Will fix. Thank you for reporting
I prepared a PR, let me know if you are able to test it. It should silently discard invalid prefixes. Adding logging would be doable in the future. I prefer this instead of dropping the whole SLURM (I may be wrong). The first error is due to JSON decoding so it just does not accept the file.
I'd be happy to test. Just let me know
I merged the change on master: could you git pull and go build inside cmd/gortr?
I don't see any more panics. Thanks for the quick fix!