[PATCH 02/11] This adds code to read OSTC3 firmware files.

[Jef Driesen]

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