CODEWITHSUNDEEP
javaencryptioncryptography
ozzymado
ozzymado

Reputation: 998

Decrypt method returning null

I have this cryptography class.

import java.security.*;
import javax.crypto.*;
import javax.crypto.spec.*;
public class Crypto
{
    public Crypto(){

    }

    public static void main(String args[]){
        Crypto crypto = new Crypto();
        byte encrypted[] = crypto.encrypt("test encryption");
        System.out.println(encrypted);
        String decrypted = crypto.decrypt(encrypted);
        System.out.println(decrypted);

    }

    public byte[] encrypt(String input){
        try{
            Crypto crypto = new Crypto();
            SecretKeySpec key = crypto.hashPhrase();
            Cipher aes = Cipher.getInstance("AES/ECB/PKCS5Padding");
            aes.init(Cipher.ENCRYPT_MODE, key);
            return aes.doFinal(input.getBytes());
        }
        catch(Exception e){
            return null;
        }
    }

    public SecretKeySpec hashPhrase(){
        try{
            String code = "some code";
            MessageDigest digest = MessageDigest.getInstance("SHA");
            digest.update(code.getBytes());
            return new SecretKeySpec(digest.digest(), 0, 16, "AES");
        }
        catch(Exception e){
            return null;
        }
    }

    public String decrypt(byte[] input){
        try{
            Crypto crypto = new Crypto();
            SecretKeySpec key = crypto.hashPhrase();
            Cipher aes = Cipher.getInstance("AES/ECB/PKCS5Padding");
            aes.init(Cipher.DECRYPT_MODE, key);
            return new String(aes.doFinal(input));
        }
        catch(Exception e){
            return null;
        }
    }
}

When I run main in this class, it works fine. I see an encrypted value, and then after the decrypt method is called I see the original input - "test encryption."

However, when I try to actually use the decrypt method, I run into issues. I've shortened this class a bit to only show the relevant parts.

public void read() throws java.io.IOException{
    Crypto crypto = new Crypto();
    byte[] input;
    BufferedReader in = new BufferedReader(new FileReader("C:\\Budget\\data.txt")); 
    while(in.ready()) {
        input = in.readLine().getBytes();
        BudgetInterface.list.add(crypto.decrypt(input)); //ArrayList of Objects
        System.out.println(crypto.decrypt(input));
        //BudgetInterface.list.add(in.readLine()); - artifact from version without cryptographic capability
    }
    in.close();
}

BudgetInterface.list is an ArrayList of objects, as mentioned, and I'm trying to add the decrypted version of the input to the array, but crypto.decrypt(input) is returning for each line in the file. If I remove the cryptography element, it's able to read in lines from the file with no problem. If I don't try to decrypt, it also reads in just fine. Why does the decrypt method return null in this method, but not in the main method of the Crypto class?

EDIT: The error I'm getting after getting the stack trace is javax.crypto.IllegalBlockSizeException: Input length must be multiple of 16 when decrypting with padded cipher I understand what this is saying, but how come this only happens when I get something encrypted from a file? The byte array that I'm reading from the file and the one I'm getting from the main method in Crypto are the same length.

Upvotes: 0

Views: 2703

Answers (2)

Jon Skeet
Jon Skeet

Reputation: 1503419

Aside from everything else, the most fundamental problem you've got here is that you're converting binary data to a string just using the platform default encoding, as if it's actually just text. It's not - it's binary data.

If you want one line of text per encrypted input, that's fine - but you need to convert the binary data to text as base64 or something similar. Never, ever, ever treat arbitrary binary data (encrypted data, compressed data, image data, music data... basically anything other than text) as if it's just encoded text. Even when you do have encoded text, specify the encoding explicitly.

Fix that - and stop using BufferedReader.ready(), preferring to call readLine() and terminate when it returns null - and you'll be in a better position. Fix the exception handling too, of course.

See my blog post about reversible transformations for more information about diagnosing this sort of thing, and Marc Gravell's post about IO for more about IO bugs.

Upvotes: 1

azurefrog
azurefrog

Reputation: 10955

Look at what your decrypt method is doing:

    catch(Exception e){
        return null;
    }

If you aren't going to handle the exception, you shouldn't catch it. Or at the very least, you should print the stack trace for the exception:

    catch(Exception e){
        e.printStackTrace();
    }

Upvotes: 0

Related Questions