mirror of
https://github.com/ceph/ceph
synced 2025-02-23 19:17:37 +00:00
mon/MonClient: bring back CEPHX_V2 authorizer challenges
Commitc58c5754df
("msg/async/ProtocolV1: use AuthServer and AuthClient") introduced a backwards compatibility issue into msgr1. To fix it, commit3215480105
("mon/MonClient: skip CEPHX_V2 challenge if client doesn't support it") set out to skip authorizer challenges for peers that don't support CEPHX_V2. However, it made it so that authorizer challenges are skipped for all peers in both msgr1 and msgr2 cases, effectively disabling the protection against replay attacks that was put in place in commitf80b848d3f
("auth/cephx: add authorizer challenge", CVE-2018-1128). This is because con->get_features() always returns 0 at that point. In msgr1 case, the peer shares its features along with the authorizer, but while they are available in connect_msg.features they aren't assigned to con until ProtocolV1::open(). In msgr2 case, the peer doesn't share its features until much later (in CLIENT_IDENT frame, i.e. after the authentication phase). The result is that !CEPHX_V2 branch is taken in all cases and replay attack protection is lost. Only clusters with cephx_service_require_version set to 2 on the service daemons would not be silently downgraded. But, since the default is 1 and there are no reports of looping on BADAUTHORIZER faults, I'm pretty sure that no one has ever done that. Note that cephx_require_version set to 2 would have no effect even though it is supposed to be stronger than cephx_service_require_version because MonClient::handle_auth_request() didn't check it. To fix: - for msgr1, check connect_msg.features (as was done before commitc58c5754df
) and challenge if CEPHX_V2 is supported. Together with two preceding patches that resurrect proper cephx_* option handling in msgr1, this covers both "I want old clients to work" and "I wish to require better authentication" use cases. - for msgr2, don't check anything and always challenge. CEPHX_V2 predates msgr2, anyone speaking msgr2 must support it. Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
This commit is contained in:
parent
6f5c4152ca
commit
4a82c72e3b
@ -192,6 +192,9 @@ struct AuthConnectionMeta {
|
|||||||
|
|
||||||
std::unique_ptr<AuthAuthorizer> authorizer;
|
std::unique_ptr<AuthAuthorizer> authorizer;
|
||||||
std::unique_ptr<AuthAuthorizerChallenge> authorizer_challenge;
|
std::unique_ptr<AuthAuthorizerChallenge> authorizer_challenge;
|
||||||
|
|
||||||
|
///< set if msgr1 peer doesn't support CEPHX_V2
|
||||||
|
bool skip_authorizer_challenge = false;
|
||||||
};
|
};
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -1583,13 +1583,8 @@ int MonClient::handle_auth_request(
|
|||||||
}
|
}
|
||||||
|
|
||||||
auto ac = &auth_meta->authorizer_challenge;
|
auto ac = &auth_meta->authorizer_challenge;
|
||||||
if (!HAVE_FEATURE(con->get_features(), CEPHX_V2)) {
|
if (auth_meta->skip_authorizer_challenge) {
|
||||||
if (cct->_conf->cephx_service_require_version >= 2) {
|
ldout(cct, 10) << __func__ << " skipping challenge on " << con << dendl;
|
||||||
ldout(cct,10) << __func__ << " client missing CEPHX_V2 ("
|
|
||||||
<< "cephx_service_requre_version = "
|
|
||||||
<< cct->_conf->cephx_service_require_version << ")" << dendl;
|
|
||||||
return -EACCES;
|
|
||||||
}
|
|
||||||
ac = nullptr;
|
ac = nullptr;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2046,6 +2046,10 @@ CtPtr ProtocolV1::handle_connect_message_2() {
|
|||||||
ceph::buffer::list auth_bl_copy = authorizer_buf;
|
ceph::buffer::list auth_bl_copy = authorizer_buf;
|
||||||
auto am = auth_meta;
|
auto am = auth_meta;
|
||||||
am->auth_method = connect_msg.authorizer_protocol;
|
am->auth_method = connect_msg.authorizer_protocol;
|
||||||
|
if (!HAVE_FEATURE((uint64_t)connect_msg.features, CEPHX_V2)) {
|
||||||
|
// peer doesn't support it and we won't get here if we require it
|
||||||
|
am->skip_authorizer_challenge = true;
|
||||||
|
}
|
||||||
connection->lock.unlock();
|
connection->lock.unlock();
|
||||||
ldout(cct,10) << __func__ << " authorizor_protocol "
|
ldout(cct,10) << __func__ << " authorizor_protocol "
|
||||||
<< connect_msg.authorizer_protocol
|
<< connect_msg.authorizer_protocol
|
||||||
|
Loading…
Reference in New Issue
Block a user