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.

Sign up for GitHub

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

Jump to bottom

Peer credentials on Unix socket #42839

Open
Kixunil opened this issue Jun 22, 2017 · 22 comments
Open

Peer credentials on Unix socket #42839

Kixunil opened this issue Jun 22, 2017 · 22 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Kixunil
Copy link
Contributor

Kixunil commented Jun 22, 2017

tokio-rs/tokio-uds#13 was recently merged, so I'd like to open an issue of adding same thing to stdlib. TL;DR It allows one to get effective UID and GID of the process which created the remote socket.

I suggest to test it with Tokio now and when there is consensus, we can merge similar change. I need this function, so I'm going to test it anyway.
@Mark-Simulacrum Mark-Simulacrum added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 22, 2017
@Kixunil Kixunil mentioned this issue Jun 22, 2017
Merged
@Kixunil
Copy link
Contributor Author

Kixunil commented Jul 11, 2017

Considering that newtypes for pid_t, uid_t and gid_t landed in nix, we shoul consider using same newtypes in std and then simply reexport them in nix.

@sfackler
Copy link
Member

This seems like a reasonable addition. It looks like there are some extra platforms that require special handling though - Solaris and FreeBSD: https://doxygen.postgresql.org/getpeereid_8c_source.html

@Kixunil
Copy link
Contributor Author

Kixunil commented Jul 11, 2017

The implementation in tokio handles Linux and FreeBSD (incl. macOS). I doesn't handle Solaris because I don't have Solaris machine.

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 27, 2017
@dtolnay dtolnay added C-feature-accepted Category: A feature request that has been accepted pending implementation. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Nov 19, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 19, 2017

This isn't my area of expertise but I would be interested to see an API proposed for this in a PR.

@zeenix zeenix mentioned this issue Dec 14, 2019
Open
@joechrisellis
Copy link
Contributor

Hi guys. We're quite keen for something like this in the stdlib. The implementation in the Tokio repository looks good to me, and from a cursory glance it looks like the stdlib can be very similar. I'm happy to give this a crack -- should I submit an RFC first or is a normal code pull request OK? 😄

Cheers! 🙂

@hug-dev
Copy link
Contributor

hug-dev commented Aug 28, 2020

The PR which implements this issue is ready for review 🚀
#75148

@Kixunil
Copy link
Contributor Author

Kixunil commented Sep 16, 2020

Since the PR was merged should the labels (and perhaps also the title) be updated to tracking issue?

@hug-dev
Copy link
Contributor

hug-dev commented Sep 16, 2020

I would agree with that!
I guess the steps to follow for stabilization are here?

@hug-dev hug-dev mentioned this issue Sep 16, 2020
Merged
@ghost ghost mentioned this issue Sep 22, 2020
Open
@woodruffw
Copy link
Contributor

I noticed that #75148 doesn't include the PID on macOS/iOS, presumably because it uses getpeereid. However, macOS/iOS do provide LOCAL_PEERPID, which can be used with getsockopt to get just the peer's PID. Would anybody be opposed to a follow-up PR that adds support via LOCAL_PEERPID on macOS?

@woodruffw woodruffw mentioned this issue Nov 24, 2020
Merged
@jonas-schievink jonas-schievink added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. and removed C-feature-accepted Category: A feature request that has been accepted pending implementation. labels Nov 24, 2020
@Kixunil
Copy link
Contributor Author

Kixunil commented Nov 24, 2020

Yeah, I don't see a reason to not do that. It'd be nice to document which platforms support it.

@woodruffw
Copy link
Contributor

Yeah, I don't see a reason to not do that. It'd be nice to document which platforms support it.

Sounds good! I've opened #79387. And yeah, documentation for these across the different Unices can be difficult to find:

@tormol
Copy link
Contributor

tormol commented Nov 24, 2020

The cr_pid on FreeBSD was added somewhat recently and is only available in FreeBSD 13 which is not released yet. The man page for FreeBSD 12 doesn't mention it.

The uds crate can be used to get peer credentials for blocking sockets on stable.

@Kixunil
Copy link
Contributor Author

Kixunil commented Nov 24, 2020

I write which platforms std supports in its doc comment.

@voidc
Copy link
Contributor

voidc commented Dec 8, 2020

With #69864 merged there are now two very similar types: UCred and SocketCred. Can these two types be combined into one?

@woodruffw
Copy link
Contributor

woodruffw commented Dec 8, 2020 •
edited
Loading

With #69864 merged there are now two very similar types: UCred and SocketCred. Can these two types be combined into one?

Potentially? Looking at the current code, the two currently perform slightly different functions: UCred (which I think is badly named, more on that below) is used in a somewhat platform agnostic way to get the peer credentials associated with a domain socket client, while SocketCred seems to be associated with ancillary data mechanisms that I'm not 100% clear on (I've never touched them at all).

IMO, it makes sense to rename UCred to PeerCred (since struct ucred is a non-POSIX implementation detail and we're only using it in a peer credential context) and to unify it with SocketCred, if possible. I'm not sure about the latter, since it means changing Option<pid_t> to a standalone pid_t, and I'm not sure if all (Unixy) OSes support getting a peer's PID.

@Kixunil
Copy link
Contributor Author

Kixunil commented Dec 8, 2020

I'm not sure if all (Unixy) OSes support getting a peer's PID.

They don't, that's the reason there's Option in the first place. The structs seem to be sufficiently different to be different types but I may be missing something.

@woodruffw
Copy link
Contributor

They don't, that's the reason there's Option in the first place.

I'm not sure if it's 100% of Unices, but I think almost all of them support some mechanism for getting the peer's PID. It's just not through struct ucred, hence my comment about UCred being a misleading wrapper.

For example, macOS doesn't have a PID in its struct ucred, but it does support getting the peer's PID through the LOCAL_PEERPID sockopt. All of the BSDs expose similar mechanisms.

@Kixunil
Copy link
Contributor Author

Kixunil commented Dec 8, 2020

UCred being a misleading wrapper.

Sure, I agree with renaming.

but it does support getting the peer's PID through the LOCAL_PEERPID sockopt.

Yes, this is implemented. :)

All of the BSDs expose similar mechanisms.

A PR would be nice if you happen to have some experience with this. Shouldn't be big, most of the stuff is already there.

@woodruffw
Copy link
Contributor

A PR would be nice if you happen to have some experience with this. Shouldn't be big, most of the stuff is already there.

I can definitely try! I have #79387 open for macOS right now, and I'll try to get some similar ones in the pipeline for the BSDs.

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 9, 2020
@bors
Auto merge of rust-lang#79387 - woodruffw-forks:ww/peer-cred-pid-maco…
c16d52d 
…s, r=Amanieu

ext/ucred: Support PID in peer creds on macOS

This is a follow-up to rust-lang#75148 (RFC: rust-lang#42839).

The original PR used `getpeereid` on macOS and the BSDs, since they don't (generally) support the `SO_PEERCRED` mechanism that Linux supplies.

This PR splits the macOS/iOS implementation of `peer_cred()` from that of the BSDs, since macOS supplies the `LOCAL_PEERPID` sockopt as a source of the missing PID. It also adds a `cfg`-gated tests that ensures that platforms with support for PIDs in `UCred` have the expected data.
@msolters
Copy link

What is the current status of this feature peer_credentials_unix_socket graduating to stable? I was surprised to find this issue is 5 years old.
It's a great feature but it sucks having to be on nightly to get it!

@Kixunil
Copy link
Contributor Author

Kixunil commented Oct 21, 2022

Looks like there were at least no complaints about it apart from UCred being misleading name. Looking at the docs the struct description could use some polishing. Maybe open an ACP? If you have some experience with using it on nightly that should help. I changed my interests over time and don't have it.

@woodruffw
Copy link
Contributor

I'm happy to help with the stabilization here as well.

@msolters: if you do an ACP for renaming UCred, I'd suggest PeerCred (per #42839 (comment)). Otherwise, I can do the ACP and from there begin the stabilization process (at least the outsider bits).

@kadiwa4 kadiwa4 mentioned this issue Mar 7, 2024
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Mar 7, 2024
@GuillaumeGomez
Rollup merge of rust-lang#122147 - kadiwa4:private_impl_mods, r=worki…
92d7e02 
…ngjubilee

Make `std::os::unix::ucred` module private

Tracking issue: rust-lang#42839

Currently, this unstable module exists: [`std::os::unix::ucred`](https://doc.rust-lang.org/stable/std/os/unix/ucred/index.html).
All it does is provide `UCred` (which is also available from `std::os::unix::net`), `impl_*` (which is probably a mishap and should be private) and `peer_cred` (which is undocumented but has a documented counterpart at `std::os::unix::net::UnixStream::peer_cred`).

This PR makes the entire `ucred` module private and moves it into `net`, because that's where it is used.

I hope it's fine to simply remove it without a deprecation phase. Otherwise, I can add back a deprecated reexport module `std::os::unix::ucred`.

`@rustbot` label: -T-libs +T-libs-api
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 8, 2024
@rust-timer
Unrolled build for rust-lang#122147
d65ead2 
Rollup merge of rust-lang#122147 - kadiwa4:private_impl_mods, r=workingjubilee

Make `std::os::unix::ucred` module private

Tracking issue: rust-lang#42839

Currently, this unstable module exists: [`std::os::unix::ucred`](https://doc.rust-lang.org/stable/std/os/unix/ucred/index.html).
All it does is provide `UCred` (which is also available from `std::os::unix::net`), `impl_*` (which is probably a mishap and should be private) and `peer_cred` (which is undocumented but has a documented counterpart at `std::os::unix::net::UnixStream::peer_cred`).

This PR makes the entire `ucred` module private and moves it into `net`, because that's where it is used.

I hope it's fine to simply remove it without a deprecation phase. Otherwise, I can add back a deprecated reexport module `std::os::unix::ucred`.

`@rustbot` label: -T-libs +T-libs-api
ThomasHabets added a commit to ThomasHabets/sim that referenced this issue Aug 8, 2024
@ThomasHabets
rust/sim: Remove dependency on nightly Rust
9dd4bb0 
Awaiting stabilization in
rust-lang/rust#42839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests