Infer interface parameter types and check for interface-template mis-declarations #206

cgzones · 2021-05-10T13:00:12Z

Infer parameter types of interfaces and templates

Infer the type of parameters of templates and interfaces.
This can help find mis-declarations, see next commit.

In the future this can be used for further checks, e.g. find
discrepancies between the kind of a parameter and its documentation.

Add checks for template/interface mis-declarations

Interfaces in the refpolicy should not:

declare anything (no side effects)
use prefix parameters

Add one check to find interfaces that should be declared as a template
and one check to find templates that can be declared as an interface.

Refpolicy findings:

qemu.if:            112: (S): Template qemu_role might be declared as an interface (S-012)
wm.if:              142: (S): Interface wm_dbus_chat should be a template, due to parameter 0 (S-011)
wm.if:              250: (S): Interface wm_write_pipes should be a template, due to parameter 0 (S-011)
gnome.if:           673: (S): Interface gnome_dbus_chat_gkeyringd should be a template, due to parameter 0 (S-011)
gnome.if:           741: (S): Interface gnome_stream_connect_gkeyringd should be a template, due to parameter 0 (S-011)
userdomain.if:     1431: (S): Template userdom_security_admin_template might be declared as an interface (S-012)
kismet.if:           18: (S): Template kismet_role might be declared as an interface (S-012)
dbus.if:            193: (S): Interface dbus_connect_spec_session_bus should be a template, due to parameter 0 (S-011)
dbus.if:            245: (S): Interface dbus_spec_session_bus_client should be a template, due to parameter 0 (S-011)
dbus.if:            298: (S): Interface dbus_send_spec_session_bus should be a template, due to parameter 0 (S-011)
dbus.if:            436: (S): Interface dbus_spec_session_domain should be a template, due to parameter 0 (S-011)
rlogin.if:           32: (S): Template rlogin_read_home_content might be declared as an interface (S-012)
git.if:              18: (S): Template git_role might be declared as an interface (S-012)
Found the following issue counts:
S-011: 8
S-012: 5

Closes: Suggest to change template to interface if appropriate #205

Infer the type of parameters of templates and interfaces. This can help find mis-declarations, see next commit. In the future this can be used for further checks, e.g. find discrepancies between the kind of a parameter and its documentation.

Following the guideline of interfaces not allowed to declare anything and not use prefix parameters, declare interfaces doing so as templates. Also declare templates not using those features and not calling templates themselves as interfaces. These changes originate from the discussion in SELinuxProject/selint#205 and are found by new proposed SELint checks at SELinuxProject/selint#206. Signed-off-by: Christian Göttsche <[email protected]>

Interfaces in the refpolicy should not: - declare anything (no side effects) - use prefix parameters Add one check to find interfaces that should be declared as a template and one check to find templates that can be declared as an interface. Refpolicy findings: qemu.if: 112: (S): Template qemu_role might be declared as an interface (S-012) wm.if: 142: (S): Interface wm_dbus_chat should be a template, due to parameter 0 (S-011) wm.if: 250: (S): Interface wm_write_pipes should be a template, due to parameter 0 (S-011) gnome.if: 673: (S): Interface gnome_dbus_chat_gkeyringd should be a template, due to parameter 0 (S-011) gnome.if: 741: (S): Interface gnome_stream_connect_gkeyringd should be a template, due to parameter 0 (S-011) userdomain.if: 1431: (S): Template userdom_security_admin_template might be declared as an interface (S-012) kismet.if: 18: (S): Template kismet_role might be declared as an interface (S-012) dbus.if: 193: (S): Interface dbus_connect_spec_session_bus should be a template, due to parameter 0 (S-011) dbus.if: 245: (S): Interface dbus_spec_session_bus_client should be a template, due to parameter 0 (S-011) dbus.if: 298: (S): Interface dbus_send_spec_session_bus should be a template, due to parameter 0 (S-011) dbus.if: 436: (S): Interface dbus_spec_session_domain should be a template, due to parameter 0 (S-011) rlogin.if: 32: (S): Template rlogin_read_home_content might be declared as an interface (S-012) git.if: 18: (S): Template git_role might be declared as an interface (S-012) Found the following issue counts: S-011: 8 S-012: 5 Closes: SELinuxProject#205

dburgener · 2021-07-03T21:55:29Z

src/infer.c

+#include "maps.h"
+#include "util.h"
+
+/* Uses directly in the testsuite */


typo, uses -> used

dburgener · 2021-07-03T22:03:54Z

src/infer.c

+ }
+}
+
+static void infer_func(const char *name, enum name_flavor flavor, unsigned short id, void *visitor_data)


Can this (and infer_interface) get a comment explaining their usage? It's not clear to me from the names, and it's slow progress trying to infer from the implementation.

Okay, reading more, I think I have a rough idea of what these are both for based on how they're used. I still would like to have something with the functions to help orient anyone reading the code.

dburgener · 2021-07-03T22:13:19Z

src/infer.c

+ bool is_inferred = true;
+ for (int i = 0; i < TRAIT_MAX_PARAMETERS; ++i) {
+ if (if_trait->parameters[i] == PARAM_UNKNOWN) {
+ is_inferred = false;


Looks like a few lines of spaces got in this file (here and line 213 below)

dburgener · 2021-07-03T22:24:45Z

src/infer.c

+ name = name + 1;
+ }
+
+ if (dollar == name && *param_end == '\0') {


I may very well be missing something here, but I'm concerned about the situation where a parameter is incorrectly used in a way that would infer as two different parameters. Something like:

allow $1 foo_t:file read;
role $1 types foo_t;

This would be a compile error, but I think the logic here would end up setting the flavor of $1 to PARAM_ROLE, since the second line just overwrites the first? It seems like it would be valuable info to have to know if a parameter was being used inconsistently.

Then again, I could definitely be missing something, so feel free to let me know if this scenario is already handled.

True, a subsequent usage of the same no yet final inferred parameter will completely override the current type.
Probably the inferred state shouldn't be a type of an enum but a bit-mask where its possible to store the information a parameter is used as a type-or-attribute, a role and a template string-part. So analysis checks can then create reports like "Parameter X has conflicting usages", "Interface foo uses templated parameter X (declare as template)".

[skip ci]

Following the guideline of interfaces not allowed to declare anything and not use prefix parameters, declare interfaces doing so as templates. Also declare templates not using those features and not calling templates themselves as interfaces. These changes originate from the discussion in SELinuxProject/selint#205 and are found by new proposed SELint checks at SELinuxProject/selint#206. Signed-off-by: Christian Göttsche <[email protected]>

Following the guideline of interfaces not allowed to declare anything and not use prefix parameters, declare interfaces doing so as templates. Also declare templates not using those features and not calling templates themselves as interfaces. These changes originate from the discussion in SELinuxProject/selint#205 and are found by new proposed SELint checks at SELinuxProject/selint#206. Signed-off-by: Christian Göttsche <[email protected]> Signed-off-by: Jason Zaman <[email protected]>

cgzones force-pushed the inferring branch from 32666b6 to 392069b Compare May 10, 2021 13:09

cgzones mentioned this pull request May 13, 2021

Use correct interface or template declaration SELinuxProject/refpolicy#375

Merged

cgzones force-pushed the inferring branch from 392069b to 9bebc76 Compare May 27, 2021 11:04

typos

2120a5e

dburgener reviewed Jul 3, 2021

View reviewed changes

Test infer conflicting usage

3910c5c

[skip ci]

cgzones marked this pull request as draft November 30, 2021 16:42

cgzones mentioned this pull request Sep 20, 2022

RFC: Expand checks about requirements to userspace classes #250

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Infer interface parameter types and check for interface-template mis-declarations #206

Infer interface parameter types and check for interface-template mis-declarations #206

cgzones commented May 10, 2021

dburgener Jul 3, 2021

dburgener Jul 3, 2021

dburgener Jul 3, 2021

dburgener Jul 3, 2021

dburgener Jul 3, 2021

cgzones Jul 5, 2021

Infer interface parameter types and check for interface-template mis-declarations #206

Are you sure you want to change the base?

Infer interface parameter types and check for interface-template mis-declarations #206

Conversation

cgzones commented May 10, 2021

dburgener Jul 3, 2021

Choose a reason for hiding this comment

dburgener Jul 3, 2021

Choose a reason for hiding this comment

dburgener Jul 3, 2021

Choose a reason for hiding this comment

dburgener Jul 3, 2021

Choose a reason for hiding this comment

dburgener Jul 3, 2021

Choose a reason for hiding this comment

cgzones Jul 5, 2021

Choose a reason for hiding this comment