diff mbox series

[v3,4/4] selinux: log invalid contexts in AVCs

Message ID 20190125100651.21753-5-omosnace@redhat.com (mailing list archive)
State Accepted
Headers show
Series Report raw context in AVCs + refactoring | expand

Commit Message

Ondrej Mosnacek Jan. 25, 2019, 10:06 a.m. UTC
In case a file has an invalid context set, in an AVC record generated
upon access to such file, the target context is always reported as
unlabeled. This patch adds new optional fields to the AVC record
(srawcon and trawcon) that report the actual context string if it
differs from the one reported in scontext/tcontext. This is useful for
diagnosing SELinux denials involving invalid contexts.

To trigger an AVC that illustrates this situation:

    # setenforce 0
    # touch /tmp/testfile
    # setfattr -n security.selinux -v system_u:object_r:banana_t:s0 /tmp/testfile
    # runcon system_u:system_r:sshd_t:s0 cat /tmp/testfile

AVC before:

type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1

AVC after:

type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1 trawcon=system_u:object_r:banana_t:s0

Note that it is also possible to encounter this situation with the
'scontext' field - e.g. when a new policy is loaded while a process is
running, whose context is not valid in the new policy.

Cc: Daniel Walsh <dwalsh@redhat.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1135683
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 security/selinux/avc.c              | 15 ++++++++++++
 security/selinux/include/security.h |  3 +++
 security/selinux/ss/services.c      | 37 +++++++++++++++++++++++++----
 3 files changed, 50 insertions(+), 5 deletions(-)

Comments

Stephen Smalley Jan. 25, 2019, 2:56 p.m. UTC | #1
On 1/25/19 5:06 AM, Ondrej Mosnacek wrote:
> In case a file has an invalid context set, in an AVC record generated
> upon access to such file, the target context is always reported as
> unlabeled. This patch adds new optional fields to the AVC record
> (srawcon and trawcon) that report the actual context string if it
> differs from the one reported in scontext/tcontext. This is useful for
> diagnosing SELinux denials involving invalid contexts.
> 
> To trigger an AVC that illustrates this situation:
> 
>      # setenforce 0
>      # touch /tmp/testfile
>      # setfattr -n security.selinux -v system_u:object_r:banana_t:s0 /tmp/testfile
>      # runcon system_u:system_r:sshd_t:s0 cat /tmp/testfile
> 
> AVC before:
> 
> type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1
> 
> AVC after:
> 
> type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1 trawcon=system_u:object_r:banana_t:s0
> 
> Note that it is also possible to encounter this situation with the
> 'scontext' field - e.g. when a new policy is loaded while a process is
> running, whose context is not valid in the new policy.
> 
> Cc: Daniel Walsh <dwalsh@redhat.com>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1135683
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>   security/selinux/avc.c              | 15 ++++++++++++
>   security/selinux/include/security.h |  3 +++
>   security/selinux/ss/services.c      | 37 +++++++++++++++++++++++++----
>   3 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 478fa4213c25..047de65589bd 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -734,6 +734,21 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
>   
>   	if (sad->denied)
>   		audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
> +
> +	/* in case of invalid context report also the actual context string */
> +	rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext,
> +					   &scontext_len);
> +	if (!rc && scontext) {
> +		audit_log_format(ab, " srawcon=%s", scontext);
> +		kfree(scontext);
> +	}
> +
> +	rc = security_sid_to_context_inval(sad->state, sad->tsid, &scontext,
> +					   &scontext_len);
> +	if (!rc && scontext) {
> +		audit_log_format(ab, " trawcon=%s", scontext);
> +		kfree(scontext);
> +	}
>   }
>   
>   /* This is the slow part of avc audit with big stack footprint */
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index ba8eedf42b90..f68fb25b5702 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -255,6 +255,9 @@ int security_sid_to_context(struct selinux_state *state, u32 sid,
>   int security_sid_to_context_force(struct selinux_state *state,
>   				  u32 sid, char **scontext, u32 *scontext_len);
>   
> +int security_sid_to_context_inval(struct selinux_state *state,
> +				  u32 sid, char **scontext, u32 *scontext_len);
> +
>   int security_context_to_sid(struct selinux_state *state,
>   			    const char *scontext, u32 scontext_len,
>   			    u32 *out_sid, gfp_t gfp);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index dd44126c8d14..9be05c3e99dc 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1281,7 +1281,8 @@ const char *security_get_initial_sid_context(u32 sid)
>   
>   static int security_sid_to_context_core(struct selinux_state *state,
>   					u32 sid, char **scontext,
> -					u32 *scontext_len, int force)
> +					u32 *scontext_len, int force,
> +					int only_invalid)
>   {
>   	struct policydb *policydb;
>   	struct sidtab *sidtab;
> @@ -1326,8 +1327,14 @@ static int security_sid_to_context_core(struct selinux_state *state,
>   		rc = -EINVAL;
>   		goto out_unlock;
>   	}
> -	rc = context_struct_to_string(policydb, context, scontext,
> -				      scontext_len);
> +	if (only_invalid && !context->len) {
> +		scontext = NULL;
> +		scontext_len = 0;
> +		rc = 0;
> +	} else {
> +		rc = context_struct_to_string(policydb, context, scontext,
> +					      scontext_len);
> +	}
>   out_unlock:
>   	read_unlock(&state->ss->policy_rwlock);
>   out:
> @@ -1349,14 +1356,34 @@ int security_sid_to_context(struct selinux_state *state,
>   			    u32 sid, char **scontext, u32 *scontext_len)
>   {
>   	return security_sid_to_context_core(state, sid, scontext,
> -					    scontext_len, 0);
> +					    scontext_len, 0, 0);
>   }
>   
>   int security_sid_to_context_force(struct selinux_state *state, u32 sid,
>   				  char **scontext, u32 *scontext_len)
>   {
>   	return security_sid_to_context_core(state, sid, scontext,
> -					    scontext_len, 1);
> +					    scontext_len, 1, 0);
> +}
> +
> +/**
> + * security_sid_to_context_inval - Obtain a context for a given SID if it
> + *                                 is invalid.
> + * @sid: security identifier, SID
> + * @scontext: security context
> + * @scontext_len: length in bytes
> + *
> + * Write the string representation of the context associated with @sid
> + * into a dynamically allocated string of the correct size, but only if the
> + * context is invalid in the current policy.  Set @scontext to point to
> + * this string (or NULL if the context is valid) and set @scontext_len to
> + * the length of the string (or 0 if the context is valid).
> + */
> +int security_sid_to_context_inval(struct selinux_state *state, u32 sid,
> +				  char **scontext, u32 *scontext_len)
> +{
> +	return security_sid_to_context_core(state, sid, scontext,
> +					    scontext_len, 1, 1);
>   }
>   
>   /*
>
Paul Moore Jan. 25, 2019, 10:35 p.m. UTC | #2
On Fri, Jan 25, 2019 at 5:07 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> In case a file has an invalid context set, in an AVC record generated
> upon access to such file, the target context is always reported as
> unlabeled. This patch adds new optional fields to the AVC record
> (srawcon and trawcon) that report the actual context string if it
> differs from the one reported in scontext/tcontext. This is useful for
> diagnosing SELinux denials involving invalid contexts.
>
> To trigger an AVC that illustrates this situation:
>
>     # setenforce 0
>     # touch /tmp/testfile
>     # setfattr -n security.selinux -v system_u:object_r:banana_t:s0 /tmp/testfile
>     # runcon system_u:system_r:sshd_t:s0 cat /tmp/testfile
>
> AVC before:
>
> type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1
>
> AVC after:
>
> type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1 trawcon=system_u:object_r:banana_t:s0
>
> Note that it is also possible to encounter this situation with the
> 'scontext' field - e.g. when a new policy is loaded while a process is
> running, whose context is not valid in the new policy.
>
> Cc: Daniel Walsh <dwalsh@redhat.com>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1135683
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  security/selinux/avc.c              | 15 ++++++++++++
>  security/selinux/include/security.h |  3 +++
>  security/selinux/ss/services.c      | 37 +++++++++++++++++++++++++----
>  3 files changed, 50 insertions(+), 5 deletions(-)

Merged, thanks.

> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 478fa4213c25..047de65589bd 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -734,6 +734,21 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
>
>         if (sad->denied)
>                 audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
> +
> +       /* in case of invalid context report also the actual context string */
> +       rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext,
> +                                          &scontext_len);
> +       if (!rc && scontext) {
> +               audit_log_format(ab, " srawcon=%s", scontext);
> +               kfree(scontext);
> +       }
> +
> +       rc = security_sid_to_context_inval(sad->state, sad->tsid, &scontext,
> +                                          &scontext_len);
> +       if (!rc && scontext) {
> +               audit_log_format(ab, " trawcon=%s", scontext);
> +               kfree(scontext);
> +       }
>  }
>
>  /* This is the slow part of avc audit with big stack footprint */
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index ba8eedf42b90..f68fb25b5702 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -255,6 +255,9 @@ int security_sid_to_context(struct selinux_state *state, u32 sid,
>  int security_sid_to_context_force(struct selinux_state *state,
>                                   u32 sid, char **scontext, u32 *scontext_len);
>
> +int security_sid_to_context_inval(struct selinux_state *state,
> +                                 u32 sid, char **scontext, u32 *scontext_len);
> +
>  int security_context_to_sid(struct selinux_state *state,
>                             const char *scontext, u32 scontext_len,
>                             u32 *out_sid, gfp_t gfp);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index dd44126c8d14..9be05c3e99dc 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1281,7 +1281,8 @@ const char *security_get_initial_sid_context(u32 sid)
>
>  static int security_sid_to_context_core(struct selinux_state *state,
>                                         u32 sid, char **scontext,
> -                                       u32 *scontext_len, int force)
> +                                       u32 *scontext_len, int force,
> +                                       int only_invalid)
>  {
>         struct policydb *policydb;
>         struct sidtab *sidtab;
> @@ -1326,8 +1327,14 @@ static int security_sid_to_context_core(struct selinux_state *state,
>                 rc = -EINVAL;
>                 goto out_unlock;
>         }
> -       rc = context_struct_to_string(policydb, context, scontext,
> -                                     scontext_len);
> +       if (only_invalid && !context->len) {
> +               scontext = NULL;
> +               scontext_len = 0;
> +               rc = 0;
> +       } else {
> +               rc = context_struct_to_string(policydb, context, scontext,
> +                                             scontext_len);
> +       }
>  out_unlock:
>         read_unlock(&state->ss->policy_rwlock);
>  out:
> @@ -1349,14 +1356,34 @@ int security_sid_to_context(struct selinux_state *state,
>                             u32 sid, char **scontext, u32 *scontext_len)
>  {
>         return security_sid_to_context_core(state, sid, scontext,
> -                                           scontext_len, 0);
> +                                           scontext_len, 0, 0);
>  }
>
>  int security_sid_to_context_force(struct selinux_state *state, u32 sid,
>                                   char **scontext, u32 *scontext_len)
>  {
>         return security_sid_to_context_core(state, sid, scontext,
> -                                           scontext_len, 1);
> +                                           scontext_len, 1, 0);
> +}
> +
> +/**
> + * security_sid_to_context_inval - Obtain a context for a given SID if it
> + *                                 is invalid.
> + * @sid: security identifier, SID
> + * @scontext: security context
> + * @scontext_len: length in bytes
> + *
> + * Write the string representation of the context associated with @sid
> + * into a dynamically allocated string of the correct size, but only if the
> + * context is invalid in the current policy.  Set @scontext to point to
> + * this string (or NULL if the context is valid) and set @scontext_len to
> + * the length of the string (or 0 if the context is valid).
> + */
> +int security_sid_to_context_inval(struct selinux_state *state, u32 sid,
> +                                 char **scontext, u32 *scontext_len)
> +{
> +       return security_sid_to_context_core(state, sid, scontext,
> +                                           scontext_len, 1, 1);
>  }
>
>  /*
> --
> 2.20.1
>
diff mbox series

Patch

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 478fa4213c25..047de65589bd 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -734,6 +734,21 @@  static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
 
 	if (sad->denied)
 		audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
+
+	/* in case of invalid context report also the actual context string */
+	rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext,
+					   &scontext_len);
+	if (!rc && scontext) {
+		audit_log_format(ab, " srawcon=%s", scontext);
+		kfree(scontext);
+	}
+
+	rc = security_sid_to_context_inval(sad->state, sad->tsid, &scontext,
+					   &scontext_len);
+	if (!rc && scontext) {
+		audit_log_format(ab, " trawcon=%s", scontext);
+		kfree(scontext);
+	}
 }
 
 /* This is the slow part of avc audit with big stack footprint */
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index ba8eedf42b90..f68fb25b5702 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -255,6 +255,9 @@  int security_sid_to_context(struct selinux_state *state, u32 sid,
 int security_sid_to_context_force(struct selinux_state *state,
 				  u32 sid, char **scontext, u32 *scontext_len);
 
+int security_sid_to_context_inval(struct selinux_state *state,
+				  u32 sid, char **scontext, u32 *scontext_len);
+
 int security_context_to_sid(struct selinux_state *state,
 			    const char *scontext, u32 scontext_len,
 			    u32 *out_sid, gfp_t gfp);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index dd44126c8d14..9be05c3e99dc 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1281,7 +1281,8 @@  const char *security_get_initial_sid_context(u32 sid)
 
 static int security_sid_to_context_core(struct selinux_state *state,
 					u32 sid, char **scontext,
-					u32 *scontext_len, int force)
+					u32 *scontext_len, int force,
+					int only_invalid)
 {
 	struct policydb *policydb;
 	struct sidtab *sidtab;
@@ -1326,8 +1327,14 @@  static int security_sid_to_context_core(struct selinux_state *state,
 		rc = -EINVAL;
 		goto out_unlock;
 	}
-	rc = context_struct_to_string(policydb, context, scontext,
-				      scontext_len);
+	if (only_invalid && !context->len) {
+		scontext = NULL;
+		scontext_len = 0;
+		rc = 0;
+	} else {
+		rc = context_struct_to_string(policydb, context, scontext,
+					      scontext_len);
+	}
 out_unlock:
 	read_unlock(&state->ss->policy_rwlock);
 out:
@@ -1349,14 +1356,34 @@  int security_sid_to_context(struct selinux_state *state,
 			    u32 sid, char **scontext, u32 *scontext_len)
 {
 	return security_sid_to_context_core(state, sid, scontext,
-					    scontext_len, 0);
+					    scontext_len, 0, 0);
 }
 
 int security_sid_to_context_force(struct selinux_state *state, u32 sid,
 				  char **scontext, u32 *scontext_len)
 {
 	return security_sid_to_context_core(state, sid, scontext,
-					    scontext_len, 1);
+					    scontext_len, 1, 0);
+}
+
+/**
+ * security_sid_to_context_inval - Obtain a context for a given SID if it
+ *                                 is invalid.
+ * @sid: security identifier, SID
+ * @scontext: security context
+ * @scontext_len: length in bytes
+ *
+ * Write the string representation of the context associated with @sid
+ * into a dynamically allocated string of the correct size, but only if the
+ * context is invalid in the current policy.  Set @scontext to point to
+ * this string (or NULL if the context is valid) and set @scontext_len to
+ * the length of the string (or 0 if the context is valid).
+ */
+int security_sid_to_context_inval(struct selinux_state *state, u32 sid,
+				  char **scontext, u32 *scontext_len)
+{
+	return security_sid_to_context_core(state, sid, scontext,
+					    scontext_len, 1, 1);
 }
 
 /*