joomla-extensions / jedchecker

Joomla extension to check components, modules or plugins for possible problems for submission to the JED -> Translations: https://joomla.crowdin.com/joomla-official-extensions
38 stars 28 forks source link

Extension Checkup Signature #111

Open anibalsanchez opened 3 years ago

anibalsanchez commented 3 years ago

Another common issue is that people don't use JED Checker to check the extension before submitting it to JED.

The idea is to add a simple signature and render the signature ONLY when the extension is OK to be submitted (no red errors)—for instance, something like SHA1(MD5("file.zip")).

We are going to require the signature in the form. If it is not validated, then the extension can't be submitted.

What do you think? Is it doable?

CC: @dryabov @Llewellynvdm

Llewellynvdm commented 3 years ago

There has been much talk over the last two years about a hash validation for extension integrity. Would this be aimed at that?

I think if this is mostly aimed at blocking extensions that do not pass the requirement then we will have edge cases that do not pass, but are in fact allowed. Like JCB that has many many base64_encoding but used in a valid allowed way to store code in the database. How will these kind of cases be handled? since the base64 issue is a red error.

I think this is a great idea, but we will need to make sure we handle edge cases well and amicable before any of this is implemented, so the developers is able to get to the front-desk without the need of a big fight. Meaning if extensions do not comply but are valid there must be a way for the developer to appeal, an easy way (even from in the extension... possible). This is what I have always said... Joomla extension directory should be the kind of door keeper that is intelligent enough to grow with ease into best practice of the PHP industry as the architecture of the core Joomla dictates. We should not become a system that has rules for the sake of rules... or even preferences. We must acknowledge that there are large debate about implementation and therefore afford the extension developers the freedom they need, with the protection our community deserves. Showing developers best practice is one thing, but forcing them is not cool...

This is also why I have not merged some PR's since they have error messages where I think the nature of the detected implementation is not worthy to disqualify the extension.

So it will follow that we closely scan over all error reports to see if the weight they have is equal their importance. Yes I want to ensure quality, but not at the price of warping freedom software principles of our GNU GPL position.

anibalsanchez commented 3 years ago

The idea of this "signature" is only to confirm that people check the extension before submitting it. Nothing more and nothing else.

Of course, if we look into the big picture, the JED Checker could be the client that generates a hard signature to sign the download and confirm that the extension has not been tampered in any step of the distribution chain. We did something like that on GSoC EEM.

So, that's why I suggest that the simple signature that I propose is simply SHA1(MD5("file.zip")). Of course, anyone can look into the code and manually generate the hash. The aim of this signature is only to force people to use JED Checker and check the extension at least the first time, the extension is submitted.

Llewellynvdm commented 3 years ago

I can agree with that, I made that long... blablabla... because of this you said:

.... OK to be submitted (no red errors)—for instance, something like SHA1(MD5("file.zip")).

Only giving the hash if there is no errors require what I explained. But if we say, just because they tested the extension, we generate an hash... this is much easier and also a good idea.

We did something like that on GSoC EEM.

Yes I remember this, was part of that years team... but dropped out due to personal reasons. Honestly I think it will be brilliant if we could go the whole nine yards... meaning actually setup the path for this more secure distribution.

But in that case I would suggest we add the key to the update server xml and not on the JED. But okay that is a whole conversation in of itself. But worth doing....

Back to this idea as a more simple and almost "just to prove you checked" signature... I think it is a good start.

dryabov commented 3 years ago

It's a good idea to promote JEDChecker, but surely we cannot trust that JEDChecker is actually passed.

@Llewellynvdm As to base64, my opinion is that it's a data obfuscation and not the code obfuscation. To transform data to code it's necessary to either eval it or save to a file and include it (so, I'd mitigate this error to a warning message). I'm working on a rule that will detects all possible RCE, but it will take months to be finalized (hope to finally include CSRF, XSS, and SQL injection as well). For example, my current draft is able to found RCE in the following code:

      $task = $app->input->getCmd('task');
      $action = $app->input->getCmd('action');
      $data = $app->input->getTrim('data');
      $result = "{$task($action,$data)}"; 

(e.g. index.php?option=com_...&task=file_put_contents&action=eval.php&data=%3C%3Fphp%20...)

anibalsanchez commented 3 years ago

we could go the whole nine yards

I've been thinking a lot about if "strong secure distribution" can be done. Nowadays, I think it is not possible. The infrastructure requirements are very high and devops time would be huge. @Llewellynvdm

we cannot trust that JEDChecker is actually passed

Yes. That's why we always manually double-check the extensions. @dryabov