Feature request: BungeeCord/Waterfall support

bigfinfrank commented 3 years ago

Is your feature request related to a problem? Please describe. As the plugin doesn't currently support BungeeCord or it's popular fork Waterfall, it's currently not possible to pick and choose connections to block from plugins placed in a server's proxy. This is rather upsetting as many plugins with Bungee versions that include the functionality Network Interceptor aims to block like unnecessary update checking and automatic updates have this same (in some cases) malicious functionality in the BungeeCord variants of their plugins, one example of this is PremiumVanish (the same functionality is probably present in SuperVanish).

Describe the solution you'd like I'd like for the plugin to have BungeeCord support or at least have an official Bungee version of the plugin available. I vaguely remember some plugins requiring functionality that is present in Waterfall and not BungeeCord in which case it would be nice to also use the functionality available through Waterfall (when available) if it would compliment the experience, somewhat similarly to how the security manager method is better compared to proxy-selector but it's not possible to use it in every scenario. It would be ideal for simplicity's sake to have the Bungee version of the plugin bundled in the same jar for neatness on the user's end if possible, but that's definitely a nit-picky preference thing.

Describe alternatives you've considered I've tried using external solutions but they can't give stack traces and are generally harder to use. It's also quite difficult to find the culprit for a certain unwanted connection in some cases and when you do sometimes it's not possible to block only that functionality as many plugins use Cloudflare and therefore the same set of proxied host IPs.

Additional Context N/A

mart-r commented 3 years ago

Would you be willing to test on a Bungee network (i.e a test network, not a production one)? I've got a test branch up: \<LINK ELIDED> That should enable Bungee support but I have no bungee server to test on.

SlimeDog commented 3 years ago

PremiumVanish (server) config.yml includes

UpdateChecker:
    # Should PV check for updates on spigot regularly? There is no automatic update; this just informs you
    Enable: true

Would not the same functionality in (bungee) configuration.yml solve the problem, in that specific case? Has that been suggested/requested?

The general case remains, of course.

SlimeDog commented 3 years ago

@bigfinfrank

Would you be willing to test on a Bungee network (i.e a test network, not a production one)? I've got a test branch up: \<LINK ELIDED> That should enable Bungee support but I have no bungee server to test on.

That was directed to you. We do not run a bungeecord server, nor are we familiar with any bungeecord plugins. Over to you.

bigfinfrank commented 3 years ago

Would you be willing to test on a Bungee network (i.e a test network, not a production one)? I've got a test branch up: \<LINK ELIDED> That should enable Bungee support but I have no bungee server to test on.

I'd happily be willing to test however I'm not familiar with compiling plugins/jars and after a bit of searching I ran into the command mvn clean package which led to a build failure, the log can be found on pastebin here. I'm not sure if I've made a mistake by running the incorrect command or if this is something on the plugin side of things. I also apologize for the slow response, my timezone is EST and I had work in the morning.

PremiumVanish (server) config.yml includes
UpdateChecker:
    # Should PV check for updates on spigot regularly? There is no automatic update; this just informs you
    Enable: true
Would not the same functionality in (bungee) configuration.yml solve the problem, in that specific case? Has that been suggested/requested?

The general case remains, of course.

I'm aware of this, I was simply giving an example Bungee plugin off the top of my head that shares the relevant functionality with its Spigot counterpart.

mart-r commented 3 years ago

I didn't mean for you to compile it. I was just inquiring whether or not you're be up for testing. I'm happy to provide a jar for you. Here it is: \<LINK ELIDED> EDIT: PS: Regardless of the name, the same jar should work for both Bungee and Spigot.

SlimeDog commented 3 years ago

I will verify on Spigot/Paper. Thanks.

SlimeDog commented 3 years ago

@bigfinfrank Please post the intercept.log file content here, as you test.

bigfinfrank commented 3 years ago

Encountering this error on startup. The config folder and file were created although config.yml is completely empty. Neither intercept.log nor intercept.log.lck were created either.

bigfinfrank commented 3 years ago

After restarting the server, the plugin added {} to config.yml on line 1, nothing else. Still with no intercept.log/intercept.log.lck.

EDIT: There's also a trailing new line creating a line 2.

SlimeDog commented 3 years ago

FWIW, still works fine on Spigot/Paper.

mart-r commented 3 years ago

Encountering this error on startup. The config folder and file were created although config.yml is completely empty. Neither intercept.log nor intercept.log.lck were created either.

Looks like I forgot to save the default config (on bungee). The following should fix that. \<LINK ELIDED>

bigfinfrank commented 3 years ago

Looks like I forgot to save the default config (on bungee). The following should fix that. \<LINK ELIDED>

Still encountering the same behavior except that now a restart doesn't add {} to the config. No intercept.log(or .lck). Using pastebin is a getting a bit annoying, I'm just going to attach the error here:

16:25:08 [WARNING] Error loading plugin NetworkInterceptor
java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
    at net.md_5.bungee.api.plugin.PluginManager.enablePlugin(PluginManager.java:341)
    at net.md_5.bungee.api.plugin.PluginManager.loadPlugins(PluginManager.java:250)
    at net.md_5.bungee.BungeeCord.start(BungeeCord.java:273)
    at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:67)
    at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15)
Caused by: java.lang.NullPointerException: Cannot read field "self" because "config" is null
    at net.md_5.bungee.config.YamlConfiguration.save(YamlConfiguration.java:65)
    at net.md_5.bungee.config.YamlConfiguration.save(YamlConfiguration.java:58)
    at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.saveDefaultConfig(BungeeNetworkInterceptor.java:59)
    at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.<init>(BungeeNetworkInterceptor.java:23)
    ... 10 more

EDIT: The error very slightly differs on Waterfall, I'm unsure if this will be useful or not but here it is:

[16:57:04 WARN]: Error loading plugin NetworkInterceptor
java.lang.reflect.InvocationTargetException: null
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:?]
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78) ~[?:?]
        at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:?]
        at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499) ~[?:?]
        at java.lang.reflect.Constructor.newInstance(Constructor.java:480) ~[?:?]
        at net.md_5.bungee.api.plugin.PluginManager.enablePlugin(PluginManager.java:395) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
        at net.md_5.bungee.api.plugin.PluginManager.loadPlugins(PluginManager.java:300) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
        at net.md_5.bungee.BungeeCord.start(BungeeCord.java:278) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
        at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:80) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
        at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
Caused by: java.lang.NullPointerException: Cannot read field "self" because "config" is null
        at net.md_5.bungee.config.YamlConfiguration.save(YamlConfiguration.java:65) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
        at net.md_5.bungee.config.YamlConfiguration.save(YamlConfiguration.java:58) ~[waterfall-1.17-443.jar:git:Waterfall-Bootstrap:1.17-R0.1-SNAPSHOT:34188b3:443]
        at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.saveDefaultConfig(BungeeNetworkInterceptor.java:59) ~[?:?]
        at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.<init>(BungeeNetworkInterceptor.java:23) ~[?:?]
        ... 10 more

bigfinfrank commented 3 years ago

After a bit of experimenting, copying the spigot config.yml into the blank Bungee config.yml results in the latest Bungee-2 variant of the plugin clearing config.yml on start up back to it being empty.

However, this isn't the case with the first Bungee version of the plugin you gave me, With the first version it removed all comments, empty lines, and whitespace ahead of hyphen-denoted lists, leaving the default configuration as simply:

methods:
- security-manager
- proxy-selector
enable-metrics: true
mode: deny
logging:
  enabled: true
  include-traces: false
  mode: file
blocking:
  enabled: false
mapping:
  enabled: true
  timer: 1000
trusted-plugins:
- none
targets:
- example.com
- example.org

Are you depending on a built in yaml parser? I know that BungeeCord's automatically removes comments and the main BungeeCord config.yml shuffles around values for whatever reason (however there is an open, in-progress PR to resolve this issue on BungeeCord's end- see https://github.com/SpigotMC/BungeeCord/pull/3067). If this is the case and you think this could be the cause of the issue, I do know that LuckPerms still supports comments on Bungee so Lucko may be able to provide a bit of insight here however I'm not sure if he's still involved as of 2.0.0.

EDIT (4:38 PM EST): Based on md-5's most recent comment on that PR, it seems like the reason this hasn't been pulled yet relates to case sensitive permissions which (correct me if I'm wrong) shouldn't be an issue for Network Interceptor. If this is the case, the creator of that PR, C4K3 may be willing to let you use their config implementation to get around the issue.

EDIT 2 (5:32 PM EST): Using the first bungee jar and manually editing the config also creates an intercept.log with this and nothing else:

Thu Jul 08 17:12:14 EDT 2021: Current Server version: git:BungeeCord-Bootstrap:1.17-R0.1-SNAPSHOT:c866619:1594
Thu Jul 08 17:12:14 EDT 2021: Current NetworkInterceptor version: 2.0.0-Bungee

mart-r commented 3 years ago

I see the error of my ways. I was overwriting the empty config on start. This one should fix that: \<LINK ELIDED>

PS: Pardon the hassle, I've never really made a Bungee plugin before.

bigfinfrank commented 3 years ago

Pardon the hassle, I've never really made a Bungee plugin before.

Not a problem.

config.yml now seems to be generating properly (you can see it here if you'd like to double check this). I've also made sure it's not getting overwritten by disabling and re-enabling metrics with restarts in-between.

intercept.log is present, however it doesn't seem to be logging connections, it only contains the version info which is as follows:


Thu Jul 08 17:43:50 EDT 2021: Current Server version: git:BungeeCord-Bootstrap:1.17-R0.1-SNAPSHOT:c866619:1594
Thu Jul 08 17:43:50 EDT 2021: Current NetworkInterceptor version: 2.0.0-Bungee-3

- intercept.log.lck looks like it's working properly; it's created and locked while BungeeCord is running and deleted when BungeeCord stops.
-  There's an error related to the security manager on startup which is as follows: 
```java
17:53:00 [INFO] [NetworkInterceptor] Blocking is not enabled
17:53:00 [INFO] [NetworkInterceptor] Using file logger
17:53:00 [INFO] [NetworkInterceptor] Interceptors: [security-manager, proxy-selector]
17:53:00 [SEVERE] [NetworkInterceptor] Exception occurred whilst enabling me.lucko.networkinterceptor.interceptors.SecurityManagerInterceptor
java.security.AccessControlException: Restricted Action
    at net.md_5.bungee.BungeeSecurityManager.checkPermission(BungeeSecurityManager.java:75)
    at java.base/java.lang.System.setSecurityManager0(System.java:374)
    at java.base/java.lang.System.setSecurityManager(System.java:358)
    at me.lucko.networkinterceptor.interceptors.SecurityManagerInterceptor.enable(SecurityManagerInterceptor.java:21)
    at me.lucko.networkinterceptor.common.CommonNetworkInterceptor.enable(CommonNetworkInterceptor.java:127)
    at me.lucko.networkinterceptor.common.CommonNetworkInterceptor.<init>(CommonNetworkInterceptor.java:58)
    at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.<init>(BungeeNetworkInterceptor.java:25)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
    at net.md_5.bungee.api.plugin.PluginManager.enablePlugin(PluginManager.java:341)
    at net.md_5.bungee.api.plugin.PluginManager.loadPlugins(PluginManager.java:250)
    at net.md_5.bungee.BungeeCord.start(BungeeCord.java:273)
    at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:67)
    at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15)
17:53:01 [INFO] [NetworkInterceptor] [ProxySelectorInterceptor] Replacing 'sun.net.spi.DefaultProxySelector' selector with logged variant.
17:53:01 [INFO] [NetworkInterceptor] bStats metrics enabled

After this it seems like other plugins are unable to make connections (even to localhost/127.0.0.1), despite blocking being set to false and the mode set to deny in config.yml. As an example, here is the relevant error from PremiumVanish:

17:53:17 [SEVERE] [PremiumVanish] Failed to connect to the database. Are your settings in the configuration.yml file correct? Does the database have a firewall? Does the MySQL-User have enough permissions? Message: Failed to initialize pool: java.lang.NoClassDefFoundError: org/bukkit/plugin/java/JavaPlugin
17:53:17 [INFO] Enabled plugin PremiumVanish version 2.7.0 by MyzelYam
17:53:17 [SEVERE] Exception in thread "ForkJoinPool.commonPool-worker-51"
17:53:17 [SEVERE] java.lang.NoClassDefFoundError: org/bukkit/plugin/java/JavaPlugin

17:53:17 [SEVERE]     at me.lucko.networkinterceptor.InterceptEvent.generateNonInternalStackTrace(InterceptEvent.java:79)

17:53:17 [SEVERE]     at me.lucko.networkinterceptor.InterceptEvent.<init>(InterceptEvent.java:24)

17:53:17 [SEVERE]     at me.lucko.networkinterceptor.interceptors.ProxySelectorInterceptor$LoggingSelector.select(ProxySelectorInterceptor.java:54)

17:53:17 [SEVERE]     at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:322)

17:53:17 [SEVERE]     at java.base/java.net.Socket.connect(Socket.java:645)

This does not occur when Network Interceptor isn't present and to be clear other plugins making connections are also blocked.

SlimeDog commented 3 years ago

Please comment out the security-manager processor in config.yml and restart.

bigfinfrank commented 3 years ago

Please comment out the security-manager processor in config.yml and restart.

The initial error from Network Interceptor isn't present but connections are still blocked (identical errors) after I commented out the security-manager method in the config like this:

methods:
#  - security-manager
  - proxy-selector

EDIT: Here is the startup information where the error would occur with security-manager enabled while it's disabled:

  18:03:35 [INFO] [NetworkInterceptor] Blocking is not enabled
18:03:35 [INFO] [NetworkInterceptor] Using file logger
18:03:35 [INFO] [NetworkInterceptor] Interceptors: [proxy-selector]
18:03:35 [INFO] [NetworkInterceptor] [ProxySelectorInterceptor] Replacing 'sun.net.spi.DefaultProxySelector' selector with logged variant.
18:03:35 [INFO] [NetworkInterceptor] bStats metrics enabled
18:03:35 [INFO] Loaded plugin NetworkInterceptor version 2.0.0-Bungee-3 by null

I should probably also note that with security-manager disabled, intercept.log has the same behavior (displaying version information but there isn't any connection logging).

SlimeDog commented 3 years ago

OK. It looks to me like Bungee has nailed down its security-manager in way(s) that Spigot and Paper have not. We can play with that later, but this allows us to focus on the blocking issue(s).

bigfinfrank commented 3 years ago

Experimenting with different method settings in config.yml further, I've figured out that connections are not blocked when there aren't any methods defined like this:

methods: []
#  - security-manager
#  - proxy-selector

In this case the startup information is as follows:

18:10:37 [INFO] [NetworkInterceptor] Blocking is not enabled
18:10:37 [INFO] [NetworkInterceptor] Using file logger
18:10:37 [INFO] [NetworkInterceptor] No methods are defined
18:10:37 [INFO] [NetworkInterceptor] bStats metrics enabled
18:10:37 [INFO] Loaded plugin NetworkInterceptor version 2.0.0-Bungee-3 by null

Additionally, connections from plugins are not blocked but the initial startup error is present when starting the server with only the security manager method enabled like this:

methods:
  - security-manager
#  - proxy-selector

The startup information in this case is as follows:

18:13:14 [SEVERE] [NetworkInterceptor] Exception occurred whilst enabling me.lucko.networkinterceptor.interceptors.SecurityManagerInterceptor
java.security.AccessControlException: Restricted Action
    at net.md_5.bungee.BungeeSecurityManager.checkPermission(BungeeSecurityManager.java:75)
    at java.base/java.lang.System.setSecurityManager0(System.java:374)
    at java.base/java.lang.System.setSecurityManager(System.java:358)
    at me.lucko.networkinterceptor.interceptors.SecurityManagerInterceptor.enable(SecurityManagerInterceptor.java:21)
    at me.lucko.networkinterceptor.common.CommonNetworkInterceptor.enable(CommonNetworkInterceptor.java:127)
    at me.lucko.networkinterceptor.common.CommonNetworkInterceptor.<init>(CommonNetworkInterceptor.java:58)
    at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.<init>(BungeeNetworkInterceptor.java:25)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
    at net.md_5.bungee.api.plugin.PluginManager.enablePlugin(PluginManager.java:341)
    at net.md_5.bungee.api.plugin.PluginManager.loadPlugins(PluginManager.java:250)
    at net.md_5.bungee.BungeeCord.start(BungeeCord.java:273)
    at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:67)
    at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15)
18:13:15 [INFO] [NetworkInterceptor] bStats metrics enabled
18:13:15 [INFO] Loaded plugin NetworkInterceptor version 2.0.0-Bungee-3 by null

SlimeDog commented 3 years ago

With no methods enabled, there is no connection detection. :)

bigfinfrank commented 3 years ago

With no methods enabled, there is no connection detection. :)

That's what I assumed, just being thorough

SlimeDog commented 3 years ago

That's what I assumed, just being thorough

Very much appreciated and helpful.

SlimeDog commented 3 years ago

Just so you know, Mart is in Europe, so timezone will start impacting his turnaround soon, unless he decides to pull an all-nighter. (I'm in western US.)

bigfinfrank commented 3 years ago

Just so you know, Mart is in Europe, to timezone will start impacting his turnaround soon, unless he decides to pull an all-nighter. (I'm in western US.)

Alright, I'm eastern US so EST/EDT. I don't have work tomorrow so I'll be available in the morning but not so much later in the day from around 2-4 PM up until 9-11 PM my time.

mart-r commented 3 years ago

OK. It looks to me like Bungee has nailed down its security-manager in way(s) that Spigot and Paper have not. We can play with that later, but this allows us to focus on the blocking issue(s).

Yes, I've now looked this up and they do indeed have a security manager that does not allow registering a new one: https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/BungeeSecurityManager.java#L70

After this it seems like other plugins are unable to make connections (even to localhost/127.0.0.1), despite blocking being set to false and the mode set to deny in config.yml. As an example, here is the relevant error from PremiumVanish:

It seems I left in some Bukkit-specific functionality to the Bungee part which is why it fails spectactularly. I've tried to fix that now. \<LINK ELIDED>

SlimeDog commented 3 years ago

I got (a little) intrigued. I configured a BungeeCord server. It's wasn't that difficult (thanks MD_5). I configured NetworkInterceptor-2.0.0-Bungee-4 the same as my Spigot/Paper installations, except

disabled methods.security-manager
logging.mode: all.

The plugins/NetworkInterceptor/intercept.log was created and populated. Here is the relevant portion of the console log

10:54:37 [INFO] [NetworkInterceptor] Using blocking strategy deny
10:54:37 [INFO] [NetworkInterceptor] Using a mapping blocker with timer of 1000ms
10:54:37 [INFO] [NetworkInterceptor] Using console+file combined logger
10:54:37 [INFO] [NetworkInterceptor] Interceptors: [proxy-selector]
10:54:37 [INFO] [NetworkInterceptor] [ProxySelectorInterceptor] Replacing 'sun.net.spi.DefaultProxySelector' selector with logged variant.
10:54:37 [INFO] [NetworkInterceptor] bStats metrics enabled
10:54:37 [INFO] Loaded plugin NetworkInterceptor version 2.0.0-Bungee-4 by null
10:54:37 [INFO] Loaded plugin cmd_list version git:cmd_list:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [WARNING] Forced host server pvp is not defined
10:54:37 [INFO] Enabled plugin reconnect_yaml version git:reconnect_yaml:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [INFO] [Bungee MOTD Changer]   ______               _
10:54:37 [INFO] [Bungee MOTD Changer]  |___  /              | |
10:54:37 [INFO] [Bungee MOTD Changer]     / / __ _ _ __ ___ | |__   ___  ___  ___   ___
10:54:37 [INFO] [Bungee MOTD Changer]    / / / _` | '_ ` _ \| '_ \ / _ \/ __|/ _ \ / _ \
10:54:37 [INFO] [Bungee MOTD Changer]   / /_| (_| | | | | | | |_) | (_) \__ \ (_) |  __/
10:54:37 [INFO] [Bungee MOTD Changer]  /_____\__,_|_| |_| |_|_.__/ \___/|___/\___/ \___|
10:54:37 [INFO] Enabled plugin Bungee MOTD Changer version 1.0.0 by BigFoxie
10:54:37 [INFO] Enabled plugin cmd_find version git:cmd_find:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [INFO] Enabled plugin cmd_server version git:cmd_server:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [INFO] Enabled plugin cmd_alert version git:cmd_alert:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [INFO] Enabled plugin BungeeWhitelist version 1.1 by snowbud56
10:54:37 [INFO] Enabled plugin cmd_send version git:cmd_send:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [INFO] Enabled plugin NetworkInterceptor version 2.0.0-Bungee-4 by null
10:54:37 [INFO] Enabled plugin cmd_list version git:cmd_list:1.17-R0.1-SNAPSHOT:c866619:1594 by SpigotMC
10:54:37 [INFO] Listening on /0.0.0.0:25565
10:54:37 [INFO] [NetworkInterceptor] Intercepted connection to mcstats.spigotmc.org
10:54:37 [INFO] [NetworkInterceptor] Blocked connection to mcstats.spigotmc.org

We should fix the "by null" bit.

The logged (blocked) connection to mcstats.spigotmc.org are from bungee itself. I will add that to the config.yml list.

I'm letting it run, to see if a bStats connection is made by NetworkInterceptor.

bigfinfrank commented 3 years ago

Security manager seems to work on Waterfall as they have a patch removing BungeeCord's security manager, you can download Waterfall here. I'm not sure if there is a way around this for BungeeCord but it might be easier to disable security-manager and warn people trying to use it on BungeeCord with a message like this, You need to use Waterfall or one of it's forks for the security-manager method to work!. Personally I'm unaware of any reason for someone to not just run Waterfall, I haven't experienced/heard of any drawbacks using Waterfall and their updates pulling from upstream are pretty timely around new releases from my experience.

EDIT: There are other plugins that do this when they require functionality in Waterfall that isn't in BungeCord, for example YouHaveTrouble/CommandWhitelist which sends a similar message:

14:42:57 [SEVERE] [CommandWhitelistBungee] Bungee tab completion blocker requires Waterfall other Waterfall fork.

SlimeDog commented 3 years ago

Is NetworkInterceptor-2.0.0-Bungee-4 working for you?

I have not seen any intercepted calls to bStats. Do you see any intercepts after the initial mcstats.spigotmc.org ?

bigfinfrank commented 3 years ago

Is NetworkInterceptor-2.0.0-Bungee-4 working for you?

I have not seen any intercepted calls to bStats. Do you see any intercepts after the initial mcstats.spigotmc.org ?

I haven't done a longer test beyond just starting it up and looking for errors/checking if anything is in intercept.log, I'll look into that now

bigfinfrank commented 3 years ago

I was able to get LuckPerms to make a connection with /lpb editor and it said it was intercepted,

>lpb editor
14:51:56 [INFO] CONSOLE executed command: /lpb editor
14:51:56 [INFO] [LP] Preparing a new editor session, please wait...
14:51:56 [INFO] [NetworkInterceptor] Intercepted connection to bytebin.lucko.me
14:51:56 [INFO] [NetworkInterceptor] Intercepted connection to bytebin.lucko.me
14:51:57 [INFO] [LP] Click the link below to open the editor:
14:51:57 [INFO] https://luckperms.net/editor/i-removed-this

After running it, this was added to intercept.log so I'd say it's working, going to test with blocking on for bytebin.lucko.me next.

SlimeDog commented 3 years ago

Yeah, I am seeing more intercepts on Waterfall, with methods.security-manager enabled. Thanks for the hint.

I'll have to document the differences, and md_5 may not be pleased when he sees it, but there it is.

Please do more extensive testing and let us know what you think.

bigfinfrank commented 3 years ago

It looks like proxy-selector is working as expected although LuckPerms doesn't seem too happy about the situation with a big stack trace.

>lpb editor
14:55:55 [INFO] CONSOLE executed command: /lpb editor
14:55:55 [INFO] [LP] Preparing a new editor session, please wait...
14:55:55 [INFO] [NetworkInterceptor] Intercepted connection to bytebin.lucko.me
14:55:55 [INFO] [NetworkInterceptor] Blocked connection to bytebin.lucko.me
14:55:55 [SEVERE] java.lang.RuntimeException: Error uploading data to bytebin

14:55:55 [SEVERE]     at me.lucko.luckperms.common.webeditor.WebEditorRequest.createSession(WebEditorRequest.java:179)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.commands.misc.EditorCommand.execute(EditorCommand.java:110)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.command.abstraction.SingleCommand.execute(SingleCommand.java:55)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.command.abstraction.SingleCommand.execute(SingleCommand.java:47)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.command.CommandManager.execute(CommandManager.java:262)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.command.CommandManager.lambda$executeCommand$1(CommandManager.java:166)

14:55:55 [SEVERE]     at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1800)

14:55:55 [SEVERE]     at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)

14:55:55 [SEVERE]     at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)

14:55:55 [SEVERE]     at java.base/java.lang.Thread.run(Thread.java:831)

14:55:55 [SEVERE] Caused by: java.net.SocketTimeoutException: Connection timed out

14:55:55 [SEVERE]     at me.lucko.networkinterceptor.interceptors.ProxySelectorInterceptor$LoggingSelector.select(ProxySelectorInterceptor.java:65)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.connection.RouteSelector.resetNextProxy(RouteSelector.java:115)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.connection.RouteSelector.<init>(RouteSelector.java:63)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.connection.ExchangeFinder.<init>(ExchangeFinder.java:75)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.connection.Transmitter.prepareToConnect(Transmitter.java:138)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:79)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:117)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:229)

14:55:55 [SEVERE]     at me.lucko.luckperms.lib.okhttp3.RealCall.execute(RealCall.java:81)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.http.AbstractHttpClient.makeHttpRequest(AbstractHttpClient.java:47)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.http.BytebinClient.makeHttpRequest(BytebinClient.java:78)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.http.BytebinClient.postContent(BytebinClient.java:98)

14:55:55 [SEVERE]     at me.lucko.luckperms.common.webeditor.WebEditorRequest.createSession(WebEditorRequest.java:174)

14:55:55 [SEVERE]     ... 9 more

14:55:55 [INFO] [LP] Unable to communicate with the editor. Check the console for errors.
>

SlimeDog commented 3 years ago

I would also like your opinions about release strategy. Should we release separate plugins for Spigot/Paper and Bungee/Waterfall, or should we release a "universal" plugin? What are the pros and cons?

bigfinfrank commented 3 years ago

Yeah, I am seeing more intercepts on Waterfall, with methods.security-manager enabled. Thanks for the hint.

I'll have to document the differences, and md_5 may not be pleased when he sees it, but there it is.

Please do more extensive testing and let us know what you think.

Planning on it, I'll try to also test through all the different config options and make sure nothing causes trouble.

I'm still so new to this. I've searched in vain for Bungee/Waterfall console command set. Do either of you have a link?

Their documentation definitely isn't the easiest to find, you can find a list of commands here for BungeeCord and as far as I'm aware Waterfall doesn't add any but their documentation can be found here.

I would also like your opinions about release strategy. Should we release separate plugins for Spigot/Paper and Bungee/Waterfall, or should we release a "universal" plugin? What are the pros and cons?

My understanding is that the pros are:

It's easier to distribute
There are fewer downloads which can lessen confusion from inexperienced users

And the cons are:

Larger jar file size - SpigotMC has a 5MB jar file size limit for plugins (to be clear I don't think you'll ever hit that with this)
In some cases it can complicate your codebase (although that's purely what I've heard, I'm not a plugin developer myself)

There is also a bit of user-preference at hand here, personally I prefer universal jars. In my opinion it should also be clearly mentioned that the plugin supports both BungeeCord/Waterfall and Spigot/Paper on the SpigotMC resource listing and in the README, I'm pretty sure that's obvious but I've seen far too many plugins hide this detail away in some FAQ page at the bottom of their obscure wiki for seemingly no reason.

SlimeDog commented 3 years ago

Thanks for the feedback. I appreciate the pros/cons.

WRT file size, we're currently at 73K for the Spigot/Paper version, and 87K for the universal we are testing. So no worries on that score.
WRT complication, the extra code is pretty neatly segregated, thanks to Mart.
WRT universal, my main concern is that a future divergence will make separate plugins a necessity. But I am agnostic on the choice, so I'll wait for Mart to offer an opinion.

It would be so useful if Bungee/Waterfall had a help (or ?) command. At least tab-completion works on Waterfall. 😎

As you can see on our SpigotMC and README and wiki pages, I have no issues with mentioning Paper (and by extension, Waterfall) explicitly, even though the Paper team claims (incorrectly) that any Spigot plugin will run without error on Paper, which would make such declarations unnecessary. After all, SpigotMC has little reason to support Paper, who view it as a death grudge match. I have no dog in that fight.

SlimeDog commented 3 years ago

Can we add the networkinterceptor info and networkinterceptor reload commands to bungee? I have no idea how to register them. But I see serverlistplus commands, so clearly it can be accomplished.

SlimeDog commented 3 years ago

Because I clean my logs files out-of-band (in my server start-up script), I didn't notice that NetworkInterceptor appends to intercept.log. Unless I am mistaken, we should add a configuration option to truncate it on start-up, true by default. Something like

logging.truncate-on-start: true

SlimeDog commented 3 years ago

I took a long bicycle ride today, in 40°C (105°F for English speakers), to think about the two-categories issue. I think there is a reason to build a separate plugin for Bungee/Waterfall, so that it can be found in resource searches. After discussion with members of the SpigotMC Resources Team, I think the best solution is:

Maintain the code as a single repository, so long as nothing in the Bungee/Waterfall universe makes that untenable.
I will edit the configuration and wiki and README to clarify that there are two resources with the same functionality, for use in the two universes.
Build a single release whenever updates are required. The fact that they are the same is irrelevant.
Publish the resource to the SpigotMC Spigot category, with a Spigot/Paper-specific description.
Publish the resource to the SpigotMC Bungee-Proxy category, with a Bungee/Waterfall-specific description.
The resource description in each category should point to the other, as a companion resource.
When Paper Hangar opens, publish the resources there as well.

Does that sound right?

bigfinfrank commented 3 years ago

Build a single release whenever updates are required. The fact that they are the same is irrelevant.

So upload a universal jar to two separate resource listings, one for Bungee and one for Spigot (then eventually hangar)?

SlimeDog commented 3 years ago

So upload a universal jar to two separate resource listings, one for Bungee and one for Spigot (then eventually hangar)?

That's what I'm thinking. I have confirmed that is acceptable with SpigotMC staff.

Open for discussion. BTW, you can always delete a comment, if there has been no response.

bigfinfrank commented 3 years ago

That's what I'm thinking. I have confirmed that is acceptable with SpigotMC staff.

Open for discussion.

I also think that's probably the best approach, especially for discoverability. I'm interested in how you think file naming should be within this dynamic, would it be best to keep it simply as NetworkInterceptor-<version>?

BTW, you can always delete a comment, if there has been no response.

I actually didn't know that was even an option somehow, thanks!

SlimeDog commented 3 years ago

My thinking is to stay with NetworkInterceptor- for all releases, regardless of platform. I am open to counter-arguments.

Delete is in the ellipses (...) pulldown.

bigfinfrank commented 3 years ago

My thinking is to stay with NetworkInterceptor- for all releases, regardless of platform. I am open to counter-arguments.

Sounds good.

I also have a stability update; I have only found a single issue with the plugin on Waterfall after what I believe is some pretty comprehensive testing. Anyways the one issue is that trusted-plugins doesn't seem to be working. I was able to reliably test this with the LuckPerms editor subcommand, /lpb editor which sends out a request to bytebin.lucko.me. For completeness, I'm using the latest LuckPerms-Bungee-5.3.51.jar with the most recent NetworkInterceptor-2.0.0-Bungee-4.jar on the latest waterfall-1.17-445. I'll have relevant config options and the relevant intercept.log entry below:

Config (only relevant options):

methods:
  - security-manager
  - proxy-selector

mode: allow

logging:
  enabled: true
  include-traces: false
  mode: all

trusted-plugins:
  - LuckPerms

targets:
  - 'api.mojang.com'
  - 'sessionserver.mojang.com'
  - 'launcher.mojang.com'
  - 'launchermeta.mojang.com'
  - '127.0.0.1'
  - 'localhost'
  - '::1'

Relevant intercept.log entry:

  Sat Jul 10 01:12:34 EDT 2021: Intercepted connection to bytebin.lucko.me
    at me.lucko.luckperms.lib.okhttp3.internal.connection.RouteSelector.resetNextProxy(RouteSelector.java:115)
    at me.lucko.luckperms.lib.okhttp3.internal.connection.RouteSelector.<init>(RouteSelector.java:63)
    at me.lucko.luckperms.lib.okhttp3.internal.connection.ExchangeFinder.<init>(ExchangeFinder.java:75)
    at me.lucko.luckperms.lib.okhttp3.internal.connection.Transmitter.prepareToConnect(Transmitter.java:138)
    at me.lucko.luckperms.lib.okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:79)
    at me.lucko.luckperms.lib.okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
    at me.lucko.luckperms.lib.okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:117)
    at me.lucko.luckperms.lib.okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:229)
    at me.lucko.luckperms.lib.okhttp3.RealCall.execute(RealCall.java:81)
    at me.lucko.luckperms.common.http.AbstractHttpClient.makeHttpRequest(AbstractHttpClient.java:47)
    at me.lucko.luckperms.common.http.BytebinClient.makeHttpRequest(BytebinClient.java:78)
    at me.lucko.luckperms.common.http.BytebinClient.postContent(BytebinClient.java:98)
    at me.lucko.luckperms.common.webeditor.WebEditorRequest.createSession(WebEditorRequest.java:174)
    at me.lucko.luckperms.common.commands.misc.EditorCommand.execute(EditorCommand.java:110)
    at me.lucko.luckperms.common.command.abstraction.SingleCommand.execute(SingleCommand.java:55)
    at me.lucko.luckperms.common.command.abstraction.SingleCommand.execute(SingleCommand.java:47)
    at me.lucko.luckperms.common.command.CommandManager.execute(CommandManager.java:262)
    at me.lucko.luckperms.common.command.CommandManager.lambda$executeCommand$1(CommandManager.java:166)
    at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1800)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
    at java.base/java.lang.Thread.run(Thread.java:831)
Sat Jul 10 01:12:34 EDT 2021: Blocked connection to bytebin.lucko.me

SlimeDog commented 3 years ago

If BungeeCord, this error should be caught and security-manager disabled as a consequence. (Maybe someday, Bungee will allow it, as Waterfall does.)

22:18:32 [SEVERE] [NetworkInterceptor] Exception occurred whilst enabling me.lucko.networkinterceptor.interceptors.SecurityManagerInterceptor
java.security.AccessControlException: Restricted Action
    at net.md_5.bungee.BungeeSecurityManager.checkPermission(BungeeSecurityManager.java:75)
    at java.base/java.lang.System.setSecurityManager0(System.java:374)
    at java.base/java.lang.System.setSecurityManager(System.java:358)
    at me.lucko.networkinterceptor.interceptors.SecurityManagerInterceptor.enable(SecurityManagerInterceptor.java:21)
    at me.lucko.networkinterceptor.common.CommonNetworkInterceptor.enable(CommonNetworkInterceptor.java:127)
    at me.lucko.networkinterceptor.common.CommonNetworkInterceptor.<init>(CommonNetworkInterceptor.java:58)
    at me.lucko.networkinterceptor.bungee.BungeeNetworkInterceptor.<init>(BungeeNetworkInterceptor.java:25)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
    at net.md_5.bungee.api.plugin.PluginManager.enablePlugin(PluginManager.java:341)
    at net.md_5.bungee.api.plugin.PluginManager.loadPlugins(PluginManager.java:250)
    at net.md_5.bungee.BungeeCord.start(BungeeCord.java:273)
    at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:67)
    at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15)

SlimeDog commented 3 years ago

I also have a stability update; I have only found a single issue with the plugin on Waterfall after what I believe is some pretty comprehensive testing. Anyways the one issue is that trusted-plugins doesn't seem to be working. I was able to reliably test this with the LuckPerms editor subcommand, /lpb editor which sends out a request to bytebin.lucko.me. For completeness, I'm using the latest LuckPerms-Bungee-5.3.51.jar with the most recent NetworkInterceptor-2.0.0-Bungee-4.jar on the latest waterfall-1.17-445. I'll have relevant config options and the relevant intercept.log entry below: ...

Actually, it works as well as it can. This particular intercept occurs too early for LuckPerms to be mapped to the FQDN (even though it is clear visually, after the fact, from the stack trace). So if you want to allow the FQDN, you'll have to specify it explicitly.

bigfinfrank commented 3 years ago

If BungeeCord, this error should be caught and security-manager disabled as a consequence. (Maybe someday, Bungee will allow it, as Waterfall does.)

A more proactive solution could be to check whether or not Waterfall is being used and avoid the error altogether. This could presumably be done by trying to access a waterfall-specific class and catching a possible ClassNotFoundException. An even better solution would be to simply check if a security manager is already present but I'm not familiar with the java security manager so I'm not sure if there's an elegant way to do this or not.

Actually, it works as well as it can. This particular intercept occurs too early for LuckPerms to be mapped to the FQDN (even though it is clear visually, after the fact, from the stack trace). So if you want to allow the FQDN, you'll have to specify it explicitly.

Well if that's the case, then there aren't any issues I could find with the Waterfall plugin.

SlimeDog commented 3 years ago

A more proactive solution could be to check whether or not Waterfall is being used and avoid the error altogether. This could presumably be done by trying to access a waterfall-specific class and catching a possible ClassNotFoundException. An even better solution would be to simply check if a security manager is already present but I'm not familiar with the java security manager so I'm not sure if there's an elegant way to do this or not.

Unless I'm mistaken, the error is not generated if Waterfall is the driver, only if BungeeCord is the driver. Did you find otherwise? But a "catch" would work in either case.

bigfinfrank commented 3 years ago

Unless I'm mistaken, the error is not generated if Waterfall is the driver, only if BungeeCord is the driver. Did you find otherwise? But a "catch" would work in either case.

You're correct and that's identical to what I experienced with my testing, plus it makes sense as Waterfall removes BungeeCord's secuirty manager in the patch I mentioned earlier. In the catch solution, the security manager would be disabled only if the ClassNotFoundException was caught, as that would mean Waterfall isn't present. I should have clarified this when I originally presented the solution, I apologize.

SlimeDog commented 3 years ago

No worries. It sounds like we're in agreement. Over to Mart for implementation.

bigfinfrank commented 3 years ago

Can we add the networkinterceptor info and networkinterceptor reload commands to bungee? I have no idea how to register them. But I see serverlistplus commands, so clearly it can be accomplished.

I typically see bungee plugins with non-bungee equivalents that could be used simultaneously adding bungee onto the end of their spigot command names, in this case networkinterceptorbungee. This is admittedly a bit long, maybe an abbreviated nib or something similar would be beneficial here? In terms of the actual implementation while I'm not familiar with the development side of Bungee plugins (especially implementation), I was able to find this short tutorial that includes registering the command as an actual command. What I believe is LuckPerms' implementation with their Bungee lpb command can be found here

SlimeDog / NetworkInterceptor

Feature request: BungeeCord/Waterfall support #12