Authentication flow is extremely insecure

[tywil04]

The current authentication situation is extremely insecure. Passwords are encrypted with a hard-coded key and then decrypted to verify. This is not how authentication should be done and its simply inexcusable. This needs to be changed.

Here is a better authentication flow I propose:
Setup:

1. The user enters username and password. The password is hashed (I’ll call this hash) with the username as a salt using a key derivation function (KDF). The client then sends the username and hash to the server.
2. The server receives the username and hash. The hash is hashed (I’ll call this secured hash) with a salt (I’ll call this secured hash salt) generated with a cryptographically secure pseudorandom number generator (CSPNG) using a KDF. The username, secured hash and secured hash salt should be stored.
3. Setup is complete.

Verification:

1. The user enters username and password. The password is hashed (I’ll call this test hash) with the username as a salt using a KDF. The client then sends the username and test hash to the server.
2. The server receives the username and test hash. The test hash is hashed (I’ll call this secured test hash) with the secured hash salt stored in the database for the user from the username using a KDF. The stored secured hash should be compared with the secured test hash using a constant time compare function. If the hashes match then the user entered the same password as used in setup. If the hashes don’t match the password differs from the password the user used in setup.
3. Depending on the result from the hash comparison do or don’t respond with a session token.

The KDF used on the client should be consistent and the KDF used on the server should be consistent. The KDF used on the client and the KDF used on the server do not need to be the same. There should be some system to allow for changes in KDF (or its settings) later on in time.

A good modern KDF is Argon2 (preferably the hybrid version Argon2id).

This authentication flow is very similar to what Bitwarden (the password manager) uses to verify a users password. The naming I used could do with some improvements.

[TuningYourCode]

i would not recommend to use the username as salt as that would basicly mean that all/most instances would have admin as hash.

i don't know how much performance the cpu has but i could image the following solutions:

• use crypt(3) compatible hashing (we could use pam / /etc/shadow for user management)
• use sha256 / sha512 - they are part of golang stdlib
• use bcrypt / pbkdf2 / argon2 - there is golang.org/x/crypto but need checking if the cpu is fast enough to use them

a big problem currently is that http is used so might stick with the aes encryption for now and edit the base image to create self signed tls certificates and use them.

[tywil04]

Yeah, I did overlook that and the username almost always being admin is a pain point of my proposal.

Realistically, the user should pick a username and password for login in on the client, not just the password. This would solve the issue with my proposal and overall would be beneficial.

Most of the cryptography happens on the client so we don't need to worry about performance too much. The second KDF step is on the NanoKVM itself so at worst the KDFs settings will need to be tuned accordingly.

[ChokunPlayZ]

looks like this is fixed in this commit.

[tywil04]

I just looked over the commit 6eb4a4e and it does make some improvements but its still far from secure.

Here is the new approach based on the changes in commit 6eb4a4e:

• Signup: When the user enters a username and password, the password is encrypted client side (using a key that's hard-coded in client code and is public). Then both the username and password are transmitted to the server. The server then decrypts the received password (using the same key, its hard coded in server code and is also public). The now decrypted password is hashed using bcrypt. The received username and hashed password are now stored and the user has signed up successfully.
• Login: When the user enters a username and password, the password is encrypted client side (using a key that's hard-coded in client code and is public). Then both the username and password are transmitted to the server. The server then decrypts the received password (using the same key, its hard coded in server code and is also public). The now decrypted password is hashed using bcrypt. The hashed password is compared against the stored hashed password which is found by the received username. If the hashes match a session token is returned if the hashes dont match then the password entered is incorrect.

Storing a hash of the users password is an improvement, so this new system is more secure than before (to be honest, its impossible for it to have gotten worse).

The main issue with the new approach is that the public and hard-coded keys used for encrypting the users password for/during transit just adds extra code complexity and no additional security whatsoever - its no different to sending the password in plaintext. Due to the fact the password is essentially transmitted in plaintext, somebody snooping on network traffic could get the users encrypted password then decrypt it with the known public encryption key. With the users password they can login normally. This is a major flaw.

The approach I first suggested would solve all of the issues with both the old and new approaches. The reason this is so important is because a KVM has full control of a computer, remotely. This is why it needs to extremely secure and while the new approach is better, its still far from secure and inexcusable for a KVM type product. It needs to be sorted.

[wj-xiao]

Please refer to #295 .

[tywil04]

#295 doesn't even begin to address the security flaws around the current aproach of authentication

[lovelytwo]

Hello @tywil04, i might be completely wrong. I only endup here because I was watching a youtube video about this kvm, and was curious what the security issues where but couldn't be bothered watching the video. I think your "better authentication flow" would be vulnerable to replay attacks.

Once the client has hashed the username and password, can't a person snooping the wire replay the hashed valve to gain access. If the server doesn't provide a challenge, then hash/salting/encrypting provides limited valve over plaintext, the one thing your approch does protect is an actors ability to decrypt/decode or otherwise determine the users plain text password.

Like i said i might be completely wrong, not understanding there is already some sort of challenge system involved or channel encryption, that is at a lower layer than what you described. The lack of channel encryption usually provided by https causes quite a few problems. Eg protecting hashes in transit, protecting session tokens, etc

[tywil04]

@lovelytwo Yes, the approach I have proposed is vulnerable to replay attacks but most approach will be given that TLS isn't enabled by default so in instances where its not enabled, replaying is potentially a very big issue.

When I claimed it was a better approach I should have more clearly specified how its better than the old approaches (which are both also vulnerable to replay attacks).

Here's why the approach I proposed is better:

• The users password is never sent in plaintext or is never encrypted with a publicly known and hard-coded key (which makes the encryption pointless). The old approaches send the password in either plaintext or essentially plaintext. The benefits of my approach means the users password can never be intercepted because its never sent. Having the hash from my approach intercepted is significantly less damaging to a user than having their password intercepted and leaked (its unfortunately very likely the user isn't following password best practices, so a password leak is BAD).
• The users password is never stored in a form where it can be replayed or recovered on the server. The first old approach encrypted the users password with a publicly known and hard-coded key (changes in 6eb4a4e did improve this). The benefits of my approach means that if someone got physical access to the server (the NanoKVM) they cannot decrypt and recover the users password or use the stored value to gain access via a replay attack.

As mentioned the changes in 6eb4a4e does improve things slightly, as it stores a bcrypt hash of the users password and not the users password in a recoverable form. But it still suffers from having the potential for the users password to be intercepted and leaked in a plaintext form.

There are ways to replay attacks such as using a Diffie-Hellman key exchange to securely establish a shared key over an insecure channel without revealing the shared key. Then using the shared key with a symmetric key encryption algorithm we can encrypt and decrypt the hash before and after transport. This will protect the hash during transport and prevent it from being replayed. This, however, is very similar to what TLS would provide and TLS will protect all data in transport, so in all honesty TLS should be enabled by default (even when you consider the performance impact).