From: Sage Weil <sage@redhat.com>
Date: Thu, 22 Jan 2015 23:40:23 +0000 (-0800)
Subject: auth/cephx: move signature calc into helper
X-Git-Tag: v0.94.7~41^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=51eaf98e25287705c9c25d2d3a5df248ba066f2d;p=ceph.git

auth/cephx: move signature calc into helper

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 4e14a5fedbfacd164639f6a59a4d152404f3f9d1)

Conflicts:
	src/auth/cephx/CephxSessionHandler.cc (trivial, const changes)
---

diff --git a/src/auth/cephx/CephxSessionHandler.cc b/src/auth/cephx/CephxSessionHandler.cc
index b2d402d2af3..d83125f637a 100644
--- a/src/auth/cephx/CephxSessionHandler.cc
+++ b/src/auth/cephx/CephxSessionHandler.cc
@@ -24,47 +24,56 @@
 
 #define dout_subsys ceph_subsys_auth
 
-int CephxSessionHandler::sign_message(Message *m)
+int CephxSessionHandler::_calc_signature(Message *m, uint64_t *psig)
 {
-  // If runtime signing option is off, just return success without signing.
-  if (!cct->_conf->cephx_sign_messages) {
-    return 0;
-  }
-  bufferlist bl_plaintext, bl_encrypted;
-  ceph_msg_header header = m->get_header();
-  std::string error;
-
-  ceph_msg_footer& en_footer = m->get_footer();
+  const ceph_msg_header& header = m->get_header();
+  const ceph_msg_footer& footer = m->get_footer();
 
+  bufferlist bl_plaintext;
   ::encode(header.crc, bl_plaintext);
-  ::encode(en_footer.front_crc, bl_plaintext);
-  ::encode(en_footer.middle_crc, bl_plaintext);
-  ::encode(en_footer.data_crc, bl_plaintext);
-
-  ldout(cct, 10) <<  "sign_message: seq # " << header.seq << " CRCs are: header " << header.crc
-		 << " front " << en_footer.front_crc << " middle " << en_footer.middle_crc
-		 << " data " << en_footer.data_crc << dendl;
+  ::encode(footer.front_crc, bl_plaintext);
+  ::encode(footer.middle_crc, bl_plaintext);
+  ::encode(footer.data_crc, bl_plaintext);
 
+  bufferlist bl_encrypted;
+  std::string error;
   if (encode_encrypt(cct, bl_plaintext, key, bl_encrypted, error)) {
     ldout(cct, 0) << "error encrypting message signature: " << error << dendl;
     ldout(cct, 0) << "no signature put on message" << dendl;
     return SESSION_SIGNATURE_FAILURE;
-  } 
+  }
 
   bufferlist::iterator ci = bl_encrypted.begin();
   // Skip the magic number up front. PLR
   ci.advance(4);
-  ::decode(en_footer.sig, ci);
+  ::decode(*psig, ci);
+  ldout(cct, 10) << __func__ << " seq " << m->get_seq()
+		 << " front_crc_ = " << footer.front_crc
+		 << " middle_crc = " << footer.middle_crc
+		 << " data_crc = " << footer.data_crc
+		 << " sig = " << *psig
+		 << dendl;
+  return 0;
+}
 
-  // There's potentially an issue with whether the encoding and decoding done here will work
-  // properly when a big endian and little endian machine are talking.  We think it's OK,
-  // but it should be tested to be sure.  PLR
+int CephxSessionHandler::sign_message(Message *m)
+{
+  // If runtime signing option is off, just return success without signing.
+  if (!cct->_conf->cephx_sign_messages) {
+    return 0;
+  }
 
-  // Receiver won't trust this flag to decide if msg should have been signed.  It's primarily
-  // to debug problems where sender and receiver disagree on need to sign msg.  PLR
-  en_footer.flags = (unsigned)en_footer.flags | CEPH_MSG_FOOTER_SIGNED;
+  uint64_t sig;
+  int r = _calc_signature(m, &sig);
+  if (r < 0)
+    return r;
+
+  ceph_msg_footer& f = m->get_footer();
+  f.sig = sig;
+  f.flags = (unsigned)f.flags | CEPH_MSG_FOOTER_SIGNED;
   messages_signed++;
-  ldout(cct, 20) << "Putting signature in client message(seq # " << header.seq << "): sig = " << en_footer.sig << dendl;
+  ldout(cct, 20) << "Putting signature in client message(seq # " << m->get_seq()
+		 << "): sig = " << sig << dendl;
   return 0;
 }
 
@@ -74,57 +83,34 @@ int CephxSessionHandler::check_message_signature(Message *m)
   if (!cct->_conf->cephx_sign_messages) {
     return 0;
   }
-
-  bufferlist bl_plaintext, bl_ciphertext;
-  std::string sig_error;
-  ceph_msg_header& header = m->get_header();
-  ceph_msg_footer& footer = m->get_footer();
-
   if ((features & CEPH_FEATURE_MSG_AUTH) == 0) {
     // it's fine, we didn't negotiate this feature.
     return 0;
   }
 
-  signatures_checked++;
+  uint64_t sig;
+  int r = _calc_signature(m, &sig);
+  if (r < 0)
+    return r;
 
-  ldout(cct, 10) << "check_message_signature: seq # = " << m->get_seq() << " front_crc_ = " << footer.front_crc
-		 << " middle_crc = " << footer.middle_crc << " data_crc = " << footer.data_crc << dendl;
-  ::encode(header.crc, bl_plaintext);
-  ::encode(footer.front_crc, bl_plaintext);
-  ::encode(footer.middle_crc, bl_plaintext);
-  ::encode(footer.data_crc, bl_plaintext);
-
-  // Encrypt the buffer containing the checksums to calculate the signature. PLR
-  if (encode_encrypt(cct, bl_plaintext, key, bl_ciphertext, sig_error)) {
-    ldout(cct, 0) << "error in encryption for checking message signature: " << sig_error << dendl;
-    return (SESSION_SIGNATURE_FAILURE);
-  } 
-
-  bufferlist::iterator ci = bl_ciphertext.begin();
-  // Skip the magic number at the front. PLR
-  ci.advance(4);
-  uint64_t sig_check;
-  ::decode(sig_check, ci);
-
-  // There's potentially an issue with whether the encoding and decoding done here will work
-  // properly when a big endian and little endian machine are talking.  We think it's OK,
-  // but it should be tested to be sure.  PLR
+  signatures_checked++;
 
-  if (sig_check != footer.sig) {
+  if (sig != m->get_footer().sig) {
     // Should have been signed, but signature check failed.  PLR
-    if (!(footer.flags & CEPH_MSG_FOOTER_SIGNED)) {
-      ldout(cct, 0) << "SIGN: MSG " << header.seq << " Sender did not set CEPH_MSG_FOOTER_SIGNED." << dendl;
+    if (!(m->get_footer().flags & CEPH_MSG_FOOTER_SIGNED)) {
+      ldout(cct, 0) << "SIGN: MSG " << m->get_seq() << " Sender did not set CEPH_MSG_FOOTER_SIGNED." << dendl;
     }
-    ldout(cct, 0) << "SIGN: MSG " << header.seq << " Message signature does not match contents." << dendl;
-    ldout(cct, 0) << "SIGN: MSG " << header.seq << "Signature on message:" << dendl;
-    ldout(cct, 0) << "SIGN: MSG " << header.seq << "    sig: " << footer.sig << dendl;
-    ldout(cct, 0) << "SIGN: MSG " << header.seq << "Locally calculated signature:" << dendl;
-    ldout(cct, 0) << "SIGN: MSG " << header.seq << "    sig_check:" << sig_check << dendl;
-
-    // For the moment, printing an error message to the log and returning failure is sufficient.
-    // In the long term, we should probably have code parsing the log looking for this kind
-    // of security failure, particularly when there are large numbers of them, since the latter
-    // is a potential sign of an attack.  PLR
+    ldout(cct, 0) << "SIGN: MSG " << m->get_seq() << " Message signature does not match contents." << dendl;
+    ldout(cct, 0) << "SIGN: MSG " << m->get_seq() << "Signature on message:" << dendl;
+    ldout(cct, 0) << "SIGN: MSG " << m->get_seq() << "    sig: " << m->get_footer().sig << dendl;
+    ldout(cct, 0) << "SIGN: MSG " << m->get_seq() << "Locally calculated signature:" << dendl;
+    ldout(cct, 0) << "SIGN: MSG " << m->get_seq() << "    sig_check:" << sig << dendl;
+
+    // For the moment, printing an error message to the log and
+    // returning failure is sufficient.  In the long term, we should
+    // probably have code parsing the log looking for this kind of
+    // security failure, particularly when there are large numbers of
+    // them, since the latter is a potential sign of an attack.  PLR
 
     signatures_failed++;
     ldout(cct, 0) << "Signature failed." << dendl;
diff --git a/src/auth/cephx/CephxSessionHandler.h b/src/auth/cephx/CephxSessionHandler.h
index 52a112e29a2..7b46e076b8a 100644
--- a/src/auth/cephx/CephxSessionHandler.h
+++ b/src/auth/cephx/CephxSessionHandler.h
@@ -31,8 +31,9 @@ public:
     return false;
   }
 
-  int sign_message(Message *m);
+  int _calc_signature(Message *m, uint64_t *psig);
 
+  int sign_message(Message *m);
   int check_message_signature(Message *m) ;
 
   // Cephx does not currently encrypt messages, so just return 0 if called.  PLR