graham/towerops-agent
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: address bugs and security gaps found in code audit

Browse source 
- HTTP checks: use io.ReadAll with LimitReader instead of single Read()
  to handle bodies spanning multiple TCP segments
- HTTP checks: enforce TLS 1.2 minimum, matching websocket and mikrotik
- Ping/credential jobs: use select/default for channel sends to prevent
  goroutine leaks when result channel is full
- LLDP: check strconv.Atoi errors in IPv6 address parsing instead of
  silently producing incorrect addresses
- SNMP: use comma-ok type assertions in snmpValueToString to prevent
  panics from malformed device responses
- WebSocket: read handshake response line-by-line via bufio.Reader
  instead of single conn.Read that could miss multi-segment responses
This commit is contained in:
Graham McIntire 2026-03-05 12:24:21 -06:00
parent adad473485
commit 7606c0aa6c
No known key found for this signature in database
5 changed files with 89 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
checks.go
View file

@ -4,6 +4,7 @@ import (
"context"
"crypto/tls"
"fmt"
"io"
"net"
"net/http"
"regexp"
@ -73,6 +74,7 @@ func executeHTTPCheck(ctx context.Context, config *pb.HttpCheckConfig, timeoutMs
Transport: &http.Transport{
TLSClientConfig: &tls.Config{
InsecureSkipVerify: !config.VerifySsl,
MinVersion: tls.VersionTLS12,
},
},
CheckRedirect: func(req *http.Request, via []*http.Request) error {
@ -119,10 +121,12 @@ func executeHTTPCheck(ctx context.Context, config *pb.HttpCheckConfig, timeoutMs
// Check content regex if provided
if config.Regex != "" {
body := make([]byte, 1024*1024) // Read up to 1MB
n, _ := resp.Body.Read(body)
body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
if err != nil {
return 2, fmt.Sprintf("Failed to read body: %v", err), responseTime
}
matched, err := regexp.MatchString(config.Regex, string(body[:n]))
matched, err := regexp.MatchString(config.Regex, string(body))
if err != nil {
return 2, fmt.Sprintf("Invalid regex: %v", err), responseTime
}

7
lldp.go
View file

@ -231,8 +231,11 @@ func parseMgmtAddr(oid string) (key string, ip string) {
// Convert 16 octets to IPv6 hex format
var ipv6Parts []string
for i := 0; i < 16; i += 2 {
a, _ := strconv.Atoi(parts[5+i])
b, _ := strconv.Atoi(parts[5+i+1])
a, errA := strconv.Atoi(parts[5+i])
b, errB := strconv.Atoi(parts[5+i+1])
if errA != nil || errB != nil {
return key, ""
}
ipv6Parts = append(ipv6Parts, fmt.Sprintf("%x", a*256+b))
}
ip = strings.Join(ipv6Parts, ":")

58
snmp.go
View file

@ -130,11 +130,15 @@ func executeCredentialTest(ctx context.Context, job *pb.AgentJob, resultCh chan<
timestamp := time.Now().Unix()
if err != nil {
resultCh <- &pb.CredentialTestResult{
select {
case resultCh <- &pb.CredentialTestResult{
TestId: job.JobId,
Success: false,
ErrorMessage: fmt.Sprintf("connection failed: %v", err),
Timestamp: timestamp,
}:
default:
slog.Warn("result channel full", "job_id", job.JobId)
}
return
}
@ -142,11 +146,15 @@ func executeCredentialTest(ctx context.Context, job *pb.AgentJob, resultCh chan<
result, err := conn.Get([]string{"1.3.6.1.2.1.1.1.0"})
if err != nil {
resultCh <- &pb.CredentialTestResult{
select {
case resultCh <- &pb.CredentialTestResult{
TestId: job.JobId,
Success: false,
ErrorMessage: fmt.Sprintf("SNMP test failed: %v", err),
Timestamp: timestamp,
}:
default:
slog.Warn("result channel full", "job_id", job.JobId)
}
return
}
@ -156,11 +164,15 @@ func executeCredentialTest(ctx context.Context, job *pb.AgentJob, resultCh chan<
sysDescr = snmpValueToString(result.Variables[0])
}
resultCh <- &pb.CredentialTestResult{
select {
case resultCh <- &pb.CredentialTestResult{
TestId: job.JobId,
Success: true,
SystemDescription: sysDescr,
Timestamp: timestamp,
}:
default:
slog.Warn("result channel full", "job_id", job.JobId)
}
}
@ -263,7 +275,10 @@ func snmpValueToString(pdu gosnmp.SnmpPDU) string {
case gosnmp.Integer:
return strconv.FormatInt(gosnmp.ToBigInt(pdu.Value).Int64(), 10)
case gosnmp.OctetString:
b := pdu.Value.([]byte)
b, ok := pdu.Value.([]byte)
if !ok {
return fmt.Sprintf("%v", pdu.Value)
}
if !utf8.Valid(b) {
return formatHex(b)
}
@ -274,21 +289,42 @@ func snmpValueToString(pdu gosnmp.SnmpPDU) string {
}
return string(b)
case gosnmp.ObjectIdentifier:
return pdu.Value.(string)
if s, ok := pdu.Value.(string); ok {
return s
}
return fmt.Sprintf("%v", pdu.Value)
case gosnmp.Counter32:
return strconv.FormatUint(uint64(pdu.Value.(uint)), 10)
if v, ok := pdu.Value.(uint); ok {
return strconv.FormatUint(uint64(v), 10)
}
return fmt.Sprintf("%v", pdu.Value)
case gosnmp.Counter64:
return strconv.FormatUint(pdu.Value.(uint64), 10)
if v, ok := pdu.Value.(uint64); ok {
return strconv.FormatUint(v, 10)
}
return fmt.Sprintf("%v", pdu.Value)
case gosnmp.Gauge32:
return strconv.FormatUint(uint64(pdu.Value.(uint)), 10)
if v, ok := pdu.Value.(uint); ok {
return strconv.FormatUint(uint64(v), 10)
}
return fmt.Sprintf("%v", pdu.Value)
case gosnmp.TimeTicks:
return strconv.FormatUint(uint64(pdu.Value.(uint32)), 10)
if v, ok := pdu.Value.(uint32); ok {
return strconv.FormatUint(uint64(v), 10)
}
return fmt.Sprintf("%v", pdu.Value)
case gosnmp.IPAddress:
return pdu.Value.(string)
if s, ok := pdu.Value.(string); ok {
return s
}
return fmt.Sprintf("%v", pdu.Value)
case gosnmp.Null, gosnmp.NoSuchObject, gosnmp.NoSuchInstance, gosnmp.EndOfMibView:
return "null"
case gosnmp.Opaque:
return formatHex(pdu.Value.([]byte))
if b, ok := pdu.Value.([]byte); ok {
return formatHex(b)
}
return fmt.Sprintf("%v", pdu.Value)
default:
return fmt.Sprintf("%v", pdu.Value)
}

12
ssh.go
View file

@ -65,19 +65,27 @@ func executePingJob(ctx context.Context, job *pb.AgentJob, resultCh chan<- *pb.M
if err != nil {
slog.Warn("device down", "device", job.DeviceId, "error", err)
resultCh <- &pb.MonitoringCheck{
select {
case resultCh <- &pb.MonitoringCheck{
DeviceId: job.DeviceId,
Status: "failure",
Timestamp: timestamp,
}:
default:
slog.Warn("result channel full", "job_id", job.JobId)
}
return
}
slog.Debug("device up", "device", job.DeviceId, "response_time_ms", responseTime)
resultCh <- &pb.MonitoringCheck{
select {
case resultCh <- &pb.MonitoringCheck{
DeviceId: job.DeviceId,
Status: "success",
ResponseTimeMs: responseTime,
Timestamp: timestamp,
}:
default:
slog.Warn("result channel full", "job_id", job.JobId)
}
}

30
websocket.go
View file

@ -118,23 +118,33 @@ func wsConnect(u *url.URL, host, network string) (*WSConn, error) {
return nil, fmt.Errorf("write handshake: %w", err)
}
// Read HTTP response (look for 101 Switching Protocols)
buf := make([]byte, 4096)
n, err := conn.Read(buf)
// Read HTTP response headers using a buffered reader.
// A single conn.Read may not capture the full response if it spans
// multiple TCP segments, so we read line by line until the blank line.
br := bufio.NewReaderSize(conn, 4096)
statusLine, err := br.ReadString('\n')
if err != nil {
_ = conn.Close()
return nil, fmt.Errorf("read handshake: %w", err)
}
resp := string(buf[:n])
if !strings.Contains(resp, "101") {
if !strings.Contains(statusLine, "101") {
_ = conn.Close()
return nil, fmt.Errorf("handshake failed: %s", strings.SplitN(resp, "\r\n", 2)[0])
return nil, fmt.Errorf("handshake failed: %s", strings.TrimSpace(statusLine))
}
// Verify Sec-WebSocket-Accept per RFC 6455
// Read remaining headers, verify Sec-WebSocket-Accept per RFC 6455
expectedAccept := computeAcceptKey(key)
acceptFound := false
for _, line := range strings.Split(resp, "\r\n") {
for {
line, err := br.ReadString('\n')
if err != nil {
_ = conn.Close()
return nil, fmt.Errorf("read handshake headers: %w", err)
}
line = strings.TrimSpace(line)
if line == "" {
break // end of headers
}
if strings.HasPrefix(strings.ToLower(line), "sec-websocket-accept: ") {
actual := strings.TrimSpace(line[len("Sec-WebSocket-Accept: "):])
if actual != expectedAccept {
@ -142,7 +152,6 @@ func wsConnect(u *url.URL, host, network string) (*WSConn, error) {
return nil, fmt.Errorf("invalid accept key: got %q, want %q", actual, expectedAccept)
}
acceptFound = true
break
}
}
if !acceptFound {
@ -153,7 +162,8 @@ func wsConnect(u *url.URL, host, network string) (*WSConn, error) {
// Clear handshake deadline — normal operation uses no deadline
_ = conn.SetDeadline(time.Time{})
return &WSConn{conn: conn, reader: bufio.NewReaderSize(conn, 8192)}, nil
// Reuse the buffered reader — it may hold leftover data from the handshake
return &WSConn{conn: conn, reader: br}, nil
}
// ReadMessage reads the next text or binary message, handling control frames internally.