Catfriend1 / syncthing-android

Syncthing-Fork - A Syncthing Wrapper for Android.
Mozilla Public License 2.0
1.13k stars 40 forks source link

Android Q - SyncthingNative can't see files in external storage #457

Closed bungabunga closed 4 years ago

bungabunga commented 4 years ago

Description of the issue

I've switched from Syncthing Android to Syncthing Android Fork when I upgraded my device to Android Q beta 3, because Syncthing always crashed at start. It worked well until Andoid Q beta 5 when I (for some other reason) reinstalled the app. From that moment on I could only sync files that are located in /storage/emulated/0/Android/data/com.github.catfriend1.syncthingandroid folder and no other folder although I granted Syncthing Fork rights to access folders like DCIM, Music, Download, Documents, Pictures... It is said that those folders can only act like "send" folders because of the my OS version which is fine with me but the files don't get synced any way, up or down. On my computer I have those folders labeled as "receive" so it should work. I know this has something to do with the new Android Q's Scoped storage, but I think the sync should work at least one way (phone to computer).

Another interesting observation: Syncthing Fork is also unable to access Syncthing (Fork) backed up files. On Beta 3 it still could access them but not on 5.

Please help!

Version Information

App Version: 1.2.0.8
Syncthing Version: v1.2.0
Android Version: Android 10 beta 5
Device manufacturer: Google
Device model: Pixel 3
Catfriend1 commented 4 years ago

Hi,

Thanks for reporting the issue.

It will need more time to have a device like yours and set up the test scenario to look into this.

For a first reading, I'd recommend looking into : https://github.com/syncthing/syncthing-android/issues/1008

Especially the latest comments about scoped storage. Not claiming this to be true, but the only way at a first glance seems to raise voice at google like others already did and try to convince them optimizing scoped storage more for file manager apps.

... To be continued.

Catfriend1 commented 4 years ago

Btw.: I would appreciate your help ... (1) grabbing some verbose logs (2) checking out if the github ".debug" version which uses other folder names works better or has the same problem on your device.

Catfriend1 commented 4 years ago

According to my research this is behavior google wants. Source: https://developer.android.com/preview/privacy/scoped-storage

About the platform

Android Developers
Platform
Releases
Android Q privacy change: Scoped storage

As of Android Q Beta 4, apps that target Android 9 (API level 28) or lower see no change, by default, to how storage works from previous Android versions. As you update your existing app to work with scoped storage, you can use the new requestLegacyExternalStorage manifest attribute to enable the new behavior for your app on Android Q devices, even if your app is targeting Android 9 or lower.

To give users more control over their files and to limit file clutter, Android Q changes how apps can access files on the device's external storage, such as the files stored at the path /sdcard. Android Q continues to use the READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE permissions, which correspond to the Storage user-facing runtime permission. However, apps targeting Android Q by default (as well as apps that opt into the change) are given a filtered view into external storage. Such apps can see only their app-specific directory and specific types of media, so the apps don't need to request any additional user permissions.

Note: The permissions specific to media collections that were introduced in earlier beta releases—READ_MEDIA_IMAGES, READ_MEDIA_AUDIO, and READ_MEDIA_VIDEO—are now obsolete.
This guide describes the files included in the filtered view, as well as how to update your app so that it can continue to share, access, and modify files that are saved on an external storage device. This guide also explains several considerations related to location information in photographs, media access from native code, and use of column names in content queries.

To learn more about changes to external storage in Android Q, see the section that discusses Improvements in creating files on external storage.

Note: If you encounter issues with using this feature, send us a bug report using this hotlist.

Filtered view into external storage
Note: This view of files into external storage was referred to as a sandboxed view in earlier beta versions.
By default, if your app targets Android Q, it has a filtered view of the files that are on an external storage device. The app can store files intended for itself under an app-specific directory using Context.getExternalFilesDir().

An app that has a filtered view always has read/write access to the files that it creates, both inside and outside its app-specific directory. Your app doesn't need to declare any storage permissions to access these files.

Your app can access files that other apps have created only if both of the following conditions are true:

Your app has been granted the READ_EXTERNAL_STORAGE permission.
The files reside in one of the following well-defined media collections:

Photos, which are stored in MediaStore.Images.
Videos, which are stored in MediaStore.Video.
Music files, which are stored in MediaStore.Audio.
In order to access any other file that another app has created, including files in a "downloads" directory, your app must use the Storage Access Framework, which allows the user to select a specific file.

Note: Apps that have a filtered view into external storage don't have direct kernel access to paths like /sdcard/DCIM/IMG1024.JPG. To access such a file, apps must use the MediaStore, calling methods like openFile().
The filtered view also imposes the following media-related data restrictions:

The Exif metadata within image files is redacted, unless your app has been granted the ACCESS_MEDIA_LOCATION permission. Learn more in the section about how to access location information in pictures.
The DATA column is redacted for each file in the media store.
The MediaStore.Files table is itself filtered, showing only photos, videos, and audio files. For example, this table no longer shows PDF files.
To access media files in native code, retrieve the file using MediaStore in your Java-based or Kotlin-based code, then pass the corresponding file descriptor into your native code. For more information, see the section on how to access media files from native code.
Catfriend1 commented 4 years ago

Sorry, I cannot fix this from the java side. Google says, SyncthingNative won't have direct kernel access in Android Q (final) and needs to do everything from Java. But: this is just a wrapper and there is no Syncthing-Java (upstream).

Somehow it's looking bad from my perspective. If Google kills the Syncthing on Android Q, then I'll personally stick with older phones. Every Update causes trouble.

Hey ho let's go, sign the petition?! https://www.change.org/p/google-inc-stop-google-from-implementing-scoped-storage-in-android

Catfriend1 commented 4 years ago

I guess the answer is no, but does this path work?

/storage/emulated/0/Android/media/com.github.catfriend1.syncthingandroid

Catfriend1 commented 4 years ago

https://issuetracker.google.com/issues/128591846

The biggest ticket

bungabunga commented 4 years ago

hey!

i don't understand, it says that for apps targeting android 9 or less everything stays the same as before (for now). and Syncthing is targeting android 9 (api 28). so it should work as before?

Btw.: I would appreciate your help ... (1) grabbing some verbose logs (2) checking out if the github ".debug" version which uses other folder names works better or has the same problem on your device.

this is the normail log. i just enabled verbosed log, if you need it i can send it to you later.

i am not comfortable to use debugged apps. i try to keep my system solid security/privacy wise.

I guess the answer is no, but does this path work?

/storage/emulated/0/Android/media/com.github.catfriend1.syncthingandroid

no, this path actually works!!! in both directions. i just tried...:)

Sorry, I cannot fix this from the java side. Google says, SyncthingNative won't have direct kernel access in Android Q (final) and needs to do everything from Java. But: this is just a wrapper and there is no Syncthing-Java (upstream).

so this can not be fixed on a mother Syncthing level?

Somehow it's looking bad from my perspective. If Google kills the Syncthing on Android Q, then I'll personally stick with older phones. Every Update causes trouble.

Hey ho let's go, sign the petition?! https://www.change.org/p/google-inc-stop-google-from-implementing-scoped-storage-in-android

i think this is not a smart solution. i've read a lot about these complains recently. android has been for years known as a very unsecure system, especially compared to iOS. google (whatever we think of that corporation) has been recently trying very hard to fix the system to be more secure (sandboxes, kernel hardening, etc). direct kernel access is unfortunately not a very safe practice. if a good app can have it, bad apps (privacy wise) like WhatsApp or even evil malicious apps can have it too. so i don't think signing petitions helps here. i think devs sometimes act to egoistically, wanting everything for their own apps, but not seeing the whole picture.

i think the answer is probably following google's suggestions how to solve these issues. but as a non developer i don't really see what this means for Syncthing Android.

Catfriend1 commented 4 years ago

So with this it works again correctly for you?

/storage/emulated/0/Android/media/com.github.catfriend1.syncthingandroid no, this path actually works!!! in both directions. i just tried...:)

Catfriend1 commented 4 years ago

The Github APK ends in ".debug" but is (from code) the same as the releases on f-droid / gplay with the same number.

Catfriend1 commented 4 years ago

Your log reads "open: permission denied" so we are surely talking about scoped storage problems. (Just to confirm)

Catfriend1 commented 4 years ago

so this can not be fixed on a mother Syncthing level?

It "could" - you'll neer to ask on the Syncthing Forum and "get" a willing dev to invest hours of work. Really. (Before opening a topic, please read the discussions already existing, the conclusions are still up2date... This has been discussed and explained VERY often.)

bungabunga commented 4 years ago

So with this it works again correctly for you?

/storage/emulated/0/Android/media/com.github.catfriend1.syncthingandroid no, this path actually works!!! in both directions. i just tried...:)

yes, it works. i can't say it works "again" because i haven't tryed to use this location previously.

Catfriend1 commented 4 years ago

Security is always vs. usabilty. That's normal with Advantage and disadvantage. I'm basically not against Security ... (Lets spare discussion about google here..)

bungabunga commented 4 years ago

Security is always vs. usabilty. That's normal with Advantage and disadvantage. I'm basically not against Security ... (Lets spare discussion about google here..)

i see scoped storage as a good thing because previously i had to use apps like Shelter (or Island) to make another - "work" - android profile, to install commercial apps like WhatsApp, Viber or banking apps there, just to prevent them reading files on my device that doesn't concern them. i think allowing/disallowing apps to check storage myself is a big privacy win.

Catfriend1 commented 4 years ago

This is not a Security feature:

Now: STA requests storage permission and can read files from java and native process. Future: STA requests scoped storage permission and can only read files from java and native cannot read any file.

If this is about Security: native and java process should be able to do the same upon which permissions the user granted. Sorry,I'm fed up with this and the solution is printed above.

Catfriend1 commented 4 years ago

https://commonsware.com/blog/2019/03/28/death-external-storage-why.html

Catfriend1 commented 4 years ago

@bungabunga You can try when the next release is up if that helps you with your scoped storage problem on Q beta 5 but I honestly doubt it.

bungabunga commented 4 years ago

that was quick, thanks! will report back.

Catfriend1 commented 4 years ago

Currently building 1.2.1.1 on top of Android Q...

Catfriend1 commented 4 years ago

Check out this please: https://github.com/Catfriend1/syncthing-android/releases/tag/rc1.2.1.1 Would love your report if it makes the situation [no, little, big] better.

bungabunga commented 4 years ago

what's the difference between the two versions?

sadly i use the f-droid version and for the obvious reasons can not backup/restore my settings anymore. will reinstall anyway and report back.

is it possible to make backup folder manualy selected by user?

Catfriend1 commented 4 years ago

Currently, there is no backup folder path selection. (This was a FR before but no one did it yet.)

Sorry, a new release is up as I made a build mistake: https://github.com/Catfriend1/syncthing-android/releases/tag/rc1.2.1.2

For the release versions. please see the wiki at: https://github.com/Catfriend1/syncthing-android/wiki/Switch-between-releases_Verify-APK-is-genuine

They are all built from the same code. One is built with a debug signature (allowing more testing possibilities for debugging issues) - GitHub. One is built to guarantee it's corresponding to the original source - F-Droid. One is built with a real signature in "gradle release mode" - G-Play.

From user perspective, they all do the same as the version number is the same.

bungabunga commented 4 years ago

installing 1.2.1.2 brings "Failed to create configuration. Check logcat output"

can't publish the log right now because i don't have time right now to inspect and redact it. will do that later...

Catfriend1 commented 4 years ago

Very interesting... I've tested it on my android 9 phone. Gladly it's just a gplay beta and not released yet. Let's wait for the logs.

bungabunga commented 4 years ago

hey, here's the log!

Catfriend1 commented 4 years ago

Hmm thats unexpected. Let's wait for the final 1.2.1 to come up upstream, if that crashes too the log should go to the Syncthing (core) devs.

Catfriend1 commented 4 years ago

So 1.2.1 is there (upstream) but I currently have no time to build ... will do it when it fits into my day :)

Catfriend1 commented 4 years ago

I've done a new build upon SyncthingNative 1.2.2-rc.2. Results:

Catfriend1 commented 4 years ago

Let's take this to the forum that SyncthingNative crashes on creating a fresh config. Link: https://forum.syncthing.net/t/problem-creating-configuration-on-android-q/13648

``` Hi, I'm currently trying to build SyncthingNative v1.2.2-rc.2 for Android Q with the legacy storage handling enabled by AndroidManifest.xml. I've done multiple attempts, first one using Go 1.12.1 and NDK r19c - second one using Go1.13beta1 and NDK r20. Some time ago I tried Go 1.12.1 with v1.2.1-rc.5 and NDK r19c. All those builds succeeded and I got "libSyncthing.so" for my APK. No matter which combination of build tools/Sdk I try, I end up in the attached go crash. EDIT: To avoid misunderstanding, upgrading an existing Syncthing instance WORKS on Android 9 and Q. SyncthingNative starts and does it’s job. Just creating a NEW config after fresh app installation is what DOES NOT work. After the crash, the private app's directory was populated with those files, so I think this may be a go code issue (and not a write access restriction). ls -a -l /data/data/com.github.catfriend1.syncthingandroid/files total 40 drwx------ 2 u0_a135 u0_a135 4096 2019-08-15 12:19 . drwx------ 6 u0_a135 u0_a135 4096 2019-08-15 12:12 .. -rw------- 1 u0_a135 u0_a135 615 2019-08-15 12:12 cert.pem -rw------- 1 u0_a135 u0_a135 288 2019-08-15 12:12 key.pem -rw-rw---- 1 u0_a135 u0_a135 265 2019-08-15 12:19 share_history.xml Unfortunately, the log is not written to the specified place, the folder [/storage/emulated/0/Android/data/com.github.catfriend1.syncthingandroid/files] is empty. I've also tried Android Pie to ensure this place is write-able but got no log at all. @imsodin: It says "nil pointer deref" in FolderConfiguration.go , any clue to this if there could be a code issue when syncthing creates a new config on Android? The crash doesn't occur on other platforms, I've tested Win10 for example. Go crash from adb logcat: 12:12:46I/FirstStartActivity User granted WRITE_EXTERNAL_STORAGE permission. 12:12:56I/FirstStartActivity User granted ACCESS_COARSE_LOCATION permission. 12:12:57I/ConfigXml (Re)Generating keys and config. 12:12:57 INFO: Device ID: 2Y4HM5Z-WTV2JFP-M43K2P6-N6PYJ7X-26WKCMD-5HZHSWL-326SQKP-YCOCUAA 12:12:57W/libsyncthing.so type=1400 audit(0.0:6847): avc: denied { read } for name="somaxconn" dev="proc" ino=114442 scontext=u:r:untrusted_app:s0:c135,c256,c512,c768 tcontext=u:object_r:proc_net:s0 tclass=file permissive=0 app=com.github.catfriend1.syncthingandroid 12:12:58E/Go panic: runtime error: invalid memory address or nil pointer dereference 12:12:58E/Go [signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x5fd34436] 12:12:58E/Go 12:12:58E/Go goroutine 1 [running]: 12:12:58E/Go github.com/syncthing/syncthing/lib/config.(*FolderConfiguration).prepare(0x628bc0b0) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/config/folderconfiguration.go:253 +0x206 12:12:58E/Go github.com/syncthing/syncthing/lib/config.NewFolderConfiguration(0x777638d6, 0x92749d36, 0x9f569bb3, 0x4ff8f9e6, 0x4c28ebf5, 0xca934f1f, 0x5a5d7db, 0xa870023f, 0x5ff844ee, 0x7, ...) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/config/folderconfiguration.go:86 +0x12f 12:12:58E/Go github.com/syncthing/syncthing/lib/syncthing.DefaultConfig(0x62a8bc20, 0x44, 0x777638d6, 0x92749d36, 0x9f569bb3, 0x4ff8f9e6, 0x4c28ebf5, 0xca934f1f, 0x5a5d7db, 0xa870023f, ...) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/syncthing/utils.go:53 +0x2d7 12:12:58E/Go main.generate(0xfffe9811, 0x39, 0x0, 0x0) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/cmd/syncthing/main.go:440 +0x452 12:12:58E/Go main.main() 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/cmd/syncthing/main.go:347 +0x23e 12:12:58E/Go 12:12:58E/Go goroutine 7 [syscall]: 12:12:58E/Go os/signal.signal_recv(0x0) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/go/src/runtime/sigqueue.go:139 +0x157 12:12:58E/Go os/signal.loop() 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/go/src/os/signal/signal_unix.go:23 +0x1b 12:12:58E/Go created by os/signal.init.0 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/go/src/os/signal/signal_unix.go:29 +0x3d 12:12:58E/Go 12:12:58E/Go goroutine 8 [chan receive]: 12:12:58E/Go github.com/syncthing/notify.(*nonrecursiveTree).dispatch(0x6286b2c0, 0x6286b240) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/pkg/mod/github.com/syncthing/notify@v0.0.0-20190709140112-69c7a957d3e2/tree_nonrecursive.go:36 +0xae 12:12:58E/Go created by github.com/syncthing/notify.newNonrecursiveTree 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/pkg/mod/github.com/syncthing/notify@v0.0.0-20190709140112-69c7a957d3e2/tree_nonrecursive.go:29 +0xc5 12:12:58E/Go 12:12:58E/Go goroutine 9 [chan receive]: 12:12:58E/Go github.com/syncthing/notify.(*nonrecursiveTree).internal(0x6286b2c0, 0x6286b280) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/pkg/mod/github.com/syncthing/notify@v0.0.0-20190709140112-69c7a957d3e2/tree_nonrecursive.go:81 +0x41 12:12:58E/Go created by github.com/syncthing/notify.newNonrecursiveTree 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/pkg/mod/github.com/syncthing/notify@v0.0.0-20190709140112-69c7a957d3e2/tree_nonrecursive.go:30 +0xf0 12:12:58E/Go 12:12:58E/Go goroutine 10 [sleep]: 12:12:58E/Go runtime.goparkunlock(...) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/go/src/runtime/proc.go:310 12:12:58E/Go time.Sleep(0x1dcd6500, 0x0) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/go/src/runtime/time.go:105 +0x17a 12:12:58E/Go github.com/syncthing/syncthing/lib/dialer.init.0.func2() 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/dialer/internal.go:55 +0x2a 12:12:58E/Go created by github.com/syncthing/syncthing/lib/dialer.init.0 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/dialer/internal.go:54 +0x282 12:12:58E/Go 12:12:58E/Go goroutine 12 [select]: 12:12:58E/Go github.com/syncthing/syncthing/lib/events.(*Logger).Serve(0x628744b0) 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/events/events.go:260 +0xdf 12:12:58E/Go created by github.com/syncthing/syncthing/lib/events.init.1 12:12:58E/Go F:/GitHub/syncthing-android/syncthing/src/github.com/syncthing/syncthing/lib/events/events.go:234 +0x5e 12:12:58W/SyncthingRunnable exit reason = exitNoUpgradeAvailable. ```
Catfriend1 commented 4 years ago

The crash you experience when generating a fresh config on the intro screen is fixed by:

Catfriend1 commented 4 years ago

@bungabunga Hi,

please try the recent release https://github.com/Catfriend1/syncthing-android/releases/tag/rc1.2.2.0 and tell me how it goes. The startup crash should be fixed but I don't know how external storage behaves on a real Android Q beta device yet. Would love your feedback and if applicable logs about this. Thanks for your patience.

Kind regards Catfriend1

bungabunga commented 4 years ago

good news: not only crash has gone but i just managed to sync a test folder on storage/emulated/0/ without any problems. sending and receiving possible. 🎉🎉🎉

Catfriend1 commented 4 years ago

Thanks for testing it :-). I think this is temporary as Google might change this behaviour any time in the future ... But for now , lets take it.

bungabunga commented 4 years ago

Thank you for your persistence!

noodle- commented 4 years ago

@Catfriend1 I don't know where else to post this so I'll just add it to this issue as it's slightly related.

I'm not sure if I'm just misunderstanding this but AFAIK The statement you included in the changelog is poorly worded and/or even simply false.

"PLEASE NOTE: Android 10's Scoped Storage can or will render this app unusable in the future. This cannot be solved by devs, check wiki."

I think this should say something like: "Android 10's Scoped Storage can or will prevent this app from being able to work with any kind of external storage like sd-cards. This cannot be solved by devs, check the wiki. Syncing folders on internal storage will still work with Android 10." Just so other people (like me) don't get a heart attack :)

I hope I'm right in this assumption, and as always thank you for all the work you put into this project. ❤️

Catfriend1 commented 4 years ago

Sorry, but imsodin and me already wrote about this after checking the google docs , if Google fully enrolls the announced concept syncthing will not be able to access any kind of storage anymore as a native process. So you are wrong sorry.

noodle- commented 4 years ago

Oh, well... damn.

Thanks for the information!

EDIT: btw maybe pinning this issue will make it easier to find

Catfriend1 commented 4 years ago

If many people ask and I get too tired of linking here thats definitely a good idea ;-). @noodle-