[CORD-1751] Preventing attacks on DHCP-Relay
Change-Id: I46f7ba2490994e71c9f7d881cbe44785720f1e37
diff --git a/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/DhcpRelayManager.java b/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/DhcpRelayManager.java
index 116f8e4..d4ae20a 100644
--- a/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/DhcpRelayManager.java
+++ b/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/DhcpRelayManager.java
@@ -23,9 +23,11 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -43,7 +45,6 @@
import org.onlab.packet.IPv6;
import org.onlab.packet.Ip4Address;
import org.onlab.packet.MacAddress;
-import org.onlab.packet.TpPort;
import org.onlab.packet.UDP;
import org.onlab.packet.VlanId;
import org.onlab.util.Tools;
@@ -61,7 +62,6 @@
import org.onosproject.dhcprelay.store.DhcpRelayStore;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.Device;
-import org.onosproject.net.DeviceId;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
import org.onosproject.net.config.Config;
@@ -73,18 +73,9 @@
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.behaviour.Pipeliner;
import org.onosproject.net.device.DeviceEvent;
import org.onosproject.net.device.DeviceListener;
import org.onosproject.net.device.DeviceService;
-import org.onosproject.net.flow.criteria.Criterion;
-import org.onosproject.net.flow.criteria.UdpPortCriterion;
-import org.onosproject.net.flowobjective.DefaultForwardingObjective;
-import org.onosproject.net.flowobjective.FlowObjectiveService;
-import org.onosproject.net.flowobjective.ForwardingObjective;
-import org.onosproject.net.flowobjective.Objective;
-import org.onosproject.net.flowobjective.ObjectiveContext;
-import org.onosproject.net.flowobjective.ObjectiveError;
import org.onosproject.net.host.HostService;
import org.onosproject.net.intf.Interface;
import org.onosproject.net.intf.InterfaceService;
@@ -98,14 +89,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.collect.HashMultimap;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Multimap;
import static org.onosproject.net.config.basics.SubjectFactories.APP_SUBJECT_FACTORY;
-import static org.onosproject.net.flowobjective.Objective.Operation.ADD;
-import static org.onosproject.net.flowobjective.Objective.Operation.REMOVE;
/**
* DHCP Relay Agent Application Component.
@@ -116,36 +102,10 @@
public static final String DHCP_RELAY_APP = "org.onosproject.dhcprelay";
public static final String ROUTE_STORE_IMPL =
"org.onosproject.routeservice.store.RouteStoreImpl";
- private static final TrafficSelector DHCP_SERVER_SELECTOR = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV4)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpDst(TpPort.tpPort(UDP.DHCP_SERVER_PORT))
- .build();
- private static final TrafficSelector DHCP_CLIENT_SELECTOR = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV4)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpDst(TpPort.tpPort(UDP.DHCP_CLIENT_PORT))
- .build();
- private static final TrafficSelector DHCP6_SERVER_SELECTOR = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV6)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpDst(TpPort.tpPort(UDP.DHCP_V6_SERVER_PORT))
- .build();
- private static final TrafficSelector DHCP6_CLIENT_SELECTOR = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV6)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpDst(TpPort.tpPort(UDP.DHCP_V6_CLIENT_PORT))
- .build();
- static final List<TrafficSelector> DHCP_SELECTORS = ImmutableList.of(
- DHCP_SERVER_SELECTOR,
- DHCP_CLIENT_SELECTOR,
- DHCP6_SERVER_SELECTOR,
- DHCP6_CLIENT_SELECTOR
- );
+
private static final TrafficSelector ARP_SELECTOR = DefaultTrafficSelector.builder()
.matchEthType(Ethernet.TYPE_ARP)
.build();
- private static final int IGNORE_CONTROL_PRIORITY = PacketPriority.CONTROL.priorityValue() + 1000;
private final Logger log = LoggerFactory.getLogger(getClass());
private final InternalConfigListener cfgListener = new InternalConfigListener();
@@ -201,9 +161,6 @@
protected ComponentConfigService compCfgService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected FlowObjectiveService flowObjectiveService;
-
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceService deviceService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY,
@@ -218,7 +175,6 @@
label = "Enable Address resolution protocol")
protected boolean arpEnabled = true;
- protected Multimap<DeviceId, VlanId> ignoredVlans = HashMultimap.create();
protected DeviceListener deviceListener = new InternalDeviceListener();
private DhcpRelayPacketProcessor dhcpRelayPacketProcessor = new DhcpRelayPacketProcessor();
private ApplicationId appId;
@@ -236,8 +192,6 @@
//add the packet processor
packetService.addProcessor(dhcpRelayPacketProcessor, PacketProcessor.director(0));
- // listen host event for dhcp server or the gateway
- requestDhcpPackets();
modified(context);
// Enable distribute route store
@@ -254,7 +208,6 @@
cfgService.removeListener(cfgListener);
factories.forEach(cfgService::unregisterConfigFactory);
packetService.removeProcessor(dhcpRelayPacketProcessor);
- cancelDhcpPackets();
cancelArpPackets();
compCfgService.unregisterProperties(getClass(), false);
deviceService.removeListener(deviceListener);
@@ -280,6 +233,12 @@
}
}
+ private static List<TrafficSelector> buildClientDhcpSelectors() {
+ return Streams.concat(Dhcp4HandlerImpl.DHCP_SELECTORS.stream(),
+ Dhcp6HandlerImpl.DHCP_SELECTORS.stream())
+ .collect(Collectors.toList());
+ }
+
/**
* Updates DHCP relay app configuration.
*/
@@ -318,7 +277,8 @@
v6Handler.setDefaultDhcpServerConfigs(defaultConfig.dhcpServerConfigs());
}
if (config instanceof IgnoreDhcpConfig) {
- updateIgnoreVlanRules((IgnoreDhcpConfig) config);
+ v4Handler.updateIgnoreVlanConfig((IgnoreDhcpConfig) config);
+ v6Handler.updateIgnoreVlanConfig((IgnoreDhcpConfig) config);
}
}
@@ -331,125 +291,11 @@
v6Handler.setDefaultDhcpServerConfigs(Collections.emptyList());
}
if (config instanceof IgnoreDhcpConfig) {
- ignoredVlans.forEach(((deviceId, vlanId) -> {
- processIgnoreVlanRule(deviceId, vlanId, REMOVE);
- }));
+ v4Handler.updateIgnoreVlanConfig(null);
+ v6Handler.updateIgnoreVlanConfig(null);
}
}
- private void updateIgnoreVlanRules(IgnoreDhcpConfig config) {
- config.ignoredVlans().forEach((deviceId, vlanId) -> {
- if (ignoredVlans.get(deviceId).contains(vlanId)) {
- // don't need to process if it already ignored
- return;
- }
- processIgnoreVlanRule(deviceId, vlanId, ADD);
- });
-
- ignoredVlans.forEach((deviceId, vlanId) -> {
- if (!config.ignoredVlans().get(deviceId).contains(vlanId)) {
- // not contains in new config, remove it
- processIgnoreVlanRule(deviceId, vlanId, REMOVE);
- }
- });
- }
-
- /**
- * Process the ignore rules.
- *
- * @param deviceId the device id
- * @param vlanId the vlan to be ignored
- * @param op the operation, ADD to install; REMOVE to uninstall rules
- */
- private void processIgnoreVlanRule(DeviceId deviceId, VlanId vlanId, Objective.Operation op) {
- TrafficTreatment dropTreatment = DefaultTrafficTreatment.emptyTreatment();
- dropTreatment.clearedDeferred();
- AtomicInteger installedCount = new AtomicInteger(DHCP_SELECTORS.size());
- DHCP_SELECTORS.forEach(trafficSelector -> {
- UdpPortCriterion udpDst = (UdpPortCriterion) trafficSelector.getCriterion(Criterion.Type.UDP_DST);
- int udpDstPort = udpDst.udpPort().toInt();
- TrafficSelector selector = DefaultTrafficSelector.builder(trafficSelector)
- .matchVlanId(vlanId)
- .build();
-
- ForwardingObjective.Builder builder = DefaultForwardingObjective.builder()
- .withFlag(ForwardingObjective.Flag.VERSATILE)
- .withSelector(selector)
- .withPriority(IGNORE_CONTROL_PRIORITY)
- .withTreatment(dropTreatment)
- .fromApp(appId);
-
-
- ObjectiveContext objectiveContext = new ObjectiveContext() {
- @Override
- public void onSuccess(Objective objective) {
- log.info("Ignore rule {} (Vlan id {}, device {}, UDP dst {})",
- op, vlanId, deviceId, udpDstPort);
- int countDown = installedCount.decrementAndGet();
- if (countDown != 0) {
- return;
- }
- switch (op) {
- case ADD:
-
- ignoredVlans.put(deviceId, vlanId);
- break;
- case REMOVE:
- ignoredVlans.remove(deviceId, vlanId);
- break;
- default:
- log.warn("Unsupported objective operation {}", op);
- break;
- }
- }
-
- @Override
- public void onError(Objective objective, ObjectiveError error) {
- log.warn("Can't {} ignore rule (vlan id {}, udp dst {}, device {}) due to {}",
- op, vlanId, udpDstPort, deviceId, error);
- }
- };
-
- ForwardingObjective fwd;
- switch (op) {
- case ADD:
- fwd = builder.add(objectiveContext);
- break;
- case REMOVE:
- fwd = builder.remove(objectiveContext);
- break;
- default:
- log.warn("Unsupported objective operation {}", op);
- return;
- }
-
- Device device = deviceService.getDevice(deviceId);
- if (device == null || !device.is(Pipeliner.class)) {
- log.warn("Device {} is not available now, wait until device is available", deviceId);
- return;
- }
- flowObjectiveService.apply(deviceId, fwd);
- });
- }
-
- /**
- * Request DHCP packet in via PacketService.
- */
- private void requestDhcpPackets() {
- DHCP_SELECTORS.forEach(trafficSelector -> {
- packetService.requestPackets(trafficSelector, PacketPriority.CONTROL, appId);
- });
- }
-
- /**
- * Cancel requested DHCP packets in via packet service.
- */
- private void cancelDhcpPackets() {
- DHCP_SELECTORS.forEach(trafficSelector -> {
- packetService.cancelPackets(trafficSelector, PacketPriority.CONTROL, appId);
- });
- }
-
/**
* Request ARP packet in via PacketService.
*/
@@ -663,14 +509,10 @@
Device device = event.subject();
switch (event.type()) {
case DEVICE_ADDED:
- deviceAdd(device.id());
- break;
- case DEVICE_REMOVED:
- ignoredVlans.removeAll(device.id());
+ updateIgnoreVlanConfigs();
break;
case DEVICE_AVAILABILITY_CHANGED:
deviceAvailabilityChanged(device);
-
default:
break;
}
@@ -678,23 +520,14 @@
private void deviceAvailabilityChanged(Device device) {
if (deviceService.isAvailable(device.id())) {
- deviceAdd(device.id());
- } else {
- ignoredVlans.removeAll(device.id());
+ updateIgnoreVlanConfigs();
}
}
- private void deviceAdd(DeviceId deviceId) {
+ private void updateIgnoreVlanConfigs() {
IgnoreDhcpConfig config = cfgService.getConfig(appId, IgnoreDhcpConfig.class);
- if (config == null) {
- log.debug("No ignoreVlan config found for {}. Do nothing.", deviceId);
- return;
- }
-
- Collection<VlanId> vlanIds = config.ignoredVlans().get(deviceId);
- vlanIds.forEach(vlanId -> {
- processIgnoreVlanRule(deviceId, vlanId, ADD);
- });
+ v4Handler.updateIgnoreVlanConfig(config);
+ v6Handler.updateIgnoreVlanConfig(config);
}
}
}