Skip to content

additional wireguard bounds for autobahn#3609

Merged
pompon0 merged 39 commits into
mainfrom
gprusak-wireguard
Jun 19, 2026
Merged

additional wireguard bounds for autobahn#3609
pompon0 merged 39 commits into
mainfrom
gprusak-wireguard

Conversation

@pompon0

@pompon0 pompon0 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
  • changed the semantics of max_count to limit field arity per message instance, rather than globally
  • changed the generated code to populate a global registry of schemas (eventually I would like to build the schemas lazily at runtime from protoreflect, but for now we stay with generated code because of gogoproto).
  • changed the api to use wireguard.Scan[T]() rather than access the schema directly
  • added additional extensions for wireguard: sized, max_size, max_total_size, so that not only arity but also size of the field can be bounded and to enforce bounded max size of the message.
  • whether the hardcoded bounds in proto are correct should be tested by constructing a message which saturates concrete limit in test and doing marshal -> scan (or unmarshal, since it triggers scan). This is especially important for things like hashes/signatures/keys, which have concrete sizes regardless of what protobuf says (i.e. we need to maintain compatible bounds between proto files and the rest of the code).
  • merged wireguard into protoutils, since wireguard.Scan is now used every time protoutils.Unmarshal is called (it is not optional).
  • extracted protoutils/runtime package which contains symbols which should be accessed only by the generated code.
  • refactored wireguard_plugin tests - plugin error codes are now using proto file fixtures, other tests have been moved to scan_test.go and made to observe the generated code behavior instead of syntax.

In the next PR, I'll implement computation of the max message size, which then can be validated against the buffer.

wen-coding and others added 15 commits June 17, 2026 13:38
…c (CON-298 follow-up)

Annotates all repeated fields in autobahn.proto with (wireguard.max_count),
then generalises the wiring so no per-channel or per-call plumbing is needed:

- Plugin now emits WireguardScan([]byte) error on each schema-bearing type
  instead of init()/registry calls.
- protoutils.Unmarshal[T] asserts wireScanner and scans before proto.Unmarshal
  automatically.
- transport.go asserts wireScanner on the channel MessageType, replacing the
  explicit PreDecode field on ChannelDescriptorT.
- Removes PreDecode from ChannelDescriptorT and all four reactor call-sites
  (blocksync, consensus, evidence, statesync); protection is now derived from
  the proto type, not the channel config.
- Fixes plugin bug: proto3 optional fields were incorrectly treated as oneof
  variants, generating non-existent wrapper type names.
- Wiring tests rewritten to call WireguardScan directly; no-op tests added for
  channels whose message variants don't reach a capped field.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
api.wireguard.go was generated alongside autobahn.wireguard.go but missed
from the initial commit — the giga LaneReq/LaneResp/etc types transitively
reach the annotated Autobahn fields and get WireguardScan automatically.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The (wireguard.max_count) options added to autobahn.proto change the
embedded raw descriptor in autobahn.pb.go. Regenerated via
sei-tendermint/internal/buf.gen.yaml.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sigs, votes, lane_ranges, lane_qcs, and block headers in QC messages
are bounded by the validator set size, which is far below 10000. Use
100 as a tighter, more accurate ceiling. Payload.txs stays at 2000.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add TestPlugin_Proto3OptionalDoesNotEmitWrapperType: a message with a
proto3 optional field (synthetic oneof in the descriptor, no Go wrapper
struct) alongside a capped repeated field. Asserts the plugin does not
emit a nonexistent Foo_Bar wrapper type and correctly caps items.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pompon0 pompon0 requested a review from wen-coding June 18, 2026 13:02
@cursor

cursor Bot commented Jun 18, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches P2P and consensus decode paths and autobahn committee limits; incorrect bounds could reject valid traffic or miss oversized malicious wire data.

Overview
Strengthens protobuf wire validation for autobahn and the rest of Tendermint by moving wireguard to a generated schema registry (protoutils/runtime) and calling protoutils.Scan / ScanAny on unmarshal and P2P receive instead of per-type WireguardScan methods.

Wireguard behavior: max_count now applies per message instance (not a global counter). New proto options (wireguard.sized), max_size, and max_total_size cap field and message sizes. Autobahn messages get concrete limits (e.g. 32-byte hashes, 64-byte sigs, payload tx count/total bytes, 100 sigs/QC entries) aligned with MaxValidators = 100 in committee construction plus tests that marshal at the limit and scan.

Call sites: Consensus block reassembly, P2P transport, and channel wiring tests switch to protoutils.Scan[*T]. Buf codegen uses wireguard_plugin (replacing the old plugin path). github.com/bufbuild/protocompile is added to go.mod for the plugin toolchain.

Reviewed by Cursor Bugbot for commit d211dc6. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 19, 2026, 7:09 PM

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.60432% with 169 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.18%. Comparing base (51ce0c0) to head (d211dc6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...rmint/internal/protoutils/wireguard_plugin/main.go 39.21% 139 Missing and 16 partials ⚠️
...-tendermint/internal/protoutils/runtime/runtime.go 82.85% 8 Missing and 4 partials ⚠️
sei-tendermint/internal/p2p/transport.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3609      +/-   ##
==========================================
- Coverage   59.02%   58.18%   -0.85%     
==========================================
  Files        2215     2153      -62     
  Lines      182521   174413    -8108     
==========================================
- Hits       107731   101477    -6254     
+ Misses      65094    63932    -1162     
+ Partials     9696     9004     -692     
Flag Coverage Δ
sei-chain-pr 76.37% <69.60%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-tendermint/autobahn/types/committee.go 98.57% <100.00%> (+0.04%) ⬆️
...dermint/internal/autobahn/pb/autobahn.wireguard.go 100.00% <100.00%> (ø)
sei-tendermint/internal/consensus/state.go 75.10% <100.00%> (+1.65%) ⬆️
...i-tendermint/internal/p2p/giga/pb/api.wireguard.go 100.00% <100.00%> (ø)
sei-tendermint/internal/protoutils/msg.go 94.44% <100.00%> (+2.77%) ⬆️
...rmint/internal/protoutils/test/a/pb/a.wireguard.go 100.00% <100.00%> (ø)
...mint/proto/tendermint/blocksync/types.wireguard.go 100.00% <100.00%> (ø)
...mint/proto/tendermint/consensus/types.wireguard.go 100.00% <100.00%> (ø)
...ermint/proto/tendermint/consensus/wal.wireguard.go 100.00% <100.00%> (ø)
...ermint/proto/tendermint/privval/types.wireguard.go 100.00% <100.00%> (ø)
... and 6 more

... and 111 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from wen/autobahn_wireguard_caps to main June 18, 2026 14:20
@@ -1274,783 +1274,786 @@ func (this *Pool) Description() (desc *github_com_gogo_protobuf_protoc_gen_gogo_
func StakingDescription() (desc *github_com_gogo_protobuf_protoc_gen_gogo_descriptor.FileDescriptorSet) {
d := &github_com_gogo_protobuf_protoc_gen_gogo_descriptor.FileDescriptorSet{}
var gzipped = []byte{
// 12410 bytes of a gzipped FileDescriptorSet
0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xec, 0xbd, 0x7b, 0x94, 0x1b, 0xe7,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this file change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because gogoproto code embeds transitive proto file descriptors, wireguard.proto in this case.

genesisTimestamp time.Time
}

const MaxValidators = 100

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a common place for all these constraints?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MaxValidators is committee property, payload constraints are with the payload type

// See `google.protobuf.Timestamp` for more detailed specification.
message Timestamp {
option (hashable.hashable) = true;
option (wireguard.sized) = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if someday someone adds a new field and forgot to add this field?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this annotation is enforcing boundedness of the message size. Adding it later is backward compatible, removing it is not.

Comment thread sei-tendermint/internal/protoutils/wireguard/plugin/main.go Outdated
}

message TestonlySizedLeaf {
repeated bytes items = 1 [(wireguard.max_total_size) = 3];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add max_size case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added more cases

extend google.protobuf.MessageOptions {
// sized marks messages that are required to have a bounded maximal wire
// size according to the wireguard plugin's structural checks.
bool sized = 414126221;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel free to grab 414126218 and change max_count

@pompon0 pompon0 Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, do we care? It is not like we have a central registry of extension numbers in our repo, so adding a new number requires inspecting all the proto files in the repo. I would feel more comfortable by assigning a random number.

@pompon0 pompon0 enabled auto-merge June 19, 2026 19:18
@pompon0 pompon0 added this pull request to the merge queue Jun 19, 2026
Merged via the queue into main with commit f83a111 Jun 19, 2026
60 checks passed
@pompon0 pompon0 deleted the gprusak-wireguard branch June 19, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants