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

add custom permission to call TermuxService from external apps #1029

Closed
wants to merge 2 commits into from

Conversation

edaubert
Copy link

This pull request is an attempt to provide a solution to #804

@EpicOrange
Copy link

Just curious, does this mean that apps with this permission effectively have access to /data/data/com.termux?

@edaubert
Copy link
Author

No you don't have access to it.

You need to use some storage permission I guess.
I guess another possibility could be to send the file to termux before trying to execute it.

@edaubert
Copy link
Author

Yet another way to do it is to give storage access to termux and store the script where it can find it.

https://wiki.termux.com/wiki/Termux-setup-storage

@edaubert
Copy link
Author

So I have completed a termux plugin on Easer which fully use the permission.
It uses the fact that termux provide a document provider. This provider allow external apps to get access to some files inside the termux environment.

Please have a look here: renyuneyun/Easer#205

@fornwall
Copy link
Member

We discussed this in a chat session (https://gitter.im/termux/dev) today, and I think this is a really interesting thing to have. Some notes from the chat:

The thing we want to minimise is the risk for evil apps to infect Termux installations (due to people ignoring/not understanding what the permission does). A conservative change could be to make this some kind of opt in, like requiring a property in termux.properties for this to be allowed
so we both require that permission, but we also need something like external-apps-can-execute=*, or external-apps-can-execute=com.app1,com.app2.

Or it could be a runtime permission. Perhaps a dialog like "The app XXX is trying to run a script. Allow that?" with a checkbox to remember the choice though that obviously complicates the code and also the user experience. Thoughts on that?

@xeffyr: A dialog like "The app XXX is trying to run a script. Allow that?" with a checkbox to remember the choice seems better than termux.properties.

@fornwall What I can't decide on is if that is necessary :). One point is that the app is a terminal emulator, users should be expected to a bit more technically skilled than an average app users and careful with accepting permissions.

... On the other hand, if the effect of opening up permissions is that several users have e.g. their ssh/gpg private keys stolen or likewise, it's bad regardless of if those users can be blamed for not reading through permissions.

Feedback, thoughts and ideas are welcome here! Even with the above said I'm leaning against merging this as is.

@edaubert
Copy link
Author

As @fornwall said, I am not sure we need to add an extra layer of security.

First as it is said the users of termux are expected to be advanced users and so, in my opinion, should be aware of what implies to accept a permission. Well for me even non advanced users should be aware of this...

Second, it is the termux app which defines the permission and so the description can be explicit. And if a user do not want to read the permission dialog why should he want to read yet another popup dialog.

About the part of "their ssh/gpg private keys stolen or likewise, it's bad regardless of if those users can be blamed for not reading through permissions" you completly right but if the user gives root access to another app, it can also stole its data and termux is not able to protect those data even without speaking of permission. In the end, and this is mostly my really subjective opinion 😃, users are still responsible of what happens and permissions are something which is not just a bother which display annoying dialog.

However, if you are still willing something, i will propose to add a notification with multiple actions.
The notification provide a small text about running a script (name of the script and who ask to run it)

  • action 1: always run it
  • action 2: run it once
  • actiin 3: never run it

Why I am proposing a notification is because I don't want to be interrupted by something that I may have not ask for (e.g. an automation script).
Plus if people do not care about popup dialog, they may care about their notifications.

But again for me, this is only an extra layer of security for supposed advanced termux users and I think they do not need that.

@ujeropoc
Copy link

little late too the party, but what about outsourcing this to another app?: #1048

@ghost
Copy link

ghost commented Feb 28, 2019

@ujeropoc Executing commands via URLs is weird solution from security side. It just won't be possible to use Android permissions here.


Main idea is to allow execution only for limited set of applications. That's not possible for URLs - only either allow all URLs or not allow. Whitelisting also bad, think 3 times why...

@alensiljak
Copy link

alensiljak commented Apr 21, 2019

What @edaubert suggests, with a dialog that resembles SuperSu's or through a notification, is handy. But it would also be good to be able to pre-configure the permissions before running because, as was mentioned, the point is to run automated and we may not know in advance when this will happen.
On the pragmatic side, would it be possible to come up even with a partial implementation, perhaps in a test version, which implements at least a rudimentary version of authorization, so that we can try it out and provide feedback? That way we could iron-out any issues that happen during actual use.
I'd like to create an app that provides a GUI for entering transactions and utilizes Ledger in the background to list the accounts defined by the user. Ledger is available as a binary package and can be run on the command line.
Thanks for all the great work so far.

Edit: My use-case shows that, if this takes on, soon it would be not only the advanced users who use this functionality. So, having a GUI option as well as editing the authorization permissions file manually, would be practical.

@Ciantic
Copy link

Ciantic commented Jul 13, 2019

I don't think this needs extra layer of security either, termux is for developers mostly anyway. Having just blanket permission is just fine. But if main devs want extra security, you could restrict it to e.g. directory:

~/appscripts/*

So that apps are allowed to run scripts only from appscripts folder.

@xalexalex
Copy link

I also agree with the fact that having a working version (whose security can later be improved) is better than not having this functionality at all.

@ghost ghost mentioned this pull request Aug 2, 2019
@RoninNada
Copy link

Would someone be able to provide an example of this being implemented/used? Sending a command an executing from another from another app? I already built a version of termux with this implemented just can't get the other part to work. I want it to work off a button press. Thank You

@ghost
Copy link

ghost commented Sep 7, 2019

@RoninNada Make sure that application implements Termux service call in the way like:

// scriptUri is URI path to script file.
Intent executeIntent = new Intent("com.termux.service_execute", scriptUri);
executeIntent.setClassName("com.termux", "com.termux.app.TermuxService");

// Whether to execute script in background.
//executeIntent.putExtra("com.termux.execute.background", true);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
    context.startForegroundService(executeIntent);
} else {
    context.startService(executeIntent);
}

Service access permission must be granted to that "another application" in order to make code above work.

@RoninNada
Copy link

@xeffyr , thank you so much really helpful

@Ciantic
Copy link

Ciantic commented Sep 9, 2019

I'm trying to understand the pushes above, was this merged? Is it now possible to call shell scripts from within other apps?

@ghost
Copy link

ghost commented Sep 9, 2019

It isn't merged and current implementation available here won't be merged. With @fornwall we agreed to:

  • Do not let applications to access TermuxService directly.
  • Implement separate service for calling with external applications (TermuxServiceForExternalApps).
  • TermuxServiceForExternalApps will do necessary checks (e.g. property allow-external-apps is enabled).

@bdovaz
Copy link

bdovaz commented Sep 9, 2019

@xeffyr How is that going?

@ghost ghost deleted a comment from soso-cloud Mar 27, 2020
ghost pushed a commit that referenced this pull request Jun 9, 2020
Re-implementation of #1029.

If Termux has property "allow-external-apps" set to "true", a third-party
program will be able to send intents for executing custom commands
within Termux environment.

Third-party program must declare permission "com.termux.permission.RUN_COMMAND".
@ghost
Copy link

ghost commented Jun 9, 2020

Implemented in db3ff7b.

@ghost ghost closed this Jun 9, 2020
@bdovaz
Copy link

bdovaz commented Jun 9, 2020

@xeffyr I just saw the commit:

db3ff7b

But how can I retrieve the command output? In my case I need that and I don't see an example.

Thanks.

@ghost
Copy link

ghost commented Jun 9, 2020

But how can I retrieve the command output? In my case I need that and I don't see an example.

This isn't possible with intents. Here you just start an activity or service with specific parameters.

@bdovaz
Copy link

bdovaz commented Jun 9, 2020

@xeffyr but is there any way to achieve what I want? Thanks.

@ZhaoTzuHsien
Copy link

ZhaoTzuHsien commented Jun 12, 2020

@xeffyr When will this important improvement be released? :)

@ghost
Copy link

ghost commented Jun 12, 2020

@ZhaoTzuHsien It is already released by db3ff7b in application version 0.95.

@bdovaz
Copy link

bdovaz commented Jun 12, 2020

@xeffyr you didn't answer my last question. Is there any way to achieve that? Or will it be possible? Or at least, can I read some file where the output it's being written so I can pool it? Thanks.

@ghost
Copy link

ghost commented Jun 12, 2020

Is there any way to achieve that? Or will it be possible?

Will be possible as soon as someone will write an API engine for that, with IPC, proper permission checks, etc.

@agnostic-apollo
Copy link
Member

agnostic-apollo commented Jun 25, 2020

@xeffyr can support be added so that the EXTRA_EXECUTE_IN_BACKGROUND is also received when the com.termux.RUN_COMMAND action intent is processed and passed to the TermuxService. It would make it easier to run background commands like with Tasker. Basically show the notification but not the session activity unless user manually opens it if required. There seems to be reasonable security for it to be an issue. Thanks

@agnostic-apollo
Copy link
Member

agnostic-apollo commented Jul 30, 2020

@xeffyr it seems that the RUN_COMMAND_ARGUMENTS is not working. You are receiving and sending it as a string extra in RunCommandService but receiving it as a string array extra in TermuxService resulting it in being ignored as far as I can see. A simple sleep 5 commands shows sleep: missing arguments. Do you want a pull request?

You probably have your own test app but leaving following instructions for others:

The com.termux.permission.RUN_COMMAND has been added to the Tasker AnndroidManifest.xml in the list of requested permissions in v5.9.3.rc. It should automatically be requested when TermuxCommand function is used in a Tasker Function action or you can grant it manually from something like Tasker app permissions -> Additional permissions -> Run commands in termux environment, specially if you intend to run am shell commands instead. When you run a command or script in the function or use am command, a new termux session will open up and run the command. It will automatically close by default when the command completes. You can put the session in background mode if you want after that. However, starting it in background mode is not supported and has been requested.

After you grant the permission, you also need to add the allow-external-apps=true property in ~/.termux/termux.properties file for intents to be allowed. You can run the following command in a non-root termux session to add the line to the end of the file if it doesn't exist already.

cd ~ && mkdir -p .termux && grep -sq  "allow-external-apps=true" ./.termux/termux.properties || echo $'\n'"allow-external-apps=true" >> ./.termux/termux.properties

After that you can run the command with a Tasker Funtion action or am command using a Run Shell action. The arguments and working directory are optional. An example am command would be something like the following.

am startservice --user 0 -n com.termux/com.termux.app.RunCommandService -a com.termux.RUN_COMMAND --es com.termux.RUN_COMMAND_PATH "/data/data/com.termux/files/usr/bin/top" --es com.termux.RUN_COMMAND_ARGUMENTS "-n 5" --es com.termux.RUN_COMMAND_WORKDIR "/data/data/com.termux/files/home"

@ghost
Copy link

ghost commented Jul 30, 2020

You are receiving and sending it as a string extra in RunCommandService but receiving it as a string array extra in TermuxService resulting it in being ignored as far as I can see. A simple sleep 5 commands shows sleep: missing arguments. Do you want a pull request?

Ok, that's a bug. You can submit a pull request to fix it.

@agnostic-apollo
Copy link
Member

Should I also include support for passing the background flag as well or not?

@ghost
Copy link

ghost commented Jul 30, 2020

Yes.

@agnostic-apollo
Copy link
Member

Okay thank. I will do it and let you know.

agnostic-apollo added a commit to agnostic-apollo/termux-app that referenced this pull request Jul 30, 2020
as a string array extra instead of a string extra since TermuxService expects it that way.

Added "RUN_COMMAND_BACKGROUND" boolean extra so that Termux session can be started in background
when running a command.

Updated usage docs.

Check termux#1029 for details.
@agnostic-apollo
Copy link
Member

I have added a pull request here. Let me know if any changes need to be made. I also updated the docs a bit, I hope that's all right.

ghost pushed a commit that referenced this pull request Jul 30, 2020
as a string array extra instead of a string extra since TermuxService expects it that way.

Added "RUN_COMMAND_BACKGROUND" boolean extra so that Termux session can be started in background
when running a command.

Updated usage docs.

Check #1029 for details.
@minhdanhsc
Copy link

Can I run the command from another application to termux? Who has a sample for my reference?

agnostic-apollo added a commit to agnostic-apollo/termux-app that referenced this pull request Mar 25, 2021
as a string array extra instead of a string extra since TermuxService expects it that way.

Added "RUN_COMMAND_BACKGROUND" boolean extra so that Termux session can be started in background
when running a command.

Updated usage docs.

Check termux#1029 for details.
AdamMickiewich pushed a commit to VolyaTeam/dzida-app that referenced this pull request Aug 8, 2022
Re-implementation of termux#1029.

If Termux has property "allow-external-apps" set to "true", a third-party
program will be able to send intents for executing custom commands
within Termux environment.

Third-party program must declare permission "com.termux.permission.RUN_COMMAND".
AdamMickiewich pushed a commit to VolyaTeam/dzida-app that referenced this pull request Aug 8, 2022
as a string array extra instead of a string extra since TermuxService expects it that way.

Added "RUN_COMMAND_BACKGROUND" boolean extra so that Termux session can be started in background
when running a command.

Updated usage docs.

Check termux#1029 for details.
shrihankp pushed a commit to reisxd/termux-app that referenced this pull request Oct 20, 2022
Re-implementation of termux#1029.

If Termux has property "allow-external-apps" set to "true", a third-party
program will be able to send intents for executing custom commands
within Termux environment.

Third-party program must declare permission "com.termux.permission.RUN_COMMAND".
shrihankp pushed a commit to reisxd/termux-app that referenced this pull request Oct 20, 2022
as a string array extra instead of a string extra since TermuxService expects it that way.

Added "RUN_COMMAND_BACKGROUND" boolean extra so that Termux session can be started in background
when running a command.

Updated usage docs.

Check termux#1029 for details.
This pull request was closed.
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.