Merge branch 'security_suggestions2' into 'master'

Add dependency verification and remove information leak from OkHttp

See merge request pixeldroid/PixelDroid!456

.gitlab-ci.yml

@@ -99,7 +99,7 @@ fdroid build:
     - ln -s $CI_PROJECT_DIR/fdroidserver /home/vagrant/fdroidserver
     - mkdir -p /vagrant/cache
     - wget -q https://services.gradle.org/distributions/gradle-5.6.2-bin.zip --output-document=/vagrant/cache/gradle-5.6.2-bin.zip
-    # Check sha256 of the gralde build
+    # Check sha256 of the gradle build
     - echo '32fce6628848f799b0ad3205ae8db67d0d828c10ffe62b748a7c0d9f4a5d9ee0 /vagrant/cache/gradle-5.6.2-bin.zip' | sha256sum -c
     - bash fdroidserver/buildserver/provision-gradle
     - bash fdroidserver/buildserver/provision-apt-get-install https://deb.debian.org/debian

app/build.gradle

@@ -203,11 +203,13 @@ dependencies {
         exclude group: "com.android.support"
     }
 
-    implementation 'com.github.bumptech.glide:okhttp-integration:4.13.2'
+    implementation 'com.github.bumptech.glide:okhttp3-integration:4.13.2'
     implementation('com.github.bumptech.glide:recyclerview-integration:4.13.2') {
         // Excludes the support library because it's already included by Glide.
         transitive = false
     }
+    implementation 'com.github.bumptech.glide:annotations:4.13.2'
+    annotationProcessor 'com.github.bumptech.glide:compiler:4.13.2'
     kapt 'com.github.bumptech.glide:compiler:4.13.2'
 
     implementation 'androidx.legacy:legacy-support-v4:1.0.0'

app/licenses.yml

@@ -245,12 +245,6 @@
   license: The Apache Software License, Version 2.0
   licenseUrl: http://www.apache.org/licenses/LICENSE-2.0.txt
   url: https://developer.android.com/jetpack/androidx
-- artifact: com.github.bumptech.glide:okhttp-integration:+
-  name: okhttp-integration
-  copyrightHolder: Google Inc.
-  license: Simplified BSD License
-  licenseUrl: http://www.opensource.org/licenses/bsd-license
-  url: https://github.com/bumptech/glide
 - artifact: com.github.bumptech.glide:glide:+
   name: glide
   copyrightHolder: Google Inc.
@@ -687,10 +681,6 @@
   license: The Apache Software License, Version 2.0
   licenseUrl: https://www.apache.org/licenses/LICENSE-2.0.txt
   url: https://github.com/Kotlin/kotlinx.coroutines
-- artifact: com.squareup.okhttp:okhttp:+
-  name: okhttp
-  copyrightHolder: Square, Inc.
-  license: The Apache Software License, Version 2.0
 - artifact: com.squareup.okio:okio:+
   name: okio
   copyrightHolder: Square, Inc.
@@ -973,3 +963,9 @@
   license: The 3-Clause BSD License
   licenseUrl: https://opensource.org/licenses/BSD-3-Clause
   url: https://github.com/tanersener/smart-exception
+- artifact: com.github.bumptech.glide:okhttp3-integration:+
+  name: okhttp3-integration
+  copyrightHolder: Google Inc.
+  license: Simplified BSD License
+  licenseUrl: http://www.opensource.org/licenses/bsd-license
+  url: https://github.com/bumptech/glide

app/proguard-rules.pro

@@ -88,6 +88,9 @@
     static void throwUninitializedPropertyAccessException(java.lang.String);
 }
 
+-keep public class * extends com.bumptech.glide.module.AppGlideModule
+-keep class com.bumptech.glide.GeneratedAppGlideModuleImpl
+
 ##---------------Begin: proguard configuration for Gson  ----------
 # Gson uses generic type information stored in a class file when working with fields. Proguard
 # removes such information by default, so configure it to keep all of it.

app/src/main/java/org/pixeldroid/app/utils/GlideApplication.kt

@@ -0,0 +1,26 @@
+package org.pixeldroid.app.utils
+
+import android.content.Context
+import com.bumptech.glide.Glide
+import com.bumptech.glide.Registry
+import com.bumptech.glide.annotation.GlideModule
+import com.bumptech.glide.integration.okhttp3.OkHttpUrlLoader
+import com.bumptech.glide.load.model.GlideUrl
+import com.bumptech.glide.module.AppGlideModule
+import okhttp3.ConnectionSpec
+import okhttp3.OkHttpClient
+import org.pixeldroid.app.utils.api.PixelfedAPI
+import java.io.InputStream
+
+@GlideModule
+class PixelDroidGlideModule : AppGlideModule() {
+    override fun registerComponents(context: Context, glide: Glide, registry: Registry) {
+        val client: OkHttpClient = OkHttpClient().newBuilder()
+            // Only do secure-ish TLS connections (no HTTP or very old SSL/TLS)
+            .connectionSpecs(listOf(ConnectionSpec.MODERN_TLS))
+            .addNetworkInterceptor(PixelfedAPI.headerInterceptor)
+            .build()
+        val factory = OkHttpUrlLoader.Factory(client)
+        glide.registry.replace(GlideUrl::class.java, InputStream::class.java, factory)
+    }
+}

app/src/main/java/org/pixeldroid/app/utils/Utils.kt

@@ -27,8 +27,14 @@ import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.RecyclerView
 import com.arthenica.ffmpegkit.FFmpegKitConfig
 import com.google.android.material.color.MaterialColors
+import com.google.gson.JsonDeserializer
+import com.google.gson.JsonElement
+import com.google.gson.JsonPrimitive
+import com.google.gson.JsonSerializer
 import okhttp3.HttpUrl
 import org.pixeldroid.app.R
+import java.time.Instant
+import java.time.format.DateTimeFormatter
 import java.util.*
 import kotlin.properties.ReadWriteProperty
 import kotlin.reflect.KProperty
@@ -227,6 +233,17 @@ fun Context.themeActionBar(): Int {
 @ColorInt
 fun Context.getColorFromAttr(@AttrRes attrColor: Int): Int = MaterialColors.getColor(this, attrColor, Color.BLACK)
 
+
+val typeAdapterInstantDeserializer: JsonDeserializer<Instant> = JsonDeserializer { json: JsonElement, _, _ ->
+    DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(
+        json.asString, Instant::from
+    )
+}
+
+val typeAdapterInstantSerializer: JsonSerializer<Instant> = JsonSerializer { src: Instant, _, _ ->
+    JsonPrimitive(DateTimeFormatter.ISO_INSTANT.format(src))
+}
+
 /**
  * Delegated property to use in fragments to prevent memory leaks of bindings.
  * This makes it unnecessary to set binding to null in onDestroyView.

app/src/main/java/org/pixeldroid/app/utils/api/PixelfedAPI.kt

@@ -2,6 +2,8 @@ package org.pixeldroid.app.utils.api
 
 import com.google.gson.*
 import io.reactivex.rxjava3.core.Observable
+import okhttp3.ConnectionSpec
+import okhttp3.Interceptor
 import org.pixeldroid.app.utils.api.objects.*
 import okhttp3.MultipartBody
 import okhttp3.OkHttpClient
@@ -9,6 +11,8 @@ import org.pixeldroid.app.utils.db.AppDatabase
 import org.pixeldroid.app.utils.db.entities.UserDatabaseEntity
 import org.pixeldroid.app.utils.di.PixelfedAPIHolder
 import org.pixeldroid.app.utils.di.TokenAuthenticator
+import org.pixeldroid.app.utils.typeAdapterInstantDeserializer
+import org.pixeldroid.app.utils.typeAdapterInstantSerializer
 import retrofit2.Response
 import retrofit2.Retrofit
 import retrofit2.adapter.rxjava3.RxJava3CallAdapterFactory
@@ -30,42 +34,45 @@ interface PixelfedAPI {
 
 
     companion object {
+        val headerInterceptor = Interceptor { chain ->
+            val requestBuilder = chain.request().newBuilder()
+                .removeHeader("User-Agent")
+                .addHeader("User-Agent", "PixelDroid") //TODO check if okay?
+            chain.proceed(requestBuilder.build())
+        }
+
         fun createFromUrl(baseUrl: String): PixelfedAPI {
             return Retrofit.Builder()
                 .baseUrl(baseUrl)
                 .addConverterFactory(GsonConverterFactory.create(gSonInstance))
                 .addCallAdapterFactory(RxJava3CallAdapterFactory.create())
+                .client(
+                    OkHttpClient().newBuilder().addNetworkInterceptor(headerInterceptor)
+                        // Only do secure-ish TLS connections (no HTTP or very old SSL/TLS)
+                        .connectionSpecs(listOf(ConnectionSpec.MODERN_TLS)).build()
+                )
                 .build().create(PixelfedAPI::class.java)
         }
 
-        private var gSonInstance: Gson = GsonBuilder()
+        private val gSonInstance: Gson = GsonBuilder()
-            .registerTypeAdapter(
+            .registerTypeAdapter(Instant::class.java, typeAdapterInstantDeserializer)
-                Instant::class.java,
+            .registerTypeAdapter(Instant::class.java, typeAdapterInstantSerializer)
-                JsonDeserializer { json: JsonElement, _, _ ->
-                    DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(
-                        json.asString, Instant::from
-                    )
-                } as JsonDeserializer<Instant>).registerTypeAdapter(
-                Instant::class.java,
-                JsonSerializer { src: Instant, _, _ ->
-                    JsonPrimitive(DateTimeFormatter.ISO_INSTANT.format(src))
-                })
             .create()
 
-        private val intermediate: Retrofit.Builder = Retrofit.Builder()
-            .addConverterFactory(GsonConverterFactory.create(gSonInstance))
-            .addCallAdapterFactory(RxJava3CallAdapterFactory.create())
-
-
         fun apiForUser(
             user: UserDatabaseEntity,
             db: AppDatabase,
             pixelfedAPIHolder: PixelfedAPIHolder
         ): PixelfedAPI =
-            intermediate
+            Retrofit.Builder()
+                .addConverterFactory(GsonConverterFactory.create(gSonInstance))
+                .addCallAdapterFactory(RxJava3CallAdapterFactory.create())
                 .baseUrl(user.instance_uri)
                 .client(
-                    OkHttpClient().newBuilder().authenticator(TokenAuthenticator(user, db, pixelfedAPIHolder))
+                    OkHttpClient().newBuilder().addNetworkInterceptor(headerInterceptor)
+                            // Only do secure-ish TLS connections (no HTTP or very old SSL/TLS)
+                        .connectionSpecs(listOf(ConnectionSpec.MODERN_TLS))
+                        .authenticator(TokenAuthenticator(user, db, pixelfedAPIHolder))
                         .addInterceptor {
                             it.request().newBuilder().run {
                                 header("Accept", "application/json")

app/src/test/java/org/pixeldroid/app/APIUnitTest.kt

@@ -2,13 +2,26 @@ package org.pixeldroid.app
 
 import com.github.tomakehurst.wiremock.client.WireMock.*
 import com.github.tomakehurst.wiremock.junit.WireMockRule
+import com.google.gson.GsonBuilder
+import com.google.gson.JsonDeserializer
+import com.google.gson.JsonElement
+import com.google.gson.JsonPrimitive
+import com.google.gson.JsonSerializer
 import org.pixeldroid.app.utils.api.PixelfedAPI
 import org.pixeldroid.app.utils.api.objects.*
 import kotlinx.coroutines.runBlocking
+import okhttp3.ConnectionSpec
+import okhttp3.OkHttpClient
 import org.junit.Assert.assertEquals
 import org.junit.Rule
 import org.junit.Test
+import org.pixeldroid.app.utils.typeAdapterInstantDeserializer
+import org.pixeldroid.app.utils.typeAdapterInstantSerializer
+import retrofit2.Retrofit
+import retrofit2.adapter.rxjava3.RxJava3CallAdapterFactory
+import retrofit2.converter.gson.GsonConverterFactory
 import java.time.Instant
+import java.time.format.DateTimeFormatter
 
 
 /**
@@ -20,6 +33,23 @@ class APIUnitTest {
     @get:Rule
     var wireMockRule = WireMockRule(8089)
 
+
+    // Same as in PixelfedAPI but allow cleartext
+    private val api: PixelfedAPI = Retrofit.Builder()
+        .baseUrl("http://localhost:8089")
+        .addConverterFactory(GsonConverterFactory.create(GsonBuilder()
+            .registerTypeAdapter(Instant::class.java, typeAdapterInstantDeserializer)
+            .registerTypeAdapter(Instant::class.java, typeAdapterInstantSerializer)
+            .create())
+        )
+        .addCallAdapterFactory(RxJava3CallAdapterFactory.create())
+        .client(
+            OkHttpClient().newBuilder().addNetworkInterceptor(PixelfedAPI.headerInterceptor)
+                // Allow cleartext
+                .connectionSpecs(listOf(ConnectionSpec.CLEARTEXT)).build()
+        )
+        .build().create(PixelfedAPI::class.java)
+
     @Test
     fun api_correctly_translated_data_class() {
         stubFor(
@@ -44,9 +74,10 @@ class APIUnitTest {
         val statusesHome: List<Status>
 
         runBlocking {
-            statuses = PixelfedAPI.createFromUrl("http://localhost:8089")
+
+            statuses = api
                     .timelinePublic(null, null, null, null, null)
-            statusesHome = PixelfedAPI.createFromUrl("http://localhost:8089")
+            statusesHome = api
                 .timelineHome(null, null, null, null, null)
         }
 
@@ -69,8 +100,7 @@ class APIUnitTest {
                         .withBody(""" {"id":3197,"name":"Pixeldroid","website":null,"redirect_uri":"urn:ietf:wg:oauth:2.0:oob","client_id":3197,"client_secret":"hhRwLupqUJPghKsZzpZtxNV67g5DBdPYCqW6XE3m","vapid_key":null}"""
                         )))
         val application: Application = runBlocking {
-            PixelfedAPI.createFromUrl("http://localhost:8089")
+            api.registerApplication("Pixeldroid", "urn:ietf:wg:oauth:2.0:oob", "read write follow")
-                .registerApplication("Pixeldroid", "urn:ietf:wg:oauth:2.0:oob", "read write follow")
         }
 
         assertEquals("3197", application.client_id)
@@ -100,8 +130,7 @@ class APIUnitTest {
         val PACKAGE_ID = "org.pixeldroid.app"
 
         val token: Token =  runBlocking {
-            PixelfedAPI.createFromUrl("http://localhost:8089")
+            api.obtainToken(
-                .obtainToken(
                     "123", "ssqdfqsdfqds", "$OAUTH_SCHEME://$PACKAGE_ID", SCOPE, "abc",
                     "authorization_code"
                 )