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

Unix Domain Socket Credential Support with Connected Trait #365

Closed
dfreese opened this issue Jun 1, 2020 · 13 comments · Fixed by #647
Closed

Unix Domain Socket Credential Support with Connected Trait #365

dfreese opened this issue Jun 1, 2020 · 13 comments · Fixed by #647
Milestone

Comments

@dfreese
Copy link
Contributor

dfreese commented Jun 1, 2020

We're using tonic, and want to be able to use it with unix domain sockets. When using them, we want to be able to validate the peer_cred of the tokio::net::UnixStream, for example.

It looks like Request gets it's information from the Connected trait, which is empty in the UDS example

Is peer_cred something that you'd be willing to expose in Request? I understand that would be a little messy, in that it starts to open you up to supporting a number of different connection types through the Request interface.

(BTW: I'm happy to add this, or something else if it seems like there would be a better way of going about it)

@LucioFranco
Copy link
Member

If I am miss understanding something, shouldn't this work? https://docs.rs/tonic/0.2.1/tonic/struct.Request.html#method.remote_addr

@dfreese
Copy link
Contributor Author

dfreese commented Jun 4, 2020

So two of the functions UnixStream exposes are peer_cred(&self) which returns tokio::net::unix::UCred and peer_addr(&self) which returns a std::os::unix::net::SocketAddr.

Neither of those are really compatible with std::net::SocketAddr because that expects an IP and a port, but the unix credentials have a user id and group id of the connecting process and the unix socket address is generally a file path.

I could, technically, shoehorn the group id and user id into the flowinfo and scope_id of std::net::SocketAddrV6, since they're u32, but that would be quite the hack isn't what you meant, I assume.

@LucioFranco
Copy link
Member

@dfreese ah yeah, socketaddr from net didn't make sense but I forgot to check the module :)

This is an interesting use case that I am not 100% sure how to fix just yet. We might want to coordinate with hyper on this as well.

@LucioFranco LucioFranco added this to the 0.4 milestone Nov 27, 2020
dfreese pushed a commit to dfreese/tonic that referenced this issue Feb 26, 2021
In the process of updating our code to tokio 1.x, I had a little more
trouble migrating the code I had on tokio 0.2 because the types that we
used disappeared, particularly tokio::next::unit::Incoming.  Drawing on
the example of hyper::server::conn::AddrIncoming, this creates a
UnixIncoming struct in the example, and then uses that within the server
code.

I generally found this cleaner, though I understand that is subjective.
I also thought it could be useful to inform any further unix api
discussions, similar in the vein of hyperium#365.
@abbec
Copy link

abbec commented Mar 22, 2021

We also have this use case :) After researching a bit seems to be more blocked by hyper capabilities than tonic atm?

@abbec
Copy link

abbec commented Mar 22, 2021

One thought might be to have the ability in the Connected trait to add custom metadata to the connection that is then accessible through the Request?

@abbec
Copy link

abbec commented Mar 22, 2021

That way you could associate data from a connection with any Request created from it

@abbec
Copy link

abbec commented Mar 22, 2021

Or any way to get back the originally yielded item in the incoming stream. I.e. if incoming is a Stream of UnixListener, we have some way of getting a reference to it from Request. We also have the same problem for Windows NamedPipes. (sorry for monologue)

@dfreese
Copy link
Contributor Author

dfreese commented Mar 22, 2021

Or any way to get back the originally yielded item in the incoming stream. I.e. if incoming is a Stream of UnixListener, we have some way of getting a reference to it from Request.

I like that approach. That would allow for a lot more flexibility for different types of connections. Obviously, I haven't yet had a chance to think about what that'd look like or potential downsides.

@LucioFranco
Copy link
Member

Yeah, i think in this case we might want to try to take advantage of https://docs.rs/http/0.2.4/http/request/struct.Request.html#method.extensions?

@LucioFranco
Copy link
Member

This should be solved via #255

@davidpdrsn
Copy link
Member

@abbec @dfreese I have a possible solution in #647. Do you wanna check it out? Do you think it would fit your use case?

@abbec
Copy link

abbec commented May 22, 2021

Yeah, looks like what we wanted, currently do not need it anymore but seems useful!

@dfreese
Copy link
Contributor Author

dfreese commented May 23, 2021

Thanks @davidpdrsn for getting back to this. That looks like it would certainly fit the use case we had here.

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 a pull request may close this issue.

4 participants