search

mellowagain / shiro

High performance, high quality osu!Bancho C++ re-implementation
GNU Affero General Public License v3.0
40 stars 6 forks source link

Replace MCrypt with Crypto++ #33

Closed emily33901 closed 6 years ago

emily33901 commented 6 years ago

Even PHP deprecated it, why don't we?

Vaela-V commented 6 years ago

Fair enough

mellowagain commented 6 years ago

We had previous issues with decryption of AES-CBC 256 with other libraries than mcrypt, and the leaked osu!Bancho source code of 2016 uses mcrypt so we decided to still use it to keep consistency with osu!Bancho.

Edit: One label per type, please.

Vaela-V commented 6 years ago

Oh ok, does it still use it though, or are you making this a (modified leak) cc only server if it doesn't?

mellowagain commented 6 years ago

@Vaela-V It's unknown if osu!Bancho still uses mcrypt today. If @josh33901 manages to get the decryption successfully working with Crypto++, we'll switch to that as mcrypt is deprecated and unmaintained - which can pose a security risk.

Vaela-V commented 6 years ago

Oh ok, sounds good 👌 I'll keep an eye out for any info regarding the matter!

emily33901 commented 6 years ago

From doing some research into the subject, osu uses aes with a 256bit block size NOT 256 bit aes (which would be a 256bit key). This is a nonstandard aes implementation. This would limit our choices to a handful of libraries (of which for c++, mcrypt is probably the only sane option). Still needs further more investigation.

emily33901 commented 6 years ago

Some thoughts on this: mcrypt has not been updated since 2003. The best "alternative" implementation i could find was from 2001. It is not possible (without significant coercion) to build mcrypt for windows (as the only precompiled binaries are x86 and from around 2001). Not to mention that the method for calling mcrypt is to perform a dlopen and then call the algorithm and then perform a dlclose (this might just be an artifact of how @Marc3842h has implemented it) however this is simply unacceptable for performance.

Potential strategy going forwards:

Short term (in order to facilitate #19): I would suggest using this other implementation as I doubt that anyone is going to host shiro on windows for a serious server.

Long term: forking Crypto++ and changing the blocksize that the AES algorithm uses would probably be the best step forwards to both ditching mcrypt and providing better cross platform compatibility.

It might turn out that the long term solution is actually trivial to get working initially, in which case I may just start working towards that.

emily33901 commented 6 years ago

Back on for october milestone. Should be finished up in a few days.