Fix crashes and memory leaks.

4 files changed, 75 insertions, 53 deletions

diff --git a/src/nat/nat.c b/src/nat/nat.c
index 42fe843e3..e51c001f2 100644
--- a/src/nat/nat.c
+++ b/src/nat/nat.c
@@ -407,7 +407,7 @@ struct GNUNET_NAT_Handle
   int use_stun;
 
   /**
-   * How often should se check STUN ?
+   * How often should we check STUN ?
    */
   struct GNUNET_TIME_Relative stun_frequency;
 
@@ -1109,7 +1109,8 @@ list_interfaces (void *cls,
  * @param cls the NAT handle
  * @param result , the status
  */
-static void stun_request_callback(void *cls,
+static void
+stun_request_callback(void *cls,
                                   enum GNUNET_NAT_StatusCode result)
 {
 
@@ -1129,16 +1130,18 @@ static void stun_request_callback(void *cls,
 };
 
 /**
- * Check if STUN can decode the packet
+ * CHECK if is a valid STUN packet sending to GNUNET_NAT_stun_handle_packet.
+ * It also check if it can handle the packet based on the NAT handler.
+ * You don't need to call anything else to check if the packet is valid,
  *
  * @param cls the NAT handle
  * @param data, packet
- * @param len, packet lenght
+ * @param len, packet length
  *
- * @return GNUNET_NO if it can't decode, GNUNET_YES if is a packet
+ * @return #GNUNET_NO if it can't decode, #GNUNET_YES if is a packet
  */
 int
-GNUNET_NAT_try_decode_stun_packet(void *cls, const void *data, size_t len)
+GNUNET_NAT_is_valid_stun_packet(void *cls, const void *data, size_t len)
 {
   struct GNUNET_NAT_Handle *h = cls;
   struct sockaddr_in answer;
@@ -1147,6 +1150,10 @@ GNUNET_NAT_try_decode_stun_packet(void *cls, const void *data, size_t len)
   if(!h->waiting_stun)
     return GNUNET_NO;
 
+  /*We dont have STUN installed*/
+  if(!h->use_stun)
+    return GNUNET_NO;
+
   /* Empty the answer structure */
   memset(&answer, 0, sizeof(struct sockaddr_in));
 
@@ -1154,7 +1161,7 @@ GNUNET_NAT_try_decode_stun_packet(void *cls, const void *data, size_t len)
   int valid = GNUNET_NAT_stun_handle_packet(data,len, &answer);
   if(valid)
   {
-    LOG (GNUNET_ERROR_TYPE_DEBUG,
+    LOG (GNUNET_ERROR_TYPE_INFO,
          "Stun server returned IP %s , with port %d \n", inet_ntoa(answer.sin_addr), ntohs(answer.sin_port));
     /* ADD IP AS VALID*/
     add_to_address_list (h, LAL_EXTERNAL_IP, (const struct sockaddr *) &answer,
@@ -1183,20 +1190,25 @@ process_stun (void *cls,
 {
   struct GNUNET_NAT_Handle *h = cls;
 
-  LOG (GNUNET_ERROR_TYPE_DEBUG,
-       "I will do a STUN request\n");
+  h->stun_task = NULL;
 
 
-  h->stun_task = NULL;
-  h->waiting_stun = GNUNET_YES;
 
   struct StunServerList* elem = h->actual_stun_server;
 
   /* Make the request */
-  LOG (GNUNET_ERROR_TYPE_DEBUG,
+  LOG (GNUNET_ERROR_TYPE_INFO,
        "I will request the stun server %s:%i !\n", elem->address, elem->port);
 
-  GNUNET_NAT_stun_make_request(elem->address, elem->port, h->socket, &stun_request_callback, NULL);
+  if(GNUNET_OK == GNUNET_NAT_stun_make_request(elem->address, elem->port, h->socket, &stun_request_callback, NULL))
+  {
+    h->waiting_stun = GNUNET_YES;
+  }
+  else
+  {
+    LOG (GNUNET_ERROR_TYPE_ERROR,
+         "STUN request failed %s:%i !\n", elem->address, elem->port);
+  }
 
   h->stun_task =
           GNUNET_SCHEDULER_add_delayed (h->stun_frequency,
@@ -1588,7 +1600,8 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
   }
 
   /* ENABLE STUN ONLY ON UDP*/
-  if(!is_tcp && (NULL != sock) && h->use_stun  ) {
+  if(!is_tcp && (NULL != sock) && h->use_stun  )
+  {
     h->socket = sock;
     h->actual_stun_server = NULL;
 
@@ -1772,6 +1785,11 @@ GNUNET_NAT_unregister (struct GNUNET_NAT_Handle *h)
     GNUNET_SCHEDULER_cancel (h->dns_task);
     h->dns_task = NULL;
   }
+  if (NULL !=   h->stun_task)
+  {
+    GNUNET_SCHEDULER_cancel (h->stun_task);
+    h->stun_task = NULL;
+  }
   if (NULL != h->server_proc)
   {
     if (0 != GNUNET_OS_process_kill (h->server_proc, GNUNET_TERM_SIG))
diff --git a/src/nat/nat.conf b/src/nat/nat.conf
index 63f24530e..04cecf70b 100644
--- a/src/nat/nat.conf
+++ b/src/nat/nat.conf
@@ -49,10 +49,10 @@ IFC_SCAN_FREQUENCY = 15 min
 DYNDNS_FREQUENCY = 7 min
 
 # SHOULD USE STUN ?
-USE_STUN = NO
+USE_STUN = YES
 STUN_FREQUENCY = 5 min
 # Default list of stun servers
-STUN_SERVERS = stun.services.mozilla.com:3478 stun2.l.google.com:19302
+STUN_SERVERS = stun.services.mozilla.com:3478 stun.ekiga.net:3478
 
 
 [gnunet-nat-server]
diff --git a/src/nat/nat_stun.c b/src/nat/nat_stun.c
index 5fff40656..4d1afb3bb 100644
--- a/src/nat/nat_stun.c
+++ b/src/nat/nat_stun.c
@@ -111,8 +111,10 @@ struct StunState {
  * @param msg the received message
  * @return the converted StunClass
  */
-static int decode_class(int msg)
+static int
+decode_class(int msg)
 {
+    /* Sorry for the magic, but this maps the class according to rfc5245 */
     return ((msg & 0x0010) >> 4) | ((msg & 0x0100) >> 7);
 }
 
@@ -122,7 +124,8 @@ static int decode_class(int msg)
  * @param msg the received message
  * @return the converted StunMethod
  */
-static int decode_method(int msg)
+static int
+decode_method(int msg)
 {
     return (msg & 0x000f) | ((msg & 0x00e0) >> 1) | ((msg & 0x3e00) >> 2);
 }
@@ -134,7 +137,8 @@ static int decode_method(int msg)
  * @param method method to be converted
  * @return message in a STUN compatible format
  */
-static int encode_message(StunClasses msg_class, StunMethods method)
+static int
+encode_message(StunClasses msg_class, StunMethods method)
 {
     return ((msg_class & 1) << 4) | ((msg_class & 2) << 7) |
            (method & 0x000f) | ((method & 0x0070) << 1) | ((method & 0x0f800) << 2);
@@ -146,7 +150,8 @@ static int encode_message(StunClasses msg_class, StunMethods method)
  * @param msg
  * @return string with the message class and method
  */
-static const char *stun_msg2str(int msg)
+static const char *
+stun_msg2str(int msg)
 {
 
     const struct { enum StunClasses value; const char *name; } classes[] = {
@@ -180,7 +185,7 @@ static const char *stun_msg2str(int msg)
         if (methods[i].value == value)
             break;
     }
-    snprintf(result, sizeof(result), "%s %s",
+    GNUNET_snprintf(result, sizeof(result), "%s %s",
              method ? : "Unknown Method",
              msg_class ? : "Unknown Class Message");
     return result;
@@ -192,7 +197,8 @@ static const char *stun_msg2str(int msg)
  * @param msg with a attribute type
  * @return string with the attribute name
  */
-static const char *stun_attr2str(int msg)
+static const char *
+stun_attr2str(int msg)
 {
     const struct { enum StunAttributes value; const char *name; } attrs[] = {
             { STUN_MAPPED_ADDRESS, "Mapped Address" },
@@ -234,7 +240,8 @@ static const char *stun_attr2str(int msg)
  *
  * @param req, stun header to be filled
  */
-static int stun_process_attr(struct StunState *state, struct stun_attr *attr)
+static int
+stun_process_attr(struct StunState *state, struct stun_attr *attr)
 {
     LOG (GNUNET_ERROR_TYPE_INFO,
          "Found STUN Attribute %s (%04x), length %d\n",
@@ -264,7 +271,7 @@ static int stun_process_attr(struct StunState *state, struct stun_attr *attr)
 static void
 generate_request_id(struct stun_header *req)
 {
-    int x;
+    unsigned int x;
     req->magic = htonl(STUN_MAGIC_COOKIE);
     for (x = 0; x < 3; x++)
         req->id.id[x] = GNUNET_CRYPTO_random_u32 (GNUNET_CRYPTO_QUALITY_NONCE,
@@ -282,7 +289,7 @@ generate_request_id(struct stun_header *req)
  * @param arg , pointer to a sockaddr_in where we will set the reported IP and port
  * @param magic , Magic cookie
  *
- * @return 0 on sucess, other value otherwise
+ * @return 0 on success, other value otherwise
  */
 static int
 stun_get_mapped(struct StunState *st, struct stun_attr *attr,struct sockaddr_in *arg, unsigned int magic)
@@ -312,7 +319,7 @@ stun_get_mapped(struct StunState *st, struct stun_attr *attr,struct sockaddr_in
     }
 
     st->attr = type;
-    /*TODO: Detect Family*/
+
     sa->sin_family = AF_INET;
     sa->sin_port = returned_addr->port ^ htons(ntohl(magic) >> 16);
     sa->sin_addr.s_addr = returned_addr->addr ^ magic;
@@ -331,12 +338,12 @@ stun_get_mapped(struct StunState *st, struct stun_attr *attr,struct sockaddr_in
  * @param len, the length of the packet
  * @param arg, sockaddr_in where we will set our discovered packet
  *
- * @return, GNUNET_OK on OK, GNUNET_NO if the packet is invalid ( not a stun packet)
+ * @return, #GNUNET_OK on OK, #GNUNET_NO if the packet is invalid ( not a stun packet)
  */
 int
 GNUNET_NAT_stun_handle_packet(const void *data, size_t len, struct sockaddr_in *arg)
 {
-    struct stun_header *hdr = (struct stun_header *)data;
+    const struct stun_header *hdr = (const struct stun_header *)data;
     struct stun_attr *attr;
     struct StunState st;
     int ret = GNUNET_OK;
@@ -367,28 +374,29 @@ GNUNET_NAT_stun_handle_packet(const void *data, size_t len, struct sockaddr_in *
     if(STUN_MAGIC_COOKIE != message_magic_cookie){
         LOG (GNUNET_ERROR_TYPE_INFO,
              "Invalid magic cookie \n");
-        GNUNET_break_op (0);
         return GNUNET_NO;
     }
 
 
-    LOG (GNUNET_ERROR_TYPE_INFO, "STUN Packet, msg %s (%04x), length: %d\n", stun_msg2str(ntohs(hdr->msgtype)), ntohs(hdr->msgtype), advertised_message_size);
+    LOG (GNUNET_ERROR_TYPE_INFO, "STUN Packet, msg %s (%04x), length: %d\n", stun_msg2str(ntohs(hdr->msgtype)),
+         ntohs(hdr->msgtype),
+         advertised_message_size);
 
 
     if (advertised_message_size > len) {
-        LOG (GNUNET_ERROR_TYPE_INFO, "Scrambled STUN packet length (got %d, expecting %d)\n", advertised_message_size, (int)len);
-        GNUNET_break_op (0);
+        LOG (GNUNET_ERROR_TYPE_INFO, "Scrambled STUN packet length (got %d, expecting %d)\n", advertised_message_size,
+             (int)len);
         return GNUNET_NO;
     } else {
         len = advertised_message_size;
     }
-    /* Zero the struct */
+
     memset(&st,0, sizeof(st));
 
     while (len > 0) {
         if (len < sizeof(struct stun_attr)) {
-            LOG (GNUNET_ERROR_TYPE_INFO, "Attribute too short (got %d, expecting %d)\n", (int)len, (int) sizeof(struct stun_attr));
-            GNUNET_break_op (0);
+            LOG (GNUNET_ERROR_TYPE_INFO, "Attribute too short (got %d, expecting %d)\n", (int)len,
+                 (int) sizeof(struct stun_attr));
             break;
         }
         attr = (struct stun_attr *)data;
@@ -398,8 +406,8 @@ GNUNET_NAT_stun_handle_packet(const void *data, size_t len, struct sockaddr_in *
 
         /* Check if we still have space in our buffer */
         if (advertised_message_size > len ) {
-            LOG (GNUNET_ERROR_TYPE_INFO, "Inconsistent Attribute (length %d exceeds remaining msg len %d)\n", advertised_message_size, (int)len);
-            GNUNET_break_op (0);
+            LOG (GNUNET_ERROR_TYPE_INFO, "Inconsistent Attribute (length %d exceeds remaining msg len %d)\n", advertised_message_size,
+                 (int)len);
             break;
         }
 
@@ -407,7 +415,8 @@ GNUNET_NAT_stun_handle_packet(const void *data, size_t len, struct sockaddr_in *
         stun_get_mapped(&st, attr, arg, hdr->magic);
 
         if (stun_process_attr(&st, attr)) {
-            LOG (GNUNET_ERROR_TYPE_INFO, "Failed to handle attribute %s (%04x)\n", stun_attr2str(ntohs(attr->attr)), ntohs(attr->attr));
+            LOG (GNUNET_ERROR_TYPE_INFO, "Failed to handle attribute %s (%04x)\n", stun_attr2str(ntohs(attr->attr)),
+                 ntohs(attr->attr));
             break;
         }
         /** Clear attribute id: in case previous entry was a string,
@@ -429,7 +438,8 @@ GNUNET_NAT_stun_handle_packet(const void *data, size_t len, struct sockaddr_in *
  *
  * @param cls our `struct GNUNET_NAT_STUN_Handle *`
  */
-void clean(struct GNUNET_NAT_STUN_Handle * handle)
+static void
+clean(struct GNUNET_NAT_STUN_Handle * handle)
 {
     GNUNET_free(handle->stun_server);
     GNUNET_free(handle);
@@ -526,9 +536,9 @@ stun_dns_callback (void *cls,
  * @param server, the address of the stun server
  * @param port, port of the stun server
  * @param sock the socket used to send the request
- * @return GNUNET_NAT_STUN_Handle on success, NULL on error.
+ * @return #GNUNET_OK success, #GNUNET_NO on error.
  */
-struct GNUNET_NAT_STUN_Handle *
+int
 GNUNET_NAT_stun_make_request(char * server, int port,
                              struct GNUNET_NETWORK_Handle * sock,GNUNET_NAT_stun_RequestCallback cb,
                              void *cb_cls)
@@ -554,9 +564,10 @@ GNUNET_NAT_stun_make_request(char * server, int port,
     if(rh->dns_active == NULL)
     {
         GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR, "Failed DNS");
-        return NULL;
+        GNUNET_free(rh);
+        return GNUNET_NO;
     }
 
 
-    return rh;
+    return GNUNET_OK;
 }
diff --git a/src/nat/test_stun.c b/src/nat/test_stun.c
index e75d55bd0..3988dc053 100644
--- a/src/nat/test_stun.c
+++ b/src/nat/test_stun.c
@@ -51,8 +51,8 @@
 static unsigned long port = 7895;
 static int ret = 1;
 
-static char *stun_server = "stun2.l.google.com";
-static int stun_port = 19302;
+static char *stun_server = "stun.ekiga.net";
+static int stun_port = 3478;
 
 /**
  * The listen socket of the service for IPv4
@@ -71,7 +71,7 @@ static struct GNUNET_SCHEDULER_Task * ltask4;
 static void
 print_answer(struct sockaddr_in* answer)
 {
-        printf("External IP is: %s , with port %d\n", inet_ntoa(answer->sin_addr), ntohs(answer->sin_port));
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO,"External IP is: %s , with port %d\n", inet_ntoa(answer->sin_addr), ntohs(answer->sin_port));
 }
 
 
@@ -91,9 +91,6 @@ do_udp_read (void *cls,
         ssize_t rlen;
         struct sockaddr_in answer;
 
-    printf("UDP READ\n");
-
-
     if ((0 != (tc->reason & GNUNET_SCHEDULER_REASON_READ_READY)) &&
       (GNUNET_NETWORK_fdset_isset (tc->read_ready,
                                    lsock4)))
@@ -161,7 +158,6 @@ stop ()
 {
     GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Stopping NAT and quitting...\n");
 
-    printf("Stopped !!\n");
     //Clean task
     if(NULL != ltask4)
         GNUNET_SCHEDULER_cancel (ltask4);
@@ -178,7 +174,6 @@ enum GNUNET_NAT_StatusCode result)
 {
     ret = result;
     stop();
-    printf("Called back\n");
 };
 
 
@@ -211,9 +206,7 @@ run (void *cls, char *const *args, const char *cfgfile,
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                 "Service listens on port %u\n",
                 port);
-        printf("Start main event\n");
     GNUNET_NAT_stun_make_request(stun_server, stun_port, lsock4, &request_callback, NULL);
-    printf("Made the requeest\n");
 
     //GNUNET_SCHEDULER_add_delayed (TIMEOUT, &stop, NULL);