Opened 4 months ago

Closed 2 months ago

#32172 closed task (implemented)

port test suite to Android to run in emulator

Reported by: eighthave Owned by:
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: Android, tbb-mobile
Cc: n8fr8, sysrqb Actual Points: .5
Parent ID: Points:
Reviewer: ahf Sponsor:

Description

Attached is the first stab at getting something from the test suite running in Android.

  • there is no /tmp on Android, there is /data/local/tmp for root and the shell user
  • no use in testing the user ID stuff, Android apps cannot ever change users

To get the full suite running, there will need to be larger changes:

  • either switch Python code to plain sh or hack to get Python running in Android emulator
  • port sh scripts to Android, it is not a full UNIX environment, so things like printf and others are not always there.
  • the shebang will need to be settable to #!/system/bin/sh or the tests need to be executed using sh test_keygen.sh

Child Tickets

Attachments (1)

0001-first-port-of-test-suite-to-Android-to-run-in-emulat.patch (2.3 KB) - added by eighthave 4 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 4 months ago by eighthave

comment:2 Changed 4 months ago by nickm

Milestone: Tor: 0.4.3.x-final
Reviewer: nickm

comment:3 Changed 4 months ago by nickm

Status: newneeds_review

comment:4 Changed 4 months ago by nickm

Status: needs_reviewneeds_revision

Good start! I've left a couple of comments on the PR. Additionally this needs a changes file.

WRT the other suggestions:

  • We already disable the python tests when python isn't found. Is there a place where we forget to do that?
  • I'm okay porting our shell scripts to a more generic sh, so long as shellcheck still passes.
  • I don't know what to do with the third one; maybe there's a way to make autoconf and automake override this? Is there a /usr/bin/env sh we could use?

comment:5 Changed 4 months ago by eighthave

I haven't even tried to use make check here, that will take a chunk of work. I assume that's where the python detection is. I'm just directly calling the test executables after copying them to the emulator, like this:

adb -e shell /data/local/tmp/test
adb -e shell /data/local/tmp/test-memwipe
adb -e shell /data/local/tmp/test-slow

From my very vague understanding of the all the bits of the test suite, I'm guessing there will be some that won't be worth porting to run in the Android emulator. So I think the question now is: what parts must be ported to have decent coverage? What parts are you worried about?

comment:6 Changed 4 months ago by teor

iOS also has a similar restriction on /tmp, apps get their own container directory with their own tmp.
https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html

comment:7 Changed 4 months ago by eighthave

Status: needs_revisionneeds_review

Since Android has no /tmp/ or mkdtemp(), I changed this to fake it, and check the results of mkdir() in the process.

That took too long, ./configure with cross bulids is always painful, and tor is unfortunately no exception. Then the tests have to run in an emulator. I spent at least 3 hours updating this.

comment:8 Changed 4 months ago by nickm

Status: needs_reviewneeds_revision

Looking better; I've added some questions and comments to the commit.

This still needs a changes file.

comment:9 Changed 4 months ago by eighthave

added a changes files

comment:10 Changed 4 months ago by teor

Status: needs_revisionneeds_review

comment:11 Changed 3 months ago by nickm

Reviewer: nickmahf

I never found that changes file, so I've made a new one. I've also fixed a couple of bugs in the old code (failure to delete correct directory, using decimal for chmod bits) and tried to document the stuff that wasn't explained.

I've made a new branch called ticket32172_redux with a PR at https://github.com/torproject/tor/pull/1592

I don't know if it will still work, or what the answers to the comments is. I think this needs another reviewer. I'll ask ahf what the thinks.

comment:12 Changed 3 months ago by nickm

[before mereging, we should IMO either understand the permissions issue that makes the setuid/setgid bits and the subdirectory necessary, or conclude that those are not necessary and remove them. ]

comment:13 Changed 3 months ago by eighthave

FYI, I'm using and maintaining this in the Guardian Project fork. All other versions of this could be outdated, as far as I know. https://github.com/guardianproject/tor specifically the tor-android-0.4.2 and tor-android-0.4.1 branches NOT master.

comment:14 Changed 3 months ago by nickm

Status: needs_reviewneeds_revision

Ahh, it looks like your current patch is much simpler:

+#elif defined(__ANDROID__)
+  /* tor might not like the default perms, so create a subdir */
+  tor_snprintf(temp_dir, sizeof(temp_dir),
+               "/data/local/tmp/tor_%d_%d_%s",
+               (int) getuid(), (int) getpid(), rnd32);
+  r = mkdir(temp_dir, 0700);
+  if (r) {
+    fprintf(stderr, "Can't create directory %s:", temp_dir);
+    perror("");
+    exit(1);

This is a lot more reasonable than what was in the branch before. I'll see if I can extract that into something mergeable.

comment:15 Changed 3 months ago by nickm

Latest, much simpler, version that copies the guardian project code is ticket32172_once_again, with PR at https://github.com/torproject/tor/pull/1595

comment:16 Changed 3 months ago by nickm

Actual Points: .5
Status: needs_revisionneeds_review

comment:17 Changed 3 months ago by nickm

(Deleted "ack wrong ticket" comment as incorrect.)

comment:18 Changed 2 months ago by ahf

Status: needs_reviewmerge_ready

Nice with the simplified patch. Looks good.

comment:19 Changed 2 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged to master.

Note: See TracTickets for help on using tickets.