Advertisement

Cryptography - am I doing it right?

Started by March 27, 2014 04:24 PM
26 comments, last by Washu 10 years, 7 months ago

I'm trying to establish a secure connection in C#, and I'm not sure I'm doing it right. :(

Here is my encryption class:


/*The contents of this file are subject to the Mozilla Public License Version 1.1
(the "License"); you may not use this file except in compliance with the
License. You may obtain a copy of the License at http://www.mozilla.org/MPL/

Software distributed under the License is distributed on an "AS IS" basis,
WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for
the specific language governing rights and limitations under the License.

The Original Code is the GonzoNet.

The Initial Developer of the Original Code is
Mats 'Afr0' Vederhus. All Rights Reserved.

Contributor(s): ______________________________________.
*/

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;
using System.Security.Cryptography;

namespace GonzoNet.Encryption
{
    public class ARC4Encryptor : Encryptor
    {
        private DESCryptoServiceProvider m_CryptoService = new DESCryptoServiceProvider();
        private ICryptoTransform m_DecryptTransformer, m_EncryptTransformer;
        public byte[] EncryptionKey;

        public ARC4Encryptor(string Password)
            : base(Password)
        {
            PasswordDeriveBytes Pwd = new PasswordDeriveBytes(Encoding.ASCII.GetBytes(Password), 
                Encoding.ASCII.GetBytes("SALT"), "SHA1", 10);
            EncryptionKey = Pwd.GetBytes(8);
            m_DecryptTransformer = m_CryptoService.CreateDecryptor(EncryptionKey, Encoding.ASCII.GetBytes("@1B2c3D4e5F6g7H8"));
            m_EncryptTransformer = m_CryptoService.CreateEncryptor(EncryptionKey, Encoding.ASCII.GetBytes("@1B2c3D4e5F6g7H8"));
        }

        public ARC4Encryptor(string Password, byte[] EncKey)
            : base(Password)
        {
            PasswordDeriveBytes Pwd = new PasswordDeriveBytes(Encoding.ASCII.GetBytes(Password),
                Encoding.ASCII.GetBytes("SALT"), "SHA1", 10);
            EncryptionKey = EncKey;
            m_DecryptTransformer = m_CryptoService.CreateDecryptor(EncryptionKey, Encoding.ASCII.GetBytes("@1B2c3D4e5F6g7H8"));
            m_EncryptTransformer = m_CryptoService.CreateEncryptor(EncryptionKey, Encoding.ASCII.GetBytes("@1B2c3D4e5F6g7H8"));
        }

        public override DecryptionArgsContainer GetDecryptionArgsContainer()
        {
            DecryptionArgsContainer DArgsContainer = new DecryptionArgsContainer();
            DArgsContainer.ARC4DecryptArgs = new ARC4DecryptionArgs();
            DArgsContainer.ARC4DecryptArgs.Transformer = m_DecryptTransformer;

            return DArgsContainer;
        }

        public override byte[] FinalizePacket(byte PacketID, byte[] PacketData)
        {
            MemoryStream FinalizedPacket = new MemoryStream();
            BinaryWriter PacketWriter = new BinaryWriter(FinalizedPacket);

            MemoryStream TempStream = new MemoryStream();
            CryptoStream EncryptedStream = new CryptoStream(TempStream,
                m_EncryptTransformer, CryptoStreamMode.Write);
            EncryptedStream.Write(PacketData, 0, PacketData.Length);
            EncryptedStream.FlushFinalBlock();

            PacketWriter.Write(PacketID);
            //The length of the encrypted data can be longer or smaller than the original length,
            //so write the length of the encrypted data.
            PacketWriter.Write((ushort)((long)PacketHeaders.ENCRYPTED + TempStream.Length));
            //Also write the length of the unencrypted data.
            PacketWriter.Write((ushort)PacketData.Length);
            PacketWriter.Flush();

            PacketWriter.Write(TempStream.ToArray());
            PacketWriter.Flush();

            byte[] ReturnPacket = FinalizedPacket.ToArray();

            PacketWriter.Close();

            return ReturnPacket;
        }

        public override MemoryStream DecryptPacket(PacketStream EncryptedPacket, DecryptionArgsContainer DecryptionArgs)
        {
            CryptoStream CStream = new CryptoStream(EncryptedPacket, m_DecryptTransformer, CryptoStreamMode.Read);

            byte[] DecryptedBuffer = new byte[DecryptionArgs.UnencryptedLength];
            CStream.Read(DecryptedBuffer, 0, DecryptedBuffer.Length);

            return new MemoryStream(DecryptedBuffer);
        }
    }
}

Then I initialize it as such on the client and server side:


            //Doing the encryption this way eliminates the need to send key across the wire! :D
            SaltedHash Hash = new SaltedHash(new SHA512Managed(), Args.Username.Length);
            byte[] HashBuf = Hash.ComputePasswordHash(Args.Username, Args.Password);
            Args.Enc = new GonzoNet.Encryption.ARC4Encryptor(Convert.ToBase64String(HashBuf));

Then when establishing the secure connection, I'm sending the hash across. What I'm wondering is: Could the hash be used to generate the key by a man-in-the-middle attacker?

If so, would it help to Blowfish the hash using itself as they key, so that if the hash stored in the DB is correct, it will be able to decrypt itself?

Should I just rewrite the protocol using Diffie-Helllman?

In short: You are doing it wrong, because you're not using TLS.

In slightly longer: The fact that you even have to ask means that you're likely doing it wrong. This means that you haven't spent the years necessary to learn about cryptography to build a robust cryptographic system. (These systems are only ever as strong as the weakest link.)

Also: Is this really an attack that you need to worry about? 99.99% of all attacks on games are social engineering ("give me your password and I'll improve your ranking") or computer malware ("install this hotkey helper that totally doesn't read your keyboard to send your password to me") not technical man-in-the-middle injections.

Note that not even Diffie-Hellman, on its own, is sufficiently secure. The reason is that a MitM can run Diffie-Hellman towards the client on one side, and then run a separate Diffie-Hellman towards the server on the other side, and read the cleartext in the middle.

If you care about men in the middle: Use TLS. Generate a certificate per user. For non-HTTPS implementations of TLS, you can use a self-signed root certificate. Part of initial sign-up could be to download this certificate, which could be automated by your UI.
enum Bool { True, False, FileNotFound };
Advertisement

I'm deeply concerned about how you're managing passwords in this code, authentication should be well-separated from encryption. Just use SSL. Please. It isn't worth it.

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

Hplus pretty much already said everything important, but to give you an idea what's also wrong: DES is way outdated ( one might argue that it was designed to be insecure). Also, never use an encryption key twice. You are at least reusing it for every session, maybe even for every packet.

Thanks guys! :)

I've had a think, and I've decided that using TLS isn't neccessary. This is an open source project, so people can figure out the protocol either way. And there's no way they can retrieve the password from the hash.

However I won't remove the encryption currently in place, because its been a learning experience. I'll switch to using RSA key exchange just because I can. :)

Thanks guys! smile.png

I've had a think, and I've decided that using TLS isn't neccessary. This is an open source project, so people can figure out the protocol either way. And there's no way they can retrieve the password from the hash.

However I won't remove the encryption currently in place, because its been a learning experience. I'll switch to using RSA key exchange just because I can. smile.png

Ignoring absolutely all of the advice in this thread is your call, but "people can figure out the protocol either way", "there's no way they can retrieve the password from the hash", "it's been a learning experience", and "just because I can" really aren't good ways to write security-related code, and don't inspire confidence. There's a reason it's recommended not to roll your own. Learning experiences in cryptography should be confined to personal experimentation, as soon as you're handling user information you are morally and ethically obligated* to secure it to the best of your ability, and if that means using existing, industry-standard, proven technology or contracting an expert to implement or audit your code, so be it. Just be aware that by making this choice, you are almost certainly not acting in your or your users' best interest.

* depending on what your program does, you may in fact be legally required to submit such code for an audit and be held legally responsible for protecting your users' privacy, whoever that may be. This probably doesn't apply in your case, though, usually this applies to banking authentication and transactions, storing employee payroll information, and so on, but it is worth keeping in mind that if a significant data breach occurs, and a company is found to have not acted responsibly by using a home-made security backend, things can occasionally become very, very unpleasant for them.

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

Advertisement

Huh?

The first thing hplus asked me was "is this really an attack you need to worry about?"

I thought about it, and I decided "no".

Also, are you suggesting that hashing a password with SHA512 isn't safe (enough?) to send in plaintext, and it needs to be encrypted in addition?

Lastly, if I'm going to use encryption at all, what's wrong with using RSA key exchange?

The main reason I really don't want to use TLS is because it just seems overly complex. I did some research, and I haven't been able to figure out how to generate a "self signed root certificate". Can I do this programmatically?

are you suggesting that hashing a password with SHA512 isn't safe (enough?) to send in plaintext


That's not the problem. The problem is that, if someone sniffs the hash of the password, they can re-play that login to your servers, and pretend that they know the password, because they know the hash of the password.

Designing cryptosystems is really hard. I suggest you read a good book, if you actually want to know about these things. There are a variety of books, some worse than others, but you can get started with an introduction for free:
http://en.wikibooks.org/wiki/Cryptography
And then perhaps something reasonable-well rated like this:
http://www.amazon.com/gp/product/0470474246/ref=as_li_ss_tl?ie=UTF8&camp=1789&creative=390957&creativeASIN=0470474246&linkCode=as2&tag=enchage-20

To use TLS, download OpenSSL, and follow the documentation :-)

If all you want to do is scramble bits on a wire, then use AES, and hard-code the key in both the client and the server. (Suggested key; "swordfish")
enum Bool { True, False, FileNotFound };

Then would it not be safer to encrypt everything in the first packet BUT the accountname with AES, so that if the accountname's corresponding hash in the DB can descramble the packet, that hash can be used to encrypt communication onwards? Why/why not?

Also, are you suggesting that hashing a password with SHA512 isn't safe (enough?) to send in plaintext, and it needs to be encrypted in addition?
At the very least, you should hash the password with a timestamp or something else that makes the hash value unique while still allowing the hash to be verified. Basically, this is none different from using a salt, only the reason why you use it is different. Otherwise, it is really trivial to exploit the system for anyone who listens to your communication (such as on an open WiFi spot in a café) by remembering that hash value. He doesn't even need to look the password up in a rainbow table, he can just replay the hash as-is.

Lastly, if I'm going to use encryption at all, what's wrong with using RSA key exchange?
Nothing as such. Except it's not trivial to get right either, and you'll likely do it wrong (or worse than using TLS properly).

The main reason I really don't want to use TLS is because it just seems overly complex.
That is, in my opinion, the only valid excuse for not always using OpenSSL/TLS for everything (otherwise I'd say use OpenSSL/TLS even when actually no encryption is needed at all).

You already need half a PhD to figure out how to use it at all, which is a shame because the universally given answer "do not run your own crypto stuff, you will do it wrong" is almost certainly true. Without even looking at your code and without even knowing your skill level, one can tell with very high confidence that you did it wrong.

That said, because I find TLS and OpenSSL obnoxious for their complexity, I would recommend to use NaCL if you can get it to compile (compiling is admittedly a challenge!), or recommend that you do an ECDH key exchange both for authentication.and establishing a session key, with the curve25519 library (same author). That one is one source file, and a single function call and so easy that even a brain-damaged monkey couldn't do wrong, and both speed and key sizes are "very manageable". As in, 32 bytes per key, and upwards of 20,000 ops per second.

For example, a scheme like this one is pretty acceptable (not 100% secure, but as good as it gets without getting overly complicated, and by all means good enough for a game):

  • Store the server's public key (32 bytes) in the client executable
  • When the user creates an account (via a web interface or whatever means), hash username+password a few dozen times. This is the user's private key. Generate a public key from that (one function call) and immediately throw the private key away after that. Store the public key in the database on the server.
  • Whenever the user wants to log in, send the username. The server sends back a message encrypted using a key derived from its private key and the user's public key. Meanwhile the client program hashes the entered password to figure out the user's private key.
  • The client decrypts the message using the user's private key and the server's public key stored in the executable. The decrypted message contains a random session key and a challenge response valid for that connection.
  • The client uses the session key to encrypt all further traffic and sends the challenge response. The server won't talk to anyone who doesn't send the correct challenge response, and someone who doesn't know the user's private key (i.e. the password) won't know the session key and won't be able to understand any of the communication or send meaningful commands.

This system is of course not 100% secure, but it is simple and immune to all common and reasonably realistic technical attacks (there are a few additional minor details about what "encrypt" means and such, but in principle it's not much more complicated than that). Social attacks of course still work, but they work on every system, regardless of how clever you think they are... you simply can't prevent a user from giving away his password or from installing that porn-trojan or that keylogger on his computer.

Someone might root the server and steal users' passwords while they create accounts, but that's something you can always do when you are in total control the server. It's no problem insofar as you already control the complete server, so stealing a user's game password is no concern.

Further, a disgruntled employee might run off with the server's private key, or a hacker might steal it, much like a hacker might steal the user password database. While the server's private key will allow for impersonating/cloning the server or for a MITM attack, it is only useful provided that you can also successfully subvert the DNS system or that you work at an ISP or such, so you can conceivably run a MITM (for most people who are not ISP employees or secret service agents, this is a rather theoretical attack without much of a practical value). Otherwise, the server's private key is completely worthless, it won't let you log into the game. Nor will the stolen user database, since they're public keys only.

For a game, I daresay this is absolutely sufficient, but if you're worried, you can still make it more complicated (letting keys expire, sign them, whatever). Personally, I think that's using a sledgehammer to crack a nut (...for a game).

Though of course, on a second thought... the most common DSL router in the EU (which powers e.g. about 50% of the internet access in Germany) can be trivially exploited and still 3/4 of users haven't upgraded their firmware two months later if you can believe what's said on the news. From that point of view, it might be conceivable for a runaway employee who isn't the leader of a top international crime organization and who isn't a CIA employee to run a MITM on users via their router. However, those users would likely be the same users who will tell you their password, too. laugh.png

This topic is closed to new replies.

Advertisement