Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/vulndb: suggestion regarding GO-2024-2527 #2952

Closed
dims opened this issue Jun 28, 2024 · 15 comments
Closed

x/vulndb: suggestion regarding GO-2024-2527 #2952

dims opened this issue Jun 28, 2024 · 15 comments
Assignees

Comments

@dims
Copy link

dims commented Jun 28, 2024

Report ID

GO-2024-2527

Suggestion/Comment

etcd advisory - GHSA-5x4g-q5rc-36jp

etcd 3.5.x series was never affected by this vulnerability as 3.5.0 was released about a year after the 3.4.x branch was fixed. So the following error message is wrong ( Fixed in: N/A )

3.5.0 release tag info - https://github.com/etcd-io/etcd/tree/v3.5.0
3.4.10 release tag info - https://github.com/etcd-io/etcd/tree/v3.4.10

Vulnerability #1: GO-2024-2527
    Etcd pkg Insecure ciphers are allowed by default in
    go.etcd.io/etcd/client/pkg/v3
  More info: https://pkg.go.dev/vuln/GO-2024-2527
  Module: go.etcd.io/etcd/client/pkg/v3
    Found in: go.etcd.io/etcd/client/pkg/v3@v3.5.13
    Fixed in: N/A
    Example traces found:
Error:       #1: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls fileutil.init
Error:       #2: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls logutil.CreateDefaultZapLogger
Error:       #3: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls logutil.init
Error:       #4: pkg/providers/v1/aws_metrics.go:67:17: providers.registerMetrics calls sync.Once.Do, which eventually calls logutil.init
Error:       #5: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls systemd.init
Error:       #6: pkg/controllers/tagging/tagging_controller.go:191:2: tagging.Controller.Run calls wait.Until, which eventually calls tlsutil.NewCert
Error:       #7: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls tlsutil.init
Error:       #8: pkg/providers/v1/aws.go:1070:38: providers.IsAWSErrorInstanceNotFound calls prometheus.MultiError.Error, which eventually calls transport.baseConfig
Error:       #9: pkg/providers/v1/aws.go:1070:38: providers.IsAWSErrorInstanceNotFound calls prometheus.MultiError.Error, which eventually calls transport.baseConfig
Error:       #10: pkg/controllers/tagging/tagging_controller.go:191:2: tagging.Controller.Run calls wait.Until, which eventually calls transport.baseConfig
Error:       #11: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls transport.init
Error:       #12: cmd/aws-cloud-controller-manager/main.go:38:2: aws.init calls options.init, which eventually calls types.init

Your code is affected by 1 vulnerability from 1 module.
This scan also found 0 vulnerabilities in packages you import and 1
vulnerability in modules you require, but your code doesn't appear to call these
vulnerabilities.
Use '-show verbose' for more details.
Error: Process completed with exit code 3.

@ahrtr
Copy link

ahrtr commented Jun 29, 2024

Etcd pkg Insecure ciphers are allowed by default in go.etcd.io/etcd/client/pkg/v3 is mentioned in https://pkg.go.dev/vuln/GO-2024-2527.

Insecure cipher suite is indeed allowed in etcd client pkg. Pls refer to client/pkg/tlsutil/cipher_suites.go#L30-L34. Users can configure it using the flag --cipher-suites, and a safe default list is used if empty.

So the GO-2024-2527 is still valid, but I think the severity is low, so no any action is required. Also Note that TLS 1.3 ciphersuites are not configurable.

@harshavardhana
Copy link

harshavardhana commented Jun 29, 2024

Incorrect assessment of the published security errata - Indicates that it's already addressed in specific versions (refer screenshot); also, --cipher-suites is an argument that the etcd server has nothing to do with dependent libraries of using etcd clients.

image

Whoever updated the Go vulnerability must revert it. Otherwise, we need to stop using this tool altogether, as it's no longer reliable.

@ahrtr
Copy link

ahrtr commented Jun 29, 2024

We need to update github.com/etcd-io/etcd/security/advisories/GHSA-5x4g-q5rc-36jp.

Not sure whether the github.com/advisories/GHSA-5x4g-q5rc-36jp will be automatically updated after we update the etcd/security/advisories/GHSA-5x4g-q5rc-36jp.

cc @jmhbnz @serathius @spzala @wenjiaswe @dims

@timothy-king
Copy link
Contributor

Thank you for the report. We will look into this.

@NSKryukov
Copy link

NSKryukov commented Jul 1, 2024

So, how to handle this vuln rn?

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/595995 mentions this issue: data/reports: withdraw GO-2024-2527

gopherbot pushed a commit that referenced this issue Jul 1, 2024
This report is a low impact vulnerability that is causing too many
false positives. Withdraw it for now to mitigate impact.

Updates #2527
Updates #2952

Change-Id: Iffad648eecbbda67e49a962592a33df9232f5fbb
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/595995
Auto-Submit: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@tatianab
Copy link
Contributor

tatianab commented Jul 1, 2024

Thank you all for reporting this issue. The report has been withdrawn and should be live in 10-15 minutes.

@ahrtr
Copy link

ahrtr commented Jul 1, 2024

@tatianab
Copy link
Contributor

tatianab commented Jul 1, 2024

To give some insight into why this report was published: we are experimenting with expanding the Go vulnerability database's scope to include vulnerabilities that primarily affect Go binaries as well as libraries. To support this increased scope, we need to automatically generate more reports. In this case, our automation saw two patched versions - v3.4.10 and v3.3.23 - in github.com/advisories/GHSA-5x4g-q5rc-36jp but these patched versions do not exist in go.etcd.io/etcd/client/pkg/v3 according to the Go module proxy (https://pkg.go.dev/go.etcd.io/etcd/client/pkg/v3?tab=versions). We thus conservatively reported all versions as being affected.

@ahrtr if you would like us to un-withdraw the report with updated version info in the future, we are happy to.

@ahrtr
Copy link

ahrtr commented Jul 1, 2024

@ahrtr if you would like us to un-withdraw the report with updated version info in the future, we are happy to.

Ideally we should mark the CVE being resolved in 3.4.22, and 3.5.0 and onwards aren't affected at all.

But it's also OK to withdraw the report. We are good as long as we can remove the security noise (false positives).

@ahrtr
Copy link

ahrtr commented Jul 1, 2024

but these patched versions do not exist in go.etcd.io/etcd/client/pkg/v3 according to the Go module proxy (https://pkg.go.dev/go.etcd.io/etcd/client/pkg/v3?tab=versions)

The reason is etcd release-3.4 and release-3.5/main have big difference regarding code structure. The go.etcd.io/etcd/client/pkg/v3 doesn't exist in release-3.4.

@ahrtr
Copy link

ahrtr commented Jul 2, 2024

FYI. we just updated the security advisory below. Thanks all!

etcd/security/advisories/GHSA-5x4g-q5rc-36jp

@timothy-king
Copy link
Contributor

@ahrtr We (vulndb) need to decide if we are going to keep this withdrawn going forward. It would be helpful to confirm the history of how etcd evolved for us to decide this.

From Go's perspective there are 4 relevant modules:

  • go.etcd.io/etcd
  • go.etcd.io/etcd/v3
  • go.etcd.io/etcd/pkg/v3
  • go.etcd.io/etcd/client/pkg/v3

From what I can tell:

  • The relevant code of the vulnerability is using the cipherSuites global variable defined in pkg/tlsutil/cipher_suites.go .
  • This global appears in git tags up to v3.4.21: https://github.com/etcd-io/etcd/blob/v3.4.21/pkg/tlsutil/cipher_suites.go . Due to limits of the tool, for vulndb to report anything, it is going to mark the entire package for tlsutil. Any transitive importer of the package would be warned.
  • The global was removed in the git tag v3.4.22. (I am being picky here as I need to distinguish git vs. the Go module.)
  • The files for the package but not the global moved to client/pkg.
  • go.etcd.io/etcd/v3 started at v3.5.0 (technically v3.5.0-alpha.0) and has never had this package.
  • go.etcd.io/etcd/pkg/v3 started at v3.5.0 and has never had this package.
  • go.etcd.io/etcd/client/pkg/v3 started at v3.5.0 and the package did move there, but not the global.
  • go.etcd.io/etcd module last tagged version that is directly 'go gettable' was 'v3.3.27+incompatible'. The last compatible semver with Go is v0.5.0.
  • Someone can with some effort download the module go.etcd.io/etcd as pseudo-versions, e.g. v3.4.21 would be v0.5.0-alpha.5.0.20220915004622-85b640cee793.

First, is all of the above history correct?

Assuming it is, here are the three options vulndb is deciding between:

  1. Leave the vulnerability as withdrawn.
  2. Unwithdraw the vulnerability, but warn all transitive users of the package go.etcd.io/etcd/pkg/tlsutil within the module go.etcd.io/etcd (no v3!) on all versions. No other modules will be warned on.
  3. Unwithdraw the vulnerability, but warn all transitive users of the package go.etcd.io/etcd/pkg/tlsutil within the module go.etcd.io/etcd (no v3!), until the pseudo version for v3.4.22.

Do you have any opinions about options 2 and 3, or a strong preference amongst these?
Note that the difference between 2 and 3 is just not warning for those synced past the date 20220915004622 (not the semver or the git tag. the date of the git commit). I expect this to only impact folks that have not upgraded >=3.4.22. I would expect option 2 to warn users of v3.4.23 through v3.4.33. v3.4.33 was published July 13, 2034 (a couple of weeks ago). (So presumably someone is using this?) I manually checked that git tags v3.4.23 through v3.4.33 are past the date 20220915004622. Option 3 has a small [theoretical?] risk of false positives. To receive a false positive, would require a new version [go.etcd.io/etcd](http://go.etcd.io/etcd) to be published in the future (say v3.4.50) that does not contain the vulnerable code with a git commit date before this (several years in the past).

I am currently leaning towards pursuing Option 3 at the moment. The chances of false positives with a date range seem low to me. If this shows itself to be problematic, there is an option of withdrawing again (after some user annoyance that we want to avoid).

I get that this is mostly a “vulndb problem”, but I am hoping to get some feedback from etcd before we un-withdraw the vulnerability for a more accurate report.

@ahrtr
Copy link

ahrtr commented Jul 4, 2024

@timothy-king I appreciate the detailed analysis!

From what I can tell:

  • The relevant code of the vulnerability is using the cipherSuites global variable defined in pkg/tlsutil/cipher_suites.go .
  • This global appears in git tags up to v3.4.21: https://github.com/etcd-io/etcd/blob/v3.4.21/pkg/tlsutil/cipher_suites.go . Due to limits of the tool, for vulndb to report anything, it is going to mark the entire package for tlsutil. Any transitive importer of the package would be warned.
  • The global was removed in the git tag v3.4.22. (I am being picky here as I need to distinguish git vs. the Go module.)
  • The files for the package but not the global moved to client/pkg.
  • go.etcd.io/etcd/v3 started at v3.5.0 (technically v3.5.0-alpha.0) and has never had this package.
  • go.etcd.io/etcd/pkg/v3 started at v3.5.0 and has never had this package.
  • go.etcd.io/etcd/client/pkg/v3 started at v3.5.0 and the package did move there, but not the global.
  • go.etcd.io/etcd module last tagged version that is directly 'go gettable' was 'v3.3.27+incompatible'. The last compatible semver with Go is v0.5.0.
  • Someone can with some effort download the module go.etcd.io/etcd as pseudo-versions, e.g. v3.4.21 would be v0.5.0-alpha.5.0.20220915004622-85b640cee793.

First, is all of the above history correct?

Yes, correct.

Assuming it is, here are the three options vulndb is deciding between:

  1. Leave the vulnerability as withdrawn.
  2. Unwithdraw the vulnerability, but warn all transitive users of the package go.etcd.io/etcd/pkg/tlsutil within the module go.etcd.io/etcd (no v3!) on all versions. No other modules will be warned on.
  3. Unwithdraw the vulnerability, but warn all transitive users of the package go.etcd.io/etcd/pkg/tlsutil within the module go.etcd.io/etcd (no v3!), until the pseudo version for v3.4.22.

Do you have any opinions about options 2 and 3, or a strong preference amongst these?

Either option 1 or 3 works for me. We are good as long as we can clear the false positive.

Honestly, we (etcd) simply inappropriately defined a global variable cipherSuites in etcd v3.4.21 and older versions, but I don't think we (etcd) should raise the security advisory GHSA-5x4g-q5rc-36jp in the first place because the behaviour has never changed from user perspective (refer to doc),

  • Users can configure the desired cipher suites using the “--cipher-suites” flag
  • A list of secure suites is used by default;

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/597355 mentions this issue: data/reports: update GO-2024-2527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants