Host handshake mostly working

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);