From 23702b63218bcbce80ad0322472296d73a9de09e Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 19 Aug 2026 16:43:43 +0200 Subject: [PATCH 1/2] fix(totp): accept secrets shorter than 128 bits RFC 4226 asks for at least 128 bits of secret material and totp-rs enforces that, so a shorter secret - GitHub hands those out - was rejected outright. Build through TOTP::new_unchecked instead and run every rule TOTP::new would have applied, minus that floor, then report the weakness through the new SecretStrength on TotpInfo. Scanning an unparseable code from the view screen silently did nothing. It now shows the same parse error dialog the create screen already had, which moved to :feature:totp so both screens can reach it. Closes #68 Co-Authored-By: Claude Opus 5 --- .../CreateNewOrUpdateLoginUseCaseTest.kt | 3 + .../create/presentation/login/LoginContent.kt | 2 +- .../create/src/main/res/values/strings.xml | 4 - .../item/view/login/ViewLoginContent.kt | 8 + .../item/view/login/ViewLoginViewModel.kt | 22 ++- .../item/view/login/model/ViewLoginState.kt | 1 + .../item/view/login/model/ViewLoginUiEvent.kt | 1 + .../component/TotpParseErrorDialog.kt | 4 +- feature/totp/src/main/res/values/strings.xml | 4 + rust/rust-code/bindings/src/totp.rs | 21 ++- rust/rust-code/lib/src/totp.rs | 159 ++++++++++++++++-- 11 files changed, 203 insertions(+), 26 deletions(-) rename feature/{item/create/src/main/kotlin/de/davis/keygo/feature/item/create => totp/src/main/kotlin/de/davis/keygo/feature/totp}/presentation/component/TotpParseErrorDialog.kt (88%) diff --git a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt b/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt index 08838a2e8..a29a4dad7 100644 --- a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt +++ b/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt @@ -33,6 +33,7 @@ import de.davis.keygo.feature.item.core.domain.model.set import de.davis.keygo.rust.FakeTotpService import de.davis.keygo.rust.totp.TotpService import de.davisalessandro.keygo.rust.Algorithm +import de.davisalessandro.keygo.rust.SecretStrength import de.davisalessandro.keygo.rust.TotpInfo import kotlin.test.BeforeTest import kotlin.test.Test @@ -773,6 +774,7 @@ class CreateNewOrUpdateLoginUseCaseTest { algorithm = Algorithm.SHA256, digits = 8, period = 60, + strength = SecretStrength.TRUSTWORTHY, ) val localUseCase = makeUseCase( totpService = FakeTotpService().apply { infoFromUriResult = parsed }, @@ -879,6 +881,7 @@ class CreateNewOrUpdateLoginUseCaseTest { algorithm = Algorithm.SHA512, digits = 8, period = 60, + strength = SecretStrength.TRUSTWORTHY, ) val result = makeUseCase( totpService = FakeTotpService().apply { infoFromUriResult = parsed }, diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt index 9af37217b..699ed810f 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt @@ -61,7 +61,6 @@ import de.davis.keygo.feature.item.create.presentation.component.KeyGoItemForm import de.davis.keygo.feature.item.create.presentation.component.OverrideTotpDialog import de.davis.keygo.feature.item.create.presentation.component.SelectItemForTotpModificationDialog import de.davis.keygo.feature.item.create.presentation.component.TAG_DELIMITERS -import de.davis.keygo.feature.item.create.presentation.component.TotpParseErrorDialog import de.davis.keygo.feature.item.create.presentation.login.model.DialogState import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState import de.davis.keygo.feature.item.create.presentation.login.model.LoginPasskeyInfo @@ -73,6 +72,7 @@ import de.davis.keygo.feature.item.create.presentation.model.SharedItemState import de.davis.keygo.feature.item.create.presentation.model.VaultsState import de.davis.keygo.feature.item.create.presentation.password.GeneratePasswordModalBottomSheet import de.davis.keygo.feature.totp.presentation.component.QRScanner +import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog import de.davis.keygo.core.item.R as CoreItemR import de.davis.keygo.feature.item.core.R as ItemCoreR diff --git a/feature/item/create/src/main/res/values/strings.xml b/feature/item/create/src/main/res/values/strings.xml index c8294dacb..819cf1e82 100644 --- a/feature/item/create/src/main/res/values/strings.xml +++ b/feature/item/create/src/main/res/values/strings.xml @@ -11,9 +11,6 @@ Override TOTP fields? The current item has the following TOTP fields that differ from the fields specified in the totp information. Select fields you want to override. - TOTP Parse Error - The TOTP code could not be parsed. - TOTP Secret Override @@ -29,7 +26,6 @@ One or more entries match the issuer or account name from the totp code. Would you like to update an existing entry or create a new one? Warning - OK Cancel Delete Passkey diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt index 89ae7a34d..a13bc2ed6 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt @@ -92,6 +92,7 @@ import de.davis.keygo.feature.item.view.login.model.ViewLoginUiEvent import de.davis.keygo.feature.item.view.onHold import de.davis.keygo.feature.totp.domain.model.TotpValue import de.davis.keygo.feature.totp.presentation.component.QRScanner +import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog import de.davis.keygo.core.item.R as CoreItemR import de.davis.keygo.core.ui.R as CoreUiR import de.davis.keygo.feature.item.core.R as ItemCoreR @@ -460,6 +461,13 @@ fun ViewLoginContent(state: ViewLoginState, onEvent: (ViewLoginUiEvent) -> Unit) } } + if (state.totpParseError) { + TotpParseErrorDialog( + onDismiss = { onEvent(ViewLoginUiEvent.OnTotpParseErrorDismiss) }, + modifier = Modifier.fillMaxWidth(), + ) + } + if (state.scanning) { QRScanner( onClose = { onEvent(ViewLoginUiEvent.OnBackClick) }, diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt index 06a557900..b080275af 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt @@ -16,6 +16,7 @@ import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver import de.davis.keygo.core.util.domain.usecase.SortUseCase import de.davis.keygo.core.util.fold import de.davis.keygo.core.util.getOrNull +import de.davis.keygo.core.util.isSuccess import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.onSuccess import de.davis.keygo.feature.item.core.domain.model.ItemUpsertError @@ -76,6 +77,7 @@ internal class ViewLoginViewModel( private val _modificationDialogState = MutableStateFlow(null) private val _scanning = MutableStateFlow(false) + private val _totpParseError = MutableStateFlow(false) private val _itemId = MutableStateFlow(null) @OptIn(ExperimentalCoroutinesApi::class) @@ -135,10 +137,12 @@ internal class ViewLoginViewModel( _stateWithoutModification, _modificationDialogState, _scanning, - ) { state, modificationDialog, scanning -> + _totpParseError, + ) { state, modificationDialog, scanning, totpParseError -> state.copy( modificationDialog = modificationDialog, scanning = scanning, + totpParseError = totpParseError, ) }.stateIn( scope = viewModelScope, @@ -230,10 +234,14 @@ internal class ViewLoginViewModel( is ViewLoginUiEvent.OnCodesScanned -> { _scanning.update { false } - val uriOrSecret = event.codes.firstNotNullOfOrNull { qrCode -> - // This just validates whether `qrCode` is a valid totp uri - we return qrCode - totpService.getInfoFromUriWithResult(qrCode).getOrNull()?.let { qrCode } - } ?: return + val uriOrSecret = event.codes.firstOrNull { + totpService.getInfoFromUriWithResult(it).isSuccess() + } + + if (uriOrSecret == null) { + _totpParseError.update { true } + return + } _itemId.value?.let { id -> viewModelScope.launch { @@ -247,6 +255,10 @@ internal class ViewLoginViewModel( } } + ViewLoginUiEvent.OnTotpParseErrorDismiss -> { + _totpParseError.update { false } + } + is ViewLoginUiEvent.OnSubmitModification -> { val dialog = _modificationDialogState.value ?: return val newText = fieldUpdate(event.input) diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginState.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginState.kt index e768dc83e..a51e8a1f2 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginState.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginState.kt @@ -21,5 +21,6 @@ data class ViewLoginState( val note: String = "", val modificationDialog: ModificationDialog? = null, val scanning: Boolean = false, + val totpParseError: Boolean = false, val pinned: Boolean = false, ) diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginUiEvent.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginUiEvent.kt index 5581b7735..c6b14a913 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginUiEvent.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginUiEvent.kt @@ -16,4 +16,5 @@ sealed interface ViewLoginUiEvent { data object OnScanCodeRequest : ViewLoginUiEvent data class OnCodesScanned(val codes: List) : ViewLoginUiEvent + data object OnTotpParseErrorDismiss : ViewLoginUiEvent } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/TotpParseErrorDialog.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt similarity index 88% rename from feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/TotpParseErrorDialog.kt rename to feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt index a3de56a9e..5a6d7b104 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/TotpParseErrorDialog.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.create.presentation.component +package de.davis.keygo.feature.totp.presentation.component import androidx.compose.material3.AlertDialog import androidx.compose.material3.Text @@ -6,7 +6,7 @@ import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource -import de.davis.keygo.feature.item.create.R +import de.davis.keygo.feature.totp.R @Composable fun TotpParseErrorDialog(onDismiss: () -> Unit, modifier: Modifier = Modifier) { diff --git a/feature/totp/src/main/res/values/strings.xml b/feature/totp/src/main/res/values/strings.xml index b0a768f6a..d8e13a17d 100644 --- a/feature/totp/src/main/res/values/strings.xml +++ b/feature/totp/src/main/res/values/strings.xml @@ -9,4 +9,8 @@ Camera permission Camera permission was denied. To scan QR codes, please enable it in your app settings. Open Settings + OK + + TOTP Parse Error + The TOTP code could not be parsed. \ No newline at end of file diff --git a/rust/rust-code/bindings/src/totp.rs b/rust/rust-code/bindings/src/totp.rs index c9f676e4b..553390267 100644 --- a/rust/rust-code/bindings/src/totp.rs +++ b/rust/rust-code/bindings/src/totp.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use lib::totp::{ - TotpInfo as CoreTotpInfo, get_totp as core_get_totp, + SecretStrength as CoreSecretStrength, TotpInfo as CoreTotpInfo, get_totp as core_get_totp, get_totp_info_from_uri as core_get_totp_info_from_uri, get_totp_url as core_get_totp_url, }; use thiserror::Error; @@ -33,6 +33,23 @@ impl From for Algorithm { } } +/// Whether a TOTP secret reaches the 128 bits RFC 4226 asks for. Shorter +/// secrets are accepted anyway, because services like GitHub issue them. +#[derive(Debug, Clone, Copy, uniffi::Enum)] +pub enum SecretStrength { + Trustworthy, + Untrusted, +} + +impl From for SecretStrength { + fn from(value: CoreSecretStrength) -> Self { + match value { + CoreSecretStrength::Trustworthy => Self::Trustworthy, + CoreSecretStrength::Untrusted => Self::Untrusted, + } + } +} + #[derive(Debug, uniffi::Record)] pub struct TotpInfo { pub secret: String, @@ -41,6 +58,7 @@ pub struct TotpInfo { pub algorithm: Algorithm, pub digits: i32, pub period: i32, + pub strength: SecretStrength, } impl From for TotpInfo { @@ -52,6 +70,7 @@ impl From for TotpInfo { algorithm: value.algorithm.into(), digits: value.digits as i32, period: value.period as i32, + strength: value.strength.into(), } } } diff --git a/rust/rust-code/lib/src/totp.rs b/rust/rust-code/lib/src/totp.rs index 1d63351bc..1b9aa4228 100644 --- a/rust/rust-code/lib/src/totp.rs +++ b/rust/rust-code/lib/src/totp.rs @@ -1,8 +1,15 @@ +use std::ops::RangeInclusive; use std::time::SystemTimeError; use thiserror::Error; pub use totp_rs::Algorithm; use totp_rs::{Secret, SecretParseError, TOTP, TotpUrlError}; +/// Secret size RFC 4226 asks for, in bytes (128 bits). +const MIN_SECRET_BYTES: usize = 16; + +/// Digit counts RFC 6238 permits. +const ALLOWED_DIGITS: RangeInclusive = 6..=8; + #[derive(Debug, Error)] pub enum TotpError { #[error("url error: {0}")] @@ -13,17 +20,72 @@ pub enum TotpError { Secret(SecretParseError), } -pub fn get_totp( +/// Whether a secret reaches [`MIN_SECRET_BYTES`]. Shorter ones stay usable, +/// because services like GitHub issue them, so callers can warn instead. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SecretStrength { + Trustworthy, + Untrusted, +} + +impl SecretStrength { + fn of(secret: &[u8]) -> Self { + if secret.len() >= MIN_SECRET_BYTES { + Self::Trustworthy + } else { + Self::Untrusted + } + } +} + +/// The checks `TOTP::new` runs, minus its 128 bit secret floor. The unchecked +/// constructors skip all of them, so every caller runs this instead. +fn validate(totp: &TOTP) -> Result<(), TotpError> { + if totp.secret.is_empty() { + return Err(TotpError::Url(TotpUrlError::SecretSize(0))); + } + + if !ALLOWED_DIGITS.contains(&totp.digits) { + return Err(TotpError::Url(TotpUrlError::DigitsNumber(totp.digits))); + } + + if let Some(issuer) = totp.issuer.as_deref().filter(|it| it.contains(':')) { + return Err(TotpError::Url(TotpUrlError::Issuer(issuer.to_string()))); + } + + if totp.account_name.contains(':') { + return Err(TotpError::Url(TotpUrlError::AccountName( + totp.account_name.clone(), + ))); + } + + Ok(()) +} + +fn build_totp( algorithm: Algorithm, digits: usize, step: u64, secret: String, -) -> Result { + issuer: Option, + account_name: String, +) -> Result { let secret = Secret::Encoded(secret) .to_bytes() .map_err(TotpError::Secret)?; - let totp = TOTP::new(algorithm, digits, 1, step, secret, None, "".to_string()) - .map_err(TotpError::Url)?; + let totp = TOTP::new_unchecked(algorithm, digits, 1, step, secret, issuer, account_name); + validate(&totp)?; + + Ok(totp) +} + +pub fn get_totp( + algorithm: Algorithm, + digits: usize, + step: u64, + secret: String, +) -> Result { + let totp = build_totp(algorithm, digits, step, secret, None, "".to_string())?; totp.generate_current().map_err(TotpError::Time) } @@ -34,18 +96,21 @@ pub struct TotpInfo { pub algorithm: Algorithm, pub digits: usize, pub period: u64, + pub strength: SecretStrength, } pub fn get_totp_info_from_uri(uri: String) -> Result { - let totp = TOTP::from_url(uri).map_err(TotpError::Url)?; - let secret = Secret::Raw(totp.secret.clone()).to_encoded().to_string(); + let totp = TOTP::from_url_unchecked(uri).map_err(TotpError::Url)?; + validate(&totp)?; + Ok(TotpInfo { - secret, + secret: Secret::Raw(totp.secret.clone()).to_encoded().to_string(), issuer: totp.issuer.clone(), account_name: totp.account_name.clone(), algorithm: totp.algorithm, digits: totp.digits, period: totp.step, + strength: SecretStrength::of(&totp.secret), }) } @@ -57,15 +122,83 @@ pub fn get_totp_url( issuer: Option, account_name: String, ) -> Result { - let secret = Secret::Encoded(secret) - .to_bytes() - .map_err(TotpError::Secret)?; - - let totp = TOTP::new(algorithm, digits, 1, step, secret, issuer, account_name) - .map_err(TotpError::Url)?; + let totp = build_totp(algorithm, digits, step, secret, issuer, account_name)?; Ok(totp.get_url()) } pub(crate) fn is_valid_totp_secret(s: &str) -> bool { base32::decode(base32::Alphabet::Rfc4648 { padding: false }, s).is_some() } + +#[cfg(test)] +mod tests { + use super::*; + + /// 80 bits, the kind of secret services like GitHub hand out. + const SHORT_SECRET: &str = "JBSWY3DPEHPK3PXP"; + /// 128 bits, the minimum RFC 4226 asks for. + const LONG_SECRET: &str = "JBSWY3DPEHPK3PXPJBSWY3DPEH"; + + fn uri(secret: &str) -> String { + format!("otpauth://totp/GitHub:alice?secret={secret}&issuer=GitHub") + } + + fn url_with_issuer(issuer: &str) -> Result { + get_totp_url( + Algorithm::SHA1, + 6, + 30, + SHORT_SECRET.to_string(), + Some(issuer.to_string()), + "alice".to_string(), + ) + } + + #[test] + fn generates_code_for_secret_below_128_bits() { + let code = get_totp(Algorithm::SHA1, 6, 30, SHORT_SECRET.to_string()).unwrap(); + assert_eq!(6, code.len()); + } + + #[test] + fn reports_strength_of_parsed_secret() { + let short = get_totp_info_from_uri(uri(SHORT_SECRET)).unwrap(); + let long = get_totp_info_from_uri(uri(LONG_SECRET)).unwrap(); + + assert_eq!(SecretStrength::Untrusted, short.strength); + assert_eq!(SecretStrength::Trustworthy, long.strength); + assert_eq!(SHORT_SECRET, short.secret); + } + + #[test] + fn builds_url_for_secret_below_128_bits() { + let url = url_with_issuer("GitHub").unwrap(); + + assert!(url.contains(SHORT_SECRET), "unexpected url: {url}"); + } + + #[test] + fn rejects_empty_secret() { + assert!(get_totp(Algorithm::SHA1, 6, 30, "".to_string()).is_err()); + } + + #[test] + fn rejects_digits_outside_rfc_range() { + assert!(get_totp(Algorithm::SHA1, 9, 30, SHORT_SECRET.to_string()).is_err()); + } + + #[test] + fn rejects_colon_in_issuer() { + assert!(url_with_issuer("Git:Hub").is_err()); + } + + #[test] + fn rejects_uri_without_secret() { + assert!(get_totp_info_from_uri(uri("")).is_err()); + } + + #[test] + fn rejects_secret_that_is_not_base32() { + assert!(get_totp(Algorithm::SHA1, 6, 30, "not base32!".to_string()).is_err()); + } +} From a51cba9a409eb05d0a1ba5442a13272cf5110e22 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 20 Aug 2026 01:44:18 +0200 Subject: [PATCH 2/2] refactor(totp): drop secret strength --- .../CreateNewOrUpdateLoginUseCaseTest.kt | 3 -- rust/rust-code/bindings/src/totp.rs | 21 +----------- rust/rust-code/lib/src/totp.rs | 32 +++---------------- 3 files changed, 5 insertions(+), 51 deletions(-) diff --git a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt b/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt index a29a4dad7..08838a2e8 100644 --- a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt +++ b/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/CreateNewOrUpdateLoginUseCaseTest.kt @@ -33,7 +33,6 @@ import de.davis.keygo.feature.item.core.domain.model.set import de.davis.keygo.rust.FakeTotpService import de.davis.keygo.rust.totp.TotpService import de.davisalessandro.keygo.rust.Algorithm -import de.davisalessandro.keygo.rust.SecretStrength import de.davisalessandro.keygo.rust.TotpInfo import kotlin.test.BeforeTest import kotlin.test.Test @@ -774,7 +773,6 @@ class CreateNewOrUpdateLoginUseCaseTest { algorithm = Algorithm.SHA256, digits = 8, period = 60, - strength = SecretStrength.TRUSTWORTHY, ) val localUseCase = makeUseCase( totpService = FakeTotpService().apply { infoFromUriResult = parsed }, @@ -881,7 +879,6 @@ class CreateNewOrUpdateLoginUseCaseTest { algorithm = Algorithm.SHA512, digits = 8, period = 60, - strength = SecretStrength.TRUSTWORTHY, ) val result = makeUseCase( totpService = FakeTotpService().apply { infoFromUriResult = parsed }, diff --git a/rust/rust-code/bindings/src/totp.rs b/rust/rust-code/bindings/src/totp.rs index 553390267..c9f676e4b 100644 --- a/rust/rust-code/bindings/src/totp.rs +++ b/rust/rust-code/bindings/src/totp.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use lib::totp::{ - SecretStrength as CoreSecretStrength, TotpInfo as CoreTotpInfo, get_totp as core_get_totp, + TotpInfo as CoreTotpInfo, get_totp as core_get_totp, get_totp_info_from_uri as core_get_totp_info_from_uri, get_totp_url as core_get_totp_url, }; use thiserror::Error; @@ -33,23 +33,6 @@ impl From for Algorithm { } } -/// Whether a TOTP secret reaches the 128 bits RFC 4226 asks for. Shorter -/// secrets are accepted anyway, because services like GitHub issue them. -#[derive(Debug, Clone, Copy, uniffi::Enum)] -pub enum SecretStrength { - Trustworthy, - Untrusted, -} - -impl From for SecretStrength { - fn from(value: CoreSecretStrength) -> Self { - match value { - CoreSecretStrength::Trustworthy => Self::Trustworthy, - CoreSecretStrength::Untrusted => Self::Untrusted, - } - } -} - #[derive(Debug, uniffi::Record)] pub struct TotpInfo { pub secret: String, @@ -58,7 +41,6 @@ pub struct TotpInfo { pub algorithm: Algorithm, pub digits: i32, pub period: i32, - pub strength: SecretStrength, } impl From for TotpInfo { @@ -70,7 +52,6 @@ impl From for TotpInfo { algorithm: value.algorithm.into(), digits: value.digits as i32, period: value.period as i32, - strength: value.strength.into(), } } } diff --git a/rust/rust-code/lib/src/totp.rs b/rust/rust-code/lib/src/totp.rs index 1b9aa4228..b63fb4ad6 100644 --- a/rust/rust-code/lib/src/totp.rs +++ b/rust/rust-code/lib/src/totp.rs @@ -4,9 +4,6 @@ use thiserror::Error; pub use totp_rs::Algorithm; use totp_rs::{Secret, SecretParseError, TOTP, TotpUrlError}; -/// Secret size RFC 4226 asks for, in bytes (128 bits). -const MIN_SECRET_BYTES: usize = 16; - /// Digit counts RFC 6238 permits. const ALLOWED_DIGITS: RangeInclusive = 6..=8; @@ -20,24 +17,6 @@ pub enum TotpError { Secret(SecretParseError), } -/// Whether a secret reaches [`MIN_SECRET_BYTES`]. Shorter ones stay usable, -/// because services like GitHub issue them, so callers can warn instead. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SecretStrength { - Trustworthy, - Untrusted, -} - -impl SecretStrength { - fn of(secret: &[u8]) -> Self { - if secret.len() >= MIN_SECRET_BYTES { - Self::Trustworthy - } else { - Self::Untrusted - } - } -} - /// The checks `TOTP::new` runs, minus its 128 bit secret floor. The unchecked /// constructors skip all of them, so every caller runs this instead. fn validate(totp: &TOTP) -> Result<(), TotpError> { @@ -96,7 +75,6 @@ pub struct TotpInfo { pub algorithm: Algorithm, pub digits: usize, pub period: u64, - pub strength: SecretStrength, } pub fn get_totp_info_from_uri(uri: String) -> Result { @@ -110,7 +88,6 @@ pub fn get_totp_info_from_uri(uri: String) -> Result { algorithm: totp.algorithm, digits: totp.digits, period: totp.step, - strength: SecretStrength::of(&totp.secret), }) } @@ -136,8 +113,8 @@ mod tests { /// 80 bits, the kind of secret services like GitHub hand out. const SHORT_SECRET: &str = "JBSWY3DPEHPK3PXP"; - /// 128 bits, the minimum RFC 4226 asks for. - const LONG_SECRET: &str = "JBSWY3DPEHPK3PXPJBSWY3DPEH"; + /// 160 bits, the length RFC 4226 recommends. + const LONG_SECRET: &str = "JBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP"; fn uri(secret: &str) -> String { format!("otpauth://totp/GitHub:alice?secret={secret}&issuer=GitHub") @@ -161,13 +138,12 @@ mod tests { } #[test] - fn reports_strength_of_parsed_secret() { + fn parses_uri_regardless_of_secret_length() { let short = get_totp_info_from_uri(uri(SHORT_SECRET)).unwrap(); let long = get_totp_info_from_uri(uri(LONG_SECRET)).unwrap(); - assert_eq!(SecretStrength::Untrusted, short.strength); - assert_eq!(SecretStrength::Trustworthy, long.strength); assert_eq!(SHORT_SECRET, short.secret); + assert_eq!(LONG_SECRET, long.secret); } #[test]