From 36ee52aa6e044057066fbcca3e0f250b96b8ba00 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 10 Jul 2023 15:42:06 +0200 Subject: [PATCH 01/12] Print out a warning if we detect local IP addresses during connections Reference #1055 --- .../games647/fastlogin/bukkit/InetUtils.java | 38 +++++++++++++++++++ .../protocollib/VerifyResponseTask.java | 27 +++++++++---- 2 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java new file mode 100644 index 00000000..d83dfb9e --- /dev/null +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java @@ -0,0 +1,38 @@ +package com.github.games647.fastlogin.bukkit; + +import java.net.InetAddress; + +public class InetUtils { + + private InetUtils() { + // Utility + } + + /** + * Verifies if the given IP address is from the local network + * + * @param address IP address + * @return true if address is from local network or even from the device itself (loopback) + */ + public static boolean isLocalAddress(InetAddress address) { + // Loopback addresses like 127.0.* (IPv4) or [::1] (IPv6) + return address.isLoopbackAddress() + // Example: 10.0.0.0, 172.16.0.0, 192.168.0.0, fec0::/10 (deprecated) + // Ref: https://en.wikipedia.org/wiki/IP_address#Private_addresses + || address.isSiteLocalAddress() + // Example: 169.254.0.0/16, fe80::/10 + // Ref: https://en.wikipedia.org/wiki/IP_address#Address_autoconfiguration + || address.isLinkLocalAddress() + // non deprecated unique site-local that java doesn't check yet -> fc00::/7 + || isIPv6UniqueSiteLocal(address); + } + + private static boolean isIPv6UniqueSiteLocal(InetAddress address) { + // ref: https://en.wikipedia.org/wiki/Unique_local_address + + // currently undefined but could be used in the near future fc00::/8 + return (address.getAddress()[0] & 0xFF) == 0xFC + // in use for unique site-local fd00::/8 + || (address.getAddress()[0] & 0xFF) == 0xFD; + } +} diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java index ef0bda01..8b8aaf7f 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java @@ -46,6 +46,7 @@ import com.github.games647.craftapi.model.skin.SkinProperty; import com.github.games647.craftapi.resolver.MojangResolver; import com.github.games647.fastlogin.bukkit.BukkitLoginSession; import com.github.games647.fastlogin.bukkit.FastLoginBukkit; +import com.github.games647.fastlogin.bukkit.InetUtils; import com.github.games647.fastlogin.bukkit.listener.protocollib.packet.ClientPublicKey; import lombok.val; import org.bukkit.entity.Player; @@ -74,7 +75,7 @@ public class VerifyResponseTask implements Runnable { static { ENCRYPTION_CLASS = MinecraftReflection.getMinecraftClass( - "util." + ENCRYPTION_CLASS_NAME, ENCRYPTION_CLASS_NAME + "util." + ENCRYPTION_CLASS_NAME, ENCRYPTION_CLASS_NAME ); } @@ -149,10 +150,20 @@ public class VerifyResponseTask implements Runnable { } else { //user tried to fake an authentication disconnect( - "invalid-session", - "GameProfile {} ({}) tried to log in with an invalid session. ServerId: {}", - session.getRequestUsername(), socketAddress, serverId + "invalid-session", + "GameProfile {} ({}) tried to log in with an invalid session. ServerId: {}", + requestedUsername, address, serverId ); + + if (InetUtils.isLocalAddress(address)) { + plugin.getLog().warn("The incoming request for player {} uses a local IP address. " + + "This indicates the use of reverse-proxy like HAProxy, TCPShield, BungeeCord, Velocity, " + + "etc. By default, configurable, this plugin requests Mojang to verify the connecting IP " + + "to this server with the one used to log into Minecraft to prevent MITM attacks. In " + + "order to work this security feature, the actual client IP needs to be forwarding " + + "(keyword IP forwarding). This process will also be useful for other server " + + "features like IP banning, so that it doesn't ban the proxy IP", requestedUsername); + } } } catch (IOException ioEx) { disconnect("error-kick", "Failed to connect to session server", ioEx); @@ -217,15 +228,15 @@ public class VerifyResponseTask implements Runnable { try { // Try to get the old (pre MC 1.16.4) encryption method encryptMethod = FuzzyReflection.fromClass(networkManagerClass) - .getMethodByParameters("a", SecretKey.class); + .getMethodByParameters("a", SecretKey.class); } catch (IllegalArgumentException exception) { // Get the new encryption method encryptMethod = FuzzyReflection.fromClass(networkManagerClass) - .getMethodByParameters("a", Cipher.class, Cipher.class); + .getMethodByParameters("a", Cipher.class, Cipher.class); // Get the needed Cipher helper method (used to generate ciphers from login key) cipherMethod = FuzzyReflection.fromClass(ENCRYPTION_CLASS) - .getMethodByParameters("a", int.class, Key.class); + .getMethodByParameters("a", int.class, Key.class); } } @@ -276,7 +287,7 @@ public class VerifyResponseTask implements Runnable { EquivalentConverter converter = BukkitConverters.getWrappedPublicKeyDataConverter(); val wrappedKey = Optional.ofNullable(clientKey).map(key -> - new WrappedProfileKeyData(clientKey.expiry(), clientKey.key(), clientKey.signature()) + new WrappedProfileKeyData(clientKey.expiry(), clientKey.key(), clientKey.signature()) ); startPacket.getOptionals(converter).write(0, wrappedKey); From 6ab81b1a2fcf22b69aea81bd48be1312c0367ac0 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 09:55:24 +0200 Subject: [PATCH 02/12] Fix formatting errors --- .../games647/fastlogin/bukkit/InetUtils.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java index d83dfb9e..78600a4f 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/InetUtils.java @@ -1,8 +1,33 @@ +/* + * SPDX-License-Identifier: MIT + * + * The MIT License (MIT) + * + * Copyright (c) 2015-2023 games647 and contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ package com.github.games647.fastlogin.bukkit; import java.net.InetAddress; -public class InetUtils { +public final class InetUtils { private InetUtils() { // Utility From ee465e381c69544eca71add6ab5499d8a787c0ab Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 10:10:03 +0200 Subject: [PATCH 03/12] Rephrase invalid session warning to make it more clear --- .../protocollib/VerifyResponseTask.java | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java index 8b8aaf7f..3b8ba923 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java @@ -72,6 +72,13 @@ public class VerifyResponseTask implements Runnable { private static final String ENCRYPTION_CLASS_NAME = "MinecraftEncryption"; private static final Class ENCRYPTION_CLASS; + private static final String ADDRESS_VERIFY_WARNING = "This indicates the use of reverse-proxy like HAProxy, " + + "TCPShield, BungeeCord, Velocity, etc. " + + "By default (configurable in the config) this plugin requests Mojang to verify the connecting IP " + + "to this server with the one used to log into Minecraft to prevent MITM attacks. In " + + "order to work this security feature, the actual client IP needs to be forwarding " + + "(keyword IP forwarding). This process will also be useful for other server " + + "features like IP banning, so that it doesn't ban the proxy IP."; static { ENCRYPTION_CLASS = MinecraftReflection.getMinecraftClass( @@ -151,18 +158,25 @@ public class VerifyResponseTask implements Runnable { //user tried to fake an authentication disconnect( "invalid-session", - "GameProfile {} ({}) tried to log in with an invalid session. ServerId: {}", - requestedUsername, address, serverId + "Session server rejected incoming connection for GameProfile {} ({}). Possible reasons are" + + "1) Client IP address contacting Mojang and server during server join were different " + + "(Do you use a reverse proxy? -> Enable IP forwarding, " + + "or disable the feature in the config). " + + "2) Player is offline, but tried to bypass the authentication" + + "3) Client uses an outdated username for connecting (Fix: Restart client)", + requestedUsername, address ); if (InetUtils.isLocalAddress(address)) { - plugin.getLog().warn("The incoming request for player {} uses a local IP address. " - + "This indicates the use of reverse-proxy like HAProxy, TCPShield, BungeeCord, Velocity, " - + "etc. By default, configurable, this plugin requests Mojang to verify the connecting IP " - + "to this server with the one used to log into Minecraft to prevent MITM attacks. In " - + "order to work this security feature, the actual client IP needs to be forwarding " - + "(keyword IP forwarding). This process will also be useful for other server " - + "features like IP banning, so that it doesn't ban the proxy IP", requestedUsername); + plugin.getLog().warn( + "The incoming request for player {} uses a local IP address", + requestedUsername + ); + plugin.getLog().warn(ADDRESS_VERIFY_WARNING); + } else { + plugin.getLog().warn("If you think this is an error, please verify that the incoming " + + "IP address {} is not associated with a server hosting company.", address); + plugin.getLog().warn(ADDRESS_VERIFY_WARNING); } } } catch (IOException ioEx) { From f06ace2f5b1671b327aac25231c7d4023f89503c Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 15:09:16 +0200 Subject: [PATCH 04/12] Add dependency graph to help GitHub find dependencies --- .github/workflows/maven.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index e30e67f7..457af499 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -41,3 +41,6 @@ jobs: # Run non-interactive, package (with compile+test), # ignore snapshot updates, because they are likely to have breaking changes, enforce checksums run: mvn test --batch-mode --threads 2.0C --no-snapshot-updates --strict-checksums --file pom.xml + + - name: Update dependency graph + uses: advanced-security/maven-dependency-submission-action@v3.0.2 From f9ca3187756a54a13633907bb8f4cd8473faaf92 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 15:13:40 +0200 Subject: [PATCH 05/12] Apply Java version selection after CodeQL init --- .github/workflows/codeql-analysis.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index dbd9d464..419442cf 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -20,6 +20,8 @@ jobs: # Environment runs-on: ubuntu-latest + timeout-minutes: ${{ (matrix.language == 'swift' && 120) || 360 }} + permissions: actions: read contents: read @@ -35,6 +37,12 @@ jobs: - name: Checkout repository uses: actions/checkout@v3 + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v2 + with: + languages: ${{ matrix.language }} + # Setup Java - name: Set up JDK uses: actions/setup-java@v3 @@ -43,12 +51,6 @@ jobs: java-version: 19 cache: 'maven' - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v2 - with: - languages: ${{ matrix.language }} - # Auto build attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild From f2fd51d4e27a2c756b0b77fe88ef241cd1136f25 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 15:17:29 +0200 Subject: [PATCH 06/12] Use suggestion dependency version --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 457af499..11fb1deb 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -43,4 +43,4 @@ jobs: run: mvn test --batch-mode --threads 2.0C --no-snapshot-updates --strict-checksums --file pom.xml - name: Update dependency graph - uses: advanced-security/maven-dependency-submission-action@v3.0.2 + uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 From dbd12742fa44e3ce38458791cd2dcb51460ccc8a Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 15:47:02 +0200 Subject: [PATCH 07/12] Only upload dependency graph on push --- .github/workflows/maven.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 11fb1deb..55b7237b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -43,4 +43,5 @@ jobs: run: mvn test --batch-mode --threads 2.0C --no-snapshot-updates --strict-checksums --file pom.xml - name: Update dependency graph + if: ${{ github.event_name == 'push' }} uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 From 610c49d62cb09dd6fc4a3f22190d5d7cf726d5f9 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 15:49:36 +0200 Subject: [PATCH 08/12] Enable write for dependency graph --- .github/workflows/maven.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 55b7237b..54aab9c5 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -21,7 +21,7 @@ jobs: # Environment image - always use the newest OS runs-on: ubuntu-latest permissions: - contents: read + contents: write # Run steps steps: @@ -44,4 +44,4 @@ jobs: - name: Update dependency graph if: ${{ github.event_name == 'push' }} - uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 + uses: advanced-security/maven-dependency-submission-action@v3.0.2 From 44d365b15d05d4fdd423e5ae81e02a0d00bb9fcb Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 15:52:54 +0200 Subject: [PATCH 09/12] Use java dot file to help autobuild select the correct version --- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/maven.yml | 2 +- .java-version | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 .java-version diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 419442cf..154014e0 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -48,7 +48,7 @@ jobs: uses: actions/setup-java@v3 with: distribution: 'temurin' - java-version: 19 + java-version-file: '.java-version' cache: 'maven' # Auto build attempts to build any compiled languages (C/C++, C#, or Java). diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 54aab9c5..f8798a15 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -33,7 +33,7 @@ jobs: uses: actions/setup-java@v3 with: distribution: 'temurin' - java-version: 19 + java-version-file: '.java-version' cache: 'maven' # Build and test (included in package) diff --git a/.java-version b/.java-version new file mode 100644 index 00000000..d6b24041 --- /dev/null +++ b/.java-version @@ -0,0 +1 @@ +19 From 053b51e96b6e81736df7f9408e0bef1b312ffd36 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 16:17:57 +0200 Subject: [PATCH 10/12] Relax java build version requirement GitHub action seems to always download Java 19 even though it is already downloaded and installed. This could also help with running the CodeQL action. --- .java-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.java-version b/.java-version index d6b24041..98d9bcb7 100644 --- a/.java-version +++ b/.java-version @@ -1 +1 @@ -19 +17 From ecbf07c0b210b06a1414e76e2c13b83ed5a15d77 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 16:26:52 +0200 Subject: [PATCH 11/12] Manually start build process in CodeQL --- .github/workflows/codeql-analysis.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 154014e0..9a9cd234 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -51,10 +51,11 @@ jobs: java-version-file: '.java-version' cache: 'maven' - # Auto build attempts to build any compiled languages (C/C++, C#, or Java). - # If this step fails, then you should remove it and run the build manually (see below) - - name: Autobuild - uses: github/codeql-action/autobuild@v2 + # Manually start the autobuild process, because autobuild always selects Java 8 as build toolchain, but + # we are doing cross-crompiling from a newer Java version + - name: Build with Maven + # Extracted from autobuild + run: mvn clean package -f "pom.xml" -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip -DskipTests -Dmaven.test.skip.exec -Dlicense.skip=true -Drat.skip=true -Dspotless.check.skip=true -t /home/runner/.m2/toolchains.xml - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v2 From 5cd10fd79574c26892769dd3e729190a4a57ac15 Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 17 Jul 2023 17:04:28 +0200 Subject: [PATCH 12/12] Run CodeQL after main build to leverage caching --- .github/workflows/codeql-analysis.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 9a9cd234..ffba61e6 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -4,11 +4,11 @@ name: "CodeQL" on: - # Scan only for push on the primary branch for now - push: - branches: [ main ] - pull_request: - branches: [ main ] + workflow_run: + workflows: ["Maven Build"] + branches: [main] + types: + - completed jobs: # job i @@ -55,7 +55,7 @@ jobs: # we are doing cross-crompiling from a newer Java version - name: Build with Maven # Extracted from autobuild - run: mvn clean package -f "pom.xml" -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip -DskipTests -Dmaven.test.skip.exec -Dlicense.skip=true -Drat.skip=true -Dspotless.check.skip=true -t /home/runner/.m2/toolchains.xml + run: mvn package -f "pom.xml" --batch-mode -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip -DskipTests -Dmaven.test.skip.exec -Dlicense.skip=true -Drat.skip=true -Dspotless.check.skip=true -t /home/runner/.m2/toolchains.xml - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v2