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

tty path checking unhelpfully hides problems #329

Closed
gdt opened this issue Nov 18, 2023 · 4 comments
Closed

tty path checking unhelpfully hides problems #329

gdt opened this issue Nov 18, 2023 · 4 comments

Comments

@gdt
Copy link

gdt commented Nov 18, 2023

I am using 1.9.14p3 on NetBSD. I changed from /dev/ttyp* to /dev/pts, but did not reboot. I found that sudo when executed as part of a package rebuild ("make replace") did not respect cached authtentication. The root causes are

  1. I had not run dev_mkdb, and devname(3) returned stale paths
  2. sudo took the device major/minor, converted it to a path, and then checked if the path was valid. If so, the dev_t was stored in the timestamp file. If not, sudo silently flipped to ppid mode.

To figure this out, I had to read code and write my own code to parse the timestamp file.

I would suggest

  1. If the tty path does not exist, log a very loud error and error out. This is a failure of something that ought to be true always, and it is both unhelpful and unsafe to do something else instead
  2. If what matters is the dev_t, do not convert to names. Just use dev_t.
millert added a commit that referenced this issue Nov 26, 2023
@millert
Copy link
Collaborator

millert commented Nov 26, 2023

Resolved by 3dfbf93 and a85494b.

  1. If sudo can determine the user's tty device but cannot resolve it to a file name, it will now warn the user (previously it was a debug message) and behave as if no terminal is present. This is safer than erroring out on systems where there is no root password and sudo is the only way to access the root account.
  2. The dev_t is now passed from the front-end to the policy module where it can be used as-is in the time stamp file.
  3. The tsdump program in the sudo source distribution can be used to list the contents of a sudo time stamp file. It is not currently installed by default, perhaps it could go in sudo's libexec directory.

@millert millert closed this as completed Nov 26, 2023
@gdt
Copy link
Author

gdt commented Nov 26, 2023

Thanks very much for addressing.

About tsdump, I concluded that the sources had no such program after seeing comments that were apparently sort of from the project indicating that such a program was harmful and people who needed it should write their own. Perhaps I misread. A possible reasonable approach would be to mention it in the README, build it as part of the build, but not install it.

@millert
Copy link
Collaborator

millert commented Nov 26, 2023

@gdt I just committed changes to build tsdump by default and mention it in the sudoers and sudoers_timestamp man pages.

@gdt
Copy link
Author

gdt commented Nov 26, 2023

Awesome, thank you for listening and improving.

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

No branches or pull requests

2 participants