Merge remote-tracking branch 'origin/master' into 0.5.2

Conflicts:
	main/core/pom.xml
	main/crypto-aes/pom.xml
	main/crypto-api/pom.xml
	main/pom.xml
	main/ui/pom.xml

README.md

@@ -5,7 +5,7 @@ Cryptomator
 
 Multiplatform transparent client-side encryption of your files in the cloud.
 
-If you want to take a look at the current beta version, go ahead and get your copy of cryptomator on  [Cryptomator.org](http://cryptomator.org) or clone and build Cryptomator using Maven (instructions below).
+If you want to take a look at the current beta version, go ahead and get your copy of cryptomator on  [Cryptomator.org](https://cryptomator.org) or clone and build Cryptomator using Maven (instructions below).
 
 ## Features
 - Totally transparent: Just work on the encrypted volume, as if it was an USB drive

main/core/pom.xml

@@ -12,7 +12,7 @@
 	<parent>
 		<groupId>org.cryptomator</groupId>
 		<artifactId>main</artifactId>
-		<version>0.5.2</version>
+		<version>0.6.0-SNAPSHOT</version>
 	</parent>
 	<artifactId>core</artifactId>
 	<name>Cryptomator WebDAV and I/O module</name>

main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedDir.java

@@ -78,9 +78,7 @@ public class EncryptedDir extends AbstractEncryptedNode {
 
 	private void addMemberFile(DavResource resource, InputContext inputContext) throws DavException {
 		final Path childPath = ResourcePathUtils.getPhysicalPath(resource);
-		SeekableByteChannel channel = null;
-		try {
-			channel = Files.newByteChannel(childPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE);
+		try (final SeekableByteChannel channel = Files.newByteChannel(childPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {
 			cryptor.encryptFile(inputContext.getInputStream(), channel);
 		} catch (SecurityException e) {
 			throw new DavException(DavServletResponse.SC_FORBIDDEN, e);
@@ -88,7 +86,6 @@ public class EncryptedDir extends AbstractEncryptedNode {
 			LOG.error("Failed to create file.", e);
 			throw new IORuntimeException(e);
 		} finally {
-			IOUtils.closeQuietly(channel);
 			IOUtils.closeQuietly(inputContext.getInputStream());
 		}
 	}
@@ -124,7 +121,9 @@ public class EncryptedDir extends AbstractEncryptedNode {
 	public void removeMember(DavResource member) throws DavException {
 		final Path memberPath = ResourcePathUtils.getPhysicalPath(member);
 		try {
-			Files.walkFileTree(memberPath, new DeletingFileVisitor());
+			if (Files.exists(memberPath)) {
+				Files.walkFileTree(memberPath, new DeletingFileVisitor());
+			}
 		} catch (SecurityException e) {
 			throw new DavException(DavServletResponse.SC_FORBIDDEN, e);
 		} catch (IOException e) {

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

@@ -16,7 +16,6 @@ import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 import java.nio.file.attribute.BasicFileAttributes;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.webdav.DavException;
 import org.apache.jackrabbit.webdav.DavResource;
 import org.apache.jackrabbit.webdav.DavResourceFactory;
@@ -30,6 +29,7 @@ import org.apache.jackrabbit.webdav.property.DavPropertyName;
 import org.apache.jackrabbit.webdav.property.DefaultDavProperty;
 import org.cryptomator.crypto.Cryptor;
 import org.cryptomator.crypto.exceptions.DecryptFailedException;
+import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException;
 import org.cryptomator.webdav.exceptions.IORuntimeException;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpHeaderValue;
@@ -70,19 +70,20 @@ public class EncryptedFile extends AbstractEncryptedNode {
 		if (Files.exists(path)) {
 			outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis());
 			outputContext.setProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString());
-			SeekableByteChannel channel = null;
-			try {
-				channel = Files.newByteChannel(path, StandardOpenOption.READ);
-				outputContext.setContentLength(cryptor.decryptedContentLength(channel));
+			try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {
+				final Long contentLength = cryptor.decryptedContentLength(channel);
+				if (contentLength != null) {
+					outputContext.setContentLength(contentLength);
+				}
 				if (outputContext.hasStream()) {
-					cryptor.decryptedFile(channel, outputContext.getOutputStream());
+					cryptor.decryptFile(channel, outputContext.getOutputStream());
 				}
 			} catch (EOFException e) {
 				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) {
 				throw new IOException("Error decrypting file " + path.toString(), e);
-			} finally {
-				IOUtils.closeQuietly(channel);
 			}
 		}
 	}
@@ -91,12 +92,15 @@ public class EncryptedFile extends AbstractEncryptedNode {
 	protected void determineProperties() {
 		final Path path = ResourcePathUtils.getPhysicalPath(this);
 		if (Files.exists(path)) {
-			SeekableByteChannel channel = null;
-			try {
-				channel = Files.newByteChannel(path, StandardOpenOption.READ);
+			try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {
 				final Long contentLength = cryptor.decryptedContentLength(channel);
 				properties.add(new DefaultDavProperty<Long>(DavPropertyName.GETCONTENTLENGTH, contentLength));
+			} catch (IOException e) {
+				LOG.error("Error reading filesize " + path.toString(), e);
+				throw new IORuntimeException(e);
+			}
 
+			try {
 				final BasicFileAttributes attrs = Files.readAttributes(path, BasicFileAttributes.class);
 				properties.add(new DefaultDavProperty<String>(DavPropertyName.CREATIONDATE, FileTimeUtils.toRfc1123String(attrs.creationTime())));
 				properties.add(new DefaultDavProperty<String>(DavPropertyName.GETLASTMODIFIED, FileTimeUtils.toRfc1123String(attrs.lastModifiedTime())));
@@ -104,8 +108,6 @@ public class EncryptedFile extends AbstractEncryptedNode {
 			} catch (IOException e) {
 				LOG.error("Error determining metadata " + path.toString(), e);
 				throw new IORuntimeException(e);
-			} finally {
-				IOUtils.closeQuietly(channel);
 			}
 		}
 	}

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

@@ -9,7 +9,6 @@ import java.nio.file.StandardOpenOption;
 import java.util.HashSet;
 import java.util.Set;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.MutablePair;
@@ -113,9 +112,7 @@ public class EncryptedFilePart extends EncryptedFile {
 		final Path path = ResourcePathUtils.getPhysicalPath(this);
 		if (Files.exists(path)) {
 			outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis());
-			SeekableByteChannel channel = null;
-			try {
-				channel = Files.newByteChannel(path, StandardOpenOption.READ);
+			try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) {
 				final Long fileSize = cryptor.decryptedContentLength(channel);
 				final Pair<Long, Long> range = getUnionRange(fileSize);
 				final Long rangeLength = range.getRight() - range.getLeft() + 1;
@@ -130,8 +127,6 @@ public class EncryptedFilePart extends EncryptedFile {
 				}
 			} catch (DecryptFailedException e) {
 				throw new IOException("Error decrypting file " + path.toString(), e);
-			} finally {
-				IOUtils.closeQuietly(channel);
 			}
 		}
 	}

main/crypto-aes/pom.xml

@@ -12,7 +12,7 @@
 	<parent>
 		<groupId>org.cryptomator</groupId>
 		<artifactId>main</artifactId>
-		<version>0.5.2</version>
+		<version>0.6.0-SNAPSHOT</version>
 	</parent>
 	<artifactId>crypto-aes</artifactId>
 	<name>Cryptomator cryptographic module (AES)</name>

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

@@ -47,6 +47,7 @@ import org.bouncycastle.crypto.generators.SCrypt;
 import org.cryptomator.crypto.AbstractCryptor;
 import org.cryptomator.crypto.CryptorIOSupport;
 import org.cryptomator.crypto.exceptions.DecryptFailedException;
+import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException;
 import org.cryptomator.crypto.exceptions.UnsupportedKeyLengthException;
 import org.cryptomator.crypto.exceptions.WrongPasswordException;
 import org.cryptomator.crypto.io.SeekableByteChannelInputStream;
@@ -369,34 +370,6 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 		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
 	public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException {
 		// skip 128bit IV + 256 bit MAC:
@@ -422,24 +395,49 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 		}
 	}
 
+	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
-	public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException {
+	public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException {
 		// read iv:
 		encryptedFile.position(0);
 		final ByteBuffer countingIv = ByteBuffer.allocate(AES_BLOCK_LENGTH);
 		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:
 		final Long fileSize = decryptedContentLength(encryptedFile);
 
 		// 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.");
 		}
 
-		// check MAC:
-		this.authenticateContent(encryptedFile);
-
 		// go to begin of content:
 		encryptedFile.position(64);
 
@@ -448,8 +446,25 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 
 		// read content
 		final InputStream in = new SeekableByteChannelInputStream(encryptedFile);
-		final InputStream cipheredIn = new CipherInputStream(in, cipher);
-		return IOUtils.copyLarge(cipheredIn, plaintextFile, 0, fileSize);
+		final InputStream macIn = new MacInputStream(in, calculatedMac);
+		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
@@ -464,9 +479,6 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 			throw new IOException("Failed to read file header.");
 		}
 
-		// check MAC:
-		this.authenticateContent(encryptedFile);
-
 		// seek relevant position and update iv:
 		long firstRelevantBlock = pos / AES_BLOCK_LENGTH; // cut of fraction!
 		long beginOfFirstRelevantBlock = firstRelevantBlock * AES_BLOCK_LENGTH;
@@ -493,7 +505,6 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 		// 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));
 		countingIv.putLong(AES_BLOCK_LENGTH - Long.BYTES, 0l);
-		countingIv.position(0);
 		encryptedFile.write(countingIv);
 
 		// init crypto stuff:
@@ -504,9 +515,8 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 		final ByteBuffer macBuffer = ByteBuffer.allocate(mac.getMacLength());
 		encryptedFile.write(macBuffer);
 
-		// init filesize buffer and skip 16 bytes
-		final ByteBuffer encryptedFileSizeBuffer = ByteBuffer.allocate(AES_BLOCK_LENGTH);
-		encryptedFile.write(encryptedFileSizeBuffer);
+		// encrypt and write "zero length" as a placeholder, which will be read by concurrent requests, as long as encryption didn't finish:
+		encryptedContentLength(encryptedFile, 0l);
 
 		// write content:
 		final OutputStream out = new SeekableByteChannelOutputStream(encryptedFile);
@@ -529,26 +539,14 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo
 		blockSizeBufferedOut.flush();
 
 		// write MAC of total ciphertext:
-		macBuffer.position(0);
+		macBuffer.clear();
 		macBuffer.put(mac.doFinal());
-		macBuffer.position(0);
+		macBuffer.flip();
 		encryptedFile.position(16); // right behind the IV
 		encryptedFile.write(macBuffer); // 256 bit MAC
 
-		// encrypt and write plaintextSize
-		try {
-			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);
-		}
+		// encrypt and write plaintextSize:
+		encryptedContentLength(encryptedFile, plaintextSize);
 
 		return plaintextSize;
 	}

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

@@ -98,7 +98,7 @@ public class Aes256CryptorTest {
 		// decrypt modified content (should fail with DecryptFailedException):
 		final SeekableByteChannel encryptedIn = new ByteBufferBackedSeekableChannel(encryptedData);
 		final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream();
-		cryptor.decryptedFile(encryptedIn, plaintextOut);
+		cryptor.decryptFile(encryptedIn, plaintextOut);
 	}
 
 	@Test
@@ -126,7 +126,7 @@ public class Aes256CryptorTest {
 
 		// decrypt:
 		final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream();
-		final Long numDecryptedBytes = cryptor.decryptedFile(encryptedIn, plaintextOut);
+		final Long numDecryptedBytes = cryptor.decryptFile(encryptedIn, plaintextOut);
 		IOUtils.closeQuietly(encryptedIn);
 		IOUtils.closeQuietly(plaintextOut);
 		Assert.assertEquals(filesize.longValue(), numDecryptedBytes.longValue());

main/crypto-api/pom.xml

@@ -12,7 +12,7 @@
 	<parent>
 		<groupId>org.cryptomator</groupId>
 		<artifactId>main</artifactId>
-		<version>0.5.2</version>
+		<version>0.6.0-SNAPSHOT</version>
 	</parent>
 	<artifactId>crypto-api</artifactId>
 	<name>Cryptomator cryptographic module API</name>

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

@@ -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.
 	 * @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)

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

@@ -82,9 +82,9 @@ public class SamplingDecorator implements Cryptor, CryptorIOSampling {
 	}
 
 	@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);
-		return cryptor.decryptedFile(encryptedFile, countingInputStream);
+		return cryptor.decryptFile(encryptedFile, countingInputStream);
 	}
 
 	@Override

main/crypto-api/src/main/java/org/cryptomator/crypto/exceptions/MacAuthenticationFailedException.java

@@ -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);
+	}
+
+}

main/pom.xml

@@ -4,7 +4,7 @@
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>org.cryptomator</groupId>
 	<artifactId>main</artifactId>
-	<version>0.5.2</version>
+	<version>0.6.0-SNAPSHOT</version>
 	<packaging>pom</packaging>
 	<name>Cryptomator</name>
 

main/ui/pom.xml

@@ -12,7 +12,7 @@
 	<parent>
 		<groupId>org.cryptomator</groupId>
 		<artifactId>main</artifactId>
-		<version>0.5.2</version>
+		<version>0.6.0-SNAPSHOT</version>
 	</parent>
 	<artifactId>ui</artifactId>
 	<name>Cryptomator GUI</name>