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

    Gravity Retired Staff

    If you don't understand the reason that we first denied it, it's better just to let those who are more experienced with CraftBukkit deal with this. Its not your fault or anything that you haven't seen how the Bukkit, CraftBukkit, and Minecraft source work together, but its not going to be easy to suggest viable options if you don't have that kind of experience.
     
  3. Offline

    Coelho

    Plugins are expected to use the Bukkit API. It is volatile (can potentially cause damage or breakage) to use the CraftBukkit internals, and as such Bukkit does not encourage it. Given, adding this setting would be 1) messy and 2) portray that Bukkit encourages the use of CraftBukkit internals and volatile nature

    I can't explain it simpler.
     
  4. Offline

    WeaselBuilds

    If the old way (please tell me if I'm wrong because I just found out) was using, say, net.minecraft.server.World it would add the version like net.minecraft.server.v1_4_5.World? So it wouldn't be a hard or long fix, just maybe a few tweaks (sorry again if I'm wrong and I'm trying not to accuse or say false things)?

    EDIT: I saw the new posts and I see that it causes a problem with this, so it's better to use the Bukkit API from now on?

    ~ Weasel
     
  5. Offline

    Gravity Retired Staff

    Precisely.
     
  6. Offline

    Coelho

    In the commit right after it was changed to add a 'v'.
     
  7. Offline

    WeaselBuilds

    So just use Bukkit API and not Craftbukkit, right, but will it make a difference?

    Thanks
     
  8. Offline

    Gravity Retired Staff

    Right indeed. Ignore my original correction.
    The simple rule-of-thumb is that if you don't know you're using CraftBukkit or don't know any reason why your plugin would be, you don't need to be using CraftBukkit. Bukkit is much more resistant to change upon Minecraft updates unlike the CraftBukkit change which is constant adaptation to provide for the needs of each Minecraft version.
     
  9. Offline

    aaronfranke

    Ok, but one last thing: If the bukkit team was added to Mojang, why is bukkit not the vanilla server, why does it take more than a few hours to make a craftbukkit build, and why can't Mojang add things to the client and server to make the code work better together?
     
  10. Offline

    chaseoes Retired Staff

    There's an online-mode option in server.properties. Does that mean running an offline mode server is endorsed? :p (I agree with you, though).
     
  11. Offline

    WeaselBuilds

    Ah ok. Thank you. BTW y'all doing a great job. :D Can't wait for 1.4.6 :)
     
  12. Offline

    rossfudgew

    • Advertising is still advertising even when using URL shorteners.
    What I don't get is why they actually care if an issue arises from plugins.

    Using plugins is a risk. You take a risk expect that there can be consequences and rewards. Why let people be responsible for their actions? If a server admin kills his server it must be our fault for not protecting them. I mean while were at it lets just kill any useful plugin that uses methods that might just make sense because they could break someone's server. Lets kill any plugin that might makeup for the short comings in our own code so people can't figure out flaws in minecraft and bukkit. In fact lets force developers to use an api they don't want to use to hide the flaws in our code since we maintain the api.

    Oh wait were serving our own interests again...

    Many plugin developers that make cool things (not just boring server administration stuff like permissions and ranking) use net.minecraft.server and org.bukkit.craftbukkit code to make it so a player sticks to a dragon, or that pigs can fly. Killing the ability for devolpers to use this code makes it harder for server admins to differentiate their servers. I mean how fun is just a survival server, when all the other servers are survival servers. Players get bored of the same old stuff and enjoy going to other servers to play with cool stuff. This is why plugins like dragon travel, spout, NPC's, traincarts, signlink, multiverse, and ect were popular. You could make a skylands world, or city world, or ect with using a map generator, put in a subway system or fly people around on dragons for transportation. Now we cant do that, because the developer community knows they won't be able to keep up with the requests to fix stuff that you broke. There is a reason dev's version their plugins and use licenses that stipulate their not responsible if stuff breaks.

    And for your information (since I saw it was brought up) its the general consensus in the spout community that the spout plugin should just die because this commit makes it so that spouts function (A plugin that allows for custom blocks by interacting with the block handler in NMS code) If you don't believe me or would like to pretend this is not the reason why I invite you to read their thread explaining their options and choices. http://goo.gl/

    //edit If you would like to remove my link again that is fine, but this is relating to a plugin and not an "unofficial" build of bukkit. I used goo.gl to remove any offending site name that might have originally been in the link

    Also this is the kind of thing I am referencing when you are silencing people's opinions on a situation you created but rather not deal with
     
  13. Offline

    SniperFodder

    I'm so confused. I had no idea there was a difference between bukkit and craftbukkit... I guess I can try to convert to Bukkit API... ?

    Well, I like how essentially half of my plugin just died with this commit since SpoutPlugin is probably going to be discontinued.

    So, when is is the Minecraft API going to get released so I can hurry up and port my plugin over? :p
     
  14. Offline

    Gravity Retired Staff

    The main Bukkit team members were hired by Mojang to help make Minecraft itself better. Right now, CraftBukkit is an addition onto the release version of Minecraft (which has obfuscated code). Instead of dealing with these updates every single time, the Bukkit team is working on an internal API that will be present in vanilla, and will not need to be updated after-the-fact, that is - after each new release.

    The release code for Minecraft and the code that they work on over at Mojang is very different I've been told, and so just because the developers of Bukkit are familiar with the core concepts and the logic behind features in the game does not mean they will be able to pick up the public version of the code and work with it just as they do at Mojang. This is combined with the fact that it just is a fairly laborious task to update all the new methods in CraftBukkit - everything has to be checked to make sure changes are accounted for and reflected in CraftBukkit code.

    I'm not entirely sure what you mean by your last question, but they have recently switched to a method of unifying the SP and MP code so that it requires less work to update each. That's physically what I think of when I see your question about the client and server working together, but you might also mean why can't the stuff we do here (at Bukkit) be present by default in the Minecraft code. If that's the case, you probably already got this question answered above; the team is working on that!
     
  15. Offline

    Coelho

    TnT Are you being serious... I dislike Spout as much as the next guy but really?!
     
  16. Offline

    Gravity Retired Staff

    We've said it before: we don't support unofficial builds. That has always been our policy.
     
  17. Offline

    Coelho

    That link was a link to Spout's thread on discontinuing Spout plugin. It had no unofficial builds of any kind. Also, there is a difference between not supporting and restricting.
     
  18. Offline

    Gravity Retired Staff

    Look again.
    If we were to allow people to discuss unofficial builds here, we would have to support them in one way or another. In any case, this is not the place for policy discussion. Our rules on this have always been the same, and are still the same here.
     
  19. Offline

    EvilSeph Retired Staff

    When a plugin breaks, do you know who people run to first? You'd think it would be the plugin developers, but no. The vast majority of people go to our issue tracker, Leaky, and report the issue there first. Only after we've looked at the report and responded that it is a plugin issue do people go to the plugin developer. Actually, most of the time people stubbornly insist it isn't a plugin even if we know for a fact it is.

    This isn't about ordinary plugins, it is about plugins that use volatile and unpredictable Minecraft and CraftBukkit internals within their code. Some developers have put plugins out with the belief that the volatile code they're depending on will and has never changed but this is not the case. This, without a doubt, is placing servers at risk and is unacceptable. If you're going to be using code you have no control over in your plugin, you need to be responsible for it and take the initiative to re-evaluate your plugin whenever a Minecraft update is released.

    And people say we're bringing unneeded drama... Using plugins isn't a risk. If it is when it comes to your plugins... then maybe you should re-evaluate how you're going about designing your plugin?

    Before you continue participating in this discussion, it is extremely important that you grasp this fact: as covered in the announcement, we are not doing anything to "kill" the ability to use Minecraft or CraftBukkit code. We are simply pushing plugin developers that do to be fully responsible for their actions.

    As stated before, the breaking of SpoutPlugin took us by surprise and prompted us to contact the developers to see why they are doing things the way they are and work with them to try and get SpoutPlugin working again.
     
  20. Offline

    fashizzles

    So many complainers.. be grateful they even offer this server software.
     
  21. Offline

    Coelho

    This is an open source project.
     
  22. Offline

    fashizzles

    Who is going to go out of their way and spend days coding something like this? I'm sure that most people wouldn't, since more than half of them don't even know the basics of Java. I know it's open source, but who would spend the time doing it?

    That's what I mean by being grateful, they are doing all the work for you really.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 30, 2016
  23. Offline

    asofold

    I think the pull request shows a problem. It was declined very fast, staff did not use their imaginative skills, and due to closing it so fast, other people could not comment on a Bukkit use case. I think a Bukkit use case is very easy to find, since Server.getVersion is just unspecified in the API, so effectively one does not know which Minecraft version is running, but it can be important for a Bukkit plugin to know such (protocol/bug changes, block types....). So either you add javadocs to the API or you something similar to add getMinecraftVersion. Old "discussion" rehashed.

    Edit1: *(javadocs specifying what Server.getVersion returns or you add ....)

    Edit2:
    Both air and numbers are a little thin on this topic in general.
     
  24. What does he have against Russians?

    ---
    Ok, so all he said in a nutshell:
    We will have to make plugin developers rethink their strategy in making plugins (I.e Stop using Minecraft code), and therefore keep server running smooth and clean.
     
  25. Offline

    c0mp

    Mostly correct, with the exception of the above statement. From the original post:
     
    vgmddg likes this.
  26. Offline

    JacobiCarter

    And this is why my server runs a custom fork of CraftBukkit and why we don't upload it's 20 custom plugins to bukkitdev. I do a full-tree diff and review it carefully every time I merge the latest bukkit/master or bleeding/master -- especially for NMS version updates, and I know what changed, what methods changed, and which plugins touch that part of NMS.

    Now, I have two choices:
    1. Unshade NMS, and rewrite the imports on the few bukkitdev plugins I use.
    2. Update all 20 of the custom plugins every update even when I know that the parts of the code that they touch haven't changed.
    *sigh*
    As for those saying that server operators cannot tell if a plugin used NMS before, they could always just look at the plugin source if open source, or if not, scan the constant pool for references to NMS.
    Also, any server that worries about corruption really should have a test server and use it to test thoroughly before promoting a new version to production, also get users who want to test on it, too, set up identically to the main server. They should also keep hourly backups of the prod server.
    This really seems like the bukkit devs are saying that their users are incompetent so they should idiot-proof the upgrade process -- which is a nice thing so long as it doesn't unduly inconvenience the competent operators. However, in this case it does.
     
  27. Offline

    rossfudgew

    Then you do what you always do, close the request send them to the plugin developer. As ironic as your statement is here is an example of the idiots already spamming you over the noclassdeffound error
    https://bukkit.atlassian.net/browse/BUKKIT-3208 (ironically closed by the guy who wrote the commit)
    This doesn't fix this issue so myth one is debunked.

    Anyone who has ever coded anything Knows that if you reference something in version A that isn't there in Version B, whatever you have coded will not work. That would be like saying to a website reference stock photo A then removing the stock photo and expecting the picture to be there miraculous the next time you load the page. If the developer references something that is say Living.Entity related, and that gets remapped, what ever they coded won't work until they remap and look over the code.

    Your argument is for cases in which you may change the referenced code without informing the developers of doing so. Its more of an excuse for you as an adminstrative team to communicate less with the community then you do now, all while pushing away the best and brightest plugin developers who would go back through, reference what has changed and fix what needs to be fixed. Its not like a living .entity or many other numerous actions that can be referenced in the NMS and OBC code will corrupt chunks. This will really only happen if people run world generators and with world generators there is risk in using them in the first place. (yes they do cause corruption).


    Unfortunately legally speaking plugin developers are in no way responsible for their code if issues arise, just like bukkit isn't liable to provide any warranty. Forcing people to be moral and ethical is in no way better than trying to be governement. Even if you have clout in the MC world, you piss off the right people and they will go out and do something about it. Why do you think people in china use proxies to avoid censorship? If you force the devs to do something they don't want to do eventually they'll just go around you.

    It took you by surprise eh? You knew exactly what you were doing when you pushed this commit. Don't try to say oh we didn't expect people who make mods to be upset. We didn't expect this to break crap. That would be me like saying "I'm gonna go through and demote everyone on my server one rank" then scratching my head when people don't exactly embrace the change.
     
  28. Offline

    feildmaster Retired Staff

    The safeguard had nothing to do with SpoutPlugin breaking (and them "stopping" their work on it), of course it would break. It more than likely would break every version anyway.

    Thanks for trying to confuse the situation.
     
  29. Offline

    andrewpo

    I'll miss bergerkiller 's traincarts/my worlds :(
    Now I have a useless world-wide train network.
     
  30. Offline

    sablednah

    It does mean tickes can be resolved instantly without having to examine them or get into discussion.

    Instead of hours or days of double checking in case it is a Bukkit bug - now it's as soon as a dev see's it

    Entities have routines that can damage/change blocks too.
    Imagine I made a mod to arm a creeper. I use use a NMS to give it a sword (276). Update occurs. NMS routine shifts to "set explode strength"... that's gotta hurt. Or more subtly - said 276 is applies as experience gained when killing. 1 week later... why are all my players level 50? Damage doesn't just mean blocks/chunks.

    Yes this is unlikely - but you can see how stuff like this is possible can't you? (and yes I know the give item is zombies + skellies atm - but you can surely extrapolate and see the potential?)

    Now imaging changes are going to get more wild this year...

    No one said legaly?
    Also they arn't forcing Morals. Just responsibility. Devs can be douch-bags if they really want too... just that they will have to deal with the fall out.

    Its not the safeguard that changed - it was a final class that broke spout. Part of the same commit - but not part of the safeguard. 2 separate things. And a thing Bukkit team have said they are offering spout help with, so what more can they do?

    UPDATE: SpoutPlugin is alive!
     
  31. Offline

    asofold

    How do they now deal with fall out? Do you mean the stack-traces due to a plugin using NMS/OBC at all will create the fall out on the devs, which effectively would then let them be the douche bags just for accessing NMS?

    The real bad ones (bypass this change, update without checking or even just update and accidentally overlook the NMS changes) will pose no difference to before, once released they provide exactly the same tickets and bug reports.

    What this change does is to rule out mismatching versions, and what it lets us gain on (without regarding other side effects and cross-dependencies) is ruling out plugins that use NMS/OBC but *never* update. All other stuff is uncertain and i am looking forward to not ever being able to judge it, because too many parameters will change and one never will be able to really tell what the change then really did (unless it turns out to be completely disastrous, which i don't assume to be too probable). But it should be clear that having plugins fail before enabling (i.e. not enable at all) would be much better. Thus evaluation of alternatives might take place in the future, from whichever side.
     
Thread Status:
Not open for further replies.

Share This Page