Infer interface parameter types and check for interface-template mis-declarations - #206
Infer interface parameter types and check for interface-template mis-declarations#206cgzones wants to merge 4 commits into
Conversation
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 <cgzones@googlemail.com>
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
| #include "maps.h" | ||
| #include "util.h" | ||
|
|
||
| /* Uses directly in the testsuite */ |
| } | ||
| } | ||
|
|
||
| static void infer_func(const char *name, enum name_flavor flavor, unsigned short id, void *visitor_data) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| bool is_inferred = true; | ||
| for (int i = 0; i < TRAIT_MAX_PARAMETERS; ++i) { | ||
| if (if_trait->parameters[i] == PARAM_UNKNOWN) { | ||
| is_inferred = false; |
There was a problem hiding this comment.
Looks like a few lines of spaces got in this file (here and line 213 below)
| name = name + 1; | ||
| } | ||
|
|
||
| if (dollar == name && *param_end == '\0') { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 <cgzones@googlemail.com> Signed-off-by: Jason Zaman <perfinion@gentoo.org>
2 participants
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:
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:
Closes: Suggest to change template to interface if appropriate #205