From e92a7164e3547a6324e161e4704fabeb6d8c5e44 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 18 Nov 2022 13:51:32 +0100 Subject: [PATCH 1/2] sys/hash/pbkdf2: Accept passwd as `void *` instead of `uint8_t *` Having to cast a password provided as `const char *` to `const uint8_t *` is a needless pain in the ass when using the API. Hence, fix it by accepting passwords and salts as `const void *` instead. --- sys/hashes/pbkdf2.c | 4 ++-- sys/include/hashes/pbkdf2.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sys/hashes/pbkdf2.c b/sys/hashes/pbkdf2.c index 4e89f391be..ab580a0005 100644 --- a/sys/hashes/pbkdf2.c +++ b/sys/hashes/pbkdf2.c @@ -43,8 +43,8 @@ static void inplace_xor_digests(uint8_t *d1, const uint8_t *d2) } } -void pbkdf2_sha256(const uint8_t *password, size_t password_len, - const uint8_t *salt, size_t salt_len, +void pbkdf2_sha256(const void *password, size_t password_len, + const void *salt, size_t salt_len, int iterations, uint8_t *output) { diff --git a/sys/include/hashes/pbkdf2.h b/sys/include/hashes/pbkdf2.h index 11483ff2ab..8e8541c72c 100644 --- a/sys/include/hashes/pbkdf2.h +++ b/sys/include/hashes/pbkdf2.h @@ -47,8 +47,8 @@ extern "C" { * recommended 10000 * @param[out] output array of size PBKDF2_KEY_SIZE */ -void pbkdf2_sha256(const uint8_t *password, size_t password_len, - const uint8_t *salt, size_t salt_len, +void pbkdf2_sha256(const void *password, size_t password_len, + const void *salt, size_t salt_len, int iterations, uint8_t *output); From 176cb9a7b0849bb5e4412e141ab68e769ffae201 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 18 Nov 2022 13:55:26 +0100 Subject: [PATCH 2/2] tests/pbkdf2: de-flanky-fy test Previously, the test vectors were encoded into the python test scripts, converted to base64, and send over to the device under test via stdio. The application sent back the output after converting it to base64 first, which was read back in by the test script and decoded. Finally, the test script compared the result with the expected result. This made the test complex, slow and, flanky, as stdio on interfaces such as UART has a high bit error rate and some quirks (e.g. the EDBG UART bridge e.g. in the samr21-xpro dropping bytes when bursts of more than 64 bytes at a time are send). This basically rewrites the test to embed the test vectors in the firmware and do the comparison on the devices. This fixes test failures on the samr21-xpro, the nRF52840-DK and likely many others. Also, it is now fast. --- tests/pbkdf2/Makefile | 15 +-- tests/pbkdf2/main.c | 185 ++++++++++++++------------------ tests/pbkdf2/tests/01-rfc.py | 42 -------- tests/pbkdf2/tests/01-run.py | 20 ++++ tests/pbkdf2/tests/02-random.py | 80 -------------- tests/pbkdf2/tests/test_base.py | 44 -------- 6 files changed, 100 insertions(+), 286 deletions(-) delete mode 100755 tests/pbkdf2/tests/01-rfc.py create mode 100755 tests/pbkdf2/tests/01-run.py delete mode 100755 tests/pbkdf2/tests/02-random.py delete mode 100644 tests/pbkdf2/tests/test_base.py diff --git a/tests/pbkdf2/Makefile b/tests/pbkdf2/Makefile index 7d0028ed49..f8d8f05e8c 100644 --- a/tests/pbkdf2/Makefile +++ b/tests/pbkdf2/Makefile @@ -1,19 +1,6 @@ include ../Makefile.tests_common -# This application uses getchar and thus expects input from stdio -USEMODULE += stdin USEMODULE += hashes -USEMODULE += base64 - -# Use a terminal that does not introduce extra characters into the stream. -RIOT_TERMINAL ?= socat - -#ensure the rx buffer has some room even with large test patterns -CFLAGS += -DSTDIO_UART_RX_BUFSIZE=128 +USEMODULE += fmt include $(RIOTBASE)/Makefile.include - -# Increase Stack size for AVR -ifneq (,$(filter avr8_common,$(USEMODULE))) - CFLAGS += -DTHREAD_STACKSIZE_MAIN=THREAD_STACKSIZE_LARGE -endif diff --git a/tests/pbkdf2/main.c b/tests/pbkdf2/main.c index b5c219939e..ecb4e789b0 100644 --- a/tests/pbkdf2/main.c +++ b/tests/pbkdf2/main.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2019 Freie Universität Berlin. + * 2022 Otto-von-Guericke-Universität Magdeburg * * This file is subject to the terms and conditions of the GNU Lesser * General Public License v2.1. See the file LICENSE in the top level @@ -13,132 +14,104 @@ * @brief Test PBKDF2-sha256 implementation. * * @author Juan Carrano + * @author Marian Buschsieweke * - * This application reads (password, salt, iterations) tuples from the - * standard input and outputs the derived key. - * - * The salt must be base64 encoded. The key is printed as base64. * @} */ +#include +#include #include #include #include -#include -#include "base64.h" +#include "container.h" +#include "fmt.h" #include "hashes/pbkdf2.h" +#include "hashes/sha256.h" +#include "kernel_defines.h" -const char error_message[] = "{error}"; -const char input_message[] = "{ready}"; +static uint8_t key[SHA256_DIGEST_LENGTH]; -#define LINEBUF_SZ (128) - -enum TEST_STATE { - TEST_READ_PASS, - TEST_READ_SALT, - TEST_READ_ITERS, - TEST_COMPUTE, - TEST_ERROR +struct testcase { + const char *password; + const char *salt; + uint16_t iterations; + const uint8_t digest[sizeof(key)]; }; -static void _clear_input(void) -{ - /* clear input buffer */ - int c; - while ( (c = getchar()) != '\n' && c != EOF ) { } -} +struct testcase testcases[] = { + { + .password = "passwd", + .salt = "salt", + .iterations = 1, + /* dig = hashlib.pbkdf2_hmac("sha256", "passwd".encode("utf-8"), + * "salt".encode("utf-8"), 1) + * "".join("0x{:02x}, ".format(b) for b in dig) + */ + .digest = { + 0x55, 0xac, 0x04, 0x6e, 0x56, 0xe3, 0x08, 0x9f, + 0xec, 0x16, 0x91, 0xc2, 0x25, 0x44, 0xb6, 0x05, + 0xf9, 0x41, 0x85, 0x21, 0x6d, 0xde, 0x04, 0x65, + 0xe6, 0x8b, 0x9d, 0x57, 0xc2, 0x0d, 0xac, 0xbc, + } + }, + { + .password = "RIOT", + .salt = "rocks", + .iterations = 16, + /* dig = hashlib.pbkdf2_hmac("sha256", "RIOT".encode("utf-8"), + * "rocks".encode("utf-8"), 16) + * "".join("0x{:02x}, ".format(b) for b in dig) + */ + .digest = { + 0x72, 0xa6, 0x06, 0xbb, 0x5c, 0xbe, 0x92, 0x4a, + 0xd2, 0x0a, 0xee, 0xc2, 0x4e, 0xa5, 0x17, 0xc4, + 0xd7, 0xb1, 0x1d, 0x04, 0x9d, 0x84, 0xbb, 0x29, + 0x6b, 0x36, 0xad, 0x90, 0x4d, 0x6f, 0x79, 0xdf, + } + }, + { + .password = "This is a secure password", /* <-- no it is NOT! */ + .salt = "and this salt is even more secure", + .iterations = 13, + /* dig = hashlib.pbkdf2_hmac("sha256", + * "This is a secure password".encode("utf-8"), + * "and this salt is even more secure".encode("utf-8"), + * 13) + * "".join("0x{:02x}, ".format(b) for b in dig) + */ + .digest = { + 0x9a, 0x41, 0x83, 0x2b, 0x77, 0xc4, 0x61, 0x64, + 0x06, 0xd3, 0x2e, 0x97, 0x06, 0x5e, 0xc5, 0xc7, + 0xe1, 0xa0, 0x18, 0x75, 0x01, 0xfe, 0xb8, 0xc8, + 0x70, 0x92, 0x28, 0x0e, 0x1d, 0x1a, 0x00, 0xb6, + } + }, +}; int main(void) { - static char linebuf[LINEBUF_SZ]; + bool failed = false; + for (size_t i = 0; i < ARRAY_SIZE(testcases); i++) { + struct testcase *tc = &testcases[i]; + size_t password_len = strlen(tc->password); + size_t salt_len = strlen(tc->salt); + memset(key, 0x00, sizeof(key)); + pbkdf2_sha256(tc->password, password_len, tc->salt, salt_len, + tc->iterations, key); - /* There will be a few bytes wasted here */ - static char password[LINEBUF_SZ]; - static uint8_t salt[LINEBUF_SZ]; - static uint8_t key[PBKDF2_KEY_SIZE]; - - size_t passwd_len = 0, salt_len = 0; - int iterations = 0; - - enum TEST_STATE state = TEST_READ_PASS; - - _clear_input(); - - while ((puts(input_message), fgets(linebuf, LINEBUF_SZ, stdin) != NULL)) { - char *s_end; - int conversion_status, line_len = strlen(linebuf)-1; - size_t b64_buff_size; - - linebuf[line_len] = '\0'; - - switch (state) { - case TEST_READ_PASS: - strcpy(password, linebuf); - passwd_len = line_len; - state++; - - break; - case TEST_READ_SALT: - /* work around bug in base64_decode */ - if (line_len == 0) { - salt_len = 0; - conversion_status = BASE64_SUCCESS; - } else { - salt_len = sizeof(salt); - conversion_status = base64_decode((uint8_t*)linebuf, - line_len+1, - salt, &salt_len); - } - - if (conversion_status == BASE64_SUCCESS) { - state++; - } else { - state = TEST_ERROR; - } - - break; - case TEST_READ_ITERS: - iterations = strtol(linebuf, &s_end, 10); - - if (*s_end != '\0') { - state = TEST_ERROR; - } else { - state++; - } - - break; - default: - assert(1); - break; + if (memcmp(tc->digest, key, sizeof(key)) != 0) { + failed = true; + print_str("Test vector "); + print_u32_dec((uint32_t)i); + print_str(": FAILED\n"); } - switch (state) { - case TEST_COMPUTE: - pbkdf2_sha256((uint8_t*)password, passwd_len, salt, salt_len, - iterations, key); + } - b64_buff_size = sizeof(linebuf); - conversion_status = base64_encode(key, sizeof(key), - (uint8_t*)linebuf, - &b64_buff_size); - - if (conversion_status == BASE64_SUCCESS) { - linebuf[b64_buff_size] = 0; - puts(linebuf); - } else { - puts(error_message); - } - - state = TEST_READ_PASS; - break; - case TEST_ERROR: - puts(error_message); - state = TEST_READ_PASS; - break; - default: - break; - } + if (!failed) { + print_str("TEST PASSED\n"); } return 0; diff --git a/tests/pbkdf2/tests/01-rfc.py b/tests/pbkdf2/tests/01-rfc.py deleted file mode 100755 index 394b891f2f..0000000000 --- a/tests/pbkdf2/tests/01-rfc.py +++ /dev/null @@ -1,42 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (C) 2019 Freie Universität Berlin -# -# This file is subject to the terms and conditions of the GNU Lesser -# General Public License v2.1. See the file LICENSE in the top level -# directory for more details. -# -# Author: Juan Carrano -"""Vector from RFC 7914 section 11""" - -import os - -import hashlib -import test_base - -KEY_SIZE = hashlib.sha256().digest_size - -v_easy = """55 ac 04 6e 56 e3 08 9f ec 16 91 c2 25 44 b6 05 - f9 41 85 21 6d de 04 65 e6 8b 9d 57 c2 0d ac bc - 49 ca 9c cc f1 79 b6 45 99 16 64 b3 9d 77 ef 31 - 7c 71 b8 45 b1 e3 0b d5 09 11 20 41 d3 a1 97 83""" - -v_hard = """ - 4d dc d8 f6 0b 98 be 21 83 0c ee 5e f2 27 01 f9 - 64 1a 44 18 d0 4c 04 14 ae ff 08 87 6b 34 ab 56 - a1 d4 25 a1 22 58 33 54 9a db 84 1b 51 c9 b3 17 - 6a 27 2b de bb a1 d0 78 47 8f 62 b3 97 f3 3c 8d""" - - -def process_octets(s): - return bytes(int(x, 16) for x in s.split())[:KEY_SIZE] - - -VECTORS = [ - ('passwd', b"salt", 1, process_octets(v_easy)) - ] - -if os.environ.get('BOARD') == 'native': - VECTORS.append(("Password", b"NaCl", 80000, process_octets(v_hard))) - -if __name__ == "__main__": - test_base.main(VECTORS) diff --git a/tests/pbkdf2/tests/01-run.py b/tests/pbkdf2/tests/01-run.py new file mode 100755 index 0000000000..60dd8f94f3 --- /dev/null +++ b/tests/pbkdf2/tests/01-run.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 + +# Copyright (C) 2020 Otto-von-Guericke-Universität Magdeburg +# +# This file is subject to the terms and conditions of the GNU Lesser +# General Public License v2.1. See the file LICENSE in the top level +# directory for more details. + +# @author Marian Buschsieweke + +import sys +from testrunner import run + + +def testfunc(child): + child.expect("TEST PASSED") + + +if __name__ == "__main__": + sys.exit(run(testfunc)) diff --git a/tests/pbkdf2/tests/02-random.py b/tests/pbkdf2/tests/02-random.py deleted file mode 100755 index b29add0d2e..0000000000 --- a/tests/pbkdf2/tests/02-random.py +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (C) 2019 Freie Universität Berlin -# -# This file is subject to the terms and conditions of the GNU Lesser -# General Public License v2.1. See the file LICENSE in the top level -# directory for more details. -# -# Author: Juan Carrano -"""Random test vectors""" - -from bisect import bisect as _bisect -from itertools import accumulate as _accumulate -import hashlib -import random as rand - -import test_base - - -_pass_chars = [c for c in (chr(x) for x in range(128)) - if c.isprintable()] - - -class random2(rand.Random): - # Murdock uses python 3.5 where random.choices is not available, this - # is a verbatim copy from python 3.6 - def choices(self, population, weights=None, *, cum_weights=None, k=1): - """Return a k sized list of population elements chosen with replacement. - If the relative weights or cumulative weights are not specified, - the selections are made with equal probability. - """ - random = self.random - if cum_weights is None: - if weights is None: - _int = int - total = len(population) - return [population[_int(random() * total)] for i in range(k)] - cum_weights = list(_accumulate(weights)) - elif weights is not None: - raise TypeError('Cannot specify both weights and cumulative weights') - if len(cum_weights) != len(population): - raise ValueError('The number of weights does not match the population') - bisect = _bisect.bisect - total = cum_weights[-1] - hi = len(cum_weights) - 1 - return [population[bisect(cum_weights, random() * total, 0, hi)] - for i in range(k)] - - -randgen = random2(42) - - -def randompass(length): - return "".join(randgen.choices(_pass_chars, k=length)) - - -def randomsalt(bytes_): - return (randgen.getrandbits(bytes_*8).to_bytes(bytes_, 'big') - if bytes_ else b'') - - -def randomvector(pass_len, salt_len, iters): - pass_ = randompass(pass_len) - salt = randomsalt(salt_len) - key = hashlib.pbkdf2_hmac('sha256', pass_.encode('ascii'), salt, iters) - - return pass_, salt, iters, key - - -VECTORS = [ - randomvector(0, 16, 10), - randomvector(8, 0, 10), - randomvector(9, 64, 1), - randomvector(65, 38, 20), - randomvector(32, 15, 12), - randomvector(48, 32, 15), - ] - - -if __name__ == "__main__": - test_base.main(VECTORS) diff --git a/tests/pbkdf2/tests/test_base.py b/tests/pbkdf2/tests/test_base.py deleted file mode 100644 index 29aec8ebaf..0000000000 --- a/tests/pbkdf2/tests/test_base.py +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright (C) 2019 Freie Universität Berlin -# -# This file is subject to the terms and conditions of the GNU Lesser -# General Public License v2.1. See the file LICENSE in the top level -# directory for more details. -# -# Author: Juan Carrano - -import sys -import base64 -from functools import partial - -from testrunner import run - -MAX_LINE = 128 - - -def safe_encode(data): - """Empty lines will confuse the target, replace them with padding.""" - return base64.b64encode(data).decode('ascii') if data else "" - - -def test(vectors, child): - def _safe_expect_exact(s): - idx = child.expect_exact([s+'\r\n', '{error}\r\n']) - assert idx == 0 - return idx - - def _safe_sendline(line): - assert len(line) < MAX_LINE - _safe_expect_exact('{ready}') - child.sendline(line) - - for passwd, salt, iters, key in vectors: - _safe_sendline(passwd) - _safe_sendline(safe_encode(salt)) - _safe_sendline(str(iters)) - - expected_key = base64.b64encode(key).decode('ascii') - _safe_expect_exact(expected_key) - - -def main(vectors): - sys.exit(run(partial(test, vectors)))