From 883b5abc7b2a770146683e7e27bf275bd4064511 Mon Sep 17 00:00:00 2001 From: pacien Date: Mon, 20 Jan 2020 17:07:12 +0100 Subject: pass network device fd via unix socket instead of inheritance Workaround for new shared memory restrictions added in Android 10 preventing file descriptor leakage to sub-processes. This change set BREAKS ENCRYPTED PRIVATE KEYS SUPPORT. GitHub: https://github.com/pacien/tincapp/issues/92 --- app/CMakeLists.txt | 9 ++-- app/build.gradle | 4 +- app/src/main/c/exec.c | 60 ---------------------- app/src/main/c/main.c | 1 + .../java/org/pacien/tincapp/commands/Executor.kt | 34 +----------- .../main/java/org/pacien/tincapp/commands/Tincd.kt | 15 +++--- .../main/java/org/pacien/tincapp/context/App.kt | 10 +++- .../org/pacien/tincapp/service/TincVpnService.kt | 34 +++++++++--- .../main/play/listings/en-US/full-description.txt | 2 +- readme.md | 2 +- 10 files changed, 55 insertions(+), 116 deletions(-) delete mode 100644 app/src/main/c/exec.c create mode 100644 app/src/main/c/main.c diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 073f7d7..6449aeb 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -54,9 +54,8 @@ ExternalProject_Add(libressl ExternalProject_Add(tinc DEPENDS lzo libressl - URL https://github.com/gsliepen/tinc/archive/017a7fb57655d9b1d706ee78f7e3d0000411b883.tar.gz - URL_HASH SHA256=27f361706d09f81fbbef7021f37adf5375f01857b23272a490df066ca290a530 - PATCH_COMMAND sed -i -e "s/test(void)/test(void *x)/" /m4/attribute.m4 + URL https://github.com/gsliepen/tinc/archive/6682a0d29cbb70b216a3fe02f2812963dee607d0.tar.gz + URL_HASH SHA256=ffb0e6c02b0112c095366b3baa89f5ed56cc5f644be40268696ce7741e76452a CONFIGURE_COMMAND autoreconf -fsi && /configure ${xCONFIG} --with-openssl=${CMAKE_CURRENT_BINARY_DIR}/usr/local @@ -72,5 +71,5 @@ ExternalProject_Add(tinc rm -r ) -add_library(exec SHARED src/main/c/exec.c) -add_dependencies(exec tinc) +add_library(main SHARED src/main/c/main.c) +add_dependencies(main tinc) diff --git a/app/build.gradle b/app/build.gradle index 5ef4f4a..25e449e 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -1,6 +1,6 @@ /* * Tinc App, an Android binding and user interface for the tinc mesh VPN daemon - * Copyright (C) 2017-2019 Pacien TRAN-GIRARD + * Copyright (C) 2017-2020 Pacien TRAN-GIRARD * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -35,7 +35,7 @@ android { defaultConfig { applicationId 'org.pacien.tincapp' minSdkVersion 21 - targetSdkVersion 28 // FIXME: bad file descriptor in daemon when targeting 29 + targetSdkVersion 29 multiDexEnabled true versionCode 28 versionName '0.28' diff --git a/app/src/main/c/exec.c b/app/src/main/c/exec.c deleted file mode 100644 index c335b20..0000000 --- a/app/src/main/c/exec.c +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Tinc App, an Android binding and user interface for the tinc mesh VPN daemon - * Copyright (C) 2017-2018 Pacien TRAN-GIRARD - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -#include -#include -#include -#include - -static inline const char **to_string_array(JNIEnv *env, jobjectArray ja) { - const int len = (*env)->GetArrayLength(env, ja); - const char **ca = calloc((size_t) len + 1, sizeof(char *)); - - for (int i = 0; i < len; ++i) { - jstring jstr = (jstring) (*env)->GetObjectArrayElement(env, ja, i); - ca[i] = (*env)->GetStringUTFChars(env, jstr, NULL); - } - - ca[len] = NULL; - return ca; -} - -static inline void exec(const char **argcv) { - execv(argcv[0], (char *const *) argcv); - exit(1); -} - -JNIEXPORT jint JNICALL -Java_org_pacien_tincapp_commands_Executor_forkExec(JNIEnv *env, __attribute__((unused)) jclass class, jobjectArray args) { - pid_t pid = fork(); - switch (pid) { - case 0: - exec(to_string_array(env, args)); - return 0; - - default: - return pid; - } -} - -JNIEXPORT jint JNICALL -Java_org_pacien_tincapp_commands_Executor_wait(__attribute__((unused))JNIEnv *env, __attribute__((unused)) jclass class, jint pid) { - int status; - waitpid(pid, &status, 0); - return WIFEXITED(status) ? WEXITSTATUS(status) : -1; -} diff --git a/app/src/main/c/main.c b/app/src/main/c/main.c new file mode 100644 index 0000000..68007d8 --- /dev/null +++ b/app/src/main/c/main.c @@ -0,0 +1 @@ +// This file intentionally left blank. diff --git a/app/src/main/java/org/pacien/tincapp/commands/Executor.kt b/app/src/main/java/org/pacien/tincapp/commands/Executor.kt index 29e011f..0a8a774 100644 --- a/app/src/main/java/org/pacien/tincapp/commands/Executor.kt +++ b/app/src/main/java/org/pacien/tincapp/commands/Executor.kt @@ -1,6 +1,6 @@ /* * Tinc App, an Android binding and user interface for the tinc mesh VPN daemon - * Copyright (C) 2017-2018 Pacien TRAN-GIRARD + * Copyright (C) 2017-2020 Pacien TRAN-GIRARD * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -30,42 +30,10 @@ import java.io.InputStreamReader * @author pacien */ internal object Executor { - private const val FAILED = -1 - private const val SUCCESS = 0 - class CommandExecutionException(msg: String) : Exception(msg) - init { - System.loadLibrary("exec") - } - - /** - * @return FAILED (-1) on error, forked child PID otherwise - */ - private external fun forkExec(args: Array): Int - - /** - * @return FAILED (-1) on error, the exit status of the process otherwise - */ - private external fun wait(pid: Int): Int - private fun read(stream: InputStream) = BufferedReader(InputStreamReader(stream)).readLines() - fun forkExec(cmd: Command): CompletableFuture { - val pid = forkExec(cmd.asArray()).also { - if (it == FAILED) throw CommandExecutionException("Could not fork child process.") - } - - return runAsyncTask { - val exitCode = wait(pid) - when (exitCode) { - SUCCESS -> Unit - FAILED -> throw CommandExecutionException("Process terminated abnormally.") - else -> throw CommandExecutionException("Non-zero exit status code ($exitCode).") - } - } - } - fun run(cmd: Command): Process = try { ProcessBuilder(cmd.asList()).start() } catch (e: IOException) { diff --git a/app/src/main/java/org/pacien/tincapp/commands/Tincd.kt b/app/src/main/java/org/pacien/tincapp/commands/Tincd.kt index 92be0f5..c0b0048 100644 --- a/app/src/main/java/org/pacien/tincapp/commands/Tincd.kt +++ b/app/src/main/java/org/pacien/tincapp/commands/Tincd.kt @@ -1,6 +1,6 @@ /* * Tinc App, an Android binding and user interface for the tinc mesh VPN daemon - * Copyright (C) 2017-2018 Pacien TRAN-GIRARD + * Copyright (C) 2017-2020 Pacien TRAN-GIRARD * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -18,20 +18,23 @@ package org.pacien.tincapp.commands +import java8.util.concurrent.CompletableFuture import org.pacien.tincapp.context.AppPaths +import java.io.File /** * @author pacien */ object Tincd { - fun start(netName: String, deviceFd: Int, ed25519PrivateKeyFd: Int? = null, rsaPrivateKeyFd: Int? = null) = - Executor.forkExec(Command(AppPaths.tincd().absolutePath) + fun start(netName: String, device: String, ed25519PrivateKey: File? = null, rsaPrivateKey: File? = null): CompletableFuture = + Executor.call(Command(AppPaths.tincd().absolutePath) .withOption("no-detach") .withOption("config", AppPaths.confDir(netName).absolutePath) .withOption("pidfile", AppPaths.pidFile(netName).absolutePath) .withOption("logfile", AppPaths.logFile(netName).absolutePath) .withOption("option", "DeviceType=fd") - .withOption("option", "Device=$deviceFd") - .apply { if (ed25519PrivateKeyFd != null) withOption("option", "Ed25519PrivateKeyFile=/proc/self/fd/$ed25519PrivateKeyFd") } - .apply { if (rsaPrivateKeyFd != null) withOption("option", "PrivateKeyFile=/proc/self/fd/$rsaPrivateKeyFd") }) + .withOption("option", "Device=@$device") + .apply { if (ed25519PrivateKey != null) withOption("option", "Ed25519PrivateKeyFile=${ed25519PrivateKey.absolutePath}") } + .apply { if (rsaPrivateKey != null) withOption("option", "PrivateKeyFile=${rsaPrivateKey.absolutePath}") } + ).thenApply { } } diff --git a/app/src/main/java/org/pacien/tincapp/context/App.kt b/app/src/main/java/org/pacien/tincapp/context/App.kt index a877929..4d8d5d0 100644 --- a/app/src/main/java/org/pacien/tincapp/context/App.kt +++ b/app/src/main/java/org/pacien/tincapp/context/App.kt @@ -1,6 +1,6 @@ /* * Tinc App, an Android binding and user interface for the tinc mesh VPN daemon - * Copyright (C) 2017-2019 Pacien TRAN-GIRARD + * Copyright (C) 2017-2020 Pacien TRAN-GIRARD * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -21,6 +21,7 @@ package org.pacien.tincapp.context import android.app.Application import android.content.Context import android.content.Intent +import android.content.pm.ApplicationInfo import android.net.Uri import android.os.Build import android.os.Handler @@ -47,7 +48,7 @@ class App : Application() { private fun setupCrashHandler() { val logger = LoggerFactory.getLogger(this.javaClass) - val systemCrashHandler = Thread.getDefaultUncaughtExceptionHandler() + val systemCrashHandler = Thread.getDefaultUncaughtExceptionHandler()!! val crashRecorder = CrashRecorder(logger, systemCrashHandler) Thread.setDefaultUncaughtExceptionHandler(crashRecorder) } @@ -61,6 +62,11 @@ class App : Application() { fun getContext() = appContext!! fun getResources() = getContext().resources!! + fun getApplicationInfo(): ApplicationInfo = + getContext() + .packageManager + .getApplicationInfo(BuildConfig.APPLICATION_ID, 0) + fun alert(@StringRes title: Int, msg: String, manualLink: String? = null) = notificationManager.notifyError(appContext!!.getString(title), msg, manualLink) diff --git a/app/src/main/java/org/pacien/tincapp/service/TincVpnService.kt b/app/src/main/java/org/pacien/tincapp/service/TincVpnService.kt index 40e9004..48cb1df 100644 --- a/app/src/main/java/org/pacien/tincapp/service/TincVpnService.kt +++ b/app/src/main/java/org/pacien/tincapp/service/TincVpnService.kt @@ -1,6 +1,6 @@ /* * Tinc App, an Android binding and user interface for the tinc mesh VPN daemon - * Copyright (C) 2017-2019 Pacien TRAN-GIRARD + * Copyright (C) 2017-2020 Pacien TRAN-GIRARD * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -21,6 +21,7 @@ package org.pacien.tincapp.service import android.app.Service import android.content.Context import android.content.Intent +import android.net.LocalServerSocket import android.net.VpnService import android.os.ParcelFileDescriptor import androidx.localbroadcastmanager.content.LocalBroadcastManager @@ -43,6 +44,7 @@ import org.pacien.tincapp.intent.Actions import org.pacien.tincapp.utils.TincKeyring import org.slf4j.LoggerFactory import java.io.FileNotFoundException +import java.security.AccessControlException /** * @author pacien @@ -100,6 +102,7 @@ class TincVpnService : VpnService() { log.info("Starting tinc daemon for network \"$netName\".") if (isConnected() || getCurrentNetName() != null) stopVpn().join() + // FIXME: pass decrypted private keys via temp file val privateKeys = try { TincConfiguration.fromTincConfiguration(AppPaths.existing(AppPaths.tincConfFile(netName))).let { tincCfg -> Pair( @@ -125,13 +128,10 @@ class TincVpnService : VpnService() { } val deviceFd = try { - val appContextFd = Builder().setSession(netName) + Builder().setSession(netName) .applyCfg(interfaceCfg) .also { applyIgnoringException(it::addDisallowedApplication, BuildConfig.APPLICATION_ID) } .establish()!! - val daemonContextFd = appContextFd.dup() // necessary since Android 10 - appContextFd.close() - daemonContextFd } catch (e: IllegalArgumentException) { return reportError(resources.getString(R.string.notification_error_message_network_config_invalid_format, e.defaultMessage()), e, "network-interface") } catch (e: NullPointerException) { @@ -140,10 +140,15 @@ class TincVpnService : VpnService() { return reportError(resources.getString(R.string.notification_error_message_could_not_configure_iface, e.defaultMessage()), e) } - val daemon = Tincd.start(netName, deviceFd.fd, privateKeys.first?.fd, privateKeys.second?.fd) + val serverSocket = LocalServerSocket(DEVICE_FD_ABSTRACT_SOCKET) + Executor.runAsyncTask { serveDeviceFd(serverSocket, deviceFd) } + + // FIXME: pass decrypted private keys via temp file + val daemon = Tincd.start(netName, DEVICE_FD_ABSTRACT_SOCKET, null, null) setState(netName, passphrase, interfaceCfg, deviceFd, daemon) waitForDaemonStartup().whenComplete { _, exception -> + serverSocket.close() deviceFd.close() privateKeys.first?.close() privateKeys.second?.close() @@ -189,6 +194,22 @@ class TincVpnService : VpnService() { LocalBroadcastManager.getInstance(this).sendBroadcast(Intent(event)) } + private fun serveDeviceFd(serverSocket: LocalServerSocket, deviceFd: ParcelFileDescriptor) = + serverSocket.accept().let { socket -> + try { + if (socket.peerCredentials.uid != App.getApplicationInfo().uid) + throw AccessControlException("Peer UID mismatch.") + + socket.setFileDescriptorsForSend(arrayOf(deviceFd.fileDescriptor)) + socket.outputStream.write(0) // dummy write + socket.outputStream.flush() + } catch (e: Exception) { + log.error("Error while serving device fd", e) + } finally { + socket.close() + } + } + private fun waitForDaemonStartup() = Executor .runAsyncTask { Thread.sleep(SETUP_DELAY) } @@ -196,6 +217,7 @@ class TincVpnService : VpnService() { companion object { private const val SETUP_DELAY = 500L // ms + private const val DEVICE_FD_ABSTRACT_SOCKET = "${BuildConfig.APPLICATION_ID}.daemon.socket" private val STORE_NAME = this::class.java.`package`!!.name private const val STORE_KEY_NETNAME = "netname" diff --git a/app/src/main/play/listings/en-US/full-description.txt b/app/src/main/play/listings/en-US/full-description.txt index 45fee19..b25756d 100644 --- a/app/src/main/play/listings/en-US/full-description.txt +++ b/app/src/main/play/listings/en-US/full-description.txt @@ -22,7 +22,7 @@ Please see the project's website for more information (setup guide, documentatio --- -Copyright © 2017-2019 Pacien TRAN-GIRARD and contributors. +Copyright © 2017-2020 Pacien TRAN-GIRARD and contributors. This program is licensed under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License or any later version. It is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details: https://www.gnu.org/licenses/ diff --git a/readme.md b/readme.md index d78fffb..f70c6e3 100644 --- a/readme.md +++ b/readme.md @@ -41,7 +41,7 @@ Requirements: License ------- -Copyright (C) 2017-2019 Pacien TRAN-GIRARD and contributors (listed in `contributors.md`). +Copyright (C) 2017-2020 Pacien TRAN-GIRARD and contributors (listed in `contributors.md`). _Tinc App_ is distributed under the terms of GNU General Public License v3.0, as detailed in the provided `license.md` file. -- cgit v1.2.3