Skip to content

Commit

Permalink
fix: address more gosec warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
bassosimone committed May 14, 2024
1 parent d964ec3 commit b767e6f
Show file tree
Hide file tree
Showing 35 changed files with 88 additions and 60 deletions.
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/config/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const ConfigVersion = 1

// ReadConfig reads the configuration from the path
func ReadConfig(path string) (*Config, error) {
b, err := os.ReadFile(path)
b, err := os.ReadFile(path) // #nosec G304 - this is working as intended
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/utils/homedir/homedir.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func dirUnix() (string, error) {
var stdout bytes.Buffer

// If that fails, try OS specific commands
cmd := execabs.Command("getent", "passwd", strconv.Itoa(os.Getuid()))
cmd := execabs.Command("getent", "passwd", strconv.Itoa(os.Getuid())) // #nosec G204 - this is fine
cmd.Stdout = &stdout
if err := cmd.Run(); err == nil {
if passwd := strings.TrimSpace(stdout.String()); passwd != "" {
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/e2epostprocess/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func main() {
}
var found int
for _, file := range files {
data, err := os.ReadFile(file)
data, err := os.ReadFile(file) // #nosec G304 - this is working as intended
fatalOnError(err)
measurements := bytes.Split(data, []byte("\n"))
for _, measurement := range measurements {
Expand All @@ -72,7 +72,7 @@ func main() {
options = append(options, *entry.Input)
}
log.Printf("run: go %s", strings.Join(options, " "))
cmd := execabs.Command("go", options...)
cmd := execabs.Command("go", options...) // #nosec G204 - this is working as intended
cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr
err = cmd.Run()
fatalOnError(err)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/gardener/internal/testlists/testlists.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func csvReadAndFilter(filepath string, shouldKeep func(URL string) bool) [][]str

// csvWriteBack writes records back to a given file.
func csvWriteBack(filename string, records [][]string) {
filep := runtimex.Try1(os.Create(filename))
filep := runtimex.Try1(os.Create(filename)) // #nosec G304 - this is working as intended
writer := csv.NewWriter(filep)
runtimex.Try0(writer.WriteAll(records))
runtimex.Try0(writer.Error())
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/ghgen/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func mustClose(c io.Closer) {

func generateWorkflowFile(name string, jobs []Job) {
filename := filepath.Join(".github", "workflows", name+".yml")
fp, err := os.Create(filename)
fp, err := os.Create(filename) // #nosec G304 - this is working as intended
runtimex.PanicOnError(err, "os.Create failed")
defer mustClose(fp)
mustFprintf(fp, "# File generated by `go run ./internal/cmd/ghgen`; DO NOT EDIT.\n")
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/miniooni/consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func acquireUserConsent(miniooniDir string, currentOptions *Options) {
// maybeWriteConsentFile writes the consent file iff the yes argument is true
func maybeWriteConsentFile(yes bool, filepath string) (err error) {
if yes {
err = os.WriteFile(filepath, []byte("\n"), 0644)
err = os.WriteFile(filepath, []byte("\n"), 0600)
}
return
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/miniooni/oonirun.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func ooniRunMain(ctx context.Context,
}
}
for _, filename := range currentOptions.InputFilePaths {
data, err := os.ReadFile(filename)
data, err := os.ReadFile(filename) // #nosec G304 - this is working as intended
if err != nil {
logger.Warnf("oonirun: reading OONI Run v2 descriptor failed: %s", err.Error())
continue
Expand Down
14 changes: 11 additions & 3 deletions internal/cmd/oohelperd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,16 @@ func main() {
} else {
w.Header().Set("WWW-Authenticate", "Basic realm=metrics")
w.WriteHeader(401)
w.Write([]byte("401 Unauthorized\n"))
_, _ = w.Write([]byte("401 Unauthorized\n"))
}
})

// create a listening server for serving ooniprobe requests
srv := &http.Server{Addr: *apiEndpoint, Handler: mux}
srv := &http.Server{
Addr: *apiEndpoint,
Handler: mux,
ReadHeaderTimeout: 8 * time.Second,
}
listener, err := net.Listen("tcp", *apiEndpoint)
runtimex.PanicOnError(err, "net.Listen failed")

Expand All @@ -121,7 +125,11 @@ func main() {
pprofMux := http.NewServeMux()
pprofMux.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile))
pprofMux.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace))
pprofSrv := &http.Server{Addr: *pprofEndpoint, Handler: pprofMux}
pprofSrv := &http.Server{
Addr: *pprofEndpoint,
Handler: pprofMux,
ReadHeaderTimeout: 8 * time.Second,
}
go pprofSrv.ListenAndServe()
log.Infof("serving CPU profile at http://%s/debug/pprof/profile", *pprofEndpoint)
log.Infof("serving execution traces at http://%s/debug/pprof/trace", *pprofEndpoint)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/oonireport/oonireport.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func fatalIfFalse(cond bool, msg string) {

func readLines(path string) []string {
// open measurement file
file, err := os.Open(path)
file, err := os.Open(path) // #nosec G304 - this is working as intended
runtimex.PanicOnError(err, "Open file error.")
defer file.Close()

Expand Down
2 changes: 1 addition & 1 deletion internal/database/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (d *Database) GetMeasurementJSON(msmtID int64) (map[string]interface{}, err
return nil, errors.New("cannot access measurement file")
}
measurementFilePath := measurement.DatabaseMeasurement.MeasurementFilePath.String
b, err := os.ReadFile(measurementFilePath)
b, err := os.ReadFile(measurementFilePath) // #nosec G304 - this is working as intended
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/enginenetx/httpsdialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func (hd *httpsDialer) dialTLS(

// create TLS configuration
tlsConfig := &tls.Config{
InsecureSkipVerify: true, // Note: we're going to verify at the end of the func!
InsecureSkipVerify: true, // #nosec G402 - we verify at end of func
NextProtos: []string{"h2", "http/1.1"},
RootCAs: hd.rootCAs,
ServerName: tactic.SNI,
Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/echcheck/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@ func genTLSConfig(sni string) *tls.Config {
RootCAs: certpool,
ServerName: sni,
NextProtos: []string{"h2", "http/1.1"},
InsecureSkipVerify: true,
InsecureSkipVerify: true, // #nosec G402 - it's fine to skip verify in a nettest
}
}
2 changes: 1 addition & 1 deletion internal/experiment/echcheck/utls.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (t *tlsHandshakerWithExtensions) Handshake(
runtimex.Assert(err == nil, "unexpected error when creating UTLSConn")

if t.extensions != nil && len(t.extensions) != 0 {
tlsConn.BuildHandshakeState()
runtimex.Try0(tlsConn.BuildHandshakeState())
tlsConn.Extensions = append(tlsConn.Extensions, t.extensions...)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/ndt7/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*
// See https://github.com/ooni/probe/issues/2413 to understand
// why we're using nil to force netxlite to use the cached
// default Mozilla cert pool.
tlsConfig := &tls.Config{
tlsConfig := &tls.Config{ // #nosec G402 - we need to use a large TLS versions range for measuring
RootCAs: nil,
}
dialer := websocket.Dialer{
Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/quicping/quic.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func buildPacket() ([]byte, connectionID, connectionID) {
// generate random payload
minPayloadSize := 1200 - 14 - (len(destConnID) + len(srcConnID))
randomPayload := make([]byte, minPayloadSize)
rand.Read(randomPayload)
_ = runtimex.Try1(rand.Read(randomPayload))

clientSecret, _ := computeSecrets(destConnID)
encrypted := encryptPayload(randomPayload, destConnID, clientSecret)
Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/tlsmiddlebox/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func genTLSConfig(sni string) *tls.Config {
RootCAs: nil,
ServerName: sni,
NextProtos: []string{"h2", "http/1.1"},
InsecureSkipVerify: true,
InsecureSkipVerify: true, // #nosec G402 - it's fine to skip verify in a nettest
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/fsx/fsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type filesystem struct{}

// Open implements fs.FS.Open.
func (filesystem) Open(pathname string) (fs.File, error) {
return os.Open(pathname)
return os.Open(pathname) // #nosec G304 - this is working as intended
}

// IsRegular returns whether a file is a regular file.
Expand Down
2 changes: 1 addition & 1 deletion internal/legacy/measurex/measurer.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ func (mx *Measurer) httpEndpointGetQUIC(ctx context.Context,
// Using a nil cert pool here forces netxlite to use a cached copy of Mozilla's
// CA bundle. See https://github.com/ooni/probe/issues/2413 for context.
qconn, err := mx.QUICHandshakeWithDB(ctx, db, epnt.Address,
&tls.Config{ // // #nosec G402 - we need to use a large TLS versions range for measuring
&tls.Config{ // #nosec G402 - we need to use a large TLS versions range for measuring
ServerName: epnt.SNI,
NextProtos: epnt.ALPN,
RootCAs: nil,
Expand Down
6 changes: 3 additions & 3 deletions internal/must/must.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ import (
// CreateFile is like [os.Create] but calls
// [runtimex.PanicOnError] on failure.
func CreateFile(name string) *File {
fp, err := os.Create(name)
fp, err := os.Create(name) // #nosec G304 - this is working as intended
runtimex.PanicOnError(err, "os.Create failed")
return &File{fp}
}

// OpenFile is like [os.Open] but calls
// [runtimex.PanicOnError] on failure.
func OpenFile(name string) *File {
fp, err := os.Open(name)
fp, err := os.Open(name) // #nosec G304 - this is working as intended
runtimex.PanicOnError(err, "os.Open failed")
return &File{fp}
}
Expand Down Expand Up @@ -143,7 +143,7 @@ func WriteFile(filename string, content []byte, mode fs.FileMode) {
// ReadFile is like [os.ReadFile] but calls
// [runtimex.PanicOnError] on failure.
func ReadFile(filename string) []byte {
data, err := os.ReadFile(filename)
data, err := os.ReadFile(filename) // #nosec G304 - this is working as intended
runtimex.PanicOnError(err, "os.ReadFile failed")
return data
}
Expand Down
6 changes: 5 additions & 1 deletion internal/netemx/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net"
"net/http"
"sync"
"time"

"github.com/ooni/netem"
"github.com/ooni/probe-cli/v3/internal/runtimex"
Expand Down Expand Up @@ -99,7 +100,10 @@ func (srv *httpCleartextServer) mustListenPortLocked(handler http.Handler, ipAdd
listener := runtimex.Try1(srv.unet.ListenTCP("tcp", addr))

// serve requests in a background goroutine
srvr := &http.Server{Handler: handler}
srvr := &http.Server{
Handler: handler,
ReadHeaderTimeout: 5 * time.Second,
}
go srvr.Serve(listener)

// make sure we track the server (the .Serve method will close the
Expand Down
6 changes: 4 additions & 2 deletions internal/netemx/https.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net"
"net/http"
"sync"
"time"

"github.com/ooni/netem"
"github.com/ooni/probe-cli/v3/internal/runtimex"
Expand Down Expand Up @@ -98,8 +99,9 @@ func (srv *httpSecureServer) mustListenPortLocked(handler http.Handler, ipAddr n

// serve requests in a background goroutine
srvr := &http.Server{
Handler: handler,
TLSConfig: tlsConfig,
Handler: handler,
ReadHeaderTimeout: 5 * time.Second,
TLSConfig: tlsConfig,
}
go srvr.ServeTLS(listener, "", "")

Expand Down
2 changes: 1 addition & 1 deletion internal/netxlite/internal/gencertifi/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func main() {
}
url := os.Args[1]

resp, err := http.Get(url)
resp, err := http.Get(url) // #nosec G107 -- this is working as intended
if err != nil {
log.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/netxlite/internal/generrno/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func mapSystemToLibrary(system string) string {
}

func fileCreate(filename string) *os.File {
filep, err := os.Create(filename)
filep, err := os.Create(filename) // #nosec G304 - this is working as intended
if err != nil {
log.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/shellx/shellx.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func cmd(config *Config, argv *Argv, envp *Envp) *execabs.Cmd {
// hence the choice to keep using x/sys/execabs everywhere.
//
// See <https://tip.golang.org/doc/go1.19> for more information.
cmd := execabs.Command(argv.P, argv.V...)
cmd := execabs.Command(argv.P, argv.V...) // #nosec G204 - this is working as intended
cmd.Env = os.Environ()
for _, entry := range envp.V {
if config.Logger != nil {
Expand Down
10 changes: 8 additions & 2 deletions internal/testingx/httptestx.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ func MustNewHTTPServerEx(addr *net.TCPAddr, httpListener TCPListener, handler ht
Path: "/",
}
srv := &HTTPServer{
Config: &http.Server{Handler: handler},
Config: &http.Server{
Handler: handler,
ReadHeaderTimeout: 5 * time.Second,
},
Listener: listener,
TLS: nil,
URL: baseURL.String(),
Expand Down Expand Up @@ -113,7 +116,10 @@ func MustNewHTTPServerTLSEx(
otherNames = append(otherNames, extraSNIs...)

srv := &HTTPServer{
Config: &http.Server{Handler: handler},
Config: &http.Server{
Handler: handler,
ReadHeaderTimeout: 5 * time.Second,
},
Listener: listener,
TLS: ca.MustNewServerTLSConfig(commonName, otherNames...),
URL: baseURL.String(),
Expand Down
2 changes: 1 addition & 1 deletion internal/testingx/tlsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ type tlsHandlerReset struct{}
// GetCertificate implements TLSHandler.
func (*tlsHandlerReset) GetCertificate(ctx context.Context, tcpConn net.Conn, chi *tls.ClientHelloInfo) (*tls.Certificate, error) {
tcpMaybeResetNetConn(tcpConn)
tcpConn.Close() // just in case to avoid the error returned here to be sent remotely as an alert
_ = tcpConn.Close() // just in case to avoid the error returned here to be sent remotely as an alert
return nil, errors.New("internal error")
}

Expand Down
2 changes: 1 addition & 1 deletion internal/torlogs/torlogs.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func ReadBootstrapLogs(logFilePath string) ([]string, error) {
if logFilePath == "" {
return nil, ErrEmptyLogFilePath
}
data, err := os.ReadFile(logFilePath)
data, err := os.ReadFile(logFilePath) // #nosec G304 - this is working as intended
if err != nil {
return nil, fmt.Errorf("%w: %s", ErrCannotReadLogFile, err.Error())
}
Expand Down
4 changes: 2 additions & 2 deletions internal/tutorial/generator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func writeString(w io.Writer, s string) {

// gen1 generates a single file within a chapter.
func gen1(destfile io.Writer, filepath string) {
srcfile, err := os.Open(filepath)
srcfile, err := os.Open(filepath) // #nosec G304 - this is working as intended
if err != nil {
log.Fatal(err)
}
Expand Down Expand Up @@ -67,7 +67,7 @@ func gen1(destfile io.Writer, filepath string) {
// gen("./experiment/torsf/chapter01", "main.go")
func gen(dirpath string, files ...string) {
readme := path.Join(dirpath, "README.md")
destfile, err := os.Create(path.Join(readme))
destfile, err := os.Create(path.Join(readme)) // #nosec G304 - this is working as intended
if err != nil {
log.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/tutorial/netxlite/chapter08/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func main() {
flag.Parse()
ctx, cancel := context.WithTimeout(context.Background(), *timeout)
defer cancel()
config := &tls.Config{
config := &tls.Config{ // #nosec G402 - we need to use a large TLS versions range for measuring
ServerName: *sni,
NextProtos: []string{"h3"},
RootCAs: nil,
Expand Down
2 changes: 1 addition & 1 deletion internal/tutorial/netxlite/chapter08/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func main() {
flag.Parse()
ctx, cancel := context.WithTimeout(context.Background(), *timeout)
defer cancel()
config := &tls.Config{
config := &tls.Config{ // #nosec G402 - we need to use a large TLS versions range for measuring
ServerName: *sni,
NextProtos: []string{"h3"},
RootCAs: nil,
Expand Down
2 changes: 1 addition & 1 deletion internal/x/dsljavascript/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func LoadExperiment(config *VMConfig, exPath string) (*VM, error) {

func (vm *VM) RunScript(exPath string) error {
// read the file content
content, err := os.ReadFile(exPath)
content, err := os.ReadFile(exPath) // #nosec G304 - this is working as intended
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/x/dslvm/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (sx *HTTPRoundTripStage[T]) roundTrip(ctx context.Context, rtx Runtime, con
}

func (sx *HTTPRoundTripStage[T]) newHTTPRequest(
ctx context.Context, conn HTTPConnection, logger model.Logger) (*http.Request, error) {
ctx context.Context, conn HTTPConnection, _ model.Logger) (*http.Request, error) {
// create the default HTTP request
URL := &url.URL{
Scheme: conn.Scheme(),
Expand Down
2 changes: 1 addition & 1 deletion internal/x/dslvm/quic.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (sx *QUICHandshakeStage) handshake(ctx context.Context, rtx Runtime, endpoi
func (sx *QUICHandshakeStage) newTLSConfig() *tls.Config {
return &tls.Config{ // #nosec G402 - we need to use a large TLS versions range for measuring
NextProtos: sx.NextProtos,
InsecureSkipVerify: sx.InsecureSkipVerify,
InsecureSkipVerify: sx.InsecureSkipVerify, // #nosec G402 - it's fine to possibly skip verify in a nettest
RootCAs: sx.RootCAs,
ServerName: sx.ServerName,
}
Expand Down
Loading

0 comments on commit b767e6f

Please sign in to comment.