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

Use OpenSSL for hash calculation #13681

Merged
merged 5 commits into from Apr 3, 2018

Conversation

@pkerling
Copy link
Member

commented Mar 23, 2018

Currently we only have a custom MD5 implementation for hash calculation. In the future we might need other hashes (e.g. for #13647) since MD5 is not appropriate for some use cases any more.

This PR adds a digest calculation method using OpenSSL code, initially supporting MD5 and most SHA variants, and ports all the code to use it instead of XBMC_MD5. If there is a need for it, it is trivial to add any other method that is supported by OpenSSL.

@Montellese

This comment has been minimized.

Copy link
Member

commented Mar 23, 2018

So you could also get rid of xbmc/network/websocket/sha1.hpp which we included directly from Boost?

@pkerling

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2018

Sure, didn't know that was there

@pkerling pkerling force-pushed the digest-openssl branch from 735b9a2 to 41586f5 Mar 23, 2018
@pkerling pkerling requested a review from Montellese Mar 23, 2018
@pkerling

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2018

@Montellese please check. I think we don't need to byte-swap any more.

@pkerling pkerling force-pushed the digest-openssl branch from 41586f5 to 083decd Apr 3, 2018
@pkerling pkerling force-pushed the digest-openssl branch from 083decd to 841cef4 Apr 3, 2018
@pkerling pkerling added this to the Leia 18.0-alpha2 milestone Apr 3, 2018
@pkerling pkerling merged commit e255e49 into master Apr 3, 2018
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@pkerling pkerling deleted the digest-openssl branch Apr 3, 2018
@stefansaraev

This comment has been minimized.

Copy link
Contributor

commented on xbmc/utils/StringUtils.cpp in d663ee1 Apr 3, 2018

this wants #include <iomanip>

This comment has been minimized.

Copy link
Member Author

replied Apr 3, 2018

Agreed. Did it fail to build somewhere or did you happen to catch it by chance?

This comment has been minimized.

Copy link
Contributor

replied Apr 3, 2018

failed to build for me.

This comment has been minimized.

Copy link
Member Author

replied Apr 3, 2018

Can you share some details about your setup (OS/Compiler)? Just to check whether adding that to Jenkins would make sense to catch more errors.

This comment has been minimized.

Copy link
Contributor

replied Apr 3, 2018

uh :)
a LE-like environment. minimal build: http://ix.io/16fZ

This comment has been minimized.

Copy link
Member Author

replied Apr 3, 2018

Ah. Then it's probably the minimal build that triggers the problem because for normal builds iomanip gets magically included in some other header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.