-
Notifications
You must be signed in to change notification settings - Fork 0
landlock: Implement scope control for pathname Unix sockets #20
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
base: landlock.scopednamedsock-base
Are you sure you want to change the base?
landlock: Implement scope control for pathname Unix sockets #20
Conversation
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.
Pull request overview
This PR extends Landlock's scope control mechanism to support pathname (filesystem path) Unix sockets in addition to the existing abstract Unix socket scoping. The changes introduce a new scope flag LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET and update the kernel's Landlock ABI to version 8.
Key changes:
- Added
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKETflag for restricting pathname Unix socket connections - Modified security hooks (
hook_unix_stream_connectandhook_unix_may_send) to differentiate between abstract and pathname sockets - Extended test suite with comprehensive pathname socket test coverage alongside existing abstract socket tests
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| security/landlock/task.c | Implements pathname socket scope checking in Unix socket connection hooks |
| include/uapi/linux/landlock.h | Adds LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET flag definition |
| security/landlock/syscalls.c | Updates ABI version from 7 to 8 |
| security/landlock/limits.h | Updates LANDLOCK_LAST_SCOPE to include pathname socket scope |
| security/landlock/audit.h | Adds LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET request type |
| security/landlock/audit.c | Adds audit blocker string for pathname socket scope |
| tools/testing/selftests/landlock/scoped_unix_test.c | Extends tests to cover both abstract and pathname socket variants |
| tools/testing/selftests/landlock/common.h | Updates set_unix_address() to support both abstract and pathname sockets |
| tools/testing/selftests/landlock/scoped_test.c | Updates ACCESS_LAST to include pathname socket scope |
| tools/testing/selftests/landlock/base_test.c | Updates expected ABI version to 8 |
| tools/testing/selftests/landlock/scoped_signal_test.c | Updates set_unix_address() call to specify abstract socket |
| tools/testing/selftests/landlock/net_test.c | Updates set_unix_address() call to specify abstract socket |
| tools/testing/selftests/landlock/fs_test.c | Fixes typo in comment ("assume" → "assumed") |
| tools/testing/selftests/landlock/ptrace_test.c | Refactors to use shared scoped_base_variants.h |
| tools/testing/selftests/landlock/scoped_base_variants.h | Updates comment documentation |
| samples/landlock/sandboxer.c | Adds "u" option for pathname socket scoping and updates for ABI 8 |
| Documentation/userspace-api/landlock.rst | Adds documentation for LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET |
Comments suppressed due to low confidence (1)
tools/testing/selftests/landlock/scoped_unix_test.c:473
- This assertion checks log_match_remaining after the stpncpy call, but log_match_remaining was never updated to reflect the remaining space after the copy operation (unlike the pathname branch at lines 493-494). The assertion should either be removed, or log_match_remaining should be updated after the stpncpy call for consistency with the pathname branch.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
include/uapi/linux/landlock.h
Outdated
| */ | ||
| /* clang-format off */ | ||
| #define LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET (1ULL << 0) | ||
| #define LANDLOCK_SCOPE_SIGNAL (1ULL << 1) |
Copilot
AI
Dec 27, 2025
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.
Inconsistent whitespace formatting in macro definitions. Line 374 uses spaces while lines 373 and 375 use tabs. This should be consistent - all three lines should use the same whitespace character (tabs) for alignment.
| #define LANDLOCK_SCOPE_SIGNAL (1ULL << 1) | |
| #define LANDLOCK_SCOPE_SIGNAL (1ULL << 1) |
include/uapi/linux/landlock.h
Outdated
| #define LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET (1ULL << 0) | ||
| #define LANDLOCK_SCOPE_SIGNAL (1ULL << 1) | ||
| #define LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (1ULL << 2) | ||
| /* clang-format on*/ |
Copilot
AI
Dec 27, 2025
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.
Missing space before the closing comment marker. Should be "/* clang-format on /" instead of "/ clang-format on*/".
| /* clang-format on*/ | |
| /* clang-format on */ |
1dccf3b to
096a208
Compare
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
tools/testing/selftests/landlock/scoped_unix_test.c:1313
- Type mismatch:
scopeis declared asconst __u64butcreate_scoped_domain()expectsconst __u16. While the values fit within 16 bits, this creates a type inconsistency. Change__u64to__u16for consistency with the function signature and other uses in this file (lines 100-101, 206-207).
tools/testing/selftests/landlock/scoped_unix_test.c:1410 - Type mismatch:
scopeis declared asconst __u64butcreate_scoped_domain()expectsconst __u16. While the values fit within 16 bits, this creates a type inconsistency. Change__u64to__u16for consistency with the function signature and other uses in this file (lines 100-101, 206-207).
tools/testing/selftests/landlock/scoped_unix_test.c:600 - Type mismatch:
scopeis declared asconst __u64butcreate_scoped_domain()expectsconst __u16. While the values fit within 16 bits, this creates a type inconsistency. Change__u64to__u16for consistency with the function signature and other uses in this file (lines 100-101, 206-207).
tools/testing/selftests/landlock/scoped_unix_test.c:890 - Type mismatch:
scopeis declared asconst __u64butcreate_scoped_domain()expectsconst __u16. While the values fit within 16 bits, this creates a type inconsistency. Change__u64to__u16for consistency with the function signature and other uses in this file (lines 100-101, 206-207).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
096a208 to
f8af697
Compare
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tools/testing/selftests/landlock/scoped_unix_test.c:1410
- The scope variable is declared as
__u64but should be__u16to match the type used consistently throughout the rest of this file (lines 100, 206, 599, 888, 1311) and to match the scope parameter type expected by create_scoped_domain.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
176b698 to
813acde
Compare
813acde to
9c05537
Compare
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0994a10 to
860fa4d
Compare
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
tools/testing/selftests/landlock/scoped_unix_test.c:1441
- The scope variable is declared as __u64 but should be __u16 to match the type used consistently throughout the rest of the file (lines 110, 216, 608, 895, 1342) and the actual Landlock scope flag definitions which are 16-bit values.
tools/testing/selftests/landlock/scoped_unix_test.c:514 - The comparison is incorrect. The regex_escape function returns a char pointer that may be cast from -ENOMEM on error (see audit.h line 225). The comparison should check if the pointer represents an error, for example by comparing against a negative value cast to a pointer or by using a macro. The current comparison treats the pointer as an integer which is likely to cause a compiler warning and may not work correctly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
68f0b27 to
2b2761b
Compare
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
tools/testing/selftests/landlock/scoped_unix_test.c:488
- The comment states "It is assumed that absolute_path does not contain control characters nor spaces" but there's a typo in the original comment at line 487-489. The phrase should be "It is assumed" not "It is assume". However, I notice this same typo appears to exist in fs_test.c where it's being fixed. This comment in scoped_unix_test.c should also use "assumed" for consistency.
tools/testing/selftests/landlock/scoped_unix_test.c:737 - Fixed: Added missing semicolon at the end of the close statement.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| This allows sandboxing applications using only Landlock to protect against | ||
| bypasses relying on connecting to Unix sockets of other services running | ||
| under the same user. These services typically assumes that any process |
Copilot
AI
Dec 28, 2025
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.
In the comment on line 633, there's a grammatical error. The phrase "Unix sockets of other services running under the same user" should use "that" instead of "These" in the next sentence. The sentence "These services typically assumes" should be "These services typically assume" (plural verb form to match plural subject).
| under the same user. These services typically assumes that any process | |
| under the same user that typically assume that any process |
| ``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET`` applies to abstract sockets, | ||
| and ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` applies to pathname | ||
| sockets. Even though pathname sockets are represented in the | ||
| filesystem, Landlock filesystem rules does not currently control |
Copilot
AI
Dec 28, 2025
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.
In the documentation at line 349, there's a grammatical error. The phrase "Landlock filesystem rules does not currently control" should use the plural verb form "do not" instead of "does not".
| filesystem, Landlock filesystem rules does not currently control | |
| filesystem, Landlock filesystem rules do not currently control |
2b2761b to
33985b3
Compare
38fddf5 to
5718e0f
Compare
161db18 to
ef4536f
Compare
5718e0f to
9517619
Compare
9517619 to
5d8dbb3
Compare
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.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
tools/testing/selftests/landlock/scoped_unix_test.c:511
- The comparison should check for NULL (error condition from regex_escape), not for a value less than 0. The function returns a char pointer, and error is indicated by returning (char *)-ENOMEM, which when cast to intptr_t would be a large positive value, not negative. The correct check should be: if (!log_match_cursor || log_match_cursor == (char *)-ENOMEM)
tools/testing/selftests/landlock/scoped_unix_test.c:62 - The mkdir call will fail with EEXIST if the directory already exists from a previous test run or another test running concurrently. Consider using mkdir with error handling that ignores EEXIST, or ensuring the directory is cleaned up between test runs. This could cause test flakiness.
tools/testing/selftests/landlock/scoped_unix_test.c:380 - The mkdir call will fail with EEXIST if the directory already exists from a previous test run or another test running concurrently. Consider using mkdir with error handling that ignores EEXIST, or ensuring the directory is cleaned up between test runs. This could cause test flakiness.
tools/testing/selftests/landlock/scoped_unix_test.c:553 - The mkdir call will fail with EEXIST if the directory already exists from a previous test run or another test running concurrently. Consider using mkdir with error handling that ignores EEXIST, or ensuring the directory is cleaned up between test runs. This could cause test flakiness.
tools/testing/selftests/landlock/scoped_unix_test.c:516 - The variable log_match_cursor is being reused after being assigned from regex_escape() without checking the return value first. If regex_escape() returns an error ((char *)-ENOMEM), this line will use that error pointer value in the calculation, leading to incorrect results. The error check on line 511 should come before this line.
tools/testing/selftests/landlock/scoped_unix_test.c:860 - The mkdir call will fail with EEXIST if the directory already exists from a previous test run or another test running concurrently. Consider using mkdir with error handling that ignores EEXIST, or ensuring the directory is cleaned up between test runs. This could cause test flakiness.
tools/testing/selftests/landlock/scoped_unix_test.c:1045 - The mkdir call will fail with EEXIST if the directory already exists from a previous test run or another test running concurrently. Consider using mkdir with error handling that ignores EEXIST, or ensuring the directory is cleaned up between test runs. This could cause test flakiness.
tools/testing/selftests/landlock/scoped_unix_test.c:1318 - The mkdir call will fail with EEXIST if the directory already exists from a previous test run or another test running concurrently. Consider using mkdir with error handling that ignores EEXIST, or ensuring the directory is cleaned up between test runs. This could cause test flakiness.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| This allows sandboxing applications using only Landlock to protect against | ||
| bypasses relying on connecting to Unix sockets of other services running | ||
| under the same user. These services typically assumes that any process |
Copilot
AI
Dec 30, 2025
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.
Grammatical error: "assumes" should be "assume" to match the plural subject "These services".
| under the same user. These services typically assumes that any process | |
| under the same user. These services typically assume that any process |
5d8dbb3 to
6b69163
Compare
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.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| This allows sandboxing applications using only Landlock to protect against | ||
| bypasses relying on connecting to Unix sockets of other services running | ||
| under the same user. These services typically assume that any process |
Copilot
AI
Dec 30, 2025
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.
Grammar error: "These services typically assumes" should be "These services typically assume" (plural subject requires plural verb form).
| /* Removes LANDLOCK_SCOPE_* for ABI < 6 */ | ||
| ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | | ||
| LANDLOCK_SCOPE_SIGNAL); | ||
| __attribute__((fallthrough)); |
Copilot
AI
Dec 30, 2025
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.
Missing case statement for ABI version 6. The switch statement jumps from case 5 to case 7, skipping case 6. This should include a case 6 to handle Landlock ABI version 6, even if no features need to be removed for that version. The pattern should maintain sequential case statements with fallthrough for clarity and correctness.
| __attribute__((fallthrough)); | |
| __attribute__((fallthrough)); | |
| case 6: | |
| /* No features removed for ABI < 7 */ | |
| __attribute__((fallthrough)); |
6b69163 to
a05f163
Compare
Add the new scope bit to the uAPI header, add documentation, and bump ABI version to 8. This documentation edit specifically calls out the security implications of not restricting sockets. Fix some minor cosmetic issue in landlock.h around the changed lines as well. Signed-off-by: Tingmao Wang <m@maowtm.org> --- Changes in v2: - Fix grammar Note that in the code block in "Defining and enforcing a security policy" the switch case currently jumps from 5 to 7. This should be fixed by https://lore.kernel.org/all/20251216210248.4150777-1-samasth.norway.ananda@oracle.com/
a05f163 to
293ef45
Compare
Extend the existing abstract UNIX socket scoping to pathname sockets as well. Basically all of the logic is reused between the two types, just that pathname sockets scoping are controlled by another bit, and has its own audit request type (since the current one is named "abstract_unix_socket"). Closes: landlock-lsm#51 Signed-off-by: Tingmao Wang <m@maowtm.org> --- Changes in v2: - Factor out common code in hook_unix_stream_connect and hook_unix_may_send into check_socket_access(), and inline is_abstract_socket().
293ef45 to
88de5be
Compare
Signed-off-by: Tingmao Wang <m@maowtm.org> --- I've decided to use "u" as the character to control this scope bit since it stands for (normal) Unix sockets. Imo using "p" or "n" would make it less clear / memorable. Open to suggestions. Also, open to suggestion whether socket scoping (pathname and abstract) should be enabled by default, if LL_SCOPED is not set. This would break backward compatibility, but maybe we shouldn't guarentee backward compatibility of this sandboxer in the first place, and almost all cases of Landlock usage would want socket scoping.
To prepare for extending the socket tests to do non-abstract sockets too, extend set_unix_address() to also be able to populate a non-abstract socket path under TMP_DIR. Also use snprintf for good measure. This also changes existing callers to pass true for the abstract argument. Signed-off-by: Tingmao Wang <m@maowtm.org>
…e sockets too. Since there is very little difference between abstract and pathname sockets in terms of testing of the scoped access checks (the only difference is in which scope bit control which form of socket), it makes sense to reuse the existing test for both type of sockets. Therefore, we rename scoped_abstract_unix_test.c to scoped_unix_test.c and extend the scoped_domains test to test pathname (i.e. non-abstract) sockets too. Since we can't change the variant data of scoped_domains (as it is defined in the shared .h file), we do this by extracting the actual test code into a function, and call it from different test cases. Also extend scoped_audit (this time we can use variants) to test both abstract and pathname sockets. For pathname sockets, audit_log_lsm_data will produce path="..." (or hex if path contains control characters) with absolute paths from the dentry, so we need to construct the escaped regex for the real path like in fs_test. Signed-off-by: Tingmao Wang <m@maowtm.org>
While this produces a lot of change, it does allow us to "simultaneously" test both abstract and pathname UNIX sockets with reletively little code duplication, since they are really similar. Tests touched: scoped_vs_unscoped, outside_socket, various_address_sockets, datagram_sockets, self_connect. Signed-off-by: Tingmao Wang <m@maowtm.org>
No description provided.