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

Respect user certificates #1215

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Respect user certificates #1215

wants to merge 1 commit into from

Conversation

sprig
Copy link

@sprig sprig commented Dec 14, 2024

Description

If one wants to run one’s own global discovery inside of a private network in a secure manner, the simplest solution is to use a self signed CA that is already trusted by all network devices. Unfortunately the app currently ignores user certificates from android and doesn’t provide a way to set one’s own, forcing users to use public domain names as a workaround. This is unfortunate.

Given the strong warnings android gives before installing custom CAs, it is likely that anyone with custom certificates installed "knows what they are doing" and out of those, people who would rather trust their CAs are a lot more common.

As such I propose enabling trusting of user CAs. If you think it would be straightforward, I'd be willing to attempt adding a toggle somewhere in the settings for this, if you think it's really necessary.

Changes

  • Respect user defined CAs in release builds.

If one wants to run one’s own global discovery inside of a private network in a secure manner, the simplest solution is to use a self signed CA that is already trusted by all network devices. Unfortunately the app currently ignores user certificates from android and doesn’t provide a way to set one’s own, forcing users to use public domain names as a workaround. This is unfortunate.

Given the strong warnings android gives before installing custom CAs, it is likely that anyone with custom certificates installed "knows what they are doing" and out of those, people who would rather trust their CAs are a lot more common.

As such I propose enabling trusting of user CAs. If you think it would be straightforward, I'd be willing to attempt adding a toggle somewhere in the settings for this, if you think it's really necessary.
@Catfriend1
Copy link
Owner

Thanks for the PR 🙂. Did you test this if it works on your phone? Where can I get the debug APK?

@sprig
Copy link
Author

sprig commented Dec 14, 2024

Hi! Not exactly; I was about to to write, so thanks for the prompt.

Im not an android dev but i experienced this problem (of apps not respecting certificates) in other apps, eg termux, and upon investigation this (my PR) was the established solution in sites like SO.

In termux specifically there’s a workaround of setting the certificates inside the app. Here I was not able to find a workaround aside from setting up a letsencrypt. I was about to open an issue but then thought “it’s a one line change, just do the pr”.

I did, then decided to try and build this but I got stuck(before even patching). build breaks in the step where syncthing is to be built using go. both for head commit and latest release tag. (building on an ubuntu:latest amd64 container).

If I run the failing command manually, i get an error message about cgo, and if i enable cgo as instructed by google, eventually something tries to execute some nonexistent version of llvm in the ndk directory and fails. If all that doesn’t make sense ill post the actual errors once im by the computer.

One thing that might have tripped it might have been the python version. I just went with whatever was available in apt repo. Ill also test with correct version before posting the errors.

@Catfriend1
Copy link
Owner

Catfriend1 commented Dec 15, 2024

It's maybe the ndk home or where it is installed when cgo is failing!?

I am not sure the native can profit from your android xml change. I only know from another ticket that env vars may be respected by the compiled native process.

@sprig
Copy link
Author

sprig commented Dec 16, 2024

Here is the initial error with python 3.9.6. I followed the instructions in the readme and failed to build. As far as I understand, the script downloads the NDK itself, so I see no reason for it to be of an incorrect version.

On an ubuntu 24.4.01 container:

apt-get -y install git openjdk-11-jdk python
mkdir -p /root/work


cd /root/work
git clone https://github.com/Catfriend1/syncthing-android.git --recursive
cd /root/work/syncthing-android
./gradlew buildNative

This then complains about the lack of java 17 - I install openjdk-17-jdk (hopefully I don't need specifically oracle java?). For python, install venv -> pip -> uv via pip -> python3.9.6 via uv -> setup and activate a 3.9.6 venv. Then I get the following error (debug output enabled. I can also attach without, since it seems like a lot of output).

Attempting to run the failing command directly, I get the following:

# go run build.go -goos android -goarch arm -cc /root/work/syncthing-android/syncthing/../../syncthing-android-prereq/android-ndk-r27/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi21-clang -version v1.28.1 -no-upgrade build
Notice: Next generation GUI will not be built; see --with-next-gen-gui.
runtime/cgo
# runtime/cgo
/root/work/syncthing-android/syncthing/../../syncthing-android-prereq/android-ndk-r27/toolchains/llvm/prebuilt/linux-x86_64/bin/clang: line 1: /root/work/syncthing-android/syncthing/../../syncthing-android-prereq/android-ndk-r27/toolchains/llvm/prebuilt/linux-x86_64/bin/clang-14: No such file or directory
exit status 1
exit status 1

OK. I install clang-14 via apt and symlink the executable to the expected location. We go a bit further:

# go run build.go -goos android -goarch arm -cc /root/work/syncthing-android/syncthing/../../syncthing-android-prereq/android-ndk-r27/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi21-clang -version v1.28.1 -no-upgrade build
Notice: Next generation GUI will not be built; see --with-next-gen-gui.
runtime/cgo
net
vendor/golang.org/x/net/http/httpproxy
log/syslog
golang.org/x/net/internal/socks
crypto/x509
net/textproto
github.com/google/uuid
golang.org/x/net/internal/socket
github.com/prometheus/procfs
github.com/jackpal/go-nat-pmp
github.com/rcrowley/go-metrics
golang.org/x/net/proxy
github.com/ccding/go-stun/stun
vendor/golang.org/x/net/http/httpguts
mime/multipart
golang.org/x/net/http/httpguts
golang.org/x/net/ipv4
golang.org/x/net/ipv6
github.com/syncthing/syncthing/lib/beacon
crypto/tls
net/http/httptrace
github.com/quic-go/quic-go/internal/qerr
github.com/syncthing/syncthing/lib/tlsutil
github.com/quic-go/quic-go/internal/qtls
github.com/quic-go/quic-go/internal/wire
github.com/quic-go/quic-go/internal/flowcontrol
net/http
github.com/quic-go/quic-go/logging
github.com/quic-go/quic-go/internal/congestion
github.com/quic-go/quic-go/internal/handshake
github.com/quic-go/quic-go/internal/ackhandler
github.com/quic-go/quic-go
expvar
github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header
github.com/syncthing/syncthing/lib/assets
github.com/syncthing/syncthing/lib/dialer
github.com/julienschmidt/httprouter
github.com/prometheus/common/expfmt
github.com/Azure/go-ntlmssp
golang.org/x/net/http2
github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil
github.com/syncthing/syncthing/lib/api/auto
net/http/httptest
net/http/pprof
github.com/go-ldap/ldap/v3
github.com/stretchr/testify/assert
github.com/prometheus/client_golang/prometheus
github.com/stretchr/testify/mock
github.com/prometheus/client_golang/prometheus/promauto
github.com/prometheus/client_golang/prometheus/promhttp
github.com/jackpal/gateway
github.com/syncthing/syncthing/lib/events
github.com/syncthing/syncthing/lib/protocol
github.com/syncthing/syncthing/lib/relay/protocol
github.com/syncthing/syncthing/lib/fs
github.com/syncthing/syncthing/lib/locations
github.com/syncthing/syncthing/lib/osutil
github.com/syncthing/syncthing/cmd/syncthing/cmdutil
github.com/syncthing/syncthing/lib/ignore
github.com/syncthing/syncthing/lib/relay/client
github.com/syncthing/syncthing/lib/db
github.com/syncthing/syncthing/lib/scanner
github.com/syncthing/syncthing/lib/stats
github.com/syncthing/syncthing/lib/config
github.com/syncthing/syncthing/cmd/syncthing/decrypt
github.com/syncthing/syncthing/lib/stun
github.com/syncthing/syncthing/lib/watchaggregator
github.com/syncthing/syncthing/lib/nat
github.com/syncthing/syncthing/lib/versioner
github.com/syncthing/syncthing/cmd/syncthing/cli
github.com/syncthing/syncthing/lib/discover
github.com/syncthing/syncthing/lib/upnp
github.com/syncthing/syncthing/lib/pmp
github.com/syncthing/syncthing/lib/connections
github.com/syncthing/syncthing/lib/ur
github.com/syncthing/syncthing/lib/model
github.com/syncthing/syncthing/lib/api
github.com/syncthing/syncthing/lib/syncthing
github.com/syncthing/syncthing/cmd/syncthing/generate
github.com/syncthing/syncthing/cmd/syncthing
# github.com/syncthing/syncthing/cmd/syncthing
/root/go/pkg/mod/golang.org/[email protected]/pkg/tool/linux_amd64/link: running /root/work/syncthing-android/syncthing/../../syncthing-android-prereq/android-ndk-r27/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi21-clang failed: exit status 1
clang: error: unable to execute command: posix_spawn failed: Exec format error
clang: error: linker command failed with exit code 1 (use -v to see invocation)

exit status 1
exit status 1

OK, sounds like I should have install a cross compiler instead. I don't recall one being available via apt, so let's try to link clan-18 from the same directory to clang-14 instead; Unfortunately it's the same error. Perhaps not a cross-compiler issue. Just to test I tried with CGO_ENABLED=0 and got

go run build.go -goos android -goarch arm -cc /root/work/syncthing-android/syncthing/../../syncthing-android-prereq/android-ndk-r27/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi21-clang -version v1.28.1 -no-upgrade build
Notice: Next generation GUI will not be built; see --with-next-gen-gui.
android/arm requires external (cgo) linking, but cgo is not enabled
exit status 1
exit status 1

In light of this, perhaps we could revisit the build instructions? Would you like me to submit an issue? Are you building on a specific kind of machine/OS? I noticed that the windows instructions has some extra steps. Are you building on Windows? I can try doing that.

@sprig
Copy link
Author

sprig commented Dec 16, 2024

It's maybe the ndk home or where it is installed when cgo is failing!?

I am not sure the native can profit from your android xml change. I only know from another ticket that env vars may be respected by the compiled native process.

I'm not sure I follow. Are you saying that the location where I made the changes is incorrect? I've very little experience with android so I'll happily change to a more suitable location. As far as I understand it, it is the lack of the User entitlement in the certificate configuration that causes ST to ignore certificates installed in android. If that should be set at a different place (e.g. the "app" instead of the embedded ST), id happily place the change there.

Or are you saying this kind of change is wrong in general and will not acheive desired effect? If so, would you propose a different mechanism?

Thanks!

@sprig
Copy link
Author

sprig commented Dec 16, 2024

Small update - I suddenly noticed that there are docker build instructions. Attempting to build with docker fails with a similar error. I see there have been some changes since v1.28.0.0, but, attempting to build the image fails on the prebuild step:

STEP 15/17: ADD prebuild.sh /opt/prebuild.sh
--> 5d8e9382e964
STEP 16/17: RUN /opt/prebuild.sh
Building standalone NDK - /opt/android-sdk/ndk-bundle/standalone-ndk/android-16-arm
HOST_OS=linux
HOST_EXE=
HOST_ARCH=x86_64
HOST_TAG=linux-x86_64
HOST_NUM_CPUS=8
BUILD_NUM_CPUS=16
ls: cannot access '/opt/android-sdk/ndk-bundle/../prebuilts/ndk/current/platforms': No such file or directory
ERROR: Failed to create toolchain.
Error: building at STEP "RUN /opt/prebuild.sh": while running runtime: exit status 1

I guess I'll try to build on windows unless you have any advice here.

@sprig
Copy link
Author

sprig commented Dec 18, 2024

Update; I get the same results on windows 11 with python 3.9.6 and go 1.2.3 (though i think the build script installs a local go). the python script to install android studio from the readme doesn’t work (ill post the error later). If I ignore it and instead just run the gradle commmand or the cmd script, i get stuck at the same place, same error as before.

@Catfriend1
Copy link
Owner

Catfriend1 commented Dec 21, 2024

Hi,

Openjdk 17 is fine. Where did you get the 11 from? I updated this some days ago...
Yes, building on windows here.
You can find Linux build instructions here (bottom of the file): https://gitlab.com/fdroid/fdroiddata/-/blob/master/metadata/com.github.catfriend1.syncthingandroid.yml
Do not disable cgo, it's the wrong way to fix the error.

Docker: currently not functional, as I have no knowledge about the github CI yet and tried porting from upstream.

It seems, one go package is affected - maybe there is sth wrong with your go build cache? Fdroid build succeeded last night, so I think there is no general problem.

The script tries to fetch ndk&go for you if it can't find it, yes.

I don't know if your change will have the desired effect. Some config comes from the android app sandbox and some is its own world as a linux native is running ("syncthing binary"). Android treats them differently, that's also the back story of sdcard access why apps can write to sdcard via android SAF but SyncthingNative can't. It may be similar in this case if certificates aren't handled by the sandbox thus ignoring your xml change. Your change may only affect the wrapper.

@sprig
Copy link
Author

sprig commented Dec 24, 2024

Hi,

Openjdk 17 is fine. Where did you get the 11 from? I updated this some days ago...

The readme :)
https://github.com/Catfriend1/syncthing-android?tab=readme-ov-file#build-on-linux

I suspected some of it may not be up to date and especially noticed that there were some changes in the dependencies and build scripts recently.

Yes, building on windows here. You can find Linux build instructions here (bottom of the file): https://gitlab.com/fdroid/fdroiddata/-/blob/master/metadata/com.github.catfriend1.syncthingandroid.yml Do not disable cgo, it's the wrong way to fix the error.

I barely read any go code ever, not really familiar with the go ecosystem. afaiu cgo is some layer of compatibility with c? Anyway, i didn’t disable it. For some reason it seems to not be enabled by default, unless i add the environment variable that forces it on. Just tried both ways to make sure. Possibly you have some private go config that is not in the repo such as eg a .env file?

Docker: currently not functional, as I have no knowledge about the github CI yet and tried porting from upstream.

I understand. The way i see it docker /is/ the official linux instructions bound in code against a specific distro.

It seems, one go package is affected - maybe there is sth wrong with your go build cache? Fdroid build succeeded last night, so I think there is no general problem.

What script is executed by fdroid builder?

The script tries to fetch ndk&go for you if it can't find it, yes.

I don't know if your change will have the desired effect. Some config comes from the android app sandbox and some is its own world as a linux native is running ("syncthing binary"). Android treats them differently, that's also the back story of sdcard access why apps can write to sdcard via android SAF but SyncthingNative can't. It may be similar in this case if certificates aren't handled by the sandbox thus ignoring your xml change. Your change may only affect the wrapper.

I understand. Kinda. I don’t know if my change is “correct”, however ST does check some specific store for root certificates. Either it takes these from android, in which case it should probably listen to the config, or some other embedded store like eg termux, which if the case, i would propose adding a small ui to add your own certificate to (in termux you just do it the normal linux way inside)

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 this pull request may close these issues.

2 participants