diff --git a/main/core/pom.xml b/main/core/pom.xml index 80c8af541..19fc63c77 100644 --- a/main/core/pom.xml +++ b/main/core/pom.xml @@ -48,7 +48,13 @@ jackrabbit-webdav ${jackrabbit.version} - + + + + com.google.guava + guava + + commons-io diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavResourceFactoryImpl.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavResourceFactoryImpl.java index 330f488b8..fac7e9072 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavResourceFactoryImpl.java +++ b/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) { diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFile.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFile.java index 5b5b3aa4b..09905a0b6 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFile.java +++ b/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)) { diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java index 98b67c3e2..b0e9f3faf 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java +++ b/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 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> requestedContentRanges = new HashSet>(); - 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; + } + } + } + } diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavServlet.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavServlet.java index afb5c54f9..c4c549c9b 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavServlet.java +++ b/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 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 diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java index a4ed5b505..df3797876 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java +++ b/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: diff --git a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java index 4b29d9aed..38f881c15 100644 --- a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java +++ b/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(); diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java index b9e05f358..d997af8f3 100644 --- a/main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java +++ b/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 diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingDecorator.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingDecorator.java index 7c3e9245f..cec189598 100644 --- a/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingDecorator.java +++ b/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); diff --git a/main/pom.xml b/main/pom.xml index 6342f352b..699883e6e 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -1,5 +1,12 @@ - + 4.0.0 org.cryptomator @@ -103,7 +110,14 @@ commons-codec ${commons-codec.version} - + + + + com.google.guava + guava + 18.0 + + com.google.inject diff --git a/main/ui/src/main/resources/fxml/mac_warnings.fxml b/main/ui/src/main/resources/fxml/mac_warnings.fxml index 4d5958993..8bff86f1a 100644 --- a/main/ui/src/main/resources/fxml/mac_warnings.fxml +++ b/main/ui/src/main/resources/fxml/mac_warnings.fxml @@ -14,12 +14,12 @@ - + -