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 81c139d71..afcc3f578 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 @@ -11,7 +11,6 @@ package org.cryptomator.webdav.jackrabbit.resources; import java.io.IOException; import java.nio.channels.SeekableByteChannel; import java.nio.file.DirectoryStream; -import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; @@ -79,12 +78,10 @@ public class EncryptedDir extends AbstractEncryptedNode { private void addMemberFile(DavResource resource, InputContext inputContext) throws DavException { final Path childPath = ResourcePathUtils.getPhysicalPath(resource); - try (final SeekableByteChannel channel = Files.newByteChannel(childPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW)) { + try (final SeekableByteChannel channel = Files.newByteChannel(childPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) { cryptor.encryptFile(inputContext.getInputStream(), channel); } catch (SecurityException e) { throw new DavException(DavServletResponse.SC_FORBIDDEN, e); - } catch (FileAlreadyExistsException e) { - throw new DavException(DavServletResponse.SC_CONFLICT, e); } catch (IOException e) { LOG.error("Failed to create file.", e); throw new IORuntimeException(e); @@ -124,7 +121,9 @@ public class EncryptedDir extends AbstractEncryptedNode { public void removeMember(DavResource member) throws DavException { final Path memberPath = ResourcePathUtils.getPhysicalPath(member); try { - Files.walkFileTree(memberPath, new DeletingFileVisitor()); + if (Files.exists(memberPath)) { + Files.walkFileTree(memberPath, new DeletingFileVisitor()); + } } catch (SecurityException e) { throw new DavException(DavServletResponse.SC_FORBIDDEN, e); } catch (IOException e) { diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedFile.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedFile.java index 0568b5b75..2cd0369fe 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedFile.java +++ b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/resources/EncryptedFile.java @@ -29,6 +29,7 @@ import org.apache.jackrabbit.webdav.property.DavPropertyName; import org.apache.jackrabbit.webdav.property.DefaultDavProperty; import org.cryptomator.crypto.Cryptor; import org.cryptomator.crypto.exceptions.DecryptFailedException; +import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException; import org.cryptomator.webdav.exceptions.IORuntimeException; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; @@ -70,12 +71,17 @@ public class EncryptedFile extends AbstractEncryptedNode { outputContext.setModificationTime(Files.getLastModifiedTime(path).toMillis()); outputContext.setProperty(HttpHeader.ACCEPT_RANGES.asString(), HttpHeaderValue.BYTES.asString()); try (final SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) { - outputContext.setContentLength(cryptor.decryptedContentLength(channel)); + final Long contentLength = cryptor.decryptedContentLength(channel); + if (contentLength != null) { + outputContext.setContentLength(contentLength); + } if (outputContext.hasStream()) { - cryptor.decryptedFile(channel, outputContext.getOutputStream()); + cryptor.decryptFile(channel, outputContext.getOutputStream()); } } catch (EOFException e) { LOG.warn("Unexpected end of stream (possibly client hung up)."); + } catch (MacAuthenticationFailedException e) { + LOG.warn("MAC authentication failed, file content {} might be compromised.", getLocator().getResourcePath()); } catch (DecryptFailedException e) { throw new IOException("Error decrypting file " + path.toString(), e); } 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 6b96859d2..a4ed5b505 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 @@ -47,6 +47,7 @@ import org.bouncycastle.crypto.generators.SCrypt; import org.cryptomator.crypto.AbstractCryptor; import org.cryptomator.crypto.CryptorIOSupport; import org.cryptomator.crypto.exceptions.DecryptFailedException; +import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException; import org.cryptomator.crypto.exceptions.UnsupportedKeyLengthException; import org.cryptomator.crypto.exceptions.WrongPasswordException; import org.cryptomator.crypto.io.SeekableByteChannelInputStream; @@ -369,34 +370,6 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo ioSupport.writePathSpecificMetadata(metadataFile, objectMapper.writeValueAsBytes(metadata)); } - private void authenticateContent(SeekableByteChannel encryptedFile) throws IOException, DecryptFailedException { - // init mac: - final Mac calculatedMac = this.hmacSha256(hMacMasterKey); - - // read stored mac: - encryptedFile.position(16); - final ByteBuffer storedMac = ByteBuffer.allocate(calculatedMac.getMacLength()); - final int numMacBytesRead = encryptedFile.read(storedMac); - - // check validity of header: - if (numMacBytesRead != calculatedMac.getMacLength()) { - throw new IOException("Failed to read file header."); - } - - // read all encrypted data and calculate mac: - encryptedFile.position(64); - final InputStream in = new SeekableByteChannelInputStream(encryptedFile); - final InputStream macIn = new MacInputStream(in, calculatedMac); - IOUtils.copyLarge(macIn, new NullOutputStream()); - - // compare (in constant time): - boolean macMatches = MessageDigest.isEqual(storedMac.array(), calculatedMac.doFinal()); - - if (!macMatches) { - throw new DecryptFailedException("MAC authentication failed."); - } - } - @Override public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException { // skip 128bit IV + 256 bit MAC: @@ -422,24 +395,49 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo } } + private void encryptedContentLength(SeekableByteChannel encryptedFile, Long contentLength) throws IOException { + final ByteBuffer encryptedFileSizeBuffer; + + // encrypt content length in ECB mode (content length is less than one block): + try { + final ByteBuffer fileSizeBuffer = ByteBuffer.allocate(Long.BYTES); + fileSizeBuffer.putLong(contentLength); + final Cipher sizeCipher = aesEcbCipher(primaryMasterKey, Cipher.ENCRYPT_MODE); + final byte[] encryptedFileSize = sizeCipher.doFinal(fileSizeBuffer.array()); + encryptedFileSizeBuffer = ByteBuffer.wrap(encryptedFileSize); + } catch (IllegalBlockSizeException | BadPaddingException e) { + throw new IllegalStateException("Block size must be valid, as padding is requested. BadPaddingException not possible in encrypt mode.", e); + } + + // skip 128bit IV + 256 bit MAC: + encryptedFile.position(48); + + // write result: + encryptedFile.write(encryptedFileSizeBuffer); + } + @Override - public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException { + public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException { // read iv: encryptedFile.position(0); final ByteBuffer countingIv = ByteBuffer.allocate(AES_BLOCK_LENGTH); final int numIvBytesRead = encryptedFile.read(countingIv); + // init mac: + final Mac calculatedMac = this.hmacSha256(hMacMasterKey); + + // read stored mac: + final ByteBuffer storedMac = ByteBuffer.allocate(calculatedMac.getMacLength()); + final int numMacBytesRead = encryptedFile.read(storedMac); + // read file size: final Long fileSize = decryptedContentLength(encryptedFile); // check validity of header: - if (numIvBytesRead != AES_BLOCK_LENGTH || fileSize == null) { + if (numIvBytesRead != AES_BLOCK_LENGTH || numMacBytesRead != calculatedMac.getMacLength() || fileSize == null) { throw new IOException("Failed to read file header."); } - // check MAC: - this.authenticateContent(encryptedFile); - // go to begin of content: encryptedFile.position(64); @@ -448,8 +446,25 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo // read content final InputStream in = new SeekableByteChannelInputStream(encryptedFile); - final InputStream cipheredIn = new CipherInputStream(in, cipher); - return IOUtils.copyLarge(cipheredIn, plaintextFile, 0, fileSize); + final InputStream macIn = new MacInputStream(in, calculatedMac); + final InputStream cipheredIn = new CipherInputStream(macIn, cipher); + final long bytesDecrypted = IOUtils.copyLarge(cipheredIn, plaintextFile, 0, fileSize); + + // drain remaining bytes to /dev/null to complete MAC calculation: + IOUtils.copyLarge(macIn, new NullOutputStream()); + + // compare (in constant time): + final boolean macMatches = MessageDigest.isEqual(storedMac.array(), calculatedMac.doFinal()); + if (!macMatches) { + // This exception will be thrown AFTER we sent the decrypted content to the user. + // This has two advantages: + // - we don't need to read files twice + // - we can still restore files suffering from non-malicious bit rotting + // Anyway me MUST make sure to warn the user. This will be done by the UI when catching this exception. + throw new MacAuthenticationFailedException("MAC authentication failed."); + } + + return bytesDecrypted; } @Override @@ -464,9 +479,6 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo throw new IOException("Failed to read file header."); } - // check MAC: - this.authenticateContent(encryptedFile); - // seek relevant position and update iv: long firstRelevantBlock = pos / AES_BLOCK_LENGTH; // cut of fraction! long beginOfFirstRelevantBlock = firstRelevantBlock * AES_BLOCK_LENGTH; @@ -493,7 +505,6 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo // use an IV, whose last 8 bytes store a long used in counter mode and write initial value to file. final ByteBuffer countingIv = ByteBuffer.wrap(randomData(AES_BLOCK_LENGTH)); countingIv.putLong(AES_BLOCK_LENGTH - Long.BYTES, 0l); - countingIv.position(0); encryptedFile.write(countingIv); // init crypto stuff: @@ -504,20 +515,8 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo final ByteBuffer macBuffer = ByteBuffer.allocate(mac.getMacLength()); encryptedFile.write(macBuffer); - // write initial file size = 0 into the next 16 bytes - final ByteBuffer encryptedFileSizeBuffer = ByteBuffer.allocate(AES_BLOCK_LENGTH); - try { - final ByteBuffer fileSizeBuffer = ByteBuffer.allocate(Long.BYTES); - fileSizeBuffer.putLong(0l); - final Cipher sizeCipher = aesEcbCipher(primaryMasterKey, Cipher.ENCRYPT_MODE); - final byte[] encryptedFileSize = sizeCipher.doFinal(fileSizeBuffer.array()); - encryptedFileSizeBuffer.position(0); - encryptedFileSizeBuffer.put(encryptedFileSize); - encryptedFileSizeBuffer.position(0); - encryptedFile.write(encryptedFileSizeBuffer); - } catch (IllegalBlockSizeException | BadPaddingException e) { - throw new IllegalStateException("Block size must be valid, as padding is requested. BadPaddingException not possible in encrypt mode.", e); - } + // encrypt and write "zero length" as a placeholder, which will be read by concurrent requests, as long as encryption didn't finish: + encryptedContentLength(encryptedFile, 0l); // write content: final OutputStream out = new SeekableByteChannelOutputStream(encryptedFile); @@ -540,26 +539,14 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo blockSizeBufferedOut.flush(); // write MAC of total ciphertext: - macBuffer.position(0); + macBuffer.clear(); macBuffer.put(mac.doFinal()); - macBuffer.position(0); + macBuffer.flip(); encryptedFile.position(16); // right behind the IV encryptedFile.write(macBuffer); // 256 bit MAC - // encrypt and write plaintextSize - try { - final ByteBuffer fileSizeBuffer = ByteBuffer.allocate(Long.BYTES); - fileSizeBuffer.putLong(plaintextSize); - final Cipher sizeCipher = aesEcbCipher(primaryMasterKey, Cipher.ENCRYPT_MODE); - final byte[] encryptedFileSize = sizeCipher.doFinal(fileSizeBuffer.array()); - encryptedFileSizeBuffer.position(0); - encryptedFileSizeBuffer.put(encryptedFileSize); - encryptedFileSizeBuffer.position(0); - encryptedFile.position(48); // right behind the IV and MAC - encryptedFile.write(encryptedFileSizeBuffer); - } catch (IllegalBlockSizeException | BadPaddingException e) { - throw new IllegalStateException("Block size must be valid, as padding is requested. BadPaddingException not possible in encrypt mode.", e); - } + // encrypt and write plaintextSize: + encryptedContentLength(encryptedFile, plaintextSize); return plaintextSize; } 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 7d486c8c8..4b29d9aed 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 @@ -98,7 +98,7 @@ public class Aes256CryptorTest { // decrypt modified content (should fail with DecryptFailedException): final SeekableByteChannel encryptedIn = new ByteBufferBackedSeekableChannel(encryptedData); final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream(); - cryptor.decryptedFile(encryptedIn, plaintextOut); + cryptor.decryptFile(encryptedIn, plaintextOut); } @Test @@ -126,7 +126,7 @@ public class Aes256CryptorTest { // decrypt: final ByteArrayOutputStream plaintextOut = new ByteArrayOutputStream(); - final Long numDecryptedBytes = cryptor.decryptedFile(encryptedIn, plaintextOut); + final Long numDecryptedBytes = cryptor.decryptFile(encryptedIn, plaintextOut); IOUtils.closeQuietly(encryptedIn); IOUtils.closeQuietly(plaintextOut); Assert.assertEquals(filesize.longValue(), numDecryptedBytes.longValue()); 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 2183c8048..b9e05f358 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 @@ -79,7 +79,7 @@ public interface Cryptor extends SensitiveDataSwipeListener { * @return Number of decrypted bytes. This might not be equal to the encrypted file size due to optional metadata written to it. * @throws DecryptFailedException If decryption failed */ - Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException; + Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException; /** * @param pos First byte (inclusive) 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 e160838ab..7c3e9245f 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 @@ -82,9 +82,9 @@ public class SamplingDecorator implements Cryptor, CryptorIOSampling { } @Override - public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException { + public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile) throws IOException, DecryptFailedException { final OutputStream countingInputStream = new CountingOutputStream(decryptedBytes, plaintextFile); - return cryptor.decryptedFile(encryptedFile, countingInputStream); + return cryptor.decryptFile(encryptedFile, countingInputStream); } @Override diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/exceptions/MacAuthenticationFailedException.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/exceptions/MacAuthenticationFailedException.java new file mode 100644 index 000000000..7535c35fe --- /dev/null +++ b/main/crypto-api/src/main/java/org/cryptomator/crypto/exceptions/MacAuthenticationFailedException.java @@ -0,0 +1,11 @@ +package org.cryptomator.crypto.exceptions; + +public class MacAuthenticationFailedException extends DecryptFailedException { + + private static final long serialVersionUID = -5577052361643658772L; + + public MacAuthenticationFailedException(String msg) { + super(msg); + } + +}