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

Restrict intents sent to control FTP server to Amaze only #1815

Merged

Conversation

TranceLove
Copy link
Collaborator

Per mail discussion offline it is found that any app may fire an Intent with action com.amaze.filemanager.services.ftpservice.FTPReceiver.ACTION_START_FTPSERVER or com.amaze.filemanager.services.ftpservice.FTPReceiver.ACTION_STOP_FTPSERVER to control Amaze's built-in FTP server without user acknowledgement.

Changes in changeset

  • add a custom permission com.amaze.filemanager.permission.CONTROL_FTP_SERVER and set it at signature level
  • FtpService requires com.amaze.filemanager.permission.CONTROL_FTP_SERVER permission
  • so only Amaze would be able to send intents to start/stop the FTP server

This supercedes #1814 which contained unfinished code that makes the PR difficult to understand. Its work (to work out a way to allow third-party apps to start/stop Amaze's FTP server) will continue on another PR.

@TranceLove TranceLove added Issue-Bug Related unexpected behavior or something worth investigating. Issue-Severe (high) Showstopper issues that require immediate attention Area-FTP Related to FTP Server. labels Jan 10, 2020
@TranceLove TranceLove added this to Pending approval in Current work Jan 10, 2020
@EmmanuelMess
Copy link
Member

Nice solution!

@@ -23,6 +23,9 @@
xmlns:tools="http://schemas.android.com/tools"
package="com.amaze.filemanager">

<permission android:name="com.amaze.filemanager.permission.CONTROL_FTP_SERVER"
android:protectionLevel="signature" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protectionLevel="signature" is a bit much isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems dangerous can still prevent unintended intents sent to FtpReceiver.

@EmmanuelMess
Copy link
Member

Using this code I am unable to start the service from another app, any ideas?

    private fun onStartServer() {
        controlFtpButton.text = getString(R.string.stop_server)

        statusTextView.text = getString(R.string.running)

        applicationContext.sendBroadcast(Intent(ACTION_START_FTPSERVER).also {
            it.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES)
        })
    }

    private fun onStopServer() {
        controlFtpButton.text = getString(R.string.start_server)

        statusTextView.text = getString(R.string.server_not_started)

        applicationContext.sendBroadcast(Intent(ACTION_STOP_FTPSERVER).also {
            it.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES)
        })
    }

Added custom permission to control FTP server, so only Amaze/apps explicitly granted such permission can send intent to start/stop FTP server.
@TranceLove
Copy link
Collaborator Author

Using this code I am unable to start the service from another app, any ideas?

    private fun onStartServer() {
        controlFtpButton.text = getString(R.string.stop_server)

        statusTextView.text = getString(R.string.running)

        applicationContext.sendBroadcast(Intent(ACTION_START_FTPSERVER).also {
            it.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES)
        })
    }

    private fun onStopServer() {
        controlFtpButton.text = getString(R.string.start_server)

        statusTextView.text = getString(R.string.server_not_started)

        applicationContext.sendBroadcast(Intent(ACTION_STOP_FTPSERVER).also {
            it.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES)
        })
    }

I got this when I fire the intent from "another app" to Amaze.

2020-01-12 22:36:59.755 1951-1980/system_process W/BroadcastQueue: Background execution not allowed: receiving Intent { act=com.amaze.filemanager.services.ftpservice.FTPReceiver.ACTION_START_FTPSERVER flg=0x30 } to com.amaze.filemanager.debug/com.amaze.filemanager.asynchronous.services.ftp.FtpReceiver

Not sure if it's related to changes to Android system... was trying on Pixel 2 emulator running Android 9.0; didn't try on earlier versions of Android though.

Ref: https://commonsware.com/blog/2017/04/11/android-o-implicit-broadcast-ban.html

@EmmanuelMess
Copy link
Member

EmmanuelMess commented Jan 12, 2020

@TranceLove AFAIK,'implicit' is when it is not defined in the manifest.

@TranceLove
Copy link
Collaborator Author

Using this code I am unable to start the service from another app, any ideas?

    private fun onStartServer() {
        controlFtpButton.text = getString(R.string.stop_server)

        statusTextView.text = getString(R.string.running)

        applicationContext.sendBroadcast(Intent(ACTION_START_FTPSERVER).also {
            it.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES)
        })
    }

    private fun onStopServer() {
        controlFtpButton.text = getString(R.string.start_server)

        statusTextView.text = getString(R.string.server_not_started)

        applicationContext.sendBroadcast(Intent(ACTION_STOP_FTPSERVER).also {
            it.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES)
        })
    }

Tried again on physical devices... possibly you didn't explicitly grant permission to the app. Don't know how to make the permission to appear on app install though...

Screenshot_20200115-223915
Screenshot_20200115-223924

  • Oneplus 2 running Slim7 (7.1.2) worked, after explicitly enabled permission in app properties
  • Fairphone 3 running Fairphone OS (9.0) not working, even after explicitly enabled permission in app properties. Still getting the same error
2020-01-15 22:28:29.726 1151-1335/? W/BroadcastQueue: Background execution not allowed: receiving Intent { act=com.amaze.filemanager.services.ftpservice.FTPReceiver.ACTION_START_FTPSERVER flg=0x30 } to com.amaze.filemanager.debug/com.amaze.filemanager.asynchronous.services.ftp.FtpReceiver

So I'm standing with my guess.

Copy link
Member

@EmmanuelMess EmmanuelMess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepted to fix urgent issue.

@EmmanuelMess EmmanuelMess merged commit 94e58ae into TeamAmaze:master Jan 29, 2020
Current work automation moved this from Pending approval to Done Jan 29, 2020
@EmmanuelMess EmmanuelMess deleted the feature/ftpserver-security branch January 29, 2020 19:14
@EmmanuelMess
Copy link
Member

EmmanuelMess commented Jan 29, 2020

@TranceLove please upload the sample app/code as a project on AmazeTeam.

EmmanuelMess added a commit that referenced this pull request Feb 1, 2020
Restrict intents sent to control FTP server to Amaze only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-FTP Related to FTP Server. Issue-Bug Related unexpected behavior or something worth investigating. Issue-Severe (high) Showstopper issues that require immediate attention
Projects
Current work
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants