Skip to content

Sudo to the user not to root for readhome#526

Open
Kunzol wants to merge 10 commits into
openpubkey:mainfrom
Kunzol:user-sudo
Open

Sudo to the user not to root for readhome#526
Kunzol wants to merge 10 commits into
openpubkey:mainfrom
Kunzol:user-sudo

Conversation

@Kunzol

@Kunzol Kunzol commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

In scenarios with user home directories in NFS and root_squash active, in this case
root accesses the user home directory as user "nobody" and can not read the
~/.opk/auth_id file. As the username is known, the sudo is using this username
to read the file as the user instead of root.

Kunzol and others added 4 commits June 12, 2026 07:37
In scenarios with user home directories in NFS and root_squash active, in this case
root accesses the user home directory as user "nobody" and can not read the
`~/.opk/auth_id` file. As the username is known, the sudo is using this username
to read the file as the user instead of root.
In scenarios with user home directories in NFS and root_squash active, in this case
root accesses the user home directory as user "nobody" and can not read the
`~/.opk/auth_id` file. As the username is known, the sudo is using this username
to read the file as the user instead of root.

Signed-off-by: Marco Schmidt <[email protected]>
@EthanHeilman

Copy link
Copy Markdown
Member

I planned to review this tonight, but I can't quite wrap my head around it at this late hour. I'll have to look at it tomorrow.

@Kunzol

Kunzol commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Take your time. Opkssh is a security application. It's crucial to examine it carefully !

From my point of view, doing sudo to a user is more secure than sudo to root.

@EthanHeilman

EthanHeilman commented Jun 22, 2026

Copy link
Copy Markdown
Member

I looked into why we use root and not the username since I wanted to do this back in March 2025. It seems like the user is that:

  1. sudo -u must be supported for this to work
  2. if sudo -u is not supported than we need sudo regex for this to be secure so that someone can't supply the username root rm -rf / and have it run sudo -u root rm -rf /; cat ~/.opk/auth_id and not everyone supports sudo regex

opkssh ALL=(ALL) NOPASSWD: sudo -u [a-z0-9-_.]+ cat ~/.opk/auth_id
This might work if I can get regex in the sudoers file to work. So far I've been running into issues with sudo regex. I have a version that should support it, but it doesn't seem to work. openpubkey/openpubkey#280 (comment)

I think your approach can be made to work, but to keep backwards compatibility we can fall back to using root:

try {
  cmd = exec.Command("sudo", "-n", "-u", username, opkBin, "readhome", username)
}
if fails {
  cmd = exec.Command("sudo", "-n", opkBin, "readhome", username)
}

If you change it to try username and the fallback to root and then add some unittests. I'll merge your PR.

Thanks for looking into this @Kunzol

@Kunzol

Kunzol commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for checking.

I have never seen a sudo without "-u", but of course, I don't know all systems in the world.

In sudoers file the part in the bracket is telling as which user a command can be run. This implies the existence of "-u". It might be worth considering if the sudoers line should be like this, to prevent the opksshuser to get root:

opksshuser ALL=(ALL,!root) NOPASSWD: /usr/bin/opkssh readhome *

Unfortunately I have no clue of programming "go". I work like a AI and check the existing code for something I need and use it. I will try ...

@Kunzol

Kunzol commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

I just tried and found another aspect:

If sudo returns an error, it is not clear if the error comes from sudo or from the command which was run by sudo.
For example, if I run this and get an error back:

sudo -n -u user /usr/local/bin/opkssh readhome user

The returned error code "1" could have these causes:

  1. the option -u is not known by sudo
  2. I am not allowed to run sudo
  3. the user user does not exist
  4. opkssh readhome user returned an error

Only in (1) it makes sense to run sudo again without -u.

With opkssh it might be possible to return a special exit code to detect (4).

But what about (2) and (3) ?

I will think about it ...

@EthanHeilman

Copy link
Copy Markdown
Member

@Kunzol I don't think we need to determine the exact error. The sudo -u issue is just about backwards compatibility, so if we try to first attempt and it fails, we try the second attempt as a backstop to ensure we don't break backwards compatibility.

@Kunzol

Kunzol commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

I don't like the idea to do sudo again with higher level of access in case of any failure.

Additionally I would like to use the timeout option of sudo, to prevent a nasty user with a large auth_id file or other nasty things.

Maybe enable "-u" and "-T" only if opkssh was compiled with CGO_ENABLED?
And/or "-T" only if a variable "OPKSSHSUDOTIMEOUT" has a valid number?

Kunzol and others added 5 commits June 24, 2026 10:45
To differentiate between sudo error and opkssh error
I added a special exit code for failure of
```opkssh readhome username```
With the exit code it is possible to choose to start
another sudo (on sudo error) or not (on opkssh error)
The constant ExitcCodeReadHome  needs to be available
in Linux and Windows builds.
@Kunzol

Kunzol commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

After reading the sudo manual more carefully, I noticed, that the -T option is the timeout for entering the password.

The timeout for the sudo command needs to be implemented in go, most probably with exec.CommandContext.

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 this pull request may close these issues.

2 participants