From 587ecdd72d8d186732651aada681c0327d34924a Mon Sep 17 00:00:00 2001 From: jaseg Date: Tue, 13 Nov 2018 15:51:35 +0900 Subject: Host handshake mostly working --- hexnoise.py | 20 +++++++++++++++++--- src/cobs.c | 6 +++++- src/demo.c | 50 +++++++++++++++++++++++++++++++++++++++----------- src/noise.c | 12 +++++++----- src/noise.h | 4 ++-- src/packet_interface.c | 18 ++++++++++++++++-- src/packet_interface.h | 3 +++ src/usart_helpers.c | 7 +++++-- 8 files changed, 94 insertions(+), 26 deletions(-) diff --git a/hexnoise.py b/hexnoise.py index 8745159..0c851d3 100755 --- a/hexnoise.py +++ b/hexnoise.py @@ -19,6 +19,9 @@ class PacketType(enum.Enum): INITIATE_HANDSHAKE = 1 HANDSHAKE = 2 DATA = 3 + COMM_ERROR = 4 + CRYPTO_ERROR = 5 + TOO_MANY_FAILS = 6 class ReportType(enum.Enum): _RESERVED = 0 @@ -45,10 +48,20 @@ class Packetizer: def receive_packet(self): packet = self.ser.read_until(b'\0') data = cobs.decode(packet[:-1]) + if self.debug: print(f'\033[93mReceived {len(data)} bytes\033[0m') hexdump(print, data, self.width) - return PacketType(data[0]), data[1:] + + pkt_type, data = PacketType(data[0]), data[1:] + if pkt_type is PacketType.COMM_ERROR: + raise ValueError('Device-side serial communication error') + elif pkt_type is PacketType.CRYPTO_ERROR: + raise ValueError('Device-side cryptographic error') + elif pkt_type is PacketType.TOO_MANY_FAILS: + raise ValueError('Device reports too many failed handshake attempts') + else: + return pkt_type, data class KeyMapper: Keycode = enum.Enum('Keycode', start=0, names=''' @@ -323,8 +336,9 @@ if __name__ == '__main__': print(noise.channel_binding_incantation()) for user_input in noise.pairing_messages(): - print('\033[2K\r', end='') - print('Pairing input:', user_input, end='', flush=True) + if not args.debug: + print('\033[2K\r', end='') + print('Pairing input:', user_input, end='' if not args.debug else '\n', flush=True) print() print('Pairing success') diff --git a/src/cobs.c b/src/cobs.c index 84991f3..27bc24d 100644 --- a/src/cobs.c +++ b/src/cobs.c @@ -216,7 +216,7 @@ void cobs_decode_incremental_initialize(struct cobs_decode_state *state) { int cobs_decode_incremental(struct cobs_decode_state *state, char *dst, size_t dstlen, char src) { if (state->p == 0) { if (src == 0) - goto errout; /* invalid framing. An empty frame would be [...] 00 01 00, not [...] 00 00 */ + goto empty_errout; /* invalid framing. An empty frame would be [...] 00 01 00, not [...] 00 00 */ state->c = (unsigned char)src; state->p++; return 0; @@ -250,6 +250,10 @@ int cobs_decode_incremental(struct cobs_decode_state *state, char *dst, size_t d errout: cobs_decode_incremental_initialize(state); return -1; + +empty_errout: + cobs_decode_incremental_initialize(state); + return -3; } #ifdef VALIDATION diff --git a/src/demo.c b/src/demo.c index 6c519e1..49157e9 100644 --- a/src/demo.c +++ b/src/demo.c @@ -50,7 +50,7 @@ #endif #ifndef MAX_FAILED_HANDSHAKES -#define MAX_FAILED_HANDSHAKES 3 +#define MAX_FAILED_HANDSHAKES 5 #endif @@ -200,6 +200,7 @@ void pairing_input(uint8_t modbyte, uint8_t keycode) { } else { /* FIXME sound alarm */ + pairing_buf_pos = 0; /* Reset input buffer */ uint8_t response = REPORT_PAIRING_ERROR; if (send_encrypted_message(&noise_state, &response, sizeof(response))) LOG_PRINTF("Error sending pairing response packet\n"); @@ -327,6 +328,13 @@ struct dma_usart_file debug_out_s = { struct dma_usart_file *debug_out = &debug_out_s; void DMA_ISR(DEBUG_USART_DMA_NUM, DEBUG_USART_DMA_STREAM_NUM)(void) { + if (dma_get_interrupt_flag(debug_out->dma, debug_out->stream, DMA_FEIF)) { + /* Ignore FIFO errors as they're 100% non-critical for UART applications */ + dma_clear_interrupt_flags(debug_out->dma, debug_out->stream, DMA_FEIF); + return; + } + + /* Transfer complete */ dma_clear_interrupt_flags(debug_out->dma, debug_out->stream, DMA_TCIF); if (debug_out->buf->wr_pos != debug_out->buf->xfr_end) /* buffer not empty */ @@ -396,25 +404,45 @@ int main(void) pairing_buf_pos = 0; /* Reset channel binding keyboard input buffer */ } else { LOG_PRINTF("Too many failed handshake attempts, not starting another one\n"); + struct control_packet out = { .type=HOST_TOO_MANY_FAILS }; + send_packet(usart2_out, (uint8_t *)&out, sizeof(out)); } } else if (pkt->type == HOST_HANDSHAKE) { LOG_PRINTF("Handling handshake packet of length %d\n", payload_length); - int consumed = 0; - try_continue_noise_handshake(&noise_state, pkt->payload, payload_length, &consumed); - if (consumed) - host_packet_length = 0; /* Acknowledge to USART ISR the buffer has been handled */ - else /* Otherwise this gets called again in the next iteration of the main loop. Usually that should not happen. */ - LOG_PRINTF("Handshake buffer unhandled. Waiting for next iteration.\n"); + if (try_continue_noise_handshake(&noise_state, pkt->payload, payload_length)) { + LOG_PRINTF("Reporting handshake error to host\n"); + struct control_packet out = { .type=HOST_CRYPTO_ERROR }; + send_packet(usart2_out, (uint8_t *)&out, sizeof(out)); + } + host_packet_length = 0; /* Acknowledge to USART ISR the buffer has been handled */ } else { + LOG_PRINTF("Unhandled packet of type %d\n", pkt->type); host_packet_length = 0; /* Acknowledge to USART ISR the buffer has been handled */ } - } - if (noise_state.handshake_state == HANDSHAKE_IN_PROGRESS) - try_continue_noise_handshake(&noise_state, NULL, 0, NULL); /* handle outgoing messages */ + } else if (host_packet_length < 0) { /* USART error */ + host_packet_length = 0; /* Acknowledge to USART ISR the error has been handled */ + if (noise_state.handshake_state < HANDSHAKE_DONE_UNKNOWN_HOST) { + LOG_PRINTF("USART error, aborting handshake\n") + + struct control_packet pkt = { .type=HOST_COMM_ERROR }; + send_packet(usart2_out, (uint8_t *)&pkt, sizeof(pkt)); - delay_ms_busy_loop(1); /* approx 1ms interval between usbh_poll() */ + if (reset_protocol_handshake(&noise_state)) + LOG_PRINTF("Error starting protocol handshake.\n"); + + pairing_buf_pos = 0; /* Reset channel binding keyboard input buffer */ + } + } + + if (noise_state.handshake_state == HANDSHAKE_IN_PROGRESS) { + if (try_continue_noise_handshake(&noise_state, NULL, 0)) { /* handle outgoing messages */ + LOG_PRINTF("Reporting handshake error to host\n"); + struct control_packet pkt = { .type=HOST_CRYPTO_ERROR }; + send_packet(usart2_out, (uint8_t *)&pkt, sizeof(pkt)); + } + } } } diff --git a/src/noise.c b/src/noise.c index e855a4a..9f898dd 100644 --- a/src/noise.c +++ b/src/noise.c @@ -17,7 +17,7 @@ volatile uint8_t host_packet_buf[MAX_HOST_PACKET_SIZE]; -volatile uint8_t host_packet_length = 0; +volatile int host_packet_length = 0; void noise_state_init(struct NoiseState *st, uint8_t *remote_key_reference) { @@ -90,7 +90,7 @@ void uninit_handshake(struct NoiseState *st, enum handshake_state new_state) { st->handshake = NULL; } -enum handshake_state try_continue_noise_handshake(struct NoiseState *st, uint8_t *buf, size_t len, int *buf_consumed) { +int try_continue_noise_handshake(struct NoiseState *st, uint8_t *buf, size_t len) { int err; struct { struct control_packet header; @@ -111,12 +111,14 @@ enum handshake_state try_continue_noise_handshake(struct NoiseState *st, uint8_t noise_buffer_set_output(noise_msg, &pkt.payload, sizeof(pkt.payload)); HANDLE_NOISE_ERROR(noise_handshakestate_write_message(st->handshake, &noise_msg, NULL), "writing handshake message"); send_packet(usart2_out, (uint8_t *)&pkt, noise_msg.size + sizeof(pkt.header)); + if (buf) { + LOG_PRINTF("Warning: dropping unneeded host buffer of length %d bytes\n", len); + } break; case NOISE_ACTION_READ_MESSAGE: if (buf) { /* Read the next handshake message and discard the payload */ - *buf_consumed = 1; noise_buffer_set_input(noise_msg, buf, len); HANDLE_NOISE_ERROR(noise_handshakestate_read_message(st->handshake, &noise_msg, NULL), "reading handshake message"); } @@ -157,12 +159,12 @@ enum handshake_state try_continue_noise_handshake(struct NoiseState *st, uint8_t goto errout; } - return st->handshake_state; + return 0; errout: uninit_handshake(st, HANDSHAKE_UNINITIALIZED); st->failed_handshakes++; LOG_PRINTF("Noise protocol handshake failed, %d failed attempts\n", st->failed_handshakes); - return st->handshake_state; + return -1; } void persist_remote_key(struct NoiseState *st) { diff --git a/src/noise.h b/src/noise.h index c777a0b..a4c1e6e 100644 --- a/src/noise.h +++ b/src/noise.h @@ -14,7 +14,7 @@ extern volatile uint8_t host_packet_buf[MAX_HOST_PACKET_SIZE]; -extern volatile uint8_t host_packet_length; +extern volatile int host_packet_length; enum handshake_state { HANDSHAKE_UNINITIALIZED, @@ -44,7 +44,7 @@ void persist_remote_key(struct NoiseState *st); int start_protocol_handshake(struct NoiseState *st); int reset_protocol_handshake(struct NoiseState *st); int generate_identity_key(struct NoiseState *st); -enum handshake_state try_continue_noise_handshake(struct NoiseState *st, uint8_t *buf, size_t len, int *buf_consumed); +int try_continue_noise_handshake(struct NoiseState *st, uint8_t *buf, size_t len); int send_encrypted_message(struct NoiseState *st, uint8_t *msg, size_t len); #endif diff --git a/src/packet_interface.c b/src/packet_interface.c index 071cf02..9c0ea5c 100644 --- a/src/packet_interface.c +++ b/src/packet_interface.c @@ -10,7 +10,7 @@ volatile struct { struct dma_buf dma; - uint8_t data[128]; + uint8_t data[256]; } usart2_buf = { .dma = { .len = sizeof(usart2_buf.data) } }; struct dma_usart_file usart2_out_s = { @@ -25,7 +25,15 @@ struct dma_usart_file usart2_out_s = { struct dma_usart_file *usart2_out = &usart2_out_s; void dma1_stream6_isr(void) { - dma_clear_interrupt_flags(usart2_out->dma, usart2_out->stream, DMA_TCIF); + if (dma_get_interrupt_flag(usart2_out->dma, usart2_out->stream, DMA_FEIF)) { + /* Ignore FIFO errors as they're 100% non-critical for UART applications */ + dma_clear_interrupt_flags(usart2_out->dma, usart2_out->stream, DMA_FEIF); + LOG_PRINTF("USART2 DMA FIFO error\n"); + return; + } + + /* Transfer complete interrupt */ + dma_clear_interrupt_flags(usart2_out->dma, usart2_out->stream, DMA_TCIF); if (usart2_out->buf->wr_pos != usart2_out->buf->xfr_end) /* buffer not empty */ schedule_dma(usart2_out); @@ -37,6 +45,7 @@ void usart2_isr(void) { LOG_PRINTF("USART2 data register overrun\n"); /* Clear interrupt flag */ (void)USART2_DR; /* FIXME make sure this read is not optimized out */ + host_packet_length = -1; return; } @@ -44,14 +53,19 @@ void usart2_isr(void) { if (host_packet_length) { LOG_PRINTF("USART2 COBS buffer overrun\n"); + host_packet_length = -1; return; } ssize_t rv = cobs_decode_incremental(&host_cobs_state, (char *)host_packet_buf, sizeof(host_packet_buf), data); if (rv == -2) { LOG_PRINTF("Host interface COBS packet too large\n"); + host_packet_length = -1; + } else if (rv == -3) { + LOG_PRINTF("Got double null byte from host\n"); } else if (rv < 0) { LOG_PRINTF("Host interface COBS framing error\n"); + host_packet_length = -1; } else if (rv > 0) { host_packet_length = rv; } /* else just return and wait for next byte */ diff --git a/src/packet_interface.h b/src/packet_interface.h index 638405a..0502325 100644 --- a/src/packet_interface.h +++ b/src/packet_interface.h @@ -11,6 +11,9 @@ enum control_packet_types { HOST_INITIATE_HANDSHAKE = 1, HOST_HANDSHAKE = 2, HOST_DATA = 3, + HOST_COMM_ERROR = 4, + HOST_CRYPTO_ERROR = 5, + HOST_TOO_MANY_FAILS = 6, }; enum packet_types { diff --git a/src/usart_helpers.c b/src/usart_helpers.c index a1f7b4e..0cfc2d5 100644 --- a/src/usart_helpers.c +++ b/src/usart_helpers.c @@ -71,6 +71,7 @@ void usart_dma_init(struct dma_usart_file *f) { dma_set_memory_size(f->dma, f->stream, DMA_SxCR_MSIZE_8BIT); dma_set_priority(f->dma, f->stream, DMA_SxCR_PL_VERY_HIGH); dma_enable_transfer_complete_interrupt(f->dma, f->stream); + dma_enable_fifo_error_interrupt(f->dma, f->stream); usart_enable_tx_dma(f->usart); } @@ -111,6 +112,7 @@ int putf(void *file, char c) { /* push char to fifo, busy-loop if stalled to wait for USART to empty fifo via DMA */ while (dma_fifo_push(f->buf, c) == -EBUSY) { nvic_enable_irq(f->irqn); + flush(f); nvic_disable_irq(f->irqn); } nvic_enable_irq(f->irqn); @@ -137,8 +139,9 @@ void flush(void *file) { nvic_disable_irq(f->irqn); /* If the DMA stream is idle right now, schedule a transfer */ - if (!(DMA_SCR(f->dma, f->stream) & DMA_SxCR_EN) /* DMA is not running */ - && !dma_get_interrupt_flag(f->dma, f->stream, DMA_TCIF)/* DMA interrupt is clear */) { + if (!(DMA_SCR(f->dma, f->stream) & DMA_SxCR_EN)) { /* DMA is not running */ + //&& !dma_get_interrupt_flag(f->dma, f->stream, DMA_TCIF)/* DMA interrupt is clear */) { + dma_clear_interrupt_flags(f->dma, f->stream, DMA_TCIF); schedule_dma(f); } nvic_enable_irq(f->irqn); -- cgit