[Linter] Flag usage of weak PRNG

[ycombinator]

What does this PR do?

This PR implements a linter check that will raise a flag wherever we use a weak pseudorandom number generator in the code.

Why is it important?

To ensure we don't use a weak PRNG in security contexts, e.g. generating random passwords or tokens.

Note that it's perfectly OK to use a weak PRNG in non-security contexts, e.g. generating a random number to use in tests. In fact, we expect that such uses will be more common than security-related uses. Still, we want to have this linter rule in place to err on the side of caution.

To prevent the linter from flagging such non-security-related uses, add the following annotation above the corresponding line of code:

//nolint:gosec // G404 <reason>

[elasticmachine]

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

[mergify]

This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

[blakerouse]

Nice! Absolutely should have this enabled.

[ycombinator]

I'm not sure this linter check is working as I'd expected. There are a number of occurrences of rand.Int* in the Agent codebase already and I was expecting this PR with the check enabled to to fail on those.

$ grep -inr 'rand.Int' *
internal/pkg/core/backoff/equal_jitter.go:33:		nextRand: time.Duration(rand.Int63n(int64(init))), //nolint:gosec
internal/pkg/core/backoff/equal_jitter.go:54:	b.nextRand = time.Duration(rand.Int63n(int64(b.duration)))
internal/pkg/scheduler/scheduler.go:133:	return time.Duration(rand.Int63n(t))
internal/pkg/agent/cmd/enroll_cmd.go:690:	t := time.NewTimer(time.Duration(rand.Int63n(int64(d))))
internal/pkg/agent/install/user_windows.go:221:		n, err := rand.Int(rand.Reader, maxN)
internal/pkg/agent/application/upgrade/marker_access_test.go:162:		rune := chars[rand.Intn(len(chars))]
internal/pkg/agent/application/upgrade/artifact/download/http/verifier_test.go:112:	first := rand.Intn(len(tt))
internal/pkg/agent/application/upgrade/artifact/download/http/verifier_test.go:113:	second := rand.Intn(len(tt))
internal/pkg/eql/compare.go:12:type operand interface{}
magefile.go:1801:		bkpName := fmt.Sprintf("./env.sh-%d", rand.Int())
Binary file pkg/component/fake/shipper/shipper matches
Binary file pkg/component/fake/component/component matches
testing/integration/install_test.go:260:		runes[i] = letters[rand.Intn(len(letters))]

Maybe the linter only checks files changed as part of the PR? I'm going to try and add a new use of rand.Intn to this PR and see if the linter catches it.

[ycombinator]

Interestingly, if I install and run gosec manually on the Agent codebase, it does flag some uses of rand.Int*:

$ curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.19.0
$ /Users/shaunak/.gvm/pkgsets/go1.21.8/global/bin/gosec -include=G404 ./...
Results:


[/Users/shaunak/development/github/elastic-agent/internal/pkg/scheduler/scheduler.go:133] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    132: 	t := int64(p.variance)
  > 133: 	return time.Duration(rand.Int63n(t))
    134: }



[/Users/shaunak/development/github/elastic-agent/internal/pkg/core/backoff/equal_jitter.go:54] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    53: 	// increase duration for next wait.
  > 54: 	b.nextRand = time.Duration(rand.Int63n(int64(b.duration)))
    55: 	b.duration *= 2



[/Users/shaunak/development/github/elastic-agent/internal/pkg/core/backoff/equal_jitter.go:33] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    32: 		max:      max,
  > 33: 		nextRand: time.Duration(rand.Int63n(int64(init))), //nolint:gosec
    34: 	}



[/Users/shaunak/development/github/elastic-agent/internal/pkg/agent/cmd/enroll_cmd.go:716] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    715: func delay(ctx context.Context, d time.Duration) {
  > 716: 	t := time.NewTimer(time.Duration(rand.Int63n(int64(d))))
    717: 	defer t.Stop()



Summary:
  Gosec  : 2.19.0
  Files  : 462
  Lines  : 85397
  Nosec  : 5
  Issues : 4

So something about how this linter rule is being enabled in golangci-lint seems to be the problem. 🤔

[elastic-sonarqube]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

[elasticmachine]

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

[ycombinator]

Going to close this PR unmerged as I'm not finding time to figure out what's going on (see #4514 (comment)).