OS-5195 DHCP spoofing protection should allow permitting all Client Identifiers

Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Dan McDonald <danmcd@joyent.com>

10 files changed, 303 insertions, 57 deletions

diff --git a/usr/src/common/net/dhcp/octet.c b/usr/src/common/net/dhcp/octet.c
index d8367bbf0b..370604c4e3 100644
--- a/usr/src/common/net/dhcp/octet.c
+++ b/usr/src/common/net/dhcp/octet.c
@@ -77,6 +77,9 @@ octet_to_hexascii(const void *nump, uint_t nlen, char *bufp, uint_t *blen)
  * Converts an ASCII string into an octet string.
  *
  * Returns 0 for success, errno otherwise.
+ *
+ * If the string contains invalid hexadecimal characters, or an odd number of
+ * characters then this function returns EINVAL.
  */
 int
 hexascii_to_octet(const char *asp, uint_t alen, void *bufp, uint_t *blen)
diff --git a/usr/src/lib/libdladm/Makefile b/usr/src/lib/libdladm/Makefile
index a74b40f58d..275c1c0866 100644
--- a/usr/src/lib/libdladm/Makefile
+++ b/usr/src/lib/libdladm/Makefile
@@ -57,7 +57,8 @@ TYPELIST =	overlay_ioc_create_t \
 		overlay_ioc_delete_t \
 		overlay_ioc_nprops_t \
 		overlay_ioc_propinfo_t \
-		overlay_ioc_prop_t
+		overlay_ioc_prop_t \
+		mac_protect_t
 
 all :=		TARGET = all
 clean :=	TARGET = clean
diff --git a/usr/src/lib/libdladm/common/libdladm_impl.h b/usr/src/lib/libdladm/common/libdladm_impl.h
index e1a3177808..08981ff65f 100644
--- a/usr/src/lib/libdladm/common/libdladm_impl.h
+++ b/usr/src/lib/libdladm/common/libdladm_impl.h
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2015, Joyent, Inc.
+ * Copyright 2017, Joyent, Inc.
  */
 
 #ifndef _LIBDLADM_IMPL_H
@@ -147,10 +147,10 @@ extern dladm_status_t	dladm_flow_proplist_extract(dladm_arg_list_t *,
  * by the pd_check function.
  */
 typedef	dladm_status_t	rp_extractf_t(val_desc_t *, uint_t, void *);
-extern rp_extractf_t	extract_maxbw, extract_priority,
-			extract_cpus, extract_protection,
-			extract_allowedips, extract_allowedcids,
-			extract_rxrings, extract_txrings, extract_pool;
+extern rp_extractf_t	extract_priority, extract_cpus,
+			extract_protection, extract_allowallcids, extract_pool,
+			extract_allowedips, extract_allowedcids, extract_maxbw,
+			extract_rxrings, extract_txrings;
 
 typedef struct resource_prop_s {
 	/*
diff --git a/usr/src/lib/libdladm/common/linkprop.c b/usr/src/lib/libdladm/common/linkprop.c
index 338ccc428e..643c0968ea 100644
--- a/usr/src/lib/libdladm/common/linkprop.c
+++ b/usr/src/lib/libdladm/common/linkprop.c
@@ -153,8 +153,8 @@ static pd_getf_t	get_zone, get_autopush, get_rate_mod, get_rate,
 			get_tagmode, get_range, get_stp, get_bridge_forward,
 			get_bridge_pvid, get_protection, get_rxrings,
 			get_txrings, get_cntavail, get_secondary_macs,
-			get_allowedips, get_allowedcids, get_pool,
-			get_rings_range, get_linkmode_prop,
+			get_allowallcids, get_allowedips, get_allowedcids,
+			get_pool, get_rings_range, get_linkmode_prop,
 			get_promisc_filtered;
 
 static pd_setf_t	set_zone, set_rate, set_powermode, set_radio,
@@ -451,6 +451,11 @@ static  val_desc_t	link_protect_vals[] = {
 	{ "dhcp-nospoof",	MPT_DHCPNOSPOOF	},
 };
 
+static val_desc_t	dladm_bool_vals[] = {
+	{ "false",	MPT_FALSE },
+	{ "true",	MPT_TRUE },
+};
+
 static  val_desc_t	link_promisc_filtered_vals[] = {
 	{ "off",	B_FALSE },
 	{ "on",		B_TRUE },
@@ -808,6 +813,11 @@ static prop_desc_t	prop_table[] = {
 	    get_allowedcids, check_allowedcids, PD_CHECK_ALLOC,
 	    DATALINK_CLASS_ALL, DATALINK_ANY_MEDIATYPE },
 
+	{ "allow-all-dhcp-cids", { "false", RESET_VAL },
+	    dladm_bool_vals, VALCNT(dladm_bool_vals), set_resource, NULL,
+	    get_allowallcids, check_prop, 0,
+	    DATALINK_CLASS_ALL, DATALINK_ANY_MEDIATYPE },
+
 	{ "rxrings", { "--", RESET_VAL }, NULL, 0,
 	    set_resource, get_rings_range, get_rxrings, check_rings, 0,
 	    DATALINK_CLASS_ALL, DATALINK_ANY_MEDIATYPE },
@@ -856,6 +866,7 @@ static resource_prop_t rsrc_prop_table[] = {
 	{"protection",		extract_protection},
 	{"allowed-ips",		extract_allowedips},
 	{"allowed-dhcp-cids",	extract_allowedcids},
+	{"allow-all-dhcp-cids",	extract_allowallcids},
 	{"rxrings",		extract_rxrings},
 	{"rxrings-effective",	extract_rxrings},
 	{"txrings",		extract_txrings},
@@ -2918,6 +2929,47 @@ dladm_str2cid(char *buf, mac_dhcpcid_t *cid)
 
 /* ARGSUSED */
 static dladm_status_t
+get_allowallcids(dladm_handle_t handle, prop_desc_t *pdp,
+    datalink_id_t linkid, char **prop_val, uint_t *val_cnt,
+    datalink_media_t media, uint_t flags, uint_t *perm_flags)
+{
+	mac_resource_props_t	mrp;
+	mac_protect_t		*p;
+	dladm_status_t		status;
+
+	if (*val_cnt < 1)
+		return (DLADM_STATUS_BADVALCNT);
+
+	status = i_dladm_get_public_prop(handle, linkid, "resource", flags,
+	    perm_flags, &mrp, sizeof (mrp));
+	if (status != DLADM_STATUS_OK)
+		return (status);
+
+	p = &mrp.mrp_protect;
+	(void) snprintf(*prop_val, DLADM_STRSIZE,
+	    p->mp_allcids ? "true" : "false");
+	*val_cnt = 1;
+	return (DLADM_STATUS_OK);
+}
+
+/* ARGSUSED */
+dladm_status_t
+extract_allowallcids(val_desc_t *vdp, uint_t cnt, void *arg)
+{
+	mac_resource_props_t	*mrp = arg;
+	mac_protect_t		*p = &mrp->mrp_protect;
+
+	if (vdp->vd_val == RESET_VAL || vdp->vd_val == MPT_FALSE) {
+		p->mp_allcids = (boolean_t)RESET_VAL;
+	} else {
+		p->mp_allcids = (boolean_t)vdp->vd_val;
+	}
+	mrp->mrp_mask |= MRP_PROTECT;
+	return (DLADM_STATUS_OK);
+}
+
+/* ARGSUSED */
+static dladm_status_t
 get_allowedcids(dladm_handle_t handle, prop_desc_t *pdp,
     datalink_id_t linkid, char **prop_val, uint_t *val_cnt,
     datalink_media_t media, uint_t flags, uint_t *perm_flags)
diff --git a/usr/src/man/man1m/dladm.1m b/usr/src/man/man1m/dladm.1m
index c647cd7f19..0519cd307f 100644
--- a/usr/src/man/man1m/dladm.1m
+++ b/usr/src/man/man1m/dladm.1m
@@ -41,9 +41,9 @@
 .\"
 .\"
 .\" Copyright (c) 2008, Sun Microsystems, Inc. All Rights Reserved
-.\" Copyright 2016 Joyent, Inc.
+.\" Copyright 2017 Joyent, Inc.
 .\"
-.TH DLADM 1M "Dec 16, 2016"
+.TH DLADM 1M "Dec 6, 2017"
 .SH NAME
 dladm \- administer data links
 .SH SYNOPSIS
@@ -4893,6 +4893,43 @@ The following general link properties are supported:
 .sp
 .ne 2
 .na
+\fB\fBallow-all-dhcp-cids\fR\fR
+.ad
+.sp .6
+.RS 4n
+One of \fBtrue\fR or \fBfalse\fR, to indicate whether or not all DHCP Client
+Identifiers should be permitted on this interface when DHCP spoofing protection
+is being used. This can be useful in cases where a DHCP client is using RFC
+4361-style Client Identifiers, which are based on a value that is opaque to the
+Global Zone, but enforcement of MAC addresses in DHCP packets is still desired.
+.RE
+
+.sp
+.ne 2
+.na
+\fB\fBallowed-dhcp-cids\fR\fR
+.ad
+.sp .6
+.RS 4n
+A comma-separated list of DHCP Client Identifiers that are allowed on the
+interface.
+.sp
+Client identifiers can be written in three different formats: a string of
+hexadecimal characters prefixed by \fB0x\fR, indicating the exact bytes used in
+the Client Identifier; an RFC 3315 DUID of the form
+"1.<hardware\ type>.<time>.<link-layer\ address>" (DUID-LLT),
+"2.<enterprise\ number>.<hex\ string>" (DUID-EN), or
+"3.<hardware\ type>.<link-layer\ address>" (DUID-LL); or a string of characters
+whose byte values should be used as the Client Identifier.
+.sp
+When specifying a string of hexadecimal characters prefixed by \fB0x\fR or as
+part of a DUID-EN string, an even number of hexadecimal characters must be
+provided in order to fully specify each byte.
+.RE
+
+.sp
+.ne 2
+.na
 \fB\fBallowed-ips\fR\fR
 .ad
 .sp .6
@@ -5965,6 +6002,16 @@ Interface Stability	Committed
 \fBacctadm\fR(1M), \fBautopush\fR(1M), \fBifconfig\fR(1M), \fBipsecconf\fR(1M),
 \fBndd\fR(1M), \fBpsrset\fR(1M), \fBwpad\fR(1M), \fBzonecfg\fR(1M),
 \fBattributes\fR(5), \fBieee802.3\fR(5), \fBoverlay\fR(5), \fBdlpi\fR(7P)
+.sp
+.LP
+R. Droms, Ed., J. Bound, B. Volz, T. Lemon, C. Perkins, M. Carney. \fIRFC 3315:
+Dynamic Host Configuration Protocol for IPv6 (DHCPv6)\fR. The Internet Society.
+July 2003.
+.sp
+.LP
+T. Lemon, B. Sommerfeld. February 2006. \fIRFC 4361: Node-specific Client
+Identifiers for Dynamic Host Configuration Protocol Version Four (DHCPv4)\fR.
+The Internet Society. January 2006.
 .SH NOTES
 .LP
 The preferred method of referring to an aggregation in the aggregation
diff --git a/usr/src/test/util-tests/tests/dladm/allowed-cids.ksh b/usr/src/test/util-tests/tests/dladm/allowed-cids.ksh
new file mode 100644
index 0000000000..9b62962baf
--- /dev/null
+++ b/usr/src/test/util-tests/tests/dladm/allowed-cids.ksh
@@ -0,0 +1,95 @@
+#!/bin/ksh
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2017 Joyent, Inc.
+#
+
+source ./common.ksh
+
+property="allowed-dhcp-cids"
+
+setup
+
+# Valid hexadecimal strings
+epass 0x1234
+epass 0x123456789abcdef0
+epass 0x123456789abcdef0
+
+# Hex strings w/ an odd number of characters are not allowed
+efail 0x0
+efail 0x1
+efail 0x1fa
+efail 0xfba39e2
+
+# Invalid hexadecimal strings
+efail 0xz
+efail 0x01234567q12
+efail 0x=+
+efail 0x-1
+efail 0x1,2,3
+
+# Valid RFC 3315 DUID strings
+
+## DUID-LLT
+epass 1.1.1234.90:b8:d0:81:91:30
+epass 1.1.1512434853.90:b8:d0:81:91:30
+epass 1.1.28530123.90:b8:d0:81:91:30
+epass 1.6.1512434853.14:10:9f:d0:5b:d3
+
+## DUID-EN
+epass 2.9.0CC084D303000912
+epass 2.9.0cc084d303000912
+epass 2.32473.45ab
+epass 2.38678.0123abcd
+
+## DUID-LL
+epass 3.1.90:b8:d0:81:91:30
+epass 3.1.90:b8:d0:4b:c7:3b
+epass 3.1.2:8:20:a4:4d:ee
+epass 3.6.14:10:9f:d0:5b:d3
+
+# Invalid RFC 3315 DUID strings
+
+## DUID-LLT
+efail 1.1.12a34.90:b8:d0:81:91:30
+efail 1.1.15-33.90:b8:d0:81:91:30
+efail 1.1.98+123.90:b8:d0:81:91:30
+efail 3.z.1512434853.14:10:9f:d0:5b:d3
+efail 3.6.1512434853.q4:10:9f:d0:5b:d3
+
+## DUID-EN
+efail 2.32473.45a
+efail 2.9.Z
+efail 2.9.-12
+efail 2.QZ4.45a
+efail 2.38d78.0123abcd
+
+## DUID-LL
+efail 3.wy.90:b8:d0:81:91:30
+efail 3.1.90:z8:di:ob:c7:3b
+efail 3.1.5.2:8:20:a4:4d:ee
+
+## Uknown DUID forms
+efail 4.1.45a
+efail 9.1.45a
+efail 23.1.45a
+
+# Random strings of bytes are also accepted,
+# if they don't have the above prefixes.
+epass 1234
+epass abcdef
+epass qsxasdasdfgfdgkj123455
+epass 0x
+
+cleanup
+printf "TEST PASS: $ai_arg0\n"
diff --git a/usr/src/test/util-tests/tests/dladm/allowed-ips.ksh b/usr/src/test/util-tests/tests/dladm/allowed-ips.ksh
index 866b8c7966..4802f86286 100644
--- a/usr/src/test/util-tests/tests/dladm/allowed-ips.ksh
+++ b/usr/src/test/util-tests/tests/dladm/allowed-ips.ksh
@@ -11,47 +11,12 @@
 #
 
 #
-# Copyright (c) 2014, Joyent, Inc.
+# Copyright 2016 Joyent, Inc.
 #
 
-ai_arg0="$(basename $0)"
-ai_stub="teststub$$"
-ai_vnic="testvnic$$"
+source ./common.ksh
 
-function fatal
-{
-	typeset msg="$*"
-	[[ -z "$msg" ]] && msg="failed"
-	echo "TEST_FAIL: $vt_arg0: $msg" >&2
-	exit 1
-}
-
-function setup
-{
-	dladm create-etherstub $ai_stub || fatal "failed to create etherstub"
-	dladm create-vnic -l $ai_stub $ai_vnic || fatal "failed to create vnic"
-}
-
-function cleanup
-{
-	dladm delete-vnic $ai_vnic || fatal "failed to remove vnic"
-	dladm delete-etherstub $ai_stub || fatal "failed to remove etherstub"
-}
-
-function runtest
-{
-	dladm set-linkprop -p allowed-ips="$@" $ai_vnic 2>/dev/null
-}
-
-function epass
-{
-	runtest $* || fatal "allowed-ips=$* failed, expected success\n"
-}
-
-function efail
-{
-	runtest $* && fatal "allowed-ips=$* succeeded, expected failure\n"
-}
+property="allowed-ips"
 
 #
 # Run through all IPv6 prefixes for validity with a token prefix
@@ -204,4 +169,4 @@ epass fe80::/15
 epass fe82::/15
 
 cleanup
-printf "TEST PASS: $ai_arg0"
+printf "TEST PASS: $ai_arg0\n"
diff --git a/usr/src/test/util-tests/tests/dladm/common.ksh b/usr/src/test/util-tests/tests/dladm/common.ksh
new file mode 100644
index 0000000000..e5301d0a52
--- /dev/null
+++ b/usr/src/test/util-tests/tests/dladm/common.ksh
@@ -0,0 +1,57 @@
+#!/bin/ksh
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2017 Joyent, Inc.
+#
+
+ai_arg0="$(basename $0)"
+ai_stub="teststub$$"
+ai_vnic="testvnic$$"
+
+typeset property
+
+function fatal
+{
+	typeset msg="$*"
+	[[ -z "$msg" ]] && msg="failed"
+	echo "TEST_FAIL: $ai_arg0: $msg" >&2
+	exit 1
+}
+
+function setup
+{
+	dladm create-etherstub $ai_stub || fatal "failed to create etherstub"
+	dladm create-vnic -l $ai_stub $ai_vnic || fatal "failed to create vnic"
+}
+
+function cleanup
+{
+	dladm delete-vnic $ai_vnic || fatal "failed to remove vnic"
+	dladm delete-etherstub $ai_stub || fatal "failed to remove etherstub"
+}
+
+function runtest
+{
+	[[ -z "$property" ]] && fatal "missing property to set"
+	dladm set-linkprop -p $property="$@" $ai_vnic 2>/dev/null
+}
+
+function epass
+{
+	runtest $* || fatal "$property=$* failed, expected success\n"
+}
+
+function efail
+{
+	runtest $* && fatal "$property=$* succeeded, expected failure\n"
+}
diff --git a/usr/src/uts/common/io/mac/mac_protect.c b/usr/src/uts/common/io/mac/mac_protect.c
index da83dc643e..fafb54cef9 100644
--- a/usr/src/uts/common/io/mac/mac_protect.c
+++ b/usr/src/uts/common/io/mac/mac_protect.c
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2015, Joyent, Inc.  All rights reserved.
+ * Copyright 2017, Joyent, Inc.  All rights reserved.
  */
 /*
  * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
@@ -2025,6 +2025,9 @@ dhcpnospoof_check_cid(mac_protect_t *p, uchar_t *cid, uint_t cidlen)
 		    bcmp(dcid->dc_id, cid, cidlen) == 0)
 			return (B_TRUE);
 	}
+
+	DTRACE_PROBE3(missing__cid, mac_protect_t *, p,
+	    uchar_t *, cid, uint_t, cidlen);
 	return (B_FALSE);
 }
 
@@ -2046,6 +2049,12 @@ dhcpnospoof_check_v4(mac_client_impl_t *mcip, mac_protect_t *p,
 	    bcmp(mcip->mci_unicast->ma_addr, dh4->chaddr, maclen) != 0) {
 		return (B_FALSE);
 	}
+
+	/* Everything after here is checking the Client Identifier */
+	if (p->mp_allcids == MPT_TRUE) {
+		return (B_TRUE);
+	}
+
 	if (get_dhcpv4_option(dh4, end, CD_CLIENT_ID, &cid, &optlen) == 0)
 		cidlen = optlen;
 
@@ -2082,6 +2091,11 @@ dhcpnospoof_check_v6(mac_client_impl_t *mcip, mac_protect_t *p,
 	    mtype == DHCPV6_MSG_RECONFIGURE)
 		return (B_TRUE);
 
+	/* Everything after here is checking the Client Identifier */
+	if (p->mp_allcids == MPT_TRUE) {
+		return (B_TRUE);
+	}
+
 	d6o = get_dhcpv6_option(&dh6[1], end - (uchar_t *)&dh6[1], NULL,
 	    DHCPV6_OPT_CLIENTID, &cidlen);
 	if (d6o == NULL || (uchar_t *)d6o + cidlen > end)
@@ -2159,7 +2173,6 @@ dhcpnospoof_check(mac_client_impl_t *mcip, mac_protect_t *protect,
 	return (0);
 
 fail:
-	/* increment dhcpnospoof stat here */
 	freemsg(nmp);
 	return (err);
 }
@@ -2487,6 +2500,11 @@ mac_protect_validate(mac_resource_props_t *mrp)
 	if ((err = validate_cids(p)) != 0)
 		return (err);
 
+	if (p->mp_allcids != MPT_FALSE && p->mp_allcids != MPT_TRUE &&
+	    p->mp_allcids != MPT_RESET) {
+		return (EINVAL);
+	}
+
 	return (0);
 }
 
@@ -2554,6 +2572,11 @@ mac_protect_update(mac_resource_props_t *new, mac_resource_props_t *curr)
 			cp->mp_cidcnt = 0;
 		}
 	}
+	if (np->mp_allcids == MPT_RESET) {
+		cp->mp_allcids = MPT_FALSE;
+	} else if (np->mp_allcids != 0) {
+		cp->mp_allcids = MPT_TRUE;
+	}
 }
 
 void
diff --git a/usr/src/uts/common/sys/mac_flow.h b/usr/src/uts/common/sys/mac_flow.h
index e290ba7dbe..d269d747c2 100644
--- a/usr/src/uts/common/sys/mac_flow.h
+++ b/usr/src/uts/common/sys/mac_flow.h
@@ -22,7 +22,7 @@
 /*
  * Copyright 2010 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
- * Copyright 2013 Joyent, Inc.  All rights reserved.
+ * Copyright 2017 Joyent, Inc.  All rights reserved.
  */
 
 #ifndef	_MAC_FLOW_H
@@ -155,6 +155,8 @@ typedef enum {
 #define	MPT_MAXIPADDR		MPT_MAXCNT
 #define	MPT_MAXCID		MPT_MAXCNT
 #define	MPT_MAXCIDLEN		256
+#define	MPT_FALSE		0x00000000
+#define	MPT_TRUE		0x00000001
 
 typedef struct mac_ipaddr_s {
 	uint32_t	ip_version;
@@ -175,11 +177,12 @@ typedef struct mac_dhcpcid_s {
 } mac_dhcpcid_t;
 
 typedef struct mac_protect_s {
-	uint32_t	mp_types;
-	uint32_t	mp_ipaddrcnt;
-	mac_ipaddr_t	mp_ipaddrs[MPT_MAXIPADDR];
-	uint32_t	mp_cidcnt;
-	mac_dhcpcid_t	mp_cids[MPT_MAXCID];
+	uint32_t	mp_types;	/* Enabled protection types */
+	uint32_t	mp_ipaddrcnt;	/* Count of allowed IPs */
+	mac_ipaddr_t	mp_ipaddrs[MPT_MAXIPADDR]; /* Allowed IPs */
+	uint32_t	mp_cidcnt;	/* Count of allowed DHCP CIDs */
+	mac_dhcpcid_t	mp_cids[MPT_MAXCID]; /* Allowed DHCP CIDs */
+	uint32_t	mp_allcids;	/* Whether to allow all CIDs through */
 } mac_protect_t;
 
 /* The default priority for links */