Various cleanup changes to session authentication code.

Signed-off-by: Peter Reiher <reiher@inktank.com>

diff --git a/src/auth/AuthSessionHandler.cc b/src/auth/AuthSessionHandler.cc
index 210a0832db234805364257fff2e933f340c71b2c..cc60b1826073183e9604692b62591b47d8441ad5 100644 (file)
--- a/src/auth/AuthSessionHandler.cc
+++ b/src/auth/AuthSessionHandler.cc
@@ -27,7 +27,7 @@ AuthSessionHandler *get_auth_session_handler(CephContext *cct, int protocol, Cry
 
   // Should add code to only print the SHA1 hash of the key, unless in secure debugging mode
 
-  ldout(cct,10) << "In get_auth_session_handler for protocol " << protocol << "and key " << key << dendl;
+  ldout(cct,10) << "In get_auth_session_handler for protocol " << protocol << dendl;
  
   switch (protocol) {
   case CEPH_AUTH_CEPHX:
@@ -41,7 +41,7 @@ AuthSessionHandler *get_auth_session_handler(CephContext *cct, int protocol, Cry
 }
 
 
-void AuthSessionHandler::printAuthSessionHandlerStats() {
+void AuthSessionHandler::print_auth_session_handler_stats() {
   ldout(cct,10) << "Auth Session Handler Stats " << this << dendl;
   ldout(cct,10) << "    Messages Signed    = " << messages_signed << dendl;
   ldout(cct,10) << "    Signatures Checked = " << signatures_checked << dendl;

diff --git a/src/auth/AuthSessionHandler.h b/src/auth/AuthSessionHandler.h
index 53a9c6b7e188f7d598bb0571133272a296e5ad92..108d6e4dfedbee83533a9c006b15ed853ee32bd9 100644 (file)
--- a/src/auth/AuthSessionHandler.h
+++ b/src/auth/AuthSessionHandler.h
@@ -44,14 +44,17 @@ public:
   int messages_encrypted;
   int messages_decrypted;
 
-  AuthSessionHandler(CephContext *cct_) : cct(cct_), messages_signed(0), signatures_checked(0), signatures_matched(0), signatures_failed(0), messages_encrypted(0), messages_decrypted(0) {}
+  AuthSessionHandler(CephContext *cct_) : cct(cct_), messages_signed(0), signatures_checked(0), 
+    signatures_matched(0), signatures_failed(0), messages_encrypted(0), messages_decrypted(0) {}
 
-  AuthSessionHandler(CephContext *cct_, int protocol_, CryptoKey key_) : cct(cct_), protocol(protocol_), key(key_), messages_signed(0), signatures_checked(0), signatures_matched(0), signatures_failed(0), messages_encrypted(0), messages_decrypted(0) {}
+  AuthSessionHandler(CephContext *cct_, int protocol_, CryptoKey key_) : cct(cct_), 
+    protocol(protocol_), key(key_), messages_signed(0), signatures_checked(0), signatures_matched(0), 
+    signatures_failed(0), messages_encrypted(0), messages_decrypted(0) {}
   virtual ~AuthSessionHandler() { }
 
-  void printAuthSessionHandlerStats() ;
+  void print_auth_session_handler_stats() ;
 
-  virtual int no_security() = 0;
+  virtual bool no_security() = 0;
   virtual int sign_message(Message *message) = 0;
   virtual int check_message_signature(Message *message) = 0;
   virtual int encrypt_message(Message *message) = 0;

diff --git a/src/auth/cephx/CephxClientHandler.cc b/src/auth/cephx/CephxClientHandler.cc
index afad79ce7f3f2c4da528b0f1e588d9477d817b45..2818b7a679abf8c2274ae7e94b34e6040c3532fa 100644 (file)
--- a/src/auth/cephx/CephxClientHandler.cc
+++ b/src/auth/cephx/CephxClientHandler.cc
@@ -156,13 +156,12 @@ int CephxClientHandler::handle_response(int ret, bufferlist::iterator& indata)
        CryptoKey secret_key;
        keyring->get_secret(cct->_conf->name, secret_key);
        std::string error;
-       decode_decrypt(cct, secrets, secret_key, indata, error);
-       if (error.empty()) {
-         rotating_secrets->set_secrets(secrets);
-       } else {
+       if (decode_decrypt(cct, secrets, secret_key, indata, error)) {
          ldout(cct, 0) << "could not set rotating key: decode_decrypt failed. error:"
            << error << dendl;
          error.clear();
+       } else {
+         rotating_secrets->set_secrets(secrets);
        }
       }
     }

diff --git a/src/auth/cephx/CephxKeyServer.cc b/src/auth/cephx/CephxKeyServer.cc
index b82de9dbbf91e9fcfb5b397e283f3f0c40920c63..1440b2c2b9f1cc72320aa93e7d3ed6d98177d955 100644 (file)
--- a/src/auth/cephx/CephxKeyServer.cc
+++ b/src/auth/cephx/CephxKeyServer.cc
@@ -359,8 +359,7 @@ bool KeyServer::get_rotating_encrypted(const EntityName& name,
   RotatingSecrets secrets = rotate_iter->second;
 
   std::string error;
-  encode_encrypt(cct, secrets, specific_key, enc_bl, error);
-  if (!error.empty())
+  if (encode_encrypt(cct, secrets, specific_key, enc_bl, error))
     return false;
 
   return true;

diff --git a/src/auth/cephx/CephxProtocol.cc b/src/auth/cephx/CephxProtocol.cc
index 937ad0bc0a54972071823519154fa88371d7fb91..9c262634e7be1bbcad04c94400deb4cedc505be2 100644 (file)
--- a/src/auth/cephx/CephxProtocol.cc
+++ b/src/auth/cephx/CephxProtocol.cc
@@ -33,8 +33,7 @@ void cephx_calc_client_server_challenge(CephContext *cct, CryptoKey& secret, uin
 
   bufferlist enc;
   std::string error;
-  encode_encrypt(cct, b, secret, enc, error);
-  if (!error.empty())
+  if (encode_encrypt(cct, b, secret, enc, error))
     return;
 
   uint64_t k = 0;
@@ -107,8 +106,7 @@ bool cephx_build_service_ticket_reply(CephContext *cct,
     msg_a.session_key = info.session_key;
     msg_a.validity = info.validity;
     std::string error;
-    encode_encrypt(cct, msg_a, principal_secret, reply, error);
-    if (!error.empty()) {
+    if (encode_encrypt(cct, msg_a, principal_secret, reply, error)) {
       ldout(cct, -1) << "error encoding encrypted: " << error << dendl;
       return false;
     }
@@ -126,8 +124,7 @@ bool cephx_build_service_ticket_reply(CephContext *cct,
 
     ::encode((__u8)should_encrypt_ticket, reply);
     if (should_encrypt_ticket) {
-      encode_encrypt(cct, service_ticket_bl, ticket_enc_key, reply, error);
-      if (!error.empty()) {
+      if (encode_encrypt(cct, service_ticket_bl, ticket_enc_key, reply, error)) {
        ldout(cct, -1) << "error encoding encrypted ticket: " << error << dendl;
         return false;
       }
@@ -150,10 +147,8 @@ bool CephXTicketHandler::verify_service_ticket_reply(CryptoKey& secret,
 
   CephXServiceTicket msg_a;
   std::string error;
-  decode_decrypt(cct, msg_a, secret, indata, error);
-  if (!error.empty()) {
-    ldout(cct, 0) << "verify_service_ticket_reply: failed decode_decrypt with secret "
-           << secret << ": " << error << dendl;
+  if (decode_decrypt(cct, msg_a, secret, indata, error)) {
+    ldout(cct, 0) << "verify_service_ticket_reply: failed decode_decrypt, error is: " << error << dendl;
     return false;
   }
   
@@ -164,8 +159,7 @@ bool CephXTicketHandler::verify_service_ticket_reply(CryptoKey& secret,
   if (ticket_enc) {
     ldout(cct, 10) << " got encrypted ticket" << dendl;
     std::string error;
-    decode_decrypt(cct, service_ticket_bl, session_key, indata, error);
-    if (!error.empty()) {
+    if (decode_decrypt(cct, service_ticket_bl, session_key, indata, error)) {
       ldout(cct, 10) << "verify_service_ticket_reply: decode_decrypt failed "
            << "with " << error << dendl;
       return false;
@@ -304,7 +298,6 @@ CephXAuthorizer *CephXTicketHandler::build_authorizer(uint64_t global_id)
 
   __u8 authorizer_v = 1;
   ::encode(authorizer_v, a->bl);
-
   ::encode(global_id, a->bl);
   ::encode(service_id, a->bl);
 
@@ -314,8 +307,7 @@ CephXAuthorizer *CephXTicketHandler::build_authorizer(uint64_t global_id)
   msg.nonce = a->nonce;
 
   std::string error;
-  encode_encrypt(cct, msg, session_key, a->bl, error);
-  if (!error.empty()) {
+  if (encode_encrypt(cct, msg, session_key, a->bl, error)) {
     ldout(cct, 0) << "failed to encrypt authorizer: " << error << dendl;
     delete a;
     return 0;
@@ -452,9 +444,8 @@ bool cephx_verify_authorizer(CephContext *cct, KeyStore *keys,
 
   // CephXAuthorize
   CephXAuthorize auth_msg;
-  decode_decrypt(cct, auth_msg, ticket_info.session_key, indata, error);
-  if (!error.empty()) {
-    ldout(cct, 0) << "verify_authorizercould not decrypt authorize request: error: "
+  if (decode_decrypt(cct, auth_msg, ticket_info.session_key, indata, error)) {
+    ldout(cct, 0) << "verify_authorizercould not decrypt authorize request with error: "
       << error << dendl;
     return false;
   }
@@ -466,8 +457,7 @@ bool cephx_verify_authorizer(CephContext *cct, KeyStore *keys,
   CephXAuthorizeReply reply;
   // reply.trans_id = auth_msg.trans_id;
   reply.nonce_plus_one = auth_msg.nonce + 1;
-  encode_encrypt(cct, reply, ticket_info.session_key, reply_bl, error);
-  if (!error.empty()) {
+  if (encode_encrypt(cct, reply, ticket_info.session_key, reply_bl, error)) {
     ldout(cct, 10) << "verify_authorizer: encode_encrypt error: " << error << dendl;
     return false;
   }
@@ -481,17 +471,10 @@ bool CephXAuthorizer::verify_reply(bufferlist::iterator& indata)
 {
   CephXAuthorizeReply reply;
 
-  try {
-    std::string error;
-    decode_decrypt(cct, reply, session_key, indata, error);
-    if (!error.empty()) {
-      ldout(cct, 0) << "verify_authorizer_reply coudln't decrypt with " << session_key 
-          << ": error: " << error << dendl;
+  std::string error;
+  if (decode_decrypt(cct, reply, session_key, indata, error)) {
+      ldout(cct, 0) << "verify_reply coudln't decrypt with error: " << error << dendl;
       return false;
-    }
-  } catch (const buffer::error &e) {
-    ldout(cct, 0) << "verify_authorizer_reply exception in decode_decrypt with " << session_key << dendl;
-    return false;
   }
 
   uint64_t expect = nonce + 1;

diff --git a/src/auth/cephx/CephxProtocol.h b/src/auth/cephx/CephxProtocol.h
index ff2717372bca7f20083198b53d2a25d3c471a806..dfa2b2f0896f8d03aacf45ec7e70405b80ed7ddd 100644 (file)
--- a/src/auth/cephx/CephxProtocol.h
+++ b/src/auth/cephx/CephxProtocol.h
@@ -81,6 +81,7 @@
 #define CEPHX_GET_ROTATING_KEY          0x0400
 
 #define CEPHX_REQUEST_TYPE_MASK            0x0F00
+#define CEPHX_CRYPT_ERR                        1
 
 #include "../Auth.h"
 #include "../RotatingKeyRing.h"
@@ -465,23 +466,28 @@ void encode_encrypt_enc_bl(CephContext *cct, const T& t, const CryptoKey& key,
 }
 
 template <typename T>
-void decode_decrypt(CephContext *cct, T& t, const CryptoKey key,
+int decode_decrypt(CephContext *cct, T& t, const CryptoKey key,
                    bufferlist::iterator& iter, std::string &error)
 {
   bufferlist bl_enc;
   ::decode(bl_enc, iter);
   decode_decrypt_enc_bl(cct, t, key, bl_enc, error);
+  if (!error.empty())
+    return CEPHX_CRYPT_ERR;
+  return 0;
 }
 
 template <typename T>
-void encode_encrypt(CephContext *cct, const T& t, const CryptoKey& key,
+int encode_encrypt(CephContext *cct, const T& t, const CryptoKey& key,
                    bufferlist& out, std::string &error)
 {
   bufferlist bl_enc;
   encode_encrypt_enc_bl(cct, t, key, bl_enc, error);
-  if (!error.empty())
-    return;
+  if (!error.empty()){
+    return CEPHX_CRYPT_ERR;
+  }
   ::encode(bl_enc, out);
+  return 0;
 }
 
 

diff --git a/src/auth/cephx/CephxSessionHandler.cc b/src/auth/cephx/CephxSessionHandler.cc
index 7e90df45b8b6ca9f2aaff77067a5109d87652087..45ea433abb901e384b6d088a2400d1a88bbed8c2 100644 (file)
--- a/src/auth/cephx/CephxSessionHandler.cc
+++ b/src/auth/cephx/CephxSessionHandler.cc
@@ -45,8 +45,7 @@ int CephxSessionHandler::sign_message(Message *m)
                 << " front " << en_footer.front_crc << " middle " << en_footer.middle_crc
                 << " data " << en_footer.data_crc << dendl;
 
-  encode_encrypt(cct, bl_plaintext, key, bl_encrypted, error);
-  if (!error.empty()) {
+  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;
@@ -91,8 +90,7 @@ int CephxSessionHandler::check_message_signature(Message *m)
   ::encode(footer.data_crc, bl_plaintext);
 
   // Encrypt the buffer containing the checksums to calculate the signature. PLR
-  encode_encrypt(cct, bl_plaintext, key, bl_ciphertext, sig_error);
-  if (!sig_error.empty()) {
+  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);
   } 

diff --git a/src/auth/cephx/CephxSessionHandler.h b/src/auth/cephx/CephxSessionHandler.h
index bd4ba7e497724bc0e9de60cd32b4f71aace0a239..8bca05d8f031d1baa4edafcf70843a97e49ca69c 100644 (file)
--- a/src/auth/cephx/CephxSessionHandler.h
+++ b/src/auth/cephx/CephxSessionHandler.h
@@ -24,8 +24,8 @@ public:
     : AuthSessionHandler(cct_, CEPH_AUTH_CEPHX, session_key) {}
   ~CephxSessionHandler() {}
   
-  int no_security() {
-    return 0;
+  bool no_security() {
+    return false;
   }
 
   int sign_message(Message *m);

diff --git a/src/auth/none/AuthNoneSessionHandler.h b/src/auth/none/AuthNoneSessionHandler.h
index 9d73bda1c2ca31692eeea1c1d5bba376686bb62c..85261bf0fc1937d656bebe9da1ce4638d7941543 100644 (file)
--- a/src/auth/none/AuthNoneSessionHandler.h
+++ b/src/auth/none/AuthNoneSessionHandler.h
@@ -25,8 +25,8 @@ public:
     : AuthSessionHandler(cct_, CEPH_AUTH_NONE, session_key) {}
   ~AuthNoneSessionHandler() {}
   
-  int no_security() {
-    return 1;
+  bool no_security() {
+    return true;
   }
 
   // The None suite neither signs nor encrypts messages, so these functions just return success.

diff --git a/src/auth/unknown/AuthUnknownSessionHandler.h b/src/auth/unknown/AuthUnknownSessionHandler.h
index 2563c9389f084d3bdd15295a35055d7dc1bbaba6..4f811ab52f1603ed6ab7f90f6c72611c97a3e50d 100644 (file)
--- a/src/auth/unknown/AuthUnknownSessionHandler.h
+++ b/src/auth/unknown/AuthUnknownSessionHandler.h
@@ -25,8 +25,8 @@ public:
     : AuthSessionHandler(cct_, CEPH_AUTH_UNKNOWN, session_key) {}
   ~AuthUnknownSessionHandler() {}
   
-  int no_security() {
-    return 1;
+  bool no_security() {
+    return true;
   }
 
   // The Unknown suite neither signs nor encrypts messages, so these functions just return success.

diff --git a/src/msg/Pipe.cc b/src/msg/Pipe.cc
index 8fc640dd701adf0665395cd8839932ce26d4120a..abbb7f0fbd1b04e9f90580e5c7068c86f5b0688b 100644 (file)
--- a/src/msg/Pipe.cc
+++ b/src/msg/Pipe.cc
@@ -74,7 +74,10 @@ Pipe::Pipe(SimpleMessenger *r, int st, Connection *con)
     connection_state->pipe = get();
   }
 
-  randomize_out_seq();
+  if (randomize_out_seq()) {
+    lsubdout(msgr->cct,ms,15) << "Pipe(): Could not get random bytes to set seq number for session reset; set seq number to " << out_seq << dendl;
+  }
+    
 
   msgr->timeout = msgr->cct->_conf->ms_tcp_read_timeout * 1000; //convert to ms
   if (msgr->timeout == 0)
@@ -1091,16 +1094,17 @@ void Pipe::fault(bool onread)
   }
 }
 
-void Pipe::randomize_out_seq()
+int Pipe::randomize_out_seq()
 {
-  // Set out_seq to a random value, so CRC won't be predictable PLR
+  // Set out_seq to a random value, so CRC won't be predictable.   Don't bother checking seq_error 
+  // here.  We'll check it on the call.  PLR
+
   int seq_error = get_random_bytes((char *)&out_seq, sizeof(out_seq));
-  if (seq_error < 0) {
-    lsubdout(msgr->cct,ms,15) << "Could not get random bytes to set seq number for session reset; setting seq number to 0." << dendl;
-    throw "was_session_reset(): get_random_bytes failed.";
-  }
+
+
   out_seq &= SEQ_MASK;
   lsubdout(msgr->cct, ms, 10) << "randomize_out_seq " << out_seq << dendl;
+  return seq_error;
 }
 
 void Pipe::was_session_reset()
@@ -1113,7 +1117,9 @@ void Pipe::was_session_reset()
 
   msgr->dispatch_queue.queue_remote_reset(connection_state);
 
-  randomize_out_seq();
+  if (randomize_out_seq()) {
+    lsubdout(msgr->cct,ms,15) << "was_session_reset(): Could not get random bytes to set seq number for session reset; set seq number to " << out_seq << dendl;
+  }
 
   in_seq = 0;
   connect_seq = 0;

diff --git a/src/msg/Pipe.h b/src/msg/Pipe.h
index 343ab4a1d5351ad015d72ea6bb41207f8599593a..748525a3f5ebe1b61b311fb85a33f5bd929887c2 100644 (file)
--- a/src/msg/Pipe.h
+++ b/src/msg/Pipe.h
@@ -133,7 +133,7 @@ class DispatchQueue;
     void writer();
     void unlock_maybe_reap();
 
-    void randomize_out_seq();
+    int randomize_out_seq();
 
     int read_message(Message **pm);
     int write_message(Message *m);