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

pkg/rpcserver: pkg/flatrpc: executor: add handshake stage 0 #5808

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

ramosian-glider
Copy link
Member

As we figured out in #5805, syz-manager treats random incoming RPC connections as trusted, and will crash if a non-executor client sends an invalid packet to it.

To address this issue, we introduce another stage of handshake, which includes a cookie exchange:

  • upon connection, the executor sends a ConnectHelloRequest containing its ID assigned to it at startup;
  • the manager responds with a ConnectHelloResponse containing a random 64-bit cookie;
  • the executor calculates a hash of that cookie and includes it into its ConnectRequest together with the other information;
  • before checking the validity of ConnectRequest, the manager ensures client sanity (passed ID didn't change, hashed cookie has the expected value)

We deliberately pick a random cookie instead of a magic number: if the fuzzer somehow learns to send packets to the manager, we don't want it to crash multiple managers on the same machine.


Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md


@ramosian-glider ramosian-glider force-pushed the rpc-handshake branch 3 times, most recently from 32a2b2b to 70e95ce Compare February 20, 2025 14:05
@ramosian-glider
Copy link
Member Author

Please note the commit description change in the latest pushes

@ramosian-glider ramosian-glider force-pushed the rpc-handshake branch 6 times, most recently from b0b0c77 to 688307b Compare February 20, 2025 16:19
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
@ramosian-glider ramosian-glider added this pull request to the merge queue Feb 20, 2025
Merged via the queue into google:master with commit 0808a66 Feb 20, 2025
17 checks passed
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