From 411d2b262b1904ba7d940e408a48dbb507ef2e06 Mon Sep 17 00:00:00 2001 From: Ben Cartwright-Cox Date: Thu, 19 Jan 2023 11:49:05 +0000 Subject: [PATCH 1/6] Remove redundant runtime.GOMAXPROCS() setting The default value of runtime.GOMAXPROCS is the number of CPUs. So there is no need for this line to exist. --- cmd/rtrdump/rtrdump.go | 3 --- cmd/rtrmon/rtrmon.go | 3 --- 2 files changed, 6 deletions(-) diff --git a/cmd/rtrdump/rtrdump.go b/cmd/rtrdump/rtrdump.go index 256be56..24b0857 100644 --- a/cmd/rtrdump/rtrdump.go +++ b/cmd/rtrdump/rtrdump.go @@ -9,7 +9,6 @@ import ( "io" "net" "os" - "runtime" "strings" rtr "github.com/bgp/stayrtr/lib" @@ -125,8 +124,6 @@ func (c *Client) ClientDisconnected(cs *rtr.ClientSession) { } func main() { - runtime.GOMAXPROCS(runtime.NumCPU()) - flag.Parse() if flag.NArg() > 0 { fmt.Printf("%s: illegal positional argument(s) provided (\"%s\") - did you mean to provide a flag?\n", os.Args[0], strings.Join(flag.Args(), " ")) diff --git a/cmd/rtrmon/rtrmon.go b/cmd/rtrmon/rtrmon.go index a4a171d..e977944 100644 --- a/cmd/rtrmon/rtrmon.go +++ b/cmd/rtrmon/rtrmon.go @@ -14,7 +14,6 @@ import ( "net/http" "net/url" "os" - "runtime" "sort" "strconv" "strings" @@ -873,8 +872,6 @@ func (c *Comparator) Start() error { } func main() { - runtime.GOMAXPROCS(runtime.NumCPU()) - flag.Parse() if flag.NArg() > 0 { fmt.Printf("%s: illegal positional argument(s) provided (\"%s\") - did you mean to provide a flag?\n", os.Args[0], strings.Join(flag.Args(), " ")) From 17b7e94876835cb1034910965e454fb9ebaca5df Mon Sep 17 00:00:00 2001 From: Ben Cartwright-Cox Date: Thu, 19 Jan 2023 12:08:29 +0000 Subject: [PATCH 2/6] Fix possible crash when it cannot create output files --- cmd/rtrdump/rtrdump.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/rtrdump/rtrdump.go b/cmd/rtrdump/rtrdump.go index 24b0857..edb7520 100644 --- a/cmd/rtrdump/rtrdump.go +++ b/cmd/rtrdump/rtrdump.go @@ -209,10 +209,10 @@ func main() { var f io.Writer if *OutFile != "" { ff, err := os.Create(*OutFile) - defer ff.Close() if err != nil { log.Fatal(err) } + defer ff.Close() f = ff } else { f = os.Stdout From 029060a6a166bf0d5b37175b551af34da0b0f6dc Mon Sep 17 00:00:00 2001 From: Ben Cartwright-Cox Date: Thu, 19 Jan 2023 12:11:02 +0000 Subject: [PATCH 3/6] Replace redudant errors.new(fmt.sprintf with fmt.errorf( They serve the same function, but it's more understandable what is going on. go-static-check raises this as a warning --- cmd/rtrdump/rtrdump.go | 2 +- lib/client.go | 2 +- prefixfile/prefixfile.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/rtrdump/rtrdump.go b/cmd/rtrdump/rtrdump.go index edb7520..c11ae07 100644 --- a/cmd/rtrdump/rtrdump.go +++ b/cmd/rtrdump/rtrdump.go @@ -164,7 +164,7 @@ func main() { serverKeyHash := ssh.FingerprintSHA256(key) if *ValidateSSH { if serverKeyHash != fmt.Sprintf("SHA256:%v", *SSHServerKey) { - return errors.New(fmt.Sprintf("Server key hash %v is different than expected key hash SHA256:%v", serverKeyHash, *SSHServerKey)) + return fmt.Errorf("server key hash %v is different than expected key hash SHA256:%v", serverKeyHash, *SSHServerKey) } } log.Infof("Connected to server %v via ssh. Fingerprint: %v", remote.String(), serverKeyHash) diff --git a/lib/client.go b/lib/client.go index 4bfa53d..05a5c6e 100644 --- a/lib/client.go +++ b/lib/client.go @@ -220,6 +220,6 @@ func (c *ClientSession) Start(addr string, connType int, configTLS *tls.Config, case TYPE_SSH: return c.StartSSH(addr, configSSH) default: - return errors.New(fmt.Sprintf("Unknown type %v", connType)) + return fmt.Errorf("unknown ClientSession type %v", connType) } } diff --git a/prefixfile/prefixfile.go b/prefixfile/prefixfile.go index a4bc3b1..eb50bb6 100644 --- a/prefixfile/prefixfile.go +++ b/prefixfile/prefixfile.go @@ -32,7 +32,7 @@ func (vrp *VRPJson) GetASN2() (uint32, error) { asnStr := strings.TrimLeft(asnc, "aAsS") asnInt, err := strconv.ParseUint(asnStr, 10, 32) if err != nil { - return 0, errors.New(fmt.Sprintf("Could not decode ASN string: %v", vrp.ASN)) + return 0, fmt.Errorf("could not decode ASN string: %v", vrp.ASN) } asn := uint32(asnInt) return asn, nil @@ -43,7 +43,7 @@ func (vrp *VRPJson) GetASN2() (uint32, error) { case int: return uint32(asnc), nil default: - return 0, errors.New(fmt.Sprintf("Could not decode ASN: %v", vrp.ASN)) + return 0, fmt.Errorf("could not decode ASN: %v", vrp.ASN) } } @@ -55,7 +55,7 @@ func (vrp *VRPJson) GetASN() uint32 { func (vrp *VRPJson) GetPrefix2() (*net.IPNet, error) { _, prefix, err := net.ParseCIDR(vrp.Prefix) if err != nil { - return nil, errors.New(fmt.Sprintf("Could not decode prefix: %v", vrp.Prefix)) + return nil, fmt.Errorf("could not decode prefix: %v", vrp.Prefix) } return prefix, nil } From c90daccd3f8510dd82fd38b01c77fe81e32044a0 Mon Sep 17 00:00:00 2001 From: Ben Cartwright-Cox Date: Thu, 19 Jan 2023 12:12:44 +0000 Subject: [PATCH 4/6] Refactor convoluted ways to recieve a message from a single channel Plus remove a pointless fmt.sprintf() --- cmd/rtrmon/rtrmon.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cmd/rtrmon/rtrmon.go b/cmd/rtrmon/rtrmon.go index e977944..7b312f5 100644 --- a/cmd/rtrmon/rtrmon.go +++ b/cmd/rtrmon/rtrmon.go @@ -262,15 +262,13 @@ func (c *Client) Start(id int, ch chan int) { } connType := pathUrl.Scheme - rtrAddr := fmt.Sprintf("%s", pathUrl.Host) + rtrAddr := pathUrl.Host bypass := true for { if !bypass { - select { - case <-time.After(c.RefreshInterval): - } + <-time.After(c.RefreshInterval) } bypass = false @@ -324,10 +322,8 @@ func (c *Client) Start(id int, ch chan int) { log.Fatal(err) } - select { - case <-c.qrtr: - log.Infof("%d: Quitting RTR session", id) - } + <-c.qrtr + log.Infof("%d: Quitting RTR session", id) } else { log.Infof("%d: Fetching %s", c.id, c.Path) data, statusCode, _, err := c.FetchConfig.FetchFile(c.Path) From 15503e8347bdbe130c0fc8ea59ca0ddf5188ea49 Mon Sep 17 00:00:00 2001 From: Ben Cartwright-Cox Date: Thu, 19 Jan 2023 12:15:33 +0000 Subject: [PATCH 5/6] Use IP.Equal rather than bytes.compare IP.Equal handles some edge cases inside how IP addresses are represented rather than just flat out comparing some byte arrays blindly. --- cmd/stayrtr/stayrtr.go | 2 +- lib/server.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/stayrtr/stayrtr.go b/cmd/stayrtr/stayrtr.go index 3d4c136..a96e745 100644 --- a/cmd/stayrtr/stayrtr.go +++ b/cmd/stayrtr/stayrtr.go @@ -256,7 +256,7 @@ func (s *state) updateFromNewState() error { sessid := s.server.GetSessionId() vrpsjson := s.lastdata.Data - if (vrpsjson == nil) { + if vrpsjson == nil { return nil } diff --git a/lib/server.go b/lib/server.go index 8a925ca..a2573f3 100644 --- a/lib/server.go +++ b/lib/server.go @@ -850,7 +850,7 @@ func (r VRP) String() string { } func (r1 VRP) Equals(r2 VRP) bool { - return r1.MaxLen == r2.MaxLen && r1.ASN == r2.ASN && bytes.Equal(r1.Prefix.IP, r2.Prefix.IP) && bytes.Equal(r1.Prefix.Mask, r2.Prefix.Mask) + return r1.MaxLen == r2.MaxLen && r1.ASN == r2.ASN && r1.Prefix.IP.Equal(r2.Prefix.IP) && bytes.Equal(r1.Prefix.Mask, r2.Prefix.Mask) } func (r1 VRP) Copy() VRP { From 13186622bde041f18f3ca631b39f45f229b559fe Mon Sep 17 00:00:00 2001 From: Ben Cartwright-Cox Date: Thu, 19 Jan 2023 12:17:23 +0000 Subject: [PATCH 6/6] Improve internal error messaging to match standard convention --- cmd/rtrdump/rtrdump.go | 1 - cmd/stayrtr/stayrtr.go | 8 ++++---- lib/client.go | 1 - lib/structs.go | 32 ++++++++++++++++---------------- prefixfile/prefixfile.go | 5 ++--- 5 files changed, 22 insertions(+), 25 deletions(-) diff --git a/cmd/rtrdump/rtrdump.go b/cmd/rtrdump/rtrdump.go index c11ae07..3972df6 100644 --- a/cmd/rtrdump/rtrdump.go +++ b/cmd/rtrdump/rtrdump.go @@ -3,7 +3,6 @@ package main import ( "crypto/tls" "encoding/json" - "errors" "flag" "fmt" "io" diff --git a/cmd/stayrtr/stayrtr.go b/cmd/stayrtr/stayrtr.go index a96e745..816dab8 100644 --- a/cmd/stayrtr/stayrtr.go +++ b/cmd/stayrtr/stayrtr.go @@ -566,7 +566,7 @@ func run() error { } if *Bind == "" && *BindTLS == "" && *BindSSH == "" { - log.Fatalf("Specify at least a bind address") + log.Fatalf("Specify at least a bind address using -bind , -tls.bind , or -ssh.bind") } _, err := s.updateFile(*CacheBin) @@ -639,7 +639,7 @@ func run() error { } private, err := ssh.ParsePrivateKey(sshkey) if err != nil { - log.Fatal("Failed to parse private key: ", err) + log.Fatal("Failed to parse SSH private key: ", err) } sshConfig := ssh.ServerConfig{} @@ -654,7 +654,7 @@ func run() error { log.Infof("Connected (ssh-password): %v/%v", conn.User(), conn.RemoteAddr()) if conn.User() != *SSHAuthUser || !bytes.Equal(suppliedPassword, []byte(password)) { log.Warnf("Wrong user or password for %v/%v. Disconnecting.", conn.User(), conn.RemoteAddr()) - return nil, errors.New("Wrong user or password") + return nil, errors.New("wrong user or password") } return &ssh.Permissions{ @@ -693,7 +693,7 @@ func run() error { } if !noKeys { log.Warnf("No key for %v/%v %v %v. Disconnecting.", conn.User(), conn.RemoteAddr(), key.Type(), keyBase64) - return nil, errors.New("Key not found") + return nil, errors.New("provided ssh key not found") } } else { log.Infof("Connected (ssh-key): %v/%v with key %v %v", conn.User(), conn.RemoteAddr(), key.Type(), keyBase64) diff --git a/lib/client.go b/lib/client.go index 05a5c6e..1d46937 100644 --- a/lib/client.go +++ b/lib/client.go @@ -2,7 +2,6 @@ package rtrlib import ( "crypto/tls" - "errors" "fmt" "io" "net" diff --git a/lib/structs.go b/lib/structs.go index 6811e14..51b9dbe 100644 --- a/lib/structs.go +++ b/lib/structs.go @@ -512,7 +512,7 @@ func DecodeBytes(b []byte) (PDU, error) { func Decode(rdr io.Reader) (PDU, error) { if rdr == nil { - return nil, errors.New("Reader for decoding is nil") + return nil, errors.New("reader for decoding is nil") } var pver uint8 var pduType uint8 @@ -536,10 +536,10 @@ func Decode(rdr io.Reader) (PDU, error) { } if length < 8 { - return nil, fmt.Errorf("Wrong length: %d < 8", length) + return nil, fmt.Errorf("wrong length: %d < 8", length) } if length > messageMaxSize { - return nil, fmt.Errorf("Wrong length: %d > %d", length, messageMaxSize) + return nil, fmt.Errorf("wrong length: %d > %d", length, messageMaxSize) } toread := make([]byte, length-8) err = binary.Read(rdr, binary.BigEndian, toread) @@ -550,7 +550,7 @@ func Decode(rdr io.Reader) (PDU, error) { switch pduType { case PDU_ID_SERIAL_NOTIFY: if len(toread) != 4 { - return nil, fmt.Errorf("Wrong length for Serial Notify PDU: %d != 4", len(toread)) + return nil, fmt.Errorf("wrong length for Serial Notify PDU: %d != 4", len(toread)) } serial := binary.BigEndian.Uint32(toread) return &PDUSerialNotify{ @@ -560,7 +560,7 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_SERIAL_QUERY: if len(toread) != 4 { - return nil, fmt.Errorf("Wrong length for Serial Query PDU: %d != 4", len(toread)) + return nil, fmt.Errorf("wrong length for Serial Query PDU: %d != 4", len(toread)) } serial := binary.BigEndian.Uint32(toread) return &PDUSerialQuery{ @@ -570,14 +570,14 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_RESET_QUERY: if len(toread) != 0 { - return nil, fmt.Errorf("Wrong length for Reset Query PDU: %d != 0", len(toread)) + return nil, fmt.Errorf("wrong length for Reset Query PDU: %d != 0", len(toread)) } return &PDUResetQuery{ Version: pver, }, nil case PDU_ID_CACHE_RESPONSE: if len(toread) != 0 { - return nil, fmt.Errorf("Wrong length for Cache Response PDU: %d != 0", len(toread)) + return nil, fmt.Errorf("wrong length for Cache Response PDU: %d != 0", len(toread)) } return &PDUCacheResponse{ Version: pver, @@ -585,7 +585,7 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_IPV4_PREFIX: if len(toread) != 12 { - return nil, fmt.Errorf("Wrong length for IPv4 Prefix PDU: %d != 12", len(toread)) + return nil, fmt.Errorf("wrong length for IPv4 Prefix PDU: %d != 12", len(toread)) } prefixLen := int(toread[1]) ip := toread[4:8] @@ -603,7 +603,7 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_IPV6_PREFIX: if len(toread) != 24 { - return nil, fmt.Errorf("Wrong length for IPv6 Prefix PDU: %d != 24", len(toread)) + return nil, fmt.Errorf("wrong length for IPv6 Prefix PDU: %d != 24", len(toread)) } prefixLen := int(toread[1]) ip := toread[4:20] @@ -621,7 +621,7 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_END_OF_DATA: if len(toread) != 4 && len(toread) != 16 { - return nil, fmt.Errorf("Wrong length for End of Data PDU: %d != 4 or != 16", len(toread)) + return nil, fmt.Errorf("wrong length for End of Data PDU: %d != 4 or != 16", len(toread)) } var serial uint32 @@ -647,14 +647,14 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_CACHE_RESET: if len(toread) != 0 { - return nil, fmt.Errorf("Wrong length for Cache Reset PDU: %d != 0", len(toread)) + return nil, fmt.Errorf("wrong length for Cache Reset PDU: %d != 0", len(toread)) } return &PDUCacheReset{ Version: pver, }, nil case PDU_ID_ROUTER_KEY: if len(toread) != 28 { - return nil, fmt.Errorf("Wrong length for Router Key PDU: %d < 8", len(toread)) + return nil, fmt.Errorf("wrong length for Router Key PDU: %d < 8", len(toread)) } asn := binary.BigEndian.Uint32(toread[20:24]) spki := binary.BigEndian.Uint32(toread[24:28]) @@ -668,18 +668,18 @@ func Decode(rdr io.Reader) (PDU, error) { }, nil case PDU_ID_ERROR_REPORT: if len(toread) < 8 { - return nil, fmt.Errorf("Wrong length for Error Report PDU: %d < 8", len(toread)) + return nil, fmt.Errorf("wrong length for Error Report PDU: %d < 8", len(toread)) } lenPdu := binary.BigEndian.Uint32(toread[0:4]) if len(toread) < int(lenPdu)+8 { - return nil, fmt.Errorf("Wrong length for Error Report PDU: %d < %d", len(toread), lenPdu+4) + return nil, fmt.Errorf("wrong length for Error Report PDU: %d < %d", len(toread), lenPdu+4) } errPdu := toread[4 : lenPdu+4] lenErrText := binary.BigEndian.Uint32(toread[lenPdu+4 : lenPdu+8]) // int casting for each value is needed here to prevent an uint32 overflow that could result in // upper bound being lower than lower bound causing a crash if len(toread) < int(lenPdu)+8+int(lenErrText) { - return nil, fmt.Errorf("Wrong length for Error Report PDU: %d < %d", len(toread), lenPdu+8+lenErrText) + return nil, fmt.Errorf("wrong length for Error Report PDU: %d < %d", len(toread), lenPdu+8+lenErrText) } errMsg := string(toread[lenPdu+8 : lenPdu+8+lenErrText]) return &PDUErrorReport{ @@ -689,6 +689,6 @@ func Decode(rdr io.Reader) (PDU, error) { ErrorMsg: errMsg, }, nil default: - return nil, errors.New("Could not decode packet") + return nil, errors.New("could not decode packet") } } diff --git a/prefixfile/prefixfile.go b/prefixfile/prefixfile.go index eb50bb6..67e4639 100644 --- a/prefixfile/prefixfile.go +++ b/prefixfile/prefixfile.go @@ -1,7 +1,6 @@ package prefixfile import ( - "errors" "fmt" "net" "strconv" @@ -17,8 +16,8 @@ type VRPJson struct { } type MetaData struct { - Counts int `json:"vrps"` - Buildtime string `json:"buildtime,omitempty"` + Counts int `json:"vrps"` + Buildtime string `json:"buildtime,omitempty"` } type VRPList struct {