Re: [Monotone-devel] Mainline DB migration breaks server keys?

[Richard Levitte]

In message <address@hidden> on Sat, 5 Apr 2008 12:06:17 -0700, "Justin Patrin" 
<address@hidden> said:

papercrane> I haven't been following recent developments too close but just got 
a
papercrane> message to migrate my monotone DB using mainline. After backing up 
my
papercrane> DB I migrated and attempted to pull again. Now I'm getting a message
papercrane> about the server key changing:
papercrane> 
papercrane> address@hidden ~/src/net.venge.monotone $ ./mtn pull && ./mtn up
papercrane> && make check
papercrane> mtn: doing anonymous pull; use -kKEYNAME if you need authentication
papercrane> mtn: connecting to monotone.ca
papercrane> mtn: @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
papercrane> mtn: @ WARNING: SERVER IDENTIFICATION HAS CHANGED              @
papercrane> mtn: @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
papercrane> mtn: IT IS POSSIBLE THAT SOMEONE IS DOING SOMETHING NASTY
papercrane> mtn: it is also possible that the server key has just been changed
papercrane> mtn: remote host sent key 3e6f5225bc2fffacbc20c9de37ff2dae1e20892e
papercrane> mtn: I expected
papercrane> 
33653666353232356263326666666163626332306339646533376666326461653165323038393265
papercrane> mtn: 'mtn unset known-servers monotone.ca' overrides this check
papercrane> mtn: error: server key changed
papercrane> address@hidden ~/src/net.venge.monotone $ ./mtn --version
papercrane> monotone 0.39 (base revision: 
c4007197c145794b99ddaa2e6f7e2bc4c014714f)
papercrane> 
papercrane> I tried pulling on my other machine with an older mainline mtn and I
papercrane> didn't get any server key change messages and a quick search of
papercrane> monotone-devel doesn't show me any messages about the server key
papercrane> changing so I'm thinking the migration code did something incorrect 
to
papercrane> the server key.

The migration code did nothing wrong, the bug is elsewhere, as has
already been shown.

You can fix this two ways, the first being this:

$ mtn unset known-servers monotone.ca
$ ./mtn pull && ./mtn up && make check
$ mtn unset known-servers monotone.ca
$ ./mtn pull

At this point, everything should be fine.  When doing the pulls,
monotone should tell you that it got a new server key with the hash
3e6f5225bc2fffacbc20c9de37ff2dae1e20892e.

The other way to fix the problem is to apply the attached patch (it's
exactly that change that Zack did.  The relevant changelog is:

 * netsync.cc (session): Replace remote_peer_key_hash variable with a
   boolean, as it is only ever used as such.
   (process_hello_cmd): Encode their_key_hash into hex before
   comparing against the cached fingerprint.

Cheers,
Richard

-----
Please consider sponsoring my work on free software.
See http://www.free.lp.se/sponsoring.html for details.

-- 
Richard Levitte                         address@hidden
                                        http://richard.levitte.org/

"When I became a man I put away childish things, including
 the fear of childishness and the desire to be very grown up."
                                                -- C.S. Lewis

#
#
# patch "netsync.cc"
#  from [502912033460ac4b1defa43c06f00aefcf6d1260]
#    to [255ab2fc2c3ca94f0686014fea03035ba7649bdf]
#
============================================================
--- netsync.cc  502912033460ac4b1defa43c06f00aefcf6d1260
+++ netsync.cc  255ab2fc2c3ca94f0686014fea03035ba7649bdf
@@ -344,7 +344,7 @@ session:
   bool armed;
   bool arm();
 
-  id remote_peer_key_hash;
+  bool received_remote_key;
   rsa_keypair_id remote_peer_key_name;
   netsync_session_key session_key;
   chained_hmac read_hmac;
@@ -565,7 +565,7 @@ session::session(options & opts,
   inbuf(),
   outbuf_size(0),
   armed(false),
-  remote_peer_key_hash(""),
+  received_remote_key(false),
   remote_peer_key_name(""),
   session_key(constants::netsync_key_initializer),
   read_hmac(netsync_session_key(constants::netsync_key_initializer),
@@ -1034,7 +1034,8 @@ session::read_some()
   Netxx::signed_size_type count = str->read(tmp, sizeof(tmp));
   if (count > 0)
     {
-      L(FL("read %d bytes from fd %d (peer %s)") % count % str->get_socketfd() 
% peer_id);
+      L(FL("read %d bytes from fd %d (peer %s)")
+        % count % str->get_socketfd() % peer_id);
       if (encountered_error)
         {
           L(FL("in error unwind mode, so throwing them into the bit bucket"));
@@ -1294,22 +1295,22 @@ session::process_hello_cmd(rsa_keypair_i
                            rsa_pub_key const & their_key,
                            id const & nonce)
 {
-  I(this->remote_peer_key_hash().size() == 0);
+  I(!this->received_remote_key);
   I(this->saved_nonce().size() == 0);
 
   if (use_transport_auth)
     {
       id their_key_hash;
       key_hash_code(their_keyname, their_key, their_key_hash);
+      var_value printable_key_hash(encode_hexenc(their_key_hash()));
       L(FL("server key has name %s, hash %s")
-        % their_keyname
-        % encode_hexenc(their_key_hash()));
+        % their_keyname % printable_key_hash);
       var_key their_key_key(known_servers_domain, var_name(peer_id));
       if (project.db.var_exists(their_key_key))
         {
           var_value expected_key_hash;
           project.db.get_var(their_key_key, expected_key_hash);
-          if (expected_key_hash() != their_key_hash())
+          if (expected_key_hash != printable_key_hash)
             {
               
P(F("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n"
                   "@ WARNING: SERVER IDENTIFICATION HAS CHANGED              
@\n"
@@ -1319,8 +1320,8 @@ session::process_hello_cmd(rsa_keypair_i
                   "remote host sent key %s\n"
                   "I expected %s\n"
                   "'%s unset %s %s' overrides this check")
-                % encode_hexenc(their_key_hash())
-                % encode_hexenc(expected_key_hash())
+                % printable_key_hash
+                % expected_key_hash
                 % ui.prog_name % their_key_key.first % their_key_key.second);
               E(false, F("server key changed"));
             }
@@ -1331,8 +1332,8 @@ session::process_hello_cmd(rsa_keypair_i
               "I'll assume it's really them, but you might want to 
double-check\n"
               "their key's fingerprint: %s")
             % peer_id
-            % encode_hexenc(their_key_hash()));
-          project.db.set_var(their_key_key, var_value(their_key_hash()));
+            % printable_key_hash);
+          project.db.set_var(their_key_key, printable_key_hash);
         }
       if (project.db.put_key(their_keyname, their_key))
         W(F("saving public key for %s to database") % their_keyname);
@@ -1341,14 +1342,13 @@ session::process_hello_cmd(rsa_keypair_i
         hexenc<id> hnonce;
         encode_hexenc(nonce, hnonce);
         L(FL("received 'hello' netcmd from server '%s' with nonce '%s'")
-          % encode_hexenc(their_key_hash())
-          % hnonce);
+          % printable_key_hash % hnonce);
       }
 
       I(project.db.public_key_exists(their_key_hash));
 
       // save their identity
-      this->remote_peer_key_hash = their_key_hash;
+      this->received_remote_key = true;
       this->remote_peer_key_name = their_keyname;
     }
 
@@ -1396,7 +1396,6 @@ session::process_hello_cmd(rsa_keypair_i
   lua.hook_note_netsync_start(session_id, "client", this->role,
                               peer_id, their_keyname,
                               our_include_pattern, our_exclude_pattern);
-
   return true;
 }
 
@@ -1514,7 +1513,7 @@ session::process_auth_cmd(protocol_role 
                           id const & nonce1,
                           rsa_sha1_signature const & signature)
 {
-  I(this->remote_peer_key_hash().size() == 0);
+  I(!this->received_remote_key);
   I(this->saved_nonce().size() == constants::merkle_hash_length_in_bytes);
 
   globish_matcher their_matcher(their_include_pattern, their_exclude_pattern);
@@ -1631,8 +1630,7 @@ session::process_auth_cmd(protocol_role 
 
   rebuild_merkle_trees(ok_branches);
 
-  // Save their identity.
-  this->remote_peer_key_hash = client;
+  this->received_remote_key = true;
 
   // Check the signature.
   if (project.db.check_signature(their_id, nonce1(), signature) == cert_ok)