From e8e80f306bfe6e99658bf7b956c3af57e39998d4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Sat, 25 Jul 2015 01:52:37 +0200 Subject: [PATCH] WebDAV range request refinements --- main/core/pom.xml | 4 +- .../jackrabbit/CryptoResourceFactory.java | 28 ++++- .../webdav/jackrabbit/WebDavServlet.java | 7 +- .../webdav/jackrabbit/RangeRequestTest.java | 110 ++++++++++++++++-- .../crypto/aes256/Aes256Cryptor.java | 10 +- .../aes256/AesCryptographicConfiguration.java | 2 +- .../crypto/SamplingCryptorDecorator.java | 8 +- 7 files changed, 146 insertions(+), 23 deletions(-) diff --git a/main/core/pom.xml b/main/core/pom.xml index 0e85e556b..9e43ba240 100644 --- a/main/core/pom.xml +++ b/main/core/pom.xml @@ -18,10 +18,8 @@ Cryptomator WebDAV and I/O module - 9.3.0.v20150612 + 9.3.1.v20150714 2.10.1 - 1.2 - 1.1 diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java index ce3accd25..229ff5ad6 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java @@ -5,6 +5,8 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.FileTime; +import java.time.format.DateTimeParseException; import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; @@ -53,14 +55,18 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants final Path filePath = getEncryptedFilePath(locator.getResourcePath()); final Path dirFilePath = getEncryptedDirectoryFilePath(locator.getResourcePath()); final String rangeHeader = request.getHeader(HttpHeader.RANGE.asString()); + final String ifRangeHeader = request.getHeader(HttpHeader.IF_RANGE.asString()); if (Files.exists(dirFilePath) || DavMethods.METHOD_MKCOL.equals(request.getMethod())) { // DIRECTORY return createDirectory(locator, request.getDavSession(), dirFilePath); - } else if (Files.exists(filePath) && DavMethods.METHOD_GET.equals(request.getMethod()) && rangeHeader != null && isRangeSatisfiable(rangeHeader)) { + } else if (Files.exists(filePath) && DavMethods.METHOD_GET.equals(request.getMethod()) && rangeHeader != null && isRangeSatisfiable(rangeHeader) && isIfRangePreconditionFulfilled(ifRangeHeader, filePath)) { // FILE RANGE final Pair requestRange = getRequestRange(rangeHeader); response.setStatus(DavServletResponse.SC_PARTIAL_CONTENT); return createFilePart(locator, request.getDavSession(), requestRange, filePath); + } else if (Files.exists(filePath) && DavMethods.METHOD_GET.equals(request.getMethod()) && rangeHeader != null && isRangeSatisfiable(rangeHeader) && !isIfRangePreconditionFulfilled(ifRangeHeader, filePath)) { + // FULL FILE (if-range not fulfilled) + return createFile(locator, request.getDavSession(), filePath); } else if (Files.exists(filePath) && DavMethods.METHOD_GET.equals(request.getMethod()) && rangeHeader != null && !isRangeSatisfiable(rangeHeader)) { // FULL FILE (unsatisfiable range) response.setStatus(DavServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); @@ -102,6 +108,26 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants return createFile(locator, session, existingFile); } + /** + * @return true if a partial response should be generated according to an If-Range precondition. + */ + private boolean isIfRangePreconditionFulfilled(String ifRangeHeader, Path filePath) throws DavException { + if (ifRangeHeader == null) { + // no header set -> fulfilled implicitly + return true; + } else { + try { + final FileTime expectedTime = FileTimeUtils.fromRfc1123String(ifRangeHeader); + final FileTime actualTime = Files.getLastModifiedTime(filePath); + return expectedTime.compareTo(actualTime) == 0; + } catch (DateTimeParseException e) { + throw new DavException(DavServletResponse.SC_BAD_REQUEST, "Unsupported If-Range header: " + ifRangeHeader); + } catch (IOException e) { + throw new DavException(DavServletResponse.SC_INTERNAL_SERVER_ERROR, e); + } + } + } + /** * @return true if and only if exactly one byte range has been requested. */ 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 8bb6686f3..d61009173 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 @@ -30,8 +30,8 @@ import org.slf4j.LoggerFactory; public class WebDavServlet extends AbstractWebdavServlet { - private static final Logger LOG = LoggerFactory.getLogger(WebDavServlet.class); private static final long serialVersionUID = 7965170007048673022L; + private static final Logger LOG = LoggerFactory.getLogger(WebDavServlet.class); public static final String CFG_FS_ROOT = "cfg.fs.root"; private DavSessionProvider davSessionProvider; private DavLocatorFactory davLocatorFactory; @@ -91,6 +91,7 @@ public class WebDavServlet extends AbstractWebdavServlet { @Override protected void doGet(WebdavRequest request, WebdavResponse response, DavResource resource) throws IOException, DavException { + long t0 = System.nanoTime(); try { super.doGet(request, response, resource); } catch (MacAuthenticationFailedException e) { @@ -98,6 +99,10 @@ public class WebDavServlet extends AbstractWebdavServlet { cryptoWarningHandler.macAuthFailed(resource.getLocator().getResourcePath()); response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE); } + if (LOG.isDebugEnabled()) { + long t1 = System.nanoTime(); + LOG.debug("REQUEST TIME: " + (t1 - t0) / 1000 / 1000.0 + " ms"); + } } } 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 index a79daf23d..9b46bf300 100644 --- a/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java +++ b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java @@ -8,9 +8,12 @@ import java.net.URL; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; +import java.util.Collections; +import java.util.List; import java.util.Random; +import java.util.concurrent.ForkJoinPool; import java.util.concurrent.ForkJoinTask; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpMethod; @@ -28,11 +31,14 @@ import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.io.Files; public class RangeRequestTest { + private static final Logger LOG = LoggerFactory.getLogger(RangeRequestTest.class); private static final Aes256Cryptor CRYPTOR = new Aes256Cryptor(); private static final WebDavServer SERVER = new WebDavServer(); private static final File TMP_VAULT = Files.createTempDir(); @@ -57,7 +63,7 @@ public class RangeRequestTest { } @Test - public void testAsyncRangeRequests() throws IOException, URISyntaxException { + public void testAsyncRangeRequests() throws IOException, URISyntaxException, InterruptedException { final URL testResourceUrl = new URL(VAULT_BASE_URI.toURL(), "asyncRangeRequestTestFile.txt"); final MultiThreadedHttpConnectionManager cm = new MultiThreadedHttpConnectionManager(); @@ -79,11 +85,87 @@ public class RangeRequestTest { Assert.assertEquals(201, putResponse); // multiple async range requests: - final Collection> tasks = new ArrayList<>(); + final List> tasks = new ArrayList<>(); final Random generator = new Random(System.currentTimeMillis()); + + final AtomicBoolean success = new AtomicBoolean(true); + + // 10 full interrupted requests: + for (int i = 0; i < 10; i++) { + final ForkJoinTask task = ForkJoinTask.adapt(() -> { + try { + final HttpMethod getMethod = new GetMethod(testResourceUrl.toString()); + final int statusCode = client.executeMethod(getMethod); + if (statusCode != 200) { + LOG.error("Invalid status code for interrupted full request"); + success.set(false); + } + getMethod.getResponseBodyAsStream().read(); + getMethod.getResponseBodyAsStream().close(); + getMethod.releaseConnection(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + tasks.add(task); + } + + // 50 crappy interrupted range requests: + for (int i = 0; i < 50; i++) { + final int lower = generator.nextInt(plaintextData.length); + final ForkJoinTask task = ForkJoinTask.adapt(() -> { + try { + final HttpMethod getMethod = new GetMethod(testResourceUrl.toString()); + getMethod.addRequestHeader("Range", "bytes=" + lower + "-"); + final int statusCode = client.executeMethod(getMethod); + if (statusCode != 206) { + LOG.error("Invalid status code for interrupted range request"); + success.set(false); + } + getMethod.getResponseBodyAsStream().read(); + getMethod.getResponseBodyAsStream().close(); + getMethod.releaseConnection(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + tasks.add(task); + } + + // 50 normal open range requests: + for (int i = 0; i < 50; i++) { + final int lower = generator.nextInt(plaintextData.length - 512); + final int upper = plaintextData.length - 1; + final ForkJoinTask task = ForkJoinTask.adapt(() -> { + try { + final HttpMethod getMethod = new GetMethod(testResourceUrl.toString()); + getMethod.addRequestHeader("Range", "bytes=" + lower + "-"); + final byte[] expected = Arrays.copyOfRange(plaintextData, lower, upper + 1); + final int statusCode = client.executeMethod(getMethod); + final byte[] responseBody = new byte[upper - lower + 10]; + final int bytesRead = IOUtils.read(getMethod.getResponseBodyAsStream(), responseBody); + getMethod.releaseConnection(); + if (statusCode != 206) { + LOG.error("Invalid status code for open range request"); + success.set(false); + } else if (upper - lower + 1 != bytesRead) { + LOG.error("Invalid response length for open range request"); + success.set(false); + } else if (!Arrays.equals(expected, Arrays.copyOfRange(responseBody, 0, bytesRead))) { + LOG.error("Invalid response body for open range request"); + success.set(false); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + tasks.add(task); + } + + // 200 normal closed range requests: for (int i = 0; i < 200; i++) { - final int pos1 = generator.nextInt(plaintextData.length); - final int pos2 = generator.nextInt(plaintextData.length); + final int pos1 = generator.nextInt(plaintextData.length - 512); + final int pos2 = pos1 + 512; final ForkJoinTask task = ForkJoinTask.adapt(() -> { try { final int lower = Math.min(pos1, pos2); @@ -94,20 +176,30 @@ public class RangeRequestTest { final byte[] responseBody = new byte[upper - lower + 1]; IOUtils.read(getMethod.getResponseBodyAsStream(), responseBody); getMethod.releaseConnection(); - Assert.assertEquals(206, statusCode); - Assert.assertArrayEquals(Arrays.copyOfRange(plaintextData, lower, upper + 1), responseBody); + if (statusCode != 206 || !Arrays.equals(Arrays.copyOfRange(plaintextData, lower, upper + 1), responseBody)) { + LOG.error("Invalid content for closed range request"); + success.set(false); + } } catch (IOException e) { throw new RuntimeException(e); } - }).fork(); + }); tasks.add(task); } + Collections.shuffle(tasks, generator); + + final ForkJoinPool pool = new ForkJoinPool(4); + for (ForkJoinTask task : tasks) { + pool.execute(task); + } for (ForkJoinTask task : tasks) { task.join(); } - + pool.shutdown(); cm.shutdown(); + + Assert.assertTrue(success.get()); } @Test 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 19e8b8f90..c1fc549da 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 @@ -379,10 +379,6 @@ public class Aes256Cryptor implements Cryptor, AesCryptographicConfiguration { final byte[] nonce = new byte[8]; headerBuf.position(16); headerBuf.get(nonce); - final ByteBuffer nonceAndCounterBuf = ByteBuffer.allocate(AES_BLOCK_LENGTH); - nonceAndCounterBuf.put(nonce); - nonceAndCounterBuf.putLong(0L); - final byte[] nonceAndCounter = nonceAndCounterBuf.array(); // read sensitive header data: final byte[] encryptedSensitiveHeaderContentBytes = new byte[48]; @@ -412,6 +408,12 @@ public class Aes256Cryptor implements Cryptor, AesCryptographicConfiguration { final Long fileSize = sensitiveHeaderContentBuf.getLong(); sensitiveHeaderContentBuf.get(fileKeyBytes); + // append counter to nonce: + final ByteBuffer nonceAndCounterBuf = ByteBuffer.allocate(AES_BLOCK_LENGTH); + nonceAndCounterBuf.put(nonce); + nonceAndCounterBuf.putLong(0L); + final byte[] nonceAndCounter = nonceAndCounterBuf.array(); + // content decryption: encryptedFile.position(104l); final SecretKey fileKey = new SecretKeySpec(fileKeyBytes, AES_KEY_ALGORITHM); 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 f2e6492e9..cbc3ad870 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 = 128 * 1024; + int CONTENT_MAC_BLOCK = 32 * 1024; /** * How to encode the encrypted file names safely. Base32 uses only alphanumeric characters and is case-insensitive. diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingCryptorDecorator.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingCryptorDecorator.java index 704e12d1c..57015c411 100644 --- a/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingCryptorDecorator.java +++ b/main/crypto-api/src/main/java/org/cryptomator/crypto/SamplingCryptorDecorator.java @@ -46,14 +46,14 @@ public class SamplingCryptorDecorator extends AbstractCryptorDecorator implement @Override public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile, boolean authenticate) throws IOException, DecryptFailedException { - final OutputStream countingInputStream = new CountingOutputStream(decryptedBytes, plaintextFile); - return cryptor.decryptFile(encryptedFile, countingInputStream, authenticate); + final OutputStream countingOutputStream = new CountingOutputStream(decryptedBytes, plaintextFile); + return cryptor.decryptFile(encryptedFile, countingOutputStream, authenticate); } @Override public Long decryptRange(SeekableByteChannel encryptedFile, OutputStream plaintextFile, long pos, long length, boolean authenticate) throws IOException, DecryptFailedException { - final OutputStream countingInputStream = new CountingOutputStream(decryptedBytes, plaintextFile); - return cryptor.decryptRange(encryptedFile, countingInputStream, pos, length, authenticate); + final OutputStream countingOutputStream = new CountingOutputStream(decryptedBytes, plaintextFile); + return cryptor.decryptRange(encryptedFile, countingOutputStream, pos, length, authenticate); } @Override