From ff14c17de794fe85299d90e34e12a677e6148b71 Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Mon, 25 Apr 2022 18:34:45 +0200 Subject: [PATCH] Use constant time conversion for crypt_hex_to_bytes. We use hexa conversions for keys, avoid possible leaks with cover channels by making these functions constant time. --- lib/utils_crypt.c | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/lib/utils_crypt.c b/lib/utils_crypt.c index 480506b5..2fde4d41 100644 --- a/lib/utils_crypt.c +++ b/lib/utils_crypt.c @@ -152,10 +152,45 @@ int crypt_parse_pbkdf(const char *s, const char **pbkdf) return 0; } +/* + * Thanks Mikulas Patocka for these two char converting functions. + * + * This function is used to load cryptographic keys, so it is coded in such a + * way that there are no conditions or memory accesses that depend on data. + * + * Explanation of the logic: + * (ch - '9' - 1) is negative if ch <= '9' + * ('0' - 1 - ch) is negative if ch >= '0' + * we "and" these two values, so the result is negative if ch is in the range + * '0' ... '9' + * we are only interested in the sign, so we do a shift ">> 8"; note that right + * shift of a negative value is implementation-defined, so we cast the + * value to (unsigned) before the shift --- we have 0xffffff if ch is in + * the range '0' ... '9', 0 otherwise + * we "and" this value with (ch - '0' + 1) --- we have a value 1 ... 10 if ch is + * in the range '0' ... '9', 0 otherwise + * we add this value to -1 --- we have a value 0 ... 9 if ch is in the range '0' + * ... '9', -1 otherwise + * the next line is similar to the previous one, but we need to decode both + * uppercase and lowercase letters, so we use (ch & 0xdf), which converts + * lowercase to uppercase + */ +static int hex_to_bin(unsigned char ch) +{ + unsigned char cu = ch & 0xdf; + return -1 + + ((ch - '0' + 1) & (unsigned)((ch - '9' - 1) & ('0' - 1 - ch)) >> 8) + + ((cu - 'A' + 11) & (unsigned)((cu - 'F' - 1) & ('A' - 1 - cu)) >> 8); +} + ssize_t crypt_hex_to_bytes(const char *hex, char **result, int safe_alloc) { - char buf[3] = "xx\0", *endp, *bytes; + char *bytes; size_t i, len; + int bl, bh; + + if (!hex || !result) + return -EINVAL; len = strlen(hex); if (len % 2) @@ -167,12 +202,13 @@ ssize_t crypt_hex_to_bytes(const char *hex, char **result, int safe_alloc) return -ENOMEM; for (i = 0; i < len; i++) { - memcpy(buf, &hex[i * 2], 2); - bytes[i] = strtoul(buf, &endp, 16); - if (endp != &buf[2]) { + bh = hex_to_bin(hex[i * 2]); + bl = hex_to_bin(hex[i * 2 + 1]); + if (bh == -1 || bl == -1) { safe_alloc ? crypt_safe_free(bytes) : free(bytes); return -EINVAL; } + bytes[i] = (bh << 4) | bl; } *result = bytes; return i;