-
Notifications
You must be signed in to change notification settings - Fork 107
Create sandbox event endpoint and handlers #784
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
base: main
Are you sure you want to change the base?
Conversation
…er validation and register some event handlers
…nternal-sandbox-event-endpoint-e2b-2486
…nternal-sandbox-event-endpoint-e2b-2486
…nternal-sandbox-event-endpoint-e2b-2486
packages/orchestrator/main.go
Outdated
sbxEventHandlers := event.NewEventHandlers(redisClient) | ||
|
||
sbxEventServer := event.NewEventServer(sbxEventServerPort, sbxEventHandlers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see proper cleanup for these two
sandboxID := r.Header.Get("E2B_SANDBOX_ID") | ||
teamID := r.Header.Get("E2B_TEAM_ID") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be spoofed by the sandbox, we shouldn't trust anything the sandbox sends us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed, but when these are used for ad-hoc events defined by users that should be fine, for our internal defined ones, they would be sent via envd which would have access to those via mmds stuff ive been working on. the same could be said about envd server. I'm thinking of maybe redirecting the requests to an envd forward proxy but that could be rerouted too with some effort i think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the security concern is that the user sandbox would pretend it's another sandbox ID and team ID -> sending the event to completely different target, potentially with malicious desires
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it really depends what the handlers do, and if we're to check if the sandbox is running and belongs to the team id that should narrow it down. the same could be said for the potential for mangling the responses from the server running in envd.
@@ -217,6 +217,12 @@ func (s *Slot) CreateNetwork() error { | |||
return fmt.Errorf("error creating postrouting rule: %w", err) | |||
} | |||
|
|||
// Redirect http://event.e2b.dev traffic destined to event server | |||
err = tables.Append("nat", "PREROUTING", "-i", s.VethName(), "-p", "tcp", "-d", "8.8.8.7", "--dport", "80", "-j", "REDIRECT", "--to-port", "5010") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use different IP than 8.8.8.7, I guess any IP from these should be more appropriate:
var blockedRanges = []string{
"10.0.0.0/8",
"169.254.0.0/16",
"192.168.0.0/16",
"172.16.0.0/12",
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally yes, i first tried private IPs, they don't leave the sandbox through the network bridge with current setup, i'd have to figure out a way to route one of the private ones this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just note, these ranges are blocked by default, you need to enable the target IP address (maybe that might be why you haven't seen them routed)
Also, how will we handle events like sandbox creation, deletion, etc? Basically events emitted from outside of the sandbox |
This is not meant to replace events emitted from outside the sandbox. |
I've meant that we don't have events emitted from the outside, so the question is if it maybe makes sense to have it united |
any event that can be listened to from the outside should continue to be—especially if it's uncertain the VM is running—any event we would have to poll for inside the sandbox ideally shouldn't. |
Description
We currently do not have a way to send events from inside a sandbox and programmatically handle them from the host, this is an attempt to solve that.
Setup networking in sandbox that routes http://events.e2b.dev requests to a server listening in orchestrator with customizable handlers.
/etc/hosts
) in sandboxsandbox_id
andteam_id
Test