From 4e927eb1cf462b5670c3962755178d634337f5af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Sun, 31 Jan 2021 12:46:34 +0100 Subject: [PATCH] sas: Fix the base64 encoding of the MAC. When calculating the MAC for a message using olm_sas_calculate_mac() and olm_sas_calculate_mac_long_kdf() the resulting MAC will be base64 encoded using _olm_encode_base64(). The _olm_encode_base64() method requires an input buffer and output buffer to be passed alongside the input length. The method is called with the same buffer, containing the MAC, for the input buffer as well as for the output buffer. This results in an incorrectly base64 encoded MAC. For example the byte array: [121, 105, 187, 19, 37, 94, 119, 248, 224, 34, 94, 29, 157, 5, 15, 230, 246, 115, 236, 217, 80, 78, 56, 200, 80, 200, 82, 158, 168, 179, 10, 230] will be encoded as eWm7NyVeVmXgbVhnYlZobllsWm9ibGxzV205aWJHeHo instead of as eWm7EyVed/jgIl4dnQUP5vZz7NlQTjjIUMhSnqizCuY Notice the different value at the 10th character. The correct result can be independently checked using Python for example: >>> from base64 import b64encode >>> mac = [121, 105, 187, 19, 37, 94, 119, 248, 224, 34, 94, 29, 157, \ 5, 15, 230, 246, 115, 236, 217, 80, 78, 56, 200, 80, 200, \ 82, 158, 168, 179, 10, 230] >>> b64encode(bytearray(mac)).rstrip(b"=") >>> b'eWm7EyVed/jgIl4dnQUP5vZz7NlQTjjIUMhSnqizCuY' This happens because the encode_base64() method that is used does not support in-place encoding in the general case. This is because the remainder for a 32 bit input will always be 2 (32 % 6 == 2). The remainder will be used over here: https://gitlab.matrix.org/matrix-org/olm/-/blob/c01164f001d57fbe2297fe11954b58077a68dc0d/src/base64.cpp#L74 The logic that gets executed if a remainder exists depends on the original input values, since those already got in-place encoded the whole block will behave differently if the input buffer is the same as the output buffer. --- src/sas.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/sas.c b/src/sas.c index 7117fed..830ae21 100644 --- a/src/sas.c +++ b/src/sas.c @@ -152,8 +152,11 @@ size_t olm_sas_calculate_mac( (const uint8_t *) info, info_length, key, 32 ); - _olm_crypto_hmac_sha256(key, 32, input, input_length, mac); - _olm_encode_base64((const uint8_t *)mac, SHA256_OUTPUT_LENGTH, (uint8_t *)mac); + + uint8_t temp_mac[32]; + _olm_crypto_hmac_sha256(key, 32, input, input_length, temp_mac); + _olm_encode_base64((const uint8_t *)temp_mac, SHA256_OUTPUT_LENGTH, (uint8_t *)mac); + return 0; } @@ -179,7 +182,10 @@ size_t olm_sas_calculate_mac_long_kdf( (const uint8_t *) info, info_length, key, 256 ); - _olm_crypto_hmac_sha256(key, 256, input, input_length, mac); - _olm_encode_base64((const uint8_t *)mac, SHA256_OUTPUT_LENGTH, (uint8_t *)mac); + + uint8_t temp_mac[32]; + _olm_crypto_hmac_sha256(key, 256, input, input_length, temp_mac); + _olm_encode_base64((const uint8_t *)temp_mac, SHA256_OUTPUT_LENGTH, (uint8_t *)mac); + return 0; }