Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote below code to crypt and decrypt some bytes in three algorithm with Java but I do not know if I wrote them in correct mode or not. Please tell me about truth of code.

First class:

public class Cryptography 
{
    Cryptography() {}

    public byte[] Encryption_AES128(byte[] plain  , byte[] key) throws Exception
    {
        AES128 aes128 = new AES128();
        return aes128.encrypt(key, plain);
    }
    public byte[] Decryption_AES128(byte[] cipher  , byte[] key) throws Exception
    {
        AES128 aes128 = new AES128();
        return aes128.decrypt(key, cipher);
    }

    public byte[] Encryption_DES(byte[] plain  , byte[] key) throws Exception
    {
        DES des = new DES(key);
        return des.encrypt(plain);
    }
    public byte[] Decryption_DES(byte[] cipher  , byte[] key) throws Exception
    {
        DES des = new DES(key);
        return des.decrypt(cipher);
    }

    public byte[] Encryption_TripleDES(byte[] plain  , byte[] key) throws Exception
    {
        TripleDES Tdes = new TripleDES(key);
        return Tdes.encrypt(plain);
    }
    public byte[] Decryption_TripleDES(byte[] cipher  , byte[] key) throws Exception
    {
        TripleDES Tdes = new TripleDES(key);
        return Tdes.decrypt(cipher);
    }
}

And three other classes:

First Triple DES:

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.DESedeKeySpec;

public class TripleDES 
{
    SecretKey key;

    TripleDES(byte [] rawkey) throws Exception
    {
        key = readKey(rawkey);
    }

    public  SecretKey readKey(byte[] rawkey) throws Exception
    {
         DESedeKeySpec keyspec = new DESedeKeySpec(rawkey);
         SecretKeyFactory keyfactory = SecretKeyFactory.getInstance("DESede");
         SecretKey key = keyfactory.generateSecret(keyspec);
         return key;
    }

    public byte[] encrypt(byte[] plain ) throws Exception
    {
        Cipher cipher = Cipher.getInstance("DESede");
        cipher.init(Cipher.ENCRYPT_MODE, key);
        byte[] encrypted = cipher.doFinal(plain);
        return encrypted;
    }
    public byte[] decrypt(byte[] cipher ) throws Exception
    {
        Cipher dcipher = Cipher.getInstance("DESede");
        dcipher.init(Cipher.DECRYPT_MODE, key);
        byte[] decrypted = dcipher.doFinal(cipher);
        return decrypted;
    }  
}

DES algorithm:

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.DESKeySpec;

public class DES 
{
    SecretKey key;
    DES(byte []  KEY) throws Exception
    {
        key = readKey(KEY);
    }

    public  SecretKey readKey(byte[] rawkey) throws Exception
    {
         DESKeySpec keyspec = new DESKeySpec(rawkey);
         SecretKeyFactory keyfactory = SecretKeyFactory.getInstance("DES");
         SecretKey key = keyfactory.generateSecret(keyspec);
         return key;
    }

    public byte[] encrypt(byte[] plain ) throws Exception
    {
        Cipher cipher = Cipher.getInstance("DES");
        cipher.init(Cipher.ENCRYPT_MODE, key);
        byte[] encrypted = cipher.doFinal(plain);
        return encrypted;
    }
    public byte[] decrypt(byte[] cipher ) throws Exception
    {
        Cipher dcipher = Cipher.getInstance("DES");
        dcipher.init(Cipher.DECRYPT_MODE, key);
        byte[] decrypted = dcipher.doFinal(cipher);
        return decrypted;
    }  
}

and AES-128:

import java.security.SecureRandom;

import javax.crypto.Cipher;
import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

public class AES128 
{
    private  byte[] getRawKey(byte[] seed) throws Exception 
    {
        KeyGenerator kgen = KeyGenerator.getInstance("AES");
        SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
        sr.setSeed(seed);
        kgen.init(128, sr); // 192 and 256 bits may not be available
        SecretKey skey = kgen.generateKey();
        byte[] raw = skey.getEncoded();
        return raw;
    }

    public  byte[] encrypt(byte[] raw, byte[] clear) throws Exception 
    {
        SecretKeySpec skeySpec = new SecretKeySpec(getRawKey(raw), "AES");
        Cipher cipher = Cipher.getInstance("AES");
        cipher.init(Cipher.ENCRYPT_MODE, skeySpec);
        byte[] encrypted = cipher.doFinal(clear);
        return encrypted;
    }

    public  byte[] decrypt(byte[] raw, byte[] encrypted) throws Exception 
    {
        SecretKeySpec skeySpec = new SecretKeySpec(getRawKey(raw), "AES");
        Cipher cipher = Cipher.getInstance("AES");
        cipher.init(Cipher.DECRYPT_MODE, skeySpec);
        byte[] decrypted = cipher.doFinal(encrypted);
        return decrypted;
    }
}
share|improve this question
add comment

1 Answer

You may want to use "DESede/CBC/PKCS5Padding" and "AES/CBC/PKCS5Padding" for added security.

  • "CBC" is chained cipher block. Both AES and DES are symmetric block ciphers, they encrypt only a fixed block of at a time. Basically, CBC makes the cipher retain state from previous blocks as you encrypt the next block. This makes it more difficult to perform known-plaintext attacks on your ciphertext.

  • "PKCS5Padding" causes the cipher to pad the data, including the size of the source block. This allows it to validate the size of the decrypted data, making it more difficult to attack the ciphertext directly.

Your classes are somewhat inconsistent. AES128 doesn't keep any state, requires the key to be passed in, and creates the keys in a different way than the others.

These could probably all be coalesced into a class hierarchy with an abstract base class that took SecureKey and Cipher objects created in the derived constructor. The encrypt/decrypt methods could then be implemented directly in the base class.

share|improve this answer
1  
In crypto you should never rely on defaults, you have to specify everything. Specify the mode: CBC or CTR if you don't want authentication, GCM if you do. Specify the padding: PKCS7 (sometimes called PKCS5). Don't rely on defaults because they can come back and bite you when the latest version changes the default under you and you can no longer decrypt old stuff. –  rossum Nov 2 '12 at 23:03
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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