pistacheio / pistache

A high-performance REST toolkit written in C++
https://pistacheio.github.io/pistache/
Apache License 2.0
3.12k stars 688 forks source link

Unix domain socket support #1155

Closed win32asm closed 11 months ago

win32asm commented 11 months ago

Adds support for unix domain listener sockets.

Makes minimal changes to do so, avoiding the temptation to refactor the IP and Address classes.

To bind a unix domain listener one can either use an Address constructors that takes a string or char argument or use the Address::fromUnix factory method with a struct sockaddr_un argument to obtain an Address that can be used to create an Endpoint. In either case, the argument must conform to the criteria listed in the Address Format section of the unix(7) manual page.

Adds clauses to code interrogating address family values to handle AF_UNIX.

Code using unix domain addressing shouldn't care about ports or hostnames. Rather than throw exceptions, queries for these values return 0 and "localhost" respectively.

Refactors Listener::bindListener to bypass calls to getaddrinfo for unix domain addresses.

Adds an addrLen_ member to the Address class to allow setting the proper address length value in calls to bind(2) for unix domain sockets.

codecov[bot] commented 11 months ago

Codecov Report

Patch coverage: 60.17% and project coverage change: -0.54% :warning:

Comparison is base (3ec5a68) 78.43% compared to head (7e7c5c9) 77.89%.

:exclamation: Current head 7e7c5c9 differs from pull request most recent head def2367. Consider uploading reports for the commit def2367 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1155 +/- ## ========================================== - Coverage 78.43% 77.89% -0.54% ========================================== Files 53 53 Lines 6923 6990 +67 ========================================== + Hits 5430 5445 +15 - Misses 1493 1545 +52 ``` | [Files Changed](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio) | Coverage Δ | | |---|---|---| | [include/pistache/listener.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9saXN0ZW5lci5o) | `0.00% <ø> (ø)` | | | [include/pistache/net.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9uZXQuaA==) | `68.75% <0.00%> (-15.87%)` | :arrow_down: | | [src/common/net.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9uZXQuY2M=) | `75.81% <43.13%> (-7.24%)` | :arrow_down: | | [src/common/peer.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9wZWVyLmNj) | `80.48% <78.57%> (-1.10%)` | :arrow_down: | | [src/server/listener.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL3NlcnZlci9saXN0ZW5lci5jYw==) | `69.47% <82.92%> (-0.75%)` | :arrow_down: | | [src/server/endpoint.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL3NlcnZlci9lbmRwb2ludC5jYw==) | `87.50% <100.00%> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/pistacheio/pistache/pull/1155/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kiplingw commented 11 months ago

Thanks @win32asm. I'm not very familiar with UNIX domain sockets. Could you let us know what the principle benefit or benefits are to the user?

win32asm commented 11 months ago

Thanks @win32asm. I'm not very familiar with UNIX domain sockets. Could you let us know what the principle benefit or benefits are to the user?

Access to unix domain sockets is provided based on the socket file properties/permissions, in a regular unix fashion - instead of firewall rules. Otherwise it`s an analog of having multiple distinct ways to address the server running on the same (localhost) system.

dennisjenkins75 commented 11 months ago

Nice. I think that I possibly filed a feature request for this years ago (but never acted on it). I'll review this PR in a bit.

dennisjenkins75 commented 11 months ago

My use-case for unix domain sockets would be a 100% hermetic unit test environment for my own code which uses Pistache. Right now, I spin up my app on TCP port 0 (so the OS chooses an ephemeral port), then report that port # by printing it so a specific file descriptor, which my test harness reads. This is clunky. If I could just tell pistache to use a unix socket then my unit test framework could be simplified. My goal was to be able to run multiple different unit tests concurrently, and not have to worry about TCP port conflicts.

glenn-skinner commented 11 months ago

I didn't see a place to reply to @Tachi107's inital comment above, so I'll do it here.

Here's some motivation (it meshes with the reverse proxy comment above):

unix domain sockets provide a host-local communication mechanism that provides socket semantics. Since communication is confined to the local host, many potential security problems become non-issues. Naming is done through the file system name space rather than through IP addresses.

(Additionally, one can use unix domain sockets to pass state from one process to another that is well-defined only within the scope of the local host. The canonical example of such state is file descriptors; for example, one can open a file and pass a descriptor for it to a peer process that might not itself have permission to do so.)

The specific use case that’s motivated us to add unix domain socket support to Pistache concerns the VMware ecosystem. VMware provides APIs to allow third party vendors (such as us) to interpose on IO operations made by virtual machines running on ESX hosts, allowing those vendors to provide services such as caching, backup and migration, and disaster recovery. These services run partially within the target VMs and partially in separate processes running as appliances, either on-host or off-host.

Communication with these appliances is structured as REST interfaces, which is where Pistache enters the picture. VMware has recently chosen to funnel all off-host communication through a proxy that makes the peer available as a unix domain socket. To maintain the REST communication abstraction we need a way to allow our server to bind to a unix domain socket listener that the proxy can connect to.

As far as class refactoring is concerned, if left with a free hand, I think the designers of the Unix socket addressing structs largely got it right and I would restructure to mirror them. I think the Address class should be thought of as roughly equivalent to struct_sockaddr, providing services that are common to all addresing schemes (or address families in socket terminology).

At the implementation level, I would push the addr_ member currently in the IP class (a struct sockaddr_storage) up into the Address class. The IP class would then either become a subclass of Address or have an Address member. (I haven't thought about it enought to decide whether an is-a of a has-a relationship would work better, although I'm leaning toward is-a.) It would rely on Address to store its socket address. There would be another class sitting alongside IP for unix domain addresses. That refactoring would remove some of the awkwardness present in the current patch request.

I haven't had a chance to look at more specific comments yet. I'll start working through them today.

(I'm a github newbie, so please forgive formatting glitches, misunderstandings about patch versions, and so on.)

kiplingw commented 11 months ago

Great work @glenn-skinner. You've got a very interesting use case for us in production code.

glenn-skinner commented 11 months ago

I think I've resolved all review comments. Please let me know if I've missed any or if the resolution is unsatisfactory.

Is the ABI change worrisome?

Is it time to remove draft status from this pull request?

dennisjenkins75 commented 11 months ago

I'll let @kiplingw and @Tachi107 debate the ABI changes. But I'm fine with moving this PR into a formal review status.

kiplingw commented 11 months ago

LGTM

Tachi107 commented 11 months ago

Merging. Thanks again!