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

Introduce --root option for (rooted) secure display support on Android 12+ #4127

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

RiggiG
Copy link

@RiggiG RiggiG commented Jun 29, 2023

Resolves #3049, launches the server as AID_SYSTEM when --root is passed.

Functionality confirmations with --root:

  • Video (13)
  • Device audio (13)
  • Clipboard from device to PC (13, 10) No error output, nor INFO: Device clipboard copied
  • Clipboard from PC to device (13, 10) Works when package name matches UID
  • Stay awake ()
  • Screen off ()

@yume-chan
Copy link
Contributor

yume-chan commented Jun 29, 2023

The option name is --root but it actually runs as uid 1000 might be a little mis-leading.

I reserved the --uid flag for you #3729 :-)

IIRC audio capture should also work with uid 0/1000 on all versions #4094

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) {
Ln.w("Audio disabled: it is not supported before Android 11");
streamer.writeDisableStream(false);
return;
}

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) {
Ln.w("Audio disabled: it is not supported before Android 11");
streamer.writeDisableStream(false);
return;
}

I remember clipboard syncing was having issues when running as uid 0, not sure about 1000. #1606

@RiggiG
Copy link
Author

RiggiG commented Jun 29, 2023

The option name is --root but it actually runs as uid 1000 might be a little mis-leading.

Yeah, I figured since 0 is an unnecessary elevation but root privs are required to elevate to 1000 for system privileges anyhow, --root would mean a lot more to the end user than --AID_SYSTEM-via-root-auth, and --uid=[value] would be more of a debug flag than a useful/friendly-to-the-end-user flag. We can just note that the flag executes it as AID_SYSTEM in docs/help output. I certainly defer to the consensus here on that though. This was briefly discussed here.

IIRC audio capture should also work with uid 0/1000 on all versions

That's a good point and makes sense, unfortunately I don't have anything configured for testing other than my current Android 13 device - I'll scrounge up my emulator and give that a shot on some older versions when I've got some time.

I remember clipboard syncing was having issues when running as uid 0, not sure about 1000.

This I can definitely test in a timely manner (if someone doesn't beat me to it), thank you.

@@ -74,12 +74,12 @@ private static void startWorkaroundAndroid11() {
Intent intent = new Intent(Intent.ACTION_MAIN);
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
intent.addCategory(Intent.CATEGORY_LAUNCHER);
intent.setComponent(new ComponentName(FakeContext.PACKAGE_NAME, "com.android.shell.HeapDumpActivity"));
intent.setComponent(new ComponentName(FakeContext.getPackageNameStatic(), "com.android.shell.HeapDumpActivity"));
Copy link
Contributor

Choose a reason for hiding this comment

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

This activity is in com.android.shell package, this line shouldn't change.

ServiceManager.getActivityManager().startActivityAsUserWithFeature(intent);
}

private static void stopWorkaroundAndroid11() {
ServiceManager.getActivityManager().forceStopPackage(FakeContext.PACKAGE_NAME);
ServiceManager.getActivityManager().forceStopPackage(FakeContext.getPackageNameStatic());
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, we want to always kill com.android.shell app.

Copy link
Author

Choose a reason for hiding this comment

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

These two should be cleared up

@@ -23,11 +24,24 @@ private FakeContext() {

@Override
public String getPackageName() {
if (Os.getuid() == 1000) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to call getPackageNameStatic here and in getOpPackageName to not duplicate code.

@yume-chan
Copy link
Contributor

I figured since 0 is an unnecessary elevation but root privs are required to elevate to 1000

Oh, sure. I was looking for past vulnerability reports that give uid 1000 without root in recent weeks (got some ideas, but haven't tested).

}

public static String getPackageNameStatic() {
if (Os.getuid() == 1000) {
return "android";
}
return PACKAGE_NAME;
return PACKAGE_SHELL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could probably assign the PACKAGE_NAME constant without a method:

public static final String PACKAGE_NAME = Os.getuid() == 1000 ? "android" : "com.android.shell";

In practice, what is the problem if the package name is "com.android.shell" while the uid is 1000?

Copy link
Author

Choose a reason for hiding this comment

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

At the very least the clipboard manager is entirely busted when UID and package don't match up -

Package com.android.shell does not belong to 1000
[server] ERROR: Could not invoke method
java.lang.reflect.InvocationTargetException
      at java.lang.reflect.Method.invoke(Native Method)
      at com.genymobile.scrcpy.wrappers.ClipboardManager.getPrimaryClip(ClipboardManager.java:87)
      at com.genymobile.scrcpy.wrappers.ClipboardManager.getText(ClipboardManager.java:118)
      at com.genymobile.scrcpy.Device.getClipboardText(Device.java:285)
      at com.genymobile.scrcpy.Device.setClipboardText(Device.java:298)
      at com.genymobile.scrcpy.Controller.setClipboard(Controller.java:398)
      at com.genymobile.scrcpy.Controller.handleEvent(Controller.java:164)
      at com.genymobile.scrcpy.Controller.control(Controller.java:83)
      at com.genymobile.scrcpy.Controller.lambda$start$0$com-genymobile-scrcpy-Controller(Controller.java:91)
      at com.genymobile.scrcpy.Controller$$ExternalSyntheticLambda1.run(Unknown Source:4)
      at java.lang.Thread.run(Thread.java:1012)
Caused by: java.lang.SecurityException: Package com.android.shell does not belong to 1000
      at android.os.Parcel.createExceptionOrNull(Parcel.java:3011)
      at android.os.Parcel.createException(Parcel.java:2995)
      at android.os.Parcel.readException(Parcel.java:2978)
      at android.os.Parcel.readException(Parcel.java:2920)
      at android.content.IClipboard$Stub$Proxy.getPrimaryClip(IClipboard.java:387)
      ... 11 more
Caused by: android.os.RemoteException: Remote stack trace:
      at android.app.AppOpsManager.checkPackage(AppOpsManager.java:8803)
      at com.android.server.clipboard.ClipboardService.clipboardAccessAllowed(ClipboardService.java:1054)
      at com.android.server.clipboard.ClipboardService.clipboardAccessAllowed(ClipboardService.java:1040)
      at com.android.server.clipboard.ClipboardService.-$$Nest$mclipboardAccessAllowed(Unknown Source:0)
      at com.android.server.clipboard.ClipboardService$ClipboardImpl.getPrimaryClip(ClipboardService.java:461)

[server] ERROR: Could not invoke method
java.lang.reflect.InvocationTargetException
      at java.lang.reflect.Method.invoke(Native Method)
      at com.genymobile.scrcpy.wrappers.ClipboardManager.setPrimaryClip(ClipboardManager.java:107)
      at com.genymobile.scrcpy.wrappers.ClipboardManager.setText(ClipboardManager.java:133)
      at com.genymobile.scrcpy.Device.setClipboardText(Device.java:308)
      at com.genymobile.scrcpy.Controller.setClipboard(Controller.java:398)
      at com.genymobile.scrcpy.Controller.handleEvent(Controller.java:164)
      at com.genymobile.scrcpy.Controller.control(Controller.java:83)
      at com.genymobile.scrcpy.Controller.lambda$start$0$com-genymobile-scrcpy-Controller(Controller.java:91)
      at com.genymobile.scrcpy.Controller$$ExternalSyntheticLambda1.run(Unknown Source:4)
      at java.lang.Thread.run(Thread.java:1012)
Caused by: java.lang.SecurityException: Package com.android.shell does not belong to 1000
      at android.os.Parcel.createExceptionOrNull(Parcel.java:3011)
      at android.os.Parcel.createException(Parcel.java:2995)
      at android.os.Parcel.readException(Parcel.java:2978)
      at android.os.Parcel.readException(Parcel.java:2920)
      at android.content.IClipboard$Stub$Proxy.setPrimaryClip(IClipboard.java:333)
      ... 10 more

Copy link
Author

Choose a reason for hiding this comment

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

We could probably assign the PACKAGE_NAME constant without a method:

Definitely yes, done

@RiggiG
Copy link
Author

RiggiG commented Jun 30, 2023

I briefly tested a rooted Android 10 emulator to see if the audio would Just Work™️, no dice with the right encoder set but the clipboard syncing worked in both directions without issue - interestingly, the INFO: Device clipboard copied message was still not printed but it still works just fine.

@yume-chan
Copy link
Contributor

I also only have rooted Android 10 emulators. No matter whether adbd is running as root, with --root I got:

> .\scrcpy.exe -Vverbose --audio-codec=aac --audio-encoder='OMX.google.aac.encoder' --root
scrcpy 2.1 <https://github.com/Genymobile/scrcpy>
DEBUG: ADB device found:
DEBUG:     --> (tcpip)         emulator-5554            device  Android_SDK_built_for_x86_64
DEBUG: Device serial: emulator-5554
DEBUG: Using server (portable): D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-win64-v2.1-root_final-5-gdae2c204\scrcpy-server
D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-wi... file pushed, 0 skipped. 67.4 MB/s (57055 bytes in 0.001s)
su: failed to exec -c: Permission denied
DEBUG: Interrupting socket
DEBUG: Server disconnected
DEBUG: Server terminated
ERROR: Server connection failed

But with adb root and no --root, audio works.

> .\scrcpy.exe -Vverbose --audio-codec=aac --audio-encoder='OMX.google.aac.encoder'
scrcpy 2.1 <https://github.com/Genymobile/scrcpy>
DEBUG: ADB device found:
DEBUG:     --> (tcpip)         emulator-5554            device  Android_SDK_built_for_x86_64
DEBUG: Device serial: emulator-5554
DEBUG: Using server (portable): D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-win64-v2.1-root_final-5-gdae2c204\scrcpy-server
D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-wi... file pushed, 0 skipped. 76.1 MB/s (57055 bytes in 0.001s)
[server] INFO: Device: [Google] google Android SDK built for x86_64 (Android 10)
[server] DEBUG: Creating audio encoder by name: 'OMX.google.aac.encoder'
DEBUG: Server connected
DEBUG: Starting controller thread
DEBUG: Starting receiver thread
[server] DEBUG: Using video encoder: 'OMX.google.h264.encoder'
INFO: Renderer: direct3d
DEBUG: Trilinear filtering disabled (not an OpenGL renderer
DEBUG: Using icon (portable): D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-win64-v2.1-root_final-5-gdae2c204\icon.png
DEBUG: Demuxer 'video': starting thread
DEBUG: Demuxer 'audio': starting thread
INFO: Texture: 1080x1920
DEBUG: [Audio] Buffer underflow, inserting silence: 224 samples
VERBOSE: [Audio] Buffering: target=2400 avg=1640.105957 cur=1440 compensation=759
VERBOSE: [Audio] Buffering: target=2400 avg=1857.855224 cur=1742 compensation=542
VERBOSE: [Audio] Buffering: target=2400 avg=1997.128784 cur=2007 compensation=402
DEBUG: User requested to quit
DEBUG: quit...
DEBUG: Demuxer 'video': en[server] DEBUG: Controller stopped
d of frames
D[server] DEBUG: Device message sender stopped
EBUG: Demuxer 'audio': end of frames
DEBUG: Receiver stopped
[server] DEBUG: Screen streaming stopped
[server] DEBUG: Audio encoder stopped
DEBUG: Server disconnected
DEBUG: Server terminated

In contrast, with adb unroot:

> .\scrcpy.exe -Vverbose --audio-codec=aac --audio-encoder='OMX.google.aac.encoder'
scrcpy 2.1 <https://github.com/Genymobile/scrcpy>
DEBUG: ADB device found:
DEBUG:     --> (tcpip)         emulator-5554            device  Android_SDK_built_for_x86_64
DEBUG: Device serial: emulator-5554
DEBUG: Using server (portable): D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-win64-v2.1-root_final-5-gdae2c204\scrcpy-server
D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-wi... file pushed, 0 skipped. 70.7 MB/s (57055 bytes in 0.001s)
[server] INFO: Device: [Google] google Android SDK built for x86_64 (Android 10)
[server] WARN: Audio disabled: it is not supported before Android 11
[server] DEBUG: Audio encoder stopped
DEBUG: Server connected
DEBUG: Starting controller thread
DEBUG: Starting [server] DEBUG: Using video encoder: 'OMX.google.h264.encoder'
receiver thread
INFO: Renderer: direct3d
DEBUG: Trilinear filtering disabled (not an OpenGL renderer
DEBUG: Using icon (portable): D:\dev\yume-chan\scrcpy-devcontainer\scrcpy\dist\scrcpy-win64-v2.1-root_final-5-gdae2c204\icon.png
DEBUG: Demuxer 'video': starting thread
DEBUG: Demuxer 'audio': starting thread
WARN: Demuxer 'audio': stream explicitly disabled by the device
INFO: Texture: 1080x1920
DEBUG: User requested to quit
DEBUG: quit...
DEBUG: Demuxer 'video': en[server] DEBUG: Controller stopped
d[server] DEBUG: Device message sender stopped
 of frames
DEBUG: Receiver stopped
WARN: Killing the server...
DEBUG: Server disconnected
DEBUG: Server terminated

@RiggiG
Copy link
Author

RiggiG commented Jul 1, 2023

I also only have rooted Android 10 emulators. No matter whether adbd is running as root, with --root I got:

su: failed to exec -c: Permission denied

Yeah, the standard su binary that you have access to in AVD is quite a bit different than those provided by SuperSU and Magisk and the like - -c isn't even a flag as it's the default behavior. I've temporarily replaced the binary while testing in my emulator via a tmp filesystem mounted at /system/xbin so it matches the behavior of an actual rooted device. I'm out of town this weekend but I can give clear steps on that when I get back.

@huyz-git
Copy link

Using --root option will cause --stay-awake option to show error:

[server] ERROR: Could not change "stay_on_while_plugged_in"
com.genymobile.scrcpy.SettingsException: Could not access settings: put global stay_on_while_plugged_in 7
        at com.genymobile.scrcpy.Settings.execSettingsPut(Settings.java:24)
        at com.genymobile.scrcpy.Settings.putValue(Settings.java:59)
        at com.genymobile.scrcpy.Settings.getAndPutValue(Settings.java:78)
        at com.genymobile.scrcpy.Server.initAndCleanUp(Server.java:63)
        at com.genymobile.scrcpy.Server.lambda$startInitThread$2(Server.java:168)
        at com.genymobile.scrcpy.Server$$ExternalSyntheticLambda3.run(Unknown Source:2)
        at java.lang.Thread.run(Thread.java:1012)
Caused by: java.io.IOException: Command [settings, put, global, stay_on_while_plugged_in, 7] returned with value 255
        at com.genymobile.scrcpy.Command.exec(Command.java:16)
        at com.genymobile.scrcpy.Settings.execSettingsPut(Settings.java:22)
        ... 6 more

@yume-chan yume-chan mentioned this pull request Aug 9, 2023
@UltraHQ
Copy link

UltraHQ commented Sep 18, 2023

Hi, is there any update when this gets merged?

@Giantvince1
Copy link

I wanted to let you guys know of three things regarding this ongoing project.

First and foremost, I have opened a PR on RiggiG's current up-to-date dev branch to fix a few things that caused build problems.

Second, I have had the chance to test out the latest work he has done, along with my few fixups, and I've found that the screen off functionality still works perfectly. However I cannot confirm if this is the case on all Android versions, as this is only done with my Moto G Stylus 2023 model running on Android 13. As such I can't say if earlier Android versions, or even Android 14, allow this to work properly; I don't have the means to test it at this time across many versions.

And last, I noticed the issue with the settings change not taking place, erroring out when attempting to set the device to stay awake with the -w flag. My suggestion for this is to adapt the settings change code to use actual root temporarily whilst it does so IF, and ONLY if, the --root flag is provided, otherwise use only the current, existing logic. This is because it seems the AID_SYSTEM user doesn't seem to have permissions to change settings on the global table, but AID_SHELL does, causing the error when running the server side as AID_SYSTEM. I haven't managed to write this into my fixup patch set yet, but I plan to start a PoC and test it myself to see how it does on a physical, rooted, device (where su -c is valid, unlike emulators).

@RiggiG
Copy link
Author

RiggiG commented Dec 14, 2023

Hey thanks @Giantvince1, the most recent changes are just from me doing a rebase late at night recently to include new scrcpy features, did not do any testing whatsoever (and it appears there was definitely some weirdness in a handful of the conflict resolutions). I'll take a look at your fixups and we can get those merged in the near future.

I haven't really looked at much else as far as discourse here in some time, unfortunately my free time has been lacking of late. As far as testing in an emulator, you can mount a tmpfs at /system/xbin and drop in a different su binary (e.g. from SuperSU or Magisk) so the fork actually runs as expected, though I think I might just add another flag to pass alongside for emulation that'll just omit the -c so it can be tested on emulators without additional effort - will have to test that as well.

@RiggiG RiggiG marked this pull request as draft December 14, 2023 02:57
@Giantvince1
Copy link

The main reason I cannot currently test via emulator is that my current PC doesn't have enough CPU or RAM available, and its aging hard disk is worsening the effects. In the next couple months I should be able to run an AVD emulator effectively as well as test on real A14 hardware, as I soon plan to replace my laptop and upgrade my Android. It depends on other external factors but that is the end goal. In the meantime I can still check out the code, test, and investigate things as needed, albeit with only a physical A13 device on hand my testing will be a bit limited as of now.

Once I have some time later I'll rebase my fixes onto your new commits if any have been made, and/or try to implement some of the changes in my fork directly and see how it goes. I thank you for your cooperation thus far and I look forward to helping this make it to the official project!

@Giantvince1
Copy link

Did a little bit of hacking together a quick and dirty method to get and put settings whilst acting as AID_SYSTEM. This workaround is set up specifically to ONLY trigger if rooted AND the program is run with the --root flag by checking the UID it is running as. Settings changes now work perfectly, provided AID_SYSTEM actually gets root access. The only thing left is to get the clipboard to be able to go from device to PC on A13 devices, and we should be in the all clear after that point!

The only reason I haven't merged upstream's most recent commits is because the fork this PR is tied to is what I forked from, and my PR is on said fork, not this one. That being said, I see no conflicts between the current state of official dev and my dev branch; no changed files overlap, at least to my knowledge. I checked through the diffs a bit and didn't find anything that would break my changes at the very least.

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.

None yet

6 participants