Security issues brought on by the plugin portal and the new 'plugins {}' syntax

ok thanks! i was having trouble understanding this because it looks like a simple implementation enable attacks.

so the ‘gradle.plugin’ prefix solution you have here fixes one problem: the ability to create globally unique artifact coordinates. but there seem to be 2 other problems that i would classify as security holes for users of gradle.

first let’s see if i got this right:

if i publish a plugin with:
-group: ‘com.foo’, name: ‘project-gradle’, ver: 1
-plugin id: ‘com.foo.project’
-no mvn coords

then i get:
-an artifact published automatically without human intervention
-with coords: ‘gradle.plugin.com.foo:project-gradle:1’
-and plugin id: ‘com.foo.project’

that i can apply using the new style like this:
plugins {
id “com.foo.project” version “1”
}

if this correct?
if so, here are the 2 security holes:


#1)
i can define a plugin ID (ie: com.foo.project) outside of my real, final group ID (ie: gradle.plugin.com.foo), so i can publish this:
-group: ‘org.android.’, name: ‘i-will-erase-your-drive’, ver: 1
-plugin id: ‘org.android.compress-resources’
-no mvn coords

then people adding this to their build:
plugins {
id “org.android.compress-resources” version “1”
}
will get their home dirs wiped.

this is unacceptable. the mere sight of the prefix ‘org.android’ will immediately inspire the assumption that the imported code is controlled by who controls the ‘org.android’ domain. or if not that, at least the publishing of ‘org.android’ artifacts. note that i didn’t refer to the artifact anywhere with the new syntax so i never know that the pulled-in artifact is actually from ‘gradle.plugin.org.android’.

how to fix:

you need to deny auto-approved publishing to plugins when the real, final group ID isnt a prefix of the plugin ID.

so the plugin id should be declared ‘gradle.plugin.org.android.compress-resources’ for auto-publish to work. this plugin id prefix would still enable phishing attacks, but at least you now have to fall for it instead of being tricked by the gradle toolchain itself.

given this requirement, it seems natural to also require the declared group to be the final real one. ie, declared groups should also start with ‘gradle.plugin’ to be candidates for auto-publishing, and the auto-prefixing you have now should be eliminated.

many people would not do this prefixing correctly the first time they publish. so the next version of the plugin publishing plugin should abort if any of those fields are not correctly prefixed (outputting and explanation of how to fix the situation) instead of submitting the plugins for manual approval. plus, there should be a new DSL property in the plugin to enable the manual approval process when required. this should keep gradleware’s manpower usage at a minimum.


#2)
fix #1 by itself does not provide any kind of trust for auto-approved plugins. it just forces them to have a plugin id starting with ‘gradle.plugin’.

so the message is:
-if you apply a plugin outside of ‘gradle.plugin’, you know that the ID is linked (by human intervention) to the purported origin, so there is some trust there, if you trust gradleware’s due diligence and the origin itself.
-but inside of ‘gradle.plugin’ its a jungle! no way of knowing where the plugin came just from looking at the ID! so either research online or dont apply it!!

the last part is exactly why serious developers wouldn’t want a prefixed ID implemented like this, and would be asking for manual approvals, which costs money to gradleware. so you have an incentive to make the auto-publish trustworthy.

how to fix:

the solution is to be even more restrictive with the prefix for auto-publishing. the required prefix should be something like ‘gradle.user.${username}’, where username is the publisher’s account name at your gradle.org site. (everything outside this prefix should fail to auto-publish, printing out how to fix the issue.)

(that is, unless the developer sets the DSL flag to request manual approval. in that case, there could also be a proofOfOwnershipUrl property through which the developer can point to some proof while submitting the plugin. for example, i can point it to a github gist i create saying i am so-and-so on gradle.org, thus proving that this gradle account also owns FunnyCat in github. once approved, i can publish under com.github.FunnyCat automatically.)

the ‘gradle.user.${username}’ scheme is prone to fishing too, because it implies some curation by gradle. say i register user “google” on you site. then i can publish under ‘gradle.user.google’ which is not nice. so some obvious names should be blacklisted from the site. but you can also do more:

IMHO, it’d be better to use ‘gradle.untrusted.${username}’ which destroys the illusion of curation. (in the same vein, google uses a different domain altogether (googleusercontent.com) for the user-supplied so that it is not even under google.com.) if you dislike the word ‘untrusted’ a less extreme variant you might like is ‘gradle.community.${username}’.

note that this creates the opportunity and temptation to grant ‘gradle.trusted.${username}’ to verified accounts. but it’s better to just approve their own domains and get gradleware off the picture completely. (ie: ‘com.google.’ in favor of 'gradle.trusted.google.’, although the second text really describes what is actually happening better.)


sorry for the long post, i just wanted to propose some improvements!

thanks again!