New file

Rev: doc/TODO:1.1
Rev: src/abstract_io.h:1.13
Rev: src/client.c:1.20
Rev: src/client_keyexchange.c:1.11
Rev: src/connection.c:1.12
Rev: src/debug.c:1.12
Rev: src/disconnect.c:1.4
Rev: src/keyexchange.c:1.16
Rev: src/packet_ignore.c:1.4
Rev: src/read_data.c:1.7
Rev: src/read_packet.c:1.19
Rev: src/server.c:1.13
Rev: src/server_keyexchange.c:1.9

doc/TODO

+Consider return values from handlers. A return value has seveal
+components:
+
+ x Success/failure indication
+ 
+ x Next action (continue, close immediately, flush buffers then close,
+   exit...)
+ 
+ x Process return value in case the proper action is to exit.
+
+It should be possible to encode all this information into a simple integer.

src/abstract_io.h

@@ -47,6 +47,29 @@ struct abstract_read
 
 #define A_READ(f, length, buffer) (f)->read(&(f), (length), (buffer))
 
+/* Return values for write- and readhandlers.
+ *
+ * Several components are OR:ed together */
+
+/* Success/fail indication */
+#define LSH_OK 0
+#define LSH_FAIL 1
+
+/* Actions */
+#define LSH_GOON 0
+#define LSH_CLOSE 2
+#define LSH_DIE  4
+#define LSH_GET_ACTION(x) ((x) & 6)
+
+/* Non-zero if there's any problem */
+#define LSH_PROBLEMP(x) (x)
+
+/* Are return codes really needed here? */
+#if 0
+#define LSH_EXIT(x) ((x) << 3)
+#define LSH_GET_EXIT(x) ((x) >> 3)
+#endif
+
 /* May store a new handler into *h. */
 struct read_handler
 {
@@ -57,21 +80,9 @@ struct read_handler
 
 #define READ_HANDLER(h, read) ((h)->handler(&(h), (read)))
 
-/* Return values for write callbacks
- *
- * FIXME: Perhaps some more values are needed? What if we want to
- * close a file, but not until all data has bee flushed? Perhaps it is
- * best not to put too much meaning into the return value, and use it
- * as a succes/fail indication only. */
-
-/* Everything is ok */
-#define WRITE_OK 1
-
-/* Write failed, and the packet could not be processed or delivered.
- * Most likely because of a protocol error */
-#define WRITE_CLOSED 0
 
-/* May store a new handler into *w. */
+/* May store a new handler into *w. FIXME: Is this indirection ever
+ * used? */
 struct abstract_write
 {
   struct lsh_object header;
@@ -81,7 +92,7 @@ struct abstract_write
 
 #define A_WRITE(f, packet) ((f)->write(&(f), (packet)))
 
-/* A processor that passes its result on to another processor */
+/* A handler that passes packets on to another processor */
 struct abstract_write_pipe
 {
   struct abstract_write super;

src/client.c

@@ -80,7 +80,7 @@ static int client_initiate(struct fd_callback **c,
   
   res = A_WRITE(connection->raw,
 		ssh_format("%lS\r\n", connection->client_version));
-  if (res != WRITE_OK)
+  if (LSH_PROBLEMP(res))
     return res;
 
   return initiate_keyexchange(connection, CONNECTION_CLIENT,

src/client_keyexchange.c

@@ -63,7 +63,7 @@ static int do_handle_dh_reply(struct packet_handler *c,
   if (!dh_process_server_msg(&closure->dh, packet))
     {
       disconnect_kex_failed(connection, "Bad dh-reply\r\n");
-      return WRITE_CLOSED;
+      return LSH_FAILURE | LSH_CLOSE;
     }
     
   v = LOOKUP_VERIFIER(closure->verifier, closure->dh.server_key);
@@ -72,7 +72,7 @@ static int do_handle_dh_reply(struct packet_handler *c,
     /* FIXME: Use a more appropriate error code? */
     {
       disconnect_kex_failed(connection, "Bad server host key\r\n");
-      return WRITE_CLOSED;
+      return LSH_FAILURE | LSH_CLOSE;
     }
   
   if (!dh_verify_server_msg(&closure->dh, v))
@@ -83,7 +83,7 @@ static int do_handle_dh_reply(struct packet_handler *c,
    * handler for recieving the newkeys message. */
 
   res = A_WRITE(connection->write, ssh_format("%c", SSH_MSG_NEWKEYS));
-  if (res != WRITE_OK)
+  if (LSH_PROBLEMP(res))
     return res;
 
   /* Record session id */
@@ -140,7 +140,7 @@ static int do_init_dh(struct keyexchange_algorithm *c,
   /* Send client's message */
   res = A_WRITE(connection->write, dh_make_client_msg(&dh->dh));
 
-  if (res != WRITE_OK)
+  if (LSH_PROBLEMP(res))
     return res;
   
   /* Install handler */
@@ -148,7 +148,7 @@ static int do_init_dh(struct keyexchange_algorithm *c,
   
   connection->kex_state = KEX_STATE_IN_PROGRESS;
   
-  return WRITE_OK;
+  return LSH_OK | LSH_GOON;
 }
 
 

src/connection.c

@@ -38,7 +38,7 @@ static int handle_connection(struct abstract_write **w,
 	{
 	  werror("Unexpected NEWKEYS message!\n");
 	  lsh_free(packet);
-	  return WRITE_CLOSED;
+	  return LSH_FAIL | LSH_DIE;
 	}
       break;
     case KEX_STATE_IGNORE:
@@ -50,7 +50,7 @@ static int handle_connection(struct abstract_write **w,
        * KEXDH_INIT or KEXDH_REPLY message. */
       closure->kex_state = KEX_STATE_IN_PROGRESS;
       lsh_free(packet);
-      return WRITE_OK;
+      return LSH_OK | LSH_GOON;
 
     case KEX_STATE_IN_PROGRESS:
       if ( (msg == SSH_MSG_NEWKEYS)
@@ -58,7 +58,7 @@ static int handle_connection(struct abstract_write **w,
 	{
 	  werror("Unexpected KEXINIT or NEWKEYS message!\n");
 	  lsh_free(packet);
-	  return WRITE_CLOSED;
+	  return LSH_FAIL | LSH_DIE;
 	}
       break;
     case KEX_STATE_NEWKEYS:
@@ -68,7 +68,7 @@ static int handle_connection(struct abstract_write **w,
 	  werror("Expected NEWKEYS message, but recieved message %d!\n",
 		 msg);
 	  lsh_free(packet);
-	  return WRITE_CLOSED;
+	  return LSH_FAIL | LSH_DIE;
 	}
       break;
     default:
@@ -85,7 +85,7 @@ static int do_fail(struct packet_handler *closure,
   MDEBUG(closure);
 
   lsh_string_free(packet);
-  return WRITE_CLOSED;
+  return LSH_FAIL | LSH_DIE;
 }
 
 struct packet_handler *make_fail_handler(void)

src/debug.c

@@ -80,7 +80,7 @@ int send_debug(struct abstract_write *write, char *msg, int always_display)
 {
   return (debug_flag)
     ? A_WRITE(write, make_debug_packet(msg, always_display))
-    : WRITE_OK;
+    : LSH_OK | LSH_GOON;
   
 }
 
@@ -88,7 +88,7 @@ int send_verbose(struct abstract_write *write, char *msg, int always_display)
 {
   return (verbose_flag)
     ? A_WRITE(write, make_debug_packet(msg, always_display))
-    : WRITE_OK;
+    : LSH_OK | LSH_GOON;
 }
 
 static int do_rec_debug(struct packet_handler *self,
@@ -111,7 +111,7 @@ static int do_rec_debug(struct packet_handler *self,
 	&& parse_eod(&buffer)))
     {
       lsh_string_free(packet);
-      return WRITE_CLOSED;
+      return LSH_FAIL | LSH_DIE;
     }
 
   if (always_display)
@@ -128,7 +128,7 @@ static int do_rec_debug(struct packet_handler *self,
     }
 
   lsh_string_free(packet);
-  return WRITE_OK;
+  return LSH_OK | LSH_GOON;
 }
 
 struct packet_handler *make_rec_debug_handler(void)

src/disconnect.c

@@ -67,7 +67,7 @@ static int do_disconnect(struct packet_handler *closure,
   /* FIXME: Mark the file as closed, somehow (probably a variable in
    * the write buffer) */
 
-  return WRITE_CLOSED;
+  return LSH_CLOSE;
 }
 
 struct packet_handler *make_disconnect_handler(void)

src/keyexchange.c

@@ -148,7 +148,7 @@ int initiate_keyexchange(struct ssh_connection *connection,
 
   res = A_WRITE(connection->write, lsh_string_dup(s));
   
-  if ( (res == WRITE_OK) && first_packet)
+  if (!LSH_PROBLEMP(res) && first_packet)
     return A_WRITE(connection->write, first_packet);
   else
     return res;
@@ -217,7 +217,7 @@ static int do_handle_kexinit(struct packet_handler *c,
       connection->literal_kexinits[closure->type] = lsh_string_dup(packet); 
       
       res = A_WRITE(connection->write, packet);
-      if (res != WRITE_OK)
+      if (LSH_PROBLEMP(res))
 	return res;
     }
 
@@ -248,9 +248,7 @@ static int do_handle_kexinit(struct packet_handler *c,
 	  disconnect_kex_failed(connection,
 				"No common key exchange method.\r\n");
 	  
-	  /* FIXME: We want the disconnect message to be sent
-	   * before the socket is closed. How? */
-	  return WRITE_CLOSED;
+	  return LSH_FAIL | LSH_CLOSE;
 	}
     }
   hostkey_algorithm
@@ -266,7 +264,7 @@ static int do_handle_kexinit(struct packet_handler *c,
       if (!parameters[i])
 	{
 	  disconnect_kex_failed(connection, "");
-	  return WRITE_CLOSED;
+	  return LSH_FAIL | LSH_CLOSE;
 	}
     }
   
@@ -433,10 +431,10 @@ static int do_handle_newkeys(struct packet_handler *c,
       connection->dispatch[SSH_MSG_NEWKEYS] = NULL;
 
       lsh_free(closure);
-      return WRITE_OK;
+      return LSH_OK | LSH_GOON;
     }
   else
-    return WRITE_CLOSED;
+    return LSH_FAIL | LSH_DIE;
 }
 
 struct packet_handler *

src/packet_ignore.c

@@ -32,7 +32,7 @@ static int do_ignore(struct packet_handler *closure,
 		     struct lsh_string *packet)
 {
   lsh_string_free(packet);
-  return WRITE_OK;
+  return LSH_OK | LSH_GOON;
 }
 
 struct packet_handler *make_ignore_handler(void)

src/read_data.c

@@ -60,14 +60,14 @@ static int do_read_data(struct read_handler **h,
 	  /* Fall through */
 	case A_EOF:
 	  CALLBACK(closure->close_callback);
-	  return WRITE_CLOSED;
+	  return LSH_OK | LSH_CLOSE;
 	default:
 	  {
 	    int res;
 	    packet->length = n;
 	    /* FIXME: Use returned value */
 	    res = A_WRITE(closure->handler, packet);
-	    if (res != WRITE_OK)
+	    if (LSH_PROBLEMP(res))
 	      return res;
 	    break;
 	  }

src/read_packet.c

@@ -77,7 +77,7 @@ lsh_string_realloc(struct lsh_string *s, UINT32 length)
   else
     return s;
 }
-    
+
 static int do_read_packet(struct read_handler **h,
 			  struct abstract_read *read)
 {
@@ -85,214 +85,202 @@ static int do_read_packet(struct read_handler **h,
 
   MDEBUG(closure);
   
-#if 0
-  while(1)
+  switch(closure->state)
     {
-#endif
-      switch(closure->state)
-	{
-	case WAIT_START:
+    case WAIT_START:
+      {
+	UINT32 block_size = closure->connection->rec_crypto
+	  ? closure->connection->rec_crypto->block_size : 8;
+
+	closure->buffer = lsh_string_realloc(closure->buffer,
+					     block_size);
+	closure->pos = 0;
+
+	closure->state = WAIT_HEADER;
+	/* FALL THROUGH */
+      }
+    case WAIT_HEADER:
+      {
+	UINT32 block_size = closure->connection->rec_crypto
+	  ? closure->connection->rec_crypto->block_size : 8;
+	UINT32 left;
+	int n;
+
+	left = block_size - closure->pos;
+	    
+	n = A_READ(read, left, closure->buffer->data + closure->pos);
+	switch(n)
 	  {
-	    UINT32 block_size = closure->connection->rec_crypto
-	      ? closure->connection->rec_crypto->block_size : 8;
-
-	    closure->buffer = lsh_string_realloc(closure->buffer,
-						 block_size);
-	    closure->pos = 0;
-
-	    closure->state = WAIT_HEADER;
-	    /* FALL THROUGH */
+	  case 0:
+	    return LSH_OK | LSH_GOON;
+	  case A_FAIL:
+	    return LSH_FAIL | LSH_DIE;
+	  case A_EOF:
+	    /* FIXME: Free associated resources! */
+	    return LSH_OK | LSH_CLOSE;
 	  }
-	case WAIT_HEADER:
+	closure->pos += n;
+
+	/* Read a complete block? */
+	if (n == left)
 	  {
-	    UINT32 block_size = closure->connection->rec_crypto
-	      ? closure->connection->rec_crypto->block_size : 8;
-	    UINT32 left;
-	    int n;
+	    UINT32 length;
 
-	    left = block_size - closure->pos;
-	    
-	    n = A_READ(read, left, closure->buffer->data + closure->pos);
-	    switch(n)
+	    if (closure->connection->rec_crypto)
+	      CRYPT(closure->connection->rec_crypto,
+		    block_size,
+		    closure->buffer->data,
+		    closure->buffer->data);
+		
+	    length = READ_UINT32(closure->buffer->data);
+	    if (length > closure->connection->rec_max_packet)
 	      {
-	      case 0:
-		return 1;
-	      case A_FAIL:
-		/* Fall through */
-	      case A_EOF:
-		/* FIXME: Free associated resources! */
-		return 0;
+		werror("read_packet: Recieving too large packet.\n"
+		       "  %d octets, limit is %d\n",
+		       length, closure->connection->rec_max_packet);
+		return LSH_FAIL | LSH_DIE;
 	      }
-	    closure->pos += n;
 
-	    /* Read a complete block? */
-	    if (n == left)
+	    if ( (length < 12)
+		 || (length < (block_size - 4))
+		 || ( (length + 4) % block_size))
 	      {
-		UINT32 length;
+		werror("read_packet: Bad packet length %d\n",
+		       length);
+		return LSH_FAIL | LSH_DIE;
+	      }
 
-		if (closure->connection->rec_crypto)
-		  CRYPT(closure->connection->rec_crypto,
-			block_size,
-			closure->buffer->data,
-			closure->buffer->data);
-		
-		length = READ_UINT32(closure->buffer->data);
-		if (length > closure->connection->rec_max_packet)
-		  {
-		    werror("read_packet: Recieving too large packet.\n"
-			   "  %d octets, limit is %d\n",
-			   length, closure->connection->rec_max_packet);
-		    return 0;
-		  }
-
-		if ( (length < 12)
-		     || (length < (block_size - 4))
-		     || ( (length + 4) % block_size))
-		  {
-		    werror("read_packet: Bad packet length %d\n",
-			   length);
-		    return 0;
-		  }
-
-		/* Process this block before the length field is lost. */
-		if (closure->connection->rec_mac)
-		  {
-		    UINT8 s[4];
-		    WRITE_UINT32(s, closure->sequence_number);
+	    /* Process this block before the length field is lost. */
+	    if (closure->connection->rec_mac)
+	      {
+		UINT8 s[4];
+		WRITE_UINT32(s, closure->sequence_number);
 		    
-		    HASH_UPDATE(closure->connection->rec_mac, 4, s);
-		    HASH_UPDATE(closure->connection->rec_mac,
-				closure->buffer->length,
-				closure->buffer->data);
-		  }
-
-		/* Allocate full packet */
-		{
-		  int done = block_size - 4;
-		  
-		  closure->buffer
-		    = ssh_format("%ls%lr",
-				 done,
-				 closure->buffer->data + 4,
-				 length - done,
-				 &closure->crypt_pos);
-
-		  /* FIXME: Is this needed anywhere? */
-		  closure->buffer->sequence_number
-		    = closure->sequence_number++;
-
-		  closure->pos = done;
-		  closure->state = WAIT_CONTENTS;
-		}
-		/* Fall through */
+		HASH_UPDATE(closure->connection->rec_mac, 4, s);
+		HASH_UPDATE(closure->connection->rec_mac,
+			    closure->buffer->length,
+			    closure->buffer->data);
 	      }
-	    else
-	      /* Try reading some more */
-	      break;
+
+	    /* Allocate full packet */
+	    {
+	      int done = block_size - 4;
+		  
+	      closure->buffer
+		= ssh_format("%ls%lr",
+			     done,
+			     closure->buffer->data + 4,
+			     length - done,
+			     &closure->crypt_pos);
+
+	      /* FIXME: Is this needed anywhere? */
+	      closure->buffer->sequence_number
+		= closure->sequence_number++;
+
+	      closure->pos = done;
+	      closure->state = WAIT_CONTENTS;
+	    }
+	    /* Fall through */
 	  }
-	case WAIT_CONTENTS:
-	  {
-	    UINT32 left = closure->buffer->length - closure->pos;
-	    int n = A_READ(read, left, closure->buffer->data + closure->pos);
+	else
+	  /* Try reading some more */
+	  break;
+      }
+    case WAIT_CONTENTS:
+      {
+	UINT32 left = closure->buffer->length - closure->pos;
+	int n = A_READ(read, left, closure->buffer->data + closure->pos);
 
-	    switch(n)
-	      {
-	      case 0:
-		return 1;
-	      case A_FAIL:
-		werror("do_read_packet: read() failed, %s\n", strerror(errno));
-		/* Fall through */
-	      case A_EOF:
-		/* FIXME: Free associated resources! */
-		return 0;
-	      }
-	    closure->pos += n;
+	switch(n)
+	  {
+	  case 0:
+	    RETURN LSH_OK | LSH_GOON;
+	  case A_FAIL:
+	    /* Fall through */
+	  case A_EOF:
+	    /* FIXME: Free associated resources! */
+	    return LSH_FAIL | LSH_DIE;
+	  }
+	closure->pos += n;
 
-	    /* Read a complete packet? */
-	    if (n == left)
+	/* Read a complete packet? */
+	if (n == left)
+	  {
+	    assert(left == ((closure->buffer->length
+			     + closure->buffer->data)
+			    - closure->crypt_pos));
+	    if (closure->connection->rec_crypto)
+	      CRYPT(closure->connection->rec_crypto,
+		    left,
+		    closure->crypt_pos,
+		    closure->crypt_pos);		      
+	    if (closure->connection->rec_mac)
 	      {
-		assert(left == ((closure->buffer->length
-				 + closure->buffer->data)
-				- closure->crypt_pos));
-		if (closure->connection->rec_crypto)
-		  CRYPT(closure->connection->rec_crypto,
-			left,
-			closure->crypt_pos,
-			closure->crypt_pos);		      
-		if (closure->connection->rec_mac)
-		  {
-		    closure->computed_mac = lsh_string_realloc
-		      (closure->computed_mac,
-		       closure->connection->rec_mac->hash_size);
-
-		    HASH_UPDATE(closure->connection->rec_mac,
-				left,
-				closure->crypt_pos);
-		    HASH_DIGEST(closure->connection->rec_mac,
-				closure->computed_mac->data);
-		  }
-		closure->state = WAIT_MAC;
-		closure->pos = 0;
-		/* Fall through */
+		closure->computed_mac = lsh_string_realloc
+		  (closure->computed_mac,
+		   closure->connection->rec_mac->hash_size);
+
+		HASH_UPDATE(closure->connection->rec_mac,
+			    left,
+			    closure->crypt_pos);
+		HASH_DIGEST(closure->connection->rec_mac,
+			    closure->computed_mac->data);
 	      }
-	    else
-	      /* Try reading some more */
-	      break;
+	    closure->state = WAIT_MAC;
+	    closure->pos = 0;
+	    /* Fall through */
 	  }
-	case WAIT_MAC:
-	  if (closure->connection->rec_mac)
-	    {
-	      UINT32 left = (closure->connection->rec_mac->mac_size
-			     - closure->pos);
-	      UINT8 *mac = alloca(left);
-
-	      int n = A_READ(read, left, mac);
-
-	      switch(n)
-		{
-		case 0:
-		  return 1;
-		case A_FAIL:
-		  werror("do_read_packet: read() failed, %s\n",
-			 strerror(errno));
-		  /* Fall through */
-		case A_EOF:
-		  /* FIXME: Free associated resources! */
-		  return 0;
-	      }
-
-	      /* FIXME: Don't fail until the entire MAC has been read.
-	       * Otherwise we will leak information about partially
-	       * correct MAC:s. */
-	      if (!memcmp(mac,
-			  closure->computed_mac + closure->pos,
-			  n))
-		/* FIXME: Free resources */
-		return 0;
+	else
+	  /* Try reading some more */
+	  break;
+      }
+    case WAIT_MAC:
+      if (closure->connection->rec_mac)
+	{
+	  UINT32 left = (closure->connection->rec_mac->mac_size
+			 - closure->pos);
+	  UINT8 *mac = alloca(left);
 
-	      closure->pos += n;
+	  int n = A_READ(read, left, mac);
 
-	      if (n < left)
-		/* Try reading more */
-		break;