Encryption Algorithm in C#

Question

I'm coding a "encryption" algorithm for learning and and don't know how good is it or how could I make it better:

private static byte[] encrypt(byte[] data, int session_id, byte[] password)
    {
        int session_key = (session_id & 0xFF);
        if (session_key == 0) session_key = (session_id << 4);
        byte[] output = new byte[data.Length];
        for (int i = 0; i < output.Length; i++)
        {
            int index = getIndex(i, password.Length);
            int num001  = session_key << 2;
            int num002 = (session_id | i) >> (i+1);
            int num003 = function(i, session_key, password[index]);
            byte encrypted = XorByte(data[i], password);
            output[i] = (byte)((encrypted ^ num003) + (num002 % num001) );
        }
        return output;
    }

    private static byte[] decrypt(byte[] data, int session_id, int length, byte[] password)
    {
        int session_key = (session_id & 0xFF);
        if (session_key == 0) session_key = (session_id << 4);
        byte[] output = new byte[data.Length];
        for (int i = 0; i < output.Length; i++)
        {
            int index = getIndex(i, password.Length);
            int num001 = session_key << 2;
            int num002 = (session_id | i) >> (i + 1);
            int num003 = function(i, session_key, password[index]);
            byte decrypted = (byte)(data[i] - (num002 % num001) ^ num003);
            output[i] = XorByte(decrypted, password);
        }
        return output;
    }
    private static int function(int index, int a, int b)
    {
        if (a > b)
            if ((b + index) < a) return a - b - index;
            else return a - b + index;
        else
            if ((a + index) < b) return b - a - index;
        else return b - a + index;
    }

    public static int getIndex(int a, int b) => (a + b) >> 2;

    private static byte XorByte(byte input, byte[] password)
    {
        byte output = input;
        for (int i = 0; i < password.Length; i++) output ^= password[i];
        return output;
    }

Basically, this code takes a byte[] (data) which will be encrypted. The session_id is a randomly-generated number (this is supposed to be used in a client-server app, and every client is assigned an ID), and a password, which is a random generated byte[].

The main functions are encrypt and decrypt, and they call another 3:

• function(int index, int a, int b) - this creates a number based on another 3 that is used to XOR each byte the input of the encryption and decryption functions.
• getIndex(int a, int b) - this returns the "middle" number (I don't know how to say it in English) of 2 numbers, that are the number of iterations and the password length. This is used to get a byte within the password.
• XorByte(byte input, byte[] password) - this applies XOR to a byte recursively for each byte of the password, so the longer the password the harder to break.

Outputs:

Using a 32 byte random password, a random ID between 1000 and 10000, and the input data: "testing string":

UTF-8 bytes: 
116  101  115  116  105  110  103   32  115  116  114  105  110  103

encrypted result: ?`R?o1?zqnrjb

UTF-8 bytes: 
160   96   82   15  237  111   49  205  122  113  110  114  106   98

Time: 112 ticks, less than 1 ms (encryption and decryption)

With the same inputs done 100.000 times:

Time: 0,08009ms each encryption/decryption, 8009ms overall

Answer 1

Your algorithm effectively combines two simple ciphers: a byte substitution cipher performed by XorByte(), and something that is similar in spirit to xoring with a poor-quality pseudo-random key stream (except for the minor difference due to the use of additive operators instead of xor for the combining step).

Calling XorByte() for each byte is wasteful; you get the same effect by calling XorByte(0, password) once, caching the result and xoring every byte with that. There's a lot more random stumblings in the dark like that. E.g.

int num002 = (session_id | i) >> (i + 1);

There's no need for oring i into the first operand since it will get shifted out completely anyway (unless operator >> implicitly performs a modulo op on its right operand, they way shift instructions do on Intel CPUs).

int index = getIndex(i, password.Length);
...    
int num003 = function(i, session_key, password[index]);

This will crash if the input size approaches or exceeds three times the password size, because getIndex() returns half the arithmetic mean.

You tried to make your key stream generator look complex but it is actually very poor. As a result you have one simple cipher (byte substitution, indirectly defined by the password) on top of a complicated but still poor one.

This algorithm will be very safe as long as it doesn't protect anything of interest. As soon as it does, it will get torn to shreds.

Don't forget that your adversaries won't necessarily hobble themselves by using an application-glue language like C#; they could use C++, effectively load all cores of their processors, and process multiple streams on each core via vector instructions (MMX/SSE). Or they could get even more performance by offloading the stuff into their graphics cards (CUDA, OpenCL). IOW, cracking performance could exceed the performance of your own code by five to ten orders of magnitude, especially when combined with a bit of math fu.

Summary: if you randomly throw stuff in a pot then what you get is a right hash.

Post scriptum: Since there weren't any other takers for reviewing this code, a few more considerations and a bit more detail.

It is rarely a good idea to feed passwords and session ids directly into cryptographic computations, because their individual bits are usually not very random (regardless of whether they contain sufficient entropy or not). The standard way of dealing with this is to distill the entropy from password and session id by running them through a hash function (or, more precisely, to apply a password-based key derivation function like PBKDF2). This effectively spreads the entropy over all bits of the resulting hash, which makes it easier to use in further processing. As a first approximation - for testing and experiments - any good hash function will do, even the trusty old MD5 (still my favourite as a general hash function, even though it can't be used for strong crypto anymore).

As was said earlier, one part of the algorithm works pretty much like a stream cipher except for the use of additive operators for the combining step instead of xor. In the following I'll gloss over the resulting differences since they are insignificant, and using xor makes epxlanations easier.

An important consideration for stream ciphers is that key streams must never be reused. If two messages are encrypted with the same key stream then a simple xor of the two cipher texts yields the xor of the two plain texts. I.e.

P  ^ S = C 
P' ^ S = C' 
C ^ C' = (P ^ S) ^ (P' ^ S) = P ^ P'

A cryptanalyst (attacker) can glean lots of invaluable information from something like that, just like when unhashed passwords are fed directly into crypto algorithms.

Consequently, stream cipher keys must never be reused. That is usually accomplished by adding a salt or nonce when hashing the password to produce the session key; the nonce - which could be a simple counter, or a unique timestamp (i.e. a timestamp plus unique identifying information like machine, process id, thread id and so on), or both. The salt/nonce must be stored with the encryption header, since it is necessary for decryption.

Also, I strongly recommend encapsulating the whole key stream generation in its own class, so that it becomes easier to use and - most importantly - easier to put through rigorous testing independent of other parts. Such a class would yield the key stream byte for byte, and perhaps also in other convenient ways.

Using 'Soopr' as placeholder for the name of the as yet anonymous algorithm, a typical use for en/decipherment might look like this:

var session_key = SooprKDF(password, session_id, nonce);
var key_stream = new SooprKeyStream(session_key);  
byte xor_byte = XorByte(0, session_key);

foreach (byte b in input)
   output = b ^ xor_byte ^ key_stream.next_byte();

An important quality indicator for cryptographic algorithms is diffusion. As a simple rule of thumb, flipping a single bit in the key material should affect every output bit with probability 50%. This applies not only to the algorithm as a whole, but most specifically to your key stream generator. Put in a different way: the output of your key stream generator should pass all standard quality tests for pseudo-randomness (most famous: Georgio Marsaglia's Diehard suite). Passing such tests is not sufficient for a good crypto algorithm, but it is necessary.

If you have your key stream generator as a separate class then you easily run some tests to get an impression of its diffusion capability. E.g. create a random session key, generate a number of key stream bytes, flip a bit in the session key, and generate the same number of key stream bytes again. Then you can count the bits that have flipped between the two stream samples.

Let your computer run a whole bunch of such random tests (with varying key stream sample lengths) - perhaps over night - and then look at how often each bit did flip for each key bit, separate per sample length. Every stream bit should have flipped about 50% of the time, for every sample length and every key bit. Tests like this make it easy to find unintended regularities that bleed through, like certain bits that never flip, or not often enough, or too often (which means they are correlated to certain parts of the key - something that shouldn't happen).