Skip to content

Commit

Permalink
catch exceptions during initial NFC connections
Browse files Browse the repository at this point in the history
  • Loading branch information
AdamVe committed Sep 12, 2024
1 parent 7fa2786 commit ea0a7d2
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ abstract class AppContextManager {

open fun onPause() {}

open fun onError() {}
open fun onError(e: Exception) {}
}

class ContextDisposedException : Exception()
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import kotlinx.coroutines.launch
import org.json.JSONObject
import org.slf4j.LoggerFactory
import java.io.Closeable
import java.io.IOException
import java.security.NoSuchAlgorithmException
import java.util.concurrent.Executors
import javax.crypto.Mac
Expand Down Expand Up @@ -310,43 +311,58 @@ class MainActivity : FlutterFragmentActivity() {
}

private suspend fun processYubiKey(device: YubiKeyDevice) {
val deviceInfo = getDeviceInfo(device)
val deviceInfo = try {

if (deviceInfo == null) {
deviceManager.setDeviceInfo(null)
return
}

if (device is NfcYubiKeyDevice) {
appMethodChannel.nfcStateChanged(NfcState.ONGOING)
}
if (device is NfcYubiKeyDevice) {
appMethodChannel.nfcStateChanged(NfcState.ONGOING)
}

deviceManager.scpKeyParams = null
// If NFC and FIPS check for SCP11b key
if (device.transport == Transport.NFC && deviceInfo.fipsCapable != 0) {
logger.debug("Checking for usable SCP11b key...")
deviceManager.scpKeyParams = try {
device.withConnection<SmartCardConnection, ScpKeyParams?> { connection ->
val scp = SecurityDomainSession(connection)
val keyRef = scp.keyInformation.keys.firstOrNull { it.kid == ScpKid.SCP11b }
keyRef?.let {
val certs = scp.getCertificateBundle(it)
if (certs.isNotEmpty()) Scp11KeyParams(
keyRef,
certs[certs.size - 1].publicKey
) else null
}?.also {
logger.debug("Found SCP11b key: {}", keyRef)
val deviceInfo = getDeviceInfo(device)

deviceManager.scpKeyParams = null
// If NFC and FIPS check for SCP11b key
if (device.transport == Transport.NFC && deviceInfo.fipsCapable != 0) {
logger.debug("Checking for usable SCP11b key...")
deviceManager.scpKeyParams = try {
device.withConnection<SmartCardConnection, ScpKeyParams?> { connection ->
val scp = SecurityDomainSession(connection)
val keyRef = scp.keyInformation.keys.firstOrNull { it.kid == ScpKid.SCP11b }
keyRef?.let {
val certs = scp.getCertificateBundle(it)
if (certs.isNotEmpty()) Scp11KeyParams(
keyRef,
certs[certs.size - 1].publicKey
) else null
}?.also {
logger.debug("Found SCP11b key: {}", keyRef)
}
}
} catch (e: Exception) {
logger.error("Exception when reading SCP key information: ", e)
// we throw IO exception to unify handling failures as we don't want
// th clear device info
throw IOException("Failure getting SCP keys")
}
} catch (e: Exception) {
logger.debug("Exception while getting scp keys: ", e)
contextManager?.onError()
if (device is NfcYubiKeyDevice) {
appMethodChannel.nfcStateChanged(NfcState.FAILURE)
}
null
}
deviceInfo
} catch (e: Exception) {
logger.debug("Exception while getting device info and scp keys: ", e)
contextManager?.onError(e)
if (device is NfcYubiKeyDevice) {
logger.debug("Setting NFC state to failure")
appMethodChannel.nfcStateChanged(NfcState.FAILURE)
}

// do not clear the device info when IOException's occur,
// this allows for retries of failed actions
if (e !is IOException) {
logger.debug("Resetting device info")
deviceManager.setDeviceInfo(null)
} else {
logger.debug("Keeping device info")
}

return
}

// this YubiKey provides SCP11b key but the phone cannot perform AESCMAC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ class FidoConnectionHelper(private val deviceManager: DeviceManager) {
return requestHandled
}

fun failPending(e: Exception) {
pendingAction?.let { action ->
logger.error("Failing pending action with {}", e.message)
action.invoke(Result.failure(e))
pendingAction = null
}
}

fun cancelPending() {
pendingAction?.let { action ->
action.invoke(Result.failure(CancellationException()))
Expand Down Expand Up @@ -80,7 +72,7 @@ class FidoConnectionHelper(private val deviceManager: DeviceManager) {
block(YubiKitFidoSession(it))
}.also {
if (updateDeviceInfo) {
deviceManager.setDeviceInfo(getDeviceInfo(device))
deviceManager.setDeviceInfo(runCatching { getDeviceInfo(device) }.getOrNull())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ class FidoManager(
}
}

override fun onError() {
super.onError()
logger.debug("Cancel any pending action because of upstream error")
override fun onError(e: Exception) {
super.onError(e)
logger.error("Cancelling pending action. Cause: ", e)
connectionHelper.cancelPending()
}

Expand Down Expand Up @@ -204,13 +204,12 @@ class FidoManager(
}

if (updateDeviceInfo.getAndSet(false)) {
deviceManager.setDeviceInfo(getDeviceInfo(device))
deviceManager.setDeviceInfo(runCatching { getDeviceInfo(device) }.getOrNull())
}
} catch (e: Exception) {
// something went wrong, try to get DeviceInfo from any available connection type
logger.error("Failure when processing YubiKey: ", e)

connectionHelper.failPending(e)
logger.error("Cancelling pending action. Cause: ", e)
connectionHelper.cancelPending()

if (e !is IOException) {
// we don't clear the session on IOExceptions so that the session is ready for
Expand Down Expand Up @@ -240,7 +239,7 @@ class FidoManager(
currentSession
)

val sameDevice = currentSession == previousSession
val sameDevice = currentSession.sameDevice(previousSession)

if (device is NfcYubiKeyDevice && (sameDevice || resetHelper.inProgress)) {
requestHandled = connectionHelper.invokePending(fidoSession)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ data class Options(
infoData.getOptionsBoolean("ep"),
)

fun sameDevice(other: Options) : Boolean {
if (this === other) return true

if (clientPin != other.clientPin) return false
if (credMgmt != other.credMgmt) return false
if (credentialMgmtPreview != other.credentialMgmtPreview) return false
if (bioEnroll != other.bioEnroll) return false
// alwaysUv may differ
// ep may differ

return true
}

companion object {
private fun InfoData.getOptionsBoolean(
key: String
Expand All @@ -67,6 +80,21 @@ data class SessionInfo(
infoData.remainingDiscoverableCredentials
)

// this is a more permissive comparison, which does not take in an account properties,
// which might change by using the FIDO authenticator
fun sameDevice(other: SessionInfo?): Boolean {
if (other == null) return false
if (this === other) return true

if (!options.sameDevice(other.options)) return false
if (!aaguid.contentEquals(other.aaguid)) return false
// minPinLength may differ
// forcePinChange may differ
// remainingDiscoverableCredentials may differ

return true
}

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ class OathManager(
private val updateDeviceInfo = AtomicBoolean(false)
private var deviceInfoTimer: TimerTask? = null

override fun onError() {
super.onError()
logger.debug("Cancel any pending action because of upstream error")
override fun onError(e: Exception) {
super.onError(e)
logger.error("Cancelling pending action in onError. Cause: ", e)
pendingAction?.let { action ->
action.invoke(Result.failure(CancellationException()))
pendingAction = null
Expand Down Expand Up @@ -343,16 +343,16 @@ class OathManager(
)

if (updateDeviceInfo.getAndSet(false)) {
deviceManager.setDeviceInfo(getDeviceInfo(device))
deviceManager.setDeviceInfo(runCatching { getDeviceInfo(device) }.getOrNull())
}
} catch (e: Exception) {
// OATH not enabled/supported, try to get DeviceInfo over other USB interfaces
logger.error("Exception during SmartCard connection/OATH session creation: ", e)

// Remove any pending action
// Cancel any pending action
pendingAction?.let { action ->
logger.error("Failing pending action with {}", e.message)
action.invoke(Result.failure(e))
logger.error("Cancelling pending action. Cause: ", e)
action.invoke(Result.failure(CancellationException()))
pendingAction = null
}

Expand Down Expand Up @@ -782,7 +782,7 @@ class OathManager(
block(getOathSession(it))
}.also {
if (updateDeviceInfo) {
deviceManager.setDeviceInfo(getDeviceInfo(device))
deviceManager.setDeviceInfo(runCatching { getDeviceInfo(device) }.getOrNull())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ class DeviceInfoHelper {
private val restrictedNfcBytes =
byteArrayOf(0x00, 0x1F, 0xD1.toByte(), 0x01, 0x1b, 0x55, 0x04) + uri

suspend fun getDeviceInfo(device: YubiKeyDevice): Info? {
suspend fun getDeviceInfo(device: YubiKeyDevice): Info {
SessionVersionOverride.set(null)
var deviceInfo = readDeviceInfo(device)
if (deviceInfo?.version?.major == 0.toByte()) {
if (deviceInfo.version.major == 0.toByte()) {
SessionVersionOverride.set(Version(5, 7, 2))
deviceInfo = readDeviceInfo(device)
}
return deviceInfo
}

private suspend fun readDeviceInfo(device: YubiKeyDevice): Info? {
private suspend fun readDeviceInfo(device: YubiKeyDevice): Info {
val pid = (device as? UsbYubiKeyDevice)?.pid

val deviceInfo = runCatching {
Expand Down Expand Up @@ -106,8 +106,8 @@ class DeviceInfoHelper {
}
} catch (e: Exception) {
// no smart card connectivity
logger.error("Failure getting device info", e)
return null
logger.error("Failure getting device info: ", e)
throw e
}
}

Expand Down
3 changes: 1 addition & 2 deletions lib/android/fido/state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,8 @@ class _FidoCredentialsNotifier extends FidoCredentialsNotifier {
var decodedException = pe.decode();
if (decodedException is CancellationException) {
_log.debug('User cancelled delete credential FIDO operation');
} else {
throw decodedException;
}
throw decodedException;
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/app/views/main_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class MainPage extends ConsumerWidget {
final prevSerial =
prev?.hasValue == true ? prev?.value?.info.serial : null;
if ((serial != null && serial == prevSerial) ||
(next.hasValue && (prev != null && prev.isLoading))) {
(next.hasValue && (prev != null && prev.isLoading)) ||
next.isLoading) {
return;
}

Expand Down
23 changes: 14 additions & 9 deletions lib/fido/views/delete_credential_dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart';
import '../../app/message.dart';
import '../../app/models.dart';
import '../../app/state.dart';
import '../../exception/cancellation_exception.dart';
import '../../widgets/responsive_dialog.dart';
import '../models.dart';
import '../state.dart';
Expand Down Expand Up @@ -57,15 +58,19 @@ class DeleteCredentialDialog extends ConsumerWidget {
actions: [
TextButton(
onPressed: () async {
await ref
.read(credentialProvider(devicePath).notifier)
.deleteCredential(credential);
await ref.read(withContextProvider)(
(context) async {
Navigator.of(context).pop(true);
showMessage(context, l10n.s_passkey_deleted);
},
);
try {
await ref
.read(credentialProvider(devicePath).notifier)
.deleteCredential(credential);
await ref.read(withContextProvider)(
(context) async {
Navigator.of(context).pop(true);
showMessage(context, l10n.s_passkey_deleted);
},
);
} on CancellationException catch (_) {
// ignored
}
},
child: Text(l10n.s_delete),
),
Expand Down
36 changes: 21 additions & 15 deletions lib/oath/views/manage_password_dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import 'package:material_symbols_icons/symbols.dart';
import '../../app/message.dart';
import '../../app/models.dart';
import '../../app/state.dart';
import '../../exception/cancellation_exception.dart';
import '../../management/models.dart';
import '../../widgets/app_input_decoration.dart';
import '../../widgets/app_text_field.dart';
Expand Down Expand Up @@ -71,23 +72,28 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
_submit() async {
_removeFocus();

final result = await ref
.read(oathStateProvider(widget.path).notifier)
.setPassword(_currentPasswordController.text, _newPassword);
if (result) {
if (mounted) {
await ref.read(withContextProvider)((context) async {
Navigator.of(context).pop();
showMessage(context, AppLocalizations.of(context)!.s_password_set);
try {
final result = await ref
.read(oathStateProvider(widget.path).notifier)
.setPassword(_currentPasswordController.text, _newPassword);
if (result) {
if (mounted) {
await ref.read(withContextProvider)((context) async {
Navigator.of(context).pop();
showMessage(context, AppLocalizations.of(context)!.s_password_set);
});
}
} else {
_currentPasswordController.selection = TextSelection(
baseOffset: 0,
extentOffset: _currentPasswordController.text.length);
_currentPasswordFocus.requestFocus();
setState(() {
_currentIsWrong = true;
});
}
} else {
_currentPasswordController.selection = TextSelection(
baseOffset: 0, extentOffset: _currentPasswordController.text.length);
_currentPasswordFocus.requestFocus();
setState(() {
_currentIsWrong = true;
});
} on CancellationException catch (_) {
// ignored
}
}

Expand Down
Loading

0 comments on commit ea0a7d2

Please sign in to comment.