diff mbox series

[v2] libselinux: Add security_reject_unknown(3) man page

Message ID 20190305093509.12760-1-plautrba@redhat.com (mailing list archive)
State Superseded
Headers show
Series [v2] libselinux: Add security_reject_unknown(3) man page | expand

Commit Message

Petr Lautrbach March 5, 2019, 9:35 a.m. UTC 
Commit c19395d7 added a new interface security_reject_unknown() which needs to
be documented.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libselinux/man/man3/security_getenforce.3     | 15 ++++++++++++++-
 libselinux/man/man3/security_reject_unknown.3 |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 libselinux/man/man3/security_reject_unknown.3

Comments

Stephen Smalley March 5, 2019, 3:44 p.m. UTC | #1 
On 3/5/19 4:35 AM, Petr Lautrbach wrote:
> Commit c19395d7 added a new interface security_reject_unknown() which needs to
> be documented.

For the kernel, checkpatch.pl requires that one specify at least 12 
characters of the sha1 followed by the one line summary log message 
quoted within parentheses, ala:
commit c19395d72295 ("libselinux: selinux_set_mapping: fix handling of 
unknown classes/perms")

selinux userspace obviously isn't bound by the kernel checkpatch.pl 
requirements but this is probably a good practice to follow so that 
reviewers don't necessarily have to look up the commit hash to have some 
idea as to what the commit was and so that there is less risk of ambiguity.

> 
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> ---
>   libselinux/man/man3/security_getenforce.3     | 15 ++++++++++++++-
>   libselinux/man/man3/security_reject_unknown.3 |  1 +
>   2 files changed, 15 insertions(+), 1 deletion(-)
>   create mode 100644 libselinux/man/man3/security_reject_unknown.3
> 
> diff --git a/libselinux/man/man3/security_getenforce.3 b/libselinux/man/man3/security_getenforce.3
> index 29cf3de7..7b0a069f 100644
> --- a/libselinux/man/man3/security_getenforce.3
> +++ b/libselinux/man/man3/security_getenforce.3
> @@ -1,6 +1,7 @@
>   .TH "security_getenforce" "3" "1 January 2004" "russell@coker.com.au" "SELinux API documentation"
>   .SH "NAME"
> -security_getenforce, security_setenforce, security_deny_unknown, security_get_checkreqprot\- get or set the enforcing state of SELinux
> +security_getenforce, security_setenforce, security_deny_unknown, security_reject_unknown,
> +security_get_checkreqprot \- get or set the enforcing state of SELinux
>   .
>   .SH "SYNOPSIS"
>   .B #include <selinux/selinux.h>
> @@ -11,6 +12,8 @@ security_getenforce, security_setenforce, security_deny_unknown, security_get_ch
>   .sp
>   .B int security_deny_unknown(void);
>   .sp
> +.B int security_reject_unknown(void);
> +.sp
>   .B int security_get_checkreqprot(void);
>   .
>   .SH "DESCRIPTION"
> @@ -27,6 +30,16 @@ returned.
>   returns 0 if SELinux treats policy queries on undefined object classes or
>   permissions as being allowed, 1 if such queries are denied, and \-1 on error.
>   
> +.BR security_reject_unknown ()
> +returns 1 if SELinux rejects loading a policy which doesn't define all kernel
> +object classes and permissions. In this state SELinux treats policy queries on
> +undefined object classes or permissions as being denied.

I'm not sure if the last part is quite correct.  If 
handle_unknown=reject and the policy doesn't define all kernel 
classes/permissions, then the policy load fails, which leaves the system 
without a policy at all (or with its previously loaded policy if one was 
already loaded successfully). If the system is enforcing and this is the 
initial policy load, then init should halt the system due to the failed 
load.  Since no policy was ever loaded, security_reject_unknown() is 
still going to return 0 in that case, but security_deny_unknown() should 
be 1.

If handle_unknown=reject and the policy defines all kernel 
classes/permissions but omits some userspace classes/permissions, then 
the policy load succeeds and the behavior of the userspace object 
managers will vary depending on what interfaces they use and how they 
handle error conditions. If they use selinux_set_mapping() to map all of 
the classes/permissions up front prior to using security_compute_av() or 
avc_has_perm(), then selinux_set_mapping() will return an error and the 
object manager likely treats this as a fatal error during startup (e.g. 
dbus-daemon appears to exit in this case; XSELinux in contrast appears 
to just disable itself).  If they instead use selinux_check_access(), 
then it will return an error and the object manager likely treats this 
like any other permission denial (but errno will differ: EINVAL vs 
EACCES, so they could distinguish if they wanted). If they directly call 
string_to_security_class() and string_to_av_perm() prior to calling 
security_compute_av() or avc_has_perm(), then the string_*() functions 
will return an error on the undefined class/perm and the object manager 
likely treats that like any other permission denial.

> +
> +It returns 0 if SELinux allows to load such policy and policy queries are
> +treated according to
> +.BR security_deny_unknown(),
> +\-1 is returned on error.
> +
>   .BR security_get_checkreqprot ()
>   can be used to determine whether SELinux is configured to check the
>   protection requested by the application or the actual protection that will
> diff --git a/libselinux/man/man3/security_reject_unknown.3 b/libselinux/man/man3/security_reject_unknown.3
> new file mode 100644
> index 00000000..d59e5c2c
> --- /dev/null
> +++ b/libselinux/man/man3/security_reject_unknown.3
> @@ -0,0 +1 @@
> +.so man3/security_getenforce.3
>
Petr Lautrbach March 6, 2019, 12:56 p.m. UTC | #2 
Stephen Smalley <sds@tycho.nsa.gov> writes:

> On 3/5/19 4:35 AM, Petr Lautrbach wrote:
>> Commit c19395d7 added a new interface security_reject_unknown() 
>> which needs to
>> be documented.
>
> For the kernel, checkpatch.pl requires that one specify at least 
> 12 characters
> of the sha1 followed by the one line summary log message quoted 
> within
> parentheses, ala:
> commit c19395d72295 ("libselinux: selinux_set_mapping: fix 
> handling of unknown
> classes/perms")
>
> selinux userspace obviously isn't bound by the kernel 
> checkpatch.pl requirements
> but this is probably a good practice to follow so that reviewers 
> don't
> necessarily have to look up the commit hash to have some idea as 
> to what the
> commit was and so that there is less risk of ambiguity.

This makes sense. I'll try to do better next time.

>>
>> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>> ---
>>   libselinux/man/man3/security_getenforce.3     | 15 
>>   ++++++++++++++-
>>   libselinux/man/man3/security_reject_unknown.3 |  1 +
>>   2 files changed, 15 insertions(+), 1 deletion(-)
>>   create mode 100644 
>>   libselinux/man/man3/security_reject_unknown.3
>>
>> diff --git a/libselinux/man/man3/security_getenforce.3 
>> b/libselinux/man/man3/security_getenforce.3
>> index 29cf3de7..7b0a069f 100644
>> --- a/libselinux/man/man3/security_getenforce.3
>> +++ b/libselinux/man/man3/security_getenforce.3
>> @@ -1,6 +1,7 @@
>>   .TH "security_getenforce" "3" "1 January 2004" 
>>   "russell@coker.com.au" "SELinux API documentation"
>>   .SH "NAME"
>> -security_getenforce, security_setenforce, 
>> security_deny_unknown, security_get_checkreqprot\- get or set 
>> the enforcing state of SELinux
>> +security_getenforce, security_setenforce, 
>> security_deny_unknown, security_reject_unknown,
>> +security_get_checkreqprot \- get or set the enforcing state of 
>> SELinux
>>   .
>>   .SH "SYNOPSIS"
>>   .B #include <selinux/selinux.h>
>> @@ -11,6 +12,8 @@ security_getenforce, security_setenforce, 
>> security_deny_unknown, security_get_ch
>>   .sp
>>   .B int security_deny_unknown(void);
>>   .sp
>> +.B int security_reject_unknown(void);
>> +.sp
>>   .B int security_get_checkreqprot(void);
>>   .
>>   .SH "DESCRIPTION"
>> @@ -27,6 +30,16 @@ returned.
>>   returns 0 if SELinux treats policy queries on undefined 
>>   object classes or
>>   permissions as being allowed, 1 if such queries are denied, 
>>   and \-1 on error.
>>   +.BR security_reject_unknown ()
>> +returns 1 if SELinux rejects loading a policy which doesn't 
>> define all kernel
>> +object classes and permissions. In this state SELinux treats 
>> policy queries on
>> +undefined object classes or permissions as being denied.
>
> I'm not sure if the last part is quite correct.  If 
> handle_unknown=reject and
> the policy doesn't define all kernel classes/permissions, then 
> the policy load
> fails, which leaves the system without a policy at all (or with 
> its previously
> loaded policy if one was already loaded successfully). If the 
> system is
> enforcing and this is the initial policy load, then init should 
> halt the system
> due to the failed load.  Since no policy was ever loaded,
> security_reject_unknown() is still going to return 0 in that 
> case, but
> security_deny_unknown() should be 1.
>
> If handle_unknown=reject and the policy defines all kernel 
> classes/permissions
> but omits some userspace classes/permissions, then the policy 
> load succeeds and
> the behavior of the userspace object managers will vary 
> depending on what
> interfaces they use and how they handle error conditions. If 
> they use
> selinux_set_mapping() to map all of the classes/permissions up 
> front prior to
> using security_compute_av() or avc_has_perm(), then 
> selinux_set_mapping() will
> return an error and the object manager likely treats this as a 
> fatal error
> during startup (e.g. dbus-daemon appears to exit in this case; 
> XSELinux in
> contrast appears to just disable itself).  If they instead use
> selinux_check_access(), then it will return an error and the 
> object manager
> likely treats this like any other permission denial (but errno 
> will differ:
> EINVAL vs EACCES, so they could distinguish if they wanted). If 
> they directly
> call string_to_security_class() and string_to_av_perm() prior to 
> calling
> security_compute_av() or avc_has_perm(), then the string_*() 
> functions will
> return an error on the undefined class/perm and the object 
> manager likely treats
> that like any other permission denial.

There's also an inaccuracy that security_reject_unknown() is
related only to the current loaded policy. Even when a policy is
incomplete it could be still loaded if it's built using
handle-unknown=allow:

# checkpolicy -U reject -M -o /etc/selinux/dummy/policy/policy.31 
  policy.conf.complete 
# load_policy 
# cat /sys/fs/selinux/reject_unknown 
1
#checkpolicy -U reject -M -o /etc/selinux/dummy/policy/policy.31 
 policy.conf
# load_policy 
SELinux:  Could not load policy file 
/etc/selinux/dummy/policy/policy.31:  Invalid argument
load_policy:  Can't load policy:  Invalid argument
# checkpolicy -U allow -M -o /etc/selinux/dummy/policy/policy.31 
  policy.conf
# load_policy 
# cat /sys/fs/selinux/reject_unknown 
0


>> +
>> +It returns 0 if SELinux allows to load such policy and policy 
>> queries are
>> +treated according to
>> +.BR security_deny_unknown(),
>> +\-1 is returned on error.
>> +
>>   .BR security_get_checkreqprot ()
>>   can be used to determine whether SELinux is configured to 
>>   check the
>>   protection requested by the application or the actual 
>>   protection that will
>> diff --git a/libselinux/man/man3/security_reject_unknown.3 
>> b/libselinux/man/man3/security_reject_unknown.3
>> new file mode 100644
>> index 00000000..d59e5c2c
>> --- /dev/null
>> +++ b/libselinux/man/man3/security_reject_unknown.3
>> @@ -0,0 +1 @@
>> +.so man3/security_getenforce.3
>>
diff mbox series

Patch

diff --git a/libselinux/man/man3/security_getenforce.3 b/libselinux/man/man3/security_getenforce.3
index 29cf3de7..7b0a069f 100644
--- a/libselinux/man/man3/security_getenforce.3
+++ b/libselinux/man/man3/security_getenforce.3
@@ -1,6 +1,7 @@ 
 .TH "security_getenforce" "3" "1 January 2004" "russell@coker.com.au" "SELinux API documentation"
 .SH "NAME"
-security_getenforce, security_setenforce, security_deny_unknown, security_get_checkreqprot\- get or set the enforcing state of SELinux
+security_getenforce, security_setenforce, security_deny_unknown, security_reject_unknown,
+security_get_checkreqprot \- get or set the enforcing state of SELinux
 .
 .SH "SYNOPSIS"
 .B #include <selinux/selinux.h>
@@ -11,6 +12,8 @@  security_getenforce, security_setenforce, security_deny_unknown, security_get_ch
 .sp
 .B int security_deny_unknown(void);
 .sp
+.B int security_reject_unknown(void);
+.sp
 .B int security_get_checkreqprot(void);
 .
 .SH "DESCRIPTION"
@@ -27,6 +30,16 @@  returned.
 returns 0 if SELinux treats policy queries on undefined object classes or
 permissions as being allowed, 1 if such queries are denied, and \-1 on error.
 
+.BR security_reject_unknown ()
+returns 1 if SELinux rejects loading a policy which doesn't define all kernel
+object classes and permissions. In this state SELinux treats policy queries on
+undefined object classes or permissions as being denied.
+
+It returns 0 if SELinux allows to load such policy and policy queries are
+treated according to
+.BR security_deny_unknown(),
+\-1 is returned on error.
+
 .BR security_get_checkreqprot ()
 can be used to determine whether SELinux is configured to check the
 protection requested by the application or the actual protection that will
diff --git a/libselinux/man/man3/security_reject_unknown.3 b/libselinux/man/man3/security_reject_unknown.3
new file mode 100644
index 00000000..d59e5c2c
--- /dev/null
+++ b/libselinux/man/man3/security_reject_unknown.3
@@ -0,0 +1 @@ 
+.so man3/security_getenforce.3