From 6dfc1a2929313e24ea03e78b486b72f7b1c1e5ec Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Fri, 25 Jan 2019 19:19:36 +0000 Subject: Add tolerance for timing issues to the TOTP tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TOTP test with the timestamp 59 often fails as the Nitrokey’s clock ticks between setting the time and generating the TOTP code. This patch also allows the TOTP code for timestamp 60 for this test case. --- CHANGELOG.md | 1 + README.md | 2 -- TODO.md | 1 - tests/otp.rs | 25 ++++++++++++++++--------- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49ff8fc..b9983ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ SPDX-License-Identifier: MIT - Return `Result` instead of `Version` from `get_library_version`. - Return `Error::Utf8Error` if libnitrokey returns an invalid UTF-8 string. - Implement `From<(T: Device, Error)>` for `Error`. +- Fix timing issues with the `totp_no_pin` and `totp_pin` test cases. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/README.md b/README.md index 8e1d98e..069fed1 100644 --- a/README.md +++ b/README.md @@ -60,8 +60,6 @@ an AES key has been built. Some tests will overwrite the data stored on the Nitrokey device or perform a factory reset. Never execute the tests if you unless yout want to destroy all data on all connected Nitrokey devices! -The `totp_no_pin` and `totp_pin` tests can occasionally fail due to bad timing. - ## Acknowledgments Thanks to Nitrokey UG for providing a Nitrokey Storage to support the diff --git a/TODO.md b/TODO.md index db45bb5..1ff723d 100644 --- a/TODO.md +++ b/TODO.md @@ -10,7 +10,6 @@ SPDX-License-Identifier: MIT - `NK_get_progress_bar_value` - `NK_list_devices_by_cpuID` - `NK_connect_with_ID` -- Fix timing issues with the `totp_no_pin` and `totp_pin` test cases. - Clear passwords from memory. - Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware issue 65][]). diff --git a/tests/otp.rs b/tests/otp.rs index fb20768..e424673 100644 --- a/tests/otp.rs +++ b/tests/otp.rs @@ -23,13 +23,13 @@ static HOTP_CODES: &[&str] = &[ // test suite according to RFC 6238, Appendix B static TOTP_SECRET: &str = "3132333435363738393031323334353637383930"; -static TOTP_CODES: &[(u64, &str)] = &[ - (59, "94287082"), - (1111111109, "07081804"), - (1111111111, "14050471"), - (1234567890, "89005924"), - (2000000000, "69279037"), - (20000000000, "65353130"), +static TOTP_CODES: &[(u64, &[&str])] = &[ + (59, &["94287082", "37359152"]), + (1111111109, &["07081804"]), + (1111111111, &["14050471"]), + (1234567890, &["89005924"]), + (2000000000, &["69279037"]), + (20000000000, &["65353130"]), ]; #[derive(PartialEq)] @@ -159,7 +159,7 @@ fn configure_totp(admin: &ConfigureOtp, factor: u64) { } fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimestampSize) { - for (base_time, code) in TOTP_CODES { + for (base_time, codes) in TOTP_CODES { let time = base_time.checked_mul(factor).unwrap(); let is_u64 = time > u32::max_value() as u64; if is_u64 != (timestamp_size == TotpTimestampSize::U64) { @@ -167,7 +167,14 @@ fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimes } assert_ok!((), device.set_time(time, true)); - assert_ok!(code.to_string(), device.get_totp_code(1)); + let code = device.get_totp_code(1).unwrap(); + assert!( + code.contains(&code), + "Generated TOTP code {} for {}, but expected one of {}", + code, + base_time, + codes.join(", ") + ); } } -- cgit v1.2.1