From d76154c8d1e70db99a14bdbf0fd397d2e8b8d69c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Jul 2015 19:30:49 +0200 Subject: [PATCH] - reduced size of chunks, a MAC is calculated for (not final yet) - faster range requests due to reduced chunk size, thus faster video playback start - fixed range requests - making file locks optional (if not supported by file system) --- main/core/pom.xml | 13 ++- .../webdav/jackrabbit/EncryptedDir.java | 7 +- .../webdav/jackrabbit/EncryptedFile.java | 10 +- .../webdav/jackrabbit/EncryptedFilePart.java | 16 +-- .../webdav/jackrabbit/FilenameTranslator.java | 6 +- .../jackrabbit/SilentlyFailingFileLock.java | 56 +++++++++ .../webdav/jackrabbit/RangeRequestTest.java | 109 ++++++++++++++++++ main/core/src/test/resources/log4j2.xml | 33 ++++++ .../aes256/AesCryptographicConfiguration.java | 2 +- .../crypto/aes256/Aes256CryptorTest.java | 6 +- 10 files changed, 233 insertions(+), 25 deletions(-) create mode 100644 main/core/src/main/java/org/cryptomator/webdav/jackrabbit/SilentlyFailingFileLock.java create mode 100644 main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java create mode 100644 main/core/src/test/resources/log4j2.xml diff --git a/main/core/pom.xml b/main/core/pom.xml index 7bf3dd84c..72ecc5ea2 100644 --- a/main/core/pom.xml +++ b/main/core/pom.xml @@ -18,7 +18,7 @@ Cryptomator WebDAV and I/O module - 9.2.10.v20150310 + 9.3.0.v20150612 2.10.1 1.2 1.1 @@ -29,6 +29,11 @@ org.cryptomator crypto-api + + org.cryptomator + crypto-aes + test + @@ -48,13 +53,13 @@ jackrabbit-webdav ${jackrabbit.version} - + com.google.guava guava - + commons-io @@ -64,7 +69,7 @@ org.apache.commons commons-lang3 - + com.fasterxml.jackson.core diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java index b06d1c042..b18129fb5 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java @@ -12,7 +12,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.DirectoryStream; @@ -156,7 +155,8 @@ class EncryptedDir extends AbstractEncryptedNode implements FileConstants { final String cleartextFilename = FilenameUtils.getName(childLocator.getResourcePath()); final String ciphertextFilename = filenameTranslator.getEncryptedFilename(cleartextFilename); final Path filePath = dirPath.resolve(ciphertextFilename); - try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); final FileLock lock = c.lock(0L, FILE_HEADER_LENGTH, false)) { + try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); + final SilentlyFailingFileLock lock = new SilentlyFailingFileLock(c, 0L, FILE_HEADER_LENGTH, false)) { cryptor.encryptFile(inputContext.getInputStream(), c); } catch (SecurityException e) { throw new DavException(DavServletResponse.SC_FORBIDDEN, e); @@ -289,7 +289,8 @@ class EncryptedDir extends AbstractEncryptedNode implements FileConstants { throw new DavException(DavServletResponse.SC_NOT_FOUND); } final String dstDirId = UUID.randomUUID().toString(); - try (final FileChannel c = FileChannel.open(dstDirFilePath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); final FileLock lock = c.lock()) { + try (final FileChannel c = FileChannel.open(dstDirFilePath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); + SilentlyFailingFileLock lock = new SilentlyFailingFileLock(c, false)) { c.write(ByteBuffer.wrap(dstDirId.getBytes(StandardCharsets.UTF_8))); } 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 daf8b73ab..d0057c598 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 @@ -11,7 +11,6 @@ package org.cryptomator.webdav.jackrabbit; import java.io.EOFException; import java.io.IOException; import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; import java.nio.channels.OverlappingFileLockException; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; @@ -43,6 +42,7 @@ class EncryptedFile extends AbstractEncryptedNode implements FileConstants { private static final Logger LOG = LoggerFactory.getLogger(EncryptedFile.class); protected final CryptoWarningHandler cryptoWarningHandler; + protected final Long contentLength; public EncryptedFile(CryptoResourceFactory factory, DavResourceLocator locator, DavSession session, LockManager lockManager, Cryptor cryptor, CryptoWarningHandler cryptoWarningHandler, Path filePath) { super(factory, locator, session, lockManager, cryptor, filePath); @@ -50,9 +50,10 @@ class EncryptedFile extends AbstractEncryptedNode implements FileConstants { throw new IllegalArgumentException("filePath must not be null"); } this.cryptoWarningHandler = cryptoWarningHandler; + Long contentLength = null; if (Files.isRegularFile(filePath)) { - try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.tryLock(0L, FILE_HEADER_LENGTH, true)) { - final Long contentLength = cryptor.decryptedContentLength(c); + try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.READ, StandardOpenOption.DSYNC); SilentlyFailingFileLock lock = new SilentlyFailingFileLock(c, true)) { + contentLength = cryptor.decryptedContentLength(c); properties.add(new DefaultDavProperty(DavPropertyName.GETCONTENTLENGTH, contentLength)); if (contentLength > RANGE_REQUEST_LOWER_LIMIT) { properties.add(new HttpHeaderProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString())); @@ -68,6 +69,7 @@ class EncryptedFile extends AbstractEncryptedNode implements FileConstants { // don't add content length DAV property } } + this.contentLength = contentLength; } @Override @@ -95,7 +97,7 @@ class EncryptedFile extends AbstractEncryptedNode implements FileConstants { if (Files.isRegularFile(filePath)) { outputContext.setModificationTime(Files.getLastModifiedTime(filePath).toMillis()); outputContext.setProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString()); - try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.READ); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) { + try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.READ); SilentlyFailingFileLock lock = new SilentlyFailingFileLock(c, true)) { final Long contentLength = cryptor.decryptedContentLength(c); if (contentLength != null) { outputContext.setContentLength(contentLength); 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 fc0490bd3..fe3963b7b 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 @@ -2,7 +2,7 @@ package org.cryptomator.webdav.jackrabbit; import java.io.EOFException; import java.io.IOException; -import java.nio.channels.SeekableByteChannel; +import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -111,13 +111,15 @@ class EncryptedFilePart extends EncryptedFile { @Override public void spool(OutputContext outputContext) throws IOException { assert Files.isRegularFile(filePath); + assert this.contentLength != null; + + final Pair range = getUnionRange(this.contentLength); + final Long rangeLength = Math.min(this.contentLength, range.getRight()) - range.getLeft() + 1; + outputContext.setContentLength(rangeLength); + outputContext.setProperty(HttpHeader.CONTENT_RANGE.asString(), getContentRangeHeader(range.getLeft(), range.getRight(), this.contentLength)); outputContext.setModificationTime(Files.getLastModifiedTime(filePath).toMillis()); - try (final SeekableByteChannel c = Files.newByteChannel(filePath, StandardOpenOption.READ)) { - final Long fileSize = cryptor.decryptedContentLength(c); - final Pair range = getUnionRange(fileSize); - final Long rangeLength = range.getRight() - range.getLeft() + 1; - outputContext.setContentLength(rangeLength); - outputContext.setProperty(HttpHeader.CONTENT_RANGE.asString(), getContentRangeHeader(range.getLeft(), range.getRight(), fileSize)); + + try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.READ)) { if (outputContext.hasStream()) { cryptor.decryptRange(c, outputContext.getOutputStream(), range.getLeft(), rangeLength); } diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java index 33cbeed01..b05fbf598 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java @@ -5,7 +5,6 @@ import java.io.IOException; import java.io.Serializable; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; import java.nio.file.FileSystems; import java.nio.file.Files; @@ -130,13 +129,14 @@ class FilenameTranslator implements FileConstants { /* Locked I/O */ private void writeAllBytesAtomically(Path path, byte[] bytes) throws IOException { - try (final FileChannel c = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); final FileLock lock = c.lock()) { + try (final FileChannel c = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); + final SilentlyFailingFileLock lock = new SilentlyFailingFileLock(c, false)) { c.write(ByteBuffer.wrap(bytes)); } } private byte[] readAllBytesAtomically(Path path) throws IOException { - try (final FileChannel c = FileChannel.open(path, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) { + try (final FileChannel c = FileChannel.open(path, StandardOpenOption.READ, StandardOpenOption.DSYNC); final SilentlyFailingFileLock lock = new SilentlyFailingFileLock(c, true)) { final ByteBuffer buffer = ByteBuffer.allocate((int) c.size()); c.read(buffer); return buffer.array(); diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/SilentlyFailingFileLock.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/SilentlyFailingFileLock.java new file mode 100644 index 000000000..d136159f2 --- /dev/null +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/SilentlyFailingFileLock.java @@ -0,0 +1,56 @@ +package org.cryptomator.webdav.jackrabbit; + +import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.NonReadableChannelException; +import java.nio.channels.NonWritableChannelException; +import java.nio.channels.OverlappingFileLockException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Instances of this class wrap a file lock, that is created upon construction and destroyed by {@link #close()}. + * + * If the construction fails (e.g. if the file system does not support locks) no exception will be thrown and no lock is created. + */ +class SilentlyFailingFileLock implements AutoCloseable { + + private static final Logger LOG = LoggerFactory.getLogger(SilentlyFailingFileLock.class); + + private final FileLock lock; + + /** + * Invokes #SilentlyFailingFileLock(FileChannel, long, long, boolean) with a position of 0 and a size of {@link Long#MAX_VALUE}. + */ + SilentlyFailingFileLock(FileChannel channel, boolean shared) { + this(channel, 0L, Long.MAX_VALUE, shared); + } + + /** + * @throws NonReadableChannelException If shared is true this channel was not opened for reading + * @throws NonWritableChannelException If shared is false but this channel was not opened for writing + * @see FileChannel#lock(long, long, boolean) + */ + SilentlyFailingFileLock(FileChannel channel, long position, long size, boolean shared) { + FileLock lock = null; + try { + lock = channel.tryLock(position, size, shared); + } catch (IOException | OverlappingFileLockException e) { + if (LOG.isDebugEnabled()) { + LOG.warn("Unable to lock file."); + } + } finally { + this.lock = lock; + } + } + + @Override + public void close() throws IOException { + if (lock != null) { + lock.close(); + } + } + +} diff --git a/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java new file mode 100644 index 000000000..40bc05d2a --- /dev/null +++ b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java @@ -0,0 +1,109 @@ +package org.cryptomator.webdav.jackrabbit; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Random; +import java.util.concurrent.ForkJoinTask; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; +import org.cryptomator.crypto.aes256.Aes256Cryptor; +import org.cryptomator.webdav.WebDavServer; +import org.cryptomator.webdav.WebDavServer.ServletLifeCycleAdapter; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.google.common.io.Files; + +public class RangeRequestTest { + + private static final Aes256Cryptor CRYPTOR = new Aes256Cryptor(); + private static final WebDavServer SERVER = new WebDavServer(); + + @BeforeClass + public static void startServer() { + SERVER.start(); + } + + @AfterClass + public static void stopServer() { + SERVER.stop(); + } + + @Test + public void testAsyncRangeRequests() throws IOException, URISyntaxException { + final File tmpVault = Files.createTempDir(); + final ServletLifeCycleAdapter servlet = SERVER.createServlet(tmpVault.toPath(), CRYPTOR, new ArrayList(), new ArrayList(), "JUnitTestVault"); + final boolean started = servlet.start(); + final URI vaultBaseUri = new URI("http", servlet.getServletUri().getSchemeSpecificPart() + "/", null); + final URL testResourceUrl = new URL(vaultBaseUri.toURL(), "testfile.txt"); + + Assert.assertTrue(started); + Assert.assertNotNull(vaultBaseUri); + + // prepare 8MiB test data: + final byte[] plaintextData = new byte[2097152 * Integer.BYTES]; + final ByteBuffer bbIn = ByteBuffer.wrap(plaintextData); + for (int i = 0; i < 2097152; i++) { + bbIn.putInt(i); + } + final InputStream plaintextIn = new ByteArrayInputStream(plaintextData); + + // put request: + final HttpURLConnection putConn = (HttpURLConnection) testResourceUrl.openConnection(); + putConn.setDoOutput(true); + putConn.setRequestMethod("PUT"); + IOUtils.copy(plaintextIn, putConn.getOutputStream()); + putConn.getOutputStream().close(); + final int putResponse = putConn.getResponseCode(); + putConn.disconnect(); + Assert.assertEquals(201, putResponse); + + // multiple async range requests: + final Collection> tasks = new ArrayList<>(); + final Random generator = new Random(System.currentTimeMillis()); + for (int i = 0; i < 100; i++) { + final int pos1 = generator.nextInt(plaintextData.length); + final int pos2 = generator.nextInt(plaintextData.length); + final ForkJoinTask task = ForkJoinTask.adapt(() -> { + try { + final HttpURLConnection conn = (HttpURLConnection) testResourceUrl.openConnection(); + final int lower = Math.min(pos1, pos2); + final int upper = Math.max(pos1, pos2); + conn.setRequestMethod("GET"); + conn.addRequestProperty("Range", "bytes=" + lower + "-" + upper); + final int rangeResponse = conn.getResponseCode(); + final byte[] buffer = new byte[upper - lower + 1]; + final int bytesReceived = IOUtils.read(conn.getInputStream(), buffer); + Assert.assertEquals(206, rangeResponse); + Assert.assertEquals(buffer.length, bytesReceived); + Assert.assertArrayEquals(Arrays.copyOfRange(plaintextData, lower, upper + 1), buffer); + } catch (IOException e) { + throw new RuntimeException(e); + } + }).fork(); + tasks.add(task); + } + + for (ForkJoinTask task : tasks) { + task.join(); + } + + servlet.stop(); + + FileUtils.deleteQuietly(tmpVault); + } + +} diff --git a/main/core/src/test/resources/log4j2.xml b/main/core/src/test/resources/log4j2.xml new file mode 100644 index 000000000..39c2f8545 --- /dev/null +++ b/main/core/src/test/resources/log4j2.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java index 6cf7408fc..7916669b1 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java @@ -84,7 +84,7 @@ interface AesCryptographicConfiguration { /** * Number of bytes, a content block over which a MAC is calculated consists of. */ - int CONTENT_MAC_BLOCK = 5 * 1024 * 1024; + int CONTENT_MAC_BLOCK = 128 * 1024; /** * How to encode the encrypted file names safely. Base32 uses only alphanumeric characters and is case-insensitive. 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 017d33312..fc87f5694 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 @@ -139,10 +139,10 @@ public class Aes256CryptorTest { @Test public void testPartialDecryption() throws IOException, DecryptFailedException, WrongPasswordException, UnsupportedKeyLengthException, EncryptFailedException { - // our test plaintext data: - final byte[] plaintextData = new byte[524288 * Integer.BYTES]; + // 8MiB test plaintext data: + final byte[] plaintextData = new byte[2097152 * Integer.BYTES]; final ByteBuffer bbIn = ByteBuffer.wrap(plaintextData); - for (int i = 0; i < 524288; i++) { + for (int i = 0; i < 2097152; i++) { bbIn.putInt(i); } final InputStream plaintextIn = new ByteArrayInputStream(plaintextData);