[WIP] Run gosec against BuildKit

[SaschaSchwarze0]

We are happily using BuildKit and would like to continue doing so. For this, we need it to pass gosec -confidence medium -severity high. Let me know if the BuildKit project is willing to fix those findings and validate against gosec rules going forward. If not, just close the PR.

I am adding gosec validation to the validation action, and start to mitigate the findings.

• In session/auth/auth.go, I switch from math/rand to crypto/rand. Mitigates G404: Insecure random number source (rand)
• In util/stack/stack.go, I switch to ParseInt to be able to provide the bit size. Mitigates G109: Potential Integer overflow made by strconv.Atoi result conversion to int16/32
• In util/testutil/integration/run.go, I am adding a nosec annotation as the insecure random is there imo fine as it is a test utility
• In util/tracing/otlptracegrpc/connection.go, I am adding a nosec annotation as I think that is okay for a random wait time before a retry

With that, gosec is not yet fully happy. What is remaining is:

[/home/sascha/git/moby/buildkit/util/resolver/resolver.go:87] - G402 (CWE-295): TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
    86: 
  > 87:         tc := &tls.Config{}
    88:         if len(c.RootCAs) > 0 {



[/home/sascha/git/moby/buildkit/cmd/buildkitd/main.go:604-606] - G402 (CWE-295): TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
    603:        }
  > 604:        tlsConf := &tls.Config{
  > 605:                Certificates: []tls.Certificate{certificate},
  > 606:        }
    607:        if caFile != "" {



[/home/sascha/git/moby/buildkit/client/client.go:213-216] - G402 (CWE-295): TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
    212: 
  > 213:        cfg := &tls.Config{
  > 214:                ServerName: opts.ServerName,
  > 215:                RootCAs:    certPool,
  > 216:        }
    217: 

I could easily specify a MinVersion there (and would set it to 1.2). But, I do not know if that would break a contract. Looking for some guidance on that aspect.

[tonistiigi]

For the tls.Config I think it is fine to raise the minimum version but I'd like the same minimum to be set in containerd as well first so we know all tools remain consistent. @thaJeztah

[SaschaSchwarze0]

For the tls.Config I think it is fine to raise the minimum version but I'd like the same minimum to be set in containerd as well first so we know all tools remain consistent. @thaJeztah

That's fine, we're not in a hurry.

[jedevc]

@SaschaSchwarze0 I think we'd still be interested in this if you're still working on it?

For the TLS minimum versions, if we're waiting on containerd to make sure we're aligned, could we compromise by ignoring those warnings for now and circling back round later? I think the other warnings still have value and be good to get in :eyes:

[thaJeztah]

@jedevc I think all that's needed is to add gosec to the list here; https://github.com/moby/buildkit/blob/master/.golangci.yml#L21, then golangci-lint should pick it up

[crazy-max]

Solved by https://github.com/moby/buildkit/pull/3224