Add HOTP initial count feature; try to fix HOTP panic

- HOTP now prompts for an initial count
- Found a code path where improperly formatted HOTP field can
lead to a 0-value being passed to a totp generator
- Added guardrails around the TOTP generator so it can't divide by 0:
it issues a warning instead.
This commit is contained in:
bunnie 2022-09-19 23:44:45 +08:00
parent e5f988c938
commit 22a440f75f
4 changed files with 35 additions and 8 deletions

View file

@ -161,6 +161,12 @@
"zh": "HOTP",
"en-tts": "HOTP"
},
"vault.hotp.count": {
"en": "Please enter the initial HOTP count:",
"ja": "最初の HOTP カウントを入力してください:",
"zh": "请输入初始 HOTP 计数:",
"en-tts": "Please enter the initial HOTP count:"
},
"vault.newitem.name": {
"en": "Please name the credential:",
"ja": "クレデンシャルに名前を付ける:",

View file

@ -391,6 +391,21 @@ impl ActionManager {
};
let validated_secret = base32::encode(base32::Alphabet::RFC4648 { padding: false }, &ss_vec);
let timestep = if !is_totp {
// get the initial count if it's an HOTP record
self.tt.sleep_ms(SWAP_DELAY_MS).unwrap();
match self.modals
.alert_builder(t!("vault.hotp.count", xous::LANG))
.field(Some("0".to_string()), Some(length_validator))
.build()
{
Ok(entry) => entry.content()[0].content.as_str().unwrap().parse::<u64>().unwrap(),
_ => {log::error!("Count entry failed"); self.action_active.store(false, Ordering::SeqCst); return}
}
} else {
30 // default TOTP timestep otherwise
};
// time, hash, etc. are all the "expected defaults" -- if you want to change them, edit the record after entering it.
let mut totp = storage::TotpRecord {
version: VAULT_TOTP_REC_VERSION,
@ -398,7 +413,7 @@ impl ActionManager {
secret: validated_secret,
algorithm: TotpAlgorithm::HmacSha1,
digits: 6,
timestep: if is_totp {30} else {0},
timestep,
ctime: 0,
is_hotp: !is_totp,
notes: t!("vault.notes", xous::LANG).to_string(),

View file

@ -61,7 +61,7 @@ impl Into<String> for TotpAlgorithm {
#[derive(Debug)]
pub(crate) struct TotpEntry {
pub step_seconds: u16,
pub step_seconds: u64,
pub shared_secret: Vec<u8>,
pub digit_count: u8,
pub algorithm: TotpAlgorithm,
@ -100,25 +100,31 @@ fn unpack_u64(v: u64) -> [u8; 8] {
fn generate_hmac_bytes(unix_timestamp: u64, totp_entry: &TotpEntry) -> Result<Vec<u8>, Error> {
let mut computed_hmac = Vec::new();
let checked_step = if totp_entry.step_seconds == 0 {
log::warn!("totp step_seconds was 0, this would cause a div-by-zero; forcing to 1. Check that this is not an HOTP record?");
1
} else {
totp_entry.step_seconds
};
match totp_entry.algorithm {
// The OpenTitan HMAC core does not support hmac-sha1. Fall back to
// a software implementation.
TotpAlgorithm::HmacSha1 => {
let mut mac: Hmac<Sha1> = Hmac::new_from_slice(&totp_entry.shared_secret)?;
mac.update(&unpack_u64(unix_timestamp / totp_entry.step_seconds as u64));
mac.update(&unpack_u64(unix_timestamp / checked_step));
let hash: &[u8] = &mac.finalize().into_bytes();
computed_hmac.extend_from_slice(hash);
}
// note: sha256/sha512 implementations not yet tested, as we have yet to find a site that uses this to test against.
TotpAlgorithm::HmacSha256 => {
let mut mac: Hmac<sha2::Sha256> = Hmac::new_from_slice(&totp_entry.shared_secret)?;
mac.update(&unpack_u64(unix_timestamp / totp_entry.step_seconds as u64));
mac.update(&unpack_u64(unix_timestamp / checked_step));
let hash: &[u8] = &mac.finalize().into_bytes();
computed_hmac.extend_from_slice(hash);
}
TotpAlgorithm::HmacSha512 => {
let mut mac: Hmac<sha2::Sha512> = Hmac::new_from_slice(&totp_entry.shared_secret)?;
mac.update(&unpack_u64(unix_timestamp / totp_entry.step_seconds as u64));
mac.update(&unpack_u64(unix_timestamp / checked_step));
let hash: &[u8] = &mac.finalize().into_bytes();
computed_hmac.extend_from_slice(hash);
}

View file

@ -598,7 +598,7 @@ impl VaultUx {
let algorithm = TotpAlgorithm::try_from(fields[3]).unwrap_or(TotpAlgorithm::HmacSha1);
let is_hotp = fields[4].to_uppercase() == "HOTP";
let totp = totp::TotpEntry {
step_seconds: if !is_hotp {step_seconds as u16} else {1}, // step_seconds is re-used by hotp as the code.
step_seconds: if !is_hotp {step_seconds} else {1}, // step_seconds is re-used by hotp as the code.
shared_secret,
digit_count,
algorithm
@ -771,9 +771,9 @@ impl VaultUx {
let digit_count = u8::from_str_radix(fields[1], 10).unwrap_or(6);
let step_seconds = u64::from_str_radix(fields[2], 10).unwrap_or(30);
let algorithm = TotpAlgorithm::try_from(fields[3]).unwrap_or(TotpAlgorithm::HmacSha1);
let is_hotp = fields[4] == "HOTP";
let is_hotp = fields[4].to_uppercase() == "HOTP";
let totp = totp::TotpEntry {
step_seconds: if !is_hotp {step_seconds as u16} else {1}, // step_seconds is re-used by hotp as the code.
step_seconds: if !is_hotp {step_seconds} else {1}, // step_seconds is re-used by hotp as the code.
shared_secret,
digit_count,
algorithm