diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e072d2a6..282b7910 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -37,12 +37,24 @@ jobs: - name: Linter (gradle) run: ./gradlew ktlintCheck + - name: Android lint (gradle) + run: ./gradlew lint --no-daemon + - name: Test (gradle) run: ./gradlew test --no-daemon + - name: Python security regressions + run: | + if [ -d tests ]; then + python3 -m unittest discover -s tests -p 'test_*.py' -v + fi + - name: Build APK (gradle) run: ./gradlew assembleDebug --no-daemon + - name: Build free release APK (gradle) + run: ./gradlew :app:assembleFreeRelease --no-daemon + - name: Upload APK uses: actions/upload-artifact@v7 with: diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavService.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavService.kt index c4002c65..57c54bab 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavService.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavService.kt @@ -1,9 +1,11 @@ package com.greenart7c3.nostrsigner.service -import com.greenart7c3.nostrsigner.okhttp.TaggedSocketFactory +import com.greenart7c3.nostrsigner.Amber +import com.greenart7c3.nostrsigner.models.TorMode +import com.greenart7c3.nostrsigner.okhttp.HttpClientManager import java.io.IOException +import java.net.Proxy import java.util.concurrent.TimeUnit -import javax.net.SocketFactory import okhttp3.Credentials import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient @@ -13,12 +15,19 @@ import okhttp3.RequestBody.Companion.toRequestBody object WebDavService { private val MEDIA_TYPE = "application/octet-stream".toMediaType() - private fun createClient(): OkHttpClient = OkHttpClient.Builder() - .connectTimeout(30, TimeUnit.SECONDS) - .readTimeout(60, TimeUnit.SECONDS) - .writeTimeout(60, TimeUnit.SECONDS) - .socketFactory(TaggedSocketFactory(SocketFactory.getDefault())) - .build() + private fun createClient(): OkHttpClient { + check(Amber.instance.isSettingsLoaded) { "Network settings are not loaded yet" } + val useProxy = Amber.instance.settings.torMode != TorMode.DISABLED + val client = HttpClientManager.getHttpClient(useProxy) + check(!useProxy || client.proxy?.type() == Proxy.Type.SOCKS) { "Tor proxy is not configured yet" } + return client.newBuilder() + .connectTimeout(30, TimeUnit.SECONDS) + .readTimeout(60, TimeUnit.SECONDS) + .writeTimeout(60, TimeUnit.SECONDS) + .followRedirects(false) + .followSslRedirects(false) + .build() + } fun uploadFile( serverUrl: String, @@ -27,6 +36,7 @@ object WebDavService { fileName: String, content: String, ): Result = try { + WebDavTransportPolicy.requireSecureServerUrl(serverUrl) val client = createClient() val url = serverUrl.trimEnd('/') + "/" + fileName val credential = Credentials.basic(username, password) @@ -54,6 +64,7 @@ object WebDavService { password: String, fileName: String, ): Result = try { + WebDavTransportPolicy.requireSecureServerUrl(serverUrl) val client = createClient() val url = serverUrl.trimEnd('/') + "/" + fileName val credential = Credentials.basic(username, password) @@ -82,6 +93,7 @@ object WebDavService { username: String, password: String, ): Result = try { + WebDavTransportPolicy.requireSecureServerUrl(serverUrl) val client = createClient() val url = serverUrl.trimEnd('/') val credential = Credentials.basic(username, password) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavTransportPolicy.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavTransportPolicy.kt new file mode 100644 index 00000000..c8e096f3 --- /dev/null +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/WebDavTransportPolicy.kt @@ -0,0 +1,15 @@ +package com.greenart7c3.nostrsigner.service + +import java.net.URI + +object WebDavTransportPolicy { + fun requireSecureServerUrl(serverUrl: String) { + val uri = URI(serverUrl) + require(uri.scheme.equals("https", ignoreCase = true) && !uri.host.isNullOrBlank()) { + "WebDAV requires an HTTPS server URL" + } + require(uri.userInfo == null && uri.query == null && uri.fragment == null) { + "WebDAV server URLs must not contain credentials, a query, or a fragment" + } + } +} diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/WebDavTransportPolicyTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/WebDavTransportPolicyTest.kt new file mode 100644 index 00000000..26212422 --- /dev/null +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/WebDavTransportPolicyTest.kt @@ -0,0 +1,18 @@ +package com.greenart7c3.nostrsigner.service + +import org.junit.Assert.assertThrows +import org.junit.Test + +class WebDavTransportPolicyTest { + @Test + fun `credentials cannot be sent to cleartext or ambiguous URLs`() { + listOf("http://backup.example", "https://user:pass@backup.example", "https://backup.example?token=secret", "https://backup.example#fragment").forEach { + assertThrows(IllegalArgumentException::class.java) { WebDavTransportPolicy.requireSecureServerUrl(it) } + } + } + + @Test + fun `secure server with a base path is accepted`() { + WebDavTransportPolicy.requireSecureServerUrl("https://backup.example:8443/remote.php/dav/files/user") + } +}