[grid] node registration (#7949)

This PR introduces a --registration-secret flag which is used to check whether a Node is trusted (where 'trusted' means knows the secret) or not when registering. Failure to provide the right secret will cause a registration failure and so will not be sent commands.

java/server/src/org/openqa/selenium/grid/commands/Hub.java

@@ -122,7 +122,8 @@ public Executable configure(String... args) {
           tracer,
           bus,
           clientFactory,
-          sessions);
+          sessions,
+          null);
       handler.addHandler(distributor);
       Router router = new Router(tracer, clientFactory, sessions, distributor);
 

java/server/src/org/openqa/selenium/grid/commands/Standalone.java

@@ -145,15 +145,16 @@ public Executable configure(String... args) {
 
       SessionMap sessions = new LocalSessionMap(tracer, bus);
       combinedHandler.addHandler(sessions);
-      Distributor distributor = new LocalDistributor(tracer, bus, clientFactory, sessions);
+      Distributor distributor = new LocalDistributor(tracer, bus, clientFactory, sessions, null);
       combinedHandler.addHandler(distributor);
       Router router = new Router(tracer, clientFactory, sessions, distributor);
 
       LocalNode.Builder nodeBuilder = LocalNode.builder(
           tracer,
           bus,
           clientFactory,
-          localhost)
+          localhost,
+          null)
           .maximumConcurrentSessions(Runtime.getRuntime().availableProcessors() * 3);
 
       new NodeOptions(config).configure(tracer, clientFactory, nodeBuilder);

java/server/src/org/openqa/selenium/grid/commands/security.txt

@@ -1,3 +1,12 @@
+Selenium 4 has a number of changes aimed at improving the security posture
+of running your own infrastructure. The ability to communicate to a Selenium
+Server (whether it is acting as a Standalone or Grid) is discussed in `Secure
+Communication` and restricting Grid Nodes to only those you trust is covered
+in `Node Registration.`
+
+1. Secure Communication
+=======================
+
 Selenium Grid by default communicates over HTTP. This is fine for a lot
 of use cases, especially if everything is contained within the firewall
 and against test sites with testing data. However, if your server is
@@ -85,4 +94,49 @@ you add it to the cacert truststore which is by default, $JAVA_HOME/jre/lib/secu
 
 More information can be found at:
 
-* MiniCA: https://github.com/jsha/minica
+* MiniCA: https://github.com/jsha/minica
+
+2. Node Registration
+====================
+
+Selenium Grid distributes script execution to worker nodes. Those nodes self-register
+with the Grid Distributor process when they start and the Distributor trusts that
+they should be getting work. This is fine in highly secured networks where everything
+is trusted, but aside from that you really want to make sure that the node is one you
+control and not a rouge node. In order to do this, you start the Distributor, Router and
+Node servers with the new `--registration-secret` command-line argument.
+
+Using the same examples as above, they become the following with this additional layer
+of security.
+
+```
+  java -jar selenium.jar \
+       distributor \
+       --https-private-key /path/to/key.pkcs8 \
+       --https-certificate /path/to/cert.pem \
+       -s https://sessions.grid.com:5556 \
+       --registration-secret cheese \
+```
+
+```
+  java -jar selenium.jar \
+       router \
+       --https-private-key /path/to/key.pkcs8 \
+       --https-certificate /path/to/cert.pem \
+       -s https://sessions.grid.com:5556 \
+       -d https://distributor.grid.com:5553 \
+       --registration-secret cheese
+```
+
+```
+  java -jar selenium.jar \
+       node \
+       --https-private-key /path/to/key.pkcs8 \
+       --https-certificate /path/to/cert.pem \
+       --detect-drivers \
+       --registration-secret cheese
+```
+Note: If the registration-secret from the Node to the Distributor (or Router) is
+incorrect, there is nothing to indicate that returned to the Node. There is an ERROR
+level log entry produced on the servers. You should be monitoring the logs for that
+and raise alarms as deemed appropriate.

java/server/src/org/openqa/selenium/grid/component/HealthCheck.java

@@ -27,10 +27,18 @@ public interface HealthCheck {
   class Result {
     private final boolean isAlive;
     private final String message;
+    private final String registrationSecret;
 
     public Result(boolean isAlive, String message) {
       this.isAlive = isAlive;
       this.message = Objects.requireNonNull(message, "Message must be set");
+      this.registrationSecret = null;
+    }
+
+    public Result(boolean isAlive, String message, String registrationSecret) {
+      this.isAlive = isAlive;
+      this.message = Objects.requireNonNull(message, "Message must be set");
+      this.registrationSecret = registrationSecret;
     }
 
     public boolean isAlive() {
@@ -41,4 +49,4 @@ public String getMessage() {
       return message;
     }
   }
-}
+}

java/server/src/org/openqa/selenium/grid/data/NodeAddedEvent.java

@@ -0,0 +1,32 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.grid.data;
+
+import org.openqa.selenium.events.Event;
+import org.openqa.selenium.events.Type;
+
+import java.util.UUID;
+
+public class NodeAddedEvent extends Event {
+
+  public static final Type NODE_ADDED = new Type("node-added");
+
+    public NodeAddedEvent(UUID nodeId) {
+    super(NODE_ADDED, nodeId);
+  }
+}

java/server/src/org/openqa/selenium/grid/data/NodeRejectedEvent.java

@@ -0,0 +1,32 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.grid.data;
+
+import org.openqa.selenium.events.Event;
+import org.openqa.selenium.events.Type;
+
+import java.net.URI;
+
+public class NodeRejectedEvent extends Event {
+
+  public static final Type NODE_REJECTED = new Type("node-rejected");
+
+  public NodeRejectedEvent(URI uri) {
+    super(NODE_REJECTED, uri);
+  }
+}

java/server/src/org/openqa/selenium/grid/data/NodeRemovedEvent.java

@@ -0,0 +1,32 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.grid.data;
+
+import org.openqa.selenium.events.Event;
+import org.openqa.selenium.events.Type;
+
+import java.util.UUID;
+
+public class NodeRemovedEvent extends Event {
+
+  public static final Type NODE_REMOVED = new Type("node-removed");
+
+  public NodeRemovedEvent(UUID nodeId) {
+    super(NODE_REMOVED, nodeId);
+  }
+}

java/server/src/org/openqa/selenium/grid/data/NodeStatus.java

@@ -31,12 +31,7 @@
 
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-import java.util.UUID;
+import java.util.*;
 
 public class NodeStatus {
 
@@ -45,20 +40,23 @@ public class NodeStatus {
   private final int maxSessionCount;
   private final Map<Capabilities, Integer> stereotypes;
   private final Set<Active> snapshot;
+  private final String registrationSecret;
 
   public NodeStatus(
       UUID nodeId,
       URI externalUri,
       int maxSessionCount,
       Map<Capabilities, Integer> stereotypes,
-      Collection<Active> snapshot) {
+      Collection<Active> snapshot,
+      String registrationSecret) {
     this.nodeId = Objects.requireNonNull(nodeId);
     this.externalUri = Objects.requireNonNull(externalUri);
     Preconditions.checkArgument(maxSessionCount > 0, "Max session count must be greater than 0.");
     this.maxSessionCount = maxSessionCount;
 
     this.stereotypes = ImmutableMap.copyOf(Objects.requireNonNull(stereotypes));
     this.snapshot = ImmutableSet.copyOf(Objects.requireNonNull(snapshot));
+    this.registrationSecret = registrationSecret;
   }
 
   public boolean hasCapacity() {
@@ -89,6 +87,9 @@ public Set<Active> getCurrentSessions() {
     return snapshot;
   }
 
+  public String getRegistrationSecret() {
+    return registrationSecret;
+  }
 
   @Override
   public boolean equals(Object o) {
@@ -101,7 +102,8 @@ public boolean equals(Object o) {
            Objects.equals(this.externalUri, that.externalUri) &&
            this.maxSessionCount == that.maxSessionCount &&
            Objects.equals(this.stereotypes, that.stereotypes) &&
-           Objects.equals(this.snapshot, that.snapshot);
+           Objects.equals(this.snapshot, that.snapshot) &&
+           Objects.equals(this.registrationSecret, that.registrationSecret);
   }
 
   @Override
@@ -110,12 +112,14 @@ public int hashCode() {
   }
 
   private Map<String, Object> toJson() {
-    return ImmutableMap.of(
-        "id", nodeId,
-        "uri", externalUri,
-        "maxSessions", maxSessionCount,
-        "stereotypes", asCapacity(stereotypes),
-        "sessions", snapshot);
+    return new ImmutableMap.Builder<String, Object>()
+        .put("id", nodeId)
+        .put("uri", externalUri)
+        .put("maxSessions", maxSessionCount)
+        .put("stereotypes", asCapacity(stereotypes))
+        .put("sessions", snapshot)
+        .put("registrationSecret", Optional.ofNullable(registrationSecret))
+        .build();
   }
 
   private List<Map<String, Object>> asCapacity(Map<Capabilities, Integer> toConvert) {
@@ -142,7 +146,8 @@ public static NodeStatus fromJson(Map<String, Object> raw) {
           new URI((String) raw.get("uri")),
           ((Number) raw.get("maxSessions")).intValue(),
           readCapacityNamed(raw, "stereotypes"),
-          sessions);
+          sessions,
+          ((String) raw.get("registrationSecret")));
     } catch (URISyntaxException e) {
       throw new JsonException(e);
     }

java/server/src/org/openqa/selenium/grid/distributor/BUILD.bazel

@@ -14,6 +14,7 @@ java_library(
         "//java/server/src/org/openqa/selenium/grid/data",
         "//java/server/src/org/openqa/selenium/grid/node",
         "//java/server/src/org/openqa/selenium/grid/node/remote",
+        "//java/server/src/org/openqa/selenium/grid/sessionmap/remote",
         "//java/server/src/org/openqa/selenium/grid/web",
         artifact("com.google.guava:guava"),
     ],

java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java

@@ -114,13 +114,14 @@ public Executable configure(String... args) {
 
       SessionMap sessions = new SessionMapOptions(config).getSessionMap();
 
+      BaseServerOptions serverOptions = new BaseServerOptions(config);
+
       Distributor distributor = new LocalDistributor(
           tracer,
           bus,
           clientFactory,
-          sessions);
-
-      BaseServerOptions serverOptions = new BaseServerOptions(config);
+          sessions,
+          serverOptions.getRegistrationSecret());
 
       Server<?> server = new NettyServer(serverOptions, distributor);
       server.start();

java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java

@@ -29,10 +29,7 @@
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.concurrent.Regularly;
 import org.openqa.selenium.events.EventBus;
-import org.openqa.selenium.grid.data.CreateSessionRequest;
-import org.openqa.selenium.grid.data.CreateSessionResponse;
-import org.openqa.selenium.grid.data.DistributorStatus;
-import org.openqa.selenium.grid.data.NodeStatus;
+import org.openqa.selenium.grid.data.*;
 import org.openqa.selenium.grid.distributor.Distributor;
 import org.openqa.selenium.grid.node.Node;
 import org.openqa.selenium.grid.node.remote.RemoteNode;
@@ -86,17 +83,20 @@ public class LocalDistributor extends Distributor {
   private final SessionMap sessions;
   private final Regularly hostChecker = new Regularly("distributor host checker");
   private final Map<UUID, Collection<Runnable>> allChecks = new ConcurrentHashMap<>();
+  private final String registrationSecret;
 
   public LocalDistributor(
       Tracer tracer,
       EventBus bus,
       HttpClient.Factory clientFactory,
-      SessionMap sessions) {
+      SessionMap sessions,
+      String registrationSecret) {
     super(tracer, clientFactory);
     this.tracer = Objects.requireNonNull(tracer);
     this.bus = Objects.requireNonNull(bus);
     this.clientFactory = Objects.requireNonNull(clientFactory);
     this.sessions = Objects.requireNonNull(sessions);
+    this.registrationSecret = registrationSecret;
 
     bus.addListener(NODE_STATUS, event -> refresh(event.getData(NodeStatus.class)));
   }
@@ -277,6 +277,13 @@ private void refresh(NodeStatus status) {
 
     LOG.fine("Refreshing: " + status.getUri());
 
+    // check registrationSecret and stop processing if it doesn't match
+    if (! Objects.equals(status.getRegistrationSecret(), registrationSecret)) {
+      LOG.severe(String.format("Node at %s failed to send correct registration secret. Node NOT registered.", status.getUri()));
+      bus.fire(new NodeRejectedEvent(status.getUri()));
+      return;
+    }
+
     // Iterate over the available nodes to find a match.
     Lock writeLock = lock.writeLock();
     writeLock.lock();
@@ -336,6 +343,7 @@ private LocalDistributor add(Node node, NodeStatus status) {
       LOG.log(Level.WARNING, "Unable to process host", t);
     } finally {
       writeLock.unlock();
+      bus.fire(new NodeAddedEvent(node.getId()));
     }
 
     return this;
@@ -350,6 +358,7 @@ public void remove(UUID nodeId) {
       allChecks.getOrDefault(nodeId, new ArrayList<>()).forEach(hostChecker::remove);
     } finally {
       writeLock.unlock();
+      bus.fire(new NodeRemovedEvent(nodeId));
     }
   }