Can't delete certain unnecessary lines of code

Zolcsi
Posts: 2
Joined: Tue Aug 16, 2022 5:45 pm

Can't delete certain unnecessary lines of code

Postby Zolcsi » Tue Aug 16, 2022 6:06 pm

Hi there,

I have a problem that is killing me... :)
I have a project where I'm using an ESP32-C3. It needs to verify a message received from a server via bluetooth. I am trying to utilise HMAC to do that. This is how it's going down at the moment:
There are 3 parties in this: 1: ESP32 powered device. 2: Mobile phone running a custom app that is communicating with the ESP via BLE (Nimble). 3: A webserver that issues the tokens to the mobile phone upon request.
So the exact steps are:
1: Phone connects to ESP32 and asks for its MAC address and a random salt
2: ESP32 generates a random salt, stores it (and deletes if after a certain period of time if nothing happens)
3: Phone now sends the salt and the MAC address to the server. If it has the necessary permission, then the server issues a token that is built like this: $hash = hash_hmac('sha256', $macAddress.$salt.$serverSalt, $sharedSecret, true); return base64_encode($serverSalt.$hash);
serverSalt is fixed length so the client knows where to split the string to get the serverSalt. Also, sharedSecret is the same, that is stored on the ESP32
4: Phone forwards the received hmac message to ESP32
5: ESP32 validates the received token.

Now, to be perfectly clear, my code work pretty much OK at the moment, but there are certain lines of codes that shouldn't be necessary, but I can't delete them, otherwise the locally calculated hmac will be different from the one provided by the server (which is the good one).

So this is my code:

Code: Select all

static int open_lock(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg)
{
    ESP_LOGI("OPEN", "=========== Trying to open lock ===========");
    ESP_LOGI("OPEN", "Current token: %s", token);

    unsigned char decodedToken[256];
    size_t decodedTokenLen;
    mbedtls_base64_decode(decodedToken, 256, &decodedTokenLen, (unsigned char *) token, strlen(token));

    char serverSalt[13] = {0};
    memcpy(serverSalt, decodedToken, 12);
    serverSalt[12] = '\0';
    ESP_LOGI("OPEN", "Server salt: %s", serverSalt);

    //XXXXXX These three rows are not needed and should be deleted, but I can't do that or the calculated hmac will differ
    unsigned char xOutput[128];
    size_t xOutputLen;
    mbedtls_base64_encode(xOutput, 128, &xOutputLen, (unsigned char*) serverSalt, strlen(serverSalt));
    //EOF Ki tudja mi

    ESP_LOGI("OPEN", "Local salt: %s", salt);
    char salt64[24];
    size_t salt64len;
    mbedtls_base64_encode((unsigned char *) salt64, 24, &salt64len, (unsigned char*) salt, strlen(salt));
    ESP_LOGI("HMAC", "Local salt (base64): %s", salt64);
    salt64[salt64len] = '\0';

    uint8_t baseMac[6];
    esp_read_mac(baseMac, ESP_MAC_BT);
    char serialNumber[18];
    sprintf(serialNumber, "%X:%X:%X:%X:%X:%X", baseMac[0], baseMac[1], baseMac[2], baseMac[3], baseMac[4], baseMac[5]);
    serialNumber[17] = '\0';
    ESP_LOGI("OPEN", "MAC address used for HMAC: %s", serialNumber);

    char data[64];
    const size_t data_len = strlen(serialNumber) + strlen(salt64) + strlen(serverSalt);
    memset(data, '\0', sizeof(data));
    //XXXXXX These three rows are not needed and should be deleted, but I can't do that or the calculated hmac will differ
    ESP_LOGI("OPEN", "serial: %d, salt: %d, serverSalt: %d", strlen(serialNumber), strlen(salt64), strlen(serverSalt));
    snprintf(data, data_len+1, "%s%s%s", serialNumber, salt64, serverSalt);
    printf("Message for HMAC coding: %s||||\n", data);

    uint8_t hmac[33];
    esp_err_t result = esp_hmac_calculate(HMAC_KEY5, data, data_len, hmac);
    hmac[32] = '\0';
    if (result == ESP_OK) {
        //successful HMAC calculation
        ESP_LOGI("TOKEN", "HMAC calculated: %s", hmac);

        char serverSaltHmac[64];
        memcpy(serverSaltHmac, serverSalt, strlen(serverSalt));
        strcat((char *)serverSaltHmac, (char *)hmac);
        serverSaltHmac[strlen((char *)serverSalt)+strlen((char *)hmac)] = '\0';

        unsigned char base64Output[128];
        size_t base64OutputLen;
        mbedtls_base64_encode(base64Output, 128, &base64OutputLen, (unsigned char *) serverSaltHmac, strlen(serverSaltHmac));
        ESP_LOGI("HMAC", "Base64 encoded output: %s", base64Output);
    } else {
        ESP_LOGI("TOKEN", "HMAC calculation error: %d", result);
    }
/*
TODO A Vegen kiirni az uj lock statust
    os_mbuf_append(ctxt->om, &status, sizeof(status));
    printf("reading lock status: %i\n", status);
    */
    //TODO Token es salt torles
    memset(token, '\0', sizeof(token));
    memset(salt, '\0', sizeof(salt));
    return 0;
}
So the code above works, however if I delete any of the indicated lines, then the calculated hmac will differ from the real one. I confirmed that "data" contains the correct value always, so the issue must be afterwards with the actual hmac generation somewhere or with the base4 output generation.
I'm pretty new to C, so I'm my best guess is that strlen is doing something to the variables because it takes a pointer as argument, but I'm really lost here, so any advice would be greatly appreciated. I'll try to take a look at it tomorrow with a clean head, but hopefully someone will spot my mistake by then :)

Thanks!

ESP_Sprite
Posts: 8921
Joined: Thu Nov 26, 2015 4:08 am

Re: Can't delete certain unnecessary lines of code

Postby ESP_Sprite » Tue Aug 16, 2022 11:38 pm

I'm not seeing a particular issue, but one thing of note is that you're handling the output of base64decode as strings. That function can output 0 bytes as well (dependent on the input) which would terminate the string right there. It's probably better if you handle binary data as binary data: don't rely on a zero terminator, but store the length in a separate variable.

Zolcsi
Posts: 2
Joined: Tue Aug 16, 2022 5:45 pm

Re: Can't delete certain unnecessary lines of code

Postby Zolcsi » Wed Aug 17, 2022 7:50 am

Thanks a lot @ESP_Sprite, your answer put me in the right direction. I rewrote the code to minimize the usage of string functions and now it works perfectly fine! Here is the rewritten code for future reference in case anybody stumbles on this topic:

Code: Select all

static int open_lock(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg)
{
    ESP_LOGI("OPEN", "=========== Trying to open lock ===========");

    unsigned char decodedToken[64] = {'\0'};
    size_t decodedTokenLen;
    mbedtls_base64_decode(decodedToken, 64, &decodedTokenLen, (unsigned char *) token, strlen(token));

    char hmacBase[128] = {'\0'};
    size_t hmacBaseLength = 0;

    char serialNumber[18];
    getMac(serialNumber);
    memcpy(hmacBase, serialNumber, 17);
    hmacBaseLength += 17;

    memcpy(hmacBase + hmacBaseLength, salt, 12);
    hmacBaseLength += 12;

    memcpy(hmacBase + hmacBaseLength, decodedToken, 12);
    hmacBaseLength += 12;

    uint8_t hmac[33];
    esp_err_t result = esp_hmac_calculate(HMAC_KEY5, hmacBase, hmacBaseLength, hmac);
    if (result == ESP_OK) {
        //successful HMAC calculation
        char serverSaltHmac[64] = {'\0'};
        memcpy(serverSaltHmac, decodedToken, 12);
        memcpy(serverSaltHmac+12, hmac, 32);
        if(strcmp((char *)serverSaltHmac, (char *)decodedToken) == 0) {
            //code matches
        } else {
            //invalid code
        }
    } else {
        ESP_LOGI("TOKEN", "HMAC calculation error: %d", result);
    }
    memset(token, '\0', sizeof(token));
    memset(salt, '\0', sizeof(salt));
    return 0;
}

ESP_Sprite
Posts: 8921
Joined: Thu Nov 26, 2015 4:08 am

Re: Can't delete certain unnecessary lines of code

Postby ESP_Sprite » Thu Aug 18, 2022 1:12 am

Nice, glad you could make it work!

Who is online

Users browsing this forum: ESP_Roland, FrankJensen and 115 guests