Conversation
moderately tested, needs more testing and code review
There was a problem hiding this comment.
Pull request overview
Adds an ICMP-based redirector/transport and refactors the existing DNS transport to use a shared, transport-agnostic “conversation” protobuf/state machine, while updating transport enums and regenerating affected protobuf/gRPC code.
Changes:
- Introduces
conv.proto/convpband a sharedconversation.Managerused by redirectors for INIT/DATA/FETCH/COMPLETE flow. - Adds a new ICMP redirector in Tavern and an ICMP transport implementation in the implant transport library.
- Updates transport type enums across UI/GraphQL/Ent schema/build tooling and regenerates protobuf/gRPC bindings accordingly.
Reviewed changes
Copilot reviewed 38 out of 42 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tavern/portals/tracepb/trace.pb.go | Regenerated Go protobuf output (tooling/version updates). |
| tavern/portals/portalpb/portal_grpc.pb.go | Regenerated gRPC bindings (new service desc/constants, version assertions). |
| tavern/portals/portalpb/portal.pb.go | Regenerated Go protobuf output (tooling/version updates). |
| tavern/internal/www/src/utils/enums.ts | Adds ICMP to supported transport enum for UI. |
| tavern/internal/www/src/pages/create-quest/hooks/useBeaconFilter.ts | Updates transport priority ordering to include ICMP. |
| tavern/internal/www/schema.graphql | Adds TRANSPORT_ICMP to GraphQL enum (www schema). |
| tavern/internal/redirectors/icmp/icmp.go | New ICMP echo-based redirector that speaks the shared conversation protocol. |
| tavern/internal/redirectors/dns/dns.go | Refactors DNS redirector to use shared conversation manager + convpb. |
| tavern/internal/graphql/schema/ent.graphql | Adds TRANSPORT_ICMP to GraphQL enum (ent schema). |
| tavern/internal/graphql/schema.graphql | Adds TRANSPORT_ICMP to GraphQL enum (server schema). |
| tavern/internal/graphql/generated/root_.generated.go | Regenerated GraphQL output reflecting ICMP transport enum. |
| tavern/internal/ent/migrate/schema.go | Adds TRANSPORT_ICMP to Ent enum migration schema. |
| tavern/internal/ent/beacon/beacon.go | Updates transport enum validation to allow ICMP. |
| tavern/internal/c2/proto/conv.proto | Renames DNS packet proto into transport-agnostic ConvPacket under conv package. |
| tavern/internal/c2/proto/c2.proto | Adds TRANSPORT_ICMP to transport enum. |
| tavern/internal/c2/generate.go | Updates go:generate to generate convpb instead of dnspb. |
| tavern/internal/c2/epb/eldritch.pb.go | Regenerated Go protobuf output (tooling/version updates). |
| tavern/internal/c2/dnspb/dns.pb.go | Removes old DNS-specific generated protobufs. |
| tavern/internal/c2/convpb/conv.pb.go | Adds new generated conversation protobufs. |
| tavern/internal/c2/conversation/manager_test.go | New test suite for shared conversation manager behavior. |
| tavern/internal/c2/conversation/manager.go | New shared conversation state machine used by redirectors. |
| tavern/internal/c2/conversation/conversation.go | Defines shared Conversation state structure. |
| tavern/internal/c2/c2pb/c2_grpc.pb.go | Regenerated gRPC bindings (new service desc/constants, version assertions). |
| tavern/internal/builder/builderpb/builder_grpc.pb.go | Regenerated gRPC bindings (new service desc/constants, version assertions). |
| tavern/internal/builder/builderpb/builder.pb.go | Regenerated Go protobuf output (tooling/version updates). |
| tavern/internal/builder/build_config.go | Maps ICMP transport enum to "icmp" string. |
| tavern/app.go | Registers the new ICMP redirector via blank import. |
| implants/lib/transport/src/lib.rs | Wires new ICMP transport behind feature flag + shared conv helpers. |
| implants/lib/transport/src/icmp.rs | New ICMP transport implementation using ConvPacket over Echo req/rep. |
| implants/lib/transport/src/dns.rs | Switches DNS transport to pb::conv::* and shared conv helpers. |
| implants/lib/transport/src/conv.rs | New shared conversation-protocol helpers for DNS/ICMP transports. |
| implants/lib/transport/Cargo.toml | Adds icmp feature and platform deps (libc, windows-sys). |
| implants/lib/pb/src/lib.rs | Renames protobuf module export from dns to conv. |
| implants/lib/pb/src/generated/conv.rs | Regenerated prost output for ConvPacket (was DNSPacket). |
| implants/lib/pb/src/generated/c2.rs | Regenerated prost output adding TransportIcmp. |
| implants/lib/pb/src/config.rs | Maps icmp:// URIs to TransportIcmp. |
| implants/lib/pb/build.rs | Updates YAML validation + compiles conv.proto instead of dns.proto. |
| implants/imix/Cargo.toml | Enables ICMP transport feature by default and adds feature flag. |
| docs/_docs/user-guide/imix.md | Documents ICMP transport config/constraints and updates DNS wording. |
| docs/_docs/admin-guide/tavern.md | Documents ICMP redirector usage + required host kernel configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Summary
Previous Results
Insights
Test Changes0 test added, 0 removed Slowest Tests
🎉 No failed tests in this run. | 🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
Regenerated c2.pb.go, conv.pb.go from merged proto files to include TRANSPORT_ICMP enum value alongside main's upstream changes.
| Before starting the ICMP redirector, the Linux kernel's automatic ICMP echo reply must be disabled. Without this, the kernel responds to incoming ICMP echo requests by mirroring the payload back to the sender before the user-space redirector can act. Agents receive this kernel reply first and parse their own request payload as a response, breaking the protocol. | ||
|
|
||
| ```bash | ||
| echo 1 | sudo tee /proc/sys/net/ipv4/icmp_echo_ignore_all |
There was a problem hiding this comment.
Please update the terraform to include support for ICMP.
Probably need to copy the DNS pattern but will need a separate VM per redirector otherwise they'll have the same IP.
There was a problem hiding this comment.
I didnt plan on supporting public infra icmp. Ill look into it tho
|
|
||
| **Other requirements:** | ||
|
|
||
| - Must run as root (raw ICMP sockets require `CAP_NET_RAW`) |
There was a problem hiding this comment.
RAW?!?!!
If we do not raw can we support windows and non root beacons?
There was a problem hiding this comment.
This is for the tavern redirector, not the beacons.
|
|
||
| This transport doesn't support eldritch functions that require bi-directional streaming like reverse shell, or SOCKS5 proxying. | ||
|
|
||
| *Note*: The URI must be the IPv4 address of the ICMP redirector, e.g. `icmp://192.168.1.1`. The redirector host must have kernel ICMP echo replies disabled - see the [ICMP Redirector](/admin-guide/tavern#icmp-redirector) section in the Tavern admin guide for setup instructions. |
There was a problem hiding this comment.
Also didnt plan on supporting this either
hulto
left a comment
There was a problem hiding this comment.
LGTM i'll test the TF once you merge.
Just nuke the tcp stuff first.
docs/_docs/user-guide/imix.md
Outdated
| transports: | ||
| - URI: <string> | ||
| type: <grpc|http1|dns> | ||
| type: <grpc|http1|dns|icmp|tcp_bind> |
There was a problem hiding this comment.
Please omit tcp bind specific stuff i can add in a diff pr.
I don't think tcp_bind is the correct string IIRC it's tcp.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Moves the dns protobuf into a shared conversation protobuf. Added ICMP redirector.
Which issue(s) this PR fixes:
Fixes #1016