open-cluster-management-io / registration

hub / spoke registration controllers
Apache License 2.0
42 stars 58 forks source link

Fix code quaility issues report by gosec. #313

Closed xuezhaojun closed 1 year ago

xuezhaojun commented 1 year ago

Fixes issues report by gosec and also add it in CI process.

The gosec report:

Results:

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/pkg/helpers/testing/testinghelpers.go:336] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    335: func NewV1beta1CSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest {
  > 336:    insecureRand := rand.New(rand.NewSource(0))
    337:    pk, err := ecdsa.GenerateKey(elliptic.P256(), insecureRand)

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/pkg/helpers/testing/testinghelpers.go:303] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    302: func NewCSR(holder CSRHolder) *certv1.CertificateSigningRequest {
  > 303:    insecureRand := rand.New(rand.NewSource(0))
    304:    pk, err := ecdsa.GenerateKey(elliptic.P256(), insecureRand)

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/pkg/clientcert/cert_controller.go:402] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    401:    }
  > 402:    newPercentage := percentage + percentage*rand.Float64()*maxFactor
    403:    return newPercentage

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:60] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    59: func createKubeConfig(context string, securePort string, serverCertFile, certFile, keyFile string) (*clientcmdapi.Config, error) {
  > 60:     caData, err := ioutil.ReadFile(serverCertFile)
    61:     if err != nil {

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/pkg/spoke/managedcluster/secret_controller.go:96] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    95:         filename := path.Clean(path.Join(outputDir, key))
  > 96:         lastData, err := ioutil.ReadFile(filename)
    97:         switch {

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/pkg/helpers/testing/assertion.go:244] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    243: func AssertFileContent(t *testing.T, filePath string, expectedContent []byte) {
  > 244:    content, _ := ioutil.ReadFile(filePath)
    245:    if !bytes.Equal(content, expectedContent) {

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:351] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    350:    for _, csr := range csrList.Items {
  > 351:        csrs = append(csrs, &csr)
    352:    }

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:191] - G301 (CWE-276): Expect directory permissions to be 0750 or less (Confidence: HIGH, Severity: MEDIUM)
    190:    if _, err := os.Stat(configDir); os.IsNotExist(err) {
  > 191:        if err = os.MkdirAll(configDir, 0755); err != nil {
    192:            return err

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:105] - G301 (CWE-276): Expect directory permissions to be 0750 or less (Confidence: HIGH, Severity: MEDIUM)
    104:    if _, err := os.Stat(CertDir); os.IsNotExist(err) {
  > 105:        if err = os.MkdirAll(CertDir, 0755); err != nil {
    106:            return err

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:199] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    198:    }
  > 199:    if err := ioutil.WriteFile(path.Join(configDir, "bootstrap.key"), keyData, 0644); err != nil {
    200:        return err

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:196] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    195: 
  > 196:    if err := ioutil.WriteFile(path.Join(configDir, "bootstrap.crt"), certData, 0644); err != nil {
    197:        return err

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:146] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    145:    }
  > 146:    if err := ioutil.WriteFile(t.caKeyFile, caKeyBuffer.Bytes(), 0644); err != nil {
    147:        return err

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/test/integration/util/util.go:137] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    136:    }
  > 137:    if err := ioutil.WriteFile(t.caFile, caCertBuffer.Bytes(), 0644); err != nil {
    138:        return err

[/Users/xuezhao/go/src/github.com/xuezhaojun/registration/pkg/helpers/testing/testinghelpers.go:525] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    524: func WriteFile(filename string, data []byte) {
  > 525:    if err := ioutil.WriteFile(filename, data, 0644); err != nil {
    526:        panic(err)

Summary:
  Gosec  : 2.15.0
  Files  : 65
  Lines  : 9425
  Nosec  : 0
  Issues : 14
xuezhaojun commented 1 year ago

should we add this in to make verify, it will be easier for developer to debug this locally

Agree, moved into verify.

xuezhaojun commented 1 year ago

/assign @qiujian16

Hi, the gosec is moved to verify, please take another look, thanks!

qiujian16 commented 1 year ago

/approve /lgtm

openshift-ci[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, xuezhaojun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/open-cluster-management-io/registration/blob/main/OWNERS)~~ [qiujian16] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment