protobuf: recompile using gogoproto 1.2 from CRDB master #64
Conversation
|
|
erikgrinaker
force-pushed
the
protobuf-recompile
branch
from
3a5e3cd
to
82f49f8
Compare
Generated Protobufs have been a mix of gogoproto 1.2 and 1.3 types, since different packages have been compiled with different Protobuf compilers. This was in part because `Makefile.update-protos` only covered `errorspb/*.proto`, with other Protobufs compiled ad hoc. This was problematic since CockroachDB currently uses gogoproto 1.2 and thus could not make use of the 1.3-generated types. This patch changes `Makefile.update-protos` to compile all Protobufs in the repo, and recompiles all Protobufs using the current Protobuf compiler used in CockroachDB. Note in particular that this downgrades generated Protobufs for `extgrpc`, `exthttp`, and `grpc` from gogoproto 1.3 to 1.2, which might be considered a breaking change. This also addresses [CVE-2021-3121](https://nvd.nist.gov/vuln/detail/CVE-2021-3121).
erikgrinaker
force-pushed
the
protobuf-recompile
branch
from
82f49f8
to
6d68a00
Compare
It came along with gogoproto 1.3.2, see gogo/protobuf@550e889 |
|
I don't get why this is needed. If the errors library was generating its code using gogo 1.3, it should be able to pick the bug fix for the skippy thing directly in the 1.3 compiler, so why the need to pull in the custom patched 1.2 compiler? |
|
The initial motivation for this was to compile gRPC error Protobufs that would be compatible with CockroachDB, using version 2 structs ( Instructions in https://github.com/cockroachdb/errors/blob/master/Makefile.update-protos#L9-L24 say that we need to generate all Protobufs with CockroachDB's Protobuf compiler, so that's what I did. Most Protobufs seemed to be generated with an old gogo 1.2 compiler, but the gRPC stuff was compiled with 1.3, so I recompiled everything using the 1.2 compiler currently used by CRDB to clean things up. |
|
Hm, tricky situation. I apologize for having not gone the extra mile to keep it v2 in the first place. Seems like the takeaway is to pin the tooling to specific versions, ideally in a way that doesn't require checking out and building CRDB itself. I've now done this successfully for several different projects; will probably get together a PR to show how this could be done. |
No worries, we should've given you a hand with this!
Yeah, that would be great! Note that we use our own patched version of the 1.2 compiler, to backport security fixes and such. We're considering options for the whole Protobuf situation, now that gogo is no longer maintained, but there are a few complications. |
This is a backport of cockroachdb#64 and cockroachdb#60, which recompiles all Protobuf files using the current gogoproto compiler in CRDB's `release-20.1` branch. This is primarily to address the "skippy pb" vulnerability in gogoproto Protobufs.
This is a backport of cockroachdb#64 and cockroachdb#60, which recompiles all Protobuf files using the current gogoproto compiler in CRDB's `release-20.1` branch. This is primarily to address the "skippy pb" vulnerability in gogoproto Protobufs.
Generated Protobufs have been a mix of gogoproto 1.2 and 1.3 types,
since different packages have been compiled with different Protobuf
compilers. This was in part because
Makefile.update-protosonlycovered
errorspb/*.proto, with other Protobufs compiled ad hoc. Thiswas problematic since CockroachDB currently uses gogoproto 1.2 and thus
could not make use of the 1.3-generated types.
This patch changes
Makefile.update-protosto compile all Protobufs inthe repo, and recompiles all Protobufs using the current Protobuf
compiler used in CockroachDB.
Note in particular that this downgrades generated Protobufs for
extgrpc,exthttp, andgrpcfrom gogoproto 1.3 to 1.2, which mightbe considered a breaking change.
This also addresses CVE-2021-3121.
Related to #63 and cockroachdb/cockroach#56208.
This change is