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

[Anton Lundin]

On 15 December, 2014 - Jef Driesen wrote:

> 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.
> 

Agreed and fixed.

> >+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.
> 

Is that such a wise idea? Sure, its a cousin of fletcher16, but do you
think anyone else will be using the same cousin of fletcher16?

If it where a straight fletcher16, i would have agreed with you but this
one, I'm not so sure. Just think, what should we name the darn thing
when we move it to checksum.c? ostc3_firmware_fletcher16_almost()?

> >+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.
> 

None of the Ostc3 or Ostc Sport firmware files have CRLF endings.
Probably due to the tool with which they bin generated.

But, when re-reading the HexFile::loadEncrypted of ostc-planner, it
would accept a file with CRLF encodings, so we should do the same.

Fixed.

> DC_STATUS_DATAFORMAT is also a more appropriate error code than DC_STATUS_IO
> here. Except for the fread one.
> 

Agreed and fixed.

> >+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?
> 

All the firmware files I've seen, they are always linear. The
ostc-planner code also assumes that they always will be and will fail
reading the file if they aren't, so thats the reason this code will
reject such a file to.

> 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.
> 

I haven't seen any files in any other format. They are all the exact
same size, 307256.

If yo take a look at the decrypted data, its padded to be this exact
firmware size.


//Anton



-- 
Anton Lundin	+46702-161604