Skip to contentSkip to conversation

netlabel: improve domain mapping validation

PATCH In review Paul Moore 3 participants 5 messages May 17, 2013 – May 19, 2013

Subsystems (4)
lore
Share & tools
lore

Paul Moore

author
#0 lore raw

PATCH 1/1 netlabel: improve domain mapping validation

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.

---
 net/netlabel/netlabel_domainhash.c |   69 ++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)
net/netlabel/netlabel_domainhash.c +69 -0
index d8d4243..6bb1d42 100644
@@ -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
#1 lore raw

Hello.

Quoted 65 lines · Paul Moore

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.

---
  net/netlabel/netlabel_domainhash.c |   69 ++++++++++++++++++++++++++++++++++++
  1 file changed, 69 insertions(+)
net/netlabel/netlabel_domainhash.c +37 -0
index d8d4243..6bb1d42 100644
@@ -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;

net/netlabel/netlabel_domainhash.c static void netlbl_domhsh_audit_add(struct netlbl_dom_map *entry, · line 294

Quoted 10 lines
net/netlabel/netlabel_domainhash.c +10 -0
+				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.

Quoted 8 lines
net/netlabel/netlabel_domainhash.c +8 -0
+		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;
+			}

net/netlabel/netlabel_domainhash.c static void netlbl_domhsh_audit_add(struct netlbl_dom_map *entry, · line 312

Quoted 10 lines
net/netlabel/netlabel_domainhash.c +10 -0
+		}
+#endif /* IPv6 */
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+

WBR, Sergei

author
#2 lore raw
Quoted 19 lines · Sergei Shtylyov

Hello.

Quoted 16 lines · Paul Moore

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.

...

Quoted 7 lines
Quoted 1 line

+#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.

Quoted 17 lines
Quoted 17 lines
net/netlabel/netlabel_domainhash.c +17 -0
+		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;
+}
Signature · 2 lines
paul moore
security and virtualization @ redhat
#3 lore raw
Quoted 18 lines · Paul Moore
Quoted 16 lines
Quoted 16 lines

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.

...

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

+#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.

Quoted 2 lines

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

maintainer
#4 lore raw
In reply to Paul Moore

From: Paul Moore <[email protected]>
Date: Fri, 17 May 2013 15:08:50 -0400

Quoted 11 lines

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.

Quoted 5 lines

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

Applied and queued up for -stable, thanks.