Hash-based cipher

Question

I've written a cipher class based off of cryptographically secure hash functions and block cipher counter mode of operation. It currently runs around 20MB/s on my machine with the Java implementation of SHA256. The class is here as well as below.

Any critique or input on the design or code?

package blackdoor.crypto;
import java.security.InvalidKeyException;
import java.security.Key;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Arrays;
import javax.crypto.SecretKey;
import javax.crypto.spec.IvParameterSpec;
import blackdoor.struct.ByteQueue;
import blackdoor.util.Misc;
/**
 * @author nfischer3
 * Secure hash encryption, uses hash algorithms in CTR mode for mirrored, symmetric encryption.
 */
public class SHECipher implements Cipher{
 public static final int MIN_KEY_SIZE = 8;
 /**
 * 
 * @return the default instance with a 256 bit block size
 */
 public static SHECipher getDefaultInstance(){
 try {
 return new SHECipher(MessageDigest.getInstance("SHA-256"));
 } catch (NoSuchAlgorithmException e) {
 e.printStackTrace();
 }
 return null;
 }
 private int blockNo = 0;
 private boolean cfg = false;
 private byte[] key;
 private ByteQueue buffer;
 private MessageDigest mD;
 private IvParameterSpec iv;
 private byte[] prehash;
 public SHECipher(MessageDigest mD){
 this.mD = mD;
 }
 public boolean isConfigured() {
 return cfg;
 }
 public String getAlgorithm(){
 return mD.getAlgorithm();
 }
 public byte[] getIV(){
 return iv.getIV();
 }
 /**
 * Initializes the cipher with key, creates a random IV to use with the cipher.
 * @param key A key to encrypt with. Key can be any length over MIN_KEY_SIZE but a key longer than the block size will run more slowly. 
 * @return An IV that has been created for this cipher to use. IV will be the same length as the key.
 * @throws InvalidKeyException 
 */
 public IvParameterSpec init(SecretKey key) throws InvalidKeyException{
 byte[] iv = new byte[key.getEncoded().length];
 new SecureRandom().nextBytes(iv);
 IvParameterSpec ivSpec = new IvParameterSpec(iv);
 init(key, ivSpec);
 return ivSpec;
 }
 /**
 * Initializes the cipher with key and iv
 * @param IV An initialization vector to use for the cipher.
 * @param key A key to encrypt with.
 * @throws InvalidKeyException 
 */
 @Override
 public void init(Key key, IvParameterSpec iv) throws InvalidKeyException {
 if(!(key instanceof SecretKey))
 throw new InvalidKeyException();
 int ivLength = iv.getIV().length;
 if(key.getEncoded().length < MIN_KEY_SIZE || key.getEncoded().length < ivLength)
 throw new InvalidKeyException("Key must be longer than " + MIN_KEY_SIZE + " bytes and key must be longer than IV.");
 this.key = key.getEncoded();
 this.iv = iv;
 prehash = Misc.XORintoA(this.key.length == ivLength ? iv.getIV() : Arrays.copyOf(iv.getIV(), this.key.length), this.key);
 blockNo = 0;
 buffer = new ByteQueue(getBlockSize()*2);
 buffer.setResizable(true);
 cfg = true;
 }
 /**
 * @return the minimum number of bytes buffered at a time for crypting.
 */
 public int getBlockSize(){
 return mD.getDigestLength();
 }
 public void reset(){
 cfg = false;
 Arrays.fill(key, (byte) 0);
 Arrays.fill(prehash, (byte) 0x0);
 buffer = null;
 mD.reset();
 }
 /**
 * Continues a multiple-part encryption or decryption operation (depending on how this cipher was initialized), processing another data part.
 * The bytes in the input buffer are processed, and the result is stored in a new buffer.
 *
 * If input has a length of zero, this method returns null.
 * @param input
 * @return
 */
 public byte[] update(byte[] input){
 if(!cfg)
 throw new Exceptions.CipherNotInitializedException();
 if(input.length == 0)
 return null;
 if(input.length > buffer.capacity())
 buffer.resize(input.length + getBlockSize());
 while(buffer.filled() < input.length){
 bufferKeystream();
 }
 return Misc.XORintoA(buffer.deQueue(input.length), input);
 }
 protected void bufferKeystream(){
 int i = blockNo % key.length;
 int inc = (blockNo/key.length) + 1;
 prehash[i] ^= key[i]; // expose IV[i] in prehash
 prehash[i] += inc; // apply ctr
 prehash[i] ^= key[i]; // cover IV[i] in prehash with key[i]
 buffer.enQueue(mD.digest(prehash)); // buffer keystream
 prehash[i] ^= key[i]; // expose IV[i[ in prehash
 prehash[i] -= inc; // remove ctr
 prehash[i] ^= key[i]; // cover IV[i[ in prehash with key[i]
 blockNo++;
 }
}

Answer 1

I'm going to comment only on the coding aspects as I am not a cryptographer. The only thing I can say is that if you aren't one you shouldn't roll out your own algorithms.

/**
 * 
 */

What are these comments for? They don't convey any information, you should better delete them.

public static SHECipher getDefaultInstance(){

I'd suggest you to get rid of this factory method and replace it with dependency injection. It might look innocuous, but with that method you're just introducing an hidden dependency in your system, which is going to cause you troubles if you ever want to swap it with another implementation or if you want to test your code, which you should.

public void init(Key key, IvParameterSpec iv) 

I'd get rid of the init method, and do all the initialisation in the constructor. Probably you want to decompose your class in two classes. A SHECiper, which represent cipher that is initialised and always in a valid state, and a SHECipherFactory that creates the instances you need and takes care of their initialisation. This would also allow you to make some of the fields of your class final, which is always a good thing.

public void reset()

Similary, you should get rid of reset too. If you don't need the cipher anymore just throw it away and create a new one instead.

Why do you keep code that is commented out? I'd delete all the zeroKey code. You're not using it anyway.

• kag0: Could you explain a bit more about dependency injection? There is currently a public constructor which uses it (I think) and the factory method doesn't introduce any new dependencies since MessageDigest has to be included regardless. With that in mind, could you explain how dependency injection applies here (I don't have much experience with the pattern)?
• mariosangiorgio: The problem with Cipher.getDefaultInstance() is that the code that uses it is strongly coupled with it and it is always going to use a cipher using SHA-256. What if you want to use something different it in the future? What if you want to use a different implementation in your tests? You might be interested in reading this post about service locator and why it is considered an anti-pattern, which explains in a detailed way the issue
• kag0: Why wouldn't the constructor public SHECipher(MessageDigest mD) be used in those cases? You can (and I have) used different implementations in my tests that way.