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

Error opening CryFS volume on USB drive #281

Open
intergalacticmonkey opened this issue Apr 19, 2024 · 20 comments
Open

Error opening CryFS volume on USB drive #281

intergalacticmonkey opened this issue Apr 19, 2024 · 20 comments

Comments

@intergalacticmonkey
Copy link
Contributor

intergalacticmonkey commented Apr 19, 2024

When I try to open a CryFS volume on a USB drive I get an error "Open failed, Unknown error code: 0". I attached the logcat and a screenshot of the error. gocryptfs volumes work fine.


droidfs_logcat.txt

If it makes any difference, I'm running GrapheneOS, but I enabled "exploit protection compatibility mode" for DroidFS so it shouldn't be running any differently than on stock Android.

  • DroidFS 2.1.3 (F-Droid)
  • GrapheneOS 2024040900
  • Google Pixel 7 Pro
@hardcore-sushi
Copy link
Owner

According to the logs, DroidFS gets a permission denied error when trying to access /mnt/media_rw. What are the paths displayed for working gocryptfs volumes?

@intergalacticmonkey
Copy link
Contributor Author

intergalacticmonkey commented Apr 20, 2024

gocryptfs volumes on USB drives use the same /mnt/media_rw path:


Also, I should've probably mentioned this before, but CryFS volumes work fine on internal storage.

@intergalacticmonkey
Copy link
Contributor Author

Getting this error when trying to build the app to test potential fixes :(

$ ./gradlew assembleRelease
Calculating task graph as no configuration cache is available for tasks: assembleRelease

FAILURE: Build failed with an exception.

* Where:
Build file '/home/ubuntu/DroidFS/app/build.gradle' line: 1

* What went wrong:
A problem occurred evaluating project ':app'.
> Failed to apply plugin 'com.android.internal.application'.
   > Android Gradle plugin requires Java 17 to run. You are currently using Java 11.
      Your current JDK is located in /usr/lib/jvm/java-11-openjdk-amd64
      You can try some of the following options:
       - changing the IDE settings.
       - changing the JAVA_HOME environment variable.
       - changing `org.gradle.java.home` in `gradle.properties`.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

BUILD FAILED in 779ms
Configuration cache entry stored.

@intergalacticmonkey
Copy link
Contributor Author

intergalacticmonkey commented Apr 20, 2024

Submitted a tiny PR to fix a build error I was having: #282

But now I'm having this and I'm not sure how to fix it. It's a long one so I've attached the full error as a .txt file.

droidfs_build_errors.txt

Edit: I see you recommend using NDK r23 in the build docs. Trying to force it to use that instead of r26.

@hardcore-sushi
Copy link
Owner

Did you create the CryFS volume with DroidFS or with the desktop cryfs program?
Does something differ if you move the volume in a sub-directory on your drive?

What fixes are you trying?

From your build logs, I see one error that should be fixed by us, but as a temporary workaround, using a previous NDK version may work. However, there's also:

boost_1_77_0.tar.bz2: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match

Which is strange. Can you check that the archive file at app/libcryfs/vendor/boost/Boost-for-Android/boost_1_77_0.tar.bz2 seems valid?

@intergalacticmonkey
Copy link
Contributor Author

Did you create the CryFS volume with DroidFS or with the desktop cryfs program?

DroidFS.

Does something differ if you move the volume in a sub-directory on your drive?

Nope.

What fixes are you trying?

Don't have anything specific in mind, just wanted to experiment with it. I thought maybe there's a tiny chance that just directly asking for permission to access the USB drive might work (even though this app accesses the USB through the filesystem and not as a block device, and the "All files access" permission is intended to also grant access to USB storage through the filesystem).

@intergalacticmonkey
Copy link
Contributor Author

Which is strange. Can you check that the archive file at app/libcryfs/vendor/boost/Boost-for-Android/boost_1_77_0.tar.bz2 seems valid?

Indeed, the sha256 of the file didn't match the expected hash. I re-downloaded it. Weird.

@intergalacticmonkey
Copy link
Contributor Author

Went ahead and spun this off into a new issue since it was polluting this one.

@intergalacticmonkey
Copy link
Contributor Author

Got some more details to share: I tried opening a CryFS volume off a USB drive on my Pixel's stock ROM just to make sure it wasn't a GrapheneOS issue. Still observed the same behavior.

@hardcore-sushi Does it work on your phone?

@hardcore-sushi
Copy link
Owner

Unfortunately, I have no USB-OTG adapter to test with.

The error seems to come from a symbolic link resolution. What's the filesystem of your USB drive?

@intergalacticmonkey
Copy link
Contributor Author

What's the filesystem of your USB drive?

I tried both exFAT and FAT32.

@intergalacticmonkey
Copy link
Contributor Author

I think I fixed it!

Basically Android throws a hissy fit when you try to call stat() on /mnt/media_rw. Calling stat() on anything below that, so long as you have the "all files" permission, is fine. It's being called because some code somewhere is calling boost::filesystem::symlink_status() on /mnt/media_rw, which returns whether or not that path is a symlink, and uses stat() to determine this.

I'm not sure where the proper place to fix this is, however. Not even sure where this function is called. I'd like to use a debugger to generate a stack trace but I don't know how to do that on Android. "ndk-gdb" seems to not be a thing anymore.

For now, here's a minimal proof-of-concept patch that fixes the issue by simply hardcoding a return value of "no, this is not a symlink" any time symlink_status() is called on /mnt/media_rw. Basically just this:

BOOST_FILESYSTEM_DECL
file_status symlink_status(path const& p, error_code* ec)
{
    // ...
    if (strcmp(p.c_str(), "/mnt/media_rw") == 0) {
        const mode_t mode = (mode_t)0700;
        return fs::file_status(fs::directory_file, static_cast< perms >(mode) & fs::perms_mask);
    }
    // ...
}

To test the patch, just replace app/libcryfs/vendor/boost/Boost-for-Android/patches/boost-1_77_0/boost-1.77.0.patch with the attached file and re-build.

boost-1.77.0.patch.txt

@intergalacticmonkey
Copy link
Contributor Author

Apparently Android Studio has an integrated LLDB debugger for native code. But I tried loading the project with Android Studio and it doesn't seem to like it. When trying to build I get cryptic errors.

@hardcore-sushi
Copy link
Owner

Wow, very good job!

I'll try to investigate this issue and find an appropriate fix. To get a backtrace, I remember having some success with boost stacktrace.

I tried both exFAT and FAT32.

Could you try on a filesystem supporting symbolic links such as ext4 or F2FS?

@hardcore-sushi
Copy link
Owner

Here's my findings.

The functions BasedirMetadata::filesystemIdForBasedirIsCorrect() and BasedirMetadata::updateFilesystemIdForBasedir() are calling jsonPathForBasedir(), which calls boost:filesystem::canonical(), that in turn seems to be calling boost::filesystem::symlink_status() on every part of the path. Therefore, I don't know how to fix this properly, except by removing the call to boost:filesystem::canonical() completely.

@intergalacticmonkey
Copy link
Contributor Author

intergalacticmonkey commented Apr 24, 2024

Could you try on a filesystem supporting symbolic links such as ext4 or F2FS?

Android only supports FAT32 and exFAT for removable storage (despite using ext4 internally).

Anyway, thanks for investigating this issue! It's a bit of a tricky situation I suppose. Ideally we could just intercept calls to stat() and make it function how it really should, as it doesn't make much sense that stat() on /mnt/media_rw isn't allowed in the first place, but I think the only way to do something like that would be to add a hardcoded check on every call and that may impact performance because stat() is not an uncommon call. Implementing a fix in a boost::filesystem function could theoretically pose a similar issue, though I think both canonical() and symlink_status() are only called once per session, so it's not a big deal for this particular app. But that would introduce the annoyance of having to maintain the patch even as you upgrade Boost (if this is something you plan to do).

For now, I propose a simple patch to libcryfs which adds a check for /mnt/media_rw into jsonPathForBasedir() for Android builds:

--- BasedirMetadata.cpp.orig
+++ BasedirMetadata.cpp
@@ -45,6 +45,19 @@
 }
 
 string jsonPathForBasedir(const bf::path &basedir) {
+#ifdef __ANDROID__
+  // if basedir starts with /mnt/media_rw
+  if (basedir.string().rfind("/mnt/media_rw", 0) == 0) {
+    // Android returns permission denied when attempting to stat() /mnt/media_rw for some reason
+    // (even with "all files access") so checking if /mnt/media_rw is a symlink, as canonical()
+    // would, results in an error. Because we know /mnt/media_rw is not a symlink, and Android
+    // doesn't support removable storage filesystems that support symlinks, the canonical path
+    // for any path starting with /mnt/media_rw is equal to the absolute path after normalization
+    // (removal of "../" and "./" elements).
+    // See: https://github.com/hardcore-sushi/DroidFS/issues/281
+    return bf::absolute(basedir).lexically_normal().string() + ".filesystemId";
+  }
+#endif
   return bf::canonical(basedir).string() + ".filesystemId";
 }
 

If Android gains support for filesystems on removable storage that support symlinks in the future, as far as I can tell, the worst that could happen is, if a user manually provides a path with symlinks under /mnt/media_rw, then later specifies a different path that points to the same folder, DroidFS will not recognize it as the same path? I don't foresee that causing any problems...

You could alternatively write your own implementation of boost::filesystem::canonical() within libcryfs that checks for the special case of /mnt/media_rw (instead of patching Boost's version directly). One straightforward way to do that might be to just copy-paste the entirety of canonical() and symlink_status(), let's say as canonical2() and symlink_status2(), then make canonical2() call symlink_status2() and apply the patch I provided above to symlink_status2().

I opened a PR: hardcore-sushi/libcryfs#1

Let me know which approach you want to go with. And thanks again for the troubleshooting help and for developing this awesome app!

@hardcore-sushi
Copy link
Owner

You are correct, but I'm afraid that /mnt/media_rw is not always the path used for mounting external devices. I'm skeptical to hardcode it. Instead, I think the call to boost:filesystem::canonical() can be simply removed. I asked the CryFS developers just to be sure it's safe.

@hardcore-sushi
Copy link
Owner

Fixed upstream: cryfs/cryfs#473

I'll update libcryfs to include this patch.

@intergalacticmonkey
Copy link
Contributor Author

Awesome, thanks for the quick resolution!

@intergalacticmonkey
Copy link
Contributor Author

Feel free to close this issue whenever you want. You may want to leave it open until the next DroidFS release so users who are having this issue can find it. But it's up to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants