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

    sablednah

    ? that didn't even make sense?

    I'll assume it was some kind of dig tho ;)
    Yes one of my plugins is a world generator - and I build a 2k radius map to test it still all work and throws in the right schematics that my other plugins reply on to spawn specific custom entities. So yeah - my point was it's non-trivial.
     
  3. Offline

    asofold

    Worlds creation in Minecraft is not as difficult as in rl :p - but i should not cite you all the time ...
     
  4. Offline

    hatstand

    No, I read it fine. You're saying this protects servers from NMS version mismatches. It does not.
     
  5. Offline

    sablednah

    So I assume that you have full sympathy for the Bukkit Team, who have to do the same job whenever MC releases an update that they have no control over? ;) If I dig around in the forum I'll find your posts of support and encouragement in the MC update threads?
     
  6. Offline

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

    While hatstand and I have been chatting about this on IRC, I'll summarize:

    Before this change:

    Plugin X calls net.minecraft.server.World.q(true); which in 1.4 does what the developer intended but in 1.5 q(true) causes world corruption because it wasn't meant to be called like that in 1.5.

    After this change:

    Plugin X compiled for 1.4 calls q(true) but will not run on 1.5 until the dev updates and hopefully changes the method call. If the dev still calls q(true) for the 1.5-compiled plugin it's due to developer negligence instead of the server admin not knowing (who would?) that the plugin calls internal, volatile methods.

    This commit doesn't magically fix developer negligence, but rather ensures that any issues caused by nms calls are developer negligence.
     
  7. Offline

    asofold

    To say it with the words of my fellow teenage players:
    aaaaaaaaaaaaaaaaaaaahhhhhhhhhhh
     
  8. Offline

    V10lator

    Which is caused by server owners not checking if the plugins they use hook into NMS. Which is caused by no warning on DBO for plugins which do that (in this sentence I told your opinion, not mine). Which is not addressed by this safeguard but worked around in a bad way.

    If, like you say, staff already knows when a plugin hooks into NMS it shouldn't be that hard to add a checkbox for DBO staff to click when such a plugin gets updated (it could even be done automatically withing their script collection so no need for any manual action would be needed from DBO staff). This checkbox could print a big fat warning that bad things may happen if used with the wrong CB version. Now ask staff why they won't do that. You might be surprised. Also keep in mind that they never asked us to write such warnings for ourself.

    You should stop claiming you are staff as you aren't.

    Fun fact: Do you even notice that staff ignores (most of) my points?

    Now the same with this safeguard:
    • A function uses NMS code to see if a new chunk is being generated.
    • For the chunk generating/populating it uses (for whatever reason) NMS code.
    • It still looks like chunks are fine...
    • ...until a Minecraft update comes out and all new chunks are generated with the vanilla generator for a unknown reason.
    • You realise you have no idea what chunks are affected and what chunks aren't.
    • You realise you don't know why some chunks are vanilla and others aren't
    • So... backups are useless at this point because you don't know the areas affected and you don't know how long you have had this problem.
     
  9. Offline

    rossfudgew

    Hmm this in reference to a commit that you forked into your own packaging (which by your own definition is official), then a complete disregard for the swaths of people that disagreed with you. You locked threads and silenced the dissidents much akin to dictators, so don't give me the bull that something you put in a dev build that was brought up on the forums wasn't official until you put it on the forums as official. The code was official as soon as it went into bukkit. There is no need to "support" (and what I mean by this is that you do not need to offer technical support on the build itself, but you need to support it in the sense that you made it. Own up to the fact you put code in there that can potentially wreck s*** and don't give dev's s*** for disagreeing with your changes) the builds, but what you did to ensure that you got this into bukkit was done through sleazy and disgusting means that's quite lawsuit worthy (blocking peoples first amendment rights is not legal even if your parent corporation is in sweden)

    And see this proves my point, bukkit is only serving their own interests and not the interests of the community. They would rather ignore everyone and push a commit that the general consensus was that there is a better way of doing the same exact thing in a less controversial way. You were presented viable alternatives, you were presented with other ways of doing this numerous times in that github thread, but instead you choose to ignore the suggestions, silence people who disagreed with you, and block anyone who was going to do anything about it.

    EVEN IF YOU THINK ITS NOT A BETTER OR "Viable" WAY OF DOING IT
    Your Developer community seems to disagree with you at how to go about this

    Just listen to the damn dev's... They're smart enough to break your code otherwise
     
  10. Offline

    hatstand

    You forfeited your rights when you stepped onto this site. Welcome to the internet.
     
    rossfudgew likes this.
  11. Offline

    TnT Retired Staff

    Good idea, but lets take that a step further. Some people don't always go to DBO pages, get their plugin "auto updated" or straight out wget the links. So to ensure we get everyone lets put in something within the code itself that warns administrators when they are about to start a server with a plugin that ties into old NMS code!

    Sorry, I'll try to refrain.

    Perhaps we think "Hey, these points are addressed in that FAQ we spent hours carefully wording to ensure there would be no mistaken beliefs." or "Hmm, a lot of what he is asking for has already been addressed in the first post in this topic." Nope, guess we're just evil.

    You think we ignore you, but in reality we simply don't have the energy to respond to every one of your points individually when we've already addressed them multiple times in the first topic. You may not agree, and that's cool, but we have addressed everything you've asked.
     
  12. Offline

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

    We closed threads and directed people to use the commit discussion to ensure that discussion of the commit was in one place. A dictatorship would have prevented discussion, not facilitated it and ensured it was in one convenient place to reply to.

    Lastly there is no law breaking here. You are grossly misinformed about how the law works.
    We listened to all of the alternatives, and replied to them. See the numerous posts on the github conversation thread explaining how they are not viable alternatives. Nothing was ignored, and the only posts removed from the conversation were unrelated to the commit (accusations of trolling, accusations of law violation, and some image meme posts). We explained in excruciating detail why alternatives brought up weren't good solutions, even when people repeatedly posted them after we'd explained once or twice. We poured a ton of effort into responding to everybody in that commit thread.

    A small, vocal minority of developers hate this change, many of whom have already displayed a strong dislike for bukkit long before this commit (some even expressing a desire to leave the community months ago, yet still came back just to complain in the commit!). That is not our "developer community" but rather a small fraction of it. Many developers are very happy with this change, and most are ambivalent because they don't utilize the volatile internals of the server.
     
  13. Offline

    asofold

    With due respect: wrong movie?
     
  14. Offline

    hatstand

    We're all putting far too much effort into this argument -.-
     
    slipcor likes this.
  15. Offline

    sablednah

    Yeah - some of us have plugins to update :) (JOKE: sorry couldn't resist ;) )
     
  16. Offline

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

    I updated my plugins over a week ago! ^_^
     
    TnT and slipcor like this.
  17. Offline

    Technius

    hatstand And it's quite futile. We will never win.

    I can provide another case for this not preventing world corruption at all.

    • Plugin sets all the blocks in a region to air using the Bukkit API
    • Plugin schedules a runnable that will execute in 10 seconds
    • The runnable attempts to restore the blocks from disk using NMS
    • The plugin crashes with a NCDFE
    • There is now an "airy" region left
     
  18. Offline

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

    And that is an author negligence problem, as they didn't check version sooner than dying. See my earlier reply on the subject :)
     
  19. Offline

    Technius

    And you didn't read my part about the "NCDFE".
     
  20. Offline

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

    No, I did. The author should be checking versions in onEnable, especially now that version checking is so easy (package check).
     
  21. Offline

    V10lator

    If they think reading changelogs isn't important it's completely their fault, don't you agree?
    You can't hide behind the FAQ anymore cause it doesn't exist in this topic. Also if I directly reply to the first post how can it be addressed there? But yea, continue claiming that.

    BTW: More and more of my beta testers (which are server admins) tell me they stop hosting bukkit servers cause of last actions happening (SpoutPlugin dieing cause of bukkit actions, for example). But no, you don't harm the community.

    BTW²: I think I updated all plugins that need an update / are worth it, so I'm really leaving NOW.
     
  22. Offline

    Technius

    And if the plugin is dead..?

    Edit:
    And we need to check packages? They can't even add a simple getMinecraftVersion() call!
     
  23. Offline

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

    Then find somebody to update it. That somebody needs to go through all nms/obc methods it calls and ensure they still have the desired effect, tracing their path through nms. Just like the developer should be doing every update.
    There was no Bukkit use case demonstrated in your own pull request.
     
  24. Offline

    TnT Retired Staff

    Didn't you write an auto updating method that allowed for downloading any newly updated plugin - with no regard for the changelog whatsoever - blindly onto a server. Do you not understand how its not so cut and dry to just put up a warning on DBO?

    The FAQ isn't invalid just because you want it to be invalid. Sorry you feel I'm hiding behind well written counter points to the arguments.

    We will talk to the Spout devs. I'm sure we can help them figure out why something completely unrelated to this change (we implemented ItemMeta, they have code incompatible with it) has broken their plugin.

    But what about next time? Are you abandoning your plugins? Just curious, so we know what to do with your projects.
     
  25. Offline

    Coelho

    This is quite excellent. Now just like on the git commit we can create another 1500 comments saying why we disapprove of this and redo the same thing over and over again.
    • Server owners lose because they can no longer please their players and have to drop a huge amount of plugins every time CraftBukkit updates. Due to this, only servers with the resources to pay for inactive plugins to be updated will be able to prosper.
    • Hosting companies receive more complaints. Sorry Bukkit, but the imbeciles don't come to you. They go to their shared hosting company.
    • Paid plugin developers gain a huge amount of money from this, while free plugin developers lose their time.
    • BukkitDev moderation loses their time.
    You are helping all the wrong people and turning the Minecraft community even worse than it already is. If you have money or development skills, you will prosper now. If you don't, you'll rot in a ditch and be kicked aside.

    Given, I shouldn't be complaining, but I have concern for those who don't have those qualities. It will make CraftBukkit less desirable.
     
  26. Offline

    V10lator

    TnT as you're almost begging for it I'll reply one last time:
    Not blindly. It printed a link to the DBO changelog for exactly that reason.
    It is invalid HERE, this isn't GitHub.
    Also all questions about points in the FAQ (not points that are answered, but that are asking for more information) are invalid for you, so yes, that makes the FAQ invalid for me.
    It is broken cause they need access to the constructors to overwrite the classes to allow custom items.
    That's covered in my goodbye thread:
     
  27. Offline

    dark navi

    While I am not here to flame, it seems like you guys are discouraging the use of code because it could be unsafe.

    While I respect the fact that BukkitDev is screened for malicious code, as a developer I feel that we're being somewhat punished for wanting to use code that could break.
     
  28. Offline

    Gravity Retired Staff

    Please explain to me how you come to the conclusion that we loose our time?
    You're not staff, you don't know what our process is nor what would/wouldn't help us because you've never had to approve a file, much less approve hundreds per day. How is it, then, that you know this? And why would any other option be better in this regard?

    Since when have plugins been pay-to-update? There are thousands upon thousands of plugins out there created by developers who just want to make cool stuff, and improve people's servers. Whether it's a plugin using Bukkit only or a CraftBukkit-using plugin, there are developers who support their code. If they don't, you shouldn't be using their plugin.

    I think we can safely say that, by the amount of reports we get that are created by plugins and not CraftBukkit, the "imbeciles" do indeed come to us. If people were reporting every error to their hosting company already, they would continue to do so. Most people who run Bukkit, however, know that it is unrelated to their host, and bring it here. Instead of trying to hunt down something that doesn't exist in our code, this means we point them towards where it's coming from with none of the guess work. How are you coming up with this? Do you run a hosting company, have you dealt with or seen this or are you just making it up?

    Again, this commit really changes nothing in that regard. If paid plugin developers were charging for updates, they will continue to do so. This changes the fact that instead of using something that is broken with a new update and potentially can ruin your server, you are getting an update for it.

    You're also continuously exaggeration the process of updating with this safeguard in place. You can speak to many plugin developers who use CraftBukkit internals who are not having the same troubles as you are because instead of utilizing their time trying to complain to the staff, they are working on their code, and have updated in a very reasonable time.

    Also, I'm a free plugin developer, and I haven't lost my time. Your arguments make it seem like everyone who does this for free will simply give up because of this. Not so; because of this I've taken the initiative to move my plugins from using CraftBukkit code to using strictly the Bukkit API, thus improving them significantly, and adding new API for everyone to use. I'd hardly call that a waste of time, the time spent arguing over this is probably more time than you'll spend updating.
     
  29. Offline

    charlie_boi

    I certainly see the benefits to this change but I don't think they outweigh the problems its going to cause, I've already seen several respectable plugin developers say they are leaving bukkit because of this.

    Its going to take a few months at the least for all of the plugins to catch up to this update and a lot never will catch up because they have been discontinued or abandoned.

    Asking/requesting that someone else takes up or updates a plugin seldom happens so really its just causing more problems by making us as server admins drop important plugins that we have come to rely on.

    I fail to see why you think it is up to you to force this upon us just because YOU think its your problem not to "let servers burn in the coming weeks" Why can't the community make that decision?
     
    vgmddg likes this.
  30. Offline

    Coelho

    I exercise common sense. I also love it how you failed to comment towards the rest of my post. I also love it how you failed to make sense of my alternative as I stated on the git commit comments either. I'm not even going to waste time explaining it again.

    The first step to finding an alternative is removing this. The second step is formulated an alternative through the minds of not one individual but hundreds in a quick driven environment.
     
  31. Offline

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

    I don't see it as a punishment but rather a really well defined encouragement to add multi-version support to my plugins. :3

    He's only responding to the part of your post he feels he is best qualified to answer, seeing as he's the BukkitDev lead. By all means, claim conspiracy.

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

Share This Page