[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #27609 [Applications/Tor Browser]: TBA: Evaluate Tor Onion Proxy Library



#27609: TBA: Evaluate Tor Onion Proxy Library
-------------------------------------------------+-------------------------
 Reporter:  sysrqb                               |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-mobile, TBA-a3,                  |  Actual Points:
  TorBrowserTeam201903, tbb-8.5                  |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor8
-------------------------------------------------+-------------------------

Comment (by sisbell):

 Replying to [comment:35 gk]:

 > Okay, I looked over all the patches for #29312, #29313, #29574, and
 #29575. Nice work! It feels we are pretty close here. We can use the
 parent ticket to address all comments/review requests in one place. Here
 are my 2cents:
 >
 > e38175545f4de75030fcf48b9950b65118228688 (#29312)
 >
 > Why are we using 0.3.5.6-rc (which is a release candidate and no stable
 tor)? Are we sure we are better off security- and/or stability- and/or
 performance-wise than with 0.3.4.9? We have been testing the latter for
 quite a while now but not the -rc yet and we plan to have a stable soon
 for Android users...

 Good point. I'm moving this back to 0.3.4.9. I'll do this within tor-
 android-service project.

 >
 > d79c8ef41190cc37c16c7b90cfe65f083d6a3b95 (#29313)
 > {{{
 > ++        ndk {
 > ++            abiFilters "armeabi-v7a"
 > ++        }
 > }}}
 > How does that work for x86 builds which we have now as well? Do we need
 a similar entry for them as well? If so, what is supposed to happen if we
 omitted both entries?

 This is no longer needed. We don't use VPN so don't need any native
 builds. I'll remove this from the patch.

 >
 > 8e3d539f76e3bd45a809dc1b14277d2b1a2fa3c4 (#29574)
 >
 > I assume "tor-0.3.4.9" is just part of the filename of the .aar file but
 there is no tor 0.3.4.9 or 0.3.4.9 related code in it? (Especially as you
 are using 0.3.5.6-rc) Otherwise I'd be worried about possible bad
 interactions with code belonging to different tor versions.

 We are just pulling in the android-binary library for the project. I'll
 align the versions to 0.3.4.9

 >
 > `orbot-uber.patch` is tricky to review, the diff to the older patchset
 is
 > {{{
 > diff --git a/app/build.gradle b/app/build.gradle
 > index 3051dd5c..4e33472c 100644
 > --- a/app/build.gradle
 > +++ b/app/build.gradle
 > @@ -76,12 +76,16 @@ android {
 > dependencies {
 > //    implementation 'com.github.delight-im:Android-Languages:v1.0.1'
 > implementation 'com.android.support.constraint:constraint-layout:1.1.3'
 > -    implementation project(':orbotservice')
 > // Match Fennec's ANDROID_SUPPORT_LIBRARY_VERSION
 > implementation 'com.android.support:design:23.4.0'
 > implementation 'pl.bclogic:pulsator4droid:1.0.3'
 > // These require higher versions of ANDROID_SUPPORT_LIBRARY_VERSION
 > //implementation 'com.github.apl-devs:appintro:v4.2.2'
 > //implementation 'com.github.javiersantos:AppUpdater:2.6.4'
 > -
 > +    implementation fileTree(dir: 'libs', include: ['*.jar', '*.aar'])
 > +    implementation 'com.android.support:appcompat-v7:26.1.0'
 > +    implementation 'net.freehaven.tor.control:jtorctl:0.2'
 > +    implementation 'org.torproject:tor-android-binary:0.3.5.6-rc'
 > +    implementation 'org.slf4j:slf4j-api:1.7.25'
 > +    implementation 'org.slf4j:slf4j-android:1.7.25'
 > }
 > diff --git a/build.gradle b/build.gradle
 > index edcfc84f..ce06f082 100644
 > --- a/build.gradle
 > +++ b/build.gradle
 > @@ -3,7 +3,6 @@ buildscript {
 > repositories {
 > jcenter()
 > google()
 > -        maven { url System.getenv("GRADLE_MAVEN_REPO") }
 > }
 > dependencies {
 > // Match Fennec
 > @@ -17,6 +16,5 @@ allprojects {
 > maven { url
 "https://raw.githubusercontent.com/guardianproject/gpmaven/master"; }
 > google()
 > maven { url 'https://jitpack.io' }
 > -        maven { url System.getenv("GRADLE_MAVEN_REPO") }
 > }
 > }
 > diff --git a/settings.gradle b/settings.gradle
 > index 9984a03e..e7b4def4 100644
 > --- a/settings.gradle
 > +++ b/settings.gradle
 > @@ -1,2 +1 @@
 > -include ':jsocksAndroid', ':orbotservice'
 > include ':app'
 > }}}

 I just have the uber patch for convenience. My expectation is that when we
 merge with the new UI there will be a bunch of new patches we apply and
 then can remove the uber patch. What I'll do is move the tor-android-
 service parts into a new patch(es) so its easier to track and migrate.
 This patch involves

  * Remove the building of orbotservice and jsocksAndroid
  * Add in gradle dependencies so that the orbot build can use the tor-
 android-service aar (this includes local libs for libraries to compile
 against)
  * Change gradle.build to use our local maven repo

 I'm not concerned with anything else touching orbot app code since Matt is
 handling that.

 >
 > We have a
 > {{{
 > +-    implementation 'com.android.support:appcompat-v7:27.1.1'
 > ++    // Match Fennec's version
 > ++    implementation 'com.android.support:appcompat-v7:23.4.0'
 > }}}
 > in the orbot patch. So, we explicitly downgraded appcompat to 23.4.0
 with the intent to match what Mozilla is using. However in the uber patch
 above you require 26.1.0. Is that necessary?
 I'll change back to using 23.4.0

 >
 > Additionally, we have
 > {{{
 > +     implementation 'com.jrummyapps:android-shell:1.0.1'
 > }}}
 > there, yet you remove all the `android-shell` bits from the gradle-
 dependencies-file. Thus, we can remove `android-shell` as a dependency in
 the `build.gradle` file as well?
 I'll remove this. It is no longer needed.

 >
 > ca269f08af2e17d07a0ec9d9612252348d6656f8 (#29575)
 >
 > Do we still need `android-shell` in the gradle-dependencies list given
 that you removed `orbotservice` from `build.gradle`?
 No longer needed
 >
 > The `dependencies.patch` should be done in the `tor-browser` repo. I
 guess a fixup to Matt's original patch
 (fbad5b5dabb3c45a9da853a6b784a28e78b9583c) would be fine.
 >
 > Leaving this ticket at `needs_revision` to address changes/questions.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/27609#comment:38>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs