patternjavaMinor
File-encryption utility application
Viewed 0 times
encryptionfileutilityapplication
Problem
I made a simple file-encryption utility application for a little school project.
I'm really interested in cryptography, and tried my best while making it.
Can you see it, and give me feedback?
The input is always a
```
public class Cryptography {
private final Cipher c;
private final MessageDigest md5;
private final String AES_CBC_PKCS5 = "AES/CBC/PKCS5Padding";
private final String MD5 = "MD5";
private final int BLOCK_SIZE = 16;
private final byte[] bytesIV = new byte[BLOCK_SIZE];
public Cryptography() throws NoSuchAlgorithmException, NoSuchPaddingException {
c = Cipher.getInstance(AES_CBC_PKCS5);
md5 = MessageDigest.getInstance(MD5);
}
public void initIV() {
new SecureRandom().nextBytes(bytesIV);
}
public void encryptStream(InputStream in, OutputStream out, byte[] key) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException, IOException {
out.write(bytesIV);
out.flush();
c.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(bytesIV));
out = new CipherOutputStream(out, c);
byte[] buf = new byte[1024];
int numRead = 0;
while ((numRead = in.read(buf)) >= 0) {
out.write(buf, 0, numRead);
}
out.close();
}
public void decryptStream(InputStream in, OutputStream out, byte[] key) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException, IOException {
in.read(bytesIV);
c.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(bytesIV));
in = new CipherInputStream(in, c);
byte[] buf = ne
I'm really interested in cryptography, and tried my best while making it.
Can you see it, and give me feedback?
The input is always a
FileInputStream (wrapped by a BufferedInputStream), and the output is a ByteBufferOutputStream, which will be written to the output file.```
public class Cryptography {
private final Cipher c;
private final MessageDigest md5;
private final String AES_CBC_PKCS5 = "AES/CBC/PKCS5Padding";
private final String MD5 = "MD5";
private final int BLOCK_SIZE = 16;
private final byte[] bytesIV = new byte[BLOCK_SIZE];
public Cryptography() throws NoSuchAlgorithmException, NoSuchPaddingException {
c = Cipher.getInstance(AES_CBC_PKCS5);
md5 = MessageDigest.getInstance(MD5);
}
public void initIV() {
new SecureRandom().nextBytes(bytesIV);
}
public void encryptStream(InputStream in, OutputStream out, byte[] key) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException, IOException {
out.write(bytesIV);
out.flush();
c.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(bytesIV));
out = new CipherOutputStream(out, c);
byte[] buf = new byte[1024];
int numRead = 0;
while ((numRead = in.read(buf)) >= 0) {
out.write(buf, 0, numRead);
}
out.close();
}
public void decryptStream(InputStream in, OutputStream out, byte[] key) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException, IOException {
in.read(bytesIV);
c.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(bytesIV));
in = new CipherInputStream(in, c);
byte[] buf = ne
Solution
- Exposing the IV only leaves room for errors. Simply generate it at the start of encryption.
- There is no point in having a completely unrelated
hashmethod on this class.
- There is not integrity protection, so an attacker who can modify the ciphertext you decrypt will likely be able to pull off a padding-oracle attack. Unfortunately it's not trivial to combine streaming decryption with integrity protection, since you must ensure that you don't release any plaintext you haven't authenticated.
- Once you eliminate the IV storage, as recommended above, you have no state to preserve across calls, which means you don't need any instance fields anymore. They only make your class thread-unsafe without any benefit.
- Closing
outseems pretty weird, I wouldn't expect that behaviour. On the other hand you don't closein.
- If you decide to dispose resources, use
try...finally(or try-with-resources) to do so reliably.
- I'd make the class immutable and threadsafe, storing only the key (and algorithm if it's not hardcoded) in it.
Context
StackExchange Code Review Q#127833, answer score: 2
Revisions (0)
No revisions yet.