firewall: only use one list

Unless you use complicated rights veto rules, WFP's policy is that
between sublayers, block always outweighs allow. It's easier, therefore,
to simply weight a single sublayer correctly, with allow rules having
heavier weight than block rules.

This basically means that we have to be careful that DNS isn't a subset
of some allow rule. One place where this would be a problem are the
permitLan* rules, which we don't use anyway, and so this commit nukes
them.

Another place would be if somebody is using a localhost/loopback resolver
for whatever reason. This is probably a "low risk" sort of thing, but we
may want to fix this by ordering the dns block just in front of the
loopback permit.

The other place is in the wireguard.exe tunnel service itself, which
does DNS lookups. Since right now we mostly enforce one-tunnel-at-a-
time, this isn't really a problem. But later if we allow nested
tunneling, it means that the DNS lookup in a second tunnel can
potentially escape the DNS server of the first tunnel. We can address
this problem later, perhaps with fancier security descriptors that we
shuffle around depending on which state the tunnel is in. And on the
bright side, this change allows people to run WireGuard over port 53
itself, which is generally a desirable thing.

2 files changed, 22 insertions, 230 deletions

diff --git a/service/firewall/blocker.go b/service/firewall/blocker.go
index 8ef26278..d709da4d 100644
--- a/service/firewall/blocker.go
+++ b/service/firewall/blocker.go
@@ -18,9 +18,8 @@ type wfpObjectInstaller func(uintptr) error
 // Fundamental WireGuard specific WFP objects.
 //
 type baseObjects struct {
-	provider  windows.GUID
-	whitelist windows.GUID
-	blacklist windows.GUID
+	provider windows.GUID
+	filters  windows.GUID
 }
 
 var wfpSession uintptr
@@ -56,19 +55,12 @@ func registerBaseObjects(session uintptr) (*baseObjects, error) {
 		Data4: [8]byte{0x8f, 0x92, 0x29, 0xd7, 0x8a, 0x8d, 0x29, 0xd3},
 	}
 	// {FE3DB7F8-4658-4DE5-8DA9-CE5086A8266B}
-	whitelistGuid := windows.GUID{
+	filtersGuid := windows.GUID{
 		Data1: 0xfe3db7f8,
 		Data2: 0x4658,
 		Data3: 0x4de5,
 		Data4: [8]byte{0x8d, 0xa9, 0xce, 0x50, 0x86, 0xa8, 0x26, 0x6b},
 	}
-	// {CE1DD58F-A7BF-46BD-B048-9C5518346CE9}
-	blacklistGuid := windows.GUID{
-		Data1: 0xce1dd58f,
-		Data2: 0xa7bf,
-		Data3: 0x46bd,
-		Data4: [8]byte{0xb0, 0x48, 0x9c, 0x55, 0x18, 0x34, 0x6c, 0xe9},
-	}
 
 	//
 	// Register provider.
@@ -90,38 +82,17 @@ func registerBaseObjects(session uintptr) (*baseObjects, error) {
 	}
 
 	//
-	// Register whitelist sublayer.
+	// Register filters sublayer.
 	//
 	{
-		displayData, err := createWtFwpmDisplayData0("WireGuard whitelist", "Permissive filters")
+		displayData, err := createWtFwpmDisplayData0("WireGuard filters", "Permissive and blocking filters")
 		if err != nil {
 			return nil, wrapErr(err)
 		}
 		sublayer := wtFwpmSublayer0{
-			subLayerKey: whitelistGuid,
+			subLayerKey: filtersGuid,
 			displayData: *displayData,
 			providerKey: &providerGuid,
-			weight:      ^uint16(0),
-		}
-		err = fwpmSubLayerAdd0(session, &sublayer, 0)
-		if err != nil {
-			return nil, wrapErr(err)
-		}
-	}
-
-	//
-	// Register blacklist sublayer.
-	//
-	{
-		displayData, err := createWtFwpmDisplayData0("WireGuard blacklist", "Blocking filters")
-		if err != nil {
-			return nil, wrapErr(err)
-		}
-		sublayer := wtFwpmSublayer0{
-			subLayerKey: blacklistGuid,
-			displayData: *displayData,
-			providerKey: &providerGuid,
-			weight:      (^uint16(0)) - 1,
 		}
 		err = fwpmSubLayerAdd0(session, &sublayer, 0)
 		if err != nil {
@@ -131,8 +102,7 @@ func registerBaseObjects(session uintptr) (*baseObjects, error) {
 
 	return &baseObjects{
 		providerGuid,
-		whitelistGuid,
-		blacklistGuid,
+		filtersGuid,
 	}, nil
 }
 
@@ -167,21 +137,6 @@ func EnableFirewall(luid uint64, restrictDNS bool, restrictAll bool) error {
 			return wrapErr(err)
 		}
 
-		/* We actually don't want to allow lan explicitly. This is controlled by the restrictAll rule.
-		 * TODO: consider removing those functions or just rethinking about how this all works.
-
-		err = permitLanIpv4(session, baseObjects)
-		if err != nil {
-			return wrapErr(err)
-		}
-
-		err = permitLanIpv6(session, baseObjects)
-		if err != nil {
-			return wrapErr(err)
-		}
-
-		*/
-
 		err = permitDhcpIpv4(session, baseObjects)
 		if err != nil {
 			return wrapErr(err)
@@ -198,7 +153,7 @@ func EnableFirewall(luid uint64, restrictDNS bool, restrictAll bool) error {
 		}
 
 		if restrictDNS {
-			err = blockDnsNonTun(session, baseObjects, luid)
+			err = blockDnsUnmatched(session, baseObjects)
 			if err != nil {
 				return wrapErr(err)
 			}
diff --git a/service/firewall/rules.go b/service/firewall/rules.go
index 74d35609..12b70742 100644
--- a/service/firewall/rules.go
+++ b/service/firewall/rules.go
@@ -23,7 +23,7 @@ func permitTunInterface(session uintptr, baseObjects *baseObjects, ifLuid uint64
 
 	filter := wtFwpmFilter0{
 		providerKey:         &baseObjects.provider,
-		subLayerKey:         baseObjects.whitelist,
+		subLayerKey:         baseObjects.filters,
 		weight:              filterWeightMax(),
 		numFilterConditions: 1,
 		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&ifaceCondition)),
@@ -187,7 +187,7 @@ func permitWireGuardService(session uintptr, baseObjects *baseObjects) error {
 	//
 	filter := wtFwpmFilter0{
 		providerKey:         &baseObjects.provider,
-		subLayerKey:         baseObjects.whitelist,
+		subLayerKey:         baseObjects.filters,
 		weight:              filterWeightMax(),
 		numFilterConditions: uint32(len(conditions)),
 		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
@@ -273,157 +273,6 @@ func permitWireGuardService(session uintptr, baseObjects *baseObjects) error {
 	return nil
 }
 
-//
-// Permit all private nets and any combination of sender/receiver.
-//
-func permitLanIpv4(session uintptr, baseObjects *baseObjects) error {
-	privateNetworks := [4]wtFwpV4AddrAndMask{
-		{0x0a000000, 0xff000000},
-		{0xac100000, 0xfff00000},
-		{0xc0a80000, 0xffff0000},
-		{0xa9fe0000, 0xffff0000},
-	}
-
-	var conditions [8]wtFwpmFilterCondition0
-
-	//
-	// Repeating a condition type is evaluated as logical OR.
-	//
-
-	for idx, addr := range privateNetworks {
-		conditions[idx].fieldKey = cFWPM_CONDITION_IP_LOCAL_ADDRESS
-		conditions[idx].matchType = cFWP_MATCH_EQUAL
-		conditions[idx].conditionValue._type = cFWP_V4_ADDR_MASK
-		conditions[idx].conditionValue.value = uintptr(unsafe.Pointer(&addr))
-
-		conditions[4+idx].fieldKey = cFWPM_CONDITION_IP_REMOTE_ADDRESS
-		conditions[4+idx].matchType = cFWP_MATCH_EQUAL
-		conditions[4+idx].conditionValue._type = cFWP_V4_ADDR_MASK
-		conditions[4+idx].conditionValue.value = uintptr(unsafe.Pointer(&addr))
-	}
-
-	//
-	// Assemble the filter.
-	//
-	filter := wtFwpmFilter0{
-		providerKey:         &baseObjects.provider,
-		subLayerKey:         baseObjects.whitelist,
-		weight:              filterWeightMax(),
-		numFilterConditions: uint32(len(conditions)),
-		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
-		action: wtFwpmAction0{
-			_type: cFWP_ACTION_PERMIT,
-		},
-	}
-
-	filterId := uint64(0)
-
-	//
-	// #1 Permit outbound LAN traffic.
-	//
-	{
-		displayData, err := createWtFwpmDisplayData0("Permit outbound LAN traffic (IPv4)", "")
-		if err != nil {
-			return wrapErr(err)
-		}
-
-		filter.displayData = *displayData
-		filter.layerKey = cFWPM_LAYER_ALE_AUTH_CONNECT_V4
-
-		err = fwpmFilterAdd0(session, &filter, 0, &filterId)
-		if err != nil {
-			return wrapErr(err)
-		}
-	}
-
-	//
-	// #2 Permit inbound LAN traffic.
-	//
-	{
-		displayData, err := createWtFwpmDisplayData0("Permit inbound LAN traffic (IPv4)", "")
-		if err != nil {
-			return wrapErr(err)
-		}
-
-		filter.displayData = *displayData
-		filter.layerKey = cFWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V4
-
-		err = fwpmFilterAdd0(session, &filter, 0, &filterId)
-		if err != nil {
-			return wrapErr(err)
-		}
-	}
-
-	return nil
-}
-
-func permitLanIpv6(session uintptr, baseObjects *baseObjects) error {
-	privateNetwork := wtFwpV6AddrAndMask{[16]uint8{0xfe, 0x80}, 10}
-
-	var conditions [2]wtFwpmFilterCondition0
-
-	conditions[0].fieldKey = cFWPM_CONDITION_IP_LOCAL_ADDRESS
-	conditions[0].matchType = cFWP_MATCH_EQUAL
-	conditions[0].conditionValue._type = cFWP_V6_ADDR_MASK
-	conditions[0].conditionValue.value = uintptr(unsafe.Pointer(&privateNetwork))
-
-	conditions[1].fieldKey = cFWPM_CONDITION_IP_REMOTE_ADDRESS
-	conditions[1].matchType = cFWP_MATCH_EQUAL
-	conditions[1].conditionValue._type = cFWP_V6_ADDR_MASK
-	conditions[1].conditionValue.value = uintptr(unsafe.Pointer(&privateNetwork))
-
-	filter := wtFwpmFilter0{
-		providerKey:         &baseObjects.provider,
-		subLayerKey:         baseObjects.whitelist,
-		weight:              filterWeightMax(),
-		numFilterConditions: uint32(len(conditions)),
-		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
-		action: wtFwpmAction0{
-			_type: cFWP_ACTION_PERMIT,
-		},
-	}
-
-	filterId := uint64(0)
-
-	//
-	// #1 Permit outbound LAN traffic.
-	//
-	{
-		displayData, err := createWtFwpmDisplayData0("Permit outbound LAN traffic (IPv6)", "")
-		if err != nil {
-			return wrapErr(err)
-		}
-
-		filter.displayData = *displayData
-		filter.layerKey = cFWPM_LAYER_ALE_AUTH_CONNECT_V6
-
-		err = fwpmFilterAdd0(session, &filter, 0, &filterId)
-		if err != nil {
-			return wrapErr(err)
-		}
-	}
-
-	//
-	// #2 Permit inbound LAN traffic.
-	//
-	{
-		displayData, err := createWtFwpmDisplayData0("Permit inbound LAN traffic (IPv6)", "")
-		if err != nil {
-			return wrapErr(err)
-		}
-
-		filter.displayData = *displayData
-		filter.layerKey = cFWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V6
-
-		err = fwpmFilterAdd0(session, &filter, 0, &filterId)
-		if err != nil {
-			return wrapErr(err)
-		}
-	}
-
-	return nil
-}
-
 func permitLoopback(session uintptr, baseObjects *baseObjects) error {
 	condition := wtFwpmFilterCondition0{
 		fieldKey:  cFWPM_CONDITION_INTERFACE_TYPE,
@@ -436,7 +285,7 @@ func permitLoopback(session uintptr, baseObjects *baseObjects) error {
 
 	filter := wtFwpmFilter0{
 		providerKey:         &baseObjects.provider,
-		subLayerKey:         baseObjects.whitelist,
+		subLayerKey:         baseObjects.filters,
 		weight:              filterWeightMax(),
 		numFilterConditions: 1,
 		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&condition)),
@@ -558,7 +407,7 @@ func permitDhcpIpv4(session uintptr, baseObjects *baseObjects) error {
 			displayData:         *displayData,
 			providerKey:         &baseObjects.provider,
 			layerKey:            cFWPM_LAYER_ALE_AUTH_CONNECT_V4,
-			subLayerKey:         baseObjects.whitelist,
+			subLayerKey:         baseObjects.filters,
 			weight:              filterWeightMax(),
 			numFilterConditions: uint32(len(conditions)),
 			filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
@@ -605,7 +454,7 @@ func permitDhcpIpv4(session uintptr, baseObjects *baseObjects) error {
 			displayData:         *displayData,
 			providerKey:         &baseObjects.provider,
 			layerKey:            cFWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V4,
-			subLayerKey:         baseObjects.whitelist,
+			subLayerKey:         baseObjects.filters,
 			weight:              filterWeightMax(),
 			numFilterConditions: uint32(len(conditions)),
 			filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
@@ -677,7 +526,7 @@ func permitDhcpIpv6(session uintptr, baseObjects *baseObjects) error {
 			displayData:         *displayData,
 			providerKey:         &baseObjects.provider,
 			layerKey:            cFWPM_LAYER_ALE_AUTH_CONNECT_V6,
-			subLayerKey:         baseObjects.whitelist,
+			subLayerKey:         baseObjects.filters,
 			weight:              filterWeightMax(),
 			numFilterConditions: uint32(len(conditions)),
 			filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
@@ -734,7 +583,7 @@ func permitDhcpIpv6(session uintptr, baseObjects *baseObjects) error {
 			displayData:         *displayData,
 			providerKey:         &baseObjects.provider,
 			layerKey:            cFWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V6,
-			subLayerKey:         baseObjects.whitelist,
+			subLayerKey:         baseObjects.filters,
 			weight:              filterWeightMax(),
 			numFilterConditions: uint32(len(conditions)),
 			filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions)),
@@ -788,7 +637,7 @@ func permitNdp(session uintptr, baseObjects *baseObjects) error {
 func blockAllUnmatched(session uintptr, baseObjects *baseObjects) error {
 	filter := wtFwpmFilter0{
 		providerKey: &baseObjects.provider,
-		subLayerKey: baseObjects.whitelist,
+		subLayerKey: baseObjects.filters,
 		weight:      filterWeightMin(),
 		action: wtFwpmAction0{
 			_type: cFWP_ACTION_BLOCK,
@@ -873,10 +722,8 @@ func blockAllUnmatched(session uintptr, baseObjects *baseObjects) error {
 }
 
 // Block all DNS except what is matched by a permissive rule.
-func blockDnsNonTun(session uintptr, baseObjects *baseObjects, ifLuid uint64) error {
-	var conditions [2]wtFwpmFilterCondition0
-
-	conditions[0] = wtFwpmFilterCondition0{
+func blockDnsUnmatched(session uintptr, baseObjects *baseObjects) error {
+	condition := wtFwpmFilterCondition0{
 		fieldKey:  cFWPM_CONDITION_IP_REMOTE_PORT,
 		matchType: cFWP_MATCH_EQUAL,
 		conditionValue: wtFwpConditionValue0{
@@ -884,23 +731,13 @@ func blockDnsNonTun(session uintptr, baseObjects *baseObjects, ifLuid uint64) er
 			value: uintptr(53),
 		},
 	}
-	conditions[1] = wtFwpmFilterCondition0{
-		fieldKey:  cFWPM_CONDITION_IP_LOCAL_INTERFACE,
-		matchType: cFWP_MATCH_NOT_EQUAL,
-		conditionValue: wtFwpConditionValue0{
-			_type: cFWP_UINT64,
-			value: (uintptr)(unsafe.Pointer(&ifLuid)),
-		},
-	}
-
-	//TODO: we want to permit port 53 traffic coming from the wireguard service, in case people are using that port for tunneling.
 
 	filter := wtFwpmFilter0{
 		providerKey:         &baseObjects.provider,
-		subLayerKey:         baseObjects.blacklist,
-		weight:              filterWeightMax(),
-		numFilterConditions: uint32(len(conditions)),
-		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&conditions[0])),
+		subLayerKey:         baseObjects.filters,
+		weight:              filterWeightMin(),
+		numFilterConditions: 1,
+		filterCondition:     (*wtFwpmFilterCondition0)(unsafe.Pointer(&condition)),
 		action: wtFwpmAction0{
 			_type: cFWP_ACTION_BLOCK,
 		},