Skip to content

Commit

Permalink
on-the-fly MAC calculation for better performance (addresses issue #38)
Browse files Browse the repository at this point in the history
we still need to add some kind of warning on the UI and create an async MAC checker for ranged requests
  • Loading branch information
Sebastian Stenzel committed Mar 1, 2015
1 parent 9433c22 commit 2849e39
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 81 deletions.
Expand Up @@ -11,7 +11,6 @@
import java.io.IOException; import java.io.IOException;
import java.nio.channels.SeekableByteChannel; import java.nio.channels.SeekableByteChannel;
import java.nio.file.DirectoryStream; import java.nio.file.DirectoryStream;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.FileVisitResult; import java.nio.file.FileVisitResult;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
Expand Down Expand Up @@ -79,12 +78,10 @@ private void addMemberDir(DavResource resource, InputContext inputContext) throw


private void addMemberFile(DavResource resource, InputContext inputContext) throws DavException { private void addMemberFile(DavResource resource, InputContext inputContext) throws DavException {
final Path childPath = ResourcePathUtils.getPhysicalPath(resource); final Path childPath = ResourcePathUtils.getPhysicalPath(resource);
try (final SeekableByteChannel channel = Files.newByteChannel(childPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW)) { try (final SeekableByteChannel channel = Files.newByteChannel(childPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {
cryptor.encryptFile(inputContext.getInputStream(), channel); cryptor.encryptFile(inputContext.getInputStream(), channel);
} catch (SecurityException e) { } catch (SecurityException e) {
throw new DavException(DavServletResponse.SC_FORBIDDEN, e); throw new DavException(DavServletResponse.SC_FORBIDDEN, e);
} catch (FileAlreadyExistsException e) {
throw new DavException(DavServletResponse.SC_CONFLICT, e);
} catch (IOException e) { } catch (IOException e) {
LOG.error("Failed to create file.", e); LOG.error("Failed to create file.", e);
throw new IORuntimeException(e); throw new IORuntimeException(e);
Expand Down Expand Up @@ -124,7 +121,9 @@ public DavResourceIterator getMembers() {
public void removeMember(DavResource member) throws DavException { public void removeMember(DavResource member) throws DavException {
final Path memberPath = ResourcePathUtils.getPhysicalPath(member); final Path memberPath = ResourcePathUtils.getPhysicalPath(member);
try { try {
Files.walkFileTree(memberPath, new DeletingFileVisitor()); if (Files.exists(memberPath)) {
Files.walkFileTree(memberPath, new DeletingFileVisitor());
}
} catch (SecurityException e) { } catch (SecurityException e) {
throw new DavException(DavServletResponse.SC_FORBIDDEN, e); throw new DavException(DavServletResponse.SC_FORBIDDEN, e);
} catch (IOException e) { } catch (IOException e) {
Expand Down
Expand Up @@ -29,6 +29,7 @@
import org.apache.jackrabbit.webdav.property.DefaultDavProperty; import org.apache.jackrabbit.webdav.property.DefaultDavProperty;
import org.cryptomator.crypto.Cryptor; import org.cryptomator.crypto.Cryptor;
import org.cryptomator.crypto.exceptions.DecryptFailedException; import org.cryptomator.crypto.exceptions.DecryptFailedException;
import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException;
import org.cryptomator.webdav.exceptions.IORuntimeException; import org.cryptomator.webdav.exceptions.IORuntimeException;
import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpHeaderValue;
Expand Down Expand Up @@ -70,12 +71,17 @@ public void spool(OutputContext outputContext) throws IOException {
outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis()); outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis());
outputContext.setProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString()); outputContext.setProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString());
try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) { try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {
outputContext.setContentLength(cryptor.decryptedContentLength(channel)); final Long contentLength = cryptor.decryptedContentLength(channel);
if (contentLength != null) {
outputContext.setContentLength(contentLength);
}
if (outputContext.hasStream()) { if (outputContext.hasStream()) {
cryptor.decryptedFile(channel, outputContext.getOutputStream()); cryptor.decryptFile(channel, outputContext.getOutputStream());
} }
} catch (EOFException e) { } catch (EOFException e) {
LOG.warn("Unexpected end of stream (possibly client hung up)."); LOG.warn("Unexpected end of stream (possibly client hung up).");
} catch (MacAuthenticationFailedException e) {
LOG.warn("MAC authentication failed, file content {} might be compromised.", getLocator().getResourcePath());
} catch (DecryptFailedException e) { } catch (DecryptFailedException e) {
throw new IOException("Error decrypting file " + path.toString(), e); throw new IOException("Error decrypting file " + path.toString(), e);
} }
Expand Down
Expand Up @@ -47,6 +47,7 @@
import org.cryptomator.crypto.AbstractCryptor; import org.cryptomator.crypto.AbstractCryptor;
import org.cryptomator.crypto.CryptorIOSupport; import org.cryptomator.crypto.CryptorIOSupport;
import org.cryptomator.crypto.exceptions.DecryptFailedException; import org.cryptomator.crypto.exceptions.DecryptFailedException;
import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException;
import org.cryptomator.crypto.exceptions.UnsupportedKeyLengthException; import org.cryptomator.crypto.exceptions.UnsupportedKeyLengthException;
import org.cryptomator.crypto.exceptions.WrongPasswordException; import org.cryptomator.crypto.exceptions.WrongPasswordException;
import org.cryptomator.crypto.io.SeekableByteChannelInputStream; import org.cryptomator.crypto.io.SeekableByteChannelInputStream;
Expand Down Expand Up @@ -369,34 +370,6 @@ private void storeMetadata(CryptorIOSupport ioSupport, String metadataFile, Long
ioSupport.writePathSpecificMetadata(metadataFile, objectMapper.writeValueAsBytes(metadata)); ioSupport.writePathSpecificMetadata(metadataFile, objectMapper.writeValueAsBytes(metadata));
} }


private void authenticateContent(SeekableByteChannel encryptedFile) throws IOException, DecryptFailedException {
// init mac:
final Mac calculatedMac = this.hmacSha256(hMacMasterKey);

// read stored mac:
encryptedFile.position(16);
final ByteBuffer storedMac = ByteBuffer.allocate(calculatedMac.getMacLength());
final int numMacBytesRead = encryptedFile.read(storedMac);

// check validity of header:
if (numMacBytesRead != calculatedMac.getMacLength()) {
throw new IOException("Failed to read file header.");
}

// read all encrypted data and calculate mac:
encryptedFile.position(64);
final InputStream in = new SeekableByteChannelInputStream(encryptedFile);
final InputStream macIn = new MacInputStream(in, calculatedMac);
IOUtils.copyLarge(macIn, new NullOutputStream());

// compare (in constant time):
boolean macMatches = MessageDigest.isEqual(storedMac.array(), calculatedMac.doFinal());

if (!macMatches) {
throw new DecryptFailedException("MAC authentication failed.");
}
}

@Override @Override
public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException { public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException {
// skip 128bit IV + 256 bit MAC: // skip 128bit IV + 256 bit MAC:
Expand All @@ -422,24 +395,49 @@ public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOE
} }
} }


private void encryptedContentLength(SeekableByteChannel encryptedFile, Long contentLength) throws IOException {
final ByteBuffer encryptedFileSizeBuffer;

// encrypt content length in ECB mode (content length is less than one block):
try {
final ByteBuffer fileSizeBuffer = ByteBuffer.allocate(Long.BYTES);
fileSizeBuffer.putLong(contentLength);
final Cipher sizeCipher = aesEcbCipher(primaryMasterKey, Cipher.ENCRYPT_MODE);
final byte[] encryptedFileSize = sizeCipher.doFinal(fileSizeBuffer.array());
encryptedFileSizeBuffer = ByteBuffer.wrap(encryptedFileSize);
} catch (IllegalBlockSizeException | BadPaddingException e) {
throw new IllegalStateException("Block size must be valid, as padding is requested. BadPaddingException not possible in encrypt mode.", e);
}

// skip 128bit IV + 256 bit MAC:
encryptedFile.position(48);

// write result:
encryptedFile.write(encryptedFileSizeBuffer);
}

@Override @Override
public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException { public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException {
// read iv: // read iv:
encryptedFile.position(0); encryptedFile.position(0);
final ByteBuffer countingIv = ByteBuffer.allocate(AES_BLOCK_LENGTH); final ByteBuffer countingIv = ByteBuffer.allocate(AES_BLOCK_LENGTH);
final int numIvBytesRead = encryptedFile.read(countingIv); final int numIvBytesRead = encryptedFile.read(countingIv);


// init mac:
final Mac calculatedMac = this.hmacSha256(hMacMasterKey);

// read stored mac:
final ByteBuffer storedMac = ByteBuffer.allocate(calculatedMac.getMacLength());
final int numMacBytesRead = encryptedFile.read(storedMac);

// read file size: // read file size:
final Long fileSize = decryptedContentLength(encryptedFile); final Long fileSize = decryptedContentLength(encryptedFile);


// check validity of header: // check validity of header:
if (numIvBytesRead != AES_BLOCK_LENGTH || fileSize == null) { if (numIvBytesRead != AES_BLOCK_LENGTH || numMacBytesRead != calculatedMac.getMacLength() || fileSize == null) {
throw new IOException("Failed to read file header."); throw new IOException("Failed to read file header.");
} }


// check MAC:
this.authenticateContent(encryptedFile);

// go to begin of content: // go to begin of content:
encryptedFile.position(64); encryptedFile.position(64);


Expand All @@ -448,8 +446,25 @@ public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaint


// read content // read content
final InputStream in = new SeekableByteChannelInputStream(encryptedFile); final InputStream in = new SeekableByteChannelInputStream(encryptedFile);
final InputStream cipheredIn = new CipherInputStream(in, cipher); final InputStream macIn = new MacInputStream(in, calculatedMac);
return IOUtils.copyLarge(cipheredIn, plaintextFile, 0, fileSize); final InputStream cipheredIn = new CipherInputStream(macIn, cipher);
final long bytesDecrypted = IOUtils.copyLarge(cipheredIn, plaintextFile, 0, fileSize);

// drain remaining bytes to /dev/null to complete MAC calculation:
IOUtils.copyLarge(macIn, new NullOutputStream());

// compare (in constant time):
final boolean macMatches = MessageDigest.isEqual(storedMac.array(), calculatedMac.doFinal());
if (!macMatches) {
// This exception will be thrown AFTER we sent the decrypted content to the user.
// This has two advantages:
// - we don't need to read files twice
// - we can still restore files suffering from non-malicious bit rotting
// Anyway me MUST make sure to warn the user. This will be done by the UI when catching this exception.
throw new MacAuthenticationFailedException("MAC authentication failed.");
}

return bytesDecrypted;
} }


@Override @Override
Expand All @@ -464,9 +479,6 @@ public Long decryptRange(SeekableByteChannel encryptedFile, OutputStream plainte
throw new IOException("Failed to read file header."); throw new IOException("Failed to read file header.");
} }


// check MAC:
this.authenticateContent(encryptedFile);

// seek relevant position and update iv: // seek relevant position and update iv:
long firstRelevantBlock = pos / AES_BLOCK_LENGTH; // cut of fraction! long firstRelevantBlock = pos / AES_BLOCK_LENGTH; // cut of fraction!
long beginOfFirstRelevantBlock = firstRelevantBlock * AES_BLOCK_LENGTH; long beginOfFirstRelevantBlock = firstRelevantBlock * AES_BLOCK_LENGTH;
Expand All @@ -493,7 +505,6 @@ public Long encryptFile(InputStream plaintextFile, SeekableByteChannel encrypted
// use an IV, whose last 8 bytes store a long used in counter mode and write initial value to file. // use an IV, whose last 8 bytes store a long used in counter mode and write initial value to file.
final ByteBuffer countingIv = ByteBuffer.wrap(randomData(AES_BLOCK_LENGTH)); final ByteBuffer countingIv = ByteBuffer.wrap(randomData(AES_BLOCK_LENGTH));
countingIv.putLong(AES_BLOCK_LENGTH - Long.BYTES, 0l); countingIv.putLong(AES_BLOCK_LENGTH - Long.BYTES, 0l);
countingIv.position(0);
encryptedFile.write(countingIv); encryptedFile.write(countingIv);


// init crypto stuff: // init crypto stuff:
Expand All @@ -504,20 +515,8 @@ public Long encryptFile(InputStream plaintextFile, SeekableByteChannel encrypted
final ByteBuffer macBuffer = ByteBuffer.allocate(mac.getMacLength()); final ByteBuffer macBuffer = ByteBuffer.allocate(mac.getMacLength());
encryptedFile.write(macBuffer); encryptedFile.write(macBuffer);


// write initial file size = 0 into the next 16 bytes // encrypt and write "zero length" as a placeholder, which will be read by concurrent requests, as long as encryption didn't finish:
final ByteBuffer encryptedFileSizeBuffer = ByteBuffer.allocate(AES_BLOCK_LENGTH); encryptedContentLength(encryptedFile, 0l);
try {
final ByteBuffer fileSizeBuffer = ByteBuffer.allocate(Long.BYTES);
fileSizeBuffer.putLong(0l);
final Cipher sizeCipher = aesEcbCipher(primaryMasterKey, Cipher.ENCRYPT_MODE);
final byte[] encryptedFileSize = sizeCipher.doFinal(fileSizeBuffer.array());
encryptedFileSizeBuffer.position(0);
encryptedFileSizeBuffer.put(encryptedFileSize);
encryptedFileSizeBuffer.position(0);
encryptedFile.write(encryptedFileSizeBuffer);
} catch (IllegalBlockSizeException | BadPaddingException e) {
throw new IllegalStateException("Block size must be valid, as padding is requested. BadPaddingException not possible in encrypt mode.", e);
}


// write content: // write content:
final OutputStream out = new SeekableByteChannelOutputStream(encryptedFile); final OutputStream out = new SeekableByteChannelOutputStream(encryptedFile);
Expand All @@ -540,26 +539,14 @@ public Long encryptFile(InputStream plaintextFile, SeekableByteChannel encrypted
blockSizeBufferedOut.flush(); blockSizeBufferedOut.flush();


// write MAC of total ciphertext: // write MAC of total ciphertext:
macBuffer.position(0); macBuffer.clear();
macBuffer.put(mac.doFinal()); macBuffer.put(mac.doFinal());
macBuffer.position(0); macBuffer.flip();
encryptedFile.position(16); // right behind the IV encryptedFile.position(16); // right behind the IV
encryptedFile.write(macBuffer); // 256 bit MAC encryptedFile.write(macBuffer); // 256 bit MAC


// encrypt and write plaintextSize // encrypt and write plaintextSize:
try { encryptedContentLength(encryptedFile, plaintextSize);
final ByteBuffer fileSizeBuffer = ByteBuffer.allocate(Long.BYTES);
fileSizeBuffer.putLong(plaintextSize);
final Cipher sizeCipher = aesEcbCipher(primaryMasterKey, Cipher.ENCRYPT_MODE);
final byte[] encryptedFileSize = sizeCipher.doFinal(fileSizeBuffer.array());
encryptedFileSizeBuffer.position(0);
encryptedFileSizeBuffer.put(encryptedFileSize);
encryptedFileSizeBuffer.position(0);
encryptedFile.position(48); // right behind the IV and MAC
encryptedFile.write(encryptedFileSizeBuffer);
} catch (IllegalBlockSizeException | BadPaddingException e) {
throw new IllegalStateException("Block size must be valid, as padding is requested. BadPaddingException not possible in encrypt mode.", e);
}


return plaintextSize; return plaintextSize;
} }
Expand Down
Expand Up @@ -98,7 +98,7 @@ public void testIntegrityAuthentication() throws IOException, DecryptFailedExcep
// decrypt modified content (should fail with DecryptFailedException): // decrypt modified content (should fail with DecryptFailedException):
final SeekableByteChannel encryptedIn = new ByteBufferBackedSeekableChannel(encryptedData); final SeekableByteChannel encryptedIn = new ByteBufferBackedSeekableChannel(encryptedData);
final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream(); final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream();
cryptor.decryptedFile(encryptedIn, plaintextOut); cryptor.decryptFile(encryptedIn, plaintextOut);
} }


@Test @Test
Expand Down Expand Up @@ -126,7 +126,7 @@ public void testEncryptionAndDecryption() throws IOException, DecryptFailedExcep


// decrypt: // decrypt:
final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream(); final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream();
final Long numDecryptedBytes = cryptor.decryptedFile(encryptedIn, plaintextOut); final Long numDecryptedBytes = cryptor.decryptFile(encryptedIn, plaintextOut);
IOUtils.closeQuietly(encryptedIn); IOUtils.closeQuietly(encryptedIn);
IOUtils.closeQuietly(plaintextOut); IOUtils.closeQuietly(plaintextOut);
Assert.assertEquals(filesize.longValue(), numDecryptedBytes.longValue()); Assert.assertEquals(filesize.longValue(), numDecryptedBytes.longValue());
Expand Down
Expand Up @@ -79,7 +79,7 @@ public interface Cryptor extends SensitiveDataSwipeListener {
* @return Number of decrypted bytes. This might not be equal to the encrypted file size due to optional metadata written to it. * @return Number of decrypted bytes. This might not be equal to the encrypted file size due to optional metadata written to it.
* @throws DecryptFailedException If decryption failed * @throws DecryptFailedException If decryption failed
*/ */
Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException; Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException;


/** /**
* @param pos First byte (inclusive) * @param pos First byte (inclusive)
Expand Down
Expand Up @@ -82,9 +82,9 @@ public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOE
} }


@Override @Override
public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException { public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException {
final OutputStream countingInputStream = new CountingOutputStream(decryptedBytes, plaintextFile); final OutputStream countingInputStream = new CountingOutputStream(decryptedBytes, plaintextFile);
return cryptor.decryptedFile(encryptedFile, countingInputStream); return cryptor.decryptFile(encryptedFile, countingInputStream);
} }


@Override @Override
Expand Down
@@ -0,0 +1,11 @@
package org.cryptomator.crypto.exceptions;

public class MacAuthenticationFailedException extends DecryptFailedException {

private static final long serialVersionUID = -5577052361643658772L;

public MacAuthenticationFailedException(String msg) {
super(msg);
}

}

0 comments on commit 2849e39

Please sign in to comment.