0

Thank you for taking you time to assist me with this!

THIS POST HAS BEEN EDITED FOR LESS INFORMATION SEE THE EDITED PART

Well I have spend ours of research on this matter and I ended up with a working piece of code..

But Encryption is not a place to make mistakes, and I wanted to ask if my code is actualy secure! It's really important for me because I want to implement it to a program so my code is...

import java.nio.file.Files;
import java.nio.file.Paths;

import java.util.Base64;

import javax.crypto.*;
import javax.crypto.spec.SecretKeySpec;
import java.security.SecureRandom;


public class EncryptFile{
    private static final String FILE_IN = "./EncryptFile.java";
    private static final String FILE_ENCR = "./EncryptFile_encr.java";
    private static final String FILE_DECR = "./EncryptFile_decr.java";
     public static void main(String []args){
        try
        {
            Encryption("passwordisnottheactual", Files.readAllBytes(Paths.get(FILE_IN)));
            Decryption("passwordisnottheactual");

        }catch(Exception e){
            System.out.println(e.getMessage());
        }
     }
     private static void Encryption(String Key, byte[] byteArray) throws Exception
     {
        // Decode the base64 encoded Key
        byte[] decodedKey = Base64.getDecoder().decode(Key);
        // Rebuild the key using SecretKeySpec
        SecretKey secretKey = new SecretKeySpec(decodedKey, 0, decodedKey.length, "AES"); 

        // Cipher gets AES Algorithm instance
        Cipher AesCipher = Cipher.getInstance("AES");

        //Initialize AesCipher with Encryption Mode, Our Key and A ?SecureRandom?
        AesCipher.init(Cipher.ENCRYPT_MODE, secretKey, new SecureRandom());
        byte[] byteCipherText = AesCipher.doFinal(byteArray);

        //Write Bytes To File
        Files.write(Paths.get(FILE_ENCR), byteCipherText);


     }
     private static void Decryption(String Key) throws Exception
     {
        //Ddecode the base64 encoded string
        byte[] decodedKey = Base64.getDecoder().decode(Key);
        //Rebuild key using SecretKeySpec
        SecretKey secretKey = new SecretKeySpec(decodedKey, 0, decodedKey.length, "AES"); 

        //Read All The Bytes From The File
        byte[] cipherText = Files.readAllBytes(Paths.get(FILE_ENCR));

        //Cipher gets AES Algorithm Instance
        Cipher AesCipher = Cipher.getInstance("AES");

        //Initialize it in Decrypt mode, with our Key, and a ?SecureRandom?
        AesCipher.init(Cipher.DECRYPT_MODE, secretKey, new SecureRandom());

        byte[] bytePlainText = AesCipher.doFinal(cipherText);
        Files.write(Paths.get(FILE_DECR), bytePlainText);
     }
}

EDIT

Possible duplicate of Simple Java AES encrypt/decrypt example – JFPicard

Well it could be but these answers Use IVParameterSpec and I wanted to know if this line of code is actually secure or if it is bad practice:

AesCipher.init(Cipher.DECRYPT_MODE, secretKey, new SecureRandom());

because I use a new SecureRandom() every time, and I haven't seen anyone use a SecureRandom object like this.

  • 3
    Better try http://codereview.stackexchange.com/ since SO is probably not going to review your code. – TobiasR. Jan 17 '17 at 14:33
  • Possible duplicate of [Simple Java AES encrypt/decrypt example](http://stackoverflow.com/questions/15554296/simple-java-aes-encrypt-decrypt-example) – JFPicard Jan 17 '17 at 14:43

1 Answers1

2
  1. Encryption key
    • The password is passes as a string but the Encryption function Base64 decoded it, that is a coding error.
    • When a password is used the encryption key should be derived from it with the PBKDF2 (aka Rfc2898DeriveBytes) function.
    • When using key derivation the salt and iteration count needs to be available for decryption, often they are provided in a prefix to the encrypted data.
  2. Encryption mode
    • No encryption mode is supplied.
    • Use CBC mode with a random IV.
    • Just prefix the encrypted data with the IV for use on decryption.
  3. Padding
    • AES is a block cipher and as such requires the input data size to be a multiple of the block size.
    • Specify PKCS#7 (née PKCS#5) padding, it will add padding on encryption and remove it on decryption.
    • On decryption do not return "padding" errors, they can provide a "Padding Oracle" attack.
  4. Explicit
    • Specify all encryption parameters and sizes.
    • Do not rely on implementation defaults.
  5. Encryption authentication
    • Consider if there is a need to know if the data is decrypted correctly.
  6. Versioning
    • Add a version indicator so that if changes are necessary later there is an compatibility path.

Or consider using RNCryptor which handles all this and more.

Update: (thx Andy for the comment)
If GCM mode is available and interoperability across platforms and libraries is not an issue GCM is arguably a better encryption mode. GCM has authentication and padding build-in making it more robust and an easier secure solution.

zaph
  • 111,848
  • 21
  • 189
  • 228
  • I would recommend `AES/GCM` over `CBC` -- CBC provides confidentiality but no integrity assurance, and the overhead of the GCM authentication tag is minor (on AES-NI chips, it is quite performant). – Andy Jan 17 '17 at 18:59
  • I agree that GCM mode is better in several respects—except availability across platforms and libraries which is poor to lacking. – zaph Jan 17 '17 at 19:23
  • Genuinely curious -- are you considering HW implementations or reduced scope platforms here as well? I can't think of anything made in the last 4 years that runs Java but can't handle GCM. OP is using NIO2, so at least Java 7. I agree in principal with your assessment of GCM availability, but didn't think it was a limiting factor here. – Andy Jan 17 '17 at 19:30
  • I am addressing global platforms and libraries and the answer is not limited to Java even though the question is. Often interoperability is a factor in a decision. In particular, one large platform, iOS, does not support GCM without going to poorly vetted 3rd party implementations and these implementations do not support hardware encryption. – zaph Jan 17 '17 at 19:43