Better IP bind/listen flow

[aviramha]

while working on #1121 I found out that on my setup the listen fails because it binds each IP separately, leading to ListenAlreadyExists error and killing the layer.
I am sending a PR that ignores this error, but we need in bind call to check if address is not already bound to preserve normal behavior (return error in that case instead of ignoring it)

Edit:
We should hook all the functions that help resolve the local stack (ip, subnet, ports) so app want to listen on "localhost" it will then resolve to the real remote ip and will be able to bind on that ip, so getsockname would return also the remote ip even before accepting any connection.

[gautamprikshit1]

Can you provide a little more insight on this one. Thanks

[aviramha]

Can you provide a little more insight on this one. Thanks

Sure. It's a bit complex issue so feel free to ask more questions.

mirrord hooks calls to bind/listen, so instead of actually binding/listening the given ports we randomize a port then pass the data to the agent so it can know what connections to steal/mirror for us.
Due to the IP being different between local/remote, we currently just ignore cases where the same port is bound twice. We ignore it because to preserve "proper" behavior we need to check if IP:PORT combination is bound, and then error in that case instead of returning success. Right now we don't preserve the bounded IP, only port so in order to support apps that bound each IP.

After writing this, I think maybe we should consider the IP specified in bind to be passed to the remote agent, but for it to work we need to also make sure that the local applications is able to obtain the remote's IPs to bind those.

[meowjesty]

While investigating #1054 we've found out that bind has to be more "correct" than what we have right now.

When running the flask test, the server address stays as some 127.0.0.1 (initial address), even though internally the requests are being handled in 1.1.1.1.

• Some logs to give a bit more information:

# first call to `getsockname`
stderr 2023-03-02T20:04:08.775908614Z 62732: 2023-03-02T20:04:08.775887Z DEBUG ThreadId(01) getsockname: mirrord_layer::socket::ops: getsockname -> local_address 127.0.0.1:33839 sockfd=10

(...)

# correct address
stderr 2023-03-02T20:04:08.787741559Z 62732: 2023-03-02T20:04:08.787718Z DEBUG ThreadId(01) getsockname: mirrord_layer::socket::ops: getsockname -> local_address 1.1.1.1:80 sockfd=11

(...)

# value from `request.server` should be `1.1.1.1`
stdout 2023-03-02T20:04:08.788315678Z 62732: ('127.0.0.1', 33839): Server value
stdout 2023-03-02T20:04:08.788342282Z 62732: HttpMethod.GET: Request completed

[aviramha]

Questions asked by @meowjesty on Discord:
Which address should we bind (and where)?

1. We should bind the remote address (the pod address)?
2. A random address in the agent?
3. A random address in the remote's network namespace?
4. User specified address in the agent?
5. User specified address in the remote's network namespace?
   Think we have to keep the bound socket alive (the one we use to detect valid bind in the agent), otherwise we could have a race condition of a layer thinking that the address is safe to bind, but something else binds + starts listenning on it before.

[aviramha]

The bind / listen restrictions should happen on layer only. There is no meaning to double binding a mirrored port for example. The objective is to not change/modify local app behavior and hide bugs (double bind for example)
This means that we should return error on bind itself if it happens twice on same (address, port)

[meowjesty]

so getsockname would return also the remote ip even before accepting any connection.

This means that on bind for localhost:XX (and equivalents), we retrieve the actual remote address of the service?

• An example:

1. User impersonates pod "Bear" which has a service:

$ kubectl get service
NAME                   TYPE            CLUSTER-IP      EXTERNAL-IP   PORT(S)            AGE
bear-service       NodePort    1.2.3.4               <none>             80:1234/TCP   1d

2. User calls bind("localhost:80");
3. mirrord does our normal bind magic, but also retrieves the address 1.2.3.4:80 as if this was the user's requested address;
4. getsockname(user_socket) returns the 1.2.3.4:80 address (same for other similar functions);
5. Proceed as normal;

[aviramha]

Yes. Exactly.
There might be an issue if you bind localhost:3770 when 3770 is an ignored port and it will fail on bypass since 1.2.3.4 isn't really available on the system. Maybe we should have a struct like this:

struct Host {
 remote_ip: SocketAddr,
 local_ip: SocketAddr
}

Host{remote_ip: "1.2.3.4", local_ip: "1.1.1.1"}

then when we do the actual bind we check if we want remote or local and use the appropriate?
We can probably first finish the more naive implementation, then add this logic (since we already resolve the remote by default)

[meowjesty]

PR #1371 addresses only the double bind issue, but we still have an IP mismatch when the user binds localhost:80, and when they start listenning.

I think the following steps would help to solve this:

1. Retrieve the impersonated pod IP;

• the easiest point to retrieve this is in from_pod, and we could "bubble up" it in the RuntimeData struct;
• we want the pod IP, not the service IP, kubectl get pod -o wide shows the address you want;

2. Pass this IP to the layer at start up, or have a message that allows the layer to retrieve it from our wrap_raw_connection;
3. Check if the user is requesting a localhost address (127.0.0.1, 0.0.0.0) for bind, and if so, use the impersonated pod's address as the requested_address instead (of localhost);

• this would solve getsockname initially giving out a localhost address, and later on changing to the pod address;

I did something like this, but ended up with a different port when a message came through the TCP sniffer (don't know why yet).

[aviramha]

PR #1371 addresses only the double bind issue, but we still have an IP mismatch when the user binds localhost:80, and when they start listenning.

I think the following steps would help to solve this:

1. Retrieve the impersonated pod IP;

• the easiest point to retrieve this is in from_pod, and we could "bubble up" it in the RuntimeData struct;

• we want the pod IP, not the service IP, kubectl get pod -o wide shows the address you want;

2. Pass this IP to the layer at start up, or have a message that allows the layer to retrieve it from our wrap_raw_connection;

3. Check if the user is requesting a localhost address (127.0.0.1, 0.0.0.0) for bind, and if so, use the impersonated pod's address as the requested_address instead (of localhost);

• this would solve getsockname initially giving out a localhost address, and later on changing to the pod address;

I did something like this, but ended up with a different port when a message came through the TCP sniffer (don't know why yet).

Not sure why we need that.
User can't bind localhost as it's a DNS and needs to use IP.
To solve the mentioned issue we just need to store the bound IP and return it for an unconnected socket on getsockname

[meowjesty]

Not sure why we need that.

You mean why we need this part?

Check if the user is requesting a localhost address (127.0.0.1, 0.0.0.0) for bind (...)

We could save it as Bound::pod_address and return it from getsockname when the socket state is Bound, thus solving #1054 right?

[aviramha]

We're already returning the requested address:

                SocketState::Bound(bound) => Detour::Success(bound.requested_address.into()),

if we want to get in the way somewhere, we need to change the way it decides what IPs are available for it.
AFAIK it users dns resolving for that (gethostname then resolve it) so we should be okay on that too.
There might be use of other libc functions but usually they'd get you the "internal" (NAT, firewall, etc) which isn't so useful

[aviramha]

I think we achieved what we want for now :)