Safeguarding Against Unchecked and Potentially Damaging Plugins

Discussion in 'Bukkit News' started by EvilSeph, Dec 19, 2012.

Thread Status:
Not open for further replies.
  1. Offline

    EvilSeph

    As Mojang continue to work towards the Minecraft Plugin API (cleaning up and rewriting the code), the code within Minecraft and CraftBukkit will undoubtedly shift. Fortunately, as the majority of the plugins available have been developed using only the Bukkit API (which was designed to be resilient and mostly update proof), this code shifting should not affect most of your servers.

    If, however, you happen to be running a plugin that uses code outside of the Bukkit API (like Minecraft or CraftBukkit code), those plugins are highly likely to break and bring down your servers with them - often without any advanced warning - whenever a Minecraft update is released. In response to this very real problem, we've had to make the difficult decision of forcing plugin developers that use Minecraft and/or CraftBukkit code within their plugins to re-evaluate their work with the release of every Minecraft update to ensure they are still functioning as intended.

    It is important to note that even if a plugin you have been using has been working fine across Minecraft updates until now, there is simply no way to guarantee that this will always be the case. Making the assumption that it will work with every update is like playing Russian roulette with your server.

    The problem:
    With the extensive work being done to Minecraft to accommodate the Minecraft Plugin API, the Minecraft code is now more unpredictable and volatile than ever before. These changes have made it clear that allowing plugins to run unchecked across Minecraft updates is a big mistake that puts your servers at significant risk of being silently damaged. Neither Bukkit nor plugin developers have any control over the Minecraft (and, as it is built upon Minecraft itself, CraftBukkit) code. Therefore, if a plugin uses code outside of the Bukkit API and it has not been verified to work on the Minecraft version your server is running, using it can only lead to unpredictable problems.

    What makes matters worse and more confusing is that there is no easy way for you, as a server admin, to tell if the plugins you are using utilise only the Bukkit API or unsupported code within Minecraft and/or CraftBukkit itself. As plugin developers have no incentive to do so, they have not been putting up a notice informing server admins that their plugins use more than just the Bukkit API and thus server admins are left in the dark. Without this important knowledge, server admins have been blindly running plugins that are not ensured to function as intended across Minecraft versions, potentially and unknowingly putting their servers at risk.

    Up until this safeguard was introduced, plugin developers were not required to verify that their plugins continued to function as they intended whenever a Minecraft update came out. As a result, potentially unstable plugins have been running unchecked on your server with no indication that they could damage your server at any time without any advanced warning. The fact of the matter is: plugins that depend on Minecraft or CraftBukkit code need to have their code verified whenever a Minecraft update is released before it can be said with absolute certainty that a plugin is safe to run on your server.

    In summary:
    - Mojang is cleaning up and rewriting the Minecraft code in anticipation for the Minecraft Plugin API.
    - Plugins that use Minecraft or CraftBukkit code will break in unpredictable ways.
    - You aren't told that a plugin is using unsupported and volatile code, so you likely aren't aware that plugins you are using could be silently breaking your servers.

    The solution:
    To address this problem, we've made the difficult decision of including a safeguard directly into CraftBukkit. This safeguard serves many purposes but the major ones are: it will help protect your server against unchecked plugins, it will make determining which plugins are breaking with every Minecraft updates and it will force plugin developers to take responsibility for what their plugins do to your server.

    With this safeguard in place, a potentially damaging plugin will not be able to run until it has been updated with a version that has been checked by the plugin developer. Granted, plugin developers have the option of completely bypassing this safeguard and putting your server at risk. However, if they choose to do this it will be very clear who was responsible for any damage done to your server and you'll know to avoid that developer's work in the future.

    Note: this safeguard is not intended to stop the use of code outside of the Bukkit API, but rather to promote more responsible use of it if a plugin developer decides to do so.

    So what does this safeguard mean for you?
    Server Admins:
    If you are a server admin that only uses plugins developed against the Bukkit API, this safeguard doesn't affect you at all. If you are a server admin that uses plugins which use Minecraft or CraftBukkit code (which we do not support or recommend using) then this safeguard means that those plugins will need to be updated with every Minecraft update.

    It is important to note that while this safeguard does force plugin developers to take some sort of action to get their plugins built against Minecraft or CraftBukkit working on a new Minecraft version, plugin developers have the option of bypassing it. They can blindly update a few lines in their code to mark it as working with a new Minecraft update or utilise a bypass to trick the safeguard into letting the plugin run. As such, we recommend that server admins be wary of plugin developers who decide to work around this, as they are willingly putting your server at risk.

    Plugin Developers:
    If you are a plugin developer that purely uses the Bukkit API this safeguard does not affect you in any way.

    If, however, you depend on the extremely volatile and unsupported CraftBukkit OR Minecraft code, you will now have to re-evaluate your plugins with every Minecraft update release. As this is what you should have been doing anyway as a responsible developer, this should not affect your update process in any way.

    We are not trying to make utilising Minecraft or CraftBukkit code within your plugins more difficult, we are simply trying to promote using it more responsibly if you have a need to do so within your plugins. If there is no way for you to avoid using the volatile and unsupported internals of Minecraft or CraftBukkit, we recommend trying to work with us to design an addition to the Bukkit API that removes this need.

    What if I'd rather take the risk?
    Server Admins:
    If you'd rather put your server at risk by running unchecked code, you are free to bypass this safeguard, however you will no longer receive support from us as a result. If you'd still like to bypass or disable this safeguard, you have the option of running an unofficial build or a tool to update the plugins you use. Unfortunately, since providing support for code we did not write is next to impossible, we still do not allow the discussion and distribution of unofficial builds within our community.

    Plugin Developers:
    Plugin developers bypassing this safeguard are willingly putting servers at risk with their unpredictable and unchecked code. If you as a plugin developer choose to bypass this safeguard bear in mind that you are taking full responsibility for anything your plugin does to a server and that this decision can affect your reputation as a developer.

    There are several ways to bypass this safeguard that I'm sure many of you will be discussing on these forums, however, we would like to make it clear that plugins using any bypass that includes dynamic code generation will be denied from BukkitDev without hesitation due to the inherent security risks it poses for servers.

    Whether you are a server admin or a plugin developer, you are free to discuss ways to get around this safeguard provided it does not involve an unofficial build. The issue with unofficial builds is that regardless of where people get them from, we almost inevitably end up having to provide support for them.

    We know that this safeguard might cause a few of you some headaches, however we feel that choosing to let servers burn in the coming weeks is not a viable option. Thank you for your continued support, cooperation and understanding in this matter. This was a difficult decision for us to make, but preventing unchecked plugins from silently destroying servers was a big incentive for us.
     
    Archarin, AlexMl, DanManB and 16 others like this.
  2. Offline

    Wolvereness Bukkit Team Member

    This is a great way for the update process to continue moving forward smoothly.
     
    fredghostkyle1 and bobacadodl like this.
  3. Offline

    SirTyler

    Well this is a double edged sword, makes it so that people are more well protected but it may also limit somethings (of the top of my head plugins like BloodMoons and Storms).

    There you go Wolvereness, now only 1399 left to go.
     
  4. Offline

    TheBeast808

    When I make plugins, I make sure to only use the Bukkit API, but I have doubts about this update. What about plugins that need things the bukkit API does not offer? If the API was feature complete, there would be no more requests for additions, but this definitely isn't true.Will the bukkit team be more willing to work with plugin developers to add needed features to the API? I know in the past several developers have left the bukkit community because the API was slow to change(remember the whole Spout thing?).
     
    vgmddg and elias79 like this.
  5. Offline

    FTWinston

    Sorry to nitpick, but Bukkit allows me to distribute a plugin using the MIT license, which (to my understanding) effectively abdicates all responsibility for anything my plugin does. So while bypassing the safeguard may mean plugin developers take a certain form of responsibility, it's not a legal one.


    On another note: how does this safeguard work with regard to "extending" other plugins? My plugin uses CraftBukkit code, and exposes an API for other plugins to use. If one of those other plugins uses only Bukkit code and my plugin's, I presume it's OK without re-verifying its code with each update, as long as my plugin does?
     
  6. Offline

    s32ialx

    I'm interested in hearing a response to this nitpickers question :D
     
    Nick Foster likes this.
  7. Offline

    rmh4209

    From my experience with doing this since the first commit changing the CraftBukkit and NMS code, all you'd need to do is use abstraction in the plugin that exposes the CraftBukkit code. We do this in one of the plugins that I help develop. Here's a link to an example of the abstraction of which I speak: RefactorInterfaceExample

    All you'd need to do is make sure that your plugin that exposes CraftBukkit methods supports the new versions of CraftBukkit (I'd say just support either betas and recommended or just recommended) and tell people the download the new version whenever CB or Minecraft updates.
     
  8. Offline

    deadlock989

    Drama much?
     
  9. Offline

    lol768

    I think the general issue is that people are using NMS + CB code rather than the Bukkit API because it can't do certain things. For example, in BattleKits I have to reference CraftBukkit to:
    • Set custom item names and lore
    • Dye leather armour
    Until these can be accomplished using the Bukkit API I have no choice but to do this. Feildmaster has stated these APIs would be added. TnT has also stated these are a priority along with other item data, which should mean that this becomes less of an issue when these APIs and others (e.g. books) are available in Bukkit without requiring messing with CraftBukkit directly. I understand that some plugins heavily rely on CraftBukkit, so I can only imagine this is a pain for the developers to deal with. I do think it is important to be 'better safe than sorry' when there is (albeit it may be small) chance that these plugins cause damage when they are not checked, putting further strain on the developers who deal with bug reports.

    I do, however, think PRs could be handled in a better way and EvilSeph provided some information on some changes they were hoping to make which sounded promising. In many cases, pull requests are rejected due to formatting or issues with the structure of the implementation. The commit comment describes what looks to me as a much better way of handling pull requests. Is there any ETA for this scheme?
     
    KollegahDerBoss likes this.
  10. Offline

    deadlock989

    Basically this means that most live servers aren't going to use the 1.4.5 "recommended" build. With 1.4.6 possibly being released tomorrow, no-one is going to use a "recommended" build that breaks most of the top plugins for the same Minecraft version, for example, WorldGuard, CommandHelper, PermissionsEx.

    For myself, I'm not bothering to update my own plugin for 1.4.5 with 1.4.6 so close and most of the other primary plugins that provide absolutely essential functionality to run a multiplayer game broken. I'll wait until enough other people have picked up the pieces and then see where we are. So we're staying on the beta 1.4.5 until things settle, which will probably be late in 1.4.6's lifetime or even not until 1.5.

    Can I ask how many of the Bukkit team run a live server? Was this change absolutely necessary to do now, between a beta and a RB? Couldn't it have waited until the next major version change of Minecraft itself, when a fresh round of plugin updates was going to happen anyway?
     
  11. Offline

    sablednah

    Plugins that modify Entity speed and behaviours use NMS alot, I know I'm involved with alot of them and have written a custom one for a server I help run.

    So with that in mind - I fully agree with this. As a dev I see so many bug reports where people are using the wrong version of Bukkit for the plugin. The ability to mark a plugin as for a certain build (or range of builds? I haven't seen HOW the safeguard is implemented yet) will save us alot of headaches where CB itself will say - sorry wrong invalid CB build/plugin combination.

    As for the claims of drama - not so.

    When using NMS - I pick up on methods with names like a() bA_() and so on.

    When a new build of MC comes out - suddenly a() might change from a method that handles range attacks or per tick code - into the ender dragons destroy block routine.. ok so that's not sound code - but you get the gist of how your code can suddenly refer to completely random functions and fields. So yeah - lots of weird stuff can happen - and it could be quite damaging.
     
    DHLF likes this.
  12. I have not seen a striking reasoning for doing it now in the 1000+ comments shootout on the GitHub page for that commit.

    Forcing boilerplate update (or worse: generic bypasses) just for the transition from 1.4.5-beta to 1.4.5-recommended does not seem to make any sense to me, and i doubt that there exists a convincing reasoning for this.

    Edit: Obviously that change establishes a kind of "version comparison", in the fastest and most simple way to bring it in.
     
  13. Offline

    Ammar Askar

  14. Offline

    deadlock989

    That's not how it works. The plugins are compiled against a package that doesn't exist any more. NMS and CB classes now have a version number in the package name.

    CB itself doesn't "say" anything. What happens is a torrent of NoClassDefFound exceptions.

    So instead of a tiny minority of evil, evil plugins wreaking devastation on hapless server admins, a large number of plugins completely fail even if the classes accessed haven't changed at all.
     
    zipfe, vgmddg, Codex Arcanum and 3 others like this.
  15. Offline

    lol768

  16. Offline

    sablednah

    Yeah I just ran the build and found that out. Would it be much work to catch those and give a friendlier warning to the server admin?

    Also I see "server.v1_4_5". I infer from this that this would only need doing per version change within MC server.
    And we should all be checking nothings changed any ways when a new MC release is made.

    I guess as 1.5 and onwards arrive big changes ARE going to happen to alot of NMS code - hence getting this in place before then. At least with the versioning we will have a clear indication of when such changes occur.
     
    vgmddg and TnT like this.
  17. Offline

    rossfudgew

    Well this does a whole lot of bad for me and essentially no good other wise. I'f this was cake and what you illustrate as the other option is death, at this point I'd take death. Tracing errors you cant trace is actually no fun. At least if you gonna put something like this in make it easy for us idiots to chase.


    and on an unrelated side note, I noticed you guys seem to lock threads for people disagreeing with you so how long until this thread is locked I wonder?
     
    Canownueasy, vgmddg and Derthmonuter like this.
  18. Offline

    Gravity

    Have you read the main post? :/
    We're not blocking any discussion about this unless it involves unofficial builds, for reasons mentioned in the post, or if it turns into a bunch of flaming. If you want to have mature discussions, by all means do so. However, it's clear to me that even by this that you have little interest in doing that, and your only goal seems to be to portray the staff in a bad light.
     
  19. The task would be to find a valid reason to bring it before 1.5 / 1.4.6, then :)

    Why clear? You may be right, you may be wrong, expect some people to just complain in some way, it is totally natural, given the nature and timing of this change...

    Edit: On a side note: i did not even get the grammar of the post you cite there :)
     
  20. Offline

    Deathmarine

    I was against this issue originally only of the fact of not understanding the reasoning. Now that it is clearly put, I agree. Even though my plugins use quite a bit of craftbukkit / nms this is a better way of handling the upcoming changes.
     
  21. Offline

    Ne0nx3r0

    The importance of a subject can be explained without fire and brimstone.

    I came here hoping to get information on how to update plugins and see what standards have been set for this change and for information on how to adapt to it, but instead find a pious diatribe about plugins that provide functionality Bukkit doesn't.
     
    Derthmonuter and vgmddg like this.
  22. Offline

    rossfudgew

    So wait what your telling me is that if the bukkit name goes on a build, but its not a reccommended build, its not bukkit official? That would be like microsoft going out and saying during the windows 8 beta "just because it has the windows name is on it doesn't mean its our software." Do you read what you are typing good sir?

    My point though was that there was from what I saw a huge amount of developer sentiment against this and a complete disregard for their feelings on this issue. Many ideas were proposed in ways which would be beneficial to the community including an enhanced versioning check with a disable feature, settings in the plugin.yml. Even as I look on other minecraft developer communities its quite apparent that mojang is no where near pushing out an official minecraft server api, and even if they were to say assume all of bukkit and use it as their server api, it would still be bukkit as we know it, so no problems there.

    When I'm left with "NoClassDefFound" I don't know what plugin did it, why its not working, or how to single out one misbehaving plugin against a field of working ones. If the sentiment before was that this will cause problems, and the result now is that it is causing problems, I do see a failure in the bukkit staff to timely inform their community to these changes, and the fault does lie on your heads. Don't try to avoid it, just admit that you were wrong, and either ask for support on this issue, or find a better way of doing what this intends to do.

    Finding solutions isn't about forcing changes that people may or may not like down their throats and telling them to accept it, especially in an open source project. Its about pooling thoughts, heads, and ideas together and finding the best path to get things done. And so far from what I see this is more of a cram it down our throats then finding a solution for a problem.

    Even with the future in mind (which is what the good intentions of this is) there is no point in saving the future if you create your future with misdeeds. Its alienating some of your best plugin developers, and causing rifts in the community. And through doing this you loose the support of your biggest allies.

    If you think I intended to create a flame war that is fine, but I would just like to express my opinion in what I am "liberally" assuming is still an open forum.

    If your onboard that's cool, but on an unrelated sidenote, can you update op verify so I don't have to deal with nodus kiddos?
     
  23. Offline

    TnT

    There is a bit of a misunderstanding I think. We do not stop plugins using internals, we just ensure those plugins must update for their use of internals that are very likely to change every new Minecraft release.

    My server is on 1.4.5-R1.0. I had ~5 plugins break due to this, and they have all had dev builds out to fix them. You're welcome to join my server until you can get yours up to date.

    Hi! *waves*

    We debated making it friendlier, but I think once a couple new Minecraft releases come out this will become status quo.

    Correct. Once per Minecraft version update. This is the initial commit, so it came mid way through 1.4.5. For 1.4.6 it will be in the first new build of CraftBukkit.

    Exactly. Quite frankly, if CraftBukkit internals never changed, you wouldn't see a great many plugins breaking every new Minecraft version like we currently do. This just safeguards against future breaking.

    Its clear you understand. +1 for you.
     
    HappyPikachu likes this.
  24. Offline

    sablednah

    Here's an example.

    Code:
    Error occurred while enabling Heroes v1.5.0-b1618 (Is it up to date?)
    java.lang.NoClassDefFoundError: net/minecraft/server/Entity
    Perfectly traceable.
    net.minecraft.server.Entity no longer exists. It's now net.minecraft.server.v1_4_5.Entity.
    All you have to do is check all your NMS code is still valid when the version changes and change your imports if it is.
     
  25. Yeah that was the fishy part of that post :) - exchanging the CraftBukkit dependency for Bukkit would probably be magnitudes faster, if using an IDE that does static code checking, though.

    Edit: And don't tell me updating the CraftBukkit dependency to the new version would do the same :p, there are some variations depending on how a project is set up of course...
     
  26. Offline

    sablednah

    Ahh I think you missed the intent there...

    A clear "fix" for this is to make a modified craftbukkit where the NMC package doesn't have the versioning number. Voilla NMS is back and all plugins find their class's. But said plugins run the risk of these classes changing.

    So quite rightly such moded builds of CB are not suported.

    To use your analogy. It would be like me taking a copy of Windows8 - editing user32.sys and kernal.sys liberally - and complaining whwn it crashes whilst running a program and expecting Microsoft to fix it.
     
  27. Offline

    rossfudgew

    Your are all nice an pretty heres an example of one of mine that breaks on config (skylands plus world generator)
    Code:
    2012-12-19 09:27:58 [SEVERE] Error occurred while enabling Multiverse-Core v2.5-b638 (Is it up to date?) - Blaming the error on multiverse, except
    java.lang.NoClassDefFoundError: net/minecraft/server/World 
        at uk.co.jacekk.bukkit.SkylandsPlus.SkylandsPlus.getDefaultWorldGenerator(SkylandsPlus.java:32) -Error was in the world generator.
        at org.bukkit.WorldCreator.getGeneratorForName(WorldCreator.java:284)
        at org.bukkit.WorldCreator.generator(WorldCreator.java:181)
        at com.onarandombox.MultiverseCore.utils.WorldManager.doLoad(WorldManager.java:374)
        at com.onarandombox.MultiverseCore.utils.WorldManager.doLoad(WorldManager.java:359)
        at com.onarandombox.MultiverseCore.utils.WorldManager.loadWorlds(WorldManager.java:650)
        at com.onarandombox.MultiverseCore.MultiverseCore.onEnable(MultiverseCore.java:304)
        at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:217)
        at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:457)
        at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:381)
        at org.bukkit.craftbukkit.v1_4_5.CraftServer.loadPlugin(CraftServer.java:307)
        at org.bukkit.craftbukkit.v1_4_5.CraftServer.enablePlugins(CraftServer.java:289)
        at net.minecraft.server.v1_4_5.MinecraftServer.j(MinecraftServer.java:325)
        at net.minecraft.server.v1_4_5.MinecraftServer.e(MinecraftServer.java:304)
        at net.minecraft.server.v1_4_5.MinecraftServer.a(MinecraftServer.java:263)
        at net.minecraft.server.v1_4_5.DedicatedServer.init(DedicatedServer.java:147)
        at net.minecraft.server.v1_4_5.MinecraftServer.run(MinecraftServer.java:403)
        at net.minecraft.server.v1_4_5.ThreadServerApplication.run(SourceFile:856)
    Caused by: java.lang.ClassNotFoundException: net.minecraft.server.World
        at org.bukkit.plugin.java.PluginClassLoader.findClass0(PluginClassLoader.java:70)
        at org.bukkit.plugin.java.PluginClassLoader.findClass(PluginClassLoader.java:53)
        at java.lang.ClassLoader.loadClass(Unknown Source)
        at java.lang.ClassLoader.loadClass(Unknown Source)
        ... 18 more
    Nevermind the fact that now some of my world guard flags cause breaks and some things in essentials don't work at all for me, and numerous other small but equally as painful issues arise as more and more things on the server get used.
     
  28. Offline

    sablednah

    Quote Snipped at the point where I can see what the cause is - SkylandsPlus is using net.minecraft.server.World.

    Next.


    EDIT: Also - why not just stick with RB0.3 until The Big Players (essentials, worldedit etc) throw out RB1.0 compatible builds?
     
    DHLF likes this.
  29. Offline

    rossfudgew

    What I was planning on doing, but I was just griping on how the bukkit staff could have timely informed everyone about this change.
     
    vgmddg likes this.
  30. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı

    For plugin developers looking for an easy way to handle this code change and support multiple versions (while not putting server admins in harms way), I wrote a very simple abstraction demonstration. You can also view my approach in live plugins VanishNoPacket and TagAPI.
    The code is available here: https://github.com/mbax/AbstractionExamplePlugin

    Forum thread for that with full explanation here.

    The big players have had compatible builds for a week.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 30, 2016
Thread Status:
Not open for further replies.

Share This Page