Skip to content

Commit

Permalink
(android) Skip TLS check for onion Electrum servers (#479)
Browse files Browse the repository at this point in the history 
Checking TLS certificates on onion address is not useful and
should be skipped by default.

Note that there's also an issue with TLS with onion services in
our tor layer which will need investigation.
  • Loading branch information
@dpad85
dpad85 authored Nov 30, 2023
1 parent ab1f104 commit ac4bd7a
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 67 deletions.
2 changes: 1 addition & 1 deletion phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/payments/ScanDataView.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ fun ReadDataView(
}

if (model is Scan.Model.LnurlServiceFetch) {
Card(modifier = Modifier.align(Alignment.Center), internalPadding = PaddingValues(24.dp)) {
Card(modifier = Modifier.align(Alignment.Center), internalPadding = PaddingValues(horizontal = 12.dp, vertical = 8.dp)) {
ProgressView(text = stringResource(R.string.scan_lnurl_fetching))
}
}
Expand Down
30 changes: 21 additions & 9 deletions ...nix/android/settings/ElectrumViewModel.kt → ...droid/settings/ElectrumDialogViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@

package fr.acinq.phoenix.android.settings

import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import fr.acinq.lightning.io.TcpSocket
import fr.acinq.lightning.utils.ServerAddress
import io.ktor.network.selector.*
import io.ktor.network.sockets.*
import io.ktor.network.tls.*
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay
import kotlinx.coroutines.withContext
Expand All @@ -31,21 +37,28 @@ import java.security.cert.CertPathValidatorException
import java.security.cert.Certificate
import kotlin.time.Duration.Companion.seconds

class ElectrumViewModel : ViewModel() {
class ElectrumDialogViewModel : ViewModel() {
val log: Logger = LoggerFactory.getLogger(this::class.java)

internal suspend fun checkCertificate(host: String, port: Int): CertificateCheckState {
return withContext(viewModelScope.coroutineContext + Dispatchers.IO) {
var state by mutableStateOf<CertificateCheckState>(CertificateCheckState.Init)

internal suspend fun checkCertificate(host: String, port: Int, onCertificateValid: (ServerAddress) -> Unit) {
return withContext(viewModelScope.coroutineContext + Dispatchers.IO + CoroutineExceptionHandler { _, e ->
log.error("error in checkCertificate: ", e)
state = CertificateCheckState.Failure(e)
}) {
state = CertificateCheckState.Checking
delay(500) // add a small pause for better ux
try {
withTimeout(10.seconds) {
val socket = aSocket(ActorSelectorManager(Dispatchers.IO)).tcp().connect(host, port).tls(viewModelScope.coroutineContext + Dispatchers.IO)
val socket = aSocket(ActorSelectorManager(Dispatchers.IO)).tcp().connect(host, port).tls(this.coroutineContext)
socket.close()
CertificateCheckState.Valid(host, port)
onCertificateValid(ServerAddress(host, port, TcpSocket.TLS.TRUSTED_CERTIFICATES()))
state = CertificateCheckState.Init
}
} catch (e: Exception) {
log.error("failed to connect to $host:$port: ${e.message}: ", e)
when (e) {
state = when (e) {
is java.security.cert.CertificateException -> {
val cause = e.cause
if (cause is CertPathValidatorException) {
Expand Down Expand Up @@ -75,11 +88,10 @@ class ElectrumViewModel : ViewModel() {
}
}

internal sealed class CertificateCheckState {
sealed class CertificateCheckState {
object Init : CertificateCheckState()
object Checking : CertificateCheckState()
data class Valid(val host: String, val port: Int) : CertificateCheckState()
data class Rejected(val host: String, val port: Int, val certificate: Certificate) : CertificateCheckState()
data class Failure(val e: Exception) : CertificateCheckState()
data class Failure(val e: Throwable) : CertificateCheckState()
}
}
104 changes: 58 additions & 46 deletions phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/settings/ElectrumView.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import androidx.lifecycle.viewmodel.compose.viewModel
import fr.acinq.lightning.blockchain.fee.FeeratePerByte
import fr.acinq.lightning.io.TcpSocket
import fr.acinq.lightning.utils.Connection
import fr.acinq.lightning.utils.ServerAddress
import fr.acinq.phoenix.android.*
import fr.acinq.phoenix.android.R
import fr.acinq.phoenix.android.components.*
import fr.acinq.phoenix.android.components.feedback.ErrorMessage
import fr.acinq.phoenix.android.components.mvi.MVIView
import fr.acinq.phoenix.android.utils.*
import fr.acinq.phoenix.android.utils.datastore.UserPrefs
Expand All @@ -62,7 +62,6 @@ fun ElectrumView() {
val scope = rememberCoroutineScope()
val prefElectrumServer = LocalElectrumServer.current
var showCustomServerDialog by rememberSaveable { mutableStateOf(false) }
val vm = viewModel<ElectrumViewModel>()

DefaultScreenLayout {
DefaultScreenHeader(
Expand All @@ -78,19 +77,14 @@ fun ElectrumView() {
if (showCustomServerDialog) {
ElectrumServerDialog(
initialAddress = prefElectrumServer,
onCheck = vm::checkCertificate,
onConfirm = { address ->
scope.launch {
UserPrefs.saveElectrumServer(context, address)
postIntent(ElectrumConfiguration.Intent.UpdateElectrumServer(address))
showCustomServerDialog = false
}
},
onDismiss = {
scope.launch {
showCustomServerDialog = false
}
}
onDismiss = { showCustomServerDialog = false }
)
}

Expand Down Expand Up @@ -154,17 +148,22 @@ fun ElectrumView() {
@Composable
private fun ElectrumServerDialog(
initialAddress: ServerAddress?,
onCheck: suspend (String, Int) -> ElectrumViewModel.CertificateCheckState,
onConfirm: (ServerAddress?) -> Unit,
onDismiss: () -> Unit
) {
val context = LocalContext.current
val scope = rememberCoroutineScope()
val keyboardManager = LocalSoftwareKeyboardController.current

var useCustomServer by rememberSaveable { mutableStateOf(initialAddress != null) }
var address by rememberSaveable { mutableStateOf(initialAddress?.run { "$host:$port" } ?: "") }
val host = remember(address) { address.trim().substringBeforeLast(":").takeIf { it.isNotBlank() } }
val port = remember(address) { address.trim().substringAfterLast(":").toIntOrNull() ?: 50002 }
val isOnionHost = remember(address) { host?.endsWith(".onion") ?: false }

var addressError by rememberSaveable { mutableStateOf(false) }
var connectionCheck by remember { mutableStateOf<ElectrumViewModel.CertificateCheckState>(ElectrumViewModel.CertificateCheckState.Init) }

val vm = viewModel<ElectrumDialogViewModel>()

Dialog(
onDismiss = onDismiss,
Expand All @@ -181,43 +180,55 @@ private fun ElectrumServerDialog(
text = stringResource(id = R.string.electrum_dialog_checkbox),
checked = useCustomServer,
onCheckedChange = {
useCustomServer = it
if (!it) {
connectionCheck = ElectrumViewModel.CertificateCheckState.Init
addressError = false
if (useCustomServer != it) {
vm.state = ElectrumDialogViewModel.CertificateCheckState.Init
}
useCustomServer = it
},
enabled = connectionCheck != ElectrumViewModel.CertificateCheckState.Checking
enabled = vm.state !is ElectrumDialogViewModel.CertificateCheckState.Checking
)
TextInput(
modifier = Modifier.fillMaxWidth(),
text = address,
onTextChange = {
addressError = false
if (address != it) {
connectionCheck = ElectrumViewModel.CertificateCheckState.Init
vm.state = ElectrumDialogViewModel.CertificateCheckState.Init
}
address = it
},
maxLines = 4,
staticLabel = stringResource(id = R.string.electrum_dialog_input),
enabled = useCustomServer && connectionCheck != ElectrumViewModel.CertificateCheckState.Checking
enabled = useCustomServer && vm.state !is ElectrumDialogViewModel.CertificateCheckState.Checking,
errorMessage = if (addressError) stringResource(id = R.string.electrum_dialog_invalid_input) else null
)
if (addressError) {
Text(stringResource(id = R.string.electrum_dialog_invalid_input))
if (isOnionHost) {
TextWithIcon(
text = stringResource(id = R.string.electrum_connection_dialog_onion_port),
textStyle = MaterialTheme.typography.subtitle2,
icon = R.drawable.ic_info,
)
} else {
TextWithIcon(
text = stringResource(id = R.string.electrum_connection_dialog_tls_port),
textStyle = MaterialTheme.typography.subtitle2,
icon = R.drawable.ic_info,
)
}
}
Spacer(modifier = Modifier.height(24.dp))
when (val check = connectionCheck) {
ElectrumViewModel.CertificateCheckState.Init, is ElectrumViewModel.CertificateCheckState.Failure -> {
if (check is ElectrumViewModel.CertificateCheckState.Failure) {
Text(
text = stringResource(
R.string.electrum_dialog_cert_failure, when (check.e) {
is UnresolvedAddressException -> stringResource(R.string.electrum_dialog_cert_unresolved)
else -> check.e.message ?: check.e.javaClass.simpleName
}
),
modifier = Modifier.padding(horizontal = 24.dp)
Spacer(modifier = Modifier.height(12.dp))
when (val state = vm.state) {
ElectrumDialogViewModel.CertificateCheckState.Init, is ElectrumDialogViewModel.CertificateCheckState.Failure -> {
if (state is ElectrumDialogViewModel.CertificateCheckState.Failure) {
ErrorMessage(
header = stringResource(R.string.electrum_dialog_cert_failure),
details = when (state.e) {
is UnresolvedAddressException -> stringResource(R.string.electrum_dialog_cert_unresolved)
else -> state.e.message ?: state.e.javaClass.simpleName
},
alignment = Alignment.CenterHorizontally,
modifier = Modifier.fillMaxWidth()
)
Spacer(Modifier.height(16.dp))
}
Expand All @@ -227,13 +238,14 @@ private fun ElectrumServerDialog(
onClick = {
keyboardManager?.hide()
if (useCustomServer) {
if (address.matches("""(.*):*(\d*)""".toRegex())) {
val (host, port) = address.trim().run {
substringBeforeLast(":") to (substringAfterLast(":").toIntOrNull() ?: 50002)
}
val host = host
if (address.matches("""(.*):*(\d*)""".toRegex()) && host != null) {
scope.launch {
connectionCheck = ElectrumViewModel.CertificateCheckState.Checking
connectionCheck = onCheck(host, port)
if (isOnionHost) {
onConfirm(ServerAddress(host, port, TcpSocket.TLS.DISABLED))
} else {
vm.checkCertificate(host, port, onCertificateValid = onConfirm)
}
}
} else {
addressError = true
Expand All @@ -242,22 +254,22 @@ private fun ElectrumServerDialog(
onConfirm(null)
}
},
text = stringResource(id = R.string.electrum_dialog_cert_check_button),
enabled = !addressError,
text = if (useCustomServer) {
stringResource(id = R.string.electrum_dialog_cert_check_button)
} else {
stringResource(id = R.string.btn_ok)
},
)
}
}
ElectrumViewModel.CertificateCheckState.Checking -> {
ElectrumDialogViewModel.CertificateCheckState.Checking -> {
Row(Modifier.align(Alignment.End)) {
ProgressView(text = stringResource(id = R.string.electrum_dialog_cert_checking))
}
}
is ElectrumViewModel.CertificateCheckState.Valid -> {
LaunchedEffect(Unit) {
onConfirm(ServerAddress(check.host, check.port, TcpSocket.TLS.TRUSTED_CERTIFICATES()))
}
}
is ElectrumViewModel.CertificateCheckState.Rejected -> {
val cert = check.certificate
is ElectrumDialogViewModel.CertificateCheckState.Rejected -> {
val cert = state.certificate
Column(
Modifier
.background(mutedBgColor)
Expand Down Expand Up @@ -313,7 +325,7 @@ private fun ElectrumServerDialog(
icon = R.drawable.ic_check_circle,
iconTint = positiveColor,
space = 8.dp,
onClick = { onConfirm(ServerAddress(check.host, check.port, TcpSocket.TLS.PINNED_PUBLIC_KEY(Base64.encodeToString(cert.publicKey.encoded, Base64.NO_WRAP)))) },
onClick = { onConfirm(ServerAddress(state.host, state.port, TcpSocket.TLS.PINNED_PUBLIC_KEY(Base64.encodeToString(cert.publicKey.encoded, Base64.NO_WRAP)))) },
)
}
}
Expand Down
5 changes: 1 addition & 4 deletions phoenix-android/src/main/res/values-cs/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,6 @@
<string name="electrum_title">Electrum server</string>
<string name="electrum_about">K zabezpečení vašich platebních kanálů Phoenix monitoruje Bitcoinový blockchain skrz Electrum servery.\n\nVe výchozím nastavení se používají náhodné servery. Phoenix můžete také nakonfigurovat tak, aby se připojoval pouze k vašemu vlastnímu serveru.</string>
<string name="electrum_block_height_label">Výška bloku</string>
<string name="electrum_tip_label">Časové razítko tipu</string>
<string name="electrum_fee_rate_next_label">Poplatky za 1 blok</string>
<string name="electrum_fee_rate_funding_label">Poplatky za 144 bloků</string>
<string name="electrum_connection_label">Server</string>

<string name="electrum_connection_closed_with_random">Odpojeno od Electrum</string>
Expand All @@ -553,7 +550,7 @@
<string name="electrum_dialog_invalid_input">Tato adresa je neplatná.</string>
<string name="electrum_dialog_cert_check_button">Připojit</string>
<string name="electrum_dialog_cert_checking">Kontrola certifikátu…</string>
<string name="electrum_dialog_cert_failure">Nepodařilo se připojit:\n%1$s</string>
<string name="electrum_dialog_cert_failure">Nepodařilo se připojit</string>
<string name="electrum_dialog_cert_unresolved">Adresu se nepodařilo přeložit.</string>
<string name="electrum_dialog_cert_header">Nedůvěryhodný certifikát</string>
<string name="electrum_dialog_cert_sha1">SHA1 Fingerprint</string>
Expand Down
4 changes: 1 addition & 3 deletions phoenix-android/src/main/res/values-de/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,6 @@
<string name="electrum_title">Electrum-Server</string>
<string name="electrum_about">Um Ihre Zahlungskanäle zu sichern, überwacht Phoenix die Bitcoin-Blockchain über Electrum-Server.\n\nStandardmäßig werden zufällige Server verwendet. Sie können Phoenix auch so konfigurieren, dass es sich nur mit Ihrem eigenen Server verbindet.</string>
<string name="electrum_block_height_label">Blockhöhe</string>
<string name="electrum_fee_rate_next_label">Gebührenrate 1 Block</string>
<string name="electrum_fee_rate_funding_label">Gebührenrate 144 Blöcke</string>
<string name="electrum_connection_label">Server</string>

<string name="electrum_connection_closed_with_random">Keine Verbindung zu Electrum</string>
Expand All @@ -524,7 +522,7 @@
<string name="electrum_dialog_invalid_input">Diese Adresse ist ungültig.</string>
<string name="electrum_dialog_cert_check_button">Verbinden</string>
<string name="electrum_dialog_cert_checking">Prüfe Zertifikat…</string>
<string name="electrum_dialog_cert_failure">Verbindung fehlgeschlagen:\n%1$s</string>
<string name="electrum_dialog_cert_failure">Verbindung fehlgeschlagen</string>
<string name="electrum_dialog_cert_unresolved">Diese Adresse konnte nicht aufgelöst werden.</string>
<string name="electrum_dialog_cert_header">Nicht vertrauenswürdiges Zertifikat</string>
<string name="electrum_dialog_cert_sha1">SHA1 Fingerabdruck</string>
Expand Down
7 changes: 3 additions & 4 deletions phoenix-android/src/main/res/values/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,9 @@
<string name="electrum_title">Electrum server</string>
<string name="electrum_about">To secure your payment channels Phoenix monitors the Bitcoin blockchain through Electrum servers.\n\nBy default, random servers are used. You can also configure Phoenix to connect only to your own server.</string>
<string name="electrum_block_height_label">Block height</string>
<string name="electrum_tip_label">Tip timestamp</string>
<string name="electrum_fee_rate_next_label">Feerate 1 block</string>
<string name="electrum_fee_rate_funding_label">Feerate 144 blocks</string>
<string name="electrum_connection_label">Server</string>
<string name="electrum_connection_dialog_tls_port">Use the TLS port (default 50002).</string>
<string name="electrum_connection_dialog_onion_port">Use the plain TCP port, not the TLS one, since this is an onion service.</string>

<string name="electrum_connection_closed_with_random">Disconnected from Electrum</string>
<string name="electrum_connection_closed_with_custom">Disconnected from %1$s</string>
Expand All @@ -595,7 +594,7 @@
<string name="electrum_dialog_invalid_input">This address is invalid.</string>
<string name="electrum_dialog_cert_check_button">Connect</string>
<string name="electrum_dialog_cert_checking">Checking certificate…</string>
<string name="electrum_dialog_cert_failure">Failed to connect:\n%1$s</string>
<string name="electrum_dialog_cert_failure">Failed to connect</string>
<string name="electrum_dialog_cert_unresolved">This address cannot be resolved.</string>
<string name="electrum_dialog_cert_header">Untrusted certificate</string>
<string name="electrum_dialog_cert_sha1">SHA1 Fingerprint</string>
Expand Down

0 comments on commit ac4bd7a

Please sign in to comment.