fix: allow tty ioctls on Linux v5+ - #310
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where TUI programs failed to operate correctly on Linux systems using Landlock ABI v5+ due to missing Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly implements support for TTY ioctls on Linux with Landlock ABI v5+. The changes are well-structured, introducing new helper functions to encapsulate the logic for granting IoctlDev access. The logic correctly restricts this permission to TTY device paths that have write access, and only when the running kernel's Landlock ABI supports it. The addition of comprehensive unit tests ensures the new functionality is well-covered. The single minor suggestion to improve code conciseness has been retained.
Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com>
b00b1bb to
c0157ae
Compare
Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com>
Replace the hardcoded TARGET_ABI V5 constant with runtime ABI detection that probes V6 down to V1 using HardRequirement. The detected ABI is threaded through sandbox application so access flags are intersected with what the kernel actually supports, with tracing warnings when flags are dropped. This makes enforcement degradation explicit instead of relying on the landlock crate's silent BestEffort flag masking. apply_with_abi() uses HardRequirement for filesystem handle_access(), so passing a forged or stale ABI higher than the kernel supports will fail rather than silently dropping flags. Supersede the IoctlDev fix from #310 (hardcoded TTY path list) with stat()-based device detection: IoctlDev is granted only when the path is an actual char/block device or a directory under /dev, checked at rule-addition time. Remove /dev/pts from system_write_linux in policy.json — /dev/tty (the process's controlling terminal) is sufficient for the base policy. Programs needing PTY allocation can add /dev/pts in their specific profiles. Library changes: - Add DetectedAbi struct with feature query methods (has_refer, has_truncate, has_network, has_ioctl_dev, has_scoping) - Add detect_abi() probing V6..V1 with HardRequirement - Add apply_with_abi() with kernel-validated ABI - Add is_device_path() and is_device_directory() for selective IoctlDev grants - Remove IoctlDev from generic Write flags and #310's is_tty_device_path / access_to_landlock_for_capability - Update is_supported() and support_info() to use detect_abi() CLI changes: - Replace local probe_landlock_abi / probe_landlock_abi_candidate / select_highest_supported_landlock_abi / landlock_feature_lines with library detect_abi() and DetectedAbi - Add ABI info to banner output on Linux (version + features + degraded) - Direct mode uses apply_with_abi() with pre-detected ABI - Remove /dev/pts from system_write_linux base policy Closes #256 #306 Signed-off-by: Luke Hinds <lukehinds@gmail.com>
* feat: ABI-aware Landlock capability system (#256, #306) Replace the hardcoded TARGET_ABI V5 constant with runtime ABI detection that probes V6 down to V1 using HardRequirement. The detected ABI is threaded through sandbox application so access flags are intersected with what the kernel actually supports, with tracing warnings when flags are dropped. This makes enforcement degradation explicit instead of relying on the landlock crate's silent BestEffort flag masking. apply_with_abi() uses HardRequirement for filesystem handle_access(), so passing a forged or stale ABI higher than the kernel supports will fail rather than silently dropping flags. Supersede the IoctlDev fix from #310 (hardcoded TTY path list) with stat()-based device detection: IoctlDev is granted only when the path is an actual char/block device or a directory under /dev, checked at rule-addition time. Remove /dev/pts from system_write_linux in policy.json — /dev/tty (the process's controlling terminal) is sufficient for the base policy. Programs needing PTY allocation can add /dev/pts in their specific profiles. Library changes: - Add DetectedAbi struct with feature query methods (has_refer, has_truncate, has_network, has_ioctl_dev, has_scoping) - Add detect_abi() probing V6..V1 with HardRequirement - Add apply_with_abi() with kernel-validated ABI - Add is_device_path() and is_device_directory() for selective IoctlDev grants - Remove IoctlDev from generic Write flags and #310's is_tty_device_path / access_to_landlock_for_capability - Update is_supported() and support_info() to use detect_abi() CLI changes: - Replace local probe_landlock_abi / probe_landlock_abi_candidate / select_highest_supported_landlock_abi / landlock_feature_lines with library detect_abi() and DetectedAbi - Add ABI info to banner output on Linux (version + features + degraded) - Direct mode uses apply_with_abi() with pre-detected ABI - Remove /dev/pts from system_write_linux base policy Closes #256 #306 Signed-off-by: Luke Hinds <lukehinds@gmail.com> * fix:rebased Landlock ABI follow-up resolving the remaining sandbox/linux merge state and keeping the ioctl narrowing intact. This preserves the ABI-aware enforcement changes, removes the broad /dev/pts write grant, and restores the test/build state expected by CI. Signed-off-by: Luke Hinds <lukehinds@gmail.com> * fix: remove stale imports Signed-off-by: Luke Hinds <lukehinds@gmail.com> --------- Signed-off-by: Luke Hinds <lukehinds@gmail.com>
* feat: ABI-aware Landlock capability system (#256, #306) Replace the hardcoded TARGET_ABI V5 constant with runtime ABI detection that probes V6 down to V1 using HardRequirement. The detected ABI is threaded through sandbox application so access flags are intersected with what the kernel actually supports, with tracing warnings when flags are dropped. This makes enforcement degradation explicit instead of relying on the landlock crate's silent BestEffort flag masking. apply_with_abi() uses HardRequirement for filesystem handle_access(), so passing a forged or stale ABI higher than the kernel supports will fail rather than silently dropping flags. Supersede the IoctlDev fix from #310 (hardcoded TTY path list) with stat()-based device detection: IoctlDev is granted only when the path is an actual char/block device or a directory under /dev, checked at rule-addition time. Remove /dev/pts from system_write_linux in policy.json — /dev/tty (the process's controlling terminal) is sufficient for the base policy. Programs needing PTY allocation can add /dev/pts in their specific profiles. Library changes: - Add DetectedAbi struct with feature query methods (has_refer, has_truncate, has_network, has_ioctl_dev, has_scoping) - Add detect_abi() probing V6..V1 with HardRequirement - Add apply_with_abi() with kernel-validated ABI - Add is_device_path() and is_device_directory() for selective IoctlDev grants - Remove IoctlDev from generic Write flags and #310's is_tty_device_path / access_to_landlock_for_capability - Update is_supported() and support_info() to use detect_abi() CLI changes: - Replace local probe_landlock_abi / probe_landlock_abi_candidate / select_highest_supported_landlock_abi / landlock_feature_lines with library detect_abi() and DetectedAbi - Add ABI info to banner output on Linux (version + features + degraded) - Direct mode uses apply_with_abi() with pre-detected ABI - Remove /dev/pts from system_write_linux base policy Closes #256 #306 Signed-off-by: Luke Hinds <lukehinds@gmail.com> * fix:rebased Landlock ABI follow-up resolving the remaining sandbox/linux merge state and keeping the ioctl narrowing intact. This preserves the ABI-aware enforcement changes, removes the broad /dev/pts write grant, and restores the test/build state expected by CI. Signed-off-by: Luke Hinds <lukehinds@gmail.com> * fix: remove stale imports Signed-off-by: Luke Hinds <lukehinds@gmail.com> --------- Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Fixes #306.
Summary
AccessFs::IoctlDevonly for TTY device capabilities on Linux when the handled Landlock ABI supports itIoctlDevscoped to/dev/ttyand/dev/ptsso non-device read-write paths do not gain ioctl permissionIoctlDevaccess mappingWhy
Landlock ABI v5+ enforces
IoctlDevwhen it is part of the handled filesystem access set.nonoalready handled ABI v5 rights in the ruleset, but never grantedIoctlDevto the TTY device paths that TUI programs use fortcsetattrand raw mode. That causedsetRawModeto fail withEACCES.Validation
Host
Linux harness
Before/after Node repro
These runs use separate
CARGO_TARGET_DIRvalues inside the Linux harness to avoid cross-worktree cache contamination.Unpatched
origin/main:Patched branch:
Linux harness environment
The runtime
nono runpath still logs the hardcoded target ABI, so I probed the highest supported ABI separately by creating hard-requirement Landlock rulesets for descending ABI candidates.Notes
In this Docker environment,
nono setup --check-onlycannot use/sys/kernel/security/lsmbecause that path is not present in the container, so the ABI claim above comes from direct Landlock ruleset probing rather than the setup command.