Async MAC authentication for HTTP range requests. Fixes #38

main/core/pom.xml

@@ -48,7 +48,13 @@
 			<artifactId>jackrabbit-webdav</artifactId>
 			<version>${jackrabbit.version}</version>
 		</dependency>
-
+	
+		<!-- Guava -->
+		<dependency>
+			<groupId>com.google.guava</groupId>
+			<artifactId>guava</artifactId>
+		</dependency>
+	
 		<!-- I/O -->
 		<dependency>
 			<groupId>commons-io</groupId>

main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavResourceFactoryImpl.java

@@ -10,6 +10,7 @@ package org.cryptomator.webdav.jackrabbit;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.concurrent.ExecutorService;
 
 import org.apache.commons.httpclient.HttpStatus;
 import org.apache.jackrabbit.webdav.DavException;
@@ -30,10 +31,12 @@ class DavResourceFactoryImpl implements DavResourceFactory {
 	private final LockManager lockManager = new SimpleLockManager();
 	private final Cryptor cryptor;
 	private final CryptoWarningHandler cryptoWarningHandler;
+	private final ExecutorService backgroundTaskExecutor;
 
-	DavResourceFactoryImpl(Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler) {
+	DavResourceFactoryImpl(Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler, ExecutorService backgroundTaskExecutor) {
 		this.cryptor = cryptor;
 		this.cryptoWarningHandler = cryptoWarningHandler;
+		this.backgroundTaskExecutor = backgroundTaskExecutor;
 	}
 
 	@Override
@@ -67,7 +70,7 @@ class DavResourceFactoryImpl implements DavResourceFactory {
 	}
 
 	private EncryptedFile createFilePart(DavResourceLocator locator, DavSession session, DavServletRequest request) {
-		return new EncryptedFilePart(this, locator, session, request, lockManager, cryptor, cryptoWarningHandler);
+		return new EncryptedFilePart(this, locator, session, request, lockManager, cryptor, cryptoWarningHandler, backgroundTaskExecutor);
 	}
 
 	private EncryptedFile createFile(DavResourceLocator locator, DavSession session) {

main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFile.java

@@ -40,7 +40,7 @@ class EncryptedFile extends AbstractEncryptedNode {
 
 	private static final Logger LOG = LoggerFactory.getLogger(EncryptedFile.class);
 
-	private final CryptoWarningHandler cryptoWarningHandler;
+	protected final CryptoWarningHandler cryptoWarningHandler;
 
 	public EncryptedFile(DavResourceFactory factory, DavResourceLocator locator, DavSession session, LockManager lockManager, Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler) {
 		super(factory, locator, session, lockManager, cryptor);
@@ -70,7 +70,7 @@ class EncryptedFile extends AbstractEncryptedNode {
 	@Override
 	public void spool(OutputContext outputContext) throws IOException {
 		final Path path = ResourcePathUtils.getPhysicalPath(this);
-		if (Files.exists(path)) {
+		if (Files.isRegularFile(path)) {
 			outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis());
 			outputContext.setProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString());
 			try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {

main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java

@@ -8,6 +8,8 @@ import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -25,6 +27,9 @@ import org.eclipse.jetty.http.HttpHeader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+
 /**
  * Delivers only the requested range of bytes from a file.
  * 
@@ -36,6 +41,7 @@ class EncryptedFilePart extends EncryptedFile {
 	private static final String BYTE_UNIT_PREFIX = "bytes=";
 	private static final char RANGE_SET_SEP = ',';
 	private static final char RANGE_SEP = '-';
+	private static final Cache<DavResourceLocator, MacAuthenticationJob> cachedMacAuthenticationJobs = CacheBuilder.newBuilder().expireAfterWrite(10, TimeUnit.MINUTES).build();
 
 	/**
 	 * e.g. range -500 (gets the last 500 bytes) -> (-1, 500)
@@ -49,13 +55,23 @@ class EncryptedFilePart extends EncryptedFile {
 
 	private final Set<Pair<Long, Long>> requestedContentRanges = new HashSet<Pair<Long, Long>>();
 
-	public EncryptedFilePart(DavResourceFactory factory, DavResourceLocator locator, DavSession session, DavServletRequest request, LockManager lockManager, Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler) {
+	public EncryptedFilePart(DavResourceFactory factory, DavResourceLocator locator, DavSession session, DavServletRequest request, LockManager lockManager, Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler,
+			ExecutorService backgroundTaskExecutor) {
 		super(factory, locator, session, lockManager, cryptor, cryptoWarningHandler);
 		final String rangeHeader = request.getHeader(HttpHeader.RANGE.asString());
 		if (rangeHeader == null) {
 			throw new IllegalArgumentException("HTTP request doesn't contain a range header");
 		}
 		determineByteRanges(rangeHeader);
+
+		synchronized (cachedMacAuthenticationJobs) {
+			if (cachedMacAuthenticationJobs.getIfPresent(locator) == null) {
+				final MacAuthenticationJob macAuthJob = new MacAuthenticationJob(locator);
+				cachedMacAuthenticationJobs.put(locator, macAuthJob);
+				backgroundTaskExecutor.submit(macAuthJob);
+			}
+		}
+
 	}
 
 	private void determineByteRanges(String rangeHeader) {
@@ -110,7 +126,7 @@ class EncryptedFilePart extends EncryptedFile {
 	@Override
 	public void spool(OutputContext outputContext) throws IOException {
 		final Path path = ResourcePathUtils.getPhysicalPath(this);
-		if (Files.exists(path)) {
+		if (Files.isRegularFile(path)) {
 			outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis());
 			try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {
 				final Long fileSize = cryptor.decryptedContentLength(channel);
@@ -135,4 +151,46 @@ class EncryptedFilePart extends EncryptedFile {
 		return String.format("%d-%d/%d", firstByte, lastByte, completeLength);
 	}
 
+	private class MacAuthenticationJob implements Runnable {
+
+		private final DavResourceLocator locator;
+
+		public MacAuthenticationJob(final DavResourceLocator locator) {
+			if (locator == null) {
+				throw new IllegalArgumentException("locator must not be null.");
+			}
+			this.locator = locator;
+		}
+
+		@Override
+		public void run() {
+			final Path path = ResourcePathUtils.getPhysicalPath(locator);
+			if (Files.isRegularFile(path) && Files.isReadable(path)) {
+				try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {
+					final boolean authentic = cryptor.isAuthentic(channel);
+					if (!authentic) {
+						cryptoWarningHandler.macAuthFailed(locator.getResourcePath());
+					}
+				} catch (IOException e) {
+					LOG.error("IOException during MAC verification of " + path.toString(), e);
+				}
+			}
+		}
+
+		@Override
+		public int hashCode() {
+			return locator.hashCode();
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			if (obj instanceof MacAuthenticationJob) {
+				final MacAuthenticationJob other = (MacAuthenticationJob) obj;
+				return this.locator.equals(other.locator);
+			} else {
+				return false;
+			}
+		}
+	}
+
 }

main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavServlet.java

@@ -9,6 +9,9 @@
 package org.cryptomator.webdav.jackrabbit;
 
 import java.util.Collection;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
@@ -30,6 +33,7 @@ public class WebDavServlet extends AbstractWebdavServlet {
 	private DavResourceFactory davResourceFactory;
 	private final Cryptor cryptor;
 	private final CryptoWarningHandler cryptoWarningHandler;
+	private ExecutorService backgroundTaskExecutor;
 
 	public WebDavServlet(final Cryptor cryptor, final Collection<String> failingMacCollection) {
 		super();
@@ -40,13 +44,26 @@ public class WebDavServlet extends AbstractWebdavServlet {
 	@Override
 	public void init(ServletConfig config) throws ServletException {
 		super.init(config);
-
-		davSessionProvider = new DavSessionProviderImpl();
-
 		final String fsRoot = config.getInitParameter(CFG_FS_ROOT);
-		this.davLocatorFactory = new DavLocatorFactoryImpl(fsRoot, cryptor);
+		backgroundTaskExecutor = Executors.newCachedThreadPool();
+		davSessionProvider = new DavSessionProviderImpl();
+		davLocatorFactory = new DavLocatorFactoryImpl(fsRoot, cryptor);
+		davResourceFactory = new DavResourceFactoryImpl(cryptor, cryptoWarningHandler, backgroundTaskExecutor);
+	}
 
-		this.davResourceFactory = new DavResourceFactoryImpl(cryptor, cryptoWarningHandler);
+	@Override
+	public void destroy() {
+		backgroundTaskExecutor.shutdown();
+		try {
+			final boolean tasksFinished = backgroundTaskExecutor.awaitTermination(10, TimeUnit.SECONDS);
+			if (!tasksFinished) {
+				backgroundTaskExecutor.shutdownNow();
+			}
+		} catch (InterruptedException e) {
+			backgroundTaskExecutor.shutdownNow();
+			Thread.currentThread().interrupt();
+		}
+		super.destroy();
 	}
 
 	@Override

main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java

@@ -416,6 +416,33 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 		encryptedFile.write(encryptedFileSizeBuffer);
 	}
 
+	@Override
+	public boolean isAuthentic(SeekableByteChannel encryptedFile) throws IOException {
+		// 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.");
+		}
+
+		// go to begin of content:
+		encryptedFile.position(64);
+
+		// calculated MAC
+		final InputStream in = new SeekableByteChannelInputStream(encryptedFile);
+		final InputStream macIn = new MacInputStream(in, calculatedMac);
+		IOUtils.copyLarge(macIn, new NullOutputStream());
+
+		// compare (in constant time):
+		return MessageDigest.isEqual(storedMac.array(), calculatedMac.doFinal());
+	}
+
 	@Override
 	public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException {
 		// read iv:

main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java

@@ -69,7 +69,7 @@ public class Aes256CryptorTest {
 		}
 	}
 
-	@Test(expected = DecryptFailedException.class)
+	@Test
 	public void testIntegrityAuthentication() throws IOException, DecryptFailedException {
 		// our test plaintext data:
 		final byte[] plaintextData = "Hello World".getBytes();
@@ -95,6 +95,38 @@ public class Aes256CryptorTest {
 
 		encryptedData.position(0);
 
+		// check mac (should return false)
+		final SeekableByteChannel encryptedIn = new ByteBufferBackedSeekableChannel(encryptedData);
+		final boolean authentic = cryptor.isAuthentic(encryptedIn);
+		Assert.assertFalse(authentic);
+	}
+
+	@Test(expected = DecryptFailedException.class)
+	public void testIntegrityViolationDuringDecryption() throws IOException, DecryptFailedException {
+		// our test plaintext data:
+		final byte[] plaintextData = "Hello World".getBytes();
+		final InputStream plaintextIn = new ByteArrayInputStream(plaintextData);
+
+		// init cryptor:
+		final Aes256Cryptor cryptor = new Aes256Cryptor();
+
+		// encrypt:
+		final ByteBuffer encryptedData = ByteBuffer.allocate(96);
+		final SeekableByteChannel encryptedOut = new ByteBufferBackedSeekableChannel(encryptedData);
+		cryptor.encryptFile(plaintextIn, encryptedOut);
+		IOUtils.closeQuietly(plaintextIn);
+		IOUtils.closeQuietly(encryptedOut);
+
+		encryptedData.position(0);
+
+		// toggle one bit inf first content byte:
+		encryptedData.position(64);
+		final byte fifthByte = encryptedData.get();
+		encryptedData.position(64);
+		encryptedData.put((byte) (fifthByte ^ 0x01));
+
+		encryptedData.position(0);
+
 		// decrypt modified content (should fail with DecryptFailedException):
 		final SeekableByteChannel encryptedIn = new ByteBufferBackedSeekableChannel(encryptedData);
 		final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream();

main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java

@@ -75,6 +75,11 @@ public interface Cryptor extends SensitiveDataSwipeListener {
 	 */
 	Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException;
 
+	/**
+	 * @return true, if the stored MAC matches the calculated one.
+	 */
+	boolean isAuthentic(SeekableByteChannel encryptedFile) throws IOException;
+
 	/**
 	 * @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

main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingDecorator.java

@@ -81,6 +81,11 @@ public class SamplingDecorator implements Cryptor, CryptorIOSampling {
 		return cryptor.decryptedContentLength(encryptedFile);
 	}
 
+	@Override
+	public boolean isAuthentic(SeekableByteChannel encryptedFile) throws IOException {
+		return cryptor.isAuthentic(encryptedFile);
+	}
+
 	@Override
 	public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException {
 		final OutputStream countingInputStream = new CountingOutputStream(decryptedBytes, plaintextFile);

main/pom.xml

@@ -1,5 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<!-- Copyright (c) 2014 Sebastian Stenzel This file is licensed under the terms of the MIT license. See the LICENSE.txt file for more info. Contributors: Sebastian Stenzel - initial API and implementation -->
+<!--
+  Copyright (c) 2014 Sebastian Stenzel
+  This file is licensed under the terms of the MIT license.
+  See the LICENSE.txt file for more info.
+  
+  Contributors:
+      Sebastian Stenzel - initial API and implementation
+-->
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>org.cryptomator</groupId>
@@ -103,7 +110,14 @@
 				<artifactId>commons-codec</artifactId>
 				<version>${commons-codec.version}</version>
 			</dependency>
-			
+
+			<!-- Guava -->
+			<dependency>
+				<groupId>com.google.guava</groupId>
+				<artifactId>guava</artifactId>
+				<version>18.0</version>
+			</dependency>
+
 			<!-- DI -->
 			<dependency>
 				<groupId>com.google.inject</groupId>

main/ui/src/main/resources/fxml/mac_warnings.fxml

@@ -14,12 +14,12 @@
 <?import javafx.scene.control.Label?>
 <?import javafx.scene.control.Button?>
 
-<VBox alignment="CENTER" prefHeight="225.0" prefWidth="400.0" spacing="12.0" fx:controller="org.cryptomator.ui.controllers.MacWarningsController" xmlns:fx="http://javafx.com/fxml">
+<VBox alignment="CENTER" prefHeight="225.0" prefWidth="525.0" spacing="12.0" fx:controller="org.cryptomator.ui.controllers.MacWarningsController" xmlns:fx="http://javafx.com/fxml">
 
 	<padding><Insets top="12.0" right="12.0" bottom="12.0" left="12.0"/></padding>
 
 	<children>
-		<Label text="%macWarnings.message" />
+		<Label textAlignment="CENTER" text="%macWarnings.message"/>
 		<ListView fx:id="warningsList" VBox.vgrow="ALWAYS" focusTraversable="false" />
 		<Button text="%macWarnings.okButton" defaultButton="true" prefWidth="150.0" onAction="#hideWindow" />
 	</children>

main/ui/src/main/resources/localization.properties

@@ -48,9 +48,9 @@ unlocked.button.lock=Lock vault
 unlocked.ioGraph.yAxis.label=Throughput (MiB/s)
 
 # mac_warnings.fxml
-macWarnings.windowTitle=Mac authentication failed
-macWarnings.message=The following files may be corrupted
-macWarnings.okButton=Dismiss warning
+macWarnings.windowTitle=MAC authentication failed
+macWarnings.message=Cryptomator detected potentially malicious corruptions in the following files:
+macWarnings.okButton=I'll be careful
 
 # tray icon
 tray.menu.open=Open