From 31e16f57b10ebb5c0f98d8dbf779774df342ed03 Mon Sep 17 00:00:00 2001 From: Samuel Jero Date: Fri, 21 Sep 2018 10:34:28 -0400 Subject: [PATCH] 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 --- .../rabbitmq/listener/MQEventHandlerTest.java | 6 +- .../onosproject/cluster/ClusterMetadata.java | 56 ++++- .../cluster/ClusterMetadataDiff.java | 23 ++ .../cluster/ClusterMetadataEventTest.java | 6 +- .../ClusterMetadataServiceAdapter.java | 3 +- .../cluster/ClusterMetadataTest.java | 6 +- ...onfigFileBasedClusterMetadataProvider.java | 15 +- .../impl/DefaultClusterMetadataProvider.java | 4 +- .../provider/lldp/impl/LldpLinkProvider.java | 27 ++- .../lldp/impl/LldpLinkProviderTest.java | 6 +- .../provider/lldpcommon/LinkDiscovery.java | 9 +- .../lldpcommon/LinkDiscoveryContext.java | 14 ++ .../NetworkConfigLinksProvider.java | 64 +++++- .../NetworkConfigLinksProviderTest.java | 6 +- tools/package/onos-run-karaf | 5 +- .../main/java/org/onlab/packet/ONOSLLDP.java | 200 +++++++++++++++++- .../java/org/onlab/packet/ONOSLLDPTest.java | 3 +- 17 files changed, 414 insertions(+), 39 deletions(-) diff --git a/apps/rabbitmq/src/test/java/org/onosproject/rabbitmq/listener/MQEventHandlerTest.java b/apps/rabbitmq/src/test/java/org/onosproject/rabbitmq/listener/MQEventHandlerTest.java index d8a2b8f19c..c1b35c3415 100644 --- a/apps/rabbitmq/src/test/java/org/onosproject/rabbitmq/listener/MQEventHandlerTest.java +++ b/apps/rabbitmq/src/test/java/org/onosproject/rabbitmq/listener/MQEventHandlerTest.java @@ -327,10 +327,10 @@ public class MQEventHandlerTest extends AbstractEventTest { @Override public InboundPacket inPacket() { - ONOSLLDP lldp = ONOSLLDP.onosLLDP(deviceService.getDevice(DID1) + ONOSLLDP lldp = ONOSLLDP.onosSecureLLDP(deviceService.getDevice(DID1) .id().toString(), - device.chassisId(), - (int) pd1.number().toLong()); + device.chassisId(), + (int) pd1.number().toLong(), "", "test"); Ethernet ethPacket = new Ethernet(); ethPacket.setEtherType(Ethernet.TYPE_LLDP); diff --git a/core/api/src/main/java/org/onosproject/cluster/ClusterMetadata.java b/core/api/src/main/java/org/onosproject/cluster/ClusterMetadata.java index e1f664ea12..bf3e4a0f18 100644 --- a/core/api/src/main/java/org/onosproject/cluster/ClusterMetadata.java +++ b/core/api/src/main/java/org/onosproject/cluster/ClusterMetadata.java @@ -46,6 +46,8 @@ public final class ClusterMetadata implements Provided { private final ControllerNode localNode; private final Set controllerNodes; private final Set storageNodes; + private final String clusterSecret; + public static final Funnel HASH_FUNNEL = new Funnel() { @Override @@ -61,6 +63,30 @@ public final class ClusterMetadata implements Provided { localNode = null; controllerNodes = null; storageNodes = null; + clusterSecret = null; + } + + /** + * @deprecated since 1.15. + * @param providerId the provider Id + * @param name The cluster Name + * @param localNode The local node + * @param controllerNodes Set of nodes in cluster + * @param storageNodes Set of storage nodes + */ + @Deprecated + public ClusterMetadata( + ProviderId providerId, + String name, + ControllerNode localNode, + Set controllerNodes, + Set storageNodes) { + this.providerId = checkNotNull(providerId); + this.name = checkNotNull(name); + this.localNode = localNode; + this.controllerNodes = ImmutableSet.copyOf(checkNotNull(controllerNodes)); + this.storageNodes = ImmutableSet.copyOf(checkNotNull(storageNodes)); + this.clusterSecret = "INSECURE!"; } public ClusterMetadata( @@ -68,17 +94,33 @@ public final class ClusterMetadata implements Provided { String name, ControllerNode localNode, Set controllerNodes, - Set storageNodes) { + Set storageNodes, + String clusterSecret) { this.providerId = checkNotNull(providerId); this.name = checkNotNull(name); this.localNode = localNode; this.controllerNodes = ImmutableSet.copyOf(checkNotNull(controllerNodes)); this.storageNodes = ImmutableSet.copyOf(checkNotNull(storageNodes)); + this.clusterSecret = clusterSecret; + } + + /** + * @deprecated since 1.15. + * @param name The cluster Name + * @param localNode The local node + * @param controllerNodes Set of nodes in cluster + * @param storageNodes Set of storage nodes + */ + @Deprecated + public ClusterMetadata( + String name, ControllerNode localNode, Set controllerNodes, Set storageNodes) { + this(new ProviderId("none", "none"), name, localNode, controllerNodes, storageNodes, "INSECURE!"); } public ClusterMetadata( - String name, ControllerNode localNode, Set controllerNodes, Set storageNodes) { - this(new ProviderId("none", "none"), name, localNode, controllerNodes, storageNodes); + String name, ControllerNode localNode, Set controllerNodes, Set storageNodes, + String clusterSecret) { + this(new ProviderId("none", "none"), name, localNode, controllerNodes, storageNodes, clusterSecret); } @Override @@ -140,6 +182,14 @@ public final class ClusterMetadata implements Provided { return Collections.emptySet(); } + /** + * Returns the cluster's shared secret. + * @return key. + */ + public String getClusterSecret() { + return clusterSecret; + } + @Override public String toString() { return MoreObjects.toStringHelper(ClusterMetadata.class) diff --git a/core/api/src/main/java/org/onosproject/cluster/ClusterMetadataDiff.java b/core/api/src/main/java/org/onosproject/cluster/ClusterMetadataDiff.java index acad25fbe7..e9f73b40a8 100644 --- a/core/api/src/main/java/org/onosproject/cluster/ClusterMetadataDiff.java +++ b/core/api/src/main/java/org/onosproject/cluster/ClusterMetadataDiff.java @@ -37,6 +37,7 @@ public class ClusterMetadataDiff { private final ClusterMetadata newValue; private final Set nodesAdded; private final Set nodesRemoved; + private final boolean secretChanged; public ClusterMetadataDiff(ClusterMetadata oldValue, ClusterMetadata newValue) { this.oldValue = oldValue; @@ -51,6 +52,20 @@ public class ClusterMetadataDiff { .stream() .map(ControllerNode::id) .collect(Collectors.toSet()); + + boolean haveOldSecret = (oldValue != null && oldValue.getClusterSecret() != null); + boolean haveNewSecret = (newValue != null && newValue.getClusterSecret() != null); + + if (!haveOldSecret && haveNewSecret) { + secretChanged = true; + } else if (haveOldSecret && haveNewSecret && + !oldValue.getClusterSecret().equals(newValue.getClusterSecret())) { + secretChanged = true; + } else if (haveOldSecret && !haveNewSecret) { + secretChanged = true; + } else { + secretChanged = false; + } } /** @@ -69,6 +84,14 @@ public class ClusterMetadataDiff { return nodesRemoved; } + /** + * Returns whether the cluster-wide shared secret changed. + * @return whether the cluster secret changed + */ + public boolean clusterSecretChanged() { + return secretChanged; + } + /** * Returns a mapping of all partition diffs. * @return partition diffs. diff --git a/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataEventTest.java b/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataEventTest.java index 49ff25fdfc..f0cb6bcfb9 100644 --- a/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataEventTest.java +++ b/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataEventTest.java @@ -38,11 +38,11 @@ public class ClusterMetadataEventTest { private final ControllerNode n2 = new DefaultControllerNode(nid2, IpAddress.valueOf("10.0.0.2"), 9876); private final ClusterMetadata metadata1 = - new ClusterMetadata("foo", n1, ImmutableSet.of(), ImmutableSet.of(n1)); + new ClusterMetadata("foo", n1, ImmutableSet.of(), ImmutableSet.of(n1), "test"); private final ClusterMetadata metadata2 = - new ClusterMetadata("bar", n1, ImmutableSet.of(), ImmutableSet.of(n1, n2)); + new ClusterMetadata("bar", n1, ImmutableSet.of(), ImmutableSet.of(n1, n2), "test"); private final ClusterMetadata metadata3 = - new ClusterMetadata("baz", n1, ImmutableSet.of(), ImmutableSet.of(n2)); + new ClusterMetadata("baz", n1, ImmutableSet.of(), ImmutableSet.of(n2), "test"); private final ClusterMetadataEvent event1 = new ClusterMetadataEvent(ClusterMetadataEvent.Type.METADATA_CHANGED, metadata1, time1); diff --git a/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataServiceAdapter.java b/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataServiceAdapter.java index f4f6100b69..8715475068 100644 --- a/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataServiceAdapter.java +++ b/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataServiceAdapter.java @@ -31,7 +31,8 @@ public class ClusterMetadataServiceAdapter implements ClusterMetadataService { "test-cluster", new DefaultControllerNode(nid, addr), Sets.newHashSet(), - Sets.newHashSet()); + Sets.newHashSet(), + "test-secret"); } @Override diff --git a/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataTest.java b/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataTest.java index 1b323639a1..04077eafad 100644 --- a/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataTest.java +++ b/core/api/src/test/java/org/onosproject/cluster/ClusterMetadataTest.java @@ -39,11 +39,11 @@ public class ClusterMetadataTest { new DefaultControllerNode(nid2, IpAddress.valueOf("10.0.0.2"), 9876); private final ClusterMetadata metadata1 = - new ClusterMetadata("foo", n1, ImmutableSet.of(), ImmutableSet.of(n1)); + new ClusterMetadata("foo", n1, ImmutableSet.of(), ImmutableSet.of(n1), ""); private final ClusterMetadata sameAsMetadata1 = - new ClusterMetadata("foo", n1, ImmutableSet.of(), ImmutableSet.of(n1)); + new ClusterMetadata("foo", n1, ImmutableSet.of(), ImmutableSet.of(n1), ""); private final ClusterMetadata metadata2 = - new ClusterMetadata("bar", n1, ImmutableSet.of(n1), ImmutableSet.of(n1, n2)); + new ClusterMetadata("bar", n1, ImmutableSet.of(n1), ImmutableSet.of(n1, n2), ""); private final ProviderId defaultProvider = new ProviderId("none", "none"); /** diff --git a/core/net/src/main/java/org/onosproject/cluster/impl/ConfigFileBasedClusterMetadataProvider.java b/core/net/src/main/java/org/onosproject/cluster/impl/ConfigFileBasedClusterMetadataProvider.java index d5ef23293e..de7a3e143c 100644 --- a/core/net/src/main/java/org/onosproject/cluster/impl/ConfigFileBasedClusterMetadataProvider.java +++ b/core/net/src/main/java/org/onosproject/cluster/impl/ConfigFileBasedClusterMetadataProvider.java @@ -119,6 +119,7 @@ public class ConfigFileBasedClusterMetadataProvider implements ClusterMetadataPr .stream() .map(this::toPrototype) .collect(Collectors.toSet())); + prototype.setClusterSecret(metadata.getClusterSecret()); return prototype; } @@ -274,7 +275,8 @@ public class ConfigFileBasedClusterMetadataProvider implements ClusterMetadataPr metadata.getStorage() .stream() .map(node -> new DefaultControllerNode(getNodeId(node), getNodeHost(node), getNodePort(node))) - .collect(Collectors.toSet())), + .collect(Collectors.toSet()), + metadata.getClusterSecret()), version); } catch (IOException e) { throw new IllegalArgumentException(e); @@ -307,6 +309,7 @@ public class ConfigFileBasedClusterMetadataProvider implements ClusterMetadataPr private NodePrototype node; private Set controller = Sets.newHashSet(); private Set storage = Sets.newHashSet(); + private String clusterSecret; public String getName() { return name; @@ -339,6 +342,14 @@ public class ConfigFileBasedClusterMetadataProvider implements ClusterMetadataPr public void setStorage(Set storage) { this.storage = storage; } + + public void setClusterSecret(String clusterSecret) { + this.clusterSecret = clusterSecret; + } + + public String getClusterSecret() { + return clusterSecret; + } } private static class NodePrototype { @@ -379,4 +390,4 @@ public class ConfigFileBasedClusterMetadataProvider implements ClusterMetadataPr this.port = port; } } -} \ No newline at end of file +} diff --git a/core/net/src/main/java/org/onosproject/cluster/impl/DefaultClusterMetadataProvider.java b/core/net/src/main/java/org/onosproject/cluster/impl/DefaultClusterMetadataProvider.java index a6f24ae671..35a56a8403 100644 --- a/core/net/src/main/java/org/onosproject/cluster/impl/DefaultClusterMetadataProvider.java +++ b/core/net/src/main/java/org/onosproject/cluster/impl/DefaultClusterMetadataProvider.java @@ -20,6 +20,7 @@ import java.net.InetAddress; import java.net.NetworkInterface; import java.util.Collections; import java.util.Set; +import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -73,7 +74,8 @@ public class DefaultClusterMetadataProvider implements ClusterMetadataProvider { ControllerNode localNode = new DefaultControllerNode(new NodeId(localIp), IpAddress.valueOf(localIp), DEFAULT_ONOS_PORT); ClusterMetadata metadata = new ClusterMetadata( - PROVIDER_ID, "default", localNode, ImmutableSet.of(), ImmutableSet.of()); + PROVIDER_ID, "default", localNode, ImmutableSet.of(), ImmutableSet.of(), + UUID.randomUUID().toString()); long version = System.currentTimeMillis(); cachedMetadata.set(new Versioned<>(metadata, version)); providerRegistry.register(this); 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 87ca7d757c..3ea56af672 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 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv 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 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv 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 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv 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 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv 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 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv useBddp = newUseBddp; probeRate = newProbeRate; staleLinkAge = newStaleLinkAge; + maxDiscoveryDelayMs = newDiscoveryDelay; if (!wasEnabled && enabled) { enable(); @@ -337,7 +348,7 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv } } - log.info(FORMAT, enabled, useBddp, probeRate, staleLinkAge); + log.info(FORMAT, enabled, useBddp, probeRate, staleLinkAge, maxDiscoveryDelayMs); } /** @@ -795,6 +806,16 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv public String fingerprint() { return buildSrcMac(); } + + @Override + public String lldpSecret() { + return clusterMetadataService.getClusterMetadata().getClusterSecret(); + } + + @Override + public long maxDiscoveryDelay() { + return maxDiscoveryDelayMs; + } } static final EnumSet 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 54bd151f28..8389d0919d 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 @@ public class LldpLinkProviderTest { @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 317fda8e27..3d69235c84 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 @@ public class LinkDiscovery implements TimerTask { } 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 @@ public class LinkDiscovery implements TimerTask { } 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 e4a025e0d0..a325b95e13 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 @@ public interface LinkDiscoveryContext { * @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 a15857f510..e811f96201 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 @@ public class NetworkConfigLinksProvider 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 discoverers = new ConcurrentHashMap<>(); @@ -265,8 +271,29 @@ public class NetworkConfigLinksProvider 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 @@ public class NetworkConfigLinksProvider 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 @@ public class NetworkConfigLinksProvider 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 117201a063..13081f08a6 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 @@ public class NetworkConfigLinksProviderTest { @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); diff --git a/tools/package/onos-run-karaf b/tools/package/onos-run-karaf index b0c371f170..ac588269c1 100755 --- a/tools/package/onos-run-karaf +++ b/tools/package/onos-run-karaf @@ -54,12 +54,13 @@ if [ ! -d $ONOS_DIR -o "$oldMD5" != "$newMD5" -o -d $ONOS_DIR -a -n "$clean" ]; [ -d $ONOS_DIR/config ] || mkdir -p $ONOS_DIR/config cat > $ONOS_DIR/config/cluster.json <<-EOF { - "name": "default", + "name": "default-$RANDOM", "node": { "id": "$IP", "ip": "$IP", "port": 9876 - } + }, + "clusterSecret": "$RANDOM" } EOF diff --git a/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java b/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java index b1e44c5a69..f0b981c486 100644 --- a/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java +++ b/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java @@ -21,9 +21,14 @@ import org.apache.commons.lang.ArrayUtils; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; import java.util.Arrays; import java.util.HashMap; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + import static org.onlab.packet.LLDPOrganizationalTLV.OUI_LENGTH; import static org.onlab.packet.LLDPOrganizationalTLV.SUBTYPE_LENGTH; @@ -39,10 +44,14 @@ public class ONOSLLDP extends LLDP { protected static final byte NAME_SUBTYPE = 1; protected static final byte DEVICE_SUBTYPE = 2; protected static final byte DOMAIN_SUBTYPE = 3; + protected static final byte TIMESTAMP_SUBTYPE = 4; + protected static final byte SIG_SUBTYPE = 5; private static final short NAME_LENGTH = OUI_LENGTH + SUBTYPE_LENGTH; private static final short DEVICE_LENGTH = OUI_LENGTH + SUBTYPE_LENGTH; private static final short DOMAIN_LENGTH = OUI_LENGTH + SUBTYPE_LENGTH; + private static final short TIMESTAMP_LENGTH = OUI_LENGTH + SUBTYPE_LENGTH; + private static final short SIG_LENGTH = OUI_LENGTH + SUBTYPE_LENGTH; private final HashMap opttlvs = Maps.newHashMap(); @@ -138,6 +147,28 @@ public class ONOSLLDP extends LLDP { this.setPortId(portTLV); } + public void setTimestamp(long timestamp) { + LLDPOrganizationalTLV tmtlv = opttlvs.get(TIMESTAMP_SUBTYPE); + if (tmtlv == null) { + return; + } + tmtlv.setInfoString(ByteBuffer.allocate(8).putLong(timestamp).array()); + tmtlv.setLength((short) (8 + TIMESTAMP_LENGTH)); + tmtlv.setSubType(TIMESTAMP_SUBTYPE); + tmtlv.setOUI(MacAddress.ONOS.oui()); + } + + public void setSig(byte[] sig) { + LLDPOrganizationalTLV sigtlv = opttlvs.get(SIG_SUBTYPE); + if (sigtlv == null) { + return; + } + sigtlv.setInfoString(sig); + sigtlv.setLength((short) (sig.length + SIG_LENGTH)); + sigtlv.setSubType(SIG_SUBTYPE); + sigtlv.setOUI(MacAddress.ONOS.oui()); + } + public LLDPOrganizationalTLV getNameTLV() { for (LLDPTLV tlv : this.getOptionalTLVList()) { if (tlv.getType() == LLDPOrganizationalTLV.ORGANIZATIONAL_TLV_TYPE) { @@ -153,7 +184,7 @@ public class ONOSLLDP extends LLDP { public LLDPOrganizationalTLV getDeviceTLV() { for (LLDPTLV tlv : this.getOptionalTLVList()) { if (tlv.getType() == LLDPOrganizationalTLV.ORGANIZATIONAL_TLV_TYPE) { - LLDPOrganizationalTLV orgTLV = (LLDPOrganizationalTLV) tlv; + LLDPOrganizationalTLV orgTLV = (LLDPOrganizationalTLV) tlv; if (orgTLV.getSubType() == DEVICE_SUBTYPE) { return orgTLV; } @@ -162,6 +193,30 @@ public class ONOSLLDP extends LLDP { return null; } + public LLDPOrganizationalTLV getTimestampTLV() { + for (LLDPTLV tlv : this.getOptionalTLVList()) { + if (tlv.getType() == LLDPOrganizationalTLV.ORGANIZATIONAL_TLV_TYPE) { + LLDPOrganizationalTLV orgTLV = (LLDPOrganizationalTLV) tlv; + if (orgTLV.getSubType() == TIMESTAMP_SUBTYPE) { + return orgTLV; + } + } + } + return null; + } + + public LLDPOrganizationalTLV getSigTLV() { + for (LLDPTLV tlv : this.getOptionalTLVList()) { + if (tlv.getType() == LLDPOrganizationalTLV.ORGANIZATIONAL_TLV_TYPE) { + LLDPOrganizationalTLV orgTLV = (LLDPOrganizationalTLV) tlv; + if (orgTLV.getSubType() == SIG_SUBTYPE) { + return orgTLV; + } + } + } + return null; + } + /** * Gets the TLV associated with remote probing. This TLV will be null if * remote probing is disabled. @@ -212,6 +267,24 @@ public class ONOSLLDP extends LLDP { portBB.position(), portBB.remaining(), StandardCharsets.UTF_8)); } + public long getTimestamp() { + LLDPOrganizationalTLV tlv = getTimestampTLV(); + if (tlv != null) { + ByteBuffer b = ByteBuffer.allocate(8).put(tlv.getInfoString()); + b.flip(); + return b.getLong(); + } + return 0; + } + + public byte[] getSig() { + LLDPOrganizationalTLV tlv = getSigTLV(); + if (tlv != null) { + return tlv.getInfoString(); + } + return null; + } + /** * Given an ethernet packet, determines if this is an LLDP from * ONOS and returns the device the LLDP came from. @@ -231,12 +304,14 @@ public class ONOSLLDP extends LLDP { /** * Creates a link probe for link discovery/verification. + * @deprecated since 1.15. Insecure, do not use. * * @param deviceId The device ID as a String * @param chassisId The chassis ID of the device * @param portNum Port number of port to send probe out of * @return ONOSLLDP probe message */ + @Deprecated public static ONOSLLDP onosLLDP(String deviceId, ChassisId chassisId, int portNum) { ONOSLLDP probe = new ONOSLLDP(NAME_SUBTYPE, DEVICE_SUBTYPE); probe.setPortId(portNum); @@ -251,13 +326,69 @@ public class ONOSLLDP extends LLDP { * @param deviceId The device ID as a String * @param chassisId The chassis ID of the device * @param portNum Port number of port to send probe out of + * @param secret LLDP secret + * @return ONOSLLDP probe message + */ + public static ONOSLLDP onosSecureLLDP(String deviceId, ChassisId chassisId, int portNum, String secret) { + ONOSLLDP probe = null; + if (secret == null) { + probe = new ONOSLLDP(NAME_SUBTYPE, DEVICE_SUBTYPE); + } else { + probe = new ONOSLLDP(NAME_SUBTYPE, DEVICE_SUBTYPE, TIMESTAMP_SUBTYPE, SIG_SUBTYPE); + } + probe.setPortId(portNum); + probe.setDevice(deviceId); + probe.setChassisId(chassisId); + + if (secret != null) { + /* Secure Mode */ + long ts = System.currentTimeMillis(); + probe.setTimestamp(ts); + byte[] sig = createSig(deviceId, portNum, ts, secret); + if (sig == null) { + return null; + } + probe.setSig(sig); + sig = null; + } + return probe; + } + + /** + * Creates a link probe for link discovery/verification. + * @deprecated since 1.15. Insecure, do not use. + * + * @param deviceId The device ID as a String + * @param chassisId The chassis ID of the device + * @param portNum Port number of port to send probe out of * @param portDesc Port description of port to send probe out of * @return ONOSLLDP probe message */ + @Deprecated public static ONOSLLDP onosLLDP(String deviceId, ChassisId chassisId, int portNum, String portDesc) { - ONOSLLDP probe = onosLLDP(deviceId, chassisId, portNum); + addPortDesc(probe, portDesc); + return probe; + } + /** + * Creates a link probe for link discovery/verification. + * + * @param deviceId The device ID as a String + * @param chassisId The chassis ID of the device + * @param portNum Port number of port to send probe out of + * @param portDesc Port description of port to send probe out of + * @param secret LLDP secret + * @return ONOSLLDP probe message + */ + public static ONOSLLDP onosSecureLLDP(String deviceId, ChassisId chassisId, int portNum, String portDesc, + String secret) { + ONOSLLDP probe = onosSecureLLDP(deviceId, chassisId, portNum, secret); + addPortDesc(probe, portDesc); + return probe; + } + + private static void addPortDesc(ONOSLLDP probe, String portDesc) { if (portDesc != null && !portDesc.isEmpty()) { byte[] bPortDesc = portDesc.getBytes(StandardCharsets.UTF_8); @@ -270,7 +401,70 @@ public class ONOSLLDP extends LLDP { .setValue(bPortDesc); probe.addOptionalTLV(portDescTlv); } - return probe; + } + + private static byte[] createSig(String deviceId, int portNum, long timestamp, String secret) { + byte[] pnb = ByteBuffer.allocate(8).putLong(portNum).array(); + byte[] tmb = ByteBuffer.allocate(8).putLong(timestamp).array(); + + try { + SecretKeySpec signingKey = new SecretKeySpec(secret.getBytes(StandardCharsets.UTF_8), "HmacSHA256"); + Mac mac = Mac.getInstance("HmacSHA256"); + mac.init(signingKey); + mac.update(deviceId.getBytes()); + mac.update(pnb); + mac.update(tmb); + byte[] sig = mac.doFinal(); + return sig; + } catch (NoSuchAlgorithmException e) { + return null; + } catch (InvalidKeyException e) { + return null; + } + } + + private static boolean verifySig(byte[] sig, String deviceId, int portNum, long timestamp, String secret) { + byte[] nsig = createSig(deviceId, portNum, timestamp, secret); + if (nsig == null) { + return false; + } + + if (!ArrayUtils.isSameLength(nsig, sig)) { + return false; + } + + boolean fail = false; + for (int i = 0; i < nsig.length; i++) { + if (sig[i] != nsig[i]) { + fail = true; + } + } + if (fail) { + return false; + } + return true; + } + + public static boolean verify(ONOSLLDP probe, String secret, long maxDelay) { + if (secret == null) { + return true; + } + + String deviceId = probe.getDeviceString(); + int portNum = probe.getPort(); + long timestamp = probe.getTimestamp(); + byte[] sig = probe.getSig(); + + if (deviceId == null || sig == null) { + return false; + } + + if (timestamp + maxDelay <= System.currentTimeMillis() || + timestamp > System.currentTimeMillis()) { + return false; + } + + return verifySig(sig, deviceId, portNum, timestamp, secret); } } diff --git a/utils/misc/src/test/java/org/onlab/packet/ONOSLLDPTest.java b/utils/misc/src/test/java/org/onlab/packet/ONOSLLDPTest.java index 268b5f5f51..b2494b54b9 100644 --- a/utils/misc/src/test/java/org/onlab/packet/ONOSLLDPTest.java +++ b/utils/misc/src/test/java/org/onlab/packet/ONOSLLDPTest.java @@ -30,8 +30,9 @@ public class ONOSLLDPTest { private static final Integer PORT_NUMBER = 2; private static final Integer PORT_NUMBER_2 = 98761234; private static final String PORT_DESC = "Ethernet1"; + private static final String TEST_SECRET = "test"; - private ONOSLLDP onoslldp = ONOSLLDP.onosLLDP(DEVICE_ID, CHASSIS_ID, PORT_NUMBER, PORT_DESC); + private ONOSLLDP onoslldp = ONOSLLDP.onosSecureLLDP(DEVICE_ID, CHASSIS_ID, PORT_NUMBER, PORT_DESC, TEST_SECRET); /** * Tests port number and getters.