Added HostKeyPair class, signatures for heartbeat messages.

remoting/host/heartbeat_sender.cc

Index: remoting/host/heartbeat_sender.cc
diff --git a/remoting/host/heartbeat_sender.cc b/remoting/host/heartbeat_sender.cc
index fa234e35ae6039ec63c644b4667de1bb9e7de54c..4ad3d927c868b7a8b9f4ff6959594c576c138037 100644
--- a/remoting/host/heartbeat_sender.cc
+++ b/remoting/host/heartbeat_sender.cc
@@ -6,70 +6,111 @@
 
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/string_util.h"
+#include "base/time.h"
 #include "remoting/base/constants.h"
 #include "remoting/host/host_config.h"
+#include "remoting/host/host_key_pair.h"
 #include "remoting/jingle_glue/iq_request.h"
 #include "remoting/jingle_glue/jingle_client.h"
 #include "remoting/jingle_glue/jingle_thread.h"
-#include "talk/xmpp/constants.h"
-#include "talk/xmllite/xmlelement.h"
+#include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
+#include "third_party/libjingle/source/talk/xmpp/constants.h"
 
 namespace remoting {
 
 namespace {
-const char * const kChromotingNamespace = "google:remoting";
-const buzz::QName kHeartbeatQuery(true, kChromotingNamespace, "heartbeat");
-const buzz::QName kHostIdAttr(true, kChromotingNamespace, "hostid");
+const char kHeartbeatQueryTag[] = "heartbeat";
+const char kHostIdAttr[] = "hostid";
+const char kHeartbeatSignatureTag[] = "signature";
+const char kSignatureTimeAttr[] = "time";
 
 // TODO(sergeyu): Make this configurable by the cloud.
 const int64 kHeartbeatPeriodMs = 5 * 60 * 1000;  // 5 minutes.
 }
 
 HeartbeatSender::HeartbeatSender()
-    : started_(false) {
+    : state_(CREATED) {
 }
 
-void HeartbeatSender::Start(MutableHostConfig* config, JingleClient* jingle_client) {
-  DCHECK(jingle_client);
-  DCHECK(!started_);
+HeartbeatSender::~HeartbeatSender() {
+  DCHECK(state_ != STARTED);

[awong, 2010/08/02 19:53:49]

Should this be a positive filter?  That is, specify all the valid end-states
with an "or"?  It's more verbose, but a bit more robust to new states being
added.

[Sergey Ulanov, 2010/08/03 02:10:39]

Done.

+}
 
-  started_ = true;
+bool HeartbeatSender::Init(MutableHostConfig* config,
+                           JingleClient* jingle_client) {
+  DCHECK(jingle_client);
+  DCHECK(state_ == CREATED);
 
   jingle_client_ = jingle_client;
   config_ = config;
 
   if (!config_->GetString(kHostIdConfigPath, &host_id_)) {
     LOG(ERROR) << "host_id is not defined in the config.";
-    return;
+    return false;
   }
 
+  key_pair_ = new HostKeyPair();
+  if (!key_pair_->Load(config)) {
+    return false;
+  }
+
+  state_ = INITIALIZED;
+
+  return true;
+}
+
+void HeartbeatSender::Start() {
+  DCHECK(state_ == INITIALIZED);
+  state_ = STARTED;
   jingle_client_->message_loop()->PostTask(
       FROM_HERE, NewRunnableMethod(this, &HeartbeatSender::DoStart));
 }
 
+void HeartbeatSender::Stop() {
+  DCHECK(state_ == STARTED);
+  state_ = STOPPED;
+  jingle_client_->message_loop()->PostTask(
+      FROM_HERE, NewRunnableMethod(this, &HeartbeatSender::DoStop));
+}
+
 void HeartbeatSender::DoStart() {
   DCHECK(MessageLoop::current() == jingle_client_->message_loop());
 
-  request_.reset(new IqRequest(jingle_client_));
+  request_.reset(jingle_client_->CreateIqRequest());
   request_->set_callback(NewCallback(this, &HeartbeatSender::ProcessResponse));
 
   jingle_client_->message_loop()->PostTask(
       FROM_HERE, NewRunnableMethod(this, &HeartbeatSender::DoSendStanza));
 }
 
-void HeartbeatSender::DoSendStanza() {
+void HeartbeatSender::DoStop() {
   DCHECK(MessageLoop::current() == jingle_client_->message_loop());
 
-  LOG(INFO) << "Sending heartbeat stanza to " << kChromotingBotJid;
+  request_.reset(NULL);
+}
+
+void HeartbeatSender::DoSendStanza() {
+  if (state_ == STARTED) {
+    // |jingle_client_| may be already destroyed if |state_| is set to
+    // |STOPPED|, so don't touch it here unless we are in |STARTED| state.
+    DCHECK(MessageLoop::current() == jingle_client_->message_loop());
+
+    LOG(INFO) << "Sending heartbeat stanza to " << kChromotingBotJid;
+
+    buzz::XmlElement* query = new buzz::XmlElement(
+        buzz::QName(kChromotingXmlNamespace, kHeartbeatQueryTag));
+    query->AddAttr(buzz::QName(kChromotingXmlNamespace, kHostIdAttr), host_id_);
+
+    query->AddElement(CreateSignature(jingle_client_->GetFullJid()));
 
-  buzz::XmlElement* stanza = new buzz::XmlElement(kHeartbeatQuery);
-  stanza->AddAttr(kHostIdAttr, host_id_);
-  request_->SendIq(buzz::STR_SET, kChromotingBotJid, stanza);
+    request_->SendIq(buzz::STR_SET, kChromotingBotJid, query);
 
-  // Schedule next heartbeat.
-  jingle_client_->message_loop()->PostDelayedTask(
-      FROM_HERE, NewRunnableMethod(this, &HeartbeatSender::DoSendStanza),
-      kHeartbeatPeriodMs);
+    // Schedule next heartbeat.
+    jingle_client_->message_loop()->PostDelayedTask(
+        FROM_HERE, NewRunnableMethod(this, &HeartbeatSender::DoSendStanza),
+        kHeartbeatPeriodMs);
+  }
 }
 
 void HeartbeatSender::ProcessResponse(const buzz::XmlElement* response) {
@@ -79,4 +120,20 @@ void HeartbeatSender::ProcessResponse(const buzz::XmlElement* response) {
   }
 }
 
+buzz::XmlElement* HeartbeatSender::CreateSignature(const std::string& jid) {

[awong, 2010/08/02 19:53:49]

Somewhere, probably at the top of the .h file for HearbeatSender, we should give
a deep description of the packet format (ie., what the XML looks like) and what
the signature is covering.

[Sergey Ulanov, 2010/08/03 02:10:39]

Added info to the .h file. BTW this info is already in the design doc.

[awong, 2010/08/03 19:21:02]

ah...forgot to look there. :)  More reason to put it in the code I guess. :P

+  buzz::XmlElement* signature_tag = new buzz::XmlElement(
+      buzz::QName(kChromotingXmlNamespace, kHeartbeatSignatureTag));
+
+  int64 time = static_cast<int64>(base::Time::Now().ToDoubleT());
+  std::string time_str(Int64ToString(time));
+  signature_tag->AddAttr(
+      buzz::QName(kChromotingXmlNamespace, kSignatureTimeAttr), time_str);
+
+  std::string message = jid + ' ' + time_str;

[awong, 2010/08/02 19:53:49]

So we only sign the jid + time_str?

[Sergey Ulanov, 2010/08/03 02:10:39]

Yes. Do we need to sign anything else?

[awong, 2010/08/03 19:21:02]

Don't think so, but I was expecting to see something that signs the a while XML
node, not just the parsed data from it.  This is fine for now, but can we add a
TODO + file a bug to consider signing the XML node itself?  Or do you think I'm
being too paranoid?

[Sergey Ulanov, 2010/08/04 01:41:12]

Yeah, that was my firth thought too, but the problem is that the JID is not in
the body of the request, it is in the envelope, i.e. the whole message looks as
follows:
 <iq from="jid" ...>
  <heartbeat>
   <signature>.signature.</signature>
  </heartbeat>
 </iq>

This particular code generates the stuff inside of <iq>, and the <iq> is
generated by IqRequset. Besides, we just need to confirm identity of the host
and signing jid is enough for that, we can trust the GTalk network that the
message is arrived the way it was sent, so no need to sign other fields.

+  std::string signature(key_pair_->GetSignature(message));
+  signature_tag->AddText(signature);
+
+  return signature_tag;
+}
+
 }  // namespace remoting