From cc497c7794e1ac969e4c13f6754f90bcf52d4bfd Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Tue, 25 Nov 2025 13:55:02 +0000 Subject: [PATCH 1/3] Output error on standard error with a new line On C++ use std::cerr instead of std::cout. On C fputs does not append a new line (compared to puts which does). Signed-off-by: Frediano Ziglio --- test/main.c | 10 +++++----- test/main.cpp | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/main.c b/test/main.c index db25566..0625287 100644 --- a/test/main.c +++ b/test/main.c @@ -238,7 +238,7 @@ int main(int argc, char** argv) int totp_err_1 = totp_now(tdata, tcode); if(totp_err_1 == OTP_ERROR) { - fputs("TOTP Error totp_now", stderr); + fputs("TOTP Error totp_now\n", stderr); return EXIT_FAILURE; } printf("totp_now() pass=1: `%s` `%d`\n", tcode, totp_err_1); @@ -251,7 +251,7 @@ int main(int argc, char** argv) int totp_err_2 = totp_at(tdata, 0, 0, tcode2); if(totp_err_2 == OTP_ERROR) { - fputs("TOTP Error totp_at", stderr); + fputs("TOTP Error totp_at\n", stderr); return EXIT_FAILURE; } printf("totp_at(0, 0) pass=1: `%s` `%d`\n", tcode2, totp_err_2); @@ -288,7 +288,7 @@ int main(int argc, char** argv) int totp_err_3 = totp_now(tdata_padding, tcode3); if(totp_err_3 == OTP_ERROR) { - fputs("TOTP Error totp_now (padding)", stderr); + fputs("TOTP Error totp_now (padding)\n", stderr); return EXIT_FAILURE; } printf("totp_now() (padding) pass=1: `%s` `%d`\n", tcode3, totp_err_3); @@ -301,7 +301,7 @@ int main(int argc, char** argv) int totp_err_4 = totp_at(tdata_padding, 0, 0, tcode4); if(totp_err_4 == OTP_ERROR) { - fputs("TOTP Error totp_at (padding)", stderr); + fputs("TOTP Error totp_at (padding)\n", stderr); return EXIT_FAILURE; } printf("totp_at(0, 0) (padding) pass=1: `%s` `%d`\n", tcode4, totp_err_4); @@ -337,7 +337,7 @@ int main(int argc, char** argv) int hotp_err_1 = hotp_at(hdata, 1, hcode); if(hotp_err_1 == OTP_ERROR) { - puts("HOTP Error hotp_at"); + fputs("HOTP Error hotp_at\n", stderr); return EXIT_FAILURE; } printf("hotp_at(1) pass=1: `%s` `%d`\n", hcode, hotp_err_1); diff --git a/test/main.cpp b/test/main.cpp index e683c09..7c44f3c 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -246,7 +246,7 @@ int main(int argc, char** argv) int totp_err_1 = tdata.now(tcode); if(totp_err_1 == OTP_ERROR) { - cout << "TOTP Error totp_now (padding)" << endl; + cerr << "TOTP Error totp_now (padding)" << endl; return EXIT_FAILURE; } cout << "totp_now() (padding) pass=1: `" << tcode << "` `" << totp_err_1 << "`" << endl; @@ -259,7 +259,7 @@ int main(int argc, char** argv) int totp_err_2 = tdata.at(0, 0, tcode2); if(totp_err_2 == 0) { - cout << "TOTP Error totp_at (padding)" << endl; + cerr << "TOTP Error totp_at (padding)" << endl; return EXIT_FAILURE; } cout << "totp_at(0, 0) (padding) pass=1: `" << tcode2 << "` `" << totp_err_2 << "`" << endl; @@ -296,7 +296,7 @@ int main(int argc, char** argv) int totp_err_3 = tdata_padding.now(tcode3); if(totp_err_3 == OTP_ERROR) { - cout << "TOTP Error totp_now" << endl; + cerr << "TOTP Error totp_now" << endl; return EXIT_FAILURE; } cout << "totp_now() pass=1: `" << tcode3 << "` `" << totp_err_3 << "`" << endl; @@ -309,7 +309,7 @@ int main(int argc, char** argv) int totp_err_4 = tdata_padding.at(0, 0, tcode4); if(totp_err_4 == 0) { - cout << "TOTP Error totp_at" << endl; + cerr << "TOTP Error totp_at" << endl; return EXIT_FAILURE; } cout << "totp_at(0, 0) pass=1: `" << tcode4 << "` `" << totp_err_4 << "`" << endl; @@ -344,7 +344,7 @@ int main(int argc, char** argv) int hotp_err_1 = hdata.at(1, hcode); if(hotp_err_1 == 0) { - cout << "HOTP Error hotp_at" << endl; + cerr << "HOTP Error hotp_at" << endl; return EXIT_FAILURE; } cout << "hotp_at(1) pass=1: `" << hcode << "`" << " `" << hotp_err_1 << "`" << endl; From 814ca78cb23f17d776f095514ce5468d5eeb6193 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Tue, 25 Nov 2025 13:55:59 +0000 Subject: [PATCH 2/3] Compute otp_num_to_bytestring in constant time Do not just stop if the number reach zero. Signed-off-by: Frediano Ziglio --- cotp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cotp.c b/cotp.c index 660a6ab..fcfa9b0 100644 --- a/cotp.c +++ b/cotp.c @@ -174,13 +174,13 @@ COTPRESULT otp_num_to_bytestring(uint64_t integer, char* out_str) if (out_str == NULL) return OTP_ERROR; - size_t i = 7; - while (integer != 0) + char *p = out_str + 8; + do { - out_str[i] = integer & 0xFF; - i--; + *--p = integer & 0xFF; integer >>= 8; } + while (p != out_str); return OTP_OK; } From ddd4f19747b1f9c0505bb4a554b75bfa114cf8c1 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Tue, 25 Nov 2025 13:59:16 +0000 Subject: [PATCH 3/3] Validate BASE32 secret and support large secrets All invalid characters in the string were interpreted as zero, same as the equal ('=') padding. For short strings treating padding as zero is fine ad the HMAC algorithm pad the secret with zeroes if it fit into the block. However if it does not fit the computation get wrong. If the BASE32 secret is pretty large (104 characters for SHA1), the key is replaced by an hashed version of it. So, to make an example if the secret is up to 103 "A" characters the binary key will be the same but adding any more "A" will cause the key to change (and so the tokens). This requires us to pass a key that is not multiple of 5 bytes, so the implementation of otp_byte_secret() was changed to satisfy this. While at it catch and report invalid characters. OTP_DEFAULT_BASE32_OFFSETS array was moved to C file, no reasons to expose it. Padding "=" characters are stripped at the beginning, allowing to pass them or not, as most implementations are allowing this. Signed-off-by: Frediano Ziglio --- cotp.c | 80 ++++++++++++++++++++++++++++++++++++++-------------------- cotp.h | 23 ----------------- 2 files changed, 52 insertions(+), 51 deletions(-) diff --git a/cotp.c b/cotp.c index fcfa9b0..44bbe8d 100644 --- a/cotp.c +++ b/cotp.c @@ -9,6 +9,32 @@ #include +// Not valid +enum { NV = 64 }; + +/* + Default characters used in BASE32 digests for security concious linear lookup. + For use with otp_byte_secret() +*/ +static const unsigned char OTP_DEFAULT_BASE32_OFFSETS[256] = { + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // -1-15 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 16-31 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 32-47 + NV, NV, 26, 27, 28, 29, 30, 31, NV, NV, NV, NV, NV, NV, NV, NV, // 48-63 ('2'-'7') + NV, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 64-79 ('A'-'O') + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, NV, NV, NV, NV, NV, // 80-95 ('P'-'Z') + NV, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 96-111 ('a'-'o') + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, NV, NV, NV, NV, NV, // 112-127 ('p'-'z') + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 128-143 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 144-159 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 160-175 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 176-191 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 192-207 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 208-223 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, // 224-239 + NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV, NV // 240-255 +}; + /* Converts an OTPType enum to string. @@ -115,6 +141,14 @@ void otp_free(OTPData* data) free(data); } +static size_t base32_len(const char *s) +{ + size_t len = s ? strlen(s) : 0; + while (len > 0 && s[len-1] == '=') + --len; + return len; +} + /* Un-base32's a base32 string stored inside an OTPData. @@ -125,39 +159,29 @@ void otp_free(OTPData* data) error, 0 */ COTPRESULT otp_byte_secret(OTPData* data, char* out_str) { - if (out_str == NULL || strlen(data->base32_secret) % 8 != 0) { + const size_t base32_length = base32_len(data->base32_secret); + + if (base32_length == 0) return OTP_ERROR; - } - - size_t base32_length = strlen(data->base32_secret); - size_t num_blocks = base32_length / 8; - size_t output_length = num_blocks * 5; - - if (output_length == 0) { - return OTP_OK; - } - int valid = 1; + unsigned char invalid = 0; + unsigned bits = 0, block_value = 0; - for (size_t i = 0; i < num_blocks; i++) { - uint64_t block_value = 0; - - for (int j = 0; j < 8; j++) { - block_value <<= 5; - char c = data->base32_secret[i * 8 + j]; - unsigned int value = (unsigned char) c < 256 ? OTP_DEFAULT_BASE32_OFFSETS[(unsigned char) c] : -1; - block_value |= value & 31; - valid &= (value >= 0); + for (size_t i = 0; i < base32_length ; i++) { + block_value <<= 5; + bits += 5; + unsigned char c = (unsigned char) data->base32_secret[i]; + unsigned char value = c < 256 ? OTP_DEFAULT_BASE32_OFFSETS[c] : NV; + block_value |= value & 31; + invalid |= value; + + if (bits >= 8) { + bits -= 8; + *out_str++ = (block_value >> bits) & 255; } - - out_str[i * 5 + 0] = block_value >> 32; - out_str[i * 5 + 1] = block_value >> 24; - out_str[i * 5 + 2] = block_value >> 16; - out_str[i * 5 + 3] = block_value >> 8; - out_str[i * 5 + 4] = block_value >> 0; } - return valid ? OTP_OK : OTP_ERROR; + return invalid < NV ? OTP_OK : OTP_ERROR; } /* @@ -448,7 +472,7 @@ COTPRESULT otp_generate(OTPData* data, uint64_t input, char* out_str) char byte_string[8+1]; memset(byte_string, 0, 8+1); - size_t bs_len = (strlen(data->base32_secret)/8)*5; + size_t bs_len = (base32_len(data->base32_secret)*5)/8; char byte_secret[bs_len + 1]; memset(byte_secret, 0, bs_len + 1); diff --git a/cotp.h b/cotp.h index 962159e..6513a26 100644 --- a/cotp.h +++ b/cotp.h @@ -14,29 +14,6 @@ typedef int COTPRESULT; #define OTP_ERROR ((COTPRESULT) 0) -/* - Default characters used in BASE32 digests for security concious linear lookup. - For use with otp_byte_secret() -*/ -static const int OTP_DEFAULT_BASE32_OFFSETS[256] = { - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0-15 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16-31 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 32-47 - 0, 0, 26, 27, 28, 29, 30, 31, 0, 0, 0, 0, 0, 0, 0, 0, // 48-63 ('2'-'7') - 0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 64-79 ('A'-'O') - 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 0, 0, 0, 0, 0, // 80-95 ('P'-'Z') - 0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 96-111 ('a'-'o') - 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 0, 0, 0, 0, 0, // 112-127 ('p'-'z') - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 128-143 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 144-159 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 160-175 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 176-191 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 192-207 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 208-223 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 224-239 - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // 240-255 -}; - /* For use with otp_random_base32()