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

Update go-apiv1 dependencies to address security vulnerabilities (0.3.x) #219

Closed

Conversation

bestbeforetoday
Copy link
Member

Go vulnerabilities:

Additional changes:

  • Update build tools.
  • Add vulnerability scan Makefile target.
  • Add scheduled vulnerability scan for go-apiv1.

Go vulnerabilities:

- CVE-2023-45288

Additional changes:

- Update build tools.
- Add vulnerability scan Makefile target.
- Add scheduled vulnerability scan for go-apiv1.

Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>
@bestbeforetoday bestbeforetoday changed the title Update go-apiv1 dependencies to address security vulnerabilities Update go-apiv1 dependencies to address security vulnerabilities (0.3.x) Apr 11, 2024
@denyeart
Copy link
Contributor

@bestbeforetoday LGTM, any reason to keep it in Draft state?

@bestbeforetoday
Copy link
Member Author

bestbeforetoday commented Apr 11, 2024

@denyeart Updating the build tools for the go-apiv1 protobufs had an unexpected result. The protoc-gen-go plugin has been implemented using the newer Go protobuf APIv2 libraries internally. This actually results in compiled protobuf output that is similar (if not identical) to that produced for go-apiv2.

Note the release notes for the version of the Go protobuf APIv1 compiler plugin from four years ago, which states:

The protoc-gen-go plugin in this module is now a thin wrapper over the protoc-gen-go plugin in the google.golang.org/protobuf module. As a result, there are many changes to the generated code. See that module’s release notes for details. Users should migrate to use the new protoc-gen-go plugin instead of the old one. Code generated by either plugin should be compatible with either module.

Trying a replace directive in Fabric's go.mod to point to this new generated code does indeed seem to give a pretty much compatible result. Other than some minor API changes and places where Fabric -- or at least its tests -- are relying on protobuf implementation details (such as exact JSON serialized message content, which the documentation says should not be relied on), even just switching to the v2 protobuf API while using this fabric-protos implementation seems to mostly work.

I'd appreciate your thoughts on this tooling update. It might actually provide us a smoother path to moving codebases to protobuf APIv2. We could also play safe and hold the build tools back at old enough versions that it generates protobuf bindings that are not implemented upon or usable with protobuf APIv2. What do you think?

@bestbeforetoday bestbeforetoday marked this pull request as ready for review April 11, 2024 18:14
@bestbeforetoday bestbeforetoday requested a review from a team as a code owner April 11, 2024 18:14
@denyeart
Copy link
Contributor

@bestbeforetoday Apologies... today I noticed the apiv1 go dependencies were behind the apiv2 go dependencies so I opened pull request #236. I had forgotten about this pull request until I saw the close notification.

I am still concerned about the apiv2 breaking changes that were discussed at
hyperledger/fabric#3650.

Since the apiv2 announcement at https://go.dev/blog/protobuf-apiv2 mentions that existing consumers can stay on apiv1 indefinitely, I haven't seen much motivation to move up with breaking changes that may impact fabric users.

In your prior note were you suggesting there is a path to update the tooling without hitting the apiv2 breaking changes? If so, I am good with that if you would like to re-open this PR or create a similar one. I'm happy to close #236 in favor of yours.

@bestbeforetoday
Copy link
Member Author

bestbeforetoday commented Jun 11, 2024

@denyeart it might be possible to update the tooling just without certain version ranges that don't generate protobuf bindings based on the newer protobuf implementation. Your PR to update the Go dependencies without changing the tooling seems quite reasonable regardless.

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

Successfully merging this pull request may close these issues.

2 participants