new, more secure encryption scheme

- fixed flaw reported by Stan Drapkin (SecurityDriven.NET) reducing effective key size to 96 bit
- multiple file content MACs for 1MB blocks, preventing chosen ciphertext attacks, as authentication now happens before decryption
- allowing files bigger than 64GiB
This commit is contained in:
Sebastian Stenzel
2015-06-21 18:51:39 +02:00
parent d7186bb2dd
commit 45cf87d089
12 changed files with 238 additions and 300 deletions
@@ -5,7 +5,6 @@ import java.nio.file.FileAlreadyExistsException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.ExecutorService;
import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.io.FilenameUtils;
@@ -28,15 +27,13 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants
private final LockManager lockManager = new SimpleLockManager();
private final Cryptor cryptor;
private final CryptoWarningHandler cryptoWarningHandler;
private final ExecutorService backgroundTaskExecutor;
private final Path dataRoot;
private final FilenameTranslator filenameTranslator;
CryptoResourceFactory(Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler, ExecutorService backgroundTaskExecutor, String vaultRoot) {
CryptoResourceFactory(Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler, String vaultRoot) {
Path vaultRootPath = FileSystems.getDefault().getPath(vaultRoot);
this.cryptor = cryptor;
this.cryptoWarningHandler = cryptoWarningHandler;
this.backgroundTaskExecutor = backgroundTaskExecutor;
this.dataRoot = vaultRootPath.resolve("d");
this.filenameTranslator = new FilenameTranslator(cryptor, vaultRootPath);
}
@@ -151,7 +148,7 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants
}
private EncryptedFile createFilePart(DavResourceLocator locator, DavSession session, DavServletRequest request, Path filePath) {
return new EncryptedFilePart(this, locator, session, request, lockManager, cryptor, cryptoWarningHandler, backgroundTaskExecutor, filePath);
return new EncryptedFilePart(this, locator, session, request, lockManager, cryptor, cryptoWarningHandler, filePath);
}
private EncryptedFile createFile(DavResourceLocator locator, DavSession session, Path filePath) {
@@ -107,7 +107,9 @@ class EncryptedFile extends AbstractEncryptedNode implements FileConstants {
} catch (EOFException e) {
LOG.warn("Unexpected end of stream (possibly client hung up).");
} catch (MacAuthenticationFailedException e) {
cryptoWarningHandler.macAuthFailed(getLocator().getResourcePath());
LOG.warn("File integrity violation for " + getLocator().getResourcePath());
throw new IOException("Error decrypting file " + filePath.toString(), e);
// cryptoWarningHandler.macAuthFailed(getLocator().getResourcePath());
} catch (DecryptFailedException e) {
throw new IOException("Error decrypting file " + filePath.toString(), e);
}
@@ -2,15 +2,12 @@ package org.cryptomator.webdav.jackrabbit;
import java.io.EOFException;
import java.io.IOException;
import java.nio.channels.ClosedByInterruptException;
import java.nio.channels.SeekableByteChannel;
import java.nio.file.Files;
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;
@@ -27,9 +24,6 @@ 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.
*
@@ -41,7 +35,6 @@ 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)
@@ -56,22 +49,13 @@ class EncryptedFilePart extends EncryptedFile {
private final Set<Pair<Long, Long>> requestedContentRanges = new HashSet<Pair<Long, Long>>();
public EncryptedFilePart(CryptoResourceFactory factory, DavResourceLocator locator, DavSession session, DavServletRequest request, LockManager lockManager, Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler,
ExecutorService backgroundTaskExecutor, Path filePath) {
Path filePath) {
super(factory, locator, session, lockManager, cryptor, cryptoWarningHandler, filePath);
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) {
@@ -149,46 +133,4 @@ 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() {
assert Files.isRegularFile(filePath);
try (final SeekableByteChannel channel = Files.newByteChannel(filePath, StandardOpenOption.READ)) {
final boolean authentic = cryptor.isAuthentic(channel);
if (!authentic) {
cryptoWarningHandler.macAuthFailed(locator.getResourcePath());
}
} catch (ClosedByInterruptException ex) {
LOG.debug("Couldn't finish MAC verification due to interruption of worker thread.");
} catch (IOException e) {
LOG.error("IOException during MAC verification of " + filePath.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;
}
}
}
}
@@ -9,9 +9,6 @@
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;
@@ -33,7 +30,6 @@ 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();
@@ -45,26 +41,9 @@ public class WebDavServlet extends AbstractWebdavServlet {
public void init(ServletConfig config) throws ServletException {
super.init(config);
final String fsRoot = config.getInitParameter(CFG_FS_ROOT);
backgroundTaskExecutor = Executors.newCachedThreadPool();
davSessionProvider = new DavSessionProviderImpl();
davLocatorFactory = new CleartextLocatorFactory(config.getServletContext().getContextPath());
davResourceFactory = new CryptoResourceFactory(cryptor, cryptoWarningHandler, backgroundTaskExecutor, fsRoot);
}
@Override
public void destroy() {
backgroundTaskExecutor.shutdown();
try {
final boolean tasksFinished = backgroundTaskExecutor.awaitTermination(2, TimeUnit.SECONDS);
if (!tasksFinished) {
backgroundTaskExecutor.shutdownNow();
}
} catch (InterruptedException e) {
backgroundTaskExecutor.shutdownNow();
Thread.currentThread().interrupt();
} finally {
super.destroy();
}
davResourceFactory = new CryptoResourceFactory(cryptor, cryptoWarningHandler, fsRoot);
}
@Override