Mdtryhht

I am making a project just for fun that implements the 3DES algorithm in Java. I was wondering if my algorithm looks secure. Is there any advice or feedback you could give me? Do I need to include an IV? Am I transferring the salt successfully?

My code can encrypt a file using the Triple DES algorithm.

public static void main(String[] args) throws Exception 

 // file to be encrypted
 Scanner inputScanner = new Scanner(System.in); 
 System.out.println("Enter filename");
 String filename = inputScanner.nextLine();

 FileInputStream inputFile = new FileInputStream("C:\Documents\Encryptor\" + filename + ".txt");

 // encrypted file
 FileOutputStream outputFile = new FileOutputStream("C:\Documents\Encryptor\encryptedfile.des");

 // password to encrypt the file
 String passKey = "tkfhkggovubm";
 byte[] salt = new byte[8];
 Random r = new Random(); 
 r.nextBytes(salt);
 PBEKeySpec pbeKeySpec = new PBEKeySpec(passKey.toCharArray());
 SecretKeyFactory secretKeyFactory = SecretKeyFactory
 .getInstance("PBEWithSHA1AndDESede");
 SecretKey secretKey = secretKeyFactory.generateSecret(pbeKeySpec);


 PBEParameterSpec pbeParameterSpec = new PBEParameterSpec(salt, 99999);
 Cipher cipher = Cipher.getInstance("PBEWithSHA1AndDESede");
 cipher.init(Cipher.ENCRYPT_MODE, secretKey, pbeParameterSpec);
 outputFile.write(salt);

 byte[] input = new byte[64];
 int bytesRead;
 while ((bytesRead = inputFile.read(input)) != -1) 
 byte[] output = cipher.update(input, 0, bytesRead);
 if (output != null)
 outputFile.write(output);
 

 byte[] output = cipher.doFinal();
 if (output != null)
 outputFile.write(output);

 inputFile.close();
 outputFile.flush();
 outputFile.close();
 inputScanner.close();
 System.out.println("File has been Encrypted.");

It's kind of secure, but it uses older algorithms.

Although Benjamin correctly identifies 3DES, I would not call 3 key triple DES "broken". It still delivers a security of about 112 bits which nobody sane will try and break.

There is a chance that somebody would try and break your password though, and the shown password is clearly not random enough as it only contains 12 lowercase characters from a 26 character alphabet, which translates in 4.7 * 12 = 56 bits of entropy (each fully random letter delivers about 4.7 bits of entropy, 5.7 if upper and lowercase are randomly mixed). It may be that the relatively high number of iterations (99,999 iterations) will save you, but you're only supplying the 3DES key with half the entropy it requires to obtain the 112 bit security, so that's not enough.

The derivation method is probably secure, but it likely also performs too many operations which may just benefit an adversary. You are much better off with a more modern key derivation method such as Argon2. Likewise, we generally try and use authenticated encryption nowadays instead of the underlying CBC mode encryption. Problem is that there is no such prebuild solution directly available from the Java API, so you'd have to implement a copy of a protocol yourself or use a good library. Fernet would e.g. give you a more modern format.

You may want to include a version number to your encrypted messages so you can upgrade your algorithms or iteration count / salt size (etc.) at a later date. That way you can recognize older ciphertext, decrypt it, reencrypt it with the newer protocol or keys and finally securely erase the old ciphertext.

SHA-1 has been broken, but not enough for it to become a problem for PBE. Of course you should still try and avoid age old algorithms such as 3DES and SHA-1 and replace them with new ones.

Oh, almost forgot. Triple DES has a block size of 64 bits / 8 bytes, which is deemed pretty small nowadays. You could lose some confidentiality when encrypting large files with it in CBC mode because blocks may start to repeat (you might lose much more confidentiality with other modes of operation).

The idea of the password consisting of characters is that you can clear a char array, while you cannot do the same thing for a String. If you supply the password as a string then you lose this ability.

Do you know that there is a CipherInputStream and CipherOutputStream that can be put in front of a FileInputStream or FileOutputStream?

No, it's not secure.

Your code is using Random instead of SecureRandom, which limits the entropy of the salt to 48 bits.

In addition, as an auditor I would immediately reject any "security code" that is implemented directly in the main method. To demonstrate that you understand the building blocks of a cipher, your code has to be structured into manageable methods that make the relation between the basic ingredients as clear as possible. The code should explain how the encryption works, without overwhelming the reader with needless technical details. Keeping track of 5 variables in your head is already difficult.

The outermost method should be encrypt(File in, File out, Key key, Random rnd). Only if you provide this kind of API can you write useful unit tests to demonstrate that the encryption code works for at least a few select examples.