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

Tests should use a modern hash (SHA-256 instead of MD5) #12363

Open
johnhawkinson opened this issue Mar 5, 2017 · 3 comments
Open

Tests should use a modern hash (SHA-256 instead of MD5) #12363

johnhawkinson opened this issue Mar 5, 2017 · 3 comments
Labels

Comments

@johnhawkinson
Copy link
Contributor

@johnhawkinson johnhawkinson commented Mar 5, 2017

youtube-dl's download tests all use MD5 as a hash. While it's probably not cryptographically critical that download testing use a secure hash, the world has moved past MD5, and past SHA-1. youtube-dl should at least support SHA-256 in the info_dict, and use whichever hash has been supplied in the _TEST's info_dict.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Mar 5, 2017

Good idea. Just checked it, python 2.6 supports up to sha512. An implementation can be allowing sha256 (or 512) for new _TESTS as changing all tests at a time is impractical.

@yan12125 yan12125 added the request label Mar 5, 2017
@rautamiekka
Copy link

@rautamiekka rautamiekka commented Mar 9, 2017

SHA-512 would be better than 256, but even better would be using SHA3-512 even if it takes having an additional lib.

But definitely SHA-256 at minimum.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Mar 9, 2017

SHA3 is not available in stdlib until Python 3.6 http://bugs.python.org/issue16113. I don't think there's a need to bring another external dependency for tests. They don't need cryptographically secure hashes.

@treakiandroid
Copy link

@treakiandroid treakiandroid commented Mar 11, 2017

@rautamiekka there is a thing like overdoing it, i guess sha128 should be enough to... You should also keep code efficiency in mind, i am using youtube-dl on embedded systems with low recourses and i dont like if there are taken unnecessary recources consuming things if another method can do it also in a sufficient way. I think the only thing that should be prevented here is that by coincidence the same hash will work for two different datastreams, which is even very unprobable with md5 but will be completely unprobable if you use anything from sha128. So imho its good do move away from md5 but you shouldnt overdo it and sha512 will be more then enogh, so do something balanced between efficiency and stability like sha128 or sha265. And dont import any unnecessary libs that have to be baybesitted (like security problems/updates/ (version) incompatibility in external libraries) to and is taking up space

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.