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

feat: enhance privilege elevation logic #722

Merged
merged 8 commits into from
Dec 31, 2024

Conversation

woshikedayaa
Copy link
Contributor

@woshikedayaa woshikedayaa commented Dec 27, 2024

Background

我查看代码发现 cmd/internal/su.go 内部中,对于dae自动使用sudo 来提权对于 linux 桌面端的支持并不是特别完善。
同时方式只有 sudo 方案,对于一些应用程序非 root 运行,需要向 stdin 内部输入 sudo 密码来提权。
同时使用 os.Geteuid 来兼容 setgid.

Checklist

Full Changelogs

把 cmd/internal/su.go 内部的提权分为了两部分

  • 先尝试使用 桌面端 的组件启动 (pkexec) (主流组件,其他组件例如 kdesu 太老了 没有做适配)
func tryPolkit() (path string, arg []string) {
	var possible = []string{"pkexec"}
	for _, v := range possible {
		path, err := exec.LookPath(v)
		if err != nil {
			continue
		}
		if isExistAndExecutable(path) {
			switch v {
			case "pkexec":
				return path, []string{path, "--keep-cwd", "--user", "root"}
			}
		}
	}
	return "", nil
}
  • 再次尝试 sudo
func trySudo() (path string, arg []string) {
	pathSudo, err := exec.LookPath("sudo")
	if err != nil || !isExistAndExecutable(pathSudo) {
		return "", nil
	}
	// https://github.com/WireGuard/wireguard-tools/blob/71799a8f6d1450b63071a21cad6ed434b348d3d5/src/wg-quick/linux.bash#L85
	return pathSudo, []string{
		pathSudo,
		"-E",
		"-p",
		fmt.Sprintf("%v must be run as root. Please enter the password for %%u to continue: ", filepath.Base(os.Args[0])),
		"--",
	}
}
  • 最后在 run 期间添加了一个 disable-sudo 的标志 来表示不需要提权。
runCmd.PersistentFlags().BoolVar(&disableAuthSudo, "disable-sudo", false, "Disable sudo prompt ,may cause startup failure due to insufficient permissions")

Issue Reference

no

Test Result

Sorry, something went wrong.

@woshikedayaa woshikedayaa requested a review from a team as a code owner December 27, 2024 03:55

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
mzz2017
mzz2017 previously approved these changes Dec 27, 2024
@Integral-Tech Integral-Tech added the triage/needs-user-input Issues which are waiting on a response from the reporter label Dec 27, 2024
@mzz2017
Copy link
Contributor

mzz2017 commented Dec 27, 2024

按照操作习惯,是否可以先尝试 sudo?

@st0nie
Copy link
Contributor

st0nie commented Dec 27, 2024

这样的话,如果桌面用户在脚本中使用 dae reload 之类的命令,也会读取到 XDG_CURRENT_DESKTOP 变量。如果用户以前就在脚本中直接使用 dae reload ,需要给所有出现该命令的地方都加上新增的命令行参数。
getopts 确认用户使用的是交互式shell再调用 pkexec 也许更好

@woshikedayaa
Copy link
Contributor Author

woshikedayaa commented Dec 27, 2024

按照操作习惯,是否可以先尝试 sudo?

我开始想开启这个 PR 的观念就是,对于有着 Linux Desktop 环境的用户,调用 pkexec 使用 Polkit 来对桌面端有着更好的集成,同时也可以寄予桌面端用户更好的提示。

@woshikedayaa
Copy link
Contributor Author

woshikedayaa commented Dec 27, 2024

这样的话,如果桌面用户在脚本中使用 dae reload 之类的命令,也会读取到 XDG_CURRENT_DESKTOP 变量。

您好,关于 XDG_CURRENT_DESKTOP 我会在下一个 commit 移除,我想了下并不是所有的使用 polkit 的都使用 有桌面环境。

如果用户以前就在脚本中直接使用 dae reload ,需要给所有出现该命令的地方都加上新增的命令行参数。

只有 run 命令拥有这个参数。并且不影响以前的兼容性
~~ 没人会 echo "password" | sudo .... 吧 ~~

getopts 确认用户使用的是交互式shell再调用 pkexec 也许更好

如果有着pkexec 更好管理,个人理解。并且如果脚本本身已经 sudo 了过后,就不会触发 pkexec

@mzz2017
Copy link
Contributor

mzz2017 commented Dec 27, 2024

我是觉得如果作为兼容性考虑,放到 sudo 后面,作为 fallback 是没问题的,放到前面可能会引起更多 break?

@mzz2017
Copy link
Contributor

mzz2017 commented Dec 27, 2024

另外, dae 作为命令行工具,是否要和桌面集成也是一个问题

@mzz2017 mzz2017 dismissed their stale review December 27, 2024 06:02

有更多意见,需要充分讨论

@Integral-Tech Integral-Tech changed the title faet(init) : enhanched auto-sudo logical feat: enhance privilege elevation logic Dec 27, 2024
@mzz2017
Copy link
Contributor

mzz2017 commented Dec 27, 2024

有人认为优先polkit会导致每次使用都需要输入密码,而sudo至少是免密一段时间。doas或许是个更好的选择。
另外,有一些人讨厌polkit,认为在终端使用dae,跳出一个窗口是很不协调的。

@Integral-Tech
Copy link
Contributor

IMO, the preferred order of precedence is "sudo > doas > polkit".

@woshikedayaa
Copy link
Contributor Author

woshikedayaa commented Dec 28, 2024

现在顺序是 sudo -> doas -> polkit
因为 doas 没有配置 prompt 的 flag(man doas) 就用默认的输出了

加了一条 info 日志来提示使用的提升权限的工具。

另外,提到的冗余代码,这样写是更明显表示是 root ,冗余无所谓。

最后感谢对于 run0 的提醒。

Doas test Conf

# /etc/doas.conf
permit keepenv <USERNAME> as root

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@mzz2017 mzz2017 added the tested label Dec 31, 2024
Copy link
Contributor

@dae-prow dae-prow bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 Since the PR has been fully tested, please consider merging it.

@mzz2017 mzz2017 merged commit b3ff5e6 into daeuniverse:main Dec 31, 2024
29 checks passed
@mzz2017
Copy link
Contributor

mzz2017 commented Dec 31, 2024

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested triage/needs-user-input Issues which are waiting on a response from the reporter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants