Add config support for using file as forwarding secret (#712)

* add config support for using file as forwarding secret

* deprecate forwarding-secret and change default to forwarding-secret-file

* change forwarding-secret-file handling to a versioned system

proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java

@@ -422,6 +422,12 @@ public class VelocityConfiguration implements ProxyConfig {
       throw new RuntimeException("Default configuration file does not exist.");
     }
 
+    // Create the forwarding-secret file on first-time startup if it doesn't exist
+    Path defaultForwardingSecretPath = Path.of("forwarding.secret");
+    if (!path.toFile().exists() && !defaultForwardingSecretPath.toFile().exists()) {
+      Files.writeString(defaultForwardingSecretPath, generateRandomString(12));
+    }
+
     boolean mustResave = false;
     CommentedFileConfig config = CommentedFileConfig.builder(path)
         .defaultData(defaultConfigLocation)
@@ -442,14 +448,39 @@ public class VelocityConfiguration implements ProxyConfig {
     CommentedFileConfig defaultConfig = CommentedFileConfig.of(tmpFile, TomlFormat.instance());
     defaultConfig.load();
 
-    // Retrieve the forwarding secret. First, from environment variable, then from config.
+    // Whether or not this config is version 1.0 which uses the deprecated "forwarding-secret" parameter
+    boolean legacyConfig = config.getOrElse("config-version", "").equalsIgnoreCase("1.0");
+
+    String forwardingSecretString;
     byte[] forwardingSecret;
-    String forwardingSecretString = System.getenv()
+
-        .getOrDefault("VELOCITY_FORWARDING_SECRET", config.get("forwarding-secret"));
+    // Handle the previous (version 1.0) config
-    if (forwardingSecretString == null || forwardingSecretString.isEmpty()) {
+    // There is duplicate/old code here in effort to make the future commit which abandons legacy config handling
-      forwardingSecretString = generateRandomString(12);
+    // easier to implement. All that would be required is removing the if statement here and keeping the contents
-      config.set("forwarding-secret", forwardingSecretString);
+    // of the else block (with slight tidying).
-      mustResave = true;
+    if (legacyConfig) {
+      logger.warn("You are currently using a deprecated configuration version. The \"forwarding-secret\""
+          + " parameter has been recognized as a security concern and has been removed in config version 2.0."
+          + " It's recommended you rename your current \"velocity.toml\" to something else to allow Velocity"
+          + " to generate a config file of the new version. You may then configure that file as you normally would."
+          + " The only differences are the config-version and \"forwarding-secret\" has been replaced"
+          + " by \"forwarding-secret-file\".");
+
+      // Default legacy handling
+      forwardingSecretString = System.getenv()
+          .getOrDefault("VELOCITY_FORWARDING_SECRET", config.get("forwarding-secret"));
+      if (forwardingSecretString == null || forwardingSecretString.isEmpty()) {
+        forwardingSecretString = generateRandomString(12);
+        config.set("forwarding-secret", forwardingSecretString);
+        mustResave = true;
+      }
+    } else {
+      // New handling
+      forwardingSecretString = System.getenv().getOrDefault("VELOCITY_FORWARDING_SECRET", "");
+      if (forwardingSecretString.isEmpty()) {
+        String forwardSecretFile = config.getOrElse("forwarding-secret-file", "");
+        forwardingSecretString = String.join("", Files.readAllLines(Path.of(forwardSecretFile)));
+      }
     }
     forwardingSecret = forwardingSecretString.getBytes(StandardCharsets.UTF_8);
 
@@ -480,6 +511,14 @@ public class VelocityConfiguration implements ProxyConfig {
     Boolean kickExisting = config.getOrElse("kick-existing-players", false);
     Boolean enablePlayerAddressLogging = config.getOrElse("enable-player-address-logging", true);
 
+    // Throw an exception if the forwarding-secret file is empty and the proxy is using a 
+    // forwarding mode that requires it.
+    if (forwardingSecret.length == 0
+        && forwardingMode == PlayerInfoForwarding.MODERN
+        || forwardingMode == PlayerInfoForwarding.BUNGEEGUARD) {
+      throw new RuntimeException("The forwarding-secret file must not be empty.");
+    }
+
     return new VelocityConfiguration(
         bind,
         motd,

proxy/src/main/resources/default-velocity.toml

@@ -1,5 +1,5 @@
 # Config version. Do not change this
-config-version = "1.0"
+config-version = "2.0"
 
 # What port should the proxy be bound to? By default, we'll bind to all addresses on port 25577.
 bind = "0.0.0.0:25577"
@@ -36,8 +36,9 @@ prevent-client-proxy-connections = false
 #                  Velocity's native forwarding. Only applicable for Minecraft 1.13 or higher.
 player-info-forwarding-mode = "NONE"
 
-# If you are using modern or BungeeGuard IP forwarding, configure a unique secret here.
+# If you are using modern or BungeeGuard IP forwarding, configure a file that contains a unique secret here.
-forwarding-secret = ""
+# The file is expected to be UTF-8 encoded and not empty.
+forwarding-secret-file = "forwarding.secret"
 
 # Announce whether or not your server supports Forge. If you run a modded server, we
 # suggest turning this on.
@@ -151,4 +152,4 @@ port = 25577
 map = "Velocity"
 
 # Whether plugins should be shown in query response by default or not
-show-plugins = false
+show-plugins = false