Secure LLDP-based Topology Detection
Current LLDP/BDDP-based Topology Detection is vulnerable to the
creation of fake links via forged, modified, or replayed LLDP packets.
This patch fixes this vulnerability by authenticating LLDP/BDDP packets
using a Message Authentication Code and adding a timestamp to prevent
replay. We use HMAC with SHA-256 has our Messge Authentication Code and
derive the key from the config/cluster.json file via the
ClusterMetadata class.
Change-Id: I01dd6edc5cffd6dfe274bcdb97189f2661a6c4f1
diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java
index 87ca7d7..3ea56af 100644
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java
@@ -101,7 +101,7 @@
private static final String FORMAT =
"Settings: enabled={}, useBDDP={}, probeRate={}, " +
- "staleLinkAge={}";
+ "staleLinkAge={}, maxLLDPage={}";
// When a Device/Port has this annotation, do not send out LLDP/BDDP
public static final String NO_LLDP = "no-lldp";
@@ -174,6 +174,12 @@
label = "Number of millis beyond which links will be considered stale")
private int staleLinkAge = DEFAULT_STALE_LINK_AGE;
+ private static final String PROP_DISCOVERY_DELAY = "maxLLDPAge";
+ private static final int DEFAULT_DISCOVERY_DELAY = 1000;
+ @Property(name = PROP_DISCOVERY_DELAY, intValue = DEFAULT_DISCOVERY_DELAY,
+ label = "Number of millis beyond which an LLDP packet will not be accepted")
+ private int maxDiscoveryDelayMs = DEFAULT_DISCOVERY_DELAY;
+
private final LinkDiscoveryContext context = new InternalDiscoveryContext();
private final InternalRoleListener roleListener = new InternalRoleListener();
private final InternalDeviceListener deviceListener = new InternalDeviceListener();
@@ -297,7 +303,7 @@
Dictionary<?, ?> properties = context != null ? context.getProperties() : new Properties();
boolean newEnabled, newUseBddp;
- int newProbeRate, newStaleLinkAge;
+ int newProbeRate, newStaleLinkAge, newDiscoveryDelay;
try {
String s = get(properties, PROP_ENABLED);
newEnabled = isNullOrEmpty(s) || Boolean.parseBoolean(s.trim());
@@ -311,12 +317,16 @@
s = get(properties, PROP_STALE_LINK_AGE);
newStaleLinkAge = isNullOrEmpty(s) ? staleLinkAge : Integer.parseInt(s.trim());
+ s = get(properties, PROP_DISCOVERY_DELAY);
+ newDiscoveryDelay = isNullOrEmpty(s) ? maxDiscoveryDelayMs : Integer.parseInt(s.trim());
+
} catch (NumberFormatException e) {
log.warn("Component configuration had invalid values", e);
newEnabled = enabled;
newUseBddp = useBddp;
newProbeRate = probeRate;
newStaleLinkAge = staleLinkAge;
+ newDiscoveryDelay = maxDiscoveryDelayMs;
}
boolean wasEnabled = enabled;
@@ -325,6 +335,7 @@
useBddp = newUseBddp;
probeRate = newProbeRate;
staleLinkAge = newStaleLinkAge;
+ maxDiscoveryDelayMs = newDiscoveryDelay;
if (!wasEnabled && enabled) {
enable();
@@ -337,7 +348,7 @@
}
}
- log.info(FORMAT, enabled, useBddp, probeRate, staleLinkAge);
+ log.info(FORMAT, enabled, useBddp, probeRate, staleLinkAge, maxDiscoveryDelayMs);
}
/**
@@ -795,6 +806,16 @@
public String fingerprint() {
return buildSrcMac();
}
+
+ @Override
+ public String lldpSecret() {
+ return clusterMetadataService.getClusterMetadata().getClusterSecret();
+ }
+
+ @Override
+ public long maxDiscoveryDelay() {
+ return maxDiscoveryDelayMs;
+ }
}
static final EnumSet<NetworkConfigEvent.Type> CONFIG_CHANGED
diff --git a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
index 54bd151..8389d09 100644
--- a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
+++ b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
@@ -651,9 +651,9 @@
@Override
public InboundPacket inPacket() {
- ONOSLLDP lldp = ONOSLLDP.onosLLDP(deviceService.getDevice(DID1).id().toString(),
- device.chassisId(),
- (int) pd1.number().toLong());
+ ONOSLLDP lldp = ONOSLLDP.onosSecureLLDP(deviceService.getDevice(DID1).id().toString(),
+ device.chassisId(),
+ (int) pd1.number().toLong(), "", "test");
Ethernet ethPacket = new Ethernet();
ethPacket.setEtherType(Ethernet.TYPE_LLDP);
diff --git a/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscovery.java b/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscovery.java
index 317fda8..3d69235 100644
--- a/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscovery.java
+++ b/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscovery.java
@@ -173,6 +173,12 @@
} else {
lt = eth.getEtherType() == Ethernet.TYPE_LLDP ?
Type.DIRECT : Type.INDIRECT;
+
+ /* Verify MAC in LLDP packets */
+ if (!ONOSLLDP.verify(onoslldp, context.lldpSecret(), context.maxDiscoveryDelay())) {
+ log.warn("LLDP Packet failed to validate!");
+ return true;
+ }
}
PortNumber srcPort = portNumber(onoslldp.getPort());
@@ -269,7 +275,8 @@
}
private ONOSLLDP getLinkProbe(Long portNumber, String portDesc) {
- return ONOSLLDP.onosLLDP(device.id().toString(), device.chassisId(), portNumber.intValue(), portDesc);
+ return ONOSLLDP.onosSecureLLDP(device.id().toString(), device.chassisId(), portNumber.intValue(), portDesc,
+ context.lldpSecret());
}
private void sendProbes(Long portNumber, String portDesc) {
diff --git a/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscoveryContext.java b/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscoveryContext.java
index e4a025e..a325b95 100644
--- a/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscoveryContext.java
+++ b/providers/lldpcommon/src/main/java/org/onosproject/provider/lldpcommon/LinkDiscoveryContext.java
@@ -81,4 +81,18 @@
* @return the cluster identifier
*/
String fingerprint();
+
+ /**
+ * Returns the cluster-wide MAC secret used to secure LLDP packets.
+ *
+ * @return the secret
+ */
+ String lldpSecret();
+
+ /**
+ * Returns the maximum delay in milliseconds between sending an LLDP packet and receiving it elsewhere.
+ *
+ * @return delay in ms
+ */
+ long maxDiscoveryDelay();
}
diff --git a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java
index a15857f..e811f96 100644
--- a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java
+++ b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java
@@ -107,6 +107,12 @@
label = "LLDP and BDDP probe rate specified in millis")
private int probeRate = DEFAULT_PROBE_RATE;
+ private static final String PROP_DISCOVERY_DELAY = "maxLLDPAge";
+ private static final int DEFAULT_DISCOVERY_DELAY = 1000;
+ @Property(name = PROP_DISCOVERY_DELAY, intValue = DEFAULT_DISCOVERY_DELAY,
+ label = "Number of millis beyond which an LLDP packet will not be accepted")
+ private int maxDiscoveryDelayMs = DEFAULT_DISCOVERY_DELAY;
+
// Device link discovery helpers.
protected final Map<DeviceId, LinkDiscovery> discoverers = new ConcurrentHashMap<>();
@@ -265,8 +271,29 @@
public DeviceService deviceService() {
return deviceService;
}
+
+ @Override
+ public String lldpSecret() {
+ return metadataService.getClusterMetadata().getClusterSecret();
+ }
+
+ @Override
+ public long maxDiscoveryDelay() {
+ return maxDiscoveryDelayMs;
+ }
}
+ // true if *NOT* this cluster's own probe.
+ private boolean isOthercluster(String mac) {
+ // if we are using DEFAULT_MAC, clustering hadn't initialized, so conservative 'yes'
+ String ourMac = context.fingerprint();
+ if (ProbedLinkProvider.defaultMac().equalsIgnoreCase(ourMac)) {
+ return true;
+ }
+ return !mac.equalsIgnoreCase(ourMac);
+ }
+
+ //doesn't validate. Used just to decide if this is expected link.
LinkKey extractLinkKey(PacketContext packetContext) {
Ethernet eth = packetContext.inPacket().parsed();
if (eth == null) {
@@ -287,6 +314,27 @@
return null;
}
+ private boolean verify(PacketContext packetContext) {
+ Ethernet eth = packetContext.inPacket().parsed();
+ if (eth == null) {
+ return false;
+ }
+
+ ONOSLLDP onoslldp = ONOSLLDP.parseONOSLLDP(eth);
+ if (onoslldp != null) {
+ if (!isOthercluster(eth.getSourceMAC().toString())) {
+ return false;
+ }
+
+ if (!ONOSLLDP.verify(onoslldp, context.lldpSecret(), context.maxDiscoveryDelay())) {
+ log.warn("LLDP Packet failed to validate!");
+ return false;
+ }
+ return true;
+ }
+ return false;
+ }
+
/**
* Removes after stopping discovery helper for specified device.
* @param deviceId device to remove
@@ -344,13 +392,15 @@
context.block();
}
} else {
- log.debug("Found link that was not in the configuration {}", linkKey);
- providerService.linkDetected(
- new DefaultLinkDescription(linkKey.src(),
- linkKey.dst(),
- Link.Type.DIRECT,
- DefaultLinkDescription.NOT_EXPECTED,
- DefaultAnnotations.EMPTY));
+ if (verify(context)) {
+ log.debug("Found link that was not in the configuration {}", linkKey);
+ providerService.linkDetected(
+ new DefaultLinkDescription(linkKey.src(),
+ linkKey.dst(),
+ Link.Type.DIRECT,
+ DefaultLinkDescription.NOT_EXPECTED,
+ DefaultAnnotations.EMPTY));
+ }
}
}
}
diff --git a/providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java b/providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java
index 117201a..13081f0 100644
--- a/providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java
+++ b/providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java
@@ -147,9 +147,9 @@
@Override
public InboundPacket inPacket() {
- ONOSLLDP lldp = ONOSLLDP.onosLLDP(src.deviceId().toString(),
- new ChassisId(),
- (int) src.port().toLong());
+ ONOSLLDP lldp = ONOSLLDP.onosSecureLLDP(src.deviceId().toString(),
+ new ChassisId(),
+ (int) src.port().toLong(), "", "test-secret");
Ethernet ethPacket = new Ethernet();
ethPacket.setEtherType(Ethernet.TYPE_LLDP);