From 951da2fbfa1639c881e46ce211db6fa762e61313 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 23 Jan 2019 10:14:16 -0600 Subject: [PATCH] auth: make connection_secret a std::string Move connection mode decision to initial auth_request point so that it can inform auth implementation how big the connection secret should be. Pass that value through where appropriate. The connection_secret is now a std::string filled with random bytes. For now the v2 protocol just uses the session_key CryptoKey to encrypt, but this is about to change. Signed-off-by: Sage Weil --- src/auth/Auth.h | 19 +++++--- src/auth/AuthAuthorizeHandler.h | 3 +- src/auth/AuthClient.h | 2 +- src/auth/AuthClientHandler.h | 2 +- src/auth/AuthServiceHandler.h | 6 ++- src/auth/AuthSessionHandler.cc | 2 +- src/auth/AuthSessionHandler.h | 8 ++-- src/auth/cephx/CephxAuthorizeHandler.cc | 7 ++- src/auth/cephx/CephxAuthorizeHandler.h | 3 +- src/auth/cephx/CephxClientHandler.cc | 5 ++- src/auth/cephx/CephxClientHandler.h | 2 +- src/auth/cephx/CephxProtocol.cc | 18 ++++---- src/auth/cephx/CephxProtocol.h | 12 ++--- src/auth/cephx/CephxServiceHandler.cc | 33 +++++++++----- src/auth/cephx/CephxServiceHandler.h | 6 ++- src/auth/cephx/CephxSessionHandler.cc | 6 ++- src/auth/cephx/CephxSessionHandler.h | 2 +- src/auth/krb/KrbAuthorizeHandler.cpp | 3 +- src/auth/krb/KrbAuthorizeHandler.hpp | 3 +- src/auth/krb/KrbClientHandler.cpp | 2 +- src/auth/krb/KrbClientHandler.hpp | 2 +- src/auth/krb/KrbProtocol.hpp | 2 +- src/auth/krb/KrbServiceHandler.cpp | 6 ++- src/auth/krb/KrbServiceHandler.hpp | 10 +++-- src/auth/krb/KrbSessionHandler.hpp | 2 +- src/auth/none/AuthNoneAuthorizeHandler.cc | 3 +- src/auth/none/AuthNoneAuthorizeHandler.h | 3 +- src/auth/none/AuthNoneClientHandler.h | 2 +- src/auth/none/AuthNoneProtocol.h | 2 +- src/auth/none/AuthNoneServiceHandler.h | 6 ++- src/auth/none/AuthNoneSessionHandler.h | 2 +- .../unknown/AuthUnknownAuthorizeHandler.cc | 3 +- .../unknown/AuthUnknownAuthorizeHandler.h | 3 +- src/auth/unknown/AuthUnknownClientHandler.h | 2 +- src/auth/unknown/AuthUnknownServiceHandler.h | 6 ++- src/auth/unknown/AuthUnknownSessionHandler.h | 2 +- src/crimson/net/SocketConnection.cc | 2 +- src/mon/AuthMonitor.cc | 5 ++- src/mon/MonClient.cc | 7 +-- src/mon/MonClient.h | 4 +- src/mon/Monitor.cc | 28 +++++++----- src/mon/Monitor.h | 2 +- src/msg/Messenger.cc | 3 +- src/msg/Messenger.h | 5 ++- src/msg/async/ProtocolV1.cc | 4 +- src/msg/async/ProtocolV2.cc | 45 +++++++++---------- src/msg/simple/Pipe.cc | 7 +-- src/msg/xio/XioConnection.cc | 1 + 48 files changed, 187 insertions(+), 126 deletions(-) diff --git a/src/auth/Auth.h b/src/auth/Auth.h index 2716a3ddb40..5e8e375fdf3 100644 --- a/src/auth/Auth.h +++ b/src/auth/Auth.h @@ -146,7 +146,7 @@ struct AuthAuthorizer { explicit AuthAuthorizer(__u32 p) : protocol(p) {} virtual ~AuthAuthorizer() {} virtual bool verify_reply(bufferlist::const_iterator& reply, - CryptoKey *connection_secret) = 0; + std::string *connection_secret) = 0; virtual bool add_challenge(CephContext *cct, const bufferlist& challenge) = 0; }; @@ -162,9 +162,6 @@ struct AuthConnectionMeta { int auth_mode = 0; ///< AUTH_MODE_* - /// server: client's preferred con_modes - std::vector preferred_con_modes; - int con_mode = 0; ///< negotiated mode bool is_mode_crc() { @@ -174,8 +171,18 @@ struct AuthConnectionMeta { return con_mode == CEPH_CON_MODE_SECURE; } - CryptoKey session_key; ///< per-ticket key - CryptoKey connection_secret; ///< per-connection key + CryptoKey session_key; ///< per-ticket key + + size_t get_connection_secret_length() const { + switch (con_mode) { + case CEPH_CON_MODE_CRC: + return 0; + case CEPH_CON_MODE_SECURE: + return 16 * 4; + } + return 0; + } + std::string connection_secret; ///< per-connection key std::unique_ptr authorizer; std::unique_ptr authorizer_challenge; diff --git a/src/auth/AuthAuthorizeHandler.h b/src/auth/AuthAuthorizeHandler.h index 74703407dc9..b912e578995 100644 --- a/src/auth/AuthAuthorizeHandler.h +++ b/src/auth/AuthAuthorizeHandler.h @@ -33,12 +33,13 @@ struct AuthAuthorizeHandler { CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) = 0; virtual int authorizer_session_crypto() = 0; }; diff --git a/src/auth/AuthClient.h b/src/auth/AuthClient.h index 28eae6decce..79912a3188b 100644 --- a/src/auth/AuthClient.h +++ b/src/auth/AuthClient.h @@ -27,7 +27,7 @@ public: uint32_t con_mode, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_key) = 0; + std::string *connection_secret) = 0; virtual int handle_auth_bad_method( Connection *con, uint32_t old_auth_method, diff --git a/src/auth/AuthClientHandler.h b/src/auth/AuthClientHandler.h index 1e8f76eb4f3..e0f3a7de952 100644 --- a/src/auth/AuthClientHandler.h +++ b/src/auth/AuthClientHandler.h @@ -54,7 +54,7 @@ public: virtual int build_request(bufferlist& bl) const = 0; virtual int handle_response(int ret, bufferlist::const_iterator& iter, CryptoKey *session_key, - CryptoKey *connection_secret) = 0; + std::string *connection_secret) = 0; virtual bool build_rotating_request(bufferlist& bl) const = 0; virtual AuthAuthorizer *build_authorizer(uint32_t service_id) const = 0; diff --git a/src/auth/AuthServiceHandler.h b/src/auth/AuthServiceHandler.h index a787663c299..9f695bfdf97 100644 --- a/src/auth/AuthServiceHandler.h +++ b/src/auth/AuthServiceHandler.h @@ -37,16 +37,18 @@ public: virtual ~AuthServiceHandler() { } virtual int start_session(const EntityName& name, + size_t connection_secret_required_length, bufferlist *result, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) = 0; + std::string *connection_secret) = 0; virtual int handle_request(bufferlist::const_iterator& indata, + size_t connection_secret_required_length, bufferlist *result, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) = 0; + std::string *connection_secret) = 0; EntityName& get_entity_name() { return entity_name; } }; diff --git a/src/auth/AuthSessionHandler.cc b/src/auth/AuthSessionHandler.cc index 3e92051455a..54dbdb79598 100644 --- a/src/auth/AuthSessionHandler.cc +++ b/src/auth/AuthSessionHandler.cc @@ -27,7 +27,7 @@ AuthSessionHandler *get_auth_session_handler( CephContext *cct, int protocol, const CryptoKey& key, - const CryptoKey& connection_secret, + const std::string& connection_secret, uint64_t features) { diff --git a/src/auth/AuthSessionHandler.h b/src/auth/AuthSessionHandler.h index 89015c839f8..28f139d45ec 100644 --- a/src/auth/AuthSessionHandler.h +++ b/src/auth/AuthSessionHandler.h @@ -30,15 +30,15 @@ struct AuthSessionHandler { protected: CephContext *cct; int protocol; - CryptoKey key; // per mon authentication - CryptoKey connection_secret; // per connection + CryptoKey key; // per mon authentication + std::string connection_secret; // per connection public: explicit AuthSessionHandler(CephContext *cct_) : cct(cct_), protocol(CEPH_AUTH_UNKNOWN) {} AuthSessionHandler(CephContext *cct_, int protocol_, const CryptoKey& key_, - const CryptoKey& cs_) + const std::string& cs_) : cct(cct_), protocol(protocol_), key(key_), @@ -69,7 +69,7 @@ public: extern AuthSessionHandler *get_auth_session_handler( CephContext *cct, int protocol, const CryptoKey& key, - const CryptoKey& connection_secret, + const std::string& connection_secret, uint64_t features); #endif diff --git a/src/auth/cephx/CephxAuthorizeHandler.cc b/src/auth/cephx/CephxAuthorizeHandler.cc index 86003dae663..6684e164728 100644 --- a/src/auth/cephx/CephxAuthorizeHandler.cc +++ b/src/auth/cephx/CephxAuthorizeHandler.cc @@ -10,12 +10,13 @@ bool CephxAuthorizeHandler::verify_authorizer( CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) { auto iter = authorizer_data.cbegin(); @@ -27,7 +28,9 @@ bool CephxAuthorizeHandler::verify_authorizer( CephXServiceTicketInfo auth_ticket_info; - bool isvalid = cephx_verify_authorizer(cct, keys, iter, auth_ticket_info, + bool isvalid = cephx_verify_authorizer(cct, keys, iter, + connection_secret_required_len, + auth_ticket_info, challenge, connection_secret, authorizer_reply); diff --git a/src/auth/cephx/CephxAuthorizeHandler.h b/src/auth/cephx/CephxAuthorizeHandler.h index 6784fa2dae9..769426c4384 100644 --- a/src/auth/cephx/CephxAuthorizeHandler.h +++ b/src/auth/cephx/CephxAuthorizeHandler.h @@ -24,12 +24,13 @@ struct CephxAuthorizeHandler : public AuthAuthorizeHandler { CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) override; int authorizer_session_crypto() override; }; diff --git a/src/auth/cephx/CephxClientHandler.cc b/src/auth/cephx/CephxClientHandler.cc index 9d0df43a8c4..77d8e687e41 100644 --- a/src/auth/cephx/CephxClientHandler.cc +++ b/src/auth/cephx/CephxClientHandler.cc @@ -119,7 +119,7 @@ int CephxClientHandler::handle_response( int ret, bufferlist::const_iterator& indata, CryptoKey *session_key, - CryptoKey *connection_secret) + std::string *connection_secret) { ldout(cct, 10) << this << " handle_response ret = " << ret << dendl; @@ -178,7 +178,8 @@ int CephxClientHandler::handle_response( lderr(cct) << __func__ << " failed to decrypt connection_secret" << dendl; } else { - ldout(cct, 10) << " got connection_secret " << dendl; + ldout(cct, 10) << " got connection_secret " + << connection_secret->size() << " bytes" << dendl; } } if (extra_tickets.length()) { diff --git a/src/auth/cephx/CephxClientHandler.h b/src/auth/cephx/CephxClientHandler.h index f56bc1d9137..cd1196a689c 100644 --- a/src/auth/cephx/CephxClientHandler.h +++ b/src/auth/cephx/CephxClientHandler.h @@ -53,7 +53,7 @@ public: int build_request(bufferlist& bl) const override; int handle_response(int ret, bufferlist::const_iterator& iter, CryptoKey *session_key, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; bool build_rotating_request(bufferlist& bl) const override; int get_protocol() const override { return CEPH_AUTH_CEPHX; } diff --git a/src/auth/cephx/CephxProtocol.cc b/src/auth/cephx/CephxProtocol.cc index 04f9e8e2249..6254d8f3c1e 100644 --- a/src/auth/cephx/CephxProtocol.cc +++ b/src/auth/cephx/CephxProtocol.cc @@ -393,9 +393,10 @@ bool cephx_decode_ticket(CephContext *cct, KeyStore *keys, uint32_t service_id, */ bool cephx_verify_authorizer(CephContext *cct, KeyStore *keys, bufferlist::const_iterator& indata, + size_t connection_secret_required_len, CephXServiceTicketInfo& ticket_info, std::unique_ptr *challenge, - CryptoKey *connection_secret, + std::string *connection_secret, bufferlist *reply_bl) { __u8 authorizer_v; @@ -496,12 +497,11 @@ bool cephx_verify_authorizer(CephContext *cct, KeyStore *keys, #ifndef WITH_SEASTAR if (connection_secret) { // generate a connection secret - bufferptr bp; - CryptoHandler *crypto = cct->get_crypto_handler(CEPH_CRYPTO_AES); - assert(crypto); - int r = crypto->create(cct->random(), bp); - assert(r >= 0); - connection_secret->set_secret(CEPH_CRYPTO_AES, bp, ceph_clock_now()); + connection_secret->resize(connection_secret_required_len); + if (connection_secret_required_len) { + cct->random()->get_bytes(connection_secret->data(), + connection_secret_required_len); + } reply.connection_secret = *connection_secret; } #endif @@ -516,7 +516,7 @@ bool cephx_verify_authorizer(CephContext *cct, KeyStore *keys, } bool CephXAuthorizer::verify_reply(bufferlist::const_iterator& indata, - CryptoKey *connection_secret) + std::string *connection_secret) { CephXAuthorizeReply reply; @@ -534,7 +534,7 @@ bool CephXAuthorizer::verify_reply(bufferlist::const_iterator& indata, } if (connection_secret && - reply.connection_secret.get_type()) { + reply.connection_secret.size()) { *connection_secret = reply.connection_secret; } return true; diff --git a/src/auth/cephx/CephxProtocol.h b/src/auth/cephx/CephxProtocol.h index c4485f6e68d..0aedc9d12d9 100644 --- a/src/auth/cephx/CephxProtocol.h +++ b/src/auth/cephx/CephxProtocol.h @@ -220,11 +220,11 @@ WRITE_CLASS_ENCODER(CephXServiceTicketRequest) struct CephXAuthorizeReply { uint64_t nonce_plus_one; - CryptoKey connection_secret; + std::string connection_secret; void encode(bufferlist& bl) const { using ceph::encode; __u8 struct_v = 1; - if (connection_secret.get_type()) { + if (connection_secret.size()) { struct_v = 2; } encode(struct_v, bl); @@ -259,7 +259,7 @@ public: bool build_authorizer(); bool verify_reply(bufferlist::const_iterator& reply, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; bool add_challenge(CephContext *cct, const bufferlist& challenge) override; }; @@ -423,11 +423,13 @@ bool cephx_decode_ticket(CephContext *cct, KeyStore *keys, * Verify authorizer and generate reply authorizer */ extern bool cephx_verify_authorizer( - CephContext *cct, KeyStore *keys, + CephContext *cct, + KeyStore *keys, bufferlist::const_iterator& indata, + size_t connection_secret_required_len, CephXServiceTicketInfo& ticket_info, std::unique_ptr *challenge, - CryptoKey *connection_secret, + std::string *connection_secret, bufferlist *reply_bl); diff --git a/src/auth/cephx/CephxServiceHandler.cc b/src/auth/cephx/CephxServiceHandler.cc index 64c0b5a0c8e..03d9c7357f1 100644 --- a/src/auth/cephx/CephxServiceHandler.cc +++ b/src/auth/cephx/CephxServiceHandler.cc @@ -27,11 +27,13 @@ #undef dout_prefix #define dout_prefix *_dout << "cephx server " << entity_name << ": " -int CephxServiceHandler::start_session(const EntityName& name, - bufferlist *result_bl, - AuthCapsInfo *caps, - CryptoKey *session_key, - CryptoKey *connection_secret) +int CephxServiceHandler::start_session( + const EntityName& name, + size_t connection_secret_required_length, + bufferlist *result_bl, + AuthCapsInfo *caps, + CryptoKey *session_key, + std::string *connection_secret) { entity_name = name; @@ -49,11 +51,12 @@ int CephxServiceHandler::start_session(const EntityName& name, int CephxServiceHandler::handle_request( bufferlist::const_iterator& indata, + size_t connection_secret_required_len, bufferlist *result_bl, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *psession_key, - CryptoKey *pconnection_secret) + std::string *pconnection_secret) { int ret = 0; @@ -169,9 +172,19 @@ int CephxServiceHandler::handle_request( // generate a connection_secret bufferlist cbl; if (pconnection_secret) { - key_server->generate_secret(*pconnection_secret); - string err; - encode_encrypt(cct, *pconnection_secret, session_key, cbl, err); + pconnection_secret->resize(connection_secret_required_len); + if (connection_secret_required_len) { + cct->random()->get_bytes(pconnection_secret->data(), + connection_secret_required_len); + } + std::string err; + if (encode_encrypt(cct, *pconnection_secret, session_key, cbl, + err) < 0) { + lderr(cct) << __func__ << " failed to encrypt connection secret, " + << err << dendl; + ret = -EACCES; + break; + } } encode(cbl, *result_bl); // provite all of the other tickets at the same time @@ -213,7 +226,7 @@ int CephxServiceHandler::handle_request( CephXServiceTicketInfo auth_ticket_info; // note: no challenge here. if (!cephx_verify_authorizer( - cct, key_server, indata, auth_ticket_info, nullptr, + cct, key_server, indata, 0, auth_ticket_info, nullptr, nullptr, &tmp_bl)) { ret = -EPERM; diff --git a/src/auth/cephx/CephxServiceHandler.h b/src/auth/cephx/CephxServiceHandler.h index 2a5af6f7af7..cb598fc870a 100644 --- a/src/auth/cephx/CephxServiceHandler.h +++ b/src/auth/cephx/CephxServiceHandler.h @@ -30,17 +30,19 @@ public: ~CephxServiceHandler() override {} int start_session(const EntityName& name, + size_t connection_secret_required_length, bufferlist *result_bl, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; int handle_request( bufferlist::const_iterator& indata, + size_t connection_secret_required_length, bufferlist *result_bl, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; void build_cephx_response_header(int request_type, int status, bufferlist& bl); }; diff --git a/src/auth/cephx/CephxSessionHandler.cc b/src/auth/cephx/CephxSessionHandler.cc index 384e92c848b..6de300dfbfe 100644 --- a/src/auth/cephx/CephxSessionHandler.cc +++ b/src/auth/cephx/CephxSessionHandler.cc @@ -207,7 +207,8 @@ int CephxSessionHandler::sign_bufferlist(bufferlist &in, bufferlist &out) int CephxSessionHandler::encrypt_bufferlist(bufferlist &in, bufferlist &out) { std::string error; try { - connection_secret.encrypt(cct, in, out, &error); +#warning fixme key + key.encrypt(cct, in, out, &error); } catch (std::exception &e) { lderr(cct) << __func__ << " failed to encrypt buffer: " << error << dendl; return -1; @@ -218,7 +219,8 @@ int CephxSessionHandler::encrypt_bufferlist(bufferlist &in, bufferlist &out) { int CephxSessionHandler::decrypt_bufferlist(bufferlist &in, bufferlist &out) { std::string error; try { - connection_secret.decrypt(cct, in, out, &error); +#warning fixme key + key.decrypt(cct, in, out, &error); } catch (std::exception &e) { lderr(cct) << __func__ << " failed to decrypt buffer: " << error << dendl; return -1; diff --git a/src/auth/cephx/CephxSessionHandler.h b/src/auth/cephx/CephxSessionHandler.h index 19147b8a169..ad4b65aafe9 100644 --- a/src/auth/cephx/CephxSessionHandler.h +++ b/src/auth/cephx/CephxSessionHandler.h @@ -25,7 +25,7 @@ class CephxSessionHandler : public AuthSessionHandler { public: CephxSessionHandler(CephContext *cct_, const CryptoKey& session_key, - const CryptoKey& connection_secret, + const std::string& connection_secret, uint64_t features) : AuthSessionHandler(cct_, CEPH_AUTH_CEPHX, session_key, connection_secret), features(features) {} diff --git a/src/auth/krb/KrbAuthorizeHandler.cpp b/src/auth/krb/KrbAuthorizeHandler.cpp index ea4fe46b84f..1faad8bbe4d 100644 --- a/src/auth/krb/KrbAuthorizeHandler.cpp +++ b/src/auth/krb/KrbAuthorizeHandler.cpp @@ -23,12 +23,13 @@ bool KrbAuthorizeHandler::verify_authorizer( CephContext* ceph_ctx, KeyStore* keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr* challenge) { auto itr(authorizer_data.cbegin()); diff --git a/src/auth/krb/KrbAuthorizeHandler.hpp b/src/auth/krb/KrbAuthorizeHandler.hpp index 3130896ab2f..bc8eac6259b 100644 --- a/src/auth/krb/KrbAuthorizeHandler.hpp +++ b/src/auth/krb/KrbAuthorizeHandler.hpp @@ -23,12 +23,13 @@ class KrbAuthorizeHandler : public AuthAuthorizeHandler { CephContext*, KeyStore*, const bufferlist&, + size_t, bufferlist *, EntityName *, uint64_t *, AuthCapsInfo *, CryptoKey *, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr< AuthAuthorizerChallenge>* = nullptr) override; diff --git a/src/auth/krb/KrbClientHandler.cpp b/src/auth/krb/KrbClientHandler.cpp index b82c8a65188..e39d074e64d 100644 --- a/src/auth/krb/KrbClientHandler.cpp +++ b/src/auth/krb/KrbClientHandler.cpp @@ -93,7 +93,7 @@ int KrbClientHandler::handle_response( int ret, bufferlist::const_iterator& buff_list, CryptoKey *session_key, - CryptoKey *connection_secret) + std::string *connection_secret) { auto result(ret); gss_buffer_desc gss_buffer_in = {0, nullptr}; diff --git a/src/auth/krb/KrbClientHandler.hpp b/src/auth/krb/KrbClientHandler.hpp index 3418f0d6d7d..a8960a8898c 100644 --- a/src/auth/krb/KrbClientHandler.hpp +++ b/src/auth/krb/KrbClientHandler.hpp @@ -54,7 +54,7 @@ class KrbClientHandler : public AuthClientHandler { int handle_response(int ret, bufferlist::const_iterator& buff_list, CryptoKey *session_key, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; bool build_rotating_request(bufferlist& buff_list) const override { return false; diff --git a/src/auth/krb/KrbProtocol.hpp b/src/auth/krb/KrbProtocol.hpp index 780f66e9b03..abddb584878 100644 --- a/src/auth/krb/KrbProtocol.hpp +++ b/src/auth/krb/KrbProtocol.hpp @@ -86,7 +86,7 @@ class KrbAuthorizer : public AuthAuthorizer { } bool verify_reply(bufferlist::const_iterator& buff_list, - CryptoKey *connection_secret) override { + std::string *connection_secret) override { return true; } bool add_challenge(CephContext* ceph_ctx, diff --git a/src/auth/krb/KrbServiceHandler.cpp b/src/auth/krb/KrbServiceHandler.cpp index 3efd8c90872..d7c0feeb34a 100644 --- a/src/auth/krb/KrbServiceHandler.cpp +++ b/src/auth/krb/KrbServiceHandler.cpp @@ -28,11 +28,12 @@ int KrbServiceHandler::handle_request( bufferlist::const_iterator& indata, + size_t connection_secret_required_length, bufferlist *buff_list, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) + std::string *connection_secret) { auto result(0); gss_buffer_desc gss_buffer_in = {0, nullptr}; @@ -153,10 +154,11 @@ int KrbServiceHandler::handle_request( int KrbServiceHandler::start_session( const EntityName& name, + size_t connection_secret_required_length, bufferlist *buff_list, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) + std::string *connection_secret) { gss_buffer_desc gss_buffer_in = {0, nullptr}; gss_OID gss_object_id = GSS_C_NT_HOSTBASED_SERVICE; diff --git a/src/auth/krb/KrbServiceHandler.hpp b/src/auth/krb/KrbServiceHandler.hpp index 8fe808a48a8..672efc54638 100644 --- a/src/auth/krb/KrbServiceHandler.hpp +++ b/src/auth/krb/KrbServiceHandler.hpp @@ -38,17 +38,19 @@ class KrbServiceHandler : public AuthServiceHandler { m_key_server(kserver) { } ~KrbServiceHandler(); int handle_request(bufferlist::const_iterator& indata, - bufferlist *buff_list, + size_t connection_secret_required_length, + bufferlist *buff_list, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; int start_session(const EntityName& name, - bufferlist *buff_list, + size_t connection_secret_required_length, + bufferlist *buff_list, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) override; + std::string *connection_secret) override; private: gss_buffer_desc m_gss_buffer_out; diff --git a/src/auth/krb/KrbSessionHandler.hpp b/src/auth/krb/KrbSessionHandler.hpp index a38bbbba10e..c5fd2de4c15 100644 --- a/src/auth/krb/KrbSessionHandler.hpp +++ b/src/auth/krb/KrbSessionHandler.hpp @@ -38,7 +38,7 @@ class KrbSessionHandler : public AuthSessionHandler { public: KrbSessionHandler(CephContext* ceph_ctx, const CryptoKey& session_key, - const CryptoKey& connection_secret) : + const std::string& connection_secret) : AuthSessionHandler(ceph_ctx, CEPH_AUTH_GSS, session_key, connection_secret) { } ~KrbSessionHandler() override = default; diff --git a/src/auth/none/AuthNoneAuthorizeHandler.cc b/src/auth/none/AuthNoneAuthorizeHandler.cc index e370507e20c..15bcc06530d 100644 --- a/src/auth/none/AuthNoneAuthorizeHandler.cc +++ b/src/auth/none/AuthNoneAuthorizeHandler.cc @@ -21,12 +21,13 @@ bool AuthNoneAuthorizeHandler::verify_authorizer( CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) { auto iter = authorizer_data.cbegin(); diff --git a/src/auth/none/AuthNoneAuthorizeHandler.h b/src/auth/none/AuthNoneAuthorizeHandler.h index 4cf9c18f66c..5b33f2fc3c3 100644 --- a/src/auth/none/AuthNoneAuthorizeHandler.h +++ b/src/auth/none/AuthNoneAuthorizeHandler.h @@ -24,12 +24,13 @@ struct AuthNoneAuthorizeHandler : public AuthAuthorizeHandler { CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) override; int authorizer_session_crypto() override; }; diff --git a/src/auth/none/AuthNoneClientHandler.h b/src/auth/none/AuthNoneClientHandler.h index 6578e2eb67e..eb3ef8f5521 100644 --- a/src/auth/none/AuthNoneClientHandler.h +++ b/src/auth/none/AuthNoneClientHandler.h @@ -32,7 +32,7 @@ public: int build_request(bufferlist& bl) const override { return 0; } int handle_response(int ret, bufferlist::const_iterator& iter, CryptoKey *session_key, - CryptoKey *connection_secret) override { return 0; } + std::string *connection_secret) override { return 0; } bool build_rotating_request(bufferlist& bl) const override { return false; } int get_protocol() const override { return CEPH_AUTH_NONE; } diff --git a/src/auth/none/AuthNoneProtocol.h b/src/auth/none/AuthNoneProtocol.h index cacfddefb70..2a501efe525 100644 --- a/src/auth/none/AuthNoneProtocol.h +++ b/src/auth/none/AuthNoneProtocol.h @@ -29,7 +29,7 @@ struct AuthNoneAuthorizer : public AuthAuthorizer { return 0; } bool verify_reply(bufferlist::const_iterator& reply, - CryptoKey *connection_secret) override { return true; } + std::string *connection_secret) override { return true; } bool add_challenge(CephContext *cct, const bufferlist& ch) override { return true; } diff --git a/src/auth/none/AuthNoneServiceHandler.h b/src/auth/none/AuthNoneServiceHandler.h index d62f2a283a4..038c75e03be 100644 --- a/src/auth/none/AuthNoneServiceHandler.h +++ b/src/auth/none/AuthNoneServiceHandler.h @@ -27,20 +27,22 @@ public: ~AuthNoneServiceHandler() override {} int start_session(const EntityName& name, + size_t connection_secret_required_length, bufferlist *result_bl, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) override { + std::string *connection_secret) override { entity_name = name; caps->allow_all = true; return 1; } int handle_request(bufferlist::const_iterator& indata, + size_t connection_secret_required_length, bufferlist *result_bl, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) override { + std::string *connection_secret) override { return 0; } void build_cephx_response_header(int request_type, int status, diff --git a/src/auth/none/AuthNoneSessionHandler.h b/src/auth/none/AuthNoneSessionHandler.h index d4b033a9c45..fca01ac2d42 100644 --- a/src/auth/none/AuthNoneSessionHandler.h +++ b/src/auth/none/AuthNoneSessionHandler.h @@ -21,7 +21,7 @@ class AuthNoneSessionHandler : public AuthSessionHandler { public: AuthNoneSessionHandler(CephContext *cct_, const CryptoKey& session_key, - const CryptoKey& connection_secret) + const std::string& connection_secret) : AuthSessionHandler(cct_, CEPH_AUTH_NONE, session_key, connection_secret) {} ~AuthNoneSessionHandler() override {} diff --git a/src/auth/unknown/AuthUnknownAuthorizeHandler.cc b/src/auth/unknown/AuthUnknownAuthorizeHandler.cc index 632e41dd764..73b393db7e3 100644 --- a/src/auth/unknown/AuthUnknownAuthorizeHandler.cc +++ b/src/auth/unknown/AuthUnknownAuthorizeHandler.cc @@ -18,12 +18,13 @@ bool AuthUnknownAuthorizeHandler::verify_authorizer( CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist * authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) { // For unknown authorizers, there's nothing to verify. They're "OK" by definition. PLR diff --git a/src/auth/unknown/AuthUnknownAuthorizeHandler.h b/src/auth/unknown/AuthUnknownAuthorizeHandler.h index 2590900b0cb..464d47f28d7 100644 --- a/src/auth/unknown/AuthUnknownAuthorizeHandler.h +++ b/src/auth/unknown/AuthUnknownAuthorizeHandler.h @@ -24,12 +24,13 @@ struct AuthUnknownAuthorizeHandler : public AuthAuthorizeHandler { CephContext *cct, KeyStore *keys, const bufferlist& authorizer_data, + size_t connection_secret_required_len, bufferlist *authorizer_reply, EntityName *entity_name, uint64_t *global_id, AuthCapsInfo *caps_info, CryptoKey *session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) override; int authorizer_session_crypto() override; }; diff --git a/src/auth/unknown/AuthUnknownClientHandler.h b/src/auth/unknown/AuthUnknownClientHandler.h index 0ffa3e50c0e..79441581130 100644 --- a/src/auth/unknown/AuthUnknownClientHandler.h +++ b/src/auth/unknown/AuthUnknownClientHandler.h @@ -31,7 +31,7 @@ public: int build_request(bufferlist& bl) const { return 0; } int handle_response(int ret, bufferlist::iterator& iter, CryptoKey *session_key, - CryptoKey *connection_secret) { return 0; } + std::string *connection_secret) { return 0; } bool build_rotating_request(bufferlist& bl) const { return false; } int get_protocol() const { return CEPH_AUTH_UNKNOWN; } diff --git a/src/auth/unknown/AuthUnknownServiceHandler.h b/src/auth/unknown/AuthUnknownServiceHandler.h index 8a2315c1276..7b4019f45d8 100644 --- a/src/auth/unknown/AuthUnknownServiceHandler.h +++ b/src/auth/unknown/AuthUnknownServiceHandler.h @@ -27,18 +27,20 @@ public: ~AuthUnknownServiceHandler() {} int start_session(const EntityName& name, + size_t connection_secret_required_length, bufferlist *result_bl, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) { + std::string *connection_secret) { return 1; } int handle_request(bufferlist::iterator& indata, + size_t connection_secret_required_length, bufferlist *result_bl, uint64_t *global_id, AuthCapsInfo *caps, CryptoKey *session_key, - CryptoKey *connection_secret) { + std::string *connection_secret) { ceph_abort(); // shouldn't get called return 0; } diff --git a/src/auth/unknown/AuthUnknownSessionHandler.h b/src/auth/unknown/AuthUnknownSessionHandler.h index e7810562c0b..b10320debda 100644 --- a/src/auth/unknown/AuthUnknownSessionHandler.h +++ b/src/auth/unknown/AuthUnknownSessionHandler.h @@ -23,7 +23,7 @@ class AuthUnknownSessionHandler : public AuthSessionHandler { public: AuthUnknownSessionHandler(CephContext *cct_, const CryptoKey& session_key, - const CryptoKey& connection_secret) + const std::string& connection_secret) : AuthSessionHandler(cct_, CEPH_AUTH_UNKNOWN, session_key, connection_secret) {} ~AuthUnknownSessionHandler() override {} diff --git a/src/crimson/net/SocketConnection.cc b/src/crimson/net/SocketConnection.cc index 601b090136f..6e2d1a02307 100644 --- a/src/crimson/net/SocketConnection.cc +++ b/src/crimson/net/SocketConnection.cc @@ -690,7 +690,7 @@ SocketConnection::handle_connect_reply(msgr_tag_t tag) h.backoff = 0ms; set_features(h.reply.features & h.connect.features); if (h.authorizer) { - CryptoKey connection_secret; // this is not used here, we just need + std::string connection_secret; // this is not used here, we just need // to make get_auth_session_handler // call happy session_security.reset( diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc index ba1f2c5e557..9ad76cdc46a 100644 --- a/src/mon/AuthMonitor.cc +++ b/src/mon/AuthMonitor.cc @@ -665,13 +665,16 @@ bool AuthMonitor::prep_auth(MonOpRequestRef op, bool paxos_writable) try { if (start) { // new session - ret = s->auth_handler->start_session(entity_name, &response_bl, + ret = s->auth_handler->start_session(entity_name, + 0, // no connection_secret needed + &response_bl, &s->con->peer_caps_info, nullptr, nullptr); } else { // request ret = s->auth_handler->handle_request( indata, + 0, // no connection_secret needed &response_bl, &s->con->peer_global_id, &s->con->peer_caps_info, diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc index c67724ffb42..182e8fcdf85 100644 --- a/src/mon/MonClient.cc +++ b/src/mon/MonClient.cc @@ -1281,7 +1281,7 @@ int MonClient::handle_auth_done( uint32_t con_mode, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_key) + std::string *connection_secret) { if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { std::lock_guard l(monc_lock); @@ -1289,7 +1289,7 @@ int MonClient::handle_auth_done( if (i.second.is_con(con)) { int r = i.second.handle_auth_done( global_id, bl, - session_key, connection_key); + session_key, connection_secret); if (r) { pending_cons.erase(i.first); if (!pending_cons.empty()) { @@ -1384,6 +1384,7 @@ int MonClient::handle_auth_request( cct, rotating_secrets.get(), payload, + auth_meta->get_connection_secret_length(), reply, &con->peer_name, &con->peer_global_id, @@ -1539,7 +1540,7 @@ int MonConnection::handle_auth_done( uint64_t new_global_id, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_secret) + std::string *connection_secret) { ldout(cct,10) << __func__ << " global_id " << new_global_id << " payload " << bl.length() diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h index b2838d9c806..d79234d2d43 100644 --- a/src/mon/MonClient.h +++ b/src/mon/MonClient.h @@ -141,7 +141,7 @@ public: uint64_t global_id, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_secret); + std::string *connection_secret); int handle_auth_bad_method( uint32_t old_auth_method, int result, @@ -284,7 +284,7 @@ public: uint32_t con_mode, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_key) override; + std::string *connection_secret) override; int handle_auth_bad_method( Connection *con, uint32_t old_auth_method, diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index 777f3822c4d..1b456434a75 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -5908,12 +5908,12 @@ int Monitor::handle_auth_done( uint32_t con_mode, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_key) + std::string *connection_secret) { // verify authorizer reply auto auth_meta = con->get_auth_meta(); auto p = bl.begin(); - if (!auth_meta->authorizer->verify_reply(p, &auth_meta->connection_secret)) { + if (!auth_meta->authorizer->verify_reply(p, connection_secret)) { dout(0) << __func__ << " failed verifying authorizer reply" << dendl; return -EACCES; } @@ -6058,6 +6058,7 @@ int Monitor::handle_auth_request( cct, &keyring, payload, + auth_meta->get_connection_secret_length(), reply, &con->peer_name, &con->peer_global_id, @@ -6145,17 +6146,24 @@ int Monitor::handle_auth_request( s->auth_handler = auth_handler; con->set_priv(RefCountedPtr{s, false}); - r = s->auth_handler->start_session(entity_name, reply, - &con->peer_caps_info, - &auth_meta->session_key, - &auth_meta->connection_secret); + r = s->auth_handler->start_session( + entity_name, + auth_meta->get_connection_secret_length(), + reply, + &con->peer_caps_info, + &auth_meta->session_key, + &auth_meta->connection_secret); } else { priv = con->get_priv(); s = static_cast(priv.get()); - r = s->auth_handler->handle_request(p, reply, &con->peer_global_id, - &con->peer_caps_info, - &auth_meta->session_key, - &auth_meta->connection_secret); + r = s->auth_handler->handle_request( + p, + auth_meta->get_connection_secret_length(), + reply, + &con->peer_global_id, + &con->peer_caps_info, + &auth_meta->session_key, + &auth_meta->connection_secret); } if (r > 0 && !s->authenticated) { diff --git a/src/mon/Monitor.h b/src/mon/Monitor.h index 5d674124ed6..d79249edb8e 100644 --- a/src/mon/Monitor.h +++ b/src/mon/Monitor.h @@ -922,7 +922,7 @@ private: uint32_t con_mode, const bufferlist& bl, CryptoKey *session_key, - CryptoKey *connection_key) override; + std::string *connection_secret) override; int handle_auth_bad_method( Connection *con, uint32_t old_auth_method, diff --git a/src/msg/Messenger.cc b/src/msg/Messenger.cc index 23ce0ad7861..efeab3902b5 100644 --- a/src/msg/Messenger.cc +++ b/src/msg/Messenger.cc @@ -115,7 +115,7 @@ bool Messenger::ms_deliver_verify_authorizer( bufferlist& authorizer_reply, bool& isvalid, CryptoKey& session_key, - CryptoKey *connection_secret, + std::string *connection_secret, std::unique_ptr *challenge) { if (authorizer.length() == 0) { @@ -148,6 +148,7 @@ bool Messenger::ms_deliver_verify_authorizer( cct, ks, authorizer, + 0, &authorizer_reply, &con->peer_name, &con->peer_global_id, diff --git a/src/msg/Messenger.h b/src/msg/Messenger.h index bfd3eeef042..911d9eea5f7 100644 --- a/src/msg/Messenger.h +++ b/src/msg/Messenger.h @@ -800,8 +800,9 @@ public: bool ms_deliver_verify_authorizer( Connection *con, int peer_type, int protocol, bufferlist& authorizer, bufferlist& authorizer_reply, - bool& isvalid, CryptoKey& session_key, - CryptoKey *connection_secret, + bool& isvalid, + CryptoKey& session_key, + std::string *connection_secret, std::unique_ptr *challenge); /** diff --git a/src/msg/async/ProtocolV1.cc b/src/msg/async/ProtocolV1.cc index fe05dead7b3..03ceae78d23 100644 --- a/src/msg/async/ProtocolV1.cc +++ b/src/msg/async/ProtocolV1.cc @@ -1682,7 +1682,7 @@ CtPtr ProtocolV1::client_ready() { << authorizer << dendl; session_security.reset(get_auth_session_handler( cct, authorizer->protocol, authorizer->session_key, - authorizer->session_key /* connection_secret */, + string() /* connection_secret */, connection->get_features())); } else { // We have no authorizer, so we shouldn't be applying security to messages @@ -2354,7 +2354,7 @@ CtPtr ProtocolV1::open(ceph_msg_connect_reply &reply, session_security.reset( get_auth_session_handler(cct, connect_msg.authorizer_protocol, session_key, - session_key /* connection secret */, + string() /* connection secret */, connection->get_features())); bufferlist reply_bl; diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index 6b3516db471..9ec9d9fc809 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -2423,7 +2423,24 @@ CtPtr ProtocolV2::handle_auth_request(char *payload, uint32_t length) { << ", payload_len=" << request.auth_payload().length() << ")" << dendl; auth_meta.auth_method = request.method(); - auth_meta.preferred_con_modes = request.preferred_modes(); + + // select a connection mode + auto& preferred_modes = request.preferred_modes(); + std::vector allowed_modes; + messenger->auth_server->get_supported_con_modes( + connection->get_peer_type(), auth_meta.auth_method, &allowed_modes); + for (auto mode : allowed_modes) { + if (std::find(preferred_modes.begin(), preferred_modes.end(), mode) + != preferred_modes.end()) { + auth_meta.con_mode = mode; + break; + } + } + if (auth_meta.con_mode == CEPH_CON_MODE_UNKNOWN) { + ldout(cct,1) << "failed to pick con mode from client's " << preferred_modes + << " and our " << allowed_modes << dendl; + return _auth_bad_method(-EOPNOTSUPP); + } return _handle_auth_request(request.auth_payload(), false); } @@ -2463,29 +2480,9 @@ CtPtr ProtocolV2::_handle_auth_request(bufferlist& auth_payload, bool more) return _fault(); } if (r == 1) { - // select a connection mode - std::vector allowed_modes; - messenger->auth_server->get_supported_con_modes( - connection->get_peer_type(), auth_meta.auth_method, &allowed_modes); - for (auto mode : allowed_modes) { - if (std::find(auth_meta.preferred_con_modes.begin(), - auth_meta.preferred_con_modes.end(), - mode) != auth_meta.preferred_con_modes.end()) { - auth_meta.con_mode = mode; - break; - } - } - if (auth_meta.con_mode == 0) { - ldout(cct, 1) << __func__ << " failed to select connection mode, i allow " - << allowed_modes - << ", client prefers " << auth_meta.preferred_con_modes - << dendl; - return _auth_bad_method(-EOPNOTSUPP); - } else { - AuthDoneFrame auth_done(connection->peer_global_id, auth_meta.con_mode, - reply); - return WRITE(auth_done.get_buffer(), "auth done", read_frame); - } + AuthDoneFrame auth_done(connection->peer_global_id, auth_meta.con_mode, + reply); + return WRITE(auth_done.get_buffer(), "auth done", read_frame); } else if (r == 0) { AuthReplyMoreFrame more(reply); return WRITE(more.get_buffer(), "auth reply more", read_frame); diff --git a/src/msg/simple/Pipe.cc b/src/msg/simple/Pipe.cc index 8f5d45763c0..271dec8e13a 100644 --- a/src/msg/simple/Pipe.cc +++ b/src/msg/simple/Pipe.cc @@ -525,7 +525,8 @@ int Pipe::accept() had_challenge = (bool)authorizer_challenge; authorizer_reply.clear(); if (!msgr->ms_deliver_verify_authorizer( - connection_state.get(), peer_type, connect.authorizer_protocol, authorizer, + connection_state.get(), peer_type, connect.authorizer_protocol, + authorizer, authorizer_reply, authorizer_valid, session_key, nullptr /* connection_secret */, need_challenge ? &authorizer_challenge : nullptr) || @@ -819,7 +820,7 @@ int Pipe::accept() get_auth_session_handler(msgr->cct, connect.authorizer_protocol, session_key, - session_key, /* connection_secret */ + string(), /* connection_secret */ connection_state->get_features())); // notify @@ -1346,7 +1347,7 @@ int Pipe::connect() msgr->cct, authorizer->protocol, authorizer->session_key, - authorizer->session_key /* connection secret*/, + string() /* connection secret*/, connection_state->get_features())); } else { // We have no authorizer, so we shouldn't be applying security to messages in this pipe. PLR diff --git a/src/msg/xio/XioConnection.cc b/src/msg/xio/XioConnection.cc index 97516f4bc6a..4bfab39b454 100644 --- a/src/msg/xio/XioConnection.cc +++ b/src/msg/xio/XioConnection.cc @@ -234,6 +234,7 @@ int XioConnection::passive_setup() msgr->ms_deliver_verify_authorizer( this, peer_type, CEPH_AUTH_NONE, auth.bl, + 0, authorizer_reply, authorizer_valid, session_key); -- 2.39.5