? ? ? ? Pending

User tests: Successful: Unsuccessful:

avatar alikon
alikon
19 Aug 2017

light porting of #17555 from 4.0 to 3.8

Summary of Changes

trigger the checksum check on install/update
if the update server manifest have an hash tag like these:

hashmanifest

only return:

  • notice when there is no hash tag in the update manifest
  • warning when a checksum ha failed
  • message when checksum is OK

the installation/update works as before

Testing Instructions

see #17555
the main difference is it install anyway so there is no force button

Expected result

checking the integrity of the package with: SHA256,SHA384,SHA512 hash algos

Actual result

no check is made

Documentation Changes Required

new tags <sha256><sha256/><sha384><sha384/><sha512><sha512/> on update server manifest

b87c8ab 19 Aug 2017 avatar alikon lang
avatar joomla-cms-bot joomla-cms-bot - change - 19 Aug 2017
Category Administration com_installer Language & Strings Libraries
avatar alikon alikon - open - 19 Aug 2017
avatar alikon alikon - change - 19 Aug 2017
Status New Pending
avatar alikon alikon - change - 19 Aug 2017
Title
[3.8] - checksum
[3.8] - checksum extensions light
avatar alikon alikon - edited - 19 Aug 2017
avatar alikon alikon - change - 19 Aug 2017
The description was changed
avatar alikon alikon - edited - 19 Aug 2017
avatar alikon alikon - change - 19 Aug 2017
Labels Added: ? ?
07fef5e 19 Aug 2017 avatar alikon tab
avatar mbabker
mbabker - comment - 19 Aug 2017
  1. This should be incorporated into com_joomlaupdate as well so core can use it.

  2. We need to make a decision on what hashes to support. In 2017, md5 and sha1 aren't great options.

avatar brianteeman
brianteeman - comment - 19 Aug 2017

We need to make a decision on what hashes to support. In 2017, md5 and sha1 aren't great options.

sha256 ?

avatar mbabker
mbabker - comment - 19 Aug 2017

I can't say I know much about sha256's security off hand.

avatar alikon
alikon - comment - 20 Aug 2017

This should be incorporated into com_joomlaupdate as well so core can use it.

sure, can we do in a separate pr ?

We need to make a decision on what hashes to support. In 2017, md5 and sha1 aren't great options.

sha256 should be OK nowdays ...or not ?

avatar alikon alikon - change - 21 Aug 2017
Labels Added: ?
avatar alikon alikon - change - 21 Aug 2017
The description was changed
avatar alikon alikon - edited - 21 Aug 2017
avatar mbabker
mbabker - comment - 21 Aug 2017

I already know what he's going to nudge toward but I'm going to ask an opinion anyway.

@paragonie-scott are there any major issues to be aware of with using sha256 or sha512 with hash_file() as a means for validating a checksum of an extension (or our core) package as a short-to-mid term solution? (Clearly we could/should continue to work for better options, so right now this is basically a starting point; the one thing this wouldn't address is a compromise of anyone's distribution platform)

avatar alikon
alikon - comment - 22 Aug 2017

in the gsoc 17 EEM meeting yesterday, i've discovered that there are some 3dp extensions that use
md5/sha1 like ARS, so apart the discussion about weakness, should we consider to support those weak algos?
one step forward (sha512) and one step back (md5,sha1)

avatar mbabker
mbabker - comment - 22 Aug 2017

I'm well aware that ARS generates them. That still doesn't necessarily make md5/sha1 good to introduce into new APIs in the core libraries in 2017 as a means for validating package integrity.

avatar paragonie-scott
paragonie-scott - comment - 22 Aug 2017

are there any major issues to be aware of with using sha256 or sha512 with hash_file() as a means for validating a checksum of an extension (or our core) package as a short-to-mid term solution?

There's a length extension attack, but that doesn't break preimage security, and since it's an unkeyed hash, it's not giving the attacker a capability they don't already have.

However, if you want to be conservative here, follow Composer's example and go with SHA384 specifically.

avatar alikon
alikon - comment - 22 Aug 2017

so we can go with: sha256,sha384,sha512 ?

avatar mbabker
mbabker - comment - 22 Aug 2017

That seems fair to me.

avatar alikon
alikon - comment - 22 Aug 2017

now hash supported are:

  • sha256
  • sha348
  • sha512
avatar alikon alikon - change - 22 Aug 2017
The description was changed
avatar alikon alikon - edited - 22 Aug 2017
avatar anibalsanchez anibalsanchez - test_item - 7 Nov 2017 - Tested successfully
avatar anibalsanchez
anibalsanchez - comment - 7 Nov 2017

I have tested this item successfully on 2f3425f

Tested OK, as part of #17632

For more info about the test and comment: https://issues.joomla.org/tracker/joomla-cms/17632#event-3


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17619.

avatar NunoLopes96 NunoLopes96 - test_item - 10 Nov 2017 - Tested successfully
avatar NunoLopes96
NunoLopes96 - comment - 10 Nov 2017

I have tested this item successfully on 2f3425f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17619.

avatar franz-wohlkoenig franz-wohlkoenig - change - 11 Nov 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Nov 2017

RTC after two successful tests.

avatar joomla-cms-bot joomla-cms-bot - change - 22 Dec 2017
Title
[3.8] - checksum extensions light
checksum extensions light
avatar joomla-cms-bot joomla-cms-bot - edited - 22 Dec 2017
avatar mbabker
mbabker - comment - 14 Feb 2018

@wilsonge Can we get this and #17632 merged into the 3.9 branch? IMO now would be a good time for the code to hit core and start documenting the hell out of the new functionality via the docs wiki and a targeted blog post on the dev network.

avatar wilsonge wilsonge - close - 8 Mar 2018
avatar wilsonge wilsonge - merge - 8 Mar 2018
avatar wilsonge wilsonge - change - 8 Mar 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-03-08 22:05:54
Closed_By wilsonge
Labels Added: ?
avatar wilsonge
wilsonge - comment - 8 Mar 2018

LGTM :)

avatar wilsonge
wilsonge - comment - 8 Mar 2018

Well bugger - didn't see this was against staging. Reverted with #17619 and going to manually re-patch against 3.9 now

avatar wilsonge
wilsonge - comment - 8 Mar 2018

Merged second time lucky with f18c20a

avatar alikon
alikon - comment - 12 Mar 2018

@wilsonge thanks
can you consider to evaluate/merge to 3.9 #17632 too after conflict has been resolved ?

Add a Comment

Login with GitHub to post a comment