[Sks-devel] Re: [PATCH] Keep DNS mappings fresh

[Kim Minh Kaplan]

This second patch on SKS 1.1.0 prevents a unknown client from keeping
partners out when one of the partners has a broken DNS server.

Kim Minh.

diff -ru sks-1.1.0.orig/membership.ml sks-1.1.0-dnsfix-2/membership.ml
--- sks-1.1.0.orig/membership.ml        2008-05-08 01:05:54.000000000 +0000
+++ sks-1.1.0-dnsfix-2/membership.ml    2009-03-23 20:59:41.000000000 +0000
@@ -44,33 +44,34 @@
 
 let local_recon_addr = Utils.unit_memoize local_recon_addr
 
-let remove_self addresses = 
-  List.filter ~f:(fun (addr,str) -> addr <> local_recon_addr ()) addresses
+let parse_address l =
+  try
+    sscanf l "%s %d"
+      (fun addr port -> addr, port)
+  with
+    Scanf.Scan_failure _ | End_of_file | Failure _ -> raise (Malformed_entry l)
 
 let convert_address l =
+  let (addr, port) = parse_address l in
   try 
-    sscanf l "%s %d" 
-    (fun addr port -> Unix.ADDR_INET (lookup_hostname addr,port))
+    Some (Unix.ADDR_INET (lookup_hostname addr, port))
   with 
-    Scanf.Scan_failure _ | End_of_file | Failure _ -> raise (Malformed_entry l)
+  | Lookup_failure addr -> perror "Lookup failure on address %s" addr;
+      None
 
 let load_membership_file file =
   let rec loop list =
     try
       let line = decomment (input_line file) in
-      let addr = convert_address line in
-      (addr,line) :: loop list
+      ignore (parse_address line);
+      loop (line :: list)
     with
       | End_of_file -> list
-      | Lookup_failure addr -> 
-         perror "Lookup failure on address %s" addr;
-         loop list
       | Malformed_entry line -> 
          perror "Malformed entry %s" line;
          loop list
   in
-  let raw_membership = loop [] in
-  Array.of_list (remove_self raw_membership)
+  loop []
 
 let get_mtime fname = 
   try
@@ -80,23 +81,11 @@
   with 
       Unix.Unix_error _ -> None
 
-let load_membership fname = 
-  let file = open_in fname in
-  protect ~f:(fun () -> 
-               let mshp = load_membership_file file in
-               match get_mtime fname with
-                 | None -> 
-                     plerror 2 "%s" 
-                       ("Unable to get mtime for membership. " ^
-                        "Failed to reload.")
-                 | Some mtime -> membership := (mshp,mtime)
-            )
-    ~finally:(fun () -> close_in file)
-
-let sockaddr_to_string sockaddr = match sockaddr with
-    Unix.ADDR_UNIX s -> sprintf "<ADDR_UNIX %s>" s
-  | Unix.ADDR_INET (addr,p) -> sprintf "<ADDR_INET %s:%d>" 
-      (Unix.string_of_inet_addr addr) p
+let sockaddr_to_string = function
+    None -> "<ADDR_UNKNOWN>"
+  | Some Unix.ADDR_UNIX s -> sprintf "<ADDR_UNIX %s>" s
+  | Some Unix.ADDR_INET (addr,p) -> sprintf "<ADDR_INET %s:%d>" 
+       (Unix.string_of_inet_addr addr) p
 
 let membership_string () = 
   let (mshp,_) = !membership in
@@ -106,19 +95,46 @@
   let strings = List.map ~f:to_string (Array.to_list mshp) in
   "Membership: " ^ String.concat ~sep:", " strings
     
+(* Refresh member n's address *)
+let refresh_member members n =
+  match members.(n) with
+    (addr, line) ->
+      let fresh_addr = convert_address line in
+      if addr <> fresh_addr then begin
+       members.(n) <- (fresh_addr, line);
+       plerror 3 "address for %s changed from %s to %s"
+         line (sockaddr_to_string addr) (sockaddr_to_string fresh_addr)
+      end
+
+(* Returns true if membership has been reloaded, false otherwise *)
+let load_membership fname = 
+  let mtime = (Unix.stat fname).Unix.st_mtime in
+  if mtime <> (snd !membership) then
+    let memberlines =
+      let file = open_in fname in
+      protect ~f:(fun () -> load_membership_file file)
+       ~finally:(fun () -> close_in file)
+    in
+    let old = Array.to_list (fst !membership) in
+    let f line =
+      try
+       List.find ~f:(fun (_, old_line) -> line = old_line) old
+      with
+       Not_found -> (None, line)
+    in
+    let merged = List.map ~f memberlines in
+    membership := (Array.of_list merged, mtime);
+    (* Try to lookup unknown names *)
+    Array.iteri
+      ~f:(fun i mb -> if fst mb = None then refresh_member (fst !membership) i)
+      (fst !membership);
+    plerror 5 "%s" (membership_string ())
 
-let reload_if_changed () = 
+let reload_if_changed () =
   let fname = Lazy.force Settings.membership_file in
-  let (mshp,old_mtime) = !membership in
-  match get_mtime fname with
-    | None -> 
-       plerror 2 "%s" ("Unable to get mtime for membership file. " ^
-                       "Can't decide whether to reload")
-    | Some mtime ->
-       if old_mtime <> mtime then 
-         ( load_membership fname;
-           plerror 5 "%s" (membership_string ())
-         )
+  try
+    load_membership fname
+  with _ -> plerror 2 "Failed to reload %s." fname
 
 let get_names () = 
   let mshp = 
@@ -136,16 +152,19 @@
   let (m,mtime) = !membership in
   membership := (m,0.)
 
-let get () = 
-  let mshp = 
-    if Sys.file_exists (Lazy.force Settings.membership_file) then (
-      reload_if_changed ();
-      let (m,mtime) = !membership in 
-      m
-    )
-    else [| |]
-  in
-  Array.map ~f:fst mshp
+let rec choose_partner () =
+  if Sys.file_exists (Lazy.force Settings.membership_file) then begin
+    reload_if_changed ();
+    let (mshp, _) = !membership in
+    let choice = Random.int (Array.length mshp) in
+    refresh_member mshp choice;
+    match mshp.(choice) with
+      (None, _) -> choose_partner()
+    | (Some addr, _) -> if addr = local_recon_addr() then
+       choose_partner () else
+       addr
+  end else
+    raise Not_found
 
 let same_inet_addr addr1 addr2 = 
   match (addr1,addr2) with
@@ -155,15 +174,15 @@
 let test addr = 
   reload_if_changed ();
   let (m,mtime) = !membership in
-  
-  let found = ref false in
-  let i = ref 0 in
-  while !i < Array.length m && not !found do 
-    if same_inet_addr addr (fst m.(!i)) then
-      found := true;
-    incr i
-  done;
-  !found
+  let rec lookup i =
+    if i < Array.length m then
+      match fst m.(i) with
+       None -> lookup (i + 1)
+      | Some iaddr -> (same_inet_addr addr iaddr) || (lookup (i + 1))
+    else
+      false
+  in
+  lookup 0
 
 
 
diff -ru sks-1.1.0.orig/reconserver.ml sks-1.1.0-dnsfix-2/reconserver.ml
--- sks-1.1.0.orig/reconserver.ml       2008-05-08 01:05:54.000000000 +0000
+++ sks-1.1.0-dnsfix-2/reconserver.ml   2009-03-23 20:59:41.000000000 +0000
@@ -74,13 +74,9 @@
        handle addr cin cout
     )
 
-  let choose array = 
-    if Array.length array = 0 then raise Not_found
-    else array.(Random.int (Array.length array))
-
   let choose_partner () = 
     try
-      choose (Membership.get ())
+      Membership.choose_partner ()
     with
        Not_found | Invalid_argument _ -> 
          failwith "No gossip partners available"