Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace SHA1 checksums as they 'practically' broken #59

Closed
emanuelb opened this issue Mar 21, 2017 · 4 comments
Closed

Replace SHA1 checksums as they 'practically' broken #59

emanuelb opened this issue Mar 21, 2017 · 4 comments
Assignees

Comments

@emanuelb
Copy link

The SHA1 hash was practically broken (regarding collision resistance), see:
https://shattered.io/
thus it's not recommended to use it in scenarios that relay on collision resistance.
Regarding PMA it's mean to remove it's usage as checksum for downloads.
(HMAC-SHA1 [cookie auth + github api] is still safe as it's security not depend on resistance to collisions)

fix:

  1. remove SHA1 checksums usage for downloadable files in:
    https://www.phpmyadmin.net/downloads/
    https://www.phpmyadmin.net/themes/
  2. optional: add BLAKE2 checksums instead (b2sum tool), more information:
    https://www.gnu.org/software/coreutils/manual/html_node/b2sum-invocation.html#b2sum-invocation
    https://leastauthority.com/blog/BLAKE2-harder-better-faster-stronger-than-MD5/

SHA256 is considered secure, it's not broken even theoretically.

nijel added a commit to phpmyadmin/phpmyadmin that referenced this issue Mar 21, 2017
As there is now way to generate collisions (and tar happily ignores
additional payload), using sha1 probably no longer makes sense here.

See phpmyadmin/website#59

Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel nijel closed this as completed in 91a9973 Mar 21, 2017
@nijel
Copy link
Contributor

nijel commented Mar 21, 2017

I've removed displaying of SHA1 hashes. It can't be completely removed as it's necessary for composer package list (see composer/composer#5940).

Also I don't think that introducing something else than SHA256 makes sense right now.

@nijel nijel self-assigned this Mar 21, 2017
@emanuelb
Copy link
Author

I've removed displaying of SHA1 hashes.

more places to remove:

<a class="btn btn-success download_popup" href="{{ file.get_absolute_url }}" title="Download {{ file.archive }} compressed release, {{ file.size | filesizeformat }}" rel="quick-download" data-sha1="{{ file.sha1 }}" data-sha256="{{ file.sha256 }}" data-pgp="{{ file.get_signed_url }}"><i class="fa fa-download"></i> Download {{ release.version }}</a>

https://github.com/phpmyadmin/themes/blob/d77636525be8ff713fa67a3ad48f4e97f9d130ea/create-release.sh#L76
https://github.com/phpmyadmin/themes/blob/d77636525be8ff713fa67a3ad48f4e97f9d130ea/create-release.sh#L97

does all the below instances required for the adding of SHA1 to packages.json ( in composer usage) ?

several mentions in (lines: 38-39, 44)

from hashlib import sha256, sha1

sha1 = models.CharField(max_length=40)

sha1 = models.CharField(max_length=40)

download.sha1 = read_sum('{0}.sha1'.format(filename))

It can't be completely removed as it's necessary for composer package list (see composer/composer#5940).

Yeah, they bit slow on the needed work regarding transition to more secure checksums & verification mechanisms :(

Also I don't think that introducing something else than SHA256 makes sense right now.

agree, thus added it as optional (BLAKE2/SHA-3 are better for new usages, still SHA-256 usage is good from security POV)

nijel added a commit that referenced this issue Mar 21, 2017
Issue #59

Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel
Copy link
Contributor

nijel commented Mar 21, 2017

All the locations in files are needed to ensure everything works with composer (some of them cover corner cases which probably won't happen). I've kept the same code on themes as well right now to have consistent code for both files and themes. It's not used there, but IMHO it's better from maintenance perspective.

As for adding another checksums, I'd prefer to wait until either of these is widely used, what doesn't seem to be case for neither of them right now (in the end we need something people know and will use).

@emanuelb
Copy link
Author

As for adding another checksums, I'd prefer to wait until either of these is widely used, what doesn't seem to be case for neither of them right now (in the end we need something people know and will use).

agree, and using sha-256 is totally fine, even if sha-3/blake2 is better (and has the related tools in coreutils as well) :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants