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
Make sure to call connection_is_allowed on xwayland sessions #2835
Conversation
bbd208e
to
7ae41d2
Compare
@@ -80,6 +84,11 @@ auto mf::XWaylandClientManager::session_for_client(pid_t client_pid) -> std::sha | |||
} | |||
else | |||
{ | |||
if (!session_authorizer->connection_is_allowed({client_pid, getuid(), getgid()})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming client_pid
is sensible, there ought to be a way to get gid/uid from that. This seems clunky but...
struct stat proc_stat;
stat(std::format("/proc/{}", client_pid).c_str(), &proc_stat);
auto const uid = proc_stat.st_uid;
auto const gid = proc_stat.st_gid;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was considering the exact same thing, so i'll try to add this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented this, but did not use std::format as that require gcc 13++
As xwayland/x11 has no way of telling us gid uid we assume it's the same as mir itself.
7ae41d2
to
fb0a05e
Compare
stat(proc.c_str(), &proc_stat); | ||
auto const uid = proc_stat.st_uid; | ||
auto const gid = proc_stat.st_gid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably needs some error handling to avoid potentially reading uninitialized data
if (!local_client_session) { | ||
rejected = true; | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!local_client_session) { | |
rejected = true; | |
return; | |
} | |
if (!local_client_session) | |
{ | |
rejected = true; | |
return; | |
} |
if (rejected) { | ||
scene_surface_close_requested(); | ||
close(); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (rejected) { | |
scene_surface_close_requested(); | |
close(); | |
return; | |
} | |
if (rejected) | |
{ | |
scene_surface_close_requested(); | |
close(); | |
return; | |
} |
@mariogrip feel free to resubmit when the build errors, etc are addressed. |
As xwayland/x11 has no way of telling us gid uid so we get this by stat() its proc path
Note i return a nullptr if session is not allowed and then closes the xwayland session, but im not sure if this is the best way of doing this.