Limit on Encoded Length Too Small?

[tommy-gilligan]

I tried using tokens from 2 different sites. Both sites gave me the token in encoded format with length 16. This was rejected by totp-rs with SecretTooSmall error.

I am not across the spec so I am not sure what the right length is but I think something might be up with the length check. Maybe the check needs to be applied before decoding instead of after?

Bypassing the length check I was able to successfully generate and use OTP.

[constantoine]

Hi!

The spec specifies a lower limit of 128 bits, which should come down to 16. This error should indeed not happen.

Using ‎TOTP::new will call rfc::‎assert_secret_length, which checks if the vector argument secret is of the correct size. I am currently working on and off rewriting the API to normalize naming conventions and types, but right now I don't see it. Do you have a minimum reproducible example? (with a dummy secret ofc) of 16 BASE-32 encoded characters/128 bits?

[tommy-gilligan]

Thank you for the quick response! Sorry I should have included more information in the original post.

use totp_rs::{Secret, TOTP, Algorithm};

fn main() {
    let totp = TOTP::new(
        Algorithm::SHA1,
        6,
        1,
        30,
        Secret::Encoded("YZ5CTEPRPXKMDO67".to_string()).to_bytes().unwrap(),
    ).unwrap();

    println!("code: {}", totp.generate_current().unwrap())
}

Output

thread 'main' panicked at examples/abc.rs:10:7:
called `Result::unwrap()` on an `Err` value: SecretSize(80)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[constantoine]

Alright, did some digging and I realize my error message was actually right, not my first comment. Nice.

As it's base32, each character actually encodes 5 bits. The secret generated here is 16 characters long, which gives 16 * 5 = 80. I think the services that generated those secrets may have made the same faulty assumption that 16 chars = 168 bits, when it's actually 165 bits due to base32 encoding.

The length check happens in the totp::new logic, not the Secret type, which turns out to be awkward to use as of now. Good thing it's part of the API that will be changed, but also the current state of things does leave much to be desired.

tl;dr: 16 characters of base32 means it's actually 80 bits encoded, not 128. Probably a mistake more than a decision on their part.