On 21-11-14 21:28, Anton Lundin wrote:
+// FIXME: This key is specific to the ostc3 +// The ostc sport has another key but the same protocoll +// How should we refactor the code for that? +static unsigned char ostc3_key[16] = { + 0xF1, 0xE9, 0xB0, 0x30, + 0x45, 0x6F, 0xBE, 0x55, + 0xFF, 0xE7, 0xF8, 0x31, + 0x13, 0x6C, 0xF2, 0xFE +};
This should be marked as const.
+static unsigned int hw_ostc3_firmware_checksum(hw_ostc3_firmware_t *firmware) +{ + unsigned short low = 0; + unsigned short high = 0; + for(unsigned int i = 0;i < SZ_FIRMWARE;i++) + { + low += firmware->data[i]; + high += low; + } + return (((unsigned int)high) << 16) + low; +}
This looks like a variant of the fletcher16 checksum (with a 16bit sum instead of 8bit sum, and modulo 2^n instead of 2^n-1). So I suggest moving it to checksum.c where it can be reused elsewhere.
+static dc_status_t +hw_ostc3_firmware_readline(FILE *fp, unsigned int addr, char data[], unsigned int size) +{ + char ascii[40]; + // 1 byte :, 6 bytes addr, X*2 bytes hex -> X bytes data. + unsigned line_size = size * 2 + 1 + 6 + 1; + unsigned char faddr_byte[3]; + unsigned int faddr = 0; + if (line_size > sizeof(ascii)) + return DC_STATUS_INVALIDARGS; + if (fread (ascii, sizeof(char), line_size, fp) != line_size) + return DC_STATUS_IO; + if (ascii[0] != ':') + return DC_STATUS_IO; + if (array_convert_hex2bin(ascii + 1, 6, faddr_byte, sizeof(faddr_byte)) != 0) { + return DC_STATUS_IO; + } + faddr = array_uint24_be (faddr_byte); + if (faddr != addr) + return DC_STATUS_IO; + if (array_convert_hex2bin (ascii + 1 + 6, size*2, data, size) != 0) + return DC_STATUS_IO; + + return DC_STATUS_SUCCESS; +}
This will needs some improvements to be able to deal with different line endings (e.g CRLF vs LF). See ihex.c for an example. DC_STATUS_DATAFORMAT is also a more appropriate error code than DC_STATUS_IO here. Except for the fread one.
+static dc_status_t +hw_ostc3_firmware_readfile (hw_ostc3_firmware_t *firmware, dc_context_t *context, const char *filename) +{ + dc_status_t rc = DC_STATUS_SUCCESS; + FILE *fp; + unsigned char iv[16] = {0}; + unsigned char tmpbuf[16] = {0}; + unsigned char encrypted[16] = {0}; + unsigned int bytes = 0, addr = 0, faddr = 0; + char checksum[4]; + + if (firmware == NULL) { + ERROR (context, "Invalid arguments."); + return DC_STATUS_INVALIDARGS; + } + + // Initialize the buffers. + memset (firmware->data, 0xFF, sizeof (firmware->data)); + firmware->checksum = 0; + + fp = fopen (filename, "rb"); + if (fp == NULL) { + ERROR (context, "Failed to open the file."); + return DC_STATUS_IO; + } + + rc = hw_ostc3_firmware_readline(fp, 0, iv, sizeof(iv)); + if (rc != DC_STATUS_SUCCESS) { + fclose (fp); + ERROR (context, "Failed to parse header."); + return rc; + } + bytes += 16; + + // Load the iv for AES-FCB-mode + AES128_ECB_encrypt(iv, ostc3_key, tmpbuf); + + for(addr = 0; addr < SZ_FIRMWARE; addr += 16, bytes += 16) { + rc = hw_ostc3_firmware_readline(fp, bytes, encrypted, sizeof(encrypted)); + if (rc != DC_STATUS_SUCCESS) { + fclose (fp); + ERROR (context, "Failed to parse file data."); + return rc; + } + + // Decrypt AES-FCB data + for(int i=0;i < 16;i++) + firmware->data[addr + i] = encrypted[i] ^ tmpbuf[i]; + + // Run the next round of encryption + AES128_ECB_encrypt(encrypted, ostc3_key, tmpbuf); + } + + // This file format contains a tail with the checksum in + rc = hw_ostc3_firmware_readline(fp, bytes, checksum, sizeof(checksum)); + if (rc != DC_STATUS_SUCCESS) { + fclose (fp); + ERROR (context, "Failed to parse file tail."); + return rc; + } + fclose(fp); + + firmware->checksum = array_uint32_le(checksum); + + if (firmware->checksum != hw_ostc3_firmware_checksum(firmware)) { + ERROR (context, "Failed to verify file checksum."); + return DC_STATUS_IO; + } + + return DC_STATUS_SUCCESS; +}
Two questions here: If I understand correctly, the address in the firmware file is not really an address in the normal sense, but more like a sequence number, right? Can they be in a random order, or are they always nicely incrementing as the code assumes? Is the payload always guaranteed to be 16 bytes (except for the last line with the checksum)? I assume it is because there is no length stored anywhere. Jef