2
\$\begingroup\$

I have got to my self these two functions (Encryption and Decryption):

public static byte[] Encrypt(string plaintext, byte[] key)
{
    using (Rijndael desObj = Rijndael.Create())
    {
        desObj.Key = key;
        desObj.Mode = CipherMode.CFB;
        desObj.Padding = PaddingMode.PKCS7;
        using (MemoryStream ms = new MemoryStream())
        {
            ms.Write(desObj.IV, 0, desObj.IV.Length);
            using (CryptoStream cs = new CryptoStream(ms, desObj.CreateEncryptor(), CryptoStreamMode.Write))
            {
                byte[] plainTextBytes = Encoding.UTF8.GetBytes(plaintext);
                cs.Write(plainTextBytes, 0, plainTextBytes.Length);
            }
            return ms.ToArray();
        }
    }
}
public static string Decrypt(byte[] cyphertext, byte[] key)
{

    using (MemoryStream ms = new MemoryStream(cyphertext))
    using (var desObj = Rijndael.Create())
    {
        desObj.Key = key;
        desObj.Mode = CipherMode.CFB;
        desObj.Padding = PaddingMode.PKCS7;

        //Read the IV from the front of the stream and assign it to our object.
        var iv = new byte[16];
        var offset = 0;
        while (offset < iv.Length)
        {
            offset += ms.Read(iv, offset, iv.Length - offset);
        }
        desObj.IV = iv;

        //Read the bytes to be decrypted
        using (var cs = new CryptoStream(ms, desObj.CreateDecryptor(), CryptoStreamMode.Read))
        using (var sr = new StreamReader(cs, Encoding.UTF8))
        {
            return sr.ReadToEnd();
        }
    }
}

This cryptography works but I would like to know whether I wrote it good and if there are any suggestion to improve. In addition, I would also like to ask if I can make a function in Python that can decrypt what these C# function encrypted (and in the opposite direction too) and how can I do that?

EDIT: These two functions are meant to encrypt or decrypt the messages to/from a socket that is connected to a server. That's why I get this as byte array in the decrypt function and return it as one in the encrypt function. The server is written in Python (I was told the python can be easy to write servers with, especially because the server is a simple one). And, I want to decrypt the messages sent from the client in the server too.

\$\endgroup\$
  • 2
    \$\begingroup\$ CreateEncryptor() and CreateDecryptor() both return objects which implement IDisposable, so they should be in their own using block before passed to the CryptoStream constructor. \$\endgroup\$ – Jesse C. Slicer Apr 20 '17 at 20:28
  • \$\begingroup\$ Rijndael - that looks like an external dependency. if so its instantiation should probably be isolated and then injected - and classes restructured. personally i would create a class and then send messsages to that class something like: MyEncryptionObject crypt = new MyEncryptionObject(); and then crypt.Encrypt("plaintext-msg") rather than passing in the keys and using a static method. more importantly, what do you plan to do with the byte array that is returned - if you could clarify that would be great? i'll probably hammer out a proper answer when i get a chance at the end of the day \$\endgroup\$ – BKSpurgeon Apr 21 '17 at 1:55
  • \$\begingroup\$ @BKSpurgeon I added in the question why do I use these functions and why a byte array. \$\endgroup\$ – Ofir Eizenberg Apr 22 '17 at 9:03
  • \$\begingroup\$ CFB is a relatively unusual mode. CBC is more common, or GCM if you want authentication included, rather than using a separate HMAC. For good security you need to create your key using KDF (Key Derivation Function) such as PBKDF2 or Argon2. \$\endgroup\$ – rossum Apr 22 '17 at 16:53
  • \$\begingroup\$ @OfirEizenberg when u mean "suggestions for improvement" what specifically are you looking for: a more OOP approach to the problem? \$\endgroup\$ – BKSpurgeon Apr 22 '17 at 23:18
1
\$\begingroup\$

The Microsoft documentation on the Rijndael class says:

You should use the Aes class instead.

In order to use encryption across programming languages, you have to make sure that all parameters of the encryption algorithm are exactly the same. For AES, this include the cipher mode and all its parameters (which for CFB are not precisely documented in the MSDN documentation). To ensure portability, you either need to find the documentation that specifies all the details, or you need to write unit tests to ensure that the parameters don't change, or you need to switch to a simpler cipher mode (but not ECB, of course).

The code looks fine to me. I would have expected it to be shorter, though I don't know whether this is Microsoft's fault (because they designed the API like this) or yours (because you use the API in a more complicated way than intended).

\$\endgroup\$
  • \$\begingroup\$ About the length, there may be simpler cryptography libraries which by using them the code will be shorter, but because I write this program for a huge school project, someone later might ask me how do I encrypt or decrypt and here I can explain each and every line. About the parameters, do you recommend about an encryption method other than CFB? (I can change to CBC/CTS/OFB) \$\endgroup\$ – Ofir Eizenberg Apr 22 '17 at 13:57
  • \$\begingroup\$ I cannot reliably recommend a cipher mode, since I know only CBC. Hopefully someone else can. \$\endgroup\$ – Roland Illig Apr 22 '17 at 14:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.