1

When I was in search about AES Encryption/Decryption implementation, I found some links of questions out there in SO, like these:

  1. AES Encryption Java -> PHP -> Java
  2. AES encryption in java
  3. AES encryption & security flaw
  4. Is it safe to use PBKDF2 with SHA256 to generate 128-bit AES keys?

And also I found the following webpage, which offers easy to use implementation of AES Encryption/Decryption algorithm in both PHP and Java as a library.

Question: Is it safe to use that library of AES implementation, directly in our real time development projects?

This may require you to go through the implementation of the source code. So as it is lengthier the PHP implementation may seemed, following I have put the essential part of the Java source code of that library.

import java.io.UnsupportedEncodingException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;
import org.apache.commons.codec.binary.Base64;
/**
Aes encryption
*/
public class AES
{

    private static SecretKeySpec secretKey ;
    private static byte[] key ;

    private static String decryptedString;
    private static String encryptedString;

    public static void setKey(String myKey){


        MessageDigest sha = null;
        try {
            key = myKey.getBytes("UTF-8");
            System.out.println(key.length);
            sha = MessageDigest.getInstance("SHA-1");
            key = sha.digest(key);
            key = Arrays.copyOf(key, 16); // use only first 128 bit
            System.out.println(key.length);
            System.out.println(new String(key,"UTF-8"));
            secretKey = new SecretKeySpec(key, "AES");


        } catch (NoSuchAlgorithmException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (UnsupportedEncodingException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }



    }

    public static String getDecryptedString() {
        return decryptedString;
    }
    public static void setDecryptedString(String decryptedString) {
        AES.decryptedString = decryptedString;
    }
    public static String getEncryptedString() {
        return encryptedString;
    }
    public static void setEncryptedString(String encryptedString) {
        AES.encryptedString = encryptedString;
    }
    public static String encrypt(String strToEncrypt)
    {
        try
        {
            Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding");

            cipher.init(Cipher.ENCRYPT_MODE, secretKey);


            setEncryptedString(Base64.encodeBase64String(cipher.doFinal(strToEncrypt.getBytes("UTF-8"))));

        }
        catch (Exception e)
        {

            System.out.println("Error while encrypting: "+e.toString());
        }
        return null;
    }
    public static String decrypt(String strToDecrypt)
    {
        try
        {
            Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5PADDING");

            cipher.init(Cipher.DECRYPT_MODE, secretKey);
            setDecryptedString(new String(cipher.doFinal(Base64.decodeBase64(strToDecrypt))));

        }
        catch (Exception e)
        {

            System.out.println("Error while decrypting: "+e.toString());
        }
        return null;
    }
}

Note: Please, don't mark this as Too Broader Question and Neglect it, I'm asking because I need to be sure before I use it for my next project.

Thanks in advance for your valuable time!

Community
  • 1
  • 1
Randika Vishman
  • 7,983
  • 3
  • 57
  • 80
  • `Cipher.getInstance("AES/ECB/PKCS5Padding");` Don't use ECB, use CBC. Also, you should [authenticate your ciphertexts](https://paragonie.com/blog/2015/05/using-encryption-and-authentication-correctly). – Scott Arciszewski Jul 07 '15 at 16:05

1 Answers1

3

No, you should absolutely not use that code.

AES class should not contain a digest, that's not part of AES. ECB is insecure; even CBC is likely insecure without integrity/authentication. Exceptions are shuffled under the table and will result in null being returned. There is no difference made between runtime exceptions and exceptions that are generated by invalid input and output. The fields are not correct. Default encoding is being used. Actually, if I had a score sheet then it would fail on about half the points at minimum.

To be able to use cryptography you need at least a minimum understanding of the subject. Otherwise you need to leave it to professionals or you need to use pre-made solutions by known experts. Just grabbing code from the internet will only provide you with a false sense of security and a source of impossible to fix bugs.

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263
  • I typed this in 3 minutes, meaning that I didn't even take a real good look at the code. The bit bucket is too good for this code. – Maarten Bodewes Jul 07 '15 at 18:21
  • Well I have a good understanding about Cryptography. But the case is I need to know if there are any vulnerability of using a library somebody made out there in our production level projects without going to reinvent the wheel. Because it's like they already know how my encryption algorithm works. that's it! – Randika Vishman Jul 08 '15 at 04:45
  • The PHP library defaults to ECB mode. I would not recommend this library to anyone; it fails to provide sane defaults. [Error-prone cryptography designs](http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-4x3.pdf) (PDF) should be discouraged. If you need a secure implementation: I vouch for [defuse/php-encryption](https://github.com/defuse/php-encryption). – Scott Arciszewski Jul 12 '15 at 06:24