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 Retired Staff

    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

    bergerkiller

    TnT
    No welcome back, just showing my warm heart to the cold project I am leaving. Thanks for finally writing up an RB btw, now fixing up final versions so I'm done with it.
     
  3. Offline

    sablednah

    Correct they didn't, but they could have.
    Bukkit have no control over NMS*. So anyone using it is subject to potential breaks on any update.

    As NSM changes ramp up, so does the potential. A mechanism has been put in place to stop damage.

    That's it. We're not 'supposed' to use NMS anyway, its not part of the API. Effectively you're no longer making a plugin - you're making a server mod. And just like with client mods you can break on any update.



    * Separate debate there I know, don't get nit-picky guys.
     
  4. Offline

    bergerkiller

    sablednah
    *Throws one of the many arguments we have already provided many times against your rehashed, repeated and ungrounded argument*

    Did you know SpoutPlugin is going to die now? They made a class they had to extend final. At least there will be no more need to point out the difference between Spout and Spout Plugin :)
     
    Technius, hatstand and TheLoneDevil like this.
  5. Offline

    TheLoneDevil

    You yourself are saying that people still wont check their code for changes they will just change the version, if they continue to even keep maintaining said plugins, someone could easily write a script to check for MC updates and changing the version to suite, completely bypassing the "safeguard" that this change supposedly enforces.
     
  6. Offline

    bergerkiller

    TheLoneDevil
    That is one of the 'many arguments we have already provided', stop trying, Bukkit doesn't listen. We have already given thousands of lines of reasoning against these changes, and all we got was this lousy sweater.
     
  7. Offline

    TnT Retired Staff

    Please re-read the first post. We already address that. Yes, developers can do that. However, now when I update a plugin my server (that is working great), and it now breaks my server after updating that plugin, I know who did it. This is valuable information!

    I give the Spout devs more credit than that. They're pretty bright you know. You should give them more credit too.
     
  8. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    And thousands of lines of reasoning in reply to your responses, but you keep ignoring that part. ;)
     
  9. Offline

    asofold

    In principle: more or less.

    The plugins will most likely enable, while the "breaking" will occur at some point later, for the most cases. This does not provide total protection. A plugin can still cause all sorts of data corruption in principle. In general but also for plugins that depend on other plugins that access NMS/OBC, it would be better to have a failure before enabling for this matter, so other plugins won't assume the functionality to be present.

    I am more interested in the total amortization, though.
     
  10. Offline

    bergerkiller

    mbaxter
    Yes, thousands upon thousands of lines of repeated arguments not answering the actual arguments we provide against you. All you do now is state that these things happen, not taking into consideration that it renders the actual thing you are doing, useless.

    In short: instead of taking the problems if your actions into consideration, you are merely listing them as side-issues and putting them into the 'not our problem' zone. You are not debunking, neither are you actually providing reasoning against, all of the counter-arguments we have provided in the past week.

    Instead, you jump to blaming them (your plugin will not be approved), fear mongering (we will not support any servers that bypass this) and, of course, name calling (all of you NMS developers are breaking our server)
     
    vgmddg, tssge and Smex like this.
  11. Offline

    sablednah

    And? A plugin that extended beyond the API will stop working? Your point?

    CraftBukkit (as opposed to the Bukkit API) and NMS where, are, and will continue to be - use at your own risk. Isn't that why they started on their own server? For the record I quite like spoutcraft... I'm not going to get dragged into a spout vs bukkit discussion here. But I will readily give then due respect for going and doing their own thing. It's one thing to have a difference of opinion in direction - its a whole more better thing to take action and go do something about it instead of just complaining ;)

    I suspect that instead of a plugin, a variant server will arise - without that final modifier and with spout plugin inbuilt and spoutcraft will continue on. They are a very resourceful team.



    Also ungrounded? really? Not sure how much simpler I can put it than..

    This change makes sure that NMS code (which is due to undergo some radical changes) can't accidentally damage a CraftBukkit server through miss-match of versions.

    Yes its a bit more work, but its work devs ARE doing anyway.

    Minecraft 1.4.6 comes out.
    Dev Bukkit comes out soon after.
    Plugins update for the dev build.
    Beta Bukkit 1.4.6 come out.
    Server owners install Beta Bukkit and compatible plugins.
    Dev builds continue with new API features.
    Plugins start to use new API.
    Bukkit RB comes out.

    Repeat for next release... last step occurs IF the time 'tween releases is long enough.

    That flow above worked fine before this change, and it's gonna be the same? So why the flurry of OMG updates will be slower now!!
     
  12. Offline

    TheLoneDevil

    please show me these "thousands of lines" plus, the reasoning is illogical to me, and from the reaction of a lot of people, its illogical to them aswell. However i understad your frustration at people (including me) repeating arguments, so i will stop with this final comment, CraftBukkit, and the plugins released are released "as is with no garrantee that it will even work" IIRC, i may be wrong/ repeating an argument, so i apologize if i am. If i am right, why is this safeguard needed?
     
  13. Offline

    asofold

    Updates will most likely be slower now :).

    Edit: Server owners perspective, mainly.
     
    vgmddg likes this.
  14. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    See the same github comment thread where the thousands of lines of response were. Same place :)

    The safeguard reasoning is described in the first post of this thread. Please read it.
     
  15. Offline

    TheLoneDevil

    can i have a link to the github comments? i would like to read them, i didnt know it exsisted, for this i apologize.
     
  16. Offline

    hatstand

    ...But it still can. If a plugin begins some process with Bukkit API calls, and finishes it using NMS/CraftBukkit stuff, that operation is going to be left half-finished, and potentially destructive. This could, and probably would have happened any way, but this commit does not stop it from happening.
     
  17. Offline

    bergerkiller

    sablednah
    Do I really have to repeat those arguments again? *sigh*
    • Many plugins require that NMS code to even function. They can not use the API and they see no future in it being implemented in the API ever. PR submissions are waiting for months, feature request tickets are closed almost instantly, and there is no way to let the NMS code leak through the Bukkit API. Many of these plugins are *big* plugins, official plugins, or libraries. Those not being accessible for even 3 days is a disaster for server admins. Now imagine this happening every 2 weeks.
    • Spout Plugin has to hook into NMS, there is no other way. Now that is in many locations blocked, Spout Plugin will cease to exist, so will SpoutCraft. This was bound to happen anyway.
    • It is a much of work? It took me 4 days to get my stuff compatible again, and that was only the native path renaming and updating a shitton of other 'funny' things they thought was nice changing in NMS for no reason at all. That was no obfuscation.
    • That flow will no longer work, because 60% of the plugins servers will use will be broken every new MC change. They can close their server down for several days until they are updated. As well plugins will cease to exist, because they are too hard to maintain after these changes. Servers WILL go down, maybe forever.
    • The amount of new plugin download submissions will be too much for dev-bukkit approval to handle. Malicious plugins will slip through their fingers (and they will be made more often, people are pretty angry now) and admins have to wait for their plugins to be available.
    Now a shitton of arguments that target the changes:
    • There are alternative ways to provide the exact same system, which is safer, provides better messaging to the admins, and doesn't require days of updating for the plugin developer.
    • These changes can and will be bypassed. People will be using server forks of CraftBukkit that bypass it all the time, rendering issue tracking pretty much useless.
    • The changes are unsafe and do not prevent world corruption. It actually makes world corruption more likely, as it can halt the server in the middle of the game now because of a forgotten reference.
    • Dirty reflection will be used to bypass this, resulting in an increased chance of plugin developer mistakes, increasing the chance of server crashes and world corruption
    • These changes are too soon, they should have waited for an RB that lasts for more than 2 weeks, then it was acceptable. Right now, it is a huge inconvenience for everyone.
    Now some arguments that target the Bukkit project:
    • Plugin developers WILL leave (this includes me, I already decided to leave)
    • Many famous plugins will cease to exist, and Bukkit needs these to even stay alive as a framework
    • Server admins will post even more issue tickets relating to NoClassDefFound or other hard-to-spot problems caused by this. The errors are not at all fool-proof and not at all accurate.
    • By banning all use of CraftBukkit, the main server software itself becomes worthless, as forks will be created that replace CraftBukkit yet preserve that old functionality. They are shooting themselves in the foot.
    Now some arguments why this harms the community:
    • Developers that had to use NMS no longer feel represented by the current 'regime'. Some of them will quit development all together, flee, or will join up with other developers to create CraftBukkit forks.
    • The lack of communication by Bukkit and lack of discussion before even writing that commit is disturbing. There was no announcement at ALL that this was going to happen. For such a big change, I find that pretty depressing.
    • We hit the maximum comment count on that breaking commit on GitHub. We, seriously, had more comments than any other commit or page on GitHub. Think about it.
    • Why bother with Bukkit when, developers that develop in their spare time, have to spend triple the time to keep their stuff supported? Some people, me included, simply don't have the time to update.
     
    tssge, Technius and Dark_Balor like this.
  18. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    Of course somebody can always call methods that break things. This commit ensures that a server admin won't be screwed over because a NMS change results in the change of functionality.
     
  19. Offline

    sablednah


    OK - here's a direct reply to this point that is supposedly being skipped.
    TnT did not say "that people still wont check their code" he said "I don't see how editing a line from "1.4.5" to "1.4.6" is much more difficult than fixing the NMS code that will undoubtedly break.".

    They already have to edit their code - changing 1.4.5 to 1.4.6 (or whatever) is barely any extra work on top of any work they already need to do.

    Even if their code works fine, changing a bunch of 1.4.5's to 1.4.6's will show that they have at leasted looked at it.

    And yes if someone can just change numbers by script/hand whatever. And if they do then they are a twat of the highest order - and will be found out as such if/when things DO break.

    Bukkit team can't stop people being a twat. But they can change neglect (not bothering to check) - into an actual act of malice (claiming to check and marking code as such).
     
  20. Offline

    hatstand

    You're missing my point. The server owner still gets screwed by a plugin using NMS calls, and it would have previously. If the NMS stuff they were using executed as intended, the server would be perfectly intact. This commit does not protect against NMS functionality breaking.
     
  21. Offline

    bergerkiller

    sablednah
    You have NO idea how much extra work this is. I have had to do this, and let me tell you, if your plugins a big, like mine are, you will lose 2 to 4 days on fixing up all the references. Be it only because you have to double-check everything again (debugging EVERY single feature of EVERY plugin) as well, because you could have made a mistake somewhere. Also, do not forget the download submission times. They will be longer, so beta versions are not possible to be uploaded. Once your plugin is finally online, you can take it down again, because some people found a bug in them. This entire process of uploading and debugging is going to take far too long for server admins to handle, and plugin developers will try to provide 'unsafe' locations to host their plugins. There goes the 'safe' file submission system, hello malicious plugins.
     
  22. Offline

    HappyPikachu

  23. Offline

    Gravity Retired Staff

    No no, you're understanding what I mean by official. Anything on the Bukkit/Bukkit or Bukkit/CraftBukkit repository is official, it doesn't matter whether it's a dev, beta, or RB. What is not official is people who fork and modify CB code in order to add, remove, or modify things that they don't like. We cannot feasibly support all the unofficial builds, so we don't allow them.

    I won't address those specific points here, because they've already been responded to and delt with on the commit's page, namely on the FAQ post, and then a conclusion on EvilSeph's last post.

    I don't know where you would see that. If a plugin tries to call a class that doesn't exist, hence the versioning in the packages being wrong, the error will clearly display the plugin's name. The difference before was that when something failed, it failed "softly", meaning that it could take time for the issue to be visible, and if there was an error, you wouldn't always know where it comes from. Now, CB-using plugins fail if they are outdated, taking the guesswork out of hunting down the root cause of a outdated plugin problem. There's nothing here we're trying to
    "avoid". We've made this decision because we think it is the best solution to the problem, and without being presented with viable alternatives, we've stuck with it. This is our explanation of that.
     
  24. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    I think you somehow replied to my post without reading it :p

    You were supposed to be doing this every single minecraft version increment anyway. Though your apparent shock at needing to do this might explain all the NoLagg-caused issues reported in tickets that wasted our time in recent months.
    No they won't. Sorry.
     
    slipcor likes this.
  25. Offline

    bergerkiller

  26. Offline

    armed_troop

    Why not simply add a supported-version field in plugin.yml and check for that? It would prevent annoying, ugly NoClassDefFound errors, unless a developer is sloppy and doesn't properly update plugins. This would be in addition to the change that was done, which while I don't like, I unfortunately don't see going away anytime soon.
     
  27. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    Below is a quote from EvilSeph's giant FAQ aimed at developers

    Why not add a plugin.yml setting?

    As stated before, we will never add anything or make any changes to the project that encourage or enable the use of Minecraft or CraftBukkit internals - this change would do that. We don't want people to be building against code we have no control over. We simply can't provide any reassurances and it isn't something we can support, even if we wanted to. A plugin.yml setting that lets developers specify what version is required for their plugin to run would not only hinder developers that only use the Bukkit API, but it would also run a very real risk of pushing people to bypass the API and just use the internals.
     
  28. Offline

    sablednah

    I have NO idea?!? Please never assume.

    Whilst MobHealth may not use any NMS it does support many that do. And I do have several large plugins for a server I run that EXTENSIVELY use NMS to play about with entities and entity behaviours and much more. Not only that but they plug deeply into other plugins such as CityWorld. My testing involves creating and destroying entire worlds amongst several other things.

    I updated those plugins today.

    Long submission times? I had two updates approved in one day yesterday.
    Also Dev versions ARE unsafe - that's why their not tagged stable!. So by all means post them on your website.
     
  29. Offline

    bergerkiller

    sablednah
    Let me redirect you to my signature to see another nice argument.

    Let's say that, in my case, where NMS logic is so extended (you have no idea how extended), it is pretty much useless to continue. In your case, however, it could very well be there are one or two classes in your project that access NMS. In my case, however, there are about 200 classes that contain some sort of NMS logic. This was worse before, mind you.
     
  30. Offline

    asofold

    Well that is not very convincing, as there are quite some useful variations to it. Also given that Bukkit plugins can be tied to MC versions, this should not be discarded in the no-eyes-no-ears manner. This "we will never"-sentences really hurt my eyes for a part.

    Edit: I forgot to mention "imagination".

    Aww.. entire worlds (ridicule, must!).
     
  31. Offline

    bergerkiller

    Anyway I'm off, at least I know some people are still actively busy rounding up the activists.

    Also, since R1.0 is finally there, expect a request thread in a few hours, one day max.
     
Thread Status:
Not open for further replies.

Share This Page