update x/net to resolve CVE-2024-45338

[TomerJLevy]

GHSA-w32m-9786-jp63

RELEASE NOTES: None

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.13%. Comparing base (063d352) to head (e231b69).
Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7964      +/-   ##
==========================================
- Coverage   82.17%   82.13%   -0.04%     
==========================================
  Files         381      381              
  Lines       38535    38539       +4     
==========================================
- Hits        31666    31655      -11     
- Misses       5564     5574      +10     
- Partials     1305     1310       +5     

see 25 files with indirect coverage changes

[stefanb]

Missed this one? For fixing CVE-2024-45338 it would require 0.33.0 or later, even if it is an indirect dependancy.

[stefanb]

Alternatively, the google.golang.org/grpc can be fixed and released first with this same PR (via main /go.mod) and then bump google.golang.org/grpc in this module to the fixed one...

[TomerJLevy]

I didn’t miss this one. However, since this go.mod uses the latest release and hasn’t replaced it with the current version, it would make more sense to:

1. Upgrade the grpc-go version in this go.mod once we have a release that includes this PR.
2. Replace grpc-go with the current build, so it consumes the updated x/net version.
3. Upgrade the indirect dependency, though I prefer to avoid upgrading indirect dependencies directly.

So, bottom line I prefer number 1, let me know what you think

[stefanb]

Option 1 is the cleanest, but requires update, release, update, release to fix all.
Option 3 is a shortcut, bypassing unit tests and release processes, requiring just one update + release
Option 2 is somewhere in between above options, not sure if shortcut justifies possible complications because of referencing current (unreleased, untagged) revision.

[TomerJLevy]

I chose the second option, even though I assume the use of a stable release was intentional. What do you think?

On second thought, I’m wondering if we should avoid any behavior changes related to this CVE. Perhaps we should stick with option 1, which I assume was the approach chosen by the repository owner.

[arjan-bal]

@TomerJLevy thank you for sending the PR. Looking at the fix and the vulnerability description, the issue was found in the HTML parsing library. gRPC doesn't perform HTML parsing, so I believe we are not be effected.

We bump all our dependencies every 6 weeks as part of our release process, this should get fixed as part of that. The next dependency bump will happen in January.

I'm closing this PR, please feel free to re-open if you think this needs to be fixed urgently.

[TomerJLevy]

Hi @arjan-bal, I understand that this shouldn’t be directly affected by the CVE, and of course, this upgrade can wait until January alongside other upgrades. However, could you respond to the conversation I had with @stefanb regarding the grpc-go version used in cmd/protoc-gen-go-grpc/go.mod? Is there a specific reason why this package is using the latest release instead of the current release?

[arjan-bal]

Hi @arjan-bal, I understand that this shouldn’t be directly affected by the CVE, and of course, this upgrade can wait until January alongside other upgrades. However, could you respond to the conversation I had with @stefanb regarding the grpc-go version used in cmd/protoc-gen-go-grpc/go.mod? Is there a specific reason why this package is using the latest release instead of the current release?

protoc-gen-go-grpc is a separate binary from the gRPC Go library. It is used to generate client and server stubs. Prior to #7438, protoc-gen-go-grpc didn't depend on gRPC at all. This PR added a test dependency on gRPC Go using a replace directive to specify current version of gRPC Go.

After releasing protoc-gen-go-grpc, we discovered that using a replace directive in a binary module is not allowed (#7448). To resolve this, the replace directive was removed in #7451. Since then, we haven't updated the grpc-go version. However, we should transition to using the the latest gRPC Go release instead remaining on v1.65.0.

FYI @dfawley