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/core/api/src/main/java/org/onosproject/cluster/ClusterMetadata.java b/core/api/src/main/java/org/onosproject/cluster/ClusterMetadata.java
index e1f664e..bf3e4a0 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 @@
private final ControllerNode localNode;
private final Set<ControllerNode> controllerNodes;
private final Set<Node> storageNodes;
+ private final String clusterSecret;
+
public static final Funnel<ClusterMetadata> HASH_FUNNEL = new Funnel<ClusterMetadata>() {
@Override
@@ -61,6 +63,30 @@
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<ControllerNode> controllerNodes,
+ Set<Node> 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 @@
String name,
ControllerNode localNode,
Set<ControllerNode> controllerNodes,
- Set<Node> storageNodes) {
+ Set<Node> 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<ControllerNode> controllerNodes, Set<Node> storageNodes) {
+ this(new ProviderId("none", "none"), name, localNode, controllerNodes, storageNodes, "INSECURE!");
}
public ClusterMetadata(
- String name, ControllerNode localNode, Set<ControllerNode> controllerNodes, Set<Node> storageNodes) {
- this(new ProviderId("none", "none"), name, localNode, controllerNodes, storageNodes);
+ String name, ControllerNode localNode, Set<ControllerNode> controllerNodes, Set<Node> storageNodes,
+ String clusterSecret) {
+ this(new ProviderId("none", "none"), name, localNode, controllerNodes, storageNodes, clusterSecret);
}
@Override
@@ -140,6 +182,14 @@
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 acad25f..e9f73b4 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 @@
private final ClusterMetadata newValue;
private final Set<ControllerNode> nodesAdded;
private final Set<NodeId> nodesRemoved;
+ private final boolean secretChanged;
public ClusterMetadataDiff(ClusterMetadata oldValue, ClusterMetadata newValue) {
this.oldValue = oldValue;
@@ -51,6 +52,20 @@
.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;
+ }
}
/**
@@ -70,6 +85,14 @@
}
/**
+ * 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 49ff25f..f0cb6bc 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 @@
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 f4f6100..8715475 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 @@
"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 1b32363..04077ea 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 @@
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");
/**