work on encoding / decoding gg_packet

* we now do the byte order convertion in libglouglou when *_send(pkt) is called
* fix encoding and decoding: byte order, packet len
* simpler packet representation, one layer of structs only
* setup a nicer solution to get packets size depending on their type
* when in verbose mode, show which check failed in pkt_decode()

WARNING: This breaks API ! All libglouglou users need to update their gg_packet
usage

2 files changed, 145 insertions, 102 deletions

diff --git a/libglouglou/libglouglou.c b/libglouglou/libglouglou.c
index 6eacc0c..16069dc 100644
--- a/libglouglou/libglouglou.c
+++ b/libglouglou/libglouglou.c
@@ -35,7 +35,7 @@ int               client_send(struct gg_client *, void *, int);
 void              cb_cli_receive(evutil_socket_t, short, void *);
 void              cb_cli_timer(evutil_socket_t, short, void *);
 struct gg_packet *pkt_decode(char **, int *);
-int               pkt_getsize(struct gg_packet *);
+struct gg_packet *pkt_encode(struct gg_packet *, int *);
 
 int _verbosity = 0;
 
@@ -101,10 +101,13 @@ err:
 int
 gg_server_send(struct gg_server *srv, struct gg_packet *pkt, struct gg_user *usr)
 {
+  static struct gg_packet *newpkt;
   int size;
 
-  size = pkt_getsize(pkt);
-  return server_send(srv, usr, pkt, size);
+  newpkt = pkt_encode(pkt, &size);
+  if (!newpkt)
+    return -1;
+  return server_send(srv, usr, newpkt, size);
 }
 
 void
@@ -298,10 +301,13 @@ err:
 int
 gg_client_send(struct gg_client *cli, struct gg_packet *pkt)
 {
+  static struct gg_packet *newpkt;
   int size;
 
-  size = pkt_getsize(pkt);
-  return client_send(cli, pkt, size);
+  newpkt = pkt_encode(pkt, &size);
+  if (!newpkt)
+    return -1;
+  return client_send(cli, newpkt, size);
 }
 
 void
@@ -420,23 +426,29 @@ pkt_decode(char **buf, int *buf_len)
 
   len = *buf_len;
 
+#define invalid(msg) \
+  { verbose(msg); goto invalid; }
+#define incomplete(msg) \
+  { verbose(msg); goto incomplete; }
+
   if (len < PACKET_HEADER_SIZE)
-    goto incomplete;
+    invalid("len");
   if (len > PACKET_BUFFER_SIZE)
-    goto invalid;
+    invalid("len");
   pkt = (struct gg_packet *)*buf;
   if (pkt->ver != PACKET_VERSION)
-    goto invalid;
+    invalid("ver");
   if (pkt->type < PACKET_TYPE_MIN || pkt->type > PACKET_TYPE_MAX)
-    goto invalid;
+    invalid("type");
+  packet_len = gg_packet_props[pkt->type].size; // XXX never overflow ?
+  debug("type %d: %d %d", pkt->type, len, packet_len);
+  if (len < packet_len)
+    invalid("type len");
 
   newpkt.ver = pkt->ver;
   newpkt.type = pkt->type;
   switch(pkt->type) {
     case PACKET_NEWCONN:
-      packet_len = PACKET_NEWCONN_SIZE;
-      if (len < packet_len)
-        goto invalid;
       newpkt.newconn_id = pkt->newconn_id;
       newpkt.newconn_src = ntohl(pkt->newconn_src);
       newpkt.newconn_dst = ntohl(pkt->newconn_dst);
@@ -444,55 +456,43 @@ pkt_decode(char **buf, int *buf_len)
       newpkt.newconn_size = ntohs(pkt->newconn_size);
       break;
     case PACKET_DELCONN:
-      packet_len = PACKET_NEWCONN_SIZE;
-      if (len < packet_len)
-        goto invalid;
       newpkt.delconn_id = pkt->delconn_id;
       break;
     case PACKET_DATA:
-      packet_len = PACKET_NEWCONN_SIZE;
-      if (len < packet_len)
-        goto invalid;
       newpkt.data_connid = pkt->data_connid;
       newpkt.data_size = ntohs(pkt->data_size);
       break;
     case PACKET_NAME:
-      packet_len = PACKET_NEWCONN_SIZE;
-      if (len < packet_len)
-        goto invalid;
-      if (len < packet_len + pkt->name_len)
-        goto invalid;
-      if (strnlen((char *)pkt->name_fqdn, len) != pkt->name_len)
-        goto invalid;
       newpkt.name_addr = ntohl(pkt->name_addr);
       newpkt.name_len = ntohs(pkt->name_len);
+      if (newpkt.name_len > GG_PKTARG_MAX)
+        invalid("type name name_len");
+      if (len < packet_len + newpkt.name_len)
+        goto incomplete;
+      packet_len = packet_len + newpkt.name_len;
       strncpy((char *)newpkt.name_fqdn, (char *)pkt->name_fqdn,
-              pkt->name_len);
+              newpkt.name_len);
       break;
-    case PACKET_PROC_FORK:
-      packet_len = PACKET_PROC_FORK_SIZE;
-      if (len < packet_len)
-        goto invalid;
-      newpkt.proc_pid = ntohl(pkt->proc_pid);
-      newpkt.proc_fork_ppid = ntohl(pkt->proc_fork_ppid);
-      newpkt.proc_fork_cpid = ntohl(pkt->proc_fork_cpid);
-      newpkt.proc_fork_tgid = ntohl(pkt->proc_fork_tgid);
+    case PACKET_FORK:
+      newpkt.fork_pid = ntohl(pkt->fork_pid);
+      newpkt.fork_ppid = ntohl(pkt->fork_ppid);
+      newpkt.fork_cpid = ntohl(pkt->fork_cpid);
+      newpkt.fork_tgid = ntohl(pkt->fork_tgid);
       break;
-    case PACKET_PROC_EXEC:
-      packet_len = PACKET_PROC_EXEC_SIZE;
-      if (len < packet_len)
-        goto invalid;
-      if (len < packet_len + pkt->proc_exec_cmdlen)
-        goto invalid;
-      if (strnlen((char *)pkt->proc_exec_cmd, len) != pkt->proc_exec_cmdlen)
-        goto invalid;
-      newpkt.proc_pid = ntohl(pkt->proc_pid);
-      newpkt.proc_exec_cmdlen = ntohs(pkt->proc_exec_cmdlen);
-      strncpy((char *)newpkt.proc_exec_cmd, (char *)pkt->proc_exec_cmd,
-              pkt->proc_exec_cmdlen);
+    case PACKET_EXEC:
+      newpkt.exec_pid = ntohl(pkt->exec_pid);
+      newpkt.exec_cmdlen = pkt->exec_cmdlen;
+      if (newpkt.exec_cmdlen > GG_PKTARG_MAX)
+        invalid("type exec cmdlen");
+      if (len < packet_len + newpkt.exec_cmdlen)
+        goto incomplete;
+      packet_len = packet_len + newpkt.exec_cmdlen;
+      strncpy((char *)newpkt.exec_cmd, (char *)pkt->exec_cmd,
+              newpkt.exec_cmdlen);
       break;
     default:
       goto invalid;
+      invalid("type switch");
   }
 
   *buf = *buf + packet_len;
@@ -510,39 +510,69 @@ invalid:
   return NULL;
 }
 
-/* get the size of a packet before sending it on the wire
- * assumes that the packet is trusted (we can do strlen on pkt->name_fqdn...)
+/*
+ * encodes a packet and returns the size before sending it on the wire.
+ * it assumes that the packet contains correct content.
  */
-int
-pkt_getsize(struct gg_packet *pkt)
+struct gg_packet *
+pkt_encode(struct gg_packet *pkt, int *len)
 {
-  int size;
+  static struct gg_packet newpkt;
+  int packet_len;
+
+  packet_len = gg_packet_props[pkt->type].size; // XXX never overflow ?
 
+  newpkt.ver = pkt->ver;
+  newpkt.type = pkt->type;
   switch(pkt->type) {
     case PACKET_NEWCONN:
-      size = PACKET_NEWCONN_SIZE;
+      newpkt.newconn_id = htons(pkt->newconn_id);
+      newpkt.newconn_src = htonl(pkt->newconn_src);
+      newpkt.newconn_dst = htonl(pkt->newconn_dst);
+      newpkt.newconn_proto = pkt->newconn_proto;
+      newpkt.newconn_size = pkt->newconn_size;
       break;
     case PACKET_DELCONN:
-      size = PACKET_DELCONN_SIZE;
+      newpkt.delconn_id = htons(pkt->delconn_id);
       break;
     case PACKET_DATA:
-      size = PACKET_DATA_SIZE;
+      newpkt.data_connid = htons(pkt->data_connid);
+      newpkt.data_size = pkt->data_size;
       break;
     case PACKET_NAME:
-      size = PACKET_NAME_SIZE +
-        strnlen((char *)pkt->name_fqdn, GG_PKTARG_MAX);
+      if (pkt->name_len > GG_PKTARG_MAX)
+        goto invalid;
+      packet_len = packet_len + pkt->name_len;
+      newpkt.name_addr = htonl(pkt->name_addr);
+      newpkt.name_len = pkt->name_len;
+      strncpy((char *)newpkt.name_fqdn, (char *)pkt->name_fqdn,
+              pkt->name_len);
       break;
-    case PACKET_PROC_FORK:
-      size = PACKET_PROC_FORK_SIZE;
+    case PACKET_FORK:
+      newpkt.fork_pid = htonl(pkt->fork_pid);
+      newpkt.fork_ppid = htonl(pkt->fork_ppid);
+      newpkt.fork_cpid = htonl(pkt->fork_cpid);
+      newpkt.fork_tgid = htonl(pkt->fork_tgid);
       break;
-    case PACKET_PROC_EXEC:
-      size = PACKET_PROC_EXEC_SIZE  +
-        strnlen((char *)pkt->proc_exec_cmd, GG_PKTARG_MAX);
+    case PACKET_EXEC:
+      if (pkt->exec_cmdlen > GG_PKTARG_MAX)
+        goto invalid;
+      packet_len = packet_len + pkt->exec_cmdlen;
+      newpkt.exec_pid = htonl(pkt->exec_pid);
+      newpkt.exec_cmdlen = pkt->exec_cmdlen;
+      strncpy((char *)newpkt.exec_cmd, (char *)pkt->exec_cmd,
+              pkt->exec_cmdlen);
       break;
     default:
-      size = 0;
+      error("Unsupported packet type");
+      return NULL;
   }
-  return size;
+  *len = packet_len;
+  return &newpkt;
+
+invalid:
+  error("invalid packet");
+  return NULL;
 }
 
 /*
diff --git a/libglouglou/libglouglou.h b/libglouglou/libglouglou.h
index fc07477..4cac24d 100644
--- a/libglouglou/libglouglou.h
+++ b/libglouglou/libglouglou.h
@@ -14,60 +14,53 @@
 #define PACKET_BUFFER_SIZE 16384
 #define GG_PKTARG_MAX 30
 
-struct gg_packet {
 #define PACKET_HEADER_SIZE 2
-	u_int8_t	ver;
-	u_int8_t	type;
-/* XXX nicer way for _SIZE ... ? */
 #define PACKET_TYPE_MIN 0x00
 #define PACKET_TYPE_MAX 0x11
-#define PACKET_NEWCONN 0x00
-#define PACKET_NEWCONN_SIZE (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.newconn)
-#define PACKET_DELCONN 0x01
-#define PACKET_DELCONN_SIZE (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.delconn)
-#define PACKET_DATA 0x02
-#define PACKET_DATA_SIZE (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.data)
-#define PACKET_NAME 0x03
-#define PACKET_NAME_SIZE ((PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.name) - GG_PKTARG_MAX)
-#define PACKET_PROC_FORK 0x10
-#define PACKET_PROC_FORK_SIZE (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.proc.ev.fork)
-#define PACKET_PROC_EXEC 0x11
-#define PACKET_PROC_EXEC_SIZE ((PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.proc.ev.exec) - GG_PKTARG_MAX)
+enum gg_packet_type { /* u_int8_t */
+  PACKET_NEWCONN = 0x00,
+  PACKET_DELCONN = 0x01,
+  PACKET_DATA = 0x02,
+  PACKET_NAME = 0x03,
+  PACKET_FORK = 0x10,
+  PACKET_EXEC = 0x11,
+};
 
+/* XXX is packed needed everywhere ? */
+struct __attribute__((packed)) gg_packet {
+	u_int8_t	ver;
+	u_int8_t	type;
 	union {
-		struct newconn {
+		struct __attribute__((packed)) newconn {
 			u_int16_t	id;
 			u_int32_t	src;
 			u_int32_t	dst;
 			u_int8_t	proto;
 			u_int8_t	size;
 		} newconn;
-		struct delconn {
+		struct __attribute__((packed)) delconn {
 			u_int16_t	id;
 		} delconn;
-		struct data {
+		struct __attribute__((packed)) data {
 			u_int16_t	connid;
 			u_int8_t	size;
 		} data;
-		struct name {
+		struct __attribute__((packed)) name {
 			u_int32_t	addr;
 			u_int8_t	len;
 			u_char		fqdn[GG_PKTARG_MAX];
 		} name;
-		struct proc {
+    struct __attribute__((packed)) fork {
 		  u_int32_t pid;
-      union {
-        struct fork {
-          u_int32_t ppid;
-          u_int32_t cpid;
-          u_int32_t tgid;
-        } fork;
-        struct exec {
-          u_int8_t cmdlen;
-          u_char   cmd[GG_PKTARG_MAX];
-        } exec;
-      } ev;
-    } proc;
+      u_int32_t ppid;
+      u_int32_t cpid;
+      u_int32_t tgid;
+    } fork;
+    struct __attribute__((packed)) exec {
+		  u_int32_t pid;
+      u_int8_t cmdlen;
+      u_char   cmd[GG_PKTARG_MAX];
+    } exec;
 	} pdat;
 #define newconn_id	pdat.newconn.id
 #define newconn_src	pdat.newconn.src
@@ -80,12 +73,32 @@ struct gg_packet {
 #define name_addr	pdat.name.addr
 #define name_len	pdat.name.len
 #define name_fqdn	pdat.name.fqdn
-#define proc_pid pdat.proc.pid
-#define proc_fork_ppid pdat.proc.ev.fork.ppid
-#define proc_fork_cpid pdat.proc.ev.fork.cpid
-#define proc_fork_tgid pdat.proc.ev.fork.tgid
-#define proc_exec_cmdlen pdat.proc.ev.exec.cmdlen
-#define proc_exec_cmd pdat.proc.ev.exec.cmd
+#define fork_pid pdat.fork.pid
+#define fork_ppid pdat.fork.ppid
+#define fork_cpid pdat.fork.cpid
+#define fork_tgid pdat.fork.tgid
+#define exec_pid pdat.exec.pid
+#define exec_cmdlen pdat.exec.cmdlen
+#define exec_cmd pdat.exec.cmd
+};
+
+typedef struct gg_packet_props_t {
+  int size;
+} gg_packet_props_t;
+
+gg_packet_props_t gg_packet_props[] = {
+  [PACKET_NEWCONN] = \
+    { (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.newconn) },
+  [PACKET_DELCONN] = \
+    { (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.delconn) },
+  [PACKET_DATA] = \
+    { (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.data) },
+  [PACKET_NAME] = \
+    { ((PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.name) - GG_PKTARG_MAX) },
+  [PACKET_FORK] = \
+    { (PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.fork) },
+  [PACKET_EXEC] = \
+    { ((PACKET_HEADER_SIZE + sizeof((struct gg_packet *)0)->pdat.exec) - GG_PKTARG_MAX) },
 };
 
 struct gg_user {