0
\$\begingroup\$

I am using the following code to encrypt a file in an SD card in Android:

I want the code to be reviewed for:

  • Security (prioritize this)
  • Coding Style
  • Performance

void encrypt() throws IOException, NoSuchAlgorithmException, NoSuchPaddingException,
        InvalidKeyException {
    String myString = getOutputFile();
    File myFile = new File(myString);
    FileInputStream inputStream = new FileInputStream(myFile);
    File encodedfile = new File(path,"filename" + ".mp4");
    FileOutputStream outputStream = new FileOutputStream(encodedfile);
    SecretKeySpec sks = new SecretKeySpec("MyDifficultPassw".getBytes(), "AES");
    Cipher cipher = Cipher.getInstance("AES");
    cipher.init(Cipher.ENCRYPT_MODE, sks);
    CipherOutputStream cos = new CipherOutputStream(outputStream, cipher);
    int b;
    byte[] d = new byte[8];
    while((b = inputStream.read(d)) != -1){
        cos.write(d, 0, b);
    }
    cos.flush();
    cos.close();
    inputStream.close();
}
\$\endgroup\$
  • \$\begingroup\$ i don't know ..... Am i using a 256 bit encryption. If not what code should i add .... -> Not suitable for this site, perhaps for stackoverflow if it isn't a duplicate \$\endgroup\$ – bhathiya-perera Aug 6 '14 at 9:43
  • \$\begingroup\$ btw Welcome to the Code Review Site, I want the file to be encrypted in aes 256 bit. -> not necessary, If that is the main purpose of asking the question It would not fit to the scope of this site. See the help center. Thanks \$\endgroup\$ – bhathiya-perera Aug 6 '14 at 9:53
1
\$\begingroup\$

Verbosity

String myString = getOutputFile();
File myFile = new File(myString);
FileInputStream inputStream = new FileInputStream(myFile);

FileInputStream can take String as argument. I'd do this instead:

FileInputStream inputStream = new FileInputStream(getOutputFile());

Whoa whoa whoa! We are reading from the output file? Did you make a mistake? I dunno, but by simply removing some lines I've suddenly exposed what could be a flaw!

Clarity

You're making a lot of objects and doing things with them. Try putting some blank space in between. It groups statements together and makes the code more logical.

void encrypt() throws IOException, NoSuchAlgorithmException, NoSuchPaddingException,
        InvalidKeyException {
    FileInputStream inputStream = new FileInputStream(getOutputFile());

    File encodedfile = new File(path,"filename" + ".mp4");
    FileOutputStream outputStream = new FileOutputStream(encodedfile);

    SecretKeySpec sks = new SecretKeySpec("MyDifficultPassw".getBytes(), "AES");
    Cipher cipher = Cipher.getInstance("AES");
    cipher.init(Cipher.ENCRYPT_MODE, sks);
    CipherOutputStream cos = new CipherOutputStream(outputStream, cipher);

    int b;
    byte[] d = new byte[8];
    while((b = inputStream.read(d)) != -1){
        cos.write(d, 0, b);
    }
    cos.flush();
    cos.close();
    inputStream.close();
}
\$\endgroup\$
  • \$\begingroup\$ The output file is the original file which i tried to encrypt. \$\endgroup\$ – Srijith Aug 6 '14 at 10:23

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.