-
Improvement
-
Resolution: Unresolved
-
Minor
-
None
-
3.3
-
None
-
MOODLE_33_STABLE
Given that a few days ago Google demonstrated the first sha1 collision, and given that there has been a major internal rewrite of the file system api in MDL-46375, now seems like an opportune time to refactor the hashing method used by the file system into a single place and make it easy to swap out with a better hashing function.
See also: https://shattered.io/
While it's probably not a practical attack for a while, I do however think it's inevitable that a bunch of students in COMP100 are gonna upload the test PDF's from shattered.io into moodle just to see what happens. Already in the wild some svn and git repo's have been busted by this and I think in the future we'll see more of it. At the very least we should be able to swap out the sha1 hash with the 'hardened sha1' which is essentially backward compatible.
So I'd like to see:
- refactor all uses of sha1 into a single static function somewhere
- add a way to override this at the config level - note this should be tangential to overriding the filesystem. You shouldn't need to extend anything in order to set a different hash
- perhaps rethink some of the api's so that clients of the filesystem api can use convenience methods instead of dealing with hashing functions inside another method.
ie instead of replacing this:
$fs->get_file_by_hash(sha1_file($fullpath))
|
$fs->get_file_by_hash(sha1($string))
|
with this:
$fs->get_file_by_hash(moodle_hash_file($fullpath))
|
$fs->get_file_by_hash(moodle_hash($string))
|
something like this so you aren't forced to deal with the hash:
$fs->get_file_by_path($fullpath)
|
$fs->get_file_by_string($string)
|
Bonus points:
- store somewhere in the filesystem which hash method to use
- implement hardened sha1 as the new hash method for existing / upgraded moodles
- implement sha256 for new moodles
I'm not sure about how to tackle filedir migrations between hashes, maybe that can be tackled later.