From 9905d748d90058db72d84d9127be5443fb4c2382 Mon Sep 17 00:00:00 2001
From: Jan Dittberner <jandd@cacert.org>
Date: Tue, 29 Nov 2022 10:29:09 +0100
Subject: [PATCH] Improve signer robustness

- let client simulator send some garbage bytes before starting real commands
- handle EOF during reads
---
 cmd/clientsim/main.go    | 100 ++++++++++++++++++++++++++-------------
 pkg/protocol/protocol.go |   4 ++
 2 files changed, 72 insertions(+), 32 deletions(-)

diff --git a/cmd/clientsim/main.go b/cmd/clientsim/main.go
index e82e17b..1bab83c 100644
--- a/cmd/clientsim/main.go
+++ b/cmd/clientsim/main.go
@@ -21,7 +21,9 @@ package main
 
 import (
 	"context"
+	"crypto/rand"
 	"fmt"
+	"io"
 	"os"
 	"sync"
 	"time"
@@ -155,16 +157,27 @@ func (g *TestCommandGenerator) HandleResponse(frame []byte) error {
 }
 
 func (g *TestCommandGenerator) GenerateCommands(ctx context.Context) error {
-	const (
-		healthInterval = 5 * time.Second
-		startPause     = 3 * time.Second
-	)
-
 	var (
 		announce *messages.CommandAnnounce
 		err      error
 	)
 
+	// write some leading garbage to test signer robustness
+	_, _ = io.CopyN(os.Stdout, rand.Reader, 50) //nolint:gomnd
+
+	announce, err = messages.BuildCommandAnnounce(messages.CmdHealth)
+	if err != nil {
+		return fmt.Errorf("build command announce failed: %w", err)
+	}
+
+	g.commands <- &protocol.Command{Announce: announce, Command: &messages.HealthCommand{}}
+
+	const (
+		healthInterval = 5 * time.Second
+		crlInterval    = 15 * time.Minute
+		startPause     = 3 * time.Second
+	)
+
 	g.logger.Info("start generating commands")
 
 	time.Sleep(startPause)
@@ -179,17 +192,18 @@ func (g *TestCommandGenerator) GenerateCommands(ctx context.Context) error {
 		Command:  &messages.FetchCRLCommand{IssuerID: "sub-ecc_person_2022"},
 	}
 
-	timer := time.NewTimer(healthInterval)
+	healthTimer := time.NewTimer(healthInterval)
+	crlTimer := time.NewTimer(crlInterval)
 
 	for {
 		select {
 		case <-ctx.Done():
-			_ = timer.Stop()
+			_ = healthTimer.Stop()
 
 			g.logger.Info("stopping health check loop")
 
 			return nil
-		case <-timer.C:
+		case <-healthTimer.C:
 			announce, err = messages.BuildCommandAnnounce(messages.CmdHealth)
 			if err != nil {
 				return fmt.Errorf("build command announce failed: %w", err)
@@ -199,9 +213,14 @@ func (g *TestCommandGenerator) GenerateCommands(ctx context.Context) error {
 				Announce: announce,
 				Command:  &messages.HealthCommand{},
 			}
-		}
 
-		timer.Reset(healthInterval)
+			healthTimer.Reset(healthInterval)
+		case <-crlTimer.C:
+			g.commands <- &protocol.Command{
+				Announce: announce,
+				Command:  &messages.FetchCRLCommand{IssuerID: "sub-ecc_person_2022"},
+			}
+		}
 	}
 }
 
@@ -249,44 +268,61 @@ func (c *clientSimulator) writeCommand() error {
 	return nil
 }
 
+const responseAnnounceTimeout = 30 * time.Second
+const responseDataTimeout = 2 * time.Second
+
 func (c *clientSimulator) handleResponseAnnounce() error {
 	c.logger.Trace("waiting for response announce")
 
-	frame := <-c.framesIn
+	select {
+	case frame := <-c.framesIn:
+		if frame == nil {
+			return nil
+		}
+
+		if err := c.commandGenerator.HandleResponseAnnounce(frame); err != nil {
+			return fmt.Errorf("response announce handling failed: %w", err)
+		}
+
+		if err := c.nextState(); err != nil {
+			return err
+		}
+	case <-time.After(responseAnnounceTimeout):
+		c.logger.Warn("response announce timeout expired")
+
+		c.protocolState = cmdAnnounce
 
-	if frame == nil {
 		return nil
 	}
 
-	if err := c.commandGenerator.HandleResponseAnnounce(frame); err != nil {
-		return fmt.Errorf("response announce handling failed: %w", err)
-	}
-
-	if err := c.nextState(); err != nil {
-		return err
-	}
-
 	return nil
 }
 
 func (c *clientSimulator) handleResponseData() error {
 	c.logger.Trace("waiting for response data")
 
-	frame := <-c.framesIn
+	select {
+	case frame := <-c.framesIn:
+		if frame == nil {
+			return nil
+		}
+
+		if err := c.commandGenerator.HandleResponse(frame); err != nil {
+			return fmt.Errorf("response handler failed: %w", err)
+		}
+
+		if err := c.nextState(); err != nil {
+			return err
+		}
+
+		return nil
+	case <-time.After(responseDataTimeout):
+		c.logger.Warn("response data timeout expired")
+
+		c.protocolState = cmdAnnounce
 
-	if frame == nil {
 		return nil
 	}
-
-	if err := c.commandGenerator.HandleResponse(frame); err != nil {
-		return fmt.Errorf("response handler failed: %w", err)
-	}
-
-	if err := c.nextState(); err != nil {
-		return err
-	}
-
-	return nil
 }
 
 func (c *clientSimulator) Run(ctx context.Context) error {
diff --git a/pkg/protocol/protocol.go b/pkg/protocol/protocol.go
index 2e1c765..58dbda6 100644
--- a/pkg/protocol/protocol.go
+++ b/pkg/protocol/protocol.go
@@ -146,6 +146,10 @@ func (c *COBSFramer) readRaw(reader io.Reader) ([]byte, error) {
 
 	count, err := reader.Read(buf)
 	if err != nil {
+		if errors.Is(err, io.EOF) {
+			return []byte{}, nil
+		}
+
 		return nil, fmt.Errorf("could not read data: %w", err)
 	}