From da52d0933832d68a13e83878562843de6dc74ed4 Mon Sep 17 00:00:00 2001 From: logsym <106287213+logsym@users.noreply.github.com> Date: Thu, 9 Jun 2022 03:28:14 -0400 Subject: [PATCH] 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/config/VelocityConfiguration.java | 53 ++++++++++++++++--- .../src/main/resources/default-velocity.toml | 9 ++-- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index f31be126b..25cf4dc08 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/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")); - if (forwardingSecretString == null || forwardingSecretString.isEmpty()) { - forwardingSecretString = generateRandomString(12); - config.set("forwarding-secret", forwardingSecretString); - mustResave = true; + + // Handle the previous (version 1.0) config + // There is duplicate/old code here in effort to make the future commit which abandons legacy config handling + // easier to implement. All that would be required is removing the if statement here and keeping the contents + // of the else block (with slight tidying). + 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, diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index 5ed6a7dcd..9abc5e4db 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/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. -forwarding-secret = "" +# If you are using modern or BungeeGuard IP forwarding, configure a file that contains a unique secret here. +# 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 \ No newline at end of file +show-plugins = false