From 884b894e0451c3586b2be5eba1d6c5d04dc34696 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 8 Dec 2014 22:25:45 +0100 Subject: [PATCH] bugfix: correct decryption of looooooong filenames (>255 chars) --- .../jackrabbit/WebDavLocatorFactory.java | 3 + .../crypto/aes256/Aes256Cryptor.java | 58 ++++++++++++++++--- .../crypto/aes256/FileNamingConventions.java | 19 +++--- .../crypto/aes256/LongFilenameMetadata.java | 31 ++++++++-- .../Default suite/Default test.html | 2 +- .../Default suite/Default test.xml | 2 +- main/crypto-aes/test-output/index.html | 8 +-- .../old/Default suite/testng.xml.html | 2 +- .../crypto-aes/test-output/testng-results.xml | 4 +- .../cryptomator/crypto/CryptorIOSupport.java | 2 +- 10 files changed, 99 insertions(+), 32 deletions(-) diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavLocatorFactory.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavLocatorFactory.java index 34d179898..576ce16f4 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavLocatorFactory.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/WebDavLocatorFactory.java @@ -118,6 +118,9 @@ public class WebDavLocatorFactory extends AbstractLocatorFactory implements Sens @Override public byte[] readPathSpecificMetadata(String encryptedPath) throws IOException { final Path metaDataFile = fsRoot.resolve(encryptedPath); + if (!Files.isReadable(metaDataFile)) { + return null; + } final long metaDataFileSize = Files.size(metaDataFile); final SeekableByteChannel channel = Files.newByteChannel(metaDataFile, StandardOpenOption.READ); try { 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 5a288fc23..405c1d122 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 @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.UUID; +import java.util.zip.CRC32; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; @@ -51,6 +52,7 @@ import org.cryptomator.crypto.exceptions.WrongPasswordException; import org.cryptomator.crypto.io.SeekableByteChannelInputStream; import org.cryptomator.crypto.io.SeekableByteChannelOutputStream; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicConfiguration, FileNamingConventions { @@ -256,6 +258,12 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo } } + private long crc32Sum(byte[] source) { + final CRC32 crc32 = new CRC32(); + crc32.update(source); + return crc32.getValue(); + } + @Override public String encryptPath(String cleartextPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) { try { @@ -272,17 +280,33 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo } } + /** + * Each path component, i.e. file or directory name separated by path separators, gets encrypted for its own.
+ * Encryption will blow up the filename length due to aes block sizes and base32 encoding. The result may be too long for some old file + * systems.
+ * This means that we need a workaround for filenames longer than the limit defined in + * {@link FileNamingConventions#ENCRYPTED_FILENAME_LENGTH_LIMIT}.
+ *
+ * In any case we will create the encrypted filename normally. For those, that are too long, we calculate a checksum. No + * cryptographically secure hash is needed here. We just want an uniform distribution for better load balancing. All encrypted filenames + * with the same checksum will then share a metadata file, in which a lookup map between encrypted filenames and short unique + * alternative names are stored.
+ *
+ * These alternative names consist of the checksum, a unique id and a special file extension defined in + * {@link FileNamingConventions#LONG_NAME_FILE_EXT}. + */ private String encryptPathComponent(final String cleartext, final SecretKey key, CryptorIOSupport ioSupport) throws IllegalBlockSizeException, BadPaddingException, IOException { final Cipher cipher = this.cipher(FILE_NAME_CIPHER, key, EMPTY_IV, Cipher.ENCRYPT_MODE); final byte[] cleartextBytes = cleartext.getBytes(Charsets.UTF_8); final byte[] encryptedBytes = cipher.doFinal(cleartextBytes); final String encrypted = ENCRYPTED_FILENAME_CODEC.encodeAsString(encryptedBytes) + BASIC_FILE_EXT; - if (cleartext.length() > PLAINTEXT_FILENAME_LENGTH_LIMIT) { - final LongFilenameMetadata metadata = new LongFilenameMetadata(); - metadata.setEncryptedFilename(encrypted); - final String alternativeFileName = UUID.randomUUID().toString() + LONG_NAME_FILE_EXT; - ioSupport.writePathSpecificMetadata(alternativeFileName + METADATA_FILE_EXT, objectMapper.writeValueAsBytes(metadata)); + if (encrypted.length() > ENCRYPTED_FILENAME_LENGTH_LIMIT) { + final String crc32 = String.valueOf(crc32Sum(encrypted.getBytes())); + final String metadataFilename = crc32 + METADATA_FILE_EXT; + final LongFilenameMetadata metadata = this.getMetadata(ioSupport, metadataFilename); + final String alternativeFileName = crc32 + LONG_NAME_PREFIX_SEPARATOR + metadata.getOrCreateUuidForEncryptedFilename(encrypted).toString() + LONG_NAME_FILE_EXT; + this.storeMetadata(ioSupport, metadataFilename, metadata); return alternativeFileName; } else { return encrypted; @@ -305,11 +329,18 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo } } + /** + * @see #encryptPathComponent(String, SecretKey, CryptorIOSupport) + */ private String decryptPathComponent(final String encrypted, final SecretKey key, CryptorIOSupport ioSupport) throws IllegalBlockSizeException, BadPaddingException, IOException { final String ciphertext; if (encrypted.endsWith(LONG_NAME_FILE_EXT)) { - final LongFilenameMetadata metadata = objectMapper.readValue(ioSupport.readPathSpecificMetadata(encrypted + METADATA_FILE_EXT), LongFilenameMetadata.class); - ciphertext = metadata.getEncryptedFilename(); + final String basename = StringUtils.removeEnd(encrypted, LONG_NAME_FILE_EXT); + final String crc32 = StringUtils.substringBefore(basename, LONG_NAME_PREFIX_SEPARATOR); + final String uuid = StringUtils.substringAfter(basename, LONG_NAME_PREFIX_SEPARATOR); + final String metadataFilename = crc32 + METADATA_FILE_EXT; + final LongFilenameMetadata metadata = this.getMetadata(ioSupport, metadataFilename); + ciphertext = metadata.getEncryptedFilenameForUUID(UUID.fromString(uuid)); } else if (encrypted.endsWith(BASIC_FILE_EXT)) { ciphertext = StringUtils.removeEndIgnoreCase(encrypted, BASIC_FILE_EXT); } else { @@ -322,6 +353,19 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo return new String(cleartextBytes, Charsets.UTF_8); } + private LongFilenameMetadata getMetadata(CryptorIOSupport ioSupport, String metadataFile) throws IOException { + final byte[] fileContent = ioSupport.readPathSpecificMetadata(metadataFile); + if (fileContent == null) { + return new LongFilenameMetadata(); + } else { + return objectMapper.readValue(fileContent, LongFilenameMetadata.class); + } + } + + private void storeMetadata(CryptorIOSupport ioSupport, String metadataFile, LongFilenameMetadata metadata) throws JsonProcessingException, IOException { + ioSupport.writePathSpecificMetadata(metadataFile, objectMapper.writeValueAsBytes(metadata)); + } + @Override public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException { final ByteBuffer sizeBuffer = ByteBuffer.allocate(SIZE_OF_LONG); diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/FileNamingConventions.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/FileNamingConventions.java index fb7ecd891..fd2866b06 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/FileNamingConventions.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/FileNamingConventions.java @@ -28,25 +28,28 @@ interface FileNamingConventions { /** * Maximum length possible on file systems with a filename limit of 255 chars.
- * 144 and 160 are multiples of 16 (128bit aes block size).
- * 144 * 8/5 (base32) = 230,..
- * 160 * 8/5 = 256
- * Base 64 isn't supported on case-insensitive file systems.
+ * Also we would need a few chars for our file extension, so lets use {@value #ENCRYPTED_FILENAME_LENGTH_LIMIT}. */ - int PLAINTEXT_FILENAME_LENGTH_LIMIT = 144; + int ENCRYPTED_FILENAME_LENGTH_LIMIT = 250; /** - * For plaintext file names <= {@value #PLAINTEXT_FILENAME_LENGTH_LIMIT} chars. + * For plaintext file names <= {@value #ENCRYPTED_FILENAME_LENGTH_LIMIT} chars. */ String BASIC_FILE_EXT = ".aes"; /** - * For plaintext file names > {@value #PLAINTEXT_FILENAME_LENGTH_LIMIT} chars. + * For plaintext file names > {@value #ENCRYPTED_FILENAME_LENGTH_LIMIT} chars. */ String LONG_NAME_FILE_EXT = ".lng.aes"; /** - * For file-related metadata. + * Prefix in file names > {@value #ENCRYPTED_FILENAME_LENGTH_LIMIT} chars used to determine the corresponding metadata file. + */ + String LONG_NAME_PREFIX_SEPARATOR = "_"; + + /** + * For metadata files for a certain group of files. The cryptor may decide what files to assign to the same group; hopefully using some + * kind of uniform distribution for better load balancing. */ String METADATA_FILE_EXT = ".meta"; diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/LongFilenameMetadata.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/LongFilenameMetadata.java index 0782440f0..db1e5cbbd 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/LongFilenameMetadata.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/LongFilenameMetadata.java @@ -9,20 +9,41 @@ package org.cryptomator.crypto.aes256; import java.io.Serializable; +import java.util.UUID; + +import org.apache.commons.collections4.BidiMap; +import org.apache.commons.collections4.bidimap.DualHashBidiMap; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; class LongFilenameMetadata implements Serializable { private static final long serialVersionUID = 6214509403824421320L; - private String encryptedFilename; + + @JsonDeserialize(as = DualHashBidiMap.class) + private BidiMap encryptedFilenames = new DualHashBidiMap<>(); /* Getter/Setter */ - public String getEncryptedFilename() { - return encryptedFilename; + public synchronized String getEncryptedFilenameForUUID(final UUID uuid) { + return encryptedFilenames.get(uuid); } - public void setEncryptedFilename(String encryptedFilename) { - this.encryptedFilename = encryptedFilename; + public synchronized UUID getOrCreateUuidForEncryptedFilename(String encryptedFilename) { + UUID uuid = encryptedFilenames.getKey(encryptedFilename); + if (uuid == null) { + uuid = UUID.randomUUID(); + encryptedFilenames.put(uuid, encryptedFilename); + } + return uuid; + } + + public BidiMap getEncryptedFilenames() { + return encryptedFilenames; + } + + public void setEncryptedFilenames(BidiMap encryptedFilenames) { + this.encryptedFilenames = encryptedFilenames; } } diff --git a/main/crypto-aes/test-output/Default suite/Default test.html b/main/crypto-aes/test-output/Default suite/Default test.html index 2b37cf450..d49bcf50a 100644 --- a/main/crypto-aes/test-output/Default suite/Default test.html +++ b/main/crypto-aes/test-output/Default suite/Default test.html @@ -57,7 +57,7 @@ function toggleAllBoxes() { Tests passed/Failed/Skipped:0/0/0 -Started on:Sat Dec 06 14:26:26 CET 2014 +Started on:Mon Dec 08 22:07:11 CET 2014 Total time:0 seconds (5 ms) diff --git a/main/crypto-aes/test-output/Default suite/Default test.xml b/main/crypto-aes/test-output/Default suite/Default test.xml index 562c127c0..ee6e23803 100644 --- a/main/crypto-aes/test-output/Default suite/Default test.xml +++ b/main/crypto-aes/test-output/Default suite/Default test.xml @@ -1,4 +1,4 @@ - + diff --git a/main/crypto-aes/test-output/index.html b/main/crypto-aes/test-output/index.html index 218a5a946..8431eb840 100644 --- a/main/crypto-aes/test-output/index.html +++ b/main/crypto-aes/test-output/index.html @@ -105,7 +105,7 @@
- /private/var/folders/t_/sydpw2q97yj_fh3p7jp6jx8w0000gn/T/testng-eclipse-1690973351/testng-customsuite.xml + /private/var/folders/t_/sydpw2q97yj_fh3p7jp6jx8w0000gn/T/testng-eclipse--34592626/testng-customsuite.xml 
@@ -114,11 +114,7 @@
 <suite name="Default suite">
   <test verbose="2" name="Default test">
     <classes>
-      <class name="org.cryptomator.crypto.aes256.Aes256CryptorTest">
-        <methods>
-          <include name="testEncryptionOfLongFilenames"/>
-        </methods>
-      </class> <!-- org.cryptomator.crypto.aes256.Aes256CryptorTest -->
+      <class name="org.cryptomator.crypto.aes256.Aes256CryptorTest"/>
     </classes>
   </test> <!-- Default test -->
 </suite> <!-- Default suite -->
diff --git a/main/crypto-aes/test-output/old/Default suite/testng.xml.html b/main/crypto-aes/test-output/old/Default suite/testng.xml.html
index fe261326f..e73c2fa8f 100644
--- a/main/crypto-aes/test-output/old/Default suite/testng.xml.html	
+++ b/main/crypto-aes/test-output/old/Default suite/testng.xml.html	
@@ -1 +1 @@
-testng.xml for Default suite<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="Default suite">
  <test verbose="2" name="Default test">
    <classes>
      <class name="org.cryptomator.crypto.aes256.Aes256CryptorTest">
        <methods>
          <include name="testEncryptionOfLongFilenames"/>
        </methods>
      </class> <!-- org.cryptomator.crypto.aes256.Aes256CryptorTest -->
    </classes>
  </test> <!-- Default test -->
</suite> <!-- Default suite -->

\ No newline at end of file
+testng.xml for Default suite<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="Default suite">
  <test verbose="2" name="Default test">
    <classes>
      <class name="org.cryptomator.crypto.aes256.Aes256CryptorTest"/>
    </classes>
  </test> <!-- Default test -->
</suite> <!-- Default suite -->

\ No newline at end of file
diff --git a/main/crypto-aes/test-output/testng-results.xml b/main/crypto-aes/test-output/testng-results.xml
index 1e3fdd7dc..a2e1015e9 100644
--- a/main/crypto-aes/test-output/testng-results.xml
+++ b/main/crypto-aes/test-output/testng-results.xml
@@ -2,10 +2,10 @@
 
   
   
-  
+  
     
     
-    
+    
      
    
 
diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/CryptorIOSupport.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/CryptorIOSupport.java
index f77048796..afed0e506 100644
--- a/main/crypto-api/src/main/java/org/cryptomator/crypto/CryptorIOSupport.java
+++ b/main/crypto-api/src/main/java/org/cryptomator/crypto/CryptorIOSupport.java
@@ -16,7 +16,7 @@ public interface CryptorIOSupport {
 	void writePathSpecificMetadata(String encryptedPath, byte[] encryptedMetadata) throws IOException;
 
 	/**
-	 * @return Previously written encryptedMetadata stored at the given encryptedPath.
+	 * @return Previously written encryptedMetadata stored at the given encryptedPath or null if no such file exists.
 	 */
 	byte[] readPathSpecificMetadata(String encryptedPath) throws IOException;