Implementation of SHA512 for ServUO shard Passwords

Dan

Dan

Moderator
@Voxpire @argalep @everyone

https://github.com/ServUO/ServUO/pull/2828

Xeroxxx submitted this pull request about a month ago. Although Dexter tagged others for review it might not be very well known.

Does anyone have any thoughts on why this would be a bad idea? Should it get implemented to offer the best possible protection? Is everything fine the way it is?

His original message from the pull request:

"SHA512 support

Password-System requires complete rework (I will do this) for proper salting (generating 'random' salt on first run) and better readability. Passwords are automatically converted from SHA1 to SHA512 after logon and next save.

NOTICE: SHA512 is the new default if you replace Config/Accounts.cfg. Change it back to NewCrypt (SHA1) in Accounts.cfg if you're in need of SHA1.

However keep in mind Server <-> Client communication stays unencrypted and is absolutely vulnerable to man in the middle attacks as client encryption is removed. It only protects if data gets stolen.

MD5 is no go and SHA1 is known to be vulnerable to collisions.

Cheers"
 
I have account integration with XenForo in my server. Therefore, there is no opinion on this issue :)
 
Only thing is your accounts.xml will grow close to 4 times as now (when SHA1 is used). But I don't see a problem there.

Next step will be proper salting. ServUO will generate a hash on first launch (this will be saved) and then passwords will be salted by that hash.

Right know its salting with the username. Maybe this should be implemented as an option so everyone can choose (salting).
 
argalep said:
I have account integration with XenForo in my server. Therefore, there is no opinion on this issue :)
Click to expand...
I only asked since you and Dexter are the two most active ServUO developers at this time and I do not know enough about it to really comment.
 
So guys whats the status here?

Will this pull request be open forever or do I need to close it myself?

I really feel being ignored and I'am not sure if there are future contributions of other things than "game-content" wanted
 
SHA512 isn't supported by all operating systems, hardware and/or software, which is why I'm sceptical about this.

In the last 20 years, have any free-shards had their passwords successfully brute forced?
I can't think of a single report... correct me if I am wrong.

I just don't see the reason for a shard with 50k accounts to have to suffer a file size increase of 10x or more for the sake of going one step further than the industry standard recommendation of using SHA-256. XML file sizes are detrimental to memory usage too, with them occupying the Large Object Heap and never being fully destructed until the application is closed.

Also, I'll get around to everything in my own time regardless of whether you feel ignored or not.
 
Voxpire said:
New SHA512 isn't supported by all operating systems, hardware and/or software, which is why I'm sceptical about this.
Click to expand...

This is supported starting .NET 1.1 and Windows XP SP3. Which are the ones thats its not supported on? Are you aming to support everything starting Windows XP/2003?

Voxpire said:
In the last 20 years, have any free-shards had their passwords successfully brute forced?
I can't think of a single report... correct me if I am wrong.
Click to expand...

Thats like "I've my passwords in a textfile and it got never stolen". MD5 and SHA1 are broken thats a fact. If there is gained access to the server, passwords maybe lost. There are still people using the same password for everything.

Voxpire said:
I just don't see the reason for a shard with 50k accounts to have to suffer a file size increase of 10x or more for the sake of going one step further than the industry standard recommendation of using SHA-256. XML file sizes are detrimental to memory usage too, with them occupying the Large Object Heap and never being fully destructed until the application is closed.
Click to expand...

Where are the downsides of a bigger account file (Increase by 4 times starting SHA1)? Its read once. Any recent source for recommendation of SHA256? ServUO is not a time critical applicaiton, passwords a submitted by the client are encrypted once to see if they match, I don't see a performance regression here. All-in-All its an OPTION, you don't have to use it.

Voxpire said:
Also, I'll get around to everything in my own time regardless of whether you feel ignored or not.
Click to expand...

I don't see any offense in my replys but that seems to be the spirit of the whole project. Thanks for
your time.

All of us doing this in our freetime.
 
Last edited:
My concerns were raised from personal experience while developing the Installer/Patcher/Launcher client/server service for Ultima Shards: Multiverse and UO Outlands.

In some cases, the checksums were inconsistent and even blank when returned.
This wasn't an issue when using 256 or lower.

256 is the industry standard minimum for TLS 1.2 (SSL).

I have invited you to have write access to ServUO, so you may implement things faster.
I rarely have the time to review PR's these days, to the detriment of the project, which sucks.
 
So any additional feedback to this implementation?

Are we gonna merge it? (with SHA1 default).
 
If you convert to the new style can you convert back? Anything that messes with user accounts is a scary thing.
 
Voxpire stated his concerns with this and does not think this is a good thing in which to add to ServUO - I'm not so sure @Dexter_Lexia wants to add this. There have no been many offering up their concerns
 
It requires a reviewer anyways and Dexter already said he does not feel comfortable doing it because he does not know enough about it.

If Voxpire or Dmurphy review and approve it then go ahead!

How many accounts has this been tested on? When converting to the new method?
 
Right now the merge does not change anything in case of account handling or encryption. SHA1 is still the default encryption.

If anyone sets it from SHA1 to SHA512 it will upgrade the passwords on login, same as it does before with MD5 to SHA1. It won't work backwards as it doesnt with SHA1 to MD5.

I've tested this with 3 accounts (I'm not submitting code that I'vent tested before). 1 admin, 2 user accounts. Creating accounts with SHA1 password and upgrade afterwards, initial creating of SHA512 accounts. Please go ahead if you've more scenarios.

However I see your concerns, I replied to the concerns. If everyone wants to see SHA256, I'm willing to implement this too. Keep in mind its an offer to the server admin, if you've concerns about memory used by a 4 times bigger account file, stay with the default, no change is needed.

I thought implementing SHA512 instead of SHA256 is a better idea. Just to be safe and ServUO is not permanently encrypting passwords, so the performance regeression of using SHA1, SHA256 or SHA512 is not messurable with cpus of the last 8 years.

Thanks again for write access to github
 
Last edited:
You must log in or register to reply here.

Active Shards

Donations

Total amount
$0.00
Goal
$1,000.00
View
Back