From 9fdd2f339c28bac467f44f9c9fbe1fe7e9263873 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Sat, 14 Feb 2015 18:55:33 +0100 Subject: [PATCH] - changed file name encryption to SIV mode - vastly improved exception handling, if decryption of a path name fails --- .../DecryptFailedRuntimeException.java | 23 ++++++++++++ .../jackrabbit/DavLocatorFactoryImpl.java | 23 +++++++++--- .../jackrabbit/DavResourceFactoryImpl.java | 4 +-- .../jackrabbit/resources/EncryptedDir.java | 12 +++++-- .../jackrabbit/resources/NonExistingNode.java | 2 +- .../crypto/aes256/Aes256Cryptor.java | 36 ++++++++----------- .../crypto/aes256/AesSivCipherUtil.java | 8 +++++ .../crypto/aes256/Aes256CryptorTest.java | 2 +- .../java/org/cryptomator/crypto/Cryptor.java | 3 +- .../cryptomator/crypto/SamplingDecorator.java | 2 +- .../org/cryptomator/ui/MainController.java | 2 +- 11 files changed, 80 insertions(+), 37 deletions(-) create mode 100644 main/core/src/main/java/org/cryptomator/webdav/exceptions/DecryptFailedRuntimeException.java diff --git a/main/core/src/main/java/org/cryptomator/webdav/exceptions/DecryptFailedRuntimeException.java b/main/core/src/main/java/org/cryptomator/webdav/exceptions/DecryptFailedRuntimeException.java new file mode 100644 index 000000000..12e28da11 --- /dev/null +++ b/main/core/src/main/java/org/cryptomator/webdav/exceptions/DecryptFailedRuntimeException.java @@ -0,0 +1,23 @@ +package org.cryptomator.webdav.exceptions; + +import org.cryptomator.crypto.exceptions.DecryptFailedException; + +public class DecryptFailedRuntimeException extends RuntimeException { + + private static final long serialVersionUID = -2726689824823439865L; + + public DecryptFailedRuntimeException(DecryptFailedException cause) { + super(cause); + } + + @Override + public String getMessage() { + return getCause().getMessage(); + } + + @Override + public String getLocalizedMessage() { + return getCause().getLocalizedMessage(); + } + +} diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavLocatorFactoryImpl.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavLocatorFactoryImpl.java index de0410d25..eca2de9d6 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavLocatorFactoryImpl.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/DavLocatorFactoryImpl.java @@ -24,6 +24,8 @@ import org.apache.jackrabbit.webdav.util.EncodeUtil; import org.cryptomator.crypto.Cryptor; import org.cryptomator.crypto.CryptorIOSupport; import org.cryptomator.crypto.SensitiveDataSwipeListener; +import org.cryptomator.crypto.exceptions.DecryptFailedException; +import org.cryptomator.webdav.exceptions.DecryptFailedRuntimeException; class DavLocatorFactoryImpl implements DavLocatorFactory, SensitiveDataSwipeListener, CryptorIOSupport { @@ -49,17 +51,28 @@ class DavLocatorFactoryImpl implements DavLocatorFactory, SensitiveDataSwipeList return new DavResourceLocatorImpl(fullPrefix, resourcePath); } + /** + * @throws DecryptFailedRuntimeException, which should a checked exception, but Jackrabbit doesn't allow that. + */ @Override public DavResourceLocator createResourceLocator(String prefix, String workspacePath, String path, boolean isResourcePath) { final String fullPrefix = prefix.endsWith("/") ? prefix : prefix + "/"; - final String resourcePath = (isResourcePath) ? path : getResourcePath(path); - return new DavResourceLocatorImpl(fullPrefix, resourcePath); + try { + final String resourcePath = (isResourcePath) ? path : getResourcePath(path); + return new DavResourceLocatorImpl(fullPrefix, resourcePath); + } catch (DecryptFailedException e) { + throw new DecryptFailedRuntimeException(e); + } } @Override public DavResourceLocator createResourceLocator(String prefix, String workspacePath, String resourcePath) { - return createResourceLocator(prefix, workspacePath, resourcePath, true); + try { + return createResourceLocator(prefix, workspacePath, resourcePath, true); + } catch (DecryptFailedRuntimeException e) { + throw new IllegalStateException("Tried to decrypt resourcePath. Only repositoryPaths can be encrypted.", e); + } } /* Encryption/Decryption */ @@ -87,7 +100,7 @@ class DavLocatorFactoryImpl implements DavLocatorFactory, SensitiveDataSwipeList /** * @return Decrypted path for use in URIs. */ - private String getResourcePath(String repositoryPath) { + private String getResourcePath(String repositoryPath) throws DecryptFailedException { String decryptedPath = pathCache.getKey(repositoryPath); if (decryptedPath == null) { decryptedPath = decryptResourcePath(repositoryPath); @@ -96,7 +109,7 @@ class DavLocatorFactoryImpl implements DavLocatorFactory, SensitiveDataSwipeList return decryptedPath; } - private String decryptResourcePath(String repositoryPath) { + private String decryptResourcePath(String repositoryPath) throws DecryptFailedException { final Path absRepoPath = FileSystems.getDefault().getPath(repositoryPath); if (fsRoot.equals(absRepoPath)) { return null; 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 834c01f9f..f8f0ddb0e 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 @@ -62,9 +62,9 @@ class DavResourceFactoryImpl implements DavResourceFactory { public DavResource createResource(DavResourceLocator locator, DavSession session) throws DavException { final Path path = ResourcePathUtils.getPhysicalPath(locator); - if (Files.isRegularFile(path)) { + if (path != null && Files.isRegularFile(path)) { return createFile(locator, session); - } else if (Files.isDirectory(path)) { + } else if (path != null && Files.isDirectory(path)) { return createDirectory(locator, session); } else { return createNonExisting(locator, session); diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedDir.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedDir.java index 17ca91926..8c95b62b3 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedDir.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedDir.java @@ -37,6 +37,7 @@ import org.apache.jackrabbit.webdav.property.DefaultDavProperty; import org.apache.jackrabbit.webdav.property.ResourceType; import org.cryptomator.crypto.Cryptor; import org.cryptomator.webdav.exceptions.DavRuntimeException; +import org.cryptomator.webdav.exceptions.DecryptFailedRuntimeException; import org.cryptomator.webdav.exceptions.IORuntimeException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -100,9 +101,14 @@ public class EncryptedDir extends AbstractEncryptedNode { final List result = new ArrayList<>(); for (final Path childPath : directoryStream) { - final DavResourceLocator childLocator = locator.getFactory().createResourceLocator(locator.getPrefix(), locator.getWorkspacePath(), childPath.toString(), false); - final DavResource resource = factory.createResource(childLocator, session); - result.add(resource); + try { + final DavResourceLocator childLocator = locator.getFactory().createResourceLocator(locator.getPrefix(), locator.getWorkspacePath(), childPath.toString(), false); + final DavResource resource = factory.createResource(childLocator, session); + result.add(resource); + } catch (DecryptFailedRuntimeException e) { + LOG.warn("Decryption of resource failed: " + childPath); + continue; + } } return new DavResourceIteratorImpl(result); } catch (IOException e) { diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/NonExistingNode.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/NonExistingNode.java index 2a58e62a2..4461dfa4b 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/NonExistingNode.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/NonExistingNode.java @@ -34,7 +34,7 @@ public class NonExistingNode extends AbstractEncryptedNode { @Override public boolean isCollection() { - throw new UnsupportedOperationException("Resource doesn't exist."); + return false; } @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 8be60eba7..a049f3677 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 @@ -305,7 +305,7 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo encryptedPathComps.add(encrypted); } return StringUtils.join(encryptedPathComps, encryptedPathSep); - } catch (IllegalBlockSizeException | BadPaddingException | IOException e) { + } catch (InvalidKeyException | IOException e) { throw new IllegalStateException("Unable to encrypt path: " + cleartextPath, e); } } @@ -325,18 +325,15 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo * 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 byte[] mac = hmacSha256(hMacMasterKey).doFinal(cleartext.getBytes()); - final byte[] partialIv = ArrayUtils.subarray(mac, 0, 10); - final ByteBuffer iv = ByteBuffer.allocate(AES_BLOCK_LENGTH); - iv.put(partialIv); - final Cipher cipher = this.aesCtrCipher(key, iv.array(), Cipher.ENCRYPT_MODE); + private String encryptPathComponent(final String cleartext, final SecretKey key, CryptorIOSupport ioSupport) throws IOException, InvalidKeyException { // add NULL padding to the cleartext to get a multiple of the block size: final byte[] cleartextBytes = cleartext.getBytes(StandardCharsets.UTF_8); final byte[] nullBytePadding = new byte[AES_BLOCK_LENGTH - cleartextBytes.length % AES_BLOCK_LENGTH]; final byte[] paddedCleartextBytes = ArrayUtils.addAll(cleartextBytes, nullBytePadding); - final byte[] encryptedBytes = cipher.doFinal(paddedCleartextBytes); - final String ivAndCiphertext = ENCRYPTED_FILENAME_CODEC.encodeAsString(partialIv) + IV_PREFIX_SEPARATOR + ENCRYPTED_FILENAME_CODEC.encodeAsString(encryptedBytes); + + // encrypt: + final byte[] encryptedBytes = AesSivCipherUtil.sivEncrypt(key.getEncoded(), paddedCleartextBytes); + final String ivAndCiphertext = ENCRYPTED_FILENAME_CODEC.encodeAsString(encryptedBytes); if (ivAndCiphertext.length() + BASIC_FILE_EXT.length() > ENCRYPTED_FILENAME_LENGTH_LIMIT) { final String crc32 = Long.toHexString(crc32Sum(ivAndCiphertext.getBytes())); @@ -351,7 +348,7 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo } @Override - public String decryptPath(String encryptedPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) { + public String decryptPath(String encryptedPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) throws DecryptFailedException { try { final String[] encryptedPathComps = StringUtils.split(encryptedPath, encryptedPathSep); final List cleartextPathComps = new ArrayList<>(encryptedPathComps.length); @@ -360,7 +357,7 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo cleartextPathComps.add(new String(cleartext)); } return StringUtils.join(cleartextPathComps, cleartextPathSep); - } catch (IllegalBlockSizeException | BadPaddingException | IOException e) { + } catch (InvalidKeyException | IOException e) { throw new IllegalStateException("Unable to decrypt path: " + encryptedPath, e); } } @@ -368,29 +365,24 @@ 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 ivAndCiphertext; + private String decryptPathComponent(final String encrypted, final SecretKey key, CryptorIOSupport ioSupport) throws IOException, InvalidKeyException, DecryptFailedException { + final String ciphertext; if (encrypted.endsWith(LONG_NAME_FILE_EXT)) { 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); - ivAndCiphertext = metadata.getEncryptedFilenameForUUID(UUID.fromString(uuid)); + ciphertext = metadata.getEncryptedFilenameForUUID(UUID.fromString(uuid)); } else if (encrypted.endsWith(BASIC_FILE_EXT)) { - ivAndCiphertext = StringUtils.removeEndIgnoreCase(encrypted, BASIC_FILE_EXT); + ciphertext = StringUtils.removeEndIgnoreCase(encrypted, BASIC_FILE_EXT); } else { throw new IllegalArgumentException("Unsupported path component: " + encrypted); } - final String partialIvStr = StringUtils.substringBefore(ivAndCiphertext, IV_PREFIX_SEPARATOR); - final String ciphertext = StringUtils.substringAfter(ivAndCiphertext, IV_PREFIX_SEPARATOR); - final ByteBuffer iv = ByteBuffer.allocate(AES_BLOCK_LENGTH); - iv.put(ENCRYPTED_FILENAME_CODEC.decode(partialIvStr)); - - final Cipher cipher = this.aesCtrCipher(key, iv.array(), Cipher.DECRYPT_MODE); + // decrypt: final byte[] encryptedBytes = ENCRYPTED_FILENAME_CODEC.decode(ciphertext); - final byte[] paddedCleartextBytes = cipher.doFinal(encryptedBytes); + final byte[] paddedCleartextBytes = AesSivCipherUtil.sivDecrypt(key.getEncoded(), encryptedBytes); // remove NULL padding (not valid in file names anyway) final int beginOfPadding = ArrayUtils.indexOf(paddedCleartextBytes, (byte) 0x00); diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesSivCipherUtil.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesSivCipherUtil.java index 790304049..9e990571f 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesSivCipherUtil.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesSivCipherUtil.java @@ -1,3 +1,11 @@ +/******************************************************************************* + * Copyright (c) 2014 Sebastian Stenzel + * This file is licensed under the terms of the MIT license. + * See the LICENSE.txt file for more info. + * + * Contributors: + * Sebastian Stenzel - initial API and implementation + ******************************************************************************/ package org.cryptomator.crypto.aes256; import java.nio.ByteBuffer; 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 bb1ff5a76..151af42dc 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 @@ -215,7 +215,7 @@ public class Aes256CryptorTest { } @Test - public void testEncryptionOfFilenames() throws IOException { + public void testEncryptionOfFilenames() throws IOException, DecryptFailedException { final CryptorIOSupport ioSupportMock = new CryptoIOSupportMock(); final Aes256Cryptor cryptor = new Aes256Cryptor(TEST_PRNG); 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 48b962cdf..96c8b9918 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 @@ -65,8 +65,9 @@ public interface Cryptor extends SensitiveDataSwipeListener { * @param metadataSupport Support object allowing the Cryptor to read and write its own metadata to the location of the encrypted file. * @return Decrypted path components concatenated by the given cleartextPathSep. Must not start with cleartextPathSep, unless the * cleartext path is explicitly absolute. + * @throws DecryptFailedException If the decryption failed for various reasons (including wrong password). */ - String decryptPath(String encryptedPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport); + String decryptPath(String encryptedPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) throws DecryptFailedException; /** * @return true If the integrity of the file can be assured. 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 19cd9a2ca..bf0d33b19 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 @@ -71,7 +71,7 @@ public class SamplingDecorator implements Cryptor, CryptorIOSampling { } @Override - public String decryptPath(String encryptedPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) { + public String decryptPath(String encryptedPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) throws DecryptFailedException { decryptedBytes.addAndGet(StringUtils.length(encryptedPath)); return cryptor.decryptPath(encryptedPath, encryptedPathSep, cleartextPathSep, ioSupport); } diff --git a/main/ui/src/main/java/org/cryptomator/ui/MainController.java b/main/ui/src/main/java/org/cryptomator/ui/MainController.java index 087f3d169..c3a92d93a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/MainController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/MainController.java @@ -158,7 +158,7 @@ public class MainController implements Initializable, InitializationListener, Un final Path vaultPath; if (path != null && Files.isDirectory(path)) { vaultPath = path; - } else if (Files.isRegularFile(path) && path.getParent().getFileName().toString().endsWith(Vault.VAULT_FILE_EXTENSION)) { + } else if (path != null && Files.isRegularFile(path) && path.getParent().getFileName().toString().endsWith(Vault.VAULT_FILE_EXTENSION)) { vaultPath = path.getParent(); } else { return;