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

Open all files with O_NOCTTY #24307

Closed
l0kod opened this issue Apr 10, 2015 · 7 comments
Closed

Open all files with O_NOCTTY #24307

l0kod opened this issue Apr 10, 2015 · 7 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@l0kod
Copy link
Contributor

l0kod commented Apr 10, 2015

To avoid inconsistent behavior, especially for daemons, the libstd should (by default) use O_NOCTTY for all open files.
The OpenOptions builder could be extended to switch this flag off.

cc #24306

@alexcrichton
Copy link
Member

This seems like a somewhat obscure flag to be passing by default, but are you aware of any other libraries which do this by default?

@steveklabnik steveklabnik added A-libs C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Apr 13, 2015
@l0kod
Copy link
Contributor Author

l0kod commented Apr 16, 2015

All software dealing with TTY (i.e. TUI tools, startup daemons…) does or should handle controlling TTY, including the libraries they could use (e.g. OpenSSL, ). Cf. Debian code for more examples.

Anyway, it seems a sane default option needed for deterministic behavior. Not using O_NOCTTY by default means doing something you don't ask for: getting a controlling TTY.

If a daemon, which does/must not have a controlling TTY, open a file or file descriptor form an insecure location, it could get an unwanted controlling TTY. It need to take care of all open(2), especially from every used Rust libraries.

This could also have an impact on sandboxing (e.g. with gaol) or cause unattended/inconsistent behavior because of signal handling.

cc @pcwalton
cc #11203

@l0kod
Copy link
Contributor Author

l0kod commented Jun 21, 2015

cc #26470

@steveklabnik
Copy link
Member

Triage: I am not aware of any changes here. @rust-lang/libs is this something we still want to consider?

@alexcrichton alexcrichton added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 4, 2017
@brson
Copy link
Contributor

brson commented Jan 9, 2017

The flag does seem appropriate to use by default from reading the docs: "If set and path identifies a terminal device, open() shall not cause the terminal device to become the controlling terminal for the process." That implies to me that std is unsuitable for opening TTY devices.

@alexcrichton
Copy link
Member

This was discussed during libs triage today and the decision was to close. This has been open quite awhile and looking back there doesn't seem to be strong motivation listed here for why we should pass this flag. If this is still an issue though please feel free to open a new issue!

@l0kod
Copy link
Contributor Author

l0kod commented Jan 10, 2017

The documentation should mention, as says @brson, that the standard library is unsuitable for opening a TTY device.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants