From 51f558581e3baec6a2436639ca8e4b40ff259e63 Mon Sep 17 00:00:00 2001 From: Tim Mwangi Date: Mon, 24 Jul 2023 10:26:56 -0700 Subject: [PATCH 1/2] fix: goagent gosec issues --- examples/gin-server/main.go | 5 ++++- examples/mux-server/main.go | 18 +++++++++++++----- examples/sql-query/main.go | 15 +++++++++++++-- instrumentation/opencensus/init.go | 2 +- .../github.com/jackc/hyperpgx/pgx.go | 2 +- sdk/instrumentation/net/http/attributes.go | 7 ++++++- 6 files changed, 38 insertions(+), 11 deletions(-) diff --git a/examples/gin-server/main.go b/examples/gin-server/main.go index b38ff09b..6d001534 100644 --- a/examples/gin-server/main.go +++ b/examples/gin-server/main.go @@ -1,6 +1,7 @@ package main import ( + "log" "net/http" "github.com/gin-gonic/gin" @@ -36,5 +37,7 @@ func main() { r := setupRouter() // Listen and Server in 0.0.0.0:8080 - r.Run(":8080") + if err := r.Run(":8080"); err != nil { + log.Fatalf("error running server: %v", err) + } } diff --git a/examples/mux-server/main.go b/examples/mux-server/main.go index 9aa29219..107d3dd0 100644 --- a/examples/mux-server/main.go +++ b/examples/mux-server/main.go @@ -8,12 +8,11 @@ import ( "net/http" "time" - "github.com/hypertrace/goagent/config" - sdkhttp "github.com/hypertrace/goagent/sdk/instrumentation/net/http" - "github.com/gorilla/mux" + "github.com/hypertrace/goagent/config" "github.com/hypertrace/goagent/instrumentation/hypertrace" "github.com/hypertrace/goagent/instrumentation/opentelemetry/github.com/gorilla/hypermux" + sdkhttp "github.com/hypertrace/goagent/sdk/instrumentation/net/http" ) func main() { @@ -26,7 +25,14 @@ func main() { r := mux.NewRouter() r.Use(hypermux.NewMiddleware(&sdkhttp.Options{})) // here we use the mux middleware r.HandleFunc("/foo", http.HandlerFunc(fooHandler)) - log.Fatal(http.ListenAndServe(":8081", r)) + srv := http.Server{ + Addr: ":8081", + Handler: r, + ReadTimeout: 60 * time.Second, + WriteTimeout: 60 * time.Second, + ReadHeaderTimeout: 60 * time.Second, + } + log.Fatal(srv.ListenAndServe()) } type person struct { @@ -52,5 +58,7 @@ func fooHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name))) + if _, err := w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name))); err != nil { + log.Printf("error while writing response body: %v", err) + } } diff --git a/examples/sql-query/main.go b/examples/sql-query/main.go index 10a130ec..4f27408b 100644 --- a/examples/sql-query/main.go +++ b/examples/sql-query/main.go @@ -40,7 +40,16 @@ func main() { http.HandlerFunc(fooHandlerFunc), "/foo", )) - log.Fatal(http.ListenAndServe(":8081", r)) + // Using log.Fatal(http.ListenAndServe(":8081", r)) causes a gosec timeout error. + // G114 (CWE-676): Use of net/http serve function that has no support for setting timeouts (Confidence: HIGH, Severity: MEDIUM) + srv := http.Server{ + Addr: ":8081", + Handler: r, + ReadTimeout: 60 * time.Second, + WriteTimeout: 60 * time.Second, + ReadHeaderTimeout: 60 * time.Second, + } + log.Fatal(srv.ListenAndServe()) } type person struct { @@ -68,7 +77,9 @@ func fooHandler(db *sql.DB, w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name))) + if _, err := w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name))); err != nil { + log.Printf("errow while writing response body: %v", err) + } } func dbConn() (db *sql.DB) { diff --git a/instrumentation/opencensus/init.go b/instrumentation/opencensus/init.go index 818306ab..d2e71373 100644 --- a/instrumentation/opencensus/init.go +++ b/instrumentation/opencensus/init.go @@ -22,7 +22,7 @@ func Init(cfg *config.AgentConfig) func() { client := &http.Client{Transport: &http.Transport{ TLSClientConfig: &tls.Config{ MinVersion: tls.VersionTLS12, - InsecureSkipVerify: !cfg.GetReporting().GetSecure().GetValue(), + InsecureSkipVerify: !cfg.GetReporting().GetSecure().GetValue(), // #nosec }, }} diff --git a/instrumentation/opentelemetry/github.com/jackc/hyperpgx/pgx.go b/instrumentation/opentelemetry/github.com/jackc/hyperpgx/pgx.go index 9443d9d2..b67fdd1e 100644 --- a/instrumentation/opentelemetry/github.com/jackc/hyperpgx/pgx.go +++ b/instrumentation/opentelemetry/github.com/jackc/hyperpgx/pgx.go @@ -8,7 +8,7 @@ import ( "github.com/hypertrace/goagent/sdk" "github.com/jackc/pgconn" "github.com/jackc/pgtype/pgxtype" - "github.com/jackc/pgx/v4" + pgx "github.com/jackc/pgx/v4" ) var _ PGXConn = (*pgx.Conn)(nil) diff --git a/sdk/instrumentation/net/http/attributes.go b/sdk/instrumentation/net/http/attributes.go index 4a838ef0..fc44b358 100644 --- a/sdk/instrumentation/net/http/attributes.go +++ b/sdk/instrumentation/net/http/attributes.go @@ -2,6 +2,7 @@ package http // import "github.com/hypertrace/goagent/sdk/instrumentation/net/ht import ( "fmt" + "log" "strings" "github.com/hypertrace/goagent/sdk" @@ -9,7 +10,7 @@ import ( // SetAttributesFromHeaders set attributes into span from a HeaderAccessor func SetAttributesFromHeaders(_type string, headers HeaderAccessor, span sdk.Span) { - headers.ForEachHeader(func(key string, values []string) error { + err := headers.ForEachHeader(func(key string, values []string) error { if len(values) == 1 { span.SetAttribute( fmt.Sprintf("http.%s.header.%s", _type, strings.ToLower(key)), @@ -26,4 +27,8 @@ func SetAttributesFromHeaders(_type string, headers HeaderAccessor, span sdk.Spa } return nil }) + + if err != nil { + log.Printf("error occurred while setting attributes from headers: %v\n", err) + } } From 42efe3d465dbae04892a884bbe10ff397f4c1041 Mon Sep 17 00:00:00 2001 From: Tim Mwangi Date: Mon, 24 Jul 2023 13:02:10 -0700 Subject: [PATCH 2/2] squash pkg golang errors --- examples/postgres-query/main.go | 5 ++++- examples/sql-query/main.go | 4 +++- instrumentation/opencensus/init.go | 6 ++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/examples/postgres-query/main.go b/examples/postgres-query/main.go index 051e6b64..8b2205d7 100644 --- a/examples/postgres-query/main.go +++ b/examples/postgres-query/main.go @@ -7,7 +7,10 @@ import ( "fmt" "os" - "github.com/hypertrace/goagent/instrumentation/hypertrace/github.com/jackc/hyperpgx" + // gosec complains about this pkg not following golang repo standards + // "could not import github.com/hypertrace/goagent/instrumentation/hypertrace/github.com/jackc/hyperpgx (invalid package name: "")" + // It is caused the pkg having its own go.mod + "github.com/hypertrace/goagent/instrumentation/hypertrace/github.com/jackc/hyperpgx" // #nosec ) func main() { diff --git a/examples/sql-query/main.go b/examples/sql-query/main.go index 4f27408b..9b155ef9 100644 --- a/examples/sql-query/main.go +++ b/examples/sql-query/main.go @@ -10,7 +10,9 @@ import ( "net/http" "time" - "github.com/go-sql-driver/mysql" + // gosec complains about github.com/go-sql-driver/mysql not following golang repo standards + // "could not import github.com/go-sql-driver/mysql (invalid package name: "")" + "github.com/go-sql-driver/mysql" // #nosec "github.com/gorilla/mux" "github.com/hypertrace/goagent/config" "github.com/hypertrace/goagent/instrumentation/hypertrace" diff --git a/instrumentation/opencensus/init.go b/instrumentation/opencensus/init.go index d2e71373..96c347bb 100644 --- a/instrumentation/opencensus/init.go +++ b/instrumentation/opencensus/init.go @@ -21,8 +21,10 @@ func Init(cfg *config.AgentConfig) func() { client := &http.Client{Transport: &http.Transport{ TLSClientConfig: &tls.Config{ - MinVersion: tls.VersionTLS12, - InsecureSkipVerify: !cfg.GetReporting().GetSecure().GetValue(), // #nosec + MinVersion: tls.VersionTLS12, + // Ignore gosec: G402 (CWE-295): TLS InsecureSkipVerify may be true. + // #nosec G402 + InsecureSkipVerify: !cfg.GetReporting().GetSecure().GetValue(), }, }}