Skip to content

std: make address resolution weirdness local to SGX #145327

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joboet
Copy link
Member

@joboet joboet commented Aug 12, 2025

Currently, the implementations of TcpStream::connect and its cousins take an io::Result<&SocketAddr> as argument, which is very weird, as most of them then ?-try the result immediately to access the actual address. This weirdness is however necessitated by a peculiarity of the SGX networking implementation:

SGX doesn't support DNS resolution but rather accepts hostnames in the same place as socket addresses. So, to make e.g.

TcpStream::connect("example.com:80")`

work, the DNS lookup returns a special error (NonIpSockAddr) instead, which contains the hostname being looked up. When .to_socket_addrs() fails, the each_addr function used to select an address will pass the error to the inner TcpStream::connect implementation, which in SGX's case will inspect the error and try recover the hostname from it. If
that succeeds, it continues with the found hostname.

This is pretty obviously a terrible hack and leads to buggy code (for instance, when users use the result of .to_socket_addrs() in their own ToSocketAddrs implementation to select from a list of possible URLs, the only URL used will be that of the last item tried). Still, without changes to the SGX usercall ABI, it cannot be avoided.

Therefore, this PR aims to minimise the impact of that weirdness and remove it from all non-SGX platforms. The inner TcpStream::connect, et al. functions now receive the ToSocketAddrs type directly and call each_addr (which is moved to sys::net::connection) themselves. On SGX, the implementation uses a special each_addr which contains the whole pass-hostname-through-error hack.

As well as making the code cleaner, this also opens up the possibility of reusing newly created sockets even if a connection request fails – but I've left that for another PR.

CC @raoulstrackx

@rustbot
Copy link
Collaborator

rustbot commented Aug 12, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 12, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved the whole platform-matching into this new file so there's a place to put each_addr.

}
}

mod connection;
Copy link
Member Author

Choose a reason for hiding this comment

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

If you were wondering why this is its own module, it's to facilitate #135141, which has a different platform distribution.

@rust-log-analyzer

This comment has been minimized.

@joboet joboet force-pushed the net-addr-sgx-hack branch from 936d414 to 842d4de Compare August 13, 2025 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants