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

Linux: Manage routes in a routing table dedicated to ZET. #892

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

tomc797
Copy link
Contributor

@tomc797 tomc797 commented Jul 9, 2024

Linux has the capability to support multiple routing tables. So let's manage ZET routes in a dedicated routing table, distinct from main and default. The routing policy database is configured to prefer the routes managed in the ZET table over those stored in the main and default routing tables. This approach provides isolation, and thus additional security for ZET routes.

This serves two purposes:

@tomc797 tomc797 requested a review from a team as a code owner July 9, 2024 06:49
@tomc797 tomc797 force-pushed the feature/isolate_routes branch 4 times, most recently from d1bee89 to c645395 Compare July 11, 2024 03:10
@tomc797 tomc797 force-pushed the feature/isolate_routes branch 3 times, most recently from 9b7f884 to 78fa829 Compare September 29, 2024 23:40
@tomc797 tomc797 force-pushed the feature/isolate_routes branch 9 times, most recently from 0b14ebf to 0db4fe8 Compare October 19, 2024 17:44
Resolve interface differences between `iproute2` and `BusyBox` related to ip-rule behavior by implementing a minimal rtnetlink library. This enables direct management of rule insertion into the Routing Policy Database (RPDB).

Signed-off-by: Tom Carroll <[email protected]>
ZET routes are managed in a dedicated routing table, distinct from `main` and `default`. The routing policy database is configured to prefer the routes managed in the ZET table over those stored in the main and default routing tables. This approach provides isolation, and thus additional security for ZET routes.

Signed-off-by: Tom Carroll <[email protected]>
@tomc797 tomc797 force-pushed the feature/isolate_routes branch 2 times, most recently from 3d2db66 to c110f8d Compare October 24, 2024 04:30
@tomc797
Copy link
Contributor Author

tomc797 commented Oct 24, 2024

Manage ziti-edge-tunnel routes in route table 90 (the decimal code for 'Z'). Rules are configured within the route policy database to utilize the route table 90. The rule has preference 23141 (The decimal code for 'Ze') and configured to select on traffic without the bypass fwmark. If sockets are configured with bypass mark, such as the sockets used to communicate with the controller, edge routers, and hosted, they bypass this lookup. In these cases, the traffic originating from these sockets will utilize the main and default routing tables. The added benefit is that if you jump networks (jump from WiFi to cell, for instance), there isn't a route that may interfere with reestablishing the connections.

@tomc797
Copy link
Contributor Author

tomc797 commented Oct 24, 2024

Would it be possible to get a review? I would appreciate feedback.

Add support to raise and restore thread capabilities. This is necessary as the `SO_MARK` socket option requires the `CAP_NET_ADMIN` capability for correct operation; otherwise, permission is denied.

Signed-off-by: Tom Carroll <[email protected]>
Implemented a socket factory that configures the socket option `SO_MARK` to bypass the ZITI routing table, ensuring proper packet marking for custom routing.

Signed-off-by: Tom Carroll <[email protected]>
Override `tls_uv`'s `tlsuv_socket` function to set the `SO_MARK` option on the created socket.

Signed-off-by: Tom Carroll <[email protected]>
@tomc797 tomc797 force-pushed the feature/isolate_routes branch from 3146172 to be26d1d Compare October 24, 2024 04:46
Introduces a customizable hook to allow overriding the default socket creation process for `hosted` sockets.

Signed-off-by: Tom Carroll <[email protected]>
Implements setting the SO_MARK option for sockets designated for the `hosted` services.

Signed-off-by: Tom Carroll <[email protected]>
@tomc797 tomc797 force-pushed the feature/isolate_routes branch from 85efa68 to e92f6ae Compare October 24, 2024 04:53
Copy link
Member

@scareything scareything left a comment

Choose a reason for hiding this comment

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

Hi and thanks for contributing! I've skimmed the hosting portion of your changes, and I'll make another pass for the routing changes in the next day or so.

if (io->bind_address && (uv_err = uv_udp_bind(&io->server.udp, io->bind_address->ai_addr, 0)) < 0)
return uv_err;

if ((uv_err = udp_connect(&io->server.udp, remote_address->ai_addr)) < 0)
Copy link
Member

Choose a reason for hiding this comment

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

should this be uv_udp_connect?

}

static hosted_io_context hosted_io_context_new(struct hosted_service_ctx_s *service_ctx, ziti_connection client,
tunneler_app_data *app_data, const char *dst_protocol, const char *dst_ip_or_hn, const char *dst_port) {
hosted_io_context io = calloc(1, sizeof(struct hosted_io_ctx_s));

if (!io)
return NULL;
Copy link
Member

Choose a reason for hiding this comment

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

This is worth logging an ERROR message

if (uv_err != 0) {
ZITI_LOG(ERROR, "hosted_service[%s], client[%s]: uv_udp_connect failed: %s",
io->service->service_name, io->client_identity, uv_strerror(uv_err));
ZITI_LOG(ERROR, "hosted_service[%s] client[%s]: uv_udp_open failed: %s",
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to log a generic message here (like you did for the TCP case) rather than mention a function that may (or may not) have been the cause of the failure.

int uv_err;

uv_err = ziti_tunnel_hosting_socket(&sock, remote_address);
if (uv_err < 0 && uv_err != UV_ENOSYS)
Copy link
Member

Choose a reason for hiding this comment

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

Please log errors for each of the failure paths in this function, also.

* remote_address is made compatible with bind_address
*/
uv_err = ziti_tunnel_hosting_socket(&sock, remote_address);
if (uv_err < 0 && uv_err != UV_ENOSYS)
Copy link
Member

Choose a reason for hiding this comment

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

Please log errors for each of the failure paths in this function. It will make it much easier to support users if we know exactly which function failed when something goes wrong.

uv_err = do_tcp_connect(io, res, on_hosted_tcp_server_connect_complete);
if (uv_err != 0) {
ZITI_LOG(ERROR, "hosted_service[%s] client[%s]: TCP connection failed: %s",
io->service->service_name, io->client_identity, uv_strerror(uv_err));
Copy link
Member

Choose a reason for hiding this comment

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

We should omit the uv error string here, especially if the specific failures are logged in do_tcp_connect

@@ -737,6 +818,9 @@ static void on_hosted_client_connect_resolved(uv_getaddrinfo_t* ai_req, int stat

uv_freeaddrinfo(res);
free(ai_req);

uv_freeaddrinfo(io->bind_address);
Copy link
Member

Choose a reason for hiding this comment

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

is it sufficient to let hosted_io_context_free do this?

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