Connect over a unix socket before trying to use am #147
Conversation
tareksander
changed the title
There was a problem hiding this comment.
Thanks for the PR! Here's some initial comments.
Scripts using -a some-action are not current handled, termux-usb -l and termux-volume music 0 for example gives something like Unsupported options: -alist.
Would it be possible to use C++ for this file instead, so you can just concatenate the strings and operate on them?
I would prefer to keep it in C. I am working on moving the code into a library instead, which would be useful for packages relying on libusb. We can then add some helper functions and link them against the library to open a usb device without having to go through the termux-api libexec binary.
I see what you mean with the ugliness of string handling in C however :) will look it over in more and see if we can improve the handling of the arguments here a bit.
termux-api.c
Outdated
| @@ -26,18 +26,17 @@ | |||
| #include <sys/un.h> | |||
| #include <time.h> | |||
| #include <unistd.h> | |||
| #include <arpa/inet.h> | |||
|
|
|||
| #define PREFIX "/data/data/com.termux/files/usr" | |||
There was a problem hiding this comment.
Please guard this with a #ifndef PREFIX, we might set/override it with a configure arg
There was a problem hiding this comment.
Oh, I forgot to delete that. When compiling in Termux it complains about PREFIX being undefined and I don't know enough about CMake to fix that, so I just defined it in the file.
termux-api.c
Outdated
| @@ -71,6 +70,197 @@ _Noreturn void exec_am_broadcast(int argc, char** argv, | |||
| exit(1); | |||
| } | |||
|
|
|||
|
|
|||
| // passes the arguments to the plugin via the unix socket, falling back to exec_am_broadcast() if that doesn't work | |||
| _Noreturn void contact_pugin(int argc, char** argv, | |||
termux-api.c
Outdated
| @@ -202,7 +392,7 @@ int main(int argc, char** argv) { | |||
| pid_t fork_result = fork(); | |||
| switch (fork_result) { | |||
| case -1: perror("fork()"); return 1; | |||
| case 0: exec_am_broadcast(argc, argv, input_address_string, | |||
| case 0: contact_pugin(argc, argv, input_address_string, | |||
| bool first = true; | ||
| err = true; | ||
| while ((ret = read(listenfd, readbuffer, 99)) > 0) { | ||
| // if a single null byte is received as the first message, the call was successfull |
Fixed that now, I don't thing there should ever be spaces inside the action string, so that doesn't need to be wrapped in quotes. |
|
The The following functions are part of a new "posixy" compliant termux library I'll be pushing hopefully soon and should work fine for complex data with DATA__NL='
'
##
# Add an key or key/value extra to an intent string.
# .
# .
# android__intent__add_intent_extra `intent_string_variable_name` `data_type` `extra_key` `extra_value` [`add_only_if_set` `enusre_set`]
##
android__intent__add_intent_extra() {
local data_type="$2"
local extra_key="$3"
local extra_value="$4"
local add_only_if_set="$5"
local enusre_set="$6"
local data_type_argument
local extra_key_argument
local extra_value_argument
case "$data_type" in
"String")
data_type_argument="--es";;
"boolean")
data_type_argument="--ez";;
"int")
data_type_argument="--ei";;
"long")
data_type_argument="--el";;
"float")
data_type_argument="--ef";;
"String[]")
data_type_argument="--esa";;
"List<String>")
data_type_argument="--esal";;
"Integer[]")
data_type_argument="--eia";;
"List<Integer>")
data_type_argument="--eial";;
"Long[]")
data_type_argument="--ela";;
"List<Long>")
data_type_argument="--elal";;
"Float[]")
data_type_argument="--efa";;
"List<Float>")
data_type_argument="--efal";;
"Component")
data_type_argument="--ecn";;
"Uri")
data_type_argument="--eu";;
"Key")
data_type_argument="--esn";;
*)
logger__log_errors "Unsupported data_type \"$data_type\" passed to android__intent__add_intent_extra"
return 1
esac
if [ "$data_type_argument" = "Key" ]; then
shell__arg__validate_argument_count eq $# 3 android__intent__add_intent_extra "$@" || return $?
else
shell__arg__validate_argument_count or $# 4 6 android__intent__add_intent_extra "$@" || return $?
fi
shell__validate_variable_set extra_key android__intent__add_intent_extra || return $?
data__escape_single_quotes extra_key_argument "$extra_key" || return $?
if [ "$data_type_argument" = "Key" ]; then
shell__append_to_variable "$1" " $data_type_argument '$extra_key_argument'" || return $?
else
if [ -z "$extra_value" ]; then
if [ "$add_only_if_set" = "1" ]; then
return 0
elif [ "$enusre_set" = "1" ]; then
logger__log_errors "The \"$extra_key\" value is not set while running \"android__intent__add_intent_extra\""
return 1
fi
else
data__escape_single_quotes extra_value_argument "$extra_value" || return $?
fi
shell__append_to_variable "$1" " $data_type_argument '$extra_key_argument' '$extra_value_argument'" || return $?
fi
}
# Is a valid boolean extra that can be passed to `--ez` parameter of `am` command.
# https://cs.android.com/android/platform/superproject/+/android-11.0.0_r3:frameworks/base/core/java/android/content/Intent.java;l=7574
# .
# .
# android__intent__is_intent_extra_boolean `value`
##
android__intent__is_intent_extra_boolean() {
case "$1" in
"true"|"t") return 0;;
"false"|"f") return 0;;
esac
data__is_unsigned_int "$1"
}
##
# android__intent__validate_is_intent_extra_boolean `label` `value` [`error_output_variable_name`]
##
android__intent__validate_is_intent_extra_boolean() {
local label="$1"
local value="$2"
local error
if ! android__intent__is_intent_extra_boolean "$value"; then
error="$label is not a valid intent extra boolean: \"$value\""
if [ -n "$3" ]; then
shell__set_variable "$3" "$error" || return $?
else
logger__log_errors "$error"
fi
return 1
else
return 0
fi
}
##
# data__is_unsigned_int `value`
##
data__is_unsigned_int() {
case "$1" in
''|*[!0-9]*) return 1;;
*) return 0;;
esac
}
##
# Print a string to stdout and append a newline.
# .
# .
# data__println_string `string`
##
data__println_string() {
#if [ "$DATA__IS_PRINTF_SHELL_BUILTIN" = "1" ]; then
printf "%s\n" "$1"
#else
# echo "$1"
#fi
}
##
# Print a string to stdout with `printf` and append a newline.
# .
# .
# data__printfln_string `string`
##
data__printfln_string() {
#if [ "$DATA__IS_PRINTF_SHELL_BUILTIN" = "1" ]; then
printf "%s\n" "$1"
#else
# data__printf_string_inner "%s" "%s\n" "$1"
#fi
}
##
# Replace a single quotes `'` in string with `'"'"'`.
# .
# .
# data__escape_single_quotes `output_variable_name` `string`
##
data__escape_single_quotes() {
data__replace_character_in_string "$1" "$2" "'" "'\"'\"'" || return $?
# Alternative
#shell__set_variable "$1" "$(data__printf_string "${string}" | sed "s/'/'\"'\"'/g")"
}
##
# Replace a character with another string.
# .
# .
# data__replace_character_in_string `output_variable_name` `string` `character_to_replace` `replace_with`
##
data__replace_character_in_string() {
local string="$2"
local character_to_replace="$3"
local replace_with="$4"
local __new_string
local char
#local string_rest
while [ -n "$string" ]; do
#string_rest="${string#?}" #1:end
#char="${string%"$string_rest"}" # 0:1
#string="$string_rest" # 1:end
char="${string:0:1}" # 0:1
string="${string:1}" # 1:end
case "$char" in
"$character_to_replace") __new_string="${__new_string}${replace_with}";;
*) __new_string="${__new_string}${char}";;
esac
done
shell__set_variable "$1" "$__new_string"
}
##
# shell__validate_variable_set `variable_name` [`function_name`]
##
shell__validate_variable_set() {
local curr_value
shell__copy_variable curr_value "$1" || return $?
if [ -z "$curr_value" ]; then
if [ -n "$2" ]; then
logger__log_errors "The $1 is not set while running \"$2\""
else
logger__log_errors "The $1 is not set"
fi
return 1
else
return 0
fi
}
##
# shell__set_variable `variable_name` `variable_value` `export_variable`
##
shell__set_variable() {
local variable_name="$1"
# If variable_name is not a valid shell variable_name
if ! shell__is_valid_shell_variable_name "$variable_name"; then
logger__log_errors "The variable_name \"$variable_name\" passed to \"shell__set_variable\" is not a valid shell variable name"
return 1
fi
#printf -v "$variable_name" "%s" "$2"
eval "$variable_name"=\"\$2\"
}
##
# shell__copy_variable `output_variable_name` `input_variable_name`
##
shell__copy_variable() {
local output_variable_name="$1"
local input_variable_name="$2"
# If variable_name is not a valid shell variable_name
if ! shell__is_valid_shell_variable_name "$output_variable_name"; then
logger__log_errors "The output_variable_name \"$output_variable_name\" passed to \"shell__copy_variable\" is not a valid shell variable name"
return 1
fi
# If variable_name is not a valid shell variable_name
if ! shell__is_valid_shell_variable_name "$input_variable_name"; then
logger__log_errors "The input_variable_name \"$input_variable_name\" passed to \"shell__copy_variable\" is not a valid shell variable name"
return 1
fi
eval "$output_variable_name"=\"\$$input_variable_name\"
}
##
# shell__append_to_variable `variable_name` `value_to_append`
##
shell__append_to_variable() {
local curr_value
shell__copy_variable curr_value "$1" || return $?
shell__set_variable "$1" "$curr_value$2" || return $?
}
##
# shell__arg__print_arguments_string `arguments...`
##
shell__arg__print_arguments_string() {
local arguments_string
shell__arg__get_arguments_string arguments_string "$@" || return $?
data__println_string "$arguments_string"
}
##
# shell__arg__get_arguments_string `output_variable_name` `arguments...`
##
shell__arg__get_arguments_string() {
local output_variable_name="$1"
shift 1
local argument
local __arguments_string
local i=1
while [ $# -ne 0 ]; do
argument="$1"
__arguments_string="$__arguments_string$i: \`$argument\`"
if [ $# -gt 1 ]; then
__arguments_string="$__arguments_string$DATA__NL"
fi
i=$((i + 1))
shift
done
shell__set_variable "$output_variable_name" "$__arguments_string" || return $?
}
##
# shell__arg__validate_argument_count `"eq"`|`"lt"`|`"le"`|`"gt"`|`"ge"` `arguments_received` `arguments_actual` `label`
# shell__arg__validate_argument_count `"or"` `arguments_received` `arguments_actual_1` `arguments_actual_2` `label`
# shell__arg__validate_argument_count `"between"` `arguments_received` `arguments_actual_min` `arguments_actual_max label`
##
shell__arg__validate_argument_count() {
local return_value=0
case "$1" in
eq) if [ "$2" -eq "$3" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$4\". Expected \"$3\" argument(s)."; shift 4; return_value=1; fi;;
lt) if [ "$2" -lt "$3" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$4\". Expected less than \"$3\" argument(s)."; shift 4; return_value=1; fi;;
le) if [ "$2" -le "$3" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$4\". Expected less than or equal to \"$3\" argument(s)."; shift 4; return_value=1; fi;;
gt) if [ "$2" -gt "$3" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$4\". Expected greater than \"$3\" argument(s)."; shift 4; return_value=1; fi;;
ge) if [ "$2" -ge "$3" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$4\". Expected greater than or equal to \"$3\" argument(s)."; shift 4; return_value=1; fi;;
or) if [ "$2" -eq "$3" ] || [ "$2" -eq "$4" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$5\". Expected \"$3\" or \"$4\" argument(s)."; shift 5; return_value=1; fi;;
between) if [ "$2" -ge "$3" ] && [ "$2" -le "$4" ]; then :; else logger__log_errors "Invalid argument count \"$2\" to \"$5\". Expected between \"$3\" and \"$4\" (inclusive) argument(s)."; shift 5; return_value=1; fi;;
*) logger__log_errors "The comparison \"$1\" while running \"shell__arg__validate_argument_count\" is invalid"; return 1;;
esac
[ $return_value -eq 0 ] && return 0
if [ $# -gt 0 ]; then
shell__arg__print_arguments_string "$@" || return $?
fi
return $return_value
}
##
# logger__log_errors `log_string...`
##
logger__log_errors() { data__printfln_string "$@" 1>&2; }You can use it like android__intent__add_intent_extra __args_string "String" "com.termux.string.extra" "$value" 0 1 || return $?
android__intent__add_intent_extra __args_string "String[]" "com.termux.string_array.extra" "$value" 1 0 || return $?
android__intent__validate_is_intent_extra_boolean "bool.extra value" "$value" || return $?
android__intent__add_intent_extra __args_string "boolean" "com.termux.bool.extra" "$value" 1 0 || return $? |
|
I just had another idea: I got that working, but I had to set the targetSdkVersion from 28 to 27. Termux still compiles and works, but I don't know enough about the app code, so I don't know if that could have any other side-effects. I also integrated |
That is a good idea. I was also planning to include
That's great.
Is is because world readable access is not available for sockets when using If access is not there, then what can be done is For that you
That's great then. My only concern is the socket shouldn't be connectable to other apps, hopefully the |
Citing from the paper "The Misuse of Android Unix Domain Sockets and Security Implications" you linked: "UID/GID checks efficiently prevent unauthorized peers, as UID and GID can never be spoofed or modified."(page 87). The only thing that could happen with the UID check is a DOS attack where another app creates an abstract UNIX socket with the same name before Termux is started. Now that I think about it, a filesystem UNIX socket may be a better idea, but Android only has library support for abstract sockets. But since Termux already uses C, you can just make a JNI interface for that. Should that got into the app component or
It seems a method Another solution could be to integrate the socket connection into termux-am instead and have a new flag to start it as a daemon, but I don't know if the Android system would kill it, because it seems to be running as an app process, but has no alive components. |
|
I managed to rewrite I also made a new branch in my Termux fork called I haven't tested it much, but it seems to work. |
Well, that is a relief. Root is not really a concern, it can directly access all termux data anyways and send intents.
If that can be done and you think its better than do it, then do it, sockets is not really my domain currently. You can place it under
Yeah, probably better, if you create a class with a path argument, then it should be usable by both
The
Yes, android would likely kill it after some time since it would be considered as an empty process. Socket threads must be part of main app process for persistence.
Great. I don't know why
Will probably have to test various shell calls for all am calls
Warnings shouldn't be removed, they help in debugging, so no need to worry about those.
You should create a static function in Use |
If you want (Instrumentation) tests, ones I've had in |
Ah, I see. Thanks. Thinking more on this, maybe we shouldn't modify We shouldn't modify |
While I'm neither for nor against moving |
I am not sure either (I mis-looked earlier), but no point in breaking something that already works and will preserve backward compatibility. It still is useful anyways even if we move. Patching a hard coded string in a fork isn't too big an issue. We can also create a new branch in TermuxAm if it shouldn't be added to |
I also think we should also keep
Sorry, I meant the warnings are only possible for the old version of am, because the Android API provides no way of knowing what happens once you sent the Intent, except if no Activity is found. So the Warnings like "Activity brought to front", etc. aren't possible. The next thing I'll do is implement a filesystem Unix socket interface via JNI, and also rewrite the little python script I made as a C program for extra speed. |
Later too IMO. You can name your wrapper something appropriate, like
Ah, sorry, should have thought of that. No worries, users can check
Yeah, |
I checked, at least for "Warning: Activity not started, intent has been delivered to currently running top-most instance." the warning doesn't get logged to logcat by the system, and I suspect for the other warnings it's the same. I now made a working filesystem UNIX socket interface and tested the rejection and logging of connections from other users by using root in my emulator (I just started the termux bash binary in the adb shell, interestingly netcat segfaulted, but python worked fine, so I just used the script from before).
That's also good. Keeping it as a faster but less verbose (missing warnings) version of |
|
@tareksander Hi, I'm very excited about this work. Regarding the uid checks for non-termux clients, I think we should allow some other clients to connect if they can be authenticated in some form, for instance, zeromq with curve keypairs . We can add the public key of other allowed clients to some places similar to ssh authorized_keys. It is more capability-based security like instead of UID-style, and the permission model is much more flexible. Would you mind considering this? |
Non-Termux apps shouldn't even be able to access the socket file, because it's in the Termux app files. But unix sockets shouldn't be used to communicate across apps, Android provides Intents and Services for such things, and those already have capability-based security with permissions. And besides, other apps can already send commands to Termux to execute with the right permission, that would also bypass the uid check, so you could just use that instead. |
I hope to use termux-api functionality from my linux-deploy chroot environment. It is a full linux environment with vanilla debian/alpine packages. The normal user uid is 60000. |
If you have a full linux environment, what prevents you from running a ssh server in termux and use termux-api over ssh? |
|
|
If other user can access the socket, then I can connect the socket using python directly, instead of wrapping my intentions through ssh subprocess commands. The time cost of ssh connection initialization could be as much as the am call. |
Even with previous design, you couldn't connect with non-termux users. With this design, if I am also against other non root and termux users to connect, specially by default. If a separate key pair based authorization can be implemented that is secure, then that could be acceptable, but should be disabled by default. |
|
I now allow root to access sockets. Users that don't have the uid of Termux or 0 can't access the socket, I tested that. I finished the C socket client. I made it with CMake, so it should build as a termux package. |
Fall back to to am if it doesn't work.
This is the corresponding PR for this PR for Termux:API.
Preparing the arguments for transmission is a bit ugly, because of string handling in C. If the transmission fails for whatever reason, it falls back to calling
am. That makes it work reliably, even if the Termux:API process gets killed while the transmission is in progress.Would it be possible to use C++ for this file instead, so you can just concatenate the strings and operate on them?
I also added 2 new scripts to start and stop the new service easily.