Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Windows Hello support #83

Merged
merged 58 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
b145a0b
Add Windows Hello stubs
purejava Sep 8, 2024
9611e68
Implement Windows Hello
purejava Sep 25, 2024
db4d6c5
Intendation is different from integrations-mac
purejava Sep 26, 2024
dda8daa
auto initialize variables
purejava Sep 26, 2024
58ca1cb
Successfully compile code and create dll using cl
purejava Oct 16, 2024
a7bf1d4
Bring Windows Hello prompt to the front
purejava Oct 17, 2024
13e8f6f
Add compiler output to .gitignore
purejava Oct 18, 2024
2fe356d
Init Windows Runtime only once
purejava Oct 18, 2024
58c2b95
Now really init Windows Runtime only once
purejava Oct 19, 2024
885e2f5
Have better understandable log messages
purejava Oct 19, 2024
4fadb66
Use the term challenge, that is more appropriate
purejava Oct 19, 2024
080121c
Start wire Windows Hello to WindowsProtectedKeychainAccess
purejava Oct 19, 2024
1343bc8
Update org_cryptomator_windows_keychain_WinHello_Native.cpp
purejava Oct 19, 2024
02168f5
Migrate existing KeychainEntries and load a passphrase
purejava Oct 20, 2024
de97298
Handle C++/WinRT errors
purejava Oct 23, 2024
15488af
Derive the encryption/decryption key using HKDF
purejava Nov 10, 2024
8860347
Fix compile time errors
purejava Nov 11, 2024
691c5c6
Revert to AesCbcPkcs7()
purejava Nov 13, 2024
30321e2
Switch to SymmetricAlgorithmNames::AesGcm()
purejava Nov 13, 2024
58518b1
Code suggestions from CodeRabbit
purejava Nov 13, 2024
26e12c5
Code suggestions from CodeRabbit
purejava Nov 13, 2024
9e8c35a
Use std:cerr constantly
purejava Nov 15, 2024
85528ea
Have thread safety for g_promptFocusCount
purejava Nov 15, 2024
b75b132
Revert "Switch to SymmetricAlgorithmNames::AesGcm()"
purejava Nov 15, 2024
9ac7cb6
Improve security by using a random IV
purejava Nov 16, 2024
4329408
Add JUnit test for Windows Hello
purejava Nov 17, 2024
9d58bef
Fix changePassphrase depending on requireAuth
purejava Nov 18, 2024
02d2383
Guard method parameter
purejava Nov 23, 2024
8e9da54
Make initializing Windows Runtime more robust
purejava Nov 24, 2024
2446a1f
Merge branch 'develop' into hello
infeo Nov 25, 2024
1dc2adb
refactor WinHelloTest to be more unit like
infeo Nov 25, 2024
1264677
Add Windows Hello provider
purejava Nov 30, 2024
5c22aa0
Return correct type
purejava Nov 30, 2024
6d4e6e7
Fix naming
purejava Nov 30, 2024
c816046
Use correct property
purejava Nov 30, 2024
472546b
Make WindowsHelloKeychainAccess visible
purejava Nov 30, 2024
1d3904b
Add check whether Windows Hello is available
purejava Nov 30, 2024
a450cb2
Refactor doubled code
purejava Nov 30, 2024
1902053
WindowsHello and DPAPI extend the FileKeychainAccessProvider
infeo Dec 3, 2024
75f5bcb
composition over inheritance
infeo Dec 3, 2024
a318ae3
add unit test for keychainBase
infeo Dec 3, 2024
eb7c6d4
doc doc doc
infeo Dec 3, 2024
2afe296
added tests for FileKeychain
infeo Dec 3, 2024
8864528
copy path parsing tests
infeo Dec 4, 2024
649c0e6
move keychain providers to new architecture
infeo Dec 4, 2024
5c1e4af
added integration test for WindowsProtectedKeychainAccess
infeo Dec 4, 2024
af248a3
cleanup
infeo Dec 4, 2024
ef57359
quote javaagent to prevent splits due to spaces in path
infeo Dec 4, 2024
3a79eca
Merge pull request #1 from cryptomator/feature/common-keychain-base
purejava Dec 11, 2024
264eed1
Merge branch 'develop' into hello
infeo Dec 11, 2024
4dc73eb
Fix setEncryptionKey and getEncryptionKey
purejava Dec 12, 2024
f44c0a2
synchronize file keychain
infeo Dec 16, 2024
1f83dc0
fix project file
infeo Dec 16, 2024
e6ce85f
implement HKDF completely
infeo Dec 16, 2024
80391b4
linting
infeo Dec 16, 2024
f1f60d1
fix localization
infeo Dec 17, 2024
145a114
make windowsHello key name configurable
infeo Dec 17, 2024
3ac569d
document property
infeo Dec 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
*.class
*.jar
*.dll
*.obj
*.lib
*.exp

# Maven #
target/
Expand Down
5 changes: 5 additions & 0 deletions .idea/codeStyles/Project.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ install:
$(HEADERS) $(SOURCES) \
-link -NXCOMPAT -DYNAMICBASE \
-implib:target/integrations.lib \
crypt32.lib shell32.lib ole32.lib uuid.lib user32.lib Advapi32.lib
crypt32.lib shell32.lib ole32.lib uuid.lib user32.lib Advapi32.lib windowsapp.lib
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Windows-specific implementations of [integrations-api](https://github.com/crypto

This project uses the following JVM properties:
* `cryptomator.integrationsWin.autoStartShellLinkName` - Name of the shell link, which is placed in the Windows startup folder to start application on user login
* `cryptomator.integrationsWin.windowsHelloKeyId` - Identifier for the Windows Hello keypair
* `cryptomator.integrationsWin.windowsHelloKeychainPaths` - Locations of the file-based windowsHello keychain
* `cryptomator.integrationsWin.keychainPaths` - List of file paths, which are checked for data encrypted with the Windows data protection api

## Building
Expand Down
4 changes: 3 additions & 1 deletion integrations-win.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,15 @@
<ItemGroup>
<ClInclude Include="src\main\headers\org_cryptomator_windows_autostart_WinShellLinks_Native.h" />
<ClInclude Include="src\main\headers\org_cryptomator_windows_keychain_WinDataProtection_Native.h" />
<ClInclude Include="src\main\headers\org_cryptomator_windows_keychain_WindowsHello_Native.h" />
<ClInclude Include="src\main\headers\org_cryptomator_windows_uiappearance_WinAppearance_Native.h" />
<ClInclude Include="src\main\resources\ktmw32_helper.h" />
</ItemGroup>
<ItemGroup>
<ClCompile Include="src\main\native\org_cryptomator_windows_autostart_WinShellLinks_Native.cpp" />
<ClCompile Include="src\main\native\org_cryptomator_windows_keychain_WinDataProtection_Native.cpp" />
<ClCompile Include="src\main\native\org_cryptomator_windows_uiappearance_WinAppearance_Native.cpp" />
<ClCompile Include="src\main\native\org_cryptomator_windows_keychain_WindowsHello_Native.cpp" />
<ClCompile Include="src\main\native\org_cryptomator_windows_uiappearnce_WinAppearance_Native.cpp" />
infeo marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.cryptomator.integrations.revealpath.RevealPathService;
import org.cryptomator.integrations.uiappearance.UiAppearanceProvider;
import org.cryptomator.windows.autostart.WindowsAutoStart;
import org.cryptomator.windows.keychain.WindowsHelloKeychainAccess;
import org.cryptomator.windows.keychain.WindowsProtectedKeychainAccess;
import org.cryptomator.windows.quickaccess.ExplorerQuickAccessService;
import org.cryptomator.windows.revealpath.ExplorerRevealPathService;
Expand All @@ -19,7 +20,7 @@
opens org.cryptomator.windows.quickaccess to org.cryptomator.integrations.api;

provides AutoStartProvider with WindowsAutoStart;
provides KeychainAccessProvider with WindowsProtectedKeychainAccess;
provides KeychainAccessProvider with WindowsProtectedKeychainAccess, WindowsHelloKeychainAccess;
provides UiAppearanceProvider with WinUiAppearanceProvider;
provides RevealPathService with ExplorerRevealPathService;
provides QuickAccessService with ExplorerQuickAccessService;
Expand Down
16 changes: 4 additions & 12 deletions src/main/java/org/cryptomator/windows/autostart/WinShellLinks.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package org.cryptomator.windows.autostart;

import org.cryptomator.windows.common.NativeLibLoader;
import org.cryptomator.windows.common.WinStrings;

import java.nio.charset.StandardCharsets;
import java.util.Arrays;

/**
* Interface to the native Windows shell link interface.
Expand All @@ -22,9 +21,9 @@ public class WinShellLinks {
*/
public int createShortcut(String target, String storagePath, String description) {
return Native.INSTANCE.createShortcut(
getNullTerminatedUTF16Representation(target),
getNullTerminatedUTF16Representation(storagePath),
getNullTerminatedUTF16Representation(description)
WinStrings.getNullTerminatedUTF16Representation(target),
WinStrings.getNullTerminatedUTF16Representation(storagePath),
WinStrings.getNullTerminatedUTF16Representation(description)
infeo marked this conversation as resolved.
Show resolved Hide resolved
);
}

Expand All @@ -36,13 +35,6 @@ public int createShortcut(String target, String storagePath, String description)
public String getPathToStartupFolder() {
return Native.INSTANCE.createAndGetStartupFolderPath();
}

// visible for testing
byte[] getNullTerminatedUTF16Representation(String source) {
byte[] bytes = source.getBytes(StandardCharsets.UTF_16LE);
return Arrays.copyOf(bytes, bytes.length + 2); // add double-width null terminator 0x00 0x00
}

private static class Native {
static final Native INSTANCE = new Native();

Expand Down
14 changes: 14 additions & 0 deletions src/main/java/org/cryptomator/windows/common/WinStrings.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.cryptomator.windows.common;

import java.nio.charset.StandardCharsets;
import java.util.Arrays;

public class WinStrings {

// visible for testing
public static byte[] getNullTerminatedUTF16Representation(String source) {
byte[] bytes = source.getBytes(StandardCharsets.UTF_16LE);
return Arrays.copyOf(bytes, bytes.length + 2); // add double-width null terminator 0x00 0x00
}
infeo marked this conversation as resolved.
Show resolved Hide resolved

}
166 changes: 166 additions & 0 deletions src/main/java/org/cryptomator/windows/keychain/FileKeychain.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package org.cryptomator.windows.keychain;

import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.cryptomator.integrations.keychain.KeychainAccessException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Reader;
import java.io.Writer;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;

import static java.nio.charset.StandardCharsets.UTF_8;

/**
* A file-based keychain. It's content is a utf-8 encoded JSON object.
*/
class FileKeychain implements WindowsKeychainAccessBase.Keychain {

private final static Logger LOG = LoggerFactory.getLogger(FileKeychain.class);
private static final ObjectMapper JSON_MAPPER = new ObjectMapper();

private final List<Path> keychainPaths;

private Map<String, KeychainEntry> cache;
private volatile boolean loaded;

FileKeychain(String keychainPathsProperty) {
keychainPaths = parsePaths(System.getProperty(keychainPathsProperty, ""), System.getProperty("path.separator"));
cache = new ConcurrentHashMap<>();
}

//testing
FileKeychain(List<Path> paths) {
keychainPaths = paths;
cache = new ConcurrentHashMap<>();
}

synchronized void load() throws KeychainAccessException {
if (!loaded) {
loadInternal();
loaded = true;
}
}

//for testing
void loadInternal() throws KeychainAccessException {
if (keychainPaths.isEmpty()) {
throw new KeychainAccessException("No path specified to store keychain");
}
//Note: We are trying out all keychainPaths to see, if we have to migrate an old keychain file to a new location
boolean useExisting = false;
for (Path keychainPath : keychainPaths) {
Optional<Map<String, KeychainEntry>> maybeKeychain = parse(keychainPath);
if (maybeKeychain.isPresent()) {
cache = maybeKeychain.get();
useExisting = true;
break;
}
}
if (!useExisting) {
LOG.debug("Keychain file not found or not parsable. Using new keychain.");
}

}

//visible for testing
Optional<Map<String, KeychainEntry>> parse(Path keychainPath) throws KeychainAccessException {
LOG.debug("Loading keychain from {}", keychainPath);
TypeReference<Map<String, KeychainEntry>> type = new TypeReference<>() {
};
try (InputStream in = Files.newInputStream(keychainPath, StandardOpenOption.READ); //
Reader reader = new InputStreamReader(in, UTF_8)) {
return Optional.ofNullable(JSON_MAPPER.readValue(reader, type));
} catch (NoSuchFileException e) {
return Optional.empty();
} catch (JacksonException je) {
LOG.warn("Ignoring existing keychain file {}: Parsing failed.", keychainPath);
return Optional.empty();
} catch (IOException e) {
//TODO: we could ignore this
throw new KeychainAccessException("Failed to read keychain from path " + keychainPath, e);
}
}

//visible for testing
synchronized void save() throws KeychainAccessException {
var keychainFile = keychainPaths.getFirst(); //Note: we are always storing the keychain to the first entry to use the 'newest' keychain path and thus migrate old data
LOG.debug("Writing keychain to {}", keychainFile);
try (OutputStream out = Files.newOutputStream(keychainFile, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); //
Writer writer = new OutputStreamWriter(out, UTF_8)) {
JSON_MAPPER.writeValue(writer, cache);
} catch (IOException e) {
throw new KeychainAccessException("Could not write keychain to path " + keychainFile, e);
}
}

static List<Path> parsePaths(String listOfPaths, String pathSeparator) {
return Arrays.stream(listOfPaths.split(pathSeparator))
.filter(Predicate.not(String::isEmpty))
.map(s -> {
try {
return Path.of(s);
} catch (InvalidPathException e) {
LOG.info("Ignoring string {} for keychain file path: Cannot be converted to a path.", s);
return null;
}
})
.filter(Objects::nonNull)
.map(Util::resolveHomeDir)
.toList();
}

@Override
public KeychainEntry put(String id, KeychainEntry value) throws KeychainAccessException {
load();
var result = cache.put(id, value);
save();
return result;
}

@Override
public KeychainEntry get(String id) throws KeychainAccessException {
load();
return cache.get(id);
}

@Override
public KeychainEntry remove(String id) throws KeychainAccessException {
load();
var result = cache.remove(id);
save();
return result;
}

@Override
public KeychainEntry change(String id, KeychainEntry newEntry) throws KeychainAccessException {
load();
var result = cache.computeIfPresent(id, (_, _) -> newEntry);
save();
return result;
}
infeo marked this conversation as resolved.
Show resolved Hide resolved

@Override
public boolean isSupported() {
//TODO: actually, we would like the location to be writable as well
return !keychainPaths.isEmpty();
}
infeo marked this conversation as resolved.
Show resolved Hide resolved
}
51 changes: 51 additions & 0 deletions src/main/java/org/cryptomator/windows/keychain/Util.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package org.cryptomator.windows.keychain;

import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.cryptomator.integrations.keychain.KeychainAccessException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Reader;
import java.io.Writer;
import java.nio.ByteBuffer;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;

import static java.nio.charset.StandardCharsets.UTF_8;

public class Util {
private static final Path USER_HOME_REL = Path.of("~");
private static final Path USER_HOME = Path.of(System.getProperty("user.home"));

static Path resolveHomeDir(Path path) {
if (path.startsWith(USER_HOME_REL)) {
return USER_HOME.resolve(USER_HOME_REL.relativize(path));
} else {
return path;
}
}
Comment on lines +34 to +40
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure proper expansion of '~' in path resolution

The current implementation uses path.startsWith(USER_HOME_REL), where USER_HOME_REL is Path.of("~"). However, the Path class in Java does not interpret the tilde ~ as the home directory. As a result, this check may not work as intended, and paths starting with ~ may not be correctly resolved to the user's home directory.

To correctly expand paths starting with ~, consider checking the first path segment as a string and performing the substitution accordingly.

Apply this diff to fix the path resolution:

 static Path resolveHomeDir(Path path) {
-    if (path.startsWith(USER_HOME_REL)) {
-        return USER_HOME.resolve(USER_HOME_REL.relativize(path));
+    if (path.getNameCount() > 0 && path.getName(0).toString().equals("~")) {
+        return USER_HOME.resolve(path.subpath(1, path.getNameCount()));
     } else {
         return path;
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static Path resolveHomeDir(Path path) {
if (path.startsWith(USER_HOME_REL)) {
return USER_HOME.resolve(USER_HOME_REL.relativize(path));
} else {
return path;
}
}
static Path resolveHomeDir(Path path) {
if (path.getNameCount() > 0 && path.getName(0).toString().equals("~")) {
return USER_HOME.resolve(path.subpath(1, path.getNameCount()));
} else {
return path;
}
}


static byte[] generateSalt() {
byte[] result = new byte[2 * Long.BYTES];
UUID uuid = UUID.randomUUID();
ByteBuffer buf = ByteBuffer.wrap(result);
buf.putLong(uuid.getMostSignificantBits());
buf.putLong(uuid.getLeastSignificantBits());
return result;
}

}
Loading