diff mbox series

[RFC,v4,07/15] landlock: user space API network support

Message ID 20220309134459.6448-8-konstantin.meskhidze@huawei.com (mailing list archive)
State New, archived
Headers show
Series Landlock LSM | expand

Commit Message

Konstantin Meskhidze (A) March 9, 2022, 1:44 p.m. UTC
User space API was refactored to support
network actions. New network access flags,
network rule and network attributes were
added.

Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---

Changes since v3:
* Split commit.
* Refactoring User API for network rule type.

---
 include/uapi/linux/landlock.h | 48 +++++++++++++++++++++++++++++++++++
 security/landlock/syscalls.c  |  5 ++--
 2 files changed, 51 insertions(+), 2 deletions(-)

--
2.25.1

Comments

Mickaël Salaün April 12, 2022, 11:21 a.m. UTC | #1
On 09/03/2022 14:44, Konstantin Meskhidze wrote:
> User space API was refactored to support
> network actions. New network access flags,
> network rule and network attributes were
> added.
> 
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> ---
> 
> Changes since v3:
> * Split commit.
> * Refactoring User API for network rule type.
> 
> ---
>   include/uapi/linux/landlock.h | 48 +++++++++++++++++++++++++++++++++++
>   security/landlock/syscalls.c  |  5 ++--
>   2 files changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index b3d952067f59..4fc6c793fdf4 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -25,6 +25,13 @@ struct landlock_ruleset_attr {
>   	 * compatibility reasons.
>   	 */
>   	__u64 handled_access_fs;
> +
> +	/**
> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
> +	 * that is handled by this ruleset and should then be forbidden if no
> +	 * rule explicitly allow them.
> +	 */
> +	__u64 handled_access_net;
>   };
> 
>   /*
> @@ -46,6 +53,11 @@ enum landlock_rule_type {
>   	 * landlock_path_beneath_attr .
>   	 */
>   	LANDLOCK_RULE_PATH_BENEATH = 1,
> +	/**
> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
> +	 * landlock_net_service_attr .
> +	 */
> +	LANDLOCK_RULE_NET_SERVICE = 2,
>   };
> 
>   /**
> @@ -70,6 +82,24 @@ struct landlock_path_beneath_attr {
>   	 */
>   } __attribute__((packed));
> 
> +/**
> + * struct landlock_net_service_attr - TCP subnet definition
> + *
> + * Argument of sys_landlock_add_rule().
> + */
> +struct landlock_net_service_attr {
> +	/**
> +	 * @allowed_access: Bitmask of allowed access network for services
> +	 * (cf. `Network flags`_).
> +	 */
> +	__u64 allowed_access;
> +	/**
> +	 * @port: Network port
> +	 */
> +	__u16 port;
> +
> +} __attribute__((packed));
> +
>   /**
>    * DOC: fs_access
>    *
> @@ -134,4 +164,22 @@ struct landlock_path_beneath_attr {
>   #define LANDLOCK_ACCESS_FS_MAKE_BLOCK			(1ULL << 11)
>   #define LANDLOCK_ACCESS_FS_MAKE_SYM			(1ULL << 12)
> 
> +/**
> + * DOC: net_access
> + *
> + * Network flags
> + * ~~~~~~~~~~~~~~~~
> + *
> + * These flags enable to restrict a sandboxed process to a set of network
> + * actions.
> + *
> + * TCP sockets with allowed actions:
> + *
> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
> + *   a remote port.
> + */
> +#define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
> +
>   #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 8c0f6165fe3a..fcbce83d64ef 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -81,8 +81,9 @@ static void build_check_abi(void)
>   	 * struct size.
>   	 */
>   	ruleset_size = sizeof(ruleset_attr.handled_access_fs);
> +	ruleset_size += sizeof(ruleset_attr.handled_access_net);
>   	BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
> -	BUILD_BUG_ON(sizeof(ruleset_attr) != 8);
> +	BUILD_BUG_ON(sizeof(ruleset_attr) != 16);
> 
>   	path_beneath_size = sizeof(path_beneath_attr.allowed_access);
>   	path_beneath_size += sizeof(path_beneath_attr.parent_fd);
> @@ -184,7 +185,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
> 
>   	/* Checks content (and 32-bits cast). */
>   	if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
> -			LANDLOCK_MASK_ACCESS_FS)
> +			 LANDLOCK_MASK_ACCESS_FS)

Don't add cosmetic changes. FYI, I'm relying on the way Vim does line 
cuts, which is mostly tabs. Please try to do the same.


>   		return -EINVAL;
>   	access_mask_set.fs = ruleset_attr.handled_access_fs;
> 
> --
> 2.25.1
> 

You need to also update Documentation/userspace-api/landlock.rst 
accordingly. You can check you changes by building the HTML doc.
Mickaël Salaün April 12, 2022, 1:48 p.m. UTC | #2
On 12/04/2022 13:21, Mickaël Salaün wrote:
> 
> On 09/03/2022 14:44, Konstantin Meskhidze wrote:

[...]

>> @@ -184,7 +185,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>>
>>       /* Checks content (and 32-bits cast). */
>>       if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
>> -            LANDLOCK_MASK_ACCESS_FS)
>> +             LANDLOCK_MASK_ACCESS_FS)
> 
> Don't add cosmetic changes. FYI, I'm relying on the way Vim does line 
> cuts, which is mostly tabs. Please try to do the same.

Well, let's make it simple and avoid tacit rules. I'll update most of 
the existing Landlock code and tests to be formatted with clang-format 
(-i *.[ch]), and I'll update the landlock-wip branch so that you can 
base your next patch series on it. There should be some exceptions that 
need customization but we'll see that in the next series. Anyway, don't 
worry too much, just make sure you don't have style-only changes in your 
patches.
Konstantin Meskhidze (A) April 12, 2022, 2:05 p.m. UTC | #3
4/12/2022 4:48 PM, Mickaël Salaün пишет:
> 
> On 12/04/2022 13:21, Mickaël Salaün wrote:
>>
>> On 09/03/2022 14:44, Konstantin Meskhidze wrote:
> 
> [...]
> 
>>> @@ -184,7 +185,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>>>
>>>       /* Checks content (and 32-bits cast). */
>>>       if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
>>> -            LANDLOCK_MASK_ACCESS_FS)
>>> +             LANDLOCK_MASK_ACCESS_FS)
>>
>> Don't add cosmetic changes. FYI, I'm relying on the way Vim does line 
>> cuts, which is mostly tabs. Please try to do the same.
> 
> Well, let's make it simple and avoid tacit rules. I'll update most of 
> the existing Landlock code and tests to be formatted with clang-format 
> (-i *.[ch]), and I'll update the landlock-wip branch so that you can 
> base your next patch series on it. There should be some exceptions that 
> need customization but we'll see that in the next series. Anyway, don't 
> worry too much, just make sure you don't have style-only changes in your 
> patches.

   I have already rebased my next patch series on your landlock-wip 
branch. So I will wait for your changes meanwhile refactoring my v5 
patch series according your comments.

Also I want to discuss adding demo in sandboxer.c to show how landlock
supports network sandboxing:

	- Add additional args like "LL_NET_BIND=port1:...:portN"
	- Add additional args like "LL_NET_CONNECT=port1:...:portN"
	- execv 2 bash procceses:
	    1. first bash listens in loop - $ nc -l -k -p <port1> -v
	    2. second bash to connects the first one - $ nc <ip> <port>

What do you think? its possible to present this demo in the next v5 
patch series.	
	
> .
Mickaël Salaün April 12, 2022, 4:10 p.m. UTC | #4
On 12/04/2022 16:05, Konstantin Meskhidze wrote:
> 
> 
> 4/12/2022 4:48 PM, Mickaël Salaün пишет:
>>
>> On 12/04/2022 13:21, Mickaël Salaün wrote:
>>>
>>> On 09/03/2022 14:44, Konstantin Meskhidze wrote:
>>
>> [...]
>>
>>>> @@ -184,7 +185,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>>>>
>>>>       /* Checks content (and 32-bits cast). */
>>>>       if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
>>>> -            LANDLOCK_MASK_ACCESS_FS)
>>>> +             LANDLOCK_MASK_ACCESS_FS)
>>>
>>> Don't add cosmetic changes. FYI, I'm relying on the way Vim does line 
>>> cuts, which is mostly tabs. Please try to do the same.
>>
>> Well, let's make it simple and avoid tacit rules. I'll update most of 
>> the existing Landlock code and tests to be formatted with clang-format 
>> (-i *.[ch]), and I'll update the landlock-wip branch so that you can 
>> base your next patch series on it. There should be some exceptions 
>> that need customization but we'll see that in the next series. Anyway, 
>> don't worry too much, just make sure you don't have style-only changes 
>> in your patches.
> 
>    I have already rebased my next patch series on your landlock-wip 
> branch. So I will wait for your changes meanwhile refactoring my v5 
> patch series according your comments.

Good.

> 
> Also I want to discuss adding demo in sandboxer.c to show how landlock
> supports network sandboxing:
> 
>      - Add additional args like "LL_NET_BIND=port1:...:portN"
>      - Add additional args like "LL_NET_CONNECT=port1:...:portN"
>      - execv 2 bash procceses:
>          1. first bash listens in loop - $ nc -l -k -p <port1> -v
>          2. second bash to connects the first one - $ nc <ip> <port>
> 
> What do you think? its possible to present this demo in the next v5 
> patch series.

This looks good! I think LL_TCP_BIND and LL_TCP_CONNECT would fit better 
though.

I'm not sure if I already said that, but please remove the "RFC " part 
for the next series.
Konstantin Meskhidze (A) April 25, 2022, 2:29 p.m. UTC | #5
4/12/2022 2:21 PM, Mickaël Salaün пишет:
> 
> On 09/03/2022 14:44, Konstantin Meskhidze wrote:
>> User space API was refactored to support
>> network actions. New network access flags,
>> network rule and network attributes were
>> added.
>>
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>>
>> Changes since v3:
>> * Split commit.
>> * Refactoring User API for network rule type.
>>
>> ---
>>   include/uapi/linux/landlock.h | 48 +++++++++++++++++++++++++++++++++++
>>   security/landlock/syscalls.c  |  5 ++--
>>   2 files changed, 51 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/landlock.h 
>> b/include/uapi/linux/landlock.h
>> index b3d952067f59..4fc6c793fdf4 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -25,6 +25,13 @@ struct landlock_ruleset_attr {
>>        * compatibility reasons.
>>        */
>>       __u64 handled_access_fs;
>> +
>> +    /**
>> +     * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
>> +     * that is handled by this ruleset and should then be forbidden 
>> if no
>> +     * rule explicitly allow them.
>> +     */
>> +    __u64 handled_access_net;
>>   };
>>
>>   /*
>> @@ -46,6 +53,11 @@ enum landlock_rule_type {
>>        * landlock_path_beneath_attr .
>>        */
>>       LANDLOCK_RULE_PATH_BENEATH = 1,
>> +    /**
>> +     * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
>> +     * landlock_net_service_attr .
>> +     */
>> +    LANDLOCK_RULE_NET_SERVICE = 2,
>>   };
>>
>>   /**
>> @@ -70,6 +82,24 @@ struct landlock_path_beneath_attr {
>>        */
>>   } __attribute__((packed));
>>
>> +/**
>> + * struct landlock_net_service_attr - TCP subnet definition
>> + *
>> + * Argument of sys_landlock_add_rule().
>> + */
>> +struct landlock_net_service_attr {
>> +    /**
>> +     * @allowed_access: Bitmask of allowed access network for services
>> +     * (cf. `Network flags`_).
>> +     */
>> +    __u64 allowed_access;
>> +    /**
>> +     * @port: Network port
>> +     */
>> +    __u16 port;
>> +
>> +} __attribute__((packed));
>> +
>>   /**
>>    * DOC: fs_access
>>    *
>> @@ -134,4 +164,22 @@ struct landlock_path_beneath_attr {
>>   #define LANDLOCK_ACCESS_FS_MAKE_BLOCK            (1ULL << 11)
>>   #define LANDLOCK_ACCESS_FS_MAKE_SYM            (1ULL << 12)
>>
>> +/**
>> + * DOC: net_access
>> + *
>> + * Network flags
>> + * ~~~~~~~~~~~~~~~~
>> + *
>> + * These flags enable to restrict a sandboxed process to a set of 
>> network
>> + * actions.
>> + *
>> + * TCP sockets with allowed actions:
>> + *
>> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
>> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
>> + *   a remote port.
>> + */
>> +#define LANDLOCK_ACCESS_NET_BIND_TCP            (1ULL << 0)
>> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP            (1ULL << 1)
>> +
>>   #endif /* _UAPI_LINUX_LANDLOCK_H */
>> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
>> index 8c0f6165fe3a..fcbce83d64ef 100644
>> --- a/security/landlock/syscalls.c
>> +++ b/security/landlock/syscalls.c
>> @@ -81,8 +81,9 @@ static void build_check_abi(void)
>>        * struct size.
>>        */
>>       ruleset_size = sizeof(ruleset_attr.handled_access_fs);
>> +    ruleset_size += sizeof(ruleset_attr.handled_access_net);
>>       BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
>> -    BUILD_BUG_ON(sizeof(ruleset_attr) != 8);
>> +    BUILD_BUG_ON(sizeof(ruleset_attr) != 16);
>>
>>       path_beneath_size = sizeof(path_beneath_attr.allowed_access);
>>       path_beneath_size += sizeof(path_beneath_attr.parent_fd);
>> @@ -184,7 +185,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>>
>>       /* Checks content (and 32-bits cast). */
>>       if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
>> -            LANDLOCK_MASK_ACCESS_FS)
>> +             LANDLOCK_MASK_ACCESS_FS)
> 
> Don't add cosmetic changes. FYI, I'm relying on the way Vim does line 
> cuts, which is mostly tabs. Please try to do the same.
> 
   Ok. I'm using VsCode as an editor. It also could be set up to 
different code styles.
> 
>>           return -EINVAL;
>>       access_mask_set.fs = ruleset_attr.handled_access_fs;
>>
>> -- 
>> 2.25.1
>>
> 
> You need to also update Documentation/userspace-api/landlock.rst 
> accordingly. You can check you changes by building the HTML doc.

   OK. I got it. Thnaks for the comment.
> .
Konstantin Meskhidze (A) April 26, 2022, 10:17 a.m. UTC | #6
4/12/2022 7:10 PM, Mickaël Salaün пишет:
> 
> On 12/04/2022 16:05, Konstantin Meskhidze wrote:
>>
>>
>> 4/12/2022 4:48 PM, Mickaël Salaün пишет:
>>>
>>> On 12/04/2022 13:21, Mickaël Salaün wrote:
>>>>
>>>> On 09/03/2022 14:44, Konstantin Meskhidze wrote:
>>>
>>> [...]
>>>
>>>>> @@ -184,7 +185,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>>>>>
>>>>>       /* Checks content (and 32-bits cast). */
>>>>>       if ((ruleset_attr.handled_access_fs | 
>>>>> LANDLOCK_MASK_ACCESS_FS) !=
>>>>> -            LANDLOCK_MASK_ACCESS_FS)
>>>>> +             LANDLOCK_MASK_ACCESS_FS)
>>>>
>>>> Don't add cosmetic changes. FYI, I'm relying on the way Vim does 
>>>> line cuts, which is mostly tabs. Please try to do the same.
>>>
>>> Well, let's make it simple and avoid tacit rules. I'll update most of 
>>> the existing Landlock code and tests to be formatted with 
>>> clang-format (-i *.[ch]), and I'll update the landlock-wip branch so 
>>> that you can base your next patch series on it. There should be some 
>>> exceptions that need customization but we'll see that in the next 
>>> series. Anyway, don't worry too much, just make sure you don't have 
>>> style-only changes in your patches.
>>
>>    I have already rebased my next patch series on your landlock-wip 
>> branch. So I will wait for your changes meanwhile refactoring my v5 
>> patch series according your comments.
> 
> Good.
> 
>>
>> Also I want to discuss adding demo in sandboxer.c to show how landlock
>> supports network sandboxing:
>>
>>      - Add additional args like "LL_NET_BIND=port1:...:portN"
>>      - Add additional args like "LL_NET_CONNECT=port1:...:portN"
>>      - execv 2 bash procceses:
>>          1. first bash listens in loop - $ nc -l -k -p <port1> -v
>>          2. second bash to connects the first one - $ nc <ip> <port>
>>
>> What do you think? its possible to present this demo in the next v5 
>> patch series.
> 
> This looks good! I think LL_TCP_BIND and LL_TCP_CONNECT would fit better 
> though.
>   Got it. Thanks
> I'm not sure if I already said that, but please remove the "RFC " part 
> for the next series.
   Ok.
> .
diff mbox series

Patch

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index b3d952067f59..4fc6c793fdf4 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -25,6 +25,13 @@  struct landlock_ruleset_attr {
 	 * compatibility reasons.
 	 */
 	__u64 handled_access_fs;
+
+	/**
+	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
+	 * that is handled by this ruleset and should then be forbidden if no
+	 * rule explicitly allow them.
+	 */
+	__u64 handled_access_net;
 };

 /*
@@ -46,6 +53,11 @@  enum landlock_rule_type {
 	 * landlock_path_beneath_attr .
 	 */
 	LANDLOCK_RULE_PATH_BENEATH = 1,
+	/**
+	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
+	 * landlock_net_service_attr .
+	 */
+	LANDLOCK_RULE_NET_SERVICE = 2,
 };

 /**
@@ -70,6 +82,24 @@  struct landlock_path_beneath_attr {
 	 */
 } __attribute__((packed));

+/**
+ * struct landlock_net_service_attr - TCP subnet definition
+ *
+ * Argument of sys_landlock_add_rule().
+ */
+struct landlock_net_service_attr {
+	/**
+	 * @allowed_access: Bitmask of allowed access network for services
+	 * (cf. `Network flags`_).
+	 */
+	__u64 allowed_access;
+	/**
+	 * @port: Network port
+	 */
+	__u16 port;
+
+} __attribute__((packed));
+
 /**
  * DOC: fs_access
  *
@@ -134,4 +164,22 @@  struct landlock_path_beneath_attr {
 #define LANDLOCK_ACCESS_FS_MAKE_BLOCK			(1ULL << 11)
 #define LANDLOCK_ACCESS_FS_MAKE_SYM			(1ULL << 12)

+/**
+ * DOC: net_access
+ *
+ * Network flags
+ * ~~~~~~~~~~~~~~~~
+ *
+ * These flags enable to restrict a sandboxed process to a set of network
+ * actions.
+ *
+ * TCP sockets with allowed actions:
+ *
+ * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
+ * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
+ *   a remote port.
+ */
+#define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
+#define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
+
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 8c0f6165fe3a..fcbce83d64ef 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -81,8 +81,9 @@  static void build_check_abi(void)
 	 * struct size.
 	 */
 	ruleset_size = sizeof(ruleset_attr.handled_access_fs);
+	ruleset_size += sizeof(ruleset_attr.handled_access_net);
 	BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
-	BUILD_BUG_ON(sizeof(ruleset_attr) != 8);
+	BUILD_BUG_ON(sizeof(ruleset_attr) != 16);

 	path_beneath_size = sizeof(path_beneath_attr.allowed_access);
 	path_beneath_size += sizeof(path_beneath_attr.parent_fd);
@@ -184,7 +185,7 @@  SYSCALL_DEFINE3(landlock_create_ruleset,

 	/* Checks content (and 32-bits cast). */
 	if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
-			LANDLOCK_MASK_ACCESS_FS)
+			 LANDLOCK_MASK_ACCESS_FS)
 		return -EINVAL;
 	access_mask_set.fs = ruleset_attr.handled_access_fs;