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

Tracking Issue for abstract namespaces in Unix domain sockets #85410

Closed
2 of 4 tasks
mdaverde opened this issue May 17, 2021 · 21 comments · Fixed by #109288
Closed
2 of 4 tasks

Tracking Issue for abstract namespaces in Unix domain sockets #85410

mdaverde opened this issue May 17, 2021 · 21 comments · Fixed by #109288
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@mdaverde
Copy link
Contributor

mdaverde commented May 17, 2021

Feature gate: #![feature(unix_socket_abstract)]

This is a tracking issue for adding abstract namespace support for Unix domain sockets on Linux.

Traditionally, Unix domain sockets (UDS) use a file to coordinate socket binding and connecting. On Linux, though, a specific extension exists to allow domain sockets to be created from a namespace which does not use the filesystem. With these changes, we extend Rust's UDS support to create sockets directly from a SocketAddr which can be itself used to create an abstract namespace.

More information

Previous discussion: #42048
Linux man page: unix(7)

Public API

Non-platform specific additions

UnixListener::bind_addr(&SocketAddr) -> Result<UnixListener>

UnixStream::connect_addr(&SocketAddr) -> Result<()>

UnixDatagram::bind_addr(&SocketAddr) -> Result<UnixDatagram>

UnixDatagram::connect_addr(&SocketAddr) -> Result<()>

UnixDatagram::send_to_addr(&self, &[u8], &SocketAddr) -> Result<usize>

Platform-specific (Linux) additions

SocketAddrExt::from_abstract_name(&[u8]) -> Result<SocketAddr>

SockerAddrExt::as_abstract_name(&self) -> Option<&[u8]>

Example

#![feature(unix_socket_abstract)]
use std::os::unix::net::{UnixListener, SocketAddr};
use std::os::linux::net::SocketAddrExt;

fn main() -> std::io::Result<()> {
    let addr = SocketAddr::from_abstract_namespace(b"namespace")?; // Linux only
    let listener = match UnixListener::bind_addr(&addr) {
        Ok(sock) => sock,
        Err(err) => {
            println!("Couldn't bind: {:?}", err);
            return Err(err);
        }
    };
    Ok(())
}

Steps / History

Unresolved Questions

  • None yet.
@mdaverde mdaverde added 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. labels May 17, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 18, 2021
Add abstract namespace support for Unix domain sockets

Hello! The other day I wanted to mess around with UDS in Rust and found that abstract namespaces ([unix(7)](https://man7.org/linux/man-pages/man7/unix.7.html)) on Linux still needed development. I took the approach of adding `_addr` specific public functions to reduce conflicts.

Feature name: `unix_socket_abstract`
Tracking issue: rust-lang#85410
Further context: rust-lang#42048

## Non-platform specific additions

`UnixListener::bind_addr(&SocketAddr) -> Result<UnixListener>`

`UnixStream::connect_addr(&SocketAddr) -> Result<()>`

`UnixDatagram::bind_addr(&SocketAddr) -> Result<UnixDatagram>`

`UnixDatagram::connect_addr(&SocketAddr) -> Result<()>`

`UnixDatagram::send_to_addr(&self, &[u8], &SocketAddr) -> Result<usize>`

## Platform-specific (Linux) additions

`SocketAddr::from_abstract_namespace(&[u8]) -> SocketAddr`

`SockerAddr::as_abstract_namespace() -> Option<&[u8]>`

## Example

```rust
#![feature(unix_socket_abstract)]
use std::os::unix::net::{UnixListener, SocketAddr};

fn main() -> std::io::Result<()> {
    let addr = SocketAddr::from_abstract_namespace(b"namespace")?; // Linux only
    let listener = match UnixListener::bind_addr(&addr) {
        Ok(sock) => sock,
        Err(err) => {
            println!("Couldn't bind: {:?}", err);
            return Err(err);
        }
    };
    Ok(())
}
```

## Further Details

The main inspiration for the implementation came from the [nix-rust](https://github.com/nix-rust/nix/blob/master/src/sys/socket/addr.rs#L558) crate but there are also other [historical](rust-lang@c4db068) [attempts](https://github.com/tormol/uds/blob/master/src/addr.rs#L324) with similar approaches.

A comment I did have was with this change, we now allow a `SocketAddr` to be constructed explicitly rather than just used almost as a handle for the return of `peer_addr` and `local_addr`. We could consider adding other explicit constructors (e.g. `SocketAddr::from_pathname`, `SockerAddr::from_unnamed`).

Cheers!
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 18, 2021
…tt,GuillaumeGomez

Add abstract namespace support for Unix domain sockets

Hello! The other day I wanted to mess around with UDS in Rust and found that abstract namespaces ([unix(7)](https://man7.org/linux/man-pages/man7/unix.7.html)) on Linux still needed development. I took the approach of adding `_addr` specific public functions to reduce conflicts.

Feature name: `unix_socket_abstract`
Tracking issue: rust-lang#85410
Further context: rust-lang#42048

## Non-platform specific additions

`UnixListener::bind_addr(&SocketAddr) -> Result<UnixListener>`

`UnixStream::connect_addr(&SocketAddr) -> Result<()>`

`UnixDatagram::bind_addr(&SocketAddr) -> Result<UnixDatagram>`

`UnixDatagram::connect_addr(&SocketAddr) -> Result<()>`

`UnixDatagram::send_to_addr(&self, &[u8], &SocketAddr) -> Result<usize>`

## Platform-specific (Linux) additions

`SocketAddr::from_abstract_namespace(&[u8]) -> SocketAddr`

`SockerAddr::as_abstract_namespace() -> Option<&[u8]>`

## Example

```rust
#![feature(unix_socket_abstract)]
use std::os::unix::net::{UnixListener, SocketAddr};

fn main() -> std::io::Result<()> {
    let addr = SocketAddr::from_abstract_namespace(b"namespace")?; // Linux only
    let listener = match UnixListener::bind_addr(&addr) {
        Ok(sock) => sock,
        Err(err) => {
            println!("Couldn't bind: {:?}", err);
            return Err(err);
        }
    };
    Ok(())
}
```

## Further Details

The main inspiration for the implementation came from the [nix-rust](https://github.com/nix-rust/nix/blob/master/src/sys/socket/addr.rs#L558) crate but there are also other [historical](rust-lang@c4db068) [attempts](https://github.com/tormol/uds/blob/master/src/addr.rs#L324) with similar approaches.

A comment I did have was with this change, we now allow a `SocketAddr` to be constructed explicitly rather than just used almost as a handle for the return of `peer_addr` and `local_addr`. We could consider adding other explicit constructors (e.g. `SocketAddr::from_pathname`, `SockerAddr::from_unnamed`).

Cheers!
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 15, 2021
Add abstract namespace support for Unix domain sockets

Hello! The other day I wanted to mess around with UDS in Rust and found that abstract namespaces ([unix(7)](https://man7.org/linux/man-pages/man7/unix.7.html)) on Linux still needed development. I took the approach of adding `_addr` specific public functions to reduce conflicts.

Feature name: `unix_socket_abstract`
Tracking issue: rust-lang#85410
Further context: rust-lang#42048

## Non-platform specific additions

`UnixListener::bind_addr(&SocketAddr) -> Result<UnixListener>`

`UnixStream::connect_addr(&SocketAddr) -> Result<()>`

`UnixDatagram::bind_addr(&SocketAddr) -> Result<UnixDatagram>`

`UnixDatagram::connect_addr(&SocketAddr) -> Result<()>`

`UnixDatagram::send_to_addr(&self, &[u8], &SocketAddr) -> Result<usize>`

## Platform-specific (Linux) additions

`SocketAddr::from_abstract_namespace(&[u8]) -> SocketAddr`

`SockerAddr::as_abstract_namespace() -> Option<&[u8]>`

## Example

```rust
#![feature(unix_socket_abstract)]
use std::os::unix::net::{UnixListener, SocketAddr};

fn main() -> std::io::Result<()> {
    let addr = SocketAddr::from_abstract_namespace(b"namespace")?; // Linux only
    let listener = match UnixListener::bind_addr(&addr) {
        Ok(sock) => sock,
        Err(err) => {
            println!("Couldn't bind: {:?}", err);
            return Err(err);
        }
    };
    Ok(())
}
```

## Further Details

The main inspiration for the implementation came from the [nix-rust](https://github.com/nix-rust/nix/blob/master/src/sys/socket/addr.rs#L558) crate but there are also other [historical](rust-lang@c4db068) [attempts](https://github.com/tormol/uds/blob/master/src/addr.rs#L324) with similar approaches.

A comment I did have was with this change, we now allow a `SocketAddr` to be constructed explicitly rather than just used almost as a handle for the return of `peer_addr` and `local_addr`. We could consider adding other explicit constructors (e.g. `SocketAddr::from_pathname`, `SockerAddr::from_unnamed`).

Cheers!
@mexus
Copy link

mexus commented Nov 17, 2021

Is there anything else that needs to be done? 🤔

Seems like the PR from the first message has been already merged, so isn't it time for the FCP? :)

Also it seems kinda strange that there's no assignees in this issue.

@DataTriny
Copy link

Hi @mdaverde ,

Thank you so much for taking care of this.

I am already using the API added on #85379 for AccessKit and I haven't encountered any issue so far.

@joshtriplett you were the one reviewing the PR. Isn't it time for the FCP now?

@the8472
Copy link
Member

the8472 commented May 9, 2022

If I understand #76915 (comment) correctly then the methods on os::unix::SocketAddr shouldn't be cfg(any(doc, target_os = "android", target_os = "linux",)) and instead should go into the os::linux and os::android modules as extension traits.

@krisnova
Copy link

krisnova commented Sep 9, 2022

Following up on this and this is my first time engaging with the Rust community so forgive me if I make any careless mistakes.

If I understand correctly this proposal needs to move to a final commenting period (FCP) until the feature reaches stabilization.

Is there anything I can do to contribute and help carry this feature forward? This is a feature is important to myself as well as most folks coming to Rust looking for support for Linux userspace feature implementation in a language other than C.

Thank you for your consideration.

@the8472
Copy link
Member

the8472 commented Sep 9, 2022

A remaining issue is that the methods are in the wrong place, they need to be moved to the os::linux module since they're linux specific and not available on other unixes.

@krisnova
Copy link

krisnova commented Sep 9, 2022

Thank you for the context. I'll see if I can't make a contribution to move them to the Linux specific module.

Do you know if there are any other outstanding items that would need to be addressed before this can move to FCP?

@jmillikin
Copy link
Contributor

I'm also interested in seeing this feature stabilized, and made an attempt at moving the API into SocketAddrExt: #101967

@joshtriplett
Copy link
Member

I've reviewed and approved #101967, which moves the Linux-specific methods into an extension trait.

With that merged, I don't think there are any other blockers here.

So, let's see if we have consensus to stabilize UNIX socket abstract namespace support.

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Nov 14, 2022

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 14, 2022
@joshtriplett joshtriplett added the I-libs-api-nominated The issue / PR has been nominated for discussion during a libs-api team meeting. label Nov 14, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 14, 2022
…dr, r=joshtriplett

Move `unix_socket_abstract` feature API to `SocketAddrExt`.

The pre-stabilized API for abstract socket addresses exposes methods on `SocketAddr` that are only enabled for `cfg(any(target_os = "android", target_os = "linux"))`. Per discussion in <rust-lang#85410>, moving these methods to an OS-specific extension trait is required before stabilization can be considered.

This PR makes four changes:
1. The internal module `std::os::net` contains logic for the unstable feature `tcp_quickack` (rust-lang#96256). I moved that code into `linux_ext/tcp.rs` and tried to adjust the module tree so it could accommodate a second unstable feature there.
2. Moves the public API out of `impl SocketAddr`, into `impl SocketAddrExt for SocketAddr` (the headline change).
3. The existing function names and docs for `unix_socket_abstract` refer to addresses as being created from abstract namespaces, but a more accurate description is that they create sockets in *the* abstract namespace. I adjusted the function signatures correspondingly and tried to update the docs to be clearer.
4. I also tweaked `from_abstract_name` so it takes an `AsRef<[u8]>` instead of `&[u8]`, allowing `b""` literals to be passed directly.

Issues:
1. The public module `std::os::linux::net` is marked as part of `tcp_quickack`. I couldn't figure out how to mark a module as being part of two unstable features, so I just left the existing attributes in place. My hope is that this will be fixed as a side-effect of stabilizing either feature.
@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Dec 30, 2022
@mllken
Copy link
Contributor

mllken commented Jan 3, 2023

Sorry for the late comment. The current signature of from_abstract_name in nightly is:

fn from_abstract_name<N>(name: &N) -> Result<SocketAddr>
where
    N: AsRef<[u8]>;

but, I think it should be:

fn from_abstract_name<N>(name: N) -> Result<SocketAddr>
where
    N: AsRef<[u8]>;

The latter allows me to easily pass in a &str or &[u8] directly, without encountering Sized errors

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jan 12, 2023
@joshtriplett
Copy link
Member

@rfcbot concern wait-for-106441

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jan 17, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jan 18, 2023
…shtriplett

relax reference requirement on SocketAddrExt::from_abstract_name

Reference: rust-lang#85410 (comment)
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jan 18, 2023
…shtriplett

relax reference requirement on SocketAddrExt::from_abstract_name

Reference: rust-lang#85410 (comment)
bors pushed a commit to rust-lang/miri that referenced this issue Jan 23, 2023
relax reference requirement on SocketAddrExt::from_abstract_name

Reference: rust-lang/rust#85410 (comment)
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
…htriplett

Move `unix_socket_abstract` feature API to `SocketAddrExt`.

The pre-stabilized API for abstract socket addresses exposes methods on `SocketAddr` that are only enabled for `cfg(any(target_os = "android", target_os = "linux"))`. Per discussion in <rust-lang/rust#85410>, moving these methods to an OS-specific extension trait is required before stabilization can be considered.

This PR makes four changes:
1. The internal module `std::os::net` contains logic for the unstable feature `tcp_quickack` (rust-lang/rust#96256). I moved that code into `linux_ext/tcp.rs` and tried to adjust the module tree so it could accommodate a second unstable feature there.
2. Moves the public API out of `impl SocketAddr`, into `impl SocketAddrExt for SocketAddr` (the headline change).
3. The existing function names and docs for `unix_socket_abstract` refer to addresses as being created from abstract namespaces, but a more accurate description is that they create sockets in *the* abstract namespace. I adjusted the function signatures correspondingly and tried to update the docs to be clearer.
4. I also tweaked `from_abstract_name` so it takes an `AsRef<[u8]>` instead of `&[u8]`, allowing `b""` literals to be passed directly.

Issues:
1. The public module `std::os::linux::net` is marked as part of `tcp_quickack`. I couldn't figure out how to mark a module as being part of two unstable features, so I just left the existing attributes in place. My hope is that this will be fixed as a side-effect of stabilizing either feature.
@JohnTitor
Copy link
Member

Triage: #106441 has been merged, the concern can be resolved now.

@jmillikin
Copy link
Contributor

@joshtriplett @JohnTitor Are there any remaining approvals required for this, or should I put together a stabilization PR?

@joshtriplett
Copy link
Member

I think this is ready for a stabilization PR.

@JohnTitor JohnTitor removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 17, 2023
@JohnTitor
Copy link
Member

Dropped the "proposed" label to reflect the status, FCP was done here.

@jmillikin
Copy link
Contributor

Thanks! Stabilization PR is #109288

@joshtriplett
Copy link
Member

@rfcbot resolved wait-for-106441

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 18, 2023
@rfcbot
Copy link

rfcbot commented Mar 18, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 18, 2023
…dr, r=joshtriplett

Stabilise `unix_socket_abstract`

Fixes rust-lang#85410
@bors bors closed this as completed in a3f3db8 Mar 18, 2023
saethlin pushed a commit to saethlin/miri that referenced this issue Mar 19, 2023
thomcc pushed a commit to tcdi/postgrestd that referenced this issue May 31, 2023
relax reference requirement on SocketAddrExt::from_abstract_name

Reference: rust-lang/rust#85410 (comment)
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jun 1, 2023
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.