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

profiles: blacklist i3 IPC socket & dir except for i3 itself #6361

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

smheidrich
Copy link
Contributor

@smheidrich smheidrich commented May 26, 2024

This closes the escape route discussed in #6357.

It's left open for i3's own profile, so that people who run i3 itself sandboxed still have the option to use IPC with it at all.

Reference for file paths: https://i3wm.org/docs/userguide.html#_interprocess_communication

Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR.

@@ -167,6 +167,10 @@ blacklist ${RUNUSER}/gnome-session-leader-fifo
blacklist ${RUNUSER}/gnome-shell
blacklist ${RUNUSER}/gsconnect

# i3 IPC socket (allows arbitrary shell script execution)
blacklist ${RUNUSER}/i3/ipc-socket.*
blacklist /tmp/i3-*/ipc-socket.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there anything else in the i3 directory?

XY: Would it make sense to blacklist the directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my system, the only other files in there are an error log and another UNIX domain socket called log-stream-socket.*. Seems neither very dangerous nor very useful, so it probably doesn't matter either way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Logs could contain sensitive information.

If there is thing important in it we should blacklist the directory IMHO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW, I agree. Targetting the directory is the better option here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, I changed it to block the entire directories.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Files that allow command execution are usually made read-only or blacklisted.

Unnecessary directories are usually blacklisted.

So why not both?

I think it would be helpful to have the actual socket paths documented and
having specific entries for them would be one way to do it.

I restored the socket paths, moved the directories to disable-programs.inc,
added the description to the commit message and force-pushed.

Thoughts?

@kmk3 kmk3 changed the title Blacklist i3 IPC socket except for i3 itself profiles: blacklist i3 IPC socket except for i3 itself May 27, 2024
Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the changes.

This closes the escape route discussed in netblue30#6357.

It's left open for i3's own profile, so that people who run i3 itself
sandboxed still have the option to use IPC with it at all.

Reference for file paths:
https://i3wm.org/docs/userguide.html#_interprocess_communication
@kmk3 kmk3 force-pushed the blacklist-i3-ipc-socket branch from 0af8540 to 87317dd Compare May 29, 2024 20:21
@kmk3 kmk3 changed the title profiles: blacklist i3 IPC socket except for i3 itself profiles: blacklist i3 IPC socket & dir except for i3 itself May 29, 2024
@kmk3 kmk3 added this to In progress in Release 0.9.74 via automation May 29, 2024
Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

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

LGTM

@glitsj16 glitsj16 merged commit 533db20 into netblue30:master Jun 8, 2024
3 checks passed
kmk3 added a commit that referenced this pull request Jun 10, 2024
@kmk3 kmk3 moved this from In progress to Done (on RELNOTES) in Release 0.9.74 Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Release 0.9.74
  
Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

None yet

4 participants