diff mbox series

[v7,16/18] seltests/landlock: add invalid input data test

Message ID 20220829170401.834298-17-konstantin.meskhidze@huawei.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Network support for Landlock | expand

Commit Message

Konstantin Meskhidze (A) Aug. 29, 2022, 5:03 p.m. UTC
This patch adds rules with invalid user space supplied data:
    - out of range ruleset attribute;
    - unhandled allowed access;
    - zero port value;
    - zero access value;

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

Changes since v6:
* Adds invalid ruleset attribute test.

Changes since v5:
* Formats code with clang-format-14.

Changes since v4:
* Refactors code with self->port variable.

Changes since v3:
* Adds inval test.

---
 tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

--
2.25.1

Comments

Mickaël Salaün Sept. 6, 2022, 8:09 a.m. UTC | #1
On 29/08/2022 19:03, Konstantin Meskhidze wrote:
> This patch adds rules with invalid user space supplied data:
>      - out of range ruleset attribute;
>      - unhandled allowed access;
>      - zero port value;
>      - zero access value;
> 
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> ---
> 
> Changes since v6:
> * Adds invalid ruleset attribute test.
> 
> Changes since v5:
> * Formats code with clang-format-14.
> 
> Changes since v4:
> * Refactors code with self->port variable.
> 
> Changes since v3:
> * Adds inval test.
> 
> ---
>   tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
>   1 file changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index a93224d1521b..067ba45f58a5 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -26,9 +26,12 @@
> 
>   #define IP_ADDRESS "127.0.0.1"
> 
> -/* Number pending connections queue to be hold */
> +/* Number pending connections queue to be hold. */

Patch of a previous patch?


>   #define BACKLOG 10
> 
> +/* Invalid attribute, out of landlock network access range. */
> +#define LANDLOCK_INVAL_ATTR 7
> +
>   FIXTURE(socket)
>   {
>   	uint port[MAX_SOCKET_NUM];
> @@ -719,4 +722,65 @@ TEST_F(socket, ruleset_expanding)
>   	/* Closes socket 1. */
>   	ASSERT_EQ(0, close(sockfd_1));
>   }
> +
> +TEST_F(socket, inval)
> +{
> +	struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
> +	};
> +	struct landlock_ruleset_attr ruleset_attr_inval = {
> +		.handled_access_net = LANDLOCK_INVAL_ATTR

Please add a test similar to TEST_F_FORK(layout1, 
file_and_dir_access_rights) instead of explicitly defining and only 
testing LANDLOCK_INVAL_ATTR.


> +	};
> +	struct landlock_net_service_attr net_service_1 = {
> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
> +				  LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +		.port = self->port[0],
> +	};
> +	struct landlock_net_service_attr net_service_2 = {
> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
> +		.port = 0,
> +	};
> +	struct landlock_net_service_attr net_service_3 = {
> +		.allowed_access = 0,
> +		.port = self->port[1],
> +	};
> +	struct landlock_net_service_attr net_service_4 = {
> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
> +		.port = self->port[2],
> +	};
> +
> +	/* Checks invalid ruleset attribute. */
> +	const int ruleset_fd_inv = landlock_create_ruleset(
> +		&ruleset_attr_inval, sizeof(ruleset_attr_inval), 0);
> +	ASSERT_EQ(-1, ruleset_fd_inv);
> +	ASSERT_EQ(EINVAL, errno);
> +
> +	/* Gets ruleset. */
> +	const int ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	/* Checks unhandled allowed_access. */
> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +					&net_service_1, 0));
> +	ASSERT_EQ(EINVAL, errno);
> +
> +	/* Checks zero port value. */
> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +					&net_service_2, 0));
> +	ASSERT_EQ(EINVAL, errno);
> +
> +	/* Checks zero access value. */
> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +					&net_service_3, 0));
> +	ASSERT_EQ(ENOMSG, errno);
> +
> +	/* Adds with legitimate values. */
> +	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +				       &net_service_4, 0));
> +
> +	/* Enforces the ruleset. */
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +}
>   TEST_HARNESS_MAIN
> --
> 2.25.1
>
Konstantin Meskhidze (A) Sept. 10, 2022, 8:51 p.m. UTC | #2
9/6/2022 11:09 AM, Mickaël Salaün пишет:
> 
> On 29/08/2022 19:03, Konstantin Meskhidze wrote:
>> This patch adds rules with invalid user space supplied data:
>>      - out of range ruleset attribute;
>>      - unhandled allowed access;
>>      - zero port value;
>>      - zero access value;
>> 
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>> 
>> Changes since v6:
>> * Adds invalid ruleset attribute test.
>> 
>> Changes since v5:
>> * Formats code with clang-format-14.
>> 
>> Changes since v4:
>> * Refactors code with self->port variable.
>> 
>> Changes since v3:
>> * Adds inval test.
>> 
>> ---
>>   tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
>>   1 file changed, 65 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
>> index a93224d1521b..067ba45f58a5 100644
>> --- a/tools/testing/selftests/landlock/net_test.c
>> +++ b/tools/testing/selftests/landlock/net_test.c
>> @@ -26,9 +26,12 @@
>> 
>>   #define IP_ADDRESS "127.0.0.1"
>> 
>> -/* Number pending connections queue to be hold */
>> +/* Number pending connections queue to be hold. */
> 
> Patch of a previous patch?
> 
> 
>>   #define BACKLOG 10
>> 
>> +/* Invalid attribute, out of landlock network access range. */
>> +#define LANDLOCK_INVAL_ATTR 7
>> +
>>   FIXTURE(socket)
>>   {
>>   	uint port[MAX_SOCKET_NUM];
>> @@ -719,4 +722,65 @@ TEST_F(socket, ruleset_expanding)
>>   	/* Closes socket 1. */
>>   	ASSERT_EQ(0, close(sockfd_1));
>>   }
>> +
>> +TEST_F(socket, inval)
>> +{
>> +	struct landlock_ruleset_attr ruleset_attr = {
>> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
>> +	};
>> +	struct landlock_ruleset_attr ruleset_attr_inval = {
>> +		.handled_access_net = LANDLOCK_INVAL_ATTR
> 
> Please add a test similar to TEST_F_FORK(layout1,
> file_and_dir_access_rights) instead of explicitly defining and only
> testing LANDLOCK_INVAL_ATTR.
> 
   Do you want fs test to be in this commit or maybe its better to add 
it into "[PATCH v7 01/18] landlock: rename access mask" one.
> 
>> +	};
>> +	struct landlock_net_service_attr net_service_1 = {
>> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
>> +				  LANDLOCK_ACCESS_NET_CONNECT_TCP,
>> +		.port = self->port[0],
>> +	};
>> +	struct landlock_net_service_attr net_service_2 = {
>> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
>> +		.port = 0,
>> +	};
>> +	struct landlock_net_service_attr net_service_3 = {
>> +		.allowed_access = 0,
>> +		.port = self->port[1],
>> +	};
>> +	struct landlock_net_service_attr net_service_4 = {
>> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
>> +		.port = self->port[2],
>> +	};
>> +
>> +	/* Checks invalid ruleset attribute. */
>> +	const int ruleset_fd_inv = landlock_create_ruleset(
>> +		&ruleset_attr_inval, sizeof(ruleset_attr_inval), 0);
>> +	ASSERT_EQ(-1, ruleset_fd_inv);
>> +	ASSERT_EQ(EINVAL, errno);
>> +
>> +	/* Gets ruleset. */
>> +	const int ruleset_fd =
>> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
>> +	ASSERT_LE(0, ruleset_fd);
>> +
>> +	/* Checks unhandled allowed_access. */
>> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> +					&net_service_1, 0));
>> +	ASSERT_EQ(EINVAL, errno);
>> +
>> +	/* Checks zero port value. */
>> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> +					&net_service_2, 0));
>> +	ASSERT_EQ(EINVAL, errno);
>> +
>> +	/* Checks zero access value. */
>> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> +					&net_service_3, 0));
>> +	ASSERT_EQ(ENOMSG, errno);
>> +
>> +	/* Adds with legitimate values. */
>> +	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> +				       &net_service_4, 0));
>> +
>> +	/* Enforces the ruleset. */
>> +	enforce_ruleset(_metadata, ruleset_fd);
>> +	ASSERT_EQ(0, close(ruleset_fd));
>> +}
>>   TEST_HARNESS_MAIN
>> --
>> 2.25.1
>> 
> .
Mickaël Salaün Sept. 12, 2022, 5:22 p.m. UTC | #3
On 10/09/2022 22:51, Konstantin Meskhidze (A) wrote:
> 
> 
> 9/6/2022 11:09 AM, Mickaël Salaün пишет:
>>
>> On 29/08/2022 19:03, Konstantin Meskhidze wrote:
>>> This patch adds rules with invalid user space supplied data:
>>>       - out of range ruleset attribute;
>>>       - unhandled allowed access;
>>>       - zero port value;
>>>       - zero access value;
>>>
>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>> ---
>>>
>>> Changes since v6:
>>> * Adds invalid ruleset attribute test.
>>>
>>> Changes since v5:
>>> * Formats code with clang-format-14.
>>>
>>> Changes since v4:
>>> * Refactors code with self->port variable.
>>>
>>> Changes since v3:
>>> * Adds inval test.
>>>
>>> ---
>>>    tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
>>>    1 file changed, 65 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
>>> index a93224d1521b..067ba45f58a5 100644
>>> --- a/tools/testing/selftests/landlock/net_test.c
>>> +++ b/tools/testing/selftests/landlock/net_test.c
>>> @@ -26,9 +26,12 @@
>>>
>>>    #define IP_ADDRESS "127.0.0.1"
>>>
>>> -/* Number pending connections queue to be hold */
>>> +/* Number pending connections queue to be hold. */
>>
>> Patch of a previous patch?
>>
>>
>>>    #define BACKLOG 10
>>>
>>> +/* Invalid attribute, out of landlock network access range. */
>>> +#define LANDLOCK_INVAL_ATTR 7
>>> +
>>>    FIXTURE(socket)
>>>    {
>>>    	uint port[MAX_SOCKET_NUM];
>>> @@ -719,4 +722,65 @@ TEST_F(socket, ruleset_expanding)
>>>    	/* Closes socket 1. */
>>>    	ASSERT_EQ(0, close(sockfd_1));
>>>    }
>>> +
>>> +TEST_F(socket, inval)
>>> +{
>>> +	struct landlock_ruleset_attr ruleset_attr = {
>>> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
>>> +	};
>>> +	struct landlock_ruleset_attr ruleset_attr_inval = {
>>> +		.handled_access_net = LANDLOCK_INVAL_ATTR
>>
>> Please add a test similar to TEST_F_FORK(layout1,
>> file_and_dir_access_rights) instead of explicitly defining and only
>> testing LANDLOCK_INVAL_ATTR.
>>
>     Do you want fs test to be in this commit or maybe its better to add
> it into "[PATCH v7 01/18] landlock: rename access mask" one.

You can squash all the new tests patches (except the "move helper 
function").
Mickaël Salaün Oct. 10, 2022, 10:37 a.m. UTC | #4
On 12/09/2022 19:22, Mickaël Salaün wrote:
> 
> On 10/09/2022 22:51, Konstantin Meskhidze (A) wrote:
>>
>>
>> 9/6/2022 11:09 AM, Mickaël Salaün пишет:
>>>
>>> On 29/08/2022 19:03, Konstantin Meskhidze wrote:
>>>> This patch adds rules with invalid user space supplied data:
>>>>        - out of range ruleset attribute;
>>>>        - unhandled allowed access;
>>>>        - zero port value;
>>>>        - zero access value;
>>>>
>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>> ---
>>>>
>>>> Changes since v6:
>>>> * Adds invalid ruleset attribute test.
>>>>
>>>> Changes since v5:
>>>> * Formats code with clang-format-14.
>>>>
>>>> Changes since v4:
>>>> * Refactors code with self->port variable.
>>>>
>>>> Changes since v3:
>>>> * Adds inval test.
>>>>
>>>> ---
>>>>     tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
>>>>     1 file changed, 65 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
>>>> index a93224d1521b..067ba45f58a5 100644
>>>> --- a/tools/testing/selftests/landlock/net_test.c
>>>> +++ b/tools/testing/selftests/landlock/net_test.c
>>>> @@ -26,9 +26,12 @@
>>>>
>>>>     #define IP_ADDRESS "127.0.0.1"
>>>>
>>>> -/* Number pending connections queue to be hold */
>>>> +/* Number pending connections queue to be hold. */
>>>
>>> Patch of a previous patch?
>>>
>>>
>>>>     #define BACKLOG 10
>>>>
>>>> +/* Invalid attribute, out of landlock network access range. */
>>>> +#define LANDLOCK_INVAL_ATTR 7
>>>> +
>>>>     FIXTURE(socket)
>>>>     {
>>>>     	uint port[MAX_SOCKET_NUM];
>>>> @@ -719,4 +722,65 @@ TEST_F(socket, ruleset_expanding)
>>>>     	/* Closes socket 1. */
>>>>     	ASSERT_EQ(0, close(sockfd_1));
>>>>     }
>>>> +
>>>> +TEST_F(socket, inval)
>>>> +{
>>>> +	struct landlock_ruleset_attr ruleset_attr = {
>>>> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
>>>> +	};
>>>> +	struct landlock_ruleset_attr ruleset_attr_inval = {
>>>> +		.handled_access_net = LANDLOCK_INVAL_ATTR
>>>
>>> Please add a test similar to TEST_F_FORK(layout1,
>>> file_and_dir_access_rights) instead of explicitly defining and only
>>> testing LANDLOCK_INVAL_ATTR.
>>>
>>      Do you want fs test to be in this commit or maybe its better to add
>> it into "[PATCH v7 01/18] landlock: rename access mask" one.

Just to make it clear, I didn't suggested an FS test, but a new network 
test similar to layout1.file_and_dir_access_rights but only related to 
the network. It should replace/extend the content of this patch (16/18).


> 
> You can squash all the new tests patches (except the "move helper
> function").
You should move most of your patch descriptions in a comment above the 
related tests. The commit message should list all the new tests and 
quickly explain which part of the kernel is covered (i.e. mostly the TCP 
part of Landlock). You can get some inspiration from 
https://git.kernel.org/mic/c/f4056b9266b571c63f30cda70c2d89f7b7e8bb7b

You need to rebase on top of my next branch (from today).
Konstantin Meskhidze (A) Oct. 11, 2022, 7:55 a.m. UTC | #5
10/10/2022 1:37 PM, Mickaël Salaün пишет:
> 
> On 12/09/2022 19:22, Mickaël Salaün wrote:
>> 
>> On 10/09/2022 22:51, Konstantin Meskhidze (A) wrote:
>>>
>>>
>>> 9/6/2022 11:09 AM, Mickaël Salaün пишет:
>>>>
>>>> On 29/08/2022 19:03, Konstantin Meskhidze wrote:
>>>>> This patch adds rules with invalid user space supplied data:
>>>>>        - out of range ruleset attribute;
>>>>>        - unhandled allowed access;
>>>>>        - zero port value;
>>>>>        - zero access value;
>>>>>
>>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>>> ---
>>>>>
>>>>> Changes since v6:
>>>>> * Adds invalid ruleset attribute test.
>>>>>
>>>>> Changes since v5:
>>>>> * Formats code with clang-format-14.
>>>>>
>>>>> Changes since v4:
>>>>> * Refactors code with self->port variable.
>>>>>
>>>>> Changes since v3:
>>>>> * Adds inval test.
>>>>>
>>>>> ---
>>>>>     tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
>>>>>     1 file changed, 65 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
>>>>> index a93224d1521b..067ba45f58a5 100644
>>>>> --- a/tools/testing/selftests/landlock/net_test.c
>>>>> +++ b/tools/testing/selftests/landlock/net_test.c
>>>>> @@ -26,9 +26,12 @@
>>>>>
>>>>>     #define IP_ADDRESS "127.0.0.1"
>>>>>
>>>>> -/* Number pending connections queue to be hold */
>>>>> +/* Number pending connections queue to be hold. */
>>>>
>>>> Patch of a previous patch?
>>>>
>>>>
>>>>>     #define BACKLOG 10
>>>>>
>>>>> +/* Invalid attribute, out of landlock network access range. */
>>>>> +#define LANDLOCK_INVAL_ATTR 7
>>>>> +
>>>>>     FIXTURE(socket)
>>>>>     {
>>>>>     	uint port[MAX_SOCKET_NUM];
>>>>> @@ -719,4 +722,65 @@ TEST_F(socket, ruleset_expanding)
>>>>>     	/* Closes socket 1. */
>>>>>     	ASSERT_EQ(0, close(sockfd_1));
>>>>>     }
>>>>> +
>>>>> +TEST_F(socket, inval)
>>>>> +{
>>>>> +	struct landlock_ruleset_attr ruleset_attr = {
>>>>> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
>>>>> +	};
>>>>> +	struct landlock_ruleset_attr ruleset_attr_inval = {
>>>>> +		.handled_access_net = LANDLOCK_INVAL_ATTR
>>>>
>>>> Please add a test similar to TEST_F_FORK(layout1,
>>>> file_and_dir_access_rights) instead of explicitly defining and only
>>>> testing LANDLOCK_INVAL_ATTR.
>>>>
>>>      Do you want fs test to be in this commit or maybe its better to add
>>> it into "[PATCH v7 01/18] landlock: rename access mask" one.
> 
> Just to make it clear, I didn't suggested an FS test, but a new network
> test similar to layout1.file_and_dir_access_rights but only related to
> the network. It should replace/extend the content of this patch (16/18).
> 
  Ok. I will check out out "layout1.file_and_dir_access_rights" one.
But anyway we need some test like TEST_F_FORK(layout1, with_net) and
TEST_F_FORK(socket, with_fs) with mixed attributes as you suggested.

> 
>> 
>> You can squash all the new tests patches (except the "move helper
>> function").
> You should move most of your patch descriptions in a comment above the
> related tests. The commit message should list all the new tests and
> quickly explain which part of the kernel is covered (i.e. mostly the TCP
> part of Landlock). You can get some inspiration from
> https://git.kernel.org/mic/c/f4056b9266b571c63f30cda70c2d89f7b7e8bb7b
> 
> You need to rebase on top of my next branch (from today).

  Ok. Thank you. Sorry for the delay - I was under the snow with 
Business Trip to China preparings. So I've got out of 10 days quarantine 
now and continued working.
> .
Mickaël Salaün Oct. 11, 2022, 8:32 a.m. UTC | #6
On 11/10/2022 09:55, Konstantin Meskhidze (A) wrote:
> 
> 
> 10/10/2022 1:37 PM, Mickaël Salaün пишет:
>>
>> On 12/09/2022 19:22, Mickaël Salaün wrote:
>>>
>>> On 10/09/2022 22:51, Konstantin Meskhidze (A) wrote:
>>>>
>>>>
>>>> 9/6/2022 11:09 AM, Mickaël Salaün пишет:
>>>>>
>>>>> On 29/08/2022 19:03, Konstantin Meskhidze wrote:
>>>>>> This patch adds rules with invalid user space supplied data:
>>>>>>         - out of range ruleset attribute;
>>>>>>         - unhandled allowed access;
>>>>>>         - zero port value;
>>>>>>         - zero access value;
>>>>>>
>>>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>>>> ---
>>>>>>
>>>>>> Changes since v6:
>>>>>> * Adds invalid ruleset attribute test.
>>>>>>
>>>>>> Changes since v5:
>>>>>> * Formats code with clang-format-14.
>>>>>>
>>>>>> Changes since v4:
>>>>>> * Refactors code with self->port variable.
>>>>>>
>>>>>> Changes since v3:
>>>>>> * Adds inval test.
>>>>>>
>>>>>> ---
>>>>>>      tools/testing/selftests/landlock/net_test.c | 66 ++++++++++++++++++++-
>>>>>>      1 file changed, 65 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
>>>>>> index a93224d1521b..067ba45f58a5 100644
>>>>>> --- a/tools/testing/selftests/landlock/net_test.c
>>>>>> +++ b/tools/testing/selftests/landlock/net_test.c
>>>>>> @@ -26,9 +26,12 @@
>>>>>>
>>>>>>      #define IP_ADDRESS "127.0.0.1"
>>>>>>
>>>>>> -/* Number pending connections queue to be hold */
>>>>>> +/* Number pending connections queue to be hold. */
>>>>>
>>>>> Patch of a previous patch?
>>>>>
>>>>>
>>>>>>      #define BACKLOG 10
>>>>>>
>>>>>> +/* Invalid attribute, out of landlock network access range. */
>>>>>> +#define LANDLOCK_INVAL_ATTR 7
>>>>>> +
>>>>>>      FIXTURE(socket)
>>>>>>      {
>>>>>>      	uint port[MAX_SOCKET_NUM];
>>>>>> @@ -719,4 +722,65 @@ TEST_F(socket, ruleset_expanding)
>>>>>>      	/* Closes socket 1. */
>>>>>>      	ASSERT_EQ(0, close(sockfd_1));
>>>>>>      }
>>>>>> +
>>>>>> +TEST_F(socket, inval)
>>>>>> +{
>>>>>> +	struct landlock_ruleset_attr ruleset_attr = {
>>>>>> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
>>>>>> +	};
>>>>>> +	struct landlock_ruleset_attr ruleset_attr_inval = {
>>>>>> +		.handled_access_net = LANDLOCK_INVAL_ATTR
>>>>>
>>>>> Please add a test similar to TEST_F_FORK(layout1,
>>>>> file_and_dir_access_rights) instead of explicitly defining and only
>>>>> testing LANDLOCK_INVAL_ATTR.
>>>>>
>>>>       Do you want fs test to be in this commit or maybe its better to add
>>>> it into "[PATCH v7 01/18] landlock: rename access mask" one.
>>
>> Just to make it clear, I didn't suggested an FS test, but a new network
>> test similar to layout1.file_and_dir_access_rights but only related to
>> the network. It should replace/extend the content of this patch (16/18).
>>
>    Ok. I will check out out "layout1.file_and_dir_access_rights" one.
> But anyway we need some test like TEST_F_FORK(layout1, with_net) and
> TEST_F_FORK(socket, with_fs) with mixed attributes as you suggested.

Right, you can add that to the main test patch.
diff mbox series

Patch

diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index a93224d1521b..067ba45f58a5 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -26,9 +26,12 @@ 

 #define IP_ADDRESS "127.0.0.1"

-/* Number pending connections queue to be hold */
+/* Number pending connections queue to be hold. */
 #define BACKLOG 10

+/* Invalid attribute, out of landlock network access range. */
+#define LANDLOCK_INVAL_ATTR 7
+
 FIXTURE(socket)
 {
 	uint port[MAX_SOCKET_NUM];
@@ -719,4 +722,65 @@  TEST_F(socket, ruleset_expanding)
 	/* Closes socket 1. */
 	ASSERT_EQ(0, close(sockfd_1));
 }
+
+TEST_F(socket, inval)
+{
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP
+	};
+	struct landlock_ruleset_attr ruleset_attr_inval = {
+		.handled_access_net = LANDLOCK_INVAL_ATTR
+	};
+	struct landlock_net_service_attr net_service_1 = {
+		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
+				  LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		.port = self->port[0],
+	};
+	struct landlock_net_service_attr net_service_2 = {
+		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
+		.port = 0,
+	};
+	struct landlock_net_service_attr net_service_3 = {
+		.allowed_access = 0,
+		.port = self->port[1],
+	};
+	struct landlock_net_service_attr net_service_4 = {
+		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
+		.port = self->port[2],
+	};
+
+	/* Checks invalid ruleset attribute. */
+	const int ruleset_fd_inv = landlock_create_ruleset(
+		&ruleset_attr_inval, sizeof(ruleset_attr_inval), 0);
+	ASSERT_EQ(-1, ruleset_fd_inv);
+	ASSERT_EQ(EINVAL, errno);
+
+	/* Gets ruleset. */
+	const int ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Checks unhandled allowed_access. */
+	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
+					&net_service_1, 0));
+	ASSERT_EQ(EINVAL, errno);
+
+	/* Checks zero port value. */
+	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
+					&net_service_2, 0));
+	ASSERT_EQ(EINVAL, errno);
+
+	/* Checks zero access value. */
+	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
+					&net_service_3, 0));
+	ASSERT_EQ(ENOMSG, errno);
+
+	/* Adds with legitimate values. */
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
+				       &net_service_4, 0));
+
+	/* Enforces the ruleset. */
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+}
 TEST_HARNESS_MAIN