Re: tmdns patches

Steve G <[email protected]> Sat, 31 Jan 2004 16:07:41 -0800 (PST)
Newsgroups gmane.network.zeroconf.workers
Message-ID <[email protected]>
>Ok, finaly got your patches and had some time to apply them. 

Thanks.

>There are some small differences, so you might should update 
>your cvs tree and have a look at it.

Sourceforge's public cvs was having problems this morning. I just
got the new tree and will re-sync.

>Most notably in patch 4 you wanted to change the type for the 
>ttl in dns.c/dns_walk_buf to "unsigned int". However this is
>actually a 32bit int in the dns packet. 

Hmmm. I suspect it should be an unsigned number since you cannot
have a negative ttl. It just doesn't make sense. I usually make
everything that cannot ever be negative unsigned so that there
are no surprises from malicious packets. Ttl, though, doesn't get
used to create a buffer...so its probably just as well being
signed. I'll look again at the signed/unsigned conflicts and see
what's new.

>I changed the types for dtype, dclass and dttl in that 
>function to u_int16_t/u_int32_t, but I don't know if 
>this was portable to other systems. This change also 
>caused some more compiler warnings, but I will look into 
>that.

I think we had some problems with that on some older BSD'd and
maybe solaris in Xinetd. I'll double check Xinetd's code and see
what we used.

>Could you send me the .spec file from mandrake? I could 
>include this into the sources.

I'll write  a new spec file and send it. Mandrake made 1 or 2
mistakes I think. For example, they set tmdns to run under the
daemon account. That means any other daemon that runs under that
account can send signals to tmdns. Tmdns should have its own
account and the spec file should create it.

>>My next step is to try sending malicious packets and do a
>>dataflow review.
>
>Looking forward to your results. Hopefully there are less
>problems than with C-Syntax ...

I'll finally get to work on that on Sunday after re-syncing with
cvs. In the meantime, I have attached a patch to this e-mail that
corrects some misspelled words. The patch also has 2 chunks that
check the return value from getifaddrs and exit if the return
value is < 0. The patch is not against the latest cvs, hopefully
the line numbers didn't change too much.

I have also noticed some problems with getifaddrs and have a
final patch that resolves all problems for portability. The
problem is related to the header file ifaddrs.h. I have 2 more
scenarios to check before I send it.

I have not looked to see where the ifaddrs.c file comes from. But
since its copyrighted 2000, there is likely to be a newer file.
Has anyone looked at how a newer implementation differs from what
is currently provided in tmdns?

I also have 2-3 machines here running ipv6 for xinetd work. After
I verify tmdns handling malicious data, I'll probably take stab
at getting IPv6 support working. It should be real simple.

Sincerely,
Steve Grubb

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free web site building tool. Try it!
http://webhosting.yahoo.com/ps/sb/
tmdns-patch6.txt (text/plain, 8.9 KB)
diff -urBb tmdns-devel-e/server/conf.c tmdns-devel/server/conf.c
--- tmdns-devel-e/server/conf.c	2004-01-29 11:00:49.000000000 -0500
+++ tmdns-devel/server/conf.c	2004-01-31 11:10:24.000000000 -0500
@@ -36,7 +36,7 @@
   default_ttl:    120 * 60 ,              /* TTL for local entries */
   pid_file:       PID_FILE_DEFAULT ,
   dns_bridge:     1,                      /* do not bridge dns queries */
-  dns_port:       53,			  /* port to listen for unixast queries */
+  dns_port:       53,			  /* port to listen for unicast queries */
   gather_delay:   2,			  /* how long to delay answers */
   allow_nonlocal: 0,			  /* forward no-local unicast queries
 					     to the mDNS group */ 
diff -urBb tmdns-devel-e/server/dns.c tmdns-devel/server/dns.c
--- tmdns-devel-e/server/dns.c	2004-01-29 10:54:34.000000000 -0500
+++ tmdns-devel/server/dns.c	2004-01-31 11:08:42.000000000 -0500
@@ -65,7 +65,7 @@
   }
 
   pkt->u.hdr.qr     = 1;       /* this is a response                   */
-  pkt->u.hdr.aa     = 0;       /* never send authoritativ answers      */
+  pkt->u.hdr.aa     = 0;       /* never send authoritative answers     */
   pkt->u.hdr.tc     = 0;       /* not truncated, but we will see ...   */
   pkt->u.hdr.rd     = 0;       /* we always do recursion (when online) */
   pkt->u.hdr.ra     = 0;       /* recursion is not available           */
diff -urBb tmdns-devel-e/server/dns.h tmdns-devel/server/dns.h
--- tmdns-devel-e/server/dns.h	2004-01-29 10:02:30.000000000 -0500
+++ tmdns-devel/server/dns.h	2004-01-31 11:06:28.000000000 -0500
@@ -62,7 +62,7 @@
 
 
 typedef struct {
-  /* struct in_addr dst_ip;	          to wich address send this msg. */
+  /* struct in_addr dst_ip;	          to which address sent this msg. */
   int           dst_port;	       /* to which port send             */
 
   socklen_t        dst_len;
diff -urBb tmdns-devel-e/server/info.c tmdns-devel/server/info.c
--- tmdns-devel-e/server/info.c	2004-01-29 11:27:57.000000000 -0500
+++ tmdns-devel/server/info.c	2004-01-31 13:14:03.000000000 -0500
@@ -340,7 +340,10 @@
      * each IPv4 address we have.
      * Also add PTR records for the reverse zone.
      */
-    getifaddrs(&ifs);
+    if (getifaddrs(&ifs) < 0) {
+        syslog(LOG_ERR,"getifaddrs failed...exiting.");
+        exit(1);
+    }
 
     for(ifnow = ifs; ifnow; ifnow = ifnow->ifa_next) {
 	if( ifnow->ifa_flags & IFF_LOOPBACK ) continue;
@@ -363,7 +366,7 @@
 
 		    asprintf( &revname , "%d.%d.%d.%d.in-addr.arpa" ,
 				    ib[3] , ib[2], ib[1], ib[0] );
-		    debug("Reverse addrees is %s\n" , revname );
+		    debug("Reverse address is %s\n" , revname );
 		    ll_add(records , newPtrRec(namebuf,revname));
 		    free(revname);
 		}
diff -urBb tmdns-devel-e/server/llist.c tmdns-devel/server/llist.c
--- tmdns-devel-e/server/llist.c	2004-01-29 09:02:37.000000000 -0500
+++ tmdns-devel/server/llist.c	2004-01-31 11:01:51.000000000 -0500
@@ -39,7 +39,7 @@
 /********************************************************
   ll_add - adds an element to the end of the linked list 
   
-  @param  control       The contollling structure as created by ll_new
+  @param  control       The controlling structure as created by ll_new
   @param  datatoinsert  A cache entry to be inserted into the linked list 
   @return 0 on success, -1 on errror
  *******************************************************/		  
@@ -49,7 +49,7 @@
 
    if(control==NULL || datatoinsert==NULL)
    {
-      debug("ll_add: Invalid paramater passed in\n");
+      debug("ll_add: Invalid parameter passed in\n");
       return -1;
    }
 
@@ -124,7 +124,7 @@
 /***********************************************
   ll_next - returns the next element in the list
   
-  @param  element An list element to be used to obtain the next element
+  @param  element A list element to be used to obtain the next element
   @return A pointer to the next element or NULL on error
  **********************************************/
 ll_entry_t *ll_next(ll_entry_t *element)
diff -urBb tmdns-devel-e/server/serv_udp.c tmdns-devel/server/serv_udp.c
--- tmdns-devel-e/server/serv_udp.c	2004-01-29 13:24:43.000000000 -0500
+++ tmdns-devel/server/serv_udp.c	2004-01-31 10:43:22.000000000 -0500
@@ -44,7 +44,7 @@
 static const char * udp_sockaddr_str( struct sockaddr * addr );
 
 /*****************************************************************************
- * open sockets we ar interested in.
+ * open sockets we are interested in.
  *
  * We will open
  *  - one unicast listener on port 53, loopback device (if enabled)
@@ -54,10 +54,10 @@
  * for IPv6 we should also open a socket for the IPv6 multicast group
  *
  * Arguments:
- *   exclude_devs  :  device names to exclude when geting the addresses.
+ *   exclude_devs  :  device names to exclude when getting the addresses.
  *   		      not used yet.
- *   sockets       :  Array of intergers where to store the file
- *   		      descriptors.
+ *   sockets       :  Array of integers where the file descriptors are
+ *                    stored.
  *
  * Returns:
  *   number of fd's opened.
@@ -180,7 +183,7 @@
  *  sock    -  sockaddr structure giving the address to listen on
  * 
  * Returns
- *   >= if the socket could be opened, < 1 if an error occured.
+ *   >= 0 if the socket is opened, < 1 if an error occured.
  *
  ****************************************************************************/
 static int udp_sock_open( int mcast , struct sockaddr * sock )
@@ -311,7 +314,6 @@
 /*****************************************************************************
  * Copy address from an incomming udp_packet to an answer packet.
  *
- *
  *****************************************************************************/
 void udp_copy_answer_address(dns_t * answer, struct udp_packet * udp_pkt) {
 
diff -urBb tmdns-devel-e/server/tmdns.c tmdns-devel/server/tmdns.c
--- tmdns-devel-e/server/tmdns.c	2004-01-29 13:32:19.000000000 -0500
+++ tmdns-devel/server/tmdns.c	2004-01-31 10:58:48.000000000 -0500
@@ -183,8 +183,8 @@
 /*****************************************************************************
  * change userid.
  *
- * Beside setting the uid of thos process, we also change the owner of 
- * log- and pidfiles to allow writing when we are not longer root.
+ * Beside setting the uid of this process, we also change the owner of 
+ * log- and pidfiles to allow writing when we are no longer root.
  * 
  *****************************************************************************/
 static void change_id(void) {
@@ -229,7 +229,8 @@
   int sockfds[ MAX_IF + 1 ];
   int usedifs = 0;
 
-  /* count this down to check when we have send all probes we should have */
+  /* count this down to check when we have sent all the probes that we should 
+     have */
   int probing    = 3;	
 
   /* when to answer the next outstanding answer */
@@ -255,7 +256,7 @@
 
   openlog("tmdns", LOG_PID , LOG_DAEMON );
 
-  /* initisalize the RRs to announce */
+  /* initialize the RRs to announce */
   info_init();
 
   for( i = 0; i < MAX_IF ; i ++ )
@@ -371,7 +372,7 @@
 
 	if( probing == 0 ) {
 	    /*
-	     * this is the first timeout after we have send our last
+	     * this is the first timeout after we have sent our last
 	     * probe. Set probing to -1 to start normal
 	     * operation.
 	     */
@@ -389,7 +390,7 @@
 
 	if( ll_first(query_list) != NULL ) {
 	    /*
-	     * Check outstanding queries that may should be answered now.
+	     * Check outstanding queries that should be answered now.
 	     */
 	     debug("handle outstanding queries ...\n");
 	     next_outstanding_answer = answer_outstanding_queries();
@@ -635,9 +636,9 @@
 /*****************************************************************************
  * Add a local (bridged) query to our list of outstanding queries.
  *
- * We create a new dns_t structure fro the answer and add our local RR's
+ * We create a new dns_t structure for the answer and add our local RR's
  * to this list (as we do it for mcast queries) but we do not answer this
- * query imediately. Instead, we send this query as a multicast query.
+ * query immediately. Instead, we send this query as a multicast query.
  *
  * When we get back some answers for the query, we will add the answer
  * records to the stored query. 
@@ -778,7 +779,6 @@
 /*****************************************************************************
  * Answer outstanding queries.
  *
- *
  *****************************************************************************/
 static int answer_outstanding_queries(void) {
 
@@ -813,7 +813,7 @@
 	} else {
 	  /*
 	   * Our list is kept in the order we've got the queries. Whenever
-	   * we find a record that is not to be send now, we can return.
+	   * we find a record that is not to be sent now, we can return.
 	   */
 	   return answer->timeout ;
 	}
@@ -915,7 +915,7 @@
 }
 
 /*****************************************************************************
- * Fill the RR's from the answer record we've got into the queries that 
+ * Fill the RR's from the answer record we've gotten into the queries that 
  * may need them.
  *
  * We do no duplicate elimination.