From 89b10939f953146a70ac45820544f58ed45b3fb4 Mon Sep 17 00:00:00 2001 From: Werner Almesberger Date: Mon, 18 Jun 2012 18:22:56 -0300 Subject: [PATCH] make firmware upload protocol less secure but lean; boot loader works --- NOTES | 2 + PROTOCOL | 10 +-- fw/Makefile | 17 +++-- fw/antorcha.c | 63 +++++++++---------- fw/boot.c | 12 ++-- fw/dispatch.c | 40 +++++------- fw/dispatch.h | 2 + fw/flash.c | 6 +- fw/flash.h | 2 +- fw/fw.c | 158 ++++++++++++++++++----------------------------- fw/fw.h | 6 +- fw/proto.h | 2 - fw/rf.c | 6 ++ tools/antorcha.c | 47 ++++---------- 14 files changed, 149 insertions(+), 224 deletions(-) diff --git a/NOTES b/NOTES index 69f19ce..4ce91f2 100644 --- a/NOTES +++ b/NOTES @@ -93,3 +93,5 @@ authentication: - shared secret (128 bit, SHA1-hashed text with 128 bit salt) - salt (128 bit) - SHA1 from avrcryptolib +- due to chip limitations, the secret key for firmware updates is sent + in the clear diff --git a/PROTOCOL b/PROTOCOL index 9f5d3b0..f55afc8 100644 --- a/PROTOCOL +++ b/PROTOCOL @@ -7,14 +7,8 @@ Protocol 1 0 0 Pong (maybe return version string in the future) - 2 0 3 Unlock salt A (64 bytes payload) - 2 1 3 Unlock salt B - 2 2 3 Unlock hash A - 2 3 3 Unlock hash B - - 3 n 0 Unlock ACK - - 4 0..N-1 N Firmware binary (64 bytes payload) + 4 0 N Unlock secret (64 bytes payload) + 4 1..N-1 N Firmware binary (64 bytes payload) 4 N N First half of hash 5 n N Firmware ACK diff --git a/fw/Makefile b/fw/Makefile index 443f05b..90e12d6 100644 --- a/fw/Makefile +++ b/fw/Makefile @@ -15,15 +15,13 @@ SHELL = /bin/bash NAME = antorcha CFLAGS = -g -mmcu=$(CHIP) \ - -DBOOT_ADDR=$(BOOT_ADDR) -DAPP_ADDR=$(APP_ADDR) -DAPP_END=$(APP_END) \ + -DBOOT_ADDR=$(BOOT_ADDR) \ -Wall -Wextra -Wshadow -Werror -Wno-unused-parameter \ -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes CHIP = atmega168 HOST = jlime -BOOT_ADDR = 0 -APP_ADDR = 0x1000 -APP_END = 0x4000 +BOOT_ADDR = 0x3800 AVR_PREFIX = $(BIN_PATH) avr- CC = $(AVR_PREFIX)gcc @@ -31,9 +29,9 @@ OBJCOPY = $(AVR_PREFIX)objcopy #OBJDUMP = $(AVR_PREFIX)objdump SIZE = $(AVR_PREFIX)size -OBJS = $(NAME).o $(COMMON_OBJS) +OBJS = $(NAME).o dispatch.o hash.o $(COMMON_OBJS) BOOT_OBJS = boot.o flash.o fw.o $(COMMON_OBJS) -COMMON_OBJS = dispatch.o hash.o rf.o spi.o +COMMON_OBJS = rf.o spi.o # ----- Verbosity control ----------------------------------------------------- @@ -64,8 +62,7 @@ all: $(NAME).bin boot.bin $(NAME).elf: $(OBJS) $(MAKE) version.o - $(CC) $(CFLAGS) -o $@ $(OBJS) version.o \ - -Wl,--section-start=.text=$(APP_ADDR) + $(CC) $(CFLAGS) -o $@ $(OBJS) version.o boot.elf: $(BOOT_OBJS) $(CC) $(CFLAGS) -o $@ $(BOOT_OBJS) \ @@ -140,7 +137,9 @@ prog-app: prog: ssh $(HOST) avrdude -F -p $(CHIP) -c nanonote_antorcha -e \ - -U flash:w:boot.hex:i + -U flash:w:boot.hex:i \ + -U efuse:w:0x00:m \ + -U lfuse:w:0xe2:m # -U lfuse:w:0x60:m \ # -U hfuse:w:0xd8:m \ # -U lock:w:0x2f:m diff --git a/fw/antorcha.c b/fw/antorcha.c index 9d36889..04e3c72 100644 --- a/fw/antorcha.c +++ b/fw/antorcha.c @@ -1,5 +1,5 @@ /* - * fw/main.c - Initialization of Antorcha firmware + * fw/antorcha.c - Initialization of Antorcha application firmware * * Written 2012 by Werner Almesberger * Copyright 2012 Werner Almesberger @@ -11,50 +11,43 @@ */ +#include +#include + #include +#define F_CPU 8000000UL +#include #include "io.h" #include "rf.h" +#include "dispatch.h" + + + +static const struct handler *protos[] = { + NULL +}; int main(void) { - /* Port B has two LEDs and the rest is SPI */ - PORTB = MASK(B, RF_nSS) | MASK(B, RF_nRST); - DDRB = MASK(B, RF_SCLK) | MASK(B, RF_MOSI) | MASK(B, RF_nSS) | - MASK(B, RF_nRST) | 0xc0; + uint8_t buf[PAYLOAD+5]; /* 3 bytes header, 2 bytes CRC */ + uint8_t got; - /* All port C pins drive LEDs */ - PORTC = 0; - DDRC = 0x3f; - - /* All port D pins drive LEDs */ - PORTD = 0; - DDRD = 0xff; - - /* disable pull-ups */ - MCUCR |= 1 << PUD; - - rf_init(); - - while (0) { - PORTC = 2; - PORTC = 0; - rf_send("HOLA ?", 6); - } + /* + * The boot loader has already initialized PORTx, DDRx, and MCUCR.PUD. + * It has also brought up RF and the underlying SPI. + */ +while (1) { + SET(LED_B8); + _delay_ms(100); + CLR(LED_B8); + _delay_ms(100); +} while (1) { - uint8_t buf[1], got; - - do { -PORTC = 1; -PORTC = 0; -got = rf_recv(buf, 1); -} - while (!got); - -PORTC = 2; -PORTC = 0; - PORTD = buf[0]; + got = rf_recv(buf, sizeof(buf)); + if (got > 2) + dispatch(buf, got-2, protos); } } diff --git a/fw/boot.c b/fw/boot.c index b019560..f497e53 100644 --- a/fw/boot.c +++ b/fw/boot.c @@ -21,19 +21,19 @@ #include "fw.h" -#define MS_TO_LOOP(ms) ((uint32_t) (ms)*335) +#define MS_TO_LOOP(ms) ((uint32_t) (ms)*106) static void wait_upload(void) { - uint8_t buf[PAYLOAD+5]; + uint8_t buf[PAYLOAD+5]; /* 3 bytes header, 2 bytes CRC */ uint32_t i; uint8_t got; restart: - for (i = 0; i != MS_TO_LOOP(2000); i++) { + for (i = 0; i != MS_TO_LOOP(5000); i++) { got = rf_recv(buf, sizeof(buf)); - if (got && dispatch(buf, got, fw_protos)) + if (got > 2 && fw_packet(buf, got-2)) goto restart; } @@ -70,9 +70,7 @@ int main(void) SET(LED_B8); wait_upload(); CLR(LED_B8); -while (1); - } while (pgm_read_byte(zero) != 0xff); - + } while (pgm_read_byte(zero) == 0xff); ((void (*)(void)) 0)(); diff --git a/fw/dispatch.c b/fw/dispatch.c index e80d3c9..feaa5bf 100644 --- a/fw/dispatch.c +++ b/fw/dispatch.c @@ -15,13 +15,9 @@ #include #include -#define F_CPU 8000000UL -#include - #include "rf.h" #include "proto.h" #include "dispatch.h" -#include "io.h" static uint8_t seq; /* last sequence number seen */ @@ -34,10 +30,7 @@ static void send_ack(const uint8_t *buf) { uint8_t ack[3] = { buf[0]+1, buf[1], 0 }; -SET(LED_B6); - _delay_ms(1); rf_send(ack, sizeof(ack)); -CLR(LED_B6); } @@ -54,8 +47,6 @@ static bool answer_ping(const uint8_t *buf) bool dispatch(const uint8_t *buf, uint8_t len, const struct handler **protos) { -SET(LED_B7); -CLR(LED_B7); if (len == 3 && buf[0] == PING) return answer_ping(buf); @@ -78,23 +69,24 @@ CLR(LED_B7); limit = buf[2]; send_ack(buf); return 1; + } else { + if (!curr_proto) + return 0; + if (buf[0] != type) + return 0; + if (buf[2] != limit) + return 0; + if (buf[1] > limit) + return 0; + + if (buf[1]+1 == seq) { + send_ack(buf); + return 0; + } + if (buf[1] != seq) + return 0; } - if (!curr_proto) - return 0; - if (buf[0] != type) - return 0; - if (buf[1] > limit) - return 0; - if (buf[2] != limit) - return 0; - - if (buf[1] == seq) { - send_ack(buf); - return 0; - } - if (buf[1] != seq+1) - return 0; if (!curr_proto->more(buf[1], limit, buf+3)) return 0; seq++; diff --git a/fw/dispatch.h b/fw/dispatch.h index 51a4bc3..ccd56ea 100644 --- a/fw/dispatch.h +++ b/fw/dispatch.h @@ -16,6 +16,8 @@ #include #include +#include "proto.h" + struct handler { enum pck_type type; diff --git a/fw/flash.c b/fw/flash.c index b0435ef..f409bea 100644 --- a/fw/flash.c +++ b/fw/flash.c @@ -26,15 +26,15 @@ static uint32_t payload; -void flash_start(uint32_t addr) +void flash_start(void) { - payload = addr; + payload = 0; } int flash_can_write(uint16_t size) { - return payload <= APP_END-size; + return payload <= BOOT_ADDR-size; } diff --git a/fw/flash.h b/fw/flash.h index c43c12a..2a4d7d0 100644 --- a/fw/flash.h +++ b/fw/flash.h @@ -16,7 +16,7 @@ #include -void flash_start(uint32_t addr); +void flash_start(void); int flash_can_write(uint16_t size); void flash_write(const uint8_t *buf, uint16_t size); void flash_end_write(void); diff --git a/fw/fw.c b/fw/fw.c index e3bcec0..1472277 100644 --- a/fw/fw.c +++ b/fw/fw.c @@ -1,5 +1,5 @@ /* - * fw/fw.h - Firmware upload protocols + * fw/fw.h - Firmware upload protocol * * Written 2012 by Werner Almesberger * Copyright 2012 Werner Almesberger @@ -11,123 +11,85 @@ */ -#include #include #include +#include -#include "hash.h" +#include "io.h" #include "flash.h" #include "proto.h" -#include "dispatch.h" +#include "rf.h" #include "fw.h" -static const uint8_t unlock_secret[] = { +static const uint8_t unlock_secret[PAYLOAD] = { #include "unlock-secret.inc" }; -static void panic(void) +static bool locked = 1; + + +static bool fw_payload(uint8_t seq, uint8_t limit, const uint8_t *payload) { - /* ??? */ -} - - -/* ----- Unlocking --------------------------------------------------------- */ - - -static bool unlocked = 0; -static bool unlock_failed; - - -static bool unlock_first(const uint8_t *payload) -{ - hash_init(); - hash_merge(unlock_secret, sizeof(unlock_secret)); - hash_merge(payload, PAYLOAD); - unlocked = 0; - unlock_failed = 0; - return 1; -} - - -static bool unlock_more(uint8_t seq, uint8_t limit, const uint8_t *payload) -{ - switch (seq) { - case 1: - hash_merge(payload, PAYLOAD); - hash_end(); - break; - case 2: - if (!hash_eq(payload, PAYLOAD, 0)) - unlock_failed = 1; - break; - case 3: - if (unlock_failed) - return 1; - if (hash_eq(payload, PAYLOAD, PAYLOAD)) - unlocked = 1; - else - unlock_failed = 1; - break; - default: - return 0; - } - return 1; -} - - -static const struct handler unlock_proto = { - .type = UNLOCK, - .first = unlock_first, - .more = unlock_more, -}; - - -/* ----- Firmware upload --------------------------------------------------- */ - - -static bool fw_first(const uint8_t *payload) -{ -// if (!unlocked) -// return 0; - hash_init(); - hash_merge(payload, PAYLOAD); - flash_start(APP_ADDR); - flash_write(payload, PAYLOAD); - return 1; -} - - -static bool fw_more(uint8_t seq, uint8_t limit, const uint8_t *payload) -{ - if (!flash_can_write(PAYLOAD)) - return 0; - if (seq != limit) { - hash_merge(payload, PAYLOAD); - flash_write(payload, PAYLOAD); + if (!seq) { + locked = memcmp(unlock_secret, payload, PAYLOAD) != 0; + flash_start(); return 1; } - flash_end_write(); - hash_end(); - if (!hash_eq(payload, PAYLOAD, 0)) - panic(); + if (locked) + return 0; + if (!flash_can_write(PAYLOAD)) + return 0; + flash_write(payload, PAYLOAD); + if (seq == limit) + flash_end_write(); return 1; } -static const struct handler fw_proto = { - .type = FIRMWARE, - .first = fw_first, - .more = fw_more, -}; +bool fw_packet(const uint8_t *buf, uint8_t len) +{ + static uint8_t seq = 0; + static uint8_t limit; + uint8_t ack[] = { FIRMWARE+1, buf[1], buf[2] }; + /* short (barely visible) flash to indicate reception */ + SET(LED_B7); + CLR(LED_B7); -/* ----- Protocol table ---------------------------------------------------- */ + /* Check packet for formal validity */ + if (len != 64+3) + return 0; + if (buf[0] != FIRMWARE) + return 0; + if (buf[1] > buf[2]) + return 0; -const struct handler *fw_protos[] = { - &unlock_proto, - &fw_proto, - NULL -}; + /* Synchronize sequence numbers */ + + if (!buf[1]) { + seq = buf[1]; + limit = buf[2]; + } else { + if (buf[2] != limit) + return 0; + if (buf[1]+1 == seq) + goto ack; + if (buf[1] != seq) + return 0; + } + + /* Process the payload */ + + if (!fw_payload(buf[1], limit, buf+3)) + return 0; + seq++; +ack: + /* clearly visible short blink to indicate progress */ + SET(LED_B6); + rf_send(ack, sizeof(ack)); + CLR(LED_B6); + return 1; +} diff --git a/fw/fw.h b/fw/fw.h index 62dfcb7..6b8d9fd 100644 --- a/fw/fw.h +++ b/fw/fw.h @@ -1,5 +1,5 @@ /* - * fw/fw.h - Firmware upload protocols + * fw/fw.h - Firmware upload protocol * * Written 2012 by Werner Almesberger * Copyright 2012 Werner Almesberger @@ -13,9 +13,9 @@ #ifndef FW_H #define FW_H -#include "proto.h" +#include -extern const struct handler *fw_protos[]; +bool fw_packet(const uint8_t *buf, uint8_t len); #endif /* !FW_H */ diff --git a/fw/proto.h b/fw/proto.h index 1679529..7f7015a 100644 --- a/fw/proto.h +++ b/fw/proto.h @@ -18,8 +18,6 @@ enum pck_type { PING = 0, /* version query */ PONG = 1, /* version response */ - UNLOCK = 2, /* unlock firmware upload */ - UNLOCK_ACK = 3, /* unlock acknowledgement */ FIRMWARE = 4, /* firmware upload */ FIRMWARE_ACK = 5, /* firmware upload acknowledgement */ IMAGE = 6, /* image upload */ diff --git a/fw/rf.c b/fw/rf.c index a7a6303..87847d5 100644 --- a/fw/rf.c +++ b/fw/rf.c @@ -89,6 +89,9 @@ void rf_send(const void *buf, uint8_t size) reg_write(REG_TRX_STATE, TRX_CMD_PLL_ON); _delay_us(1); /* tTR9 = 1 us */ + /* be nice to senders with long turn-around time, e.g., atusb */ + _delay_ms(2); + spi_begin(); spi_send(AT86RF230_BUF_WRITE); spi_send(size+2); /* CRC */ @@ -115,6 +118,9 @@ uint8_t rf_recv(void *buf, uint8_t size) { uint8_t irq, len, i; + if (!PIN(RF_IRQ)) + return 0; + irq = reg_read(REG_IRQ_STATUS); if (!(irq & IRQ_TRX_END)) return 0; diff --git a/tools/antorcha.c b/tools/antorcha.c index 4881e82..d4754d2 100644 --- a/tools/antorcha.c +++ b/tools/antorcha.c @@ -45,13 +45,17 @@ static void rf_init(struct atrf_dsc *dsc, int trim, int channel) static void rf_send(struct atrf_dsc *dsc, void *buf, int len) { + uint8_t tmp[MAX_PSDU]; + + /* Copy the message to append the CRC placeholders */ + memcpy(tmp, buf, len); atrf_reg_write(dsc, REG_TRX_STATE, TRX_CMD_PLL_ON); - atrf_buf_write(dsc, buf, len); + atrf_buf_write(dsc, tmp, len+2); atrf_reg_write(dsc, REG_TRX_STATE, TRX_CMD_TX_START); wait_for_interrupt(dsc, IRQ_TRX_END, IRQ_TRX_END | IRQ_PLL_LOCK, 10); atrf_reg_write(dsc, REG_TRX_STATE, TRX_CMD_RX_ON); -#if 1 +#if 0 int i; fprintf(stderr, "\r%d:", len); for (i = 0; i != len; i++) @@ -96,7 +100,7 @@ static void ping(struct atrf_dsc *dsc) static void packet(struct atrf_dsc *dsc, - uint8_t type, uint8_t seq, uint8_t last, void *payload, int len) + uint8_t type, uint8_t seq, uint8_t last, const void *payload, int len) { uint8_t tx_buf[PAYLOAD+3] = { type, seq, last }; uint8_t rx_buf[10]; @@ -109,8 +113,11 @@ static void packet(struct atrf_dsc *dsc, if (verbose) write(2, ">", 1); got = rf_recv(dsc, rx_buf, sizeof(rx_buf)); - if (got <= 0) + if (got <= 0) { + if (!seq && verbose) + write(2, "\b", 1); continue; + } if (verbose) write(2, "\b?", 2); if (got < 3) @@ -126,34 +133,11 @@ static void packet(struct atrf_dsc *dsc, } -static const uint8_t unlock_secret[] = { +static const uint8_t unlock_secret[PAYLOAD] = { #include "unlock-secret.inc" }; -static void unlock(struct atrf_dsc *dsc) -{ - uint8_t payload[PAYLOAD]; - - if (verbose) - write(2, "unlock ", 9); - memset(payload, 0, PAYLOAD); - hash_init(); - hash_merge(unlock_secret, sizeof(unlock_secret)); - hash_merge(payload, PAYLOAD); - hash_merge(payload, PAYLOAD); - packet(dsc, UNLOCK, 0, 3, payload, PAYLOAD); - packet(dsc, UNLOCK, 1, 3, payload, PAYLOAD); - hash_end(); - hash_cp(payload, PAYLOAD, 0); - packet(dsc, UNLOCK, 2, 3, payload, PAYLOAD); - hash_cp(payload, PAYLOAD, PAYLOAD); - packet(dsc, UNLOCK, 3, 3, payload, PAYLOAD); - if (verbose) - write(2, "\n", 1); -} - - static void send_firmware(struct atrf_dsc *dsc, void *buf, int len) { uint8_t payload[PAYLOAD]; @@ -163,9 +147,9 @@ static void send_firmware(struct atrf_dsc *dsc, void *buf, int len) write(2, "firmware ", 9); last = (len+63)/64; seq = 0; + packet(dsc, FIRMWARE, seq++, last, unlock_secret, PAYLOAD); while (len >= PAYLOAD) { packet(dsc, FIRMWARE, seq++, last, buf, PAYLOAD); - hash_merge(buf, PAYLOAD); buf += PAYLOAD; len -= PAYLOAD; } @@ -173,11 +157,7 @@ static void send_firmware(struct atrf_dsc *dsc, void *buf, int len) memcpy(payload, buf, len); memset(payload+len, 0, PAYLOAD-len); packet(dsc, FIRMWARE, seq++, last, payload, PAYLOAD); - hash_merge(payload, PAYLOAD); } - hash_end(); - hash_cp(payload, PAYLOAD, 0); - packet(dsc, FIRMWARE, seq, last, payload, PAYLOAD); if (verbose) write(2, "\n", 1); } @@ -201,7 +181,6 @@ static void firmware(struct atrf_dsc *dsc, const char *name) } fclose(file); - unlock(dsc); send_firmware(dsc, fw, len); }