netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netlabel: improve domain mapping validation
@ 2013-05-17 19:08 Paul Moore
  2013-05-17 19:23 ` Sergei Shtylyov
  2013-05-19 21:50 ` David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: Paul Moore @ 2013-05-17 19:08 UTC (permalink / raw)
  To: netdev; +Cc: linux-security-module, vlad.halilov, selinux

The net/netlabel/netlabel_domainhash.c:netlbl_domhsh_add() function
does not properly validate new domain hash entries resulting in
potential problems when an administrator attempts to add an invalid
entry.  One such problem, as reported by Vlad Halilov, is a kernel
BUG (found in netlabel_domainhash.c:netlbl_domhsh_audit_add()) when
adding an IPv6 outbound mapping with a CIPSO configuration.

This patch corrects this problem by adding the necessary validation
code to netlbl_domhsh_add() via the newly created
netlbl_domhsh_validate() function.

Ideally this patch should also be pushed to the currently active
-stable trees.

Reported-by: Vlad Halilov <vlad.halilov@gmail.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 net/netlabel/netlabel_domainhash.c |   69 ++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/net/netlabel/netlabel_domainhash.c b/net/netlabel/netlabel_domainhash.c
index d8d4243..6bb1d42 100644
--- a/net/netlabel/netlabel_domainhash.c
+++ b/net/netlabel/netlabel_domainhash.c
@@ -245,6 +245,71 @@ static void netlbl_domhsh_audit_add(struct netlbl_dom_map *entry,
 	}
 }
 
+/**
+ * netlbl_domhsh_validate - Validate a new domain mapping entry
+ * @entry: the entry to validate
+ *
+ * This function validates the new domain mapping entry to ensure that it is
+ * a valid entry.  Returns zero on success, negative values on failure.
+ *
+ */
+static int netlbl_domhsh_validate(const struct netlbl_dom_map *entry)
+{
+	struct netlbl_af4list *iter4;
+	struct netlbl_domaddr4_map *map4;
+#if IS_ENABLED(CONFIG_IPV6)
+	struct netlbl_af6list *iter6;
+	struct netlbl_domaddr6_map *map6;
+#endif /* IPv6 */
+
+	if (entry == NULL)
+		return -EINVAL;
+
+	switch (entry->type) {
+	case NETLBL_NLTYPE_UNLABELED:
+		if (entry->type_def.cipsov4 != NULL ||
+		    entry->type_def.addrsel != NULL)
+			return -EINVAL;
+		break;
+	case NETLBL_NLTYPE_CIPSOV4:
+		if (entry->type_def.cipsov4 == NULL)
+			return -EINVAL;
+		break;
+	case NETLBL_NLTYPE_ADDRSELECT:
+		netlbl_af4list_foreach(iter4, &entry->type_def.addrsel->list4) {
+			map4 = netlbl_domhsh_addr4_entry(iter4);
+			switch (map4->type) {
+			case NETLBL_NLTYPE_UNLABELED:
+				if (map4->type_def.cipsov4 != NULL)
+					return -EINVAL;
+				break;
+			case NETLBL_NLTYPE_CIPSOV4:
+				if (map4->type_def.cipsov4 == NULL)
+					return -EINVAL;
+				break;
+			default:
+				return -EINVAL;
+			}
+		}
+#if IS_ENABLED(CONFIG_IPV6)
+		netlbl_af6list_foreach(iter6, &entry->type_def.addrsel->list6) {
+			map6 = netlbl_domhsh_addr6_entry(iter6);
+			switch (map6->type) {
+			case NETLBL_NLTYPE_UNLABELED:
+				break;
+			default:
+				return -EINVAL;
+			}
+		}
+#endif /* IPv6 */
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * Domain Hash Table Functions
  */
@@ -311,6 +376,10 @@ int netlbl_domhsh_add(struct netlbl_dom_map *entry,
 	struct netlbl_af6list *tmp6;
 #endif /* IPv6 */
 
+	ret_val = netlbl_domhsh_validate(entry);
+	if (ret_val != 0)
+		return ret_val;
+
 	/* XXX - we can remove this RCU read lock as the spinlock protects the
 	 *       entire function, but before we do we need to fixup the
 	 *       netlbl_af[4,6]list RCU functions to do "the right thing" with


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] netlabel: improve domain mapping validation
  2013-05-17 19:08 [PATCH] netlabel: improve domain mapping validation Paul Moore
@ 2013-05-17 19:23 ` Sergei Shtylyov
  2013-05-17 19:37   ` Paul Moore
  2013-05-19 21:50 ` David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2013-05-17 19:23 UTC (permalink / raw)
  To: Paul Moore; +Cc: netdev, linux-security-module, vlad.halilov, selinux

Hello.

On 05/17/2013 11:08 PM, Paul Moore wrote:

> The net/netlabel/netlabel_domainhash.c:netlbl_domhsh_add() function
> does not properly validate new domain hash entries resulting in
> potential problems when an administrator attempts to add an invalid
> entry.  One such problem, as reported by Vlad Halilov, is a kernel
> BUG (found in netlabel_domainhash.c:netlbl_domhsh_audit_add()) when
> adding an IPv6 outbound mapping with a CIPSO configuration.
>
> This patch corrects this problem by adding the necessary validation
> code to netlbl_domhsh_add() via the newly created
> netlbl_domhsh_validate() function.
>
> Ideally this patch should also be pushed to the currently active
> -stable trees.
>
> Reported-by: Vlad Halilov <vlad.halilov@gmail.com>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> ---
>   net/netlabel/netlabel_domainhash.c |   69 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 69 insertions(+)
>
> diff --git a/net/netlabel/netlabel_domainhash.c b/net/netlabel/netlabel_domainhash.c
> index d8d4243..6bb1d42 100644
> --- a/net/netlabel/netlabel_domainhash.c
> +++ b/net/netlabel/netlabel_domainhash.c
> @@ -245,6 +245,71 @@ static void netlbl_domhsh_audit_add(struct netlbl_dom_map *entry,
>   	}
>   }
>   
> +/**
> + * netlbl_domhsh_validate - Validate a new domain mapping entry
> + * @entry: the entry to validate
> + *
> + * This function validates the new domain mapping entry to ensure that it is
> + * a valid entry.  Returns zero on success, negative values on failure.
> + *
> + */
> +static int netlbl_domhsh_validate(const struct netlbl_dom_map *entry)
> +{
> +	struct netlbl_af4list *iter4;
> +	struct netlbl_domaddr4_map *map4;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	struct netlbl_af6list *iter6;
> +	struct netlbl_domaddr6_map *map6;
> +#endif /* IPv6 */
> +
> +	if (entry == NULL)
> +		return -EINVAL;
> +
> +	switch (entry->type) {
> +	case NETLBL_NLTYPE_UNLABELED:
> +		if (entry->type_def.cipsov4 != NULL ||
> +		    entry->type_def.addrsel != NULL)
> +			return -EINVAL;
> +		break;
> +	case NETLBL_NLTYPE_CIPSOV4:
> +		if (entry->type_def.cipsov4 == NULL)
> +			return -EINVAL;
> +		break;
> +	case NETLBL_NLTYPE_ADDRSELECT:
> +		netlbl_af4list_foreach(iter4, &entry->type_def.addrsel->list4) {
> +			map4 = netlbl_domhsh_addr4_entry(iter4);
> +			switch (map4->type) {
> +			case NETLBL_NLTYPE_UNLABELED:
> +				if (map4->type_def.cipsov4 != NULL)
> +					return -EINVAL;
> +				break;
> +			case NETLBL_NLTYPE_CIPSOV4:
> +				if (map4->type_def.cipsov4 == NULL)
> +					return -EINVAL;
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +		}
> +#if IS_ENABLED(CONFIG_IPV6)

   Why not:

         if (IS_ENABLED(CONFIG_IPV6))

    #if's in the function body are frowned upon.

> +		netlbl_af6list_foreach(iter6, &entry->type_def.addrsel->list6) {
> +			map6 = netlbl_domhsh_addr6_entry(iter6);
> +			switch (map6->type) {
> +			case NETLBL_NLTYPE_UNLABELED:
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +		}
> +#endif /* IPv6 */
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +

WBR, Sergei


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] netlabel: improve domain mapping validation
  2013-05-17 19:23 ` Sergei Shtylyov
@ 2013-05-17 19:37   ` Paul Moore
  2013-05-17 21:47     ` Sergei Shtylyov
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Moore @ 2013-05-17 19:37 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev, linux-security-module, vlad.halilov, selinux

On Friday, May 17, 2013 11:23:03 PM Sergei Shtylyov wrote:
> Hello.
> 
> On 05/17/2013 11:08 PM, Paul Moore wrote:
> > The net/netlabel/netlabel_domainhash.c:netlbl_domhsh_add() function
> > does not properly validate new domain hash entries resulting in
> > potential problems when an administrator attempts to add an invalid
> > entry.  One such problem, as reported by Vlad Halilov, is a kernel
> > BUG (found in netlabel_domainhash.c:netlbl_domhsh_audit_add()) when
> > adding an IPv6 outbound mapping with a CIPSO configuration.
> > 
> > This patch corrects this problem by adding the necessary validation
> > code to netlbl_domhsh_add() via the newly created
> > netlbl_domhsh_validate() function.
> > 
> > Ideally this patch should also be pushed to the currently active
> > -stable trees.
> > 
> > Reported-by: Vlad Halilov <vlad.halilov@gmail.com>
> > Signed-off-by: Paul Moore <pmoore@redhat.com>

...

> > +#if IS_ENABLED(CONFIG_IPV6)
> 
>    Why not:
> 
>          if (IS_ENABLED(CONFIG_IPV6))
> 
>     #if's in the function body are frowned upon.

The dependent functions/types are not defined when !IS_ENABLED(CONFIG_IPV6); 
please look at the rest of the NetLabel code (net/netlabel).

Perhaps we can do some work to fixup some of that in the future, but that 
shouldn't hold up this fix.
 
> > +		netlbl_af6list_foreach(iter6, &entry->type_def.addrsel->list6) {
> > +			map6 = netlbl_domhsh_addr6_entry(iter6);
> > +			switch (map6->type) {
> > +			case NETLBL_NLTYPE_UNLABELED:
> > +				break;
> > +			default:
> > +				return -EINVAL;
> > +			}
> > +		}
> > +#endif /* IPv6 */
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}

-- 
paul moore
security and virtualization @ redhat


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] netlabel: improve domain mapping validation
  2013-05-17 19:37   ` Paul Moore
@ 2013-05-17 21:47     ` Sergei Shtylyov
  0 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2013-05-17 21:47 UTC (permalink / raw)
  To: Paul Moore; +Cc: netdev, linux-security-module, vlad.halilov, selinux

On 05/17/2013 11:37 PM, Paul Moore wrote:

>>> The net/netlabel/netlabel_domainhash.c:netlbl_domhsh_add() function
>>> does not properly validate new domain hash entries resulting in
>>> potential problems when an administrator attempts to add an invalid
>>> entry.  One such problem, as reported by Vlad Halilov, is a kernel
>>> BUG (found in netlabel_domainhash.c:netlbl_domhsh_audit_add()) when
>>> adding an IPv6 outbound mapping with a CIPSO configuration.
>>>
>>> This patch corrects this problem by adding the necessary validation
>>> code to netlbl_domhsh_add() via the newly created
>>> netlbl_domhsh_validate() function.
>>>
>>> Ideally this patch should also be pushed to the currently active
>>> -stable trees.
>>>
>>> Reported-by: Vlad Halilov <vlad.halilov@gmail.com>
>>> Signed-off-by: Paul Moore <pmoore@redhat.com>
> ...
>
>>> +#if IS_ENABLED(CONFIG_IPV6)
>>     Why not:
>>
>>           if (IS_ENABLED(CONFIG_IPV6))
>>
>>      #if's in the function body are frowned upon.
> The dependent functions/types are not defined when !IS_ENABLED(CONFIG_IPV6);
> please look at the rest of the NetLabel code (net/netlabel).

     Sorry, didn't think about declarations.

> Perhaps we can do some work to fixup some of that in the future, but that
> shouldn't hold up this fix.

     Perhaps factoring out the #ifdef'ed code into a separate function 
would help
if at all possible.

WBR, Sergei


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] netlabel: improve domain mapping validation
  2013-05-17 19:08 [PATCH] netlabel: improve domain mapping validation Paul Moore
  2013-05-17 19:23 ` Sergei Shtylyov
@ 2013-05-19 21:50 ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2013-05-19 21:50 UTC (permalink / raw)
  To: pmoore; +Cc: netdev, linux-security-module, vlad.halilov, selinux

From: Paul Moore <pmoore@redhat.com>
Date: Fri, 17 May 2013 15:08:50 -0400

> The net/netlabel/netlabel_domainhash.c:netlbl_domhsh_add() function
> does not properly validate new domain hash entries resulting in
> potential problems when an administrator attempts to add an invalid
> entry.  One such problem, as reported by Vlad Halilov, is a kernel
> BUG (found in netlabel_domainhash.c:netlbl_domhsh_audit_add()) when
> adding an IPv6 outbound mapping with a CIPSO configuration.
> 
> This patch corrects this problem by adding the necessary validation
> code to netlbl_domhsh_add() via the newly created
> netlbl_domhsh_validate() function.
> 
> Ideally this patch should also be pushed to the currently active
> -stable trees.
> 
> Reported-by: Vlad Halilov <vlad.halilov@gmail.com>
> Signed-off-by: Paul Moore <pmoore@redhat.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-05-19 21:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-17 19:08 [PATCH] netlabel: improve domain mapping validation Paul Moore
2013-05-17 19:23 ` Sergei Shtylyov
2013-05-17 19:37   ` Paul Moore
2013-05-17 21:47     ` Sergei Shtylyov
2013-05-19 21:50 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).