Skip to content

Commit fb7ccb9

Browse files
committed
Use lastModified timestamp to check if a in-use file was stolen/moved
1 parent eba5f29 commit fb7ccb9

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

src/main/java/org/cryptomator/cryptofs/inuse/RealUseToken.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public static RealUseToken createWithExistingFile(Path p, String owner, Cryptor
5555
private volatile Path filePath;
5656
private volatile FileChannel channel;
5757
private volatile boolean closed;
58+
private volatile long lastModified;
5859

5960
RealUseToken(Path filePath, String owner, Cryptor cryptor, ConcurrentMap<Path, RealUseToken> useTokens, Executor tokenPersistor, OpenOption openMode) {
6061
var delayedExecutor = CompletableFuture.delayedExecutor(Constants.INUSE_DELAY_MILLIS, TimeUnit.MILLISECONDS, tokenPersistor);
@@ -96,9 +97,18 @@ void refresh() {
9697
if (closed || channel == null) {
9798
return;
9899
}
100+
var currentLastModfied = Files.getLastModifiedTime(filePath).toMillis();
101+
if (currentLastModfied != lastModified) {
102+
throw new ModifiedFileException(); //someone edited _our_ file.
103+
}
99104
writeInUseFile();
105+
} catch (ModifiedFileException e) {
106+
//TODO: event? we have no access to the cleartext!
107+
LOG.debug("Failed to refresh in-use file {}.", filePath, e);
108+
close(false);
100109
} catch (IOException e) {
101110
LOG.debug("Failed to refresh in-use file {}.", filePath, e);
111+
close();
102112
} finally {
103113
fileCreationSync.unlock();
104114
}
@@ -116,7 +126,8 @@ int writeInUseFile() throws IOException {
116126
prop.store(rawInfo, null);
117127
bytesWritten = encChannel.write(ByteBuffer.wrap(rawInfo.toByteArray()));
118128
}
119-
channel.force(false);
129+
channel.force(true);
130+
lastModified = Files.getLastModifiedTime(filePath).toMillis();
120131
return bytesWritten;
121132
}
122133

@@ -136,6 +147,7 @@ void moveToInternal(Path newFilePath) {
136147
useTokens.compute(newFilePath, (_, _) -> {
137148
try {
138149
if (channel != null) {
150+
//TODO: does this affect the lastModified file?
139151
Files.move(filePath, newFilePath, StandardCopyOption.REPLACE_EXISTING);
140152
}
141153
return this;
@@ -160,6 +172,10 @@ public boolean isClosed() {
160172

161173
@Override
162174
public void close() {
175+
close(true);
176+
}
177+
178+
void close(boolean deleteFile) {
163179
fileCreationSync.lock();
164180
try {
165181
if (closed) {
@@ -171,7 +187,9 @@ public void close() {
171187
if (channel != null) {
172188
try {
173189
channel.close();
174-
Files.deleteIfExists(filePath);
190+
if(deleteFile) {
191+
Files.deleteIfExists(filePath);
192+
}
175193
} catch (IOException e) {
176194
//ignore
177195
LOG.warn("Failed to delete inUse File {}. Must be deleted manually.", path);
@@ -184,6 +202,8 @@ public void close() {
184202
}
185203
}
186204

205+
//--- glue code ---
206+
187207
interface EncryptionDecorator {
188208

189209
WritableByteChannel wrapWithEncryption(ByteChannel ch, Cryptor c);
@@ -211,4 +231,8 @@ public int read(ByteBuffer dst) throws IOException {
211231
return delegate.read(dst);
212232
}
213233
}
234+
235+
static class ModifiedFileException extends RuntimeException {
236+
237+
}
214238
}

src/test/java/org/cryptomator/cryptofs/inuse/RealUseTokenTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,21 @@
88
import org.junit.jupiter.api.Assertions;
99
import org.junit.jupiter.api.BeforeEach;
1010
import org.junit.jupiter.api.DisplayName;
11+
import org.junit.jupiter.api.RepeatedTest;
1112
import org.junit.jupiter.api.Test;
1213
import org.junit.jupiter.api.io.TempDir;
14+
import org.junit.jupiter.api.parallel.Execution;
15+
import org.junit.jupiter.api.parallel.ExecutionMode;
1316

1417
import java.io.ByteArrayInputStream;
1518
import java.io.IOException;
19+
import java.nio.channels.FileChannel;
1620
import java.nio.file.Files;
1721
import java.nio.file.Path;
1822
import java.nio.file.StandardOpenOption;
1923
import java.nio.file.StandardWatchEventKinds;
2024
import java.nio.file.WatchService;
25+
import java.nio.file.attribute.FileTime;
2126
import java.time.Duration;
2227
import java.time.Instant;
2328
import java.util.Properties;
@@ -80,6 +85,7 @@ public void testFileCreation() {
8085
Assertions.assertTrue(Files.notExists(filePath));
8186
}
8287

88+
//TODO: test is flaky. Why?
8389
@Test
8490
@DisplayName("The properties file contains required keys with valid content")
8591
public void testFileContent() throws IOException {
@@ -244,6 +250,26 @@ public void testFileRefresh() throws IOException {
244250
}
245251
}
246252

253+
@RepeatedTest(value = 5, failureThreshold = 1)
254+
@Execution(ExecutionMode.SAME_THREAD)
255+
@DisplayName("Refreshing a token with not-matching last-modified date closes token, but does not delete file ")
256+
public void testFileRefreshWrongLastModified() throws IOException {
257+
var filePath = tmpDir.resolve("inUse.file");
258+
259+
try (var token = new RealUseToken(filePath, "test3000", cryptor, useTokens, tokenPersistor, StandardOpenOption.CREATE_NEW, encWrapper)) {
260+
Awaitility.await().atLeast(FILE_OPERATION_DELAY).atMost(FILE_OPERATION_MAX).until(() -> Files.exists(filePath));
261+
Awaitility.await().pollDelay(Duration.ofMillis(10)).until(() -> true);
262+
263+
Files.setLastModifiedTime(filePath, FileTime.from(Instant.ofEpochMilli(0)));
264+
265+
token.refresh();
266+
267+
Assertions.assertTrue(token.isClosed());
268+
Assertions.assertTrue(Files.exists(filePath));
269+
}
270+
Assertions.assertTrue(Files.exists(filePath));
271+
}
272+
247273
@Test
248274
@DisplayName("Before token persisting, refreshing a token does nothing")
249275
public void testFileRefreshSkip() throws IOException {

0 commit comments

Comments
 (0)