Skip to content

Commit bfbfbbf

Browse files
batrickbatrick
committed
mon/MonClient: handle ms_handle_fast_authentication return
Fixes: https://tracker.ceph.com/issues/62382 Signed-off-by: Patrick Donnelly <[email protected]>
1 parent cf51417 commit bfbfbbf

File tree

14 files changed

+73
-69
lines changed

14 files changed

+73
-69
lines changed

‎src/mds/MDSDaemon.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,11 +1151,11 @@ bool MDSDaemon::parse_caps(const AuthCapsInfo& info, MDSAuthCaps& caps)
11511151
}
11521152
}
11531153

1154-
int MDSDaemon::ms_handle_fast_authentication(Connection *con)
1154+
bool MDSDaemon::ms_handle_fast_authentication(Connection *con)
11551155
{
11561156
/* N.B. without mds_lock! */
11571157
MDSAuthCaps caps;
1158-
return parse_caps(con->get_peer_caps_info(), caps) ? 0 : -1;
1158+
return parse_caps(con->get_peer_caps_info(), caps);
11591159
}
11601160

11611161
void MDSDaemon::ms_handle_accept(Connection *con)

‎src/mds/MDSDaemon.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ class MDSDaemon : public Dispatcher {
146146

147147
private:
148148
bool ms_dispatch2(const ref_t<Message> &m) override;
149-
int ms_handle_fast_authentication(Connection *con) override;
149+
bool ms_handle_fast_authentication(Connection *con) override;
150150
void ms_handle_accept(Connection *con) override;
151151
void ms_handle_connect(Connection *con) override;
152152
bool ms_handle_reset(Connection *con) override;

‎src/mgr/DaemonServer.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ entity_addrvec_t DaemonServer::get_myaddrs() const
257257
return msgr->get_myaddrs();
258258
}
259259

260-
int DaemonServer::ms_handle_fast_authentication(Connection *con)
260+
bool DaemonServer::ms_handle_fast_authentication(Connection *con)
261261
{
262262
auto s = ceph::make_ref<MgrSession>(cct);
263263
con->set_priv(s);
@@ -282,17 +282,17 @@ int DaemonServer::ms_handle_fast_authentication(Connection *con)
282282
catch (buffer::error& e) {
283283
dout(10) << " session " << s << " " << s->entity_name
284284
<< " failed to decode caps" << dendl;
285-
return -EACCES;
285+
return false;
286286
}
287287
if (!s->caps.parse(str)) {
288288
dout(10) << " session " << s << " " << s->entity_name
289289
<< " failed to parse caps '" << str << "'" << dendl;
290-
return -EACCES;
290+
return false;
291291
}
292292
dout(10) << " session " << s << " " << s->entity_name
293293
<< " has caps " << s->caps << " '" << str << "'" << dendl;
294294
}
295-
return 1;
295+
return true;
296296
}
297297

298298
void DaemonServer::ms_handle_accept(Connection* con)

‎src/mgr/DaemonServer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ class DaemonServer : public Dispatcher, public md_config_obs_t
272272
~DaemonServer() override;
273273

274274
bool ms_dispatch2(const ceph::ref_t<Message>& m) override;
275-
int ms_handle_fast_authentication(Connection *con) override;
275+
bool ms_handle_fast_authentication(Connection *con) override;
276276
void ms_handle_accept(Connection *con) override;
277277
bool ms_handle_reset(Connection *con) override;
278278
void ms_handle_remote_reset(Connection *con) override {}

‎src/mon/MonClient.cc‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,8 +1609,9 @@ int MonClient::handle_auth_request(
16091609
// for some channels prior to nautilus (osd heartbeat), we
16101610
// tolerate the lack of an authorizer.
16111611
if (!con->get_messenger()->require_authorizer) {
1612-
handle_authentication_dispatcher->ms_handle_fast_authentication(con);
1613-
return 1;
1612+
if (handle_authentication_dispatcher->ms_handle_fast_authentication(con)) {
1613+
return 1;
1614+
}
16141615
}
16151616
return -EACCES;
16161617
}
@@ -1647,8 +1648,10 @@ int MonClient::handle_auth_request(
16471648
&auth_meta->connection_secret,
16481649
ac);
16491650
if (isvalid) {
1650-
handle_authentication_dispatcher->ms_handle_fast_authentication(con);
1651-
return 1;
1651+
if (handle_authentication_dispatcher->ms_handle_fast_authentication(con)) {
1652+
return 1;
1653+
}
1654+
return -EACCES;
16521655
}
16531656
if (!more && !was_challenge && auth_meta->authorizer_challenge) {
16541657
ldout(cct,10) << __func__ << " added challenge on " << con << dendl;

‎src/mon/Monitor.cc‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6402,7 +6402,9 @@ int Monitor::handle_auth_request(
64026402
&auth_meta->connection_secret,
64036403
&auth_meta->authorizer_challenge);
64046404
if (isvalid) {
6405-
ms_handle_fast_authentication(con);
6405+
if (!ms_handle_fast_authentication(con)) {
6406+
return -EACCES;
6407+
}
64066408
return 1;
64076409
}
64086410
if (!more && !was_challenge && auth_meta->authorizer_challenge) {
@@ -6523,7 +6525,9 @@ int Monitor::handle_auth_request(
65236525
}
65246526
if (r > 0 &&
65256527
!s->authenticated) {
6526-
ms_handle_fast_authentication(con);
6528+
if (!ms_handle_fast_authentication(con)) {
6529+
return -EACCES;
6530+
}
65276531
}
65286532

65296533
dout(30) << " r " << r << " reply:\n";
@@ -6561,12 +6565,12 @@ void Monitor::ms_handle_accept(Connection *con)
65616565
}
65626566
}
65636567

6564-
int Monitor::ms_handle_fast_authentication(Connection *con)
6568+
bool Monitor::ms_handle_fast_authentication(Connection *con)
65656569
{
65666570
if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) {
65676571
// mon <-> mon connections need no Session, and setting one up
65686572
// creates an awkward ref cycle between Session and Connection.
6569-
return 1;
6573+
return true;
65706574
}
65716575

65726576
auto priv = con->get_priv();
@@ -6576,7 +6580,7 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
65766580
if (state == STATE_SHUTDOWN) {
65776581
dout(10) << __func__ << " ignoring new con " << con << " (shutdown)" << dendl;
65786582
con->mark_down();
6579-
return -EACCES;
6583+
return false;
65806584
}
65816585
s = session_map.new_session(
65826586
entity_name_t(con->get_peer_type(), -1), // we don't know yet
@@ -6594,11 +6598,10 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
65946598
<< " " << *s << dendl;
65956599

65966600
AuthCapsInfo &caps_info = con->get_peer_caps_info();
6597-
int ret = 0;
65986601
if (caps_info.allow_all) {
65996602
s->caps.set_allow_all();
66006603
s->authenticated = true;
6601-
ret = 1;
6604+
return true;
66026605
} else if (caps_info.caps.length()) {
66036606
bufferlist::const_iterator p = caps_info.caps.cbegin();
66046607
string str;
@@ -6607,22 +6610,19 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
66076610
} catch (const ceph::buffer::error &err) {
66086611
derr << __func__ << " corrupt cap data for " << con->get_peer_entity_name()
66096612
<< " in auth db" << dendl;
6610-
str.clear();
6611-
ret = -EACCES;
6613+
return false;
66126614
}
6613-
if (ret >= 0) {
6614-
if (s->caps.parse(str, NULL)) {
6615-
s->authenticated = true;
6616-
ret = 1;
6617-
} else {
6618-
derr << __func__ << " unparseable caps '" << str << "' for "
6619-
<< con->get_peer_entity_name() << dendl;
6620-
ret = -EACCES;
6621-
}
6615+
if (s->caps.parse(str, NULL)) {
6616+
s->authenticated = true;
6617+
return true;
6618+
} else {
6619+
derr << __func__ << " unparseable caps '" << str << "' for "
6620+
<< con->get_peer_entity_name() << dendl;
6621+
return false;
66226622
}
6623+
} else {
6624+
return false;
66236625
}
6624-
6625-
return ret;
66266626
}
66276627

66286628
void Monitor::set_mon_crush_location(const string& loc)

‎src/mon/Monitor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,7 @@ class Monitor : public Dispatcher,
957957
MonCap mon_caps;
958958
bool get_authorizer(int dest_type, AuthAuthorizer **authorizer);
959959
public: // for AuthMonitor msgr1:
960-
int ms_handle_fast_authentication(Connection *con) override;
960+
bool ms_handle_fast_authentication(Connection *con) override;
961961
private:
962962
void ms_handle_accept(Connection *con) override;
963963
bool ms_handle_reset(Connection *con) override;

‎src/msg/Dispatcher.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -215,12 +215,14 @@ class Dispatcher {
215215
*
216216
* Do not acquire locks in this method! It is considered "fast" delivery.
217217
*
218-
* return 1 for success
219-
* return 0 for no action (let another Dispatcher handle it)
220-
* return <0 for failure (failure to parse caps, for instance)
218+
* Note: MonClient is the only caller of this method and it is configured
219+
* to only call a single dispatcher.
220+
*
221+
* return true for success (auth succeeds for this stage of session construction)
222+
* return false for failure (failure to parse caps, for instance)
221223
*/
222-
virtual int ms_handle_fast_authentication(Connection *con) {
223-
return 0;
224+
[[nodiscard]] virtual bool ms_handle_fast_authentication(Connection *con) {
225+
return false;
224226
}
225227

226228
/**

‎src/osd/OSD.cc‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7645,9 +7645,8 @@ void OSD::ms_fast_dispatch(Message *m)
76457645
OID_EVENT_TRACE_WITH_MSG(m, "MS_FAST_DISPATCH_END", false);
76467646
}
76477647

7648-
int OSD::ms_handle_fast_authentication(Connection *con)
7648+
bool OSD::ms_handle_fast_authentication(Connection *con)
76497649
{
7650-
int ret = 0;
76517650
auto s = ceph::ref_cast<Session>(con->get_priv());
76527651
if (!s) {
76537652
s = ceph::make_ref<Session>(cct, con);
@@ -7665,6 +7664,7 @@ int OSD::ms_handle_fast_authentication(Connection *con)
76657664
AuthCapsInfo &caps_info = con->get_peer_caps_info();
76667665
if (caps_info.allow_all) {
76677666
s->caps.set_allow_all();
7667+
return true;
76687668
} else if (caps_info.caps.length() > 0) {
76697669
bufferlist::const_iterator p = caps_info.caps.cbegin();
76707670
string str;
@@ -7674,23 +7674,22 @@ int OSD::ms_handle_fast_authentication(Connection *con)
76747674
catch (ceph::buffer::error& e) {
76757675
dout(10) << __func__ << " session " << s << " " << s->entity_name
76767676
<< " failed to decode caps string" << dendl;
7677-
ret = -EACCES;
7678-
}
7679-
if (!ret) {
7680-
bool success = s->caps.parse(str);
7681-
if (success) {
7682-
dout(10) << __func__ << " session " << s
7683-
<< " " << s->entity_name
7684-
<< " has caps " << s->caps << " '" << str << "'" << dendl;
7685-
ret = 1;
7686-
} else {
7687-
dout(10) << __func__ << " session " << s << " " << s->entity_name
7688-
<< " failed to parse caps '" << str << "'" << dendl;
7689-
ret = -EACCES;
7690-
}
7677+
return false;
76917678
}
7679+
bool success = s->caps.parse(str);
7680+
if (success) {
7681+
dout(10) << __func__ << " session " << s
7682+
<< " " << s->entity_name
7683+
<< " has caps " << s->caps << " '" << str << "'" << dendl;
7684+
return true;
7685+
} else {
7686+
dout(10) << __func__ << " session " << s << " " << s->entity_name
7687+
<< " failed to parse caps '" << str << "'" << dendl;
7688+
return false;
7689+
}
7690+
} else {
7691+
return false;
76927692
}
7693-
return ret;
76947693
}
76957694

76967695
void OSD::_dispatch(Message *m)

‎src/osd/OSD.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,7 +1507,7 @@ class OSD : public Dispatcher,
15071507
bool ms_handle_refused(Connection *con) override {
15081508
return osd->ms_handle_refused(con);
15091509
}
1510-
int ms_handle_fast_authentication(Connection *con) override {
1510+
bool ms_handle_fast_authentication(Connection *con) override {
15111511
return true;
15121512
}
15131513
} heartbeat_dispatcher;
@@ -1945,7 +1945,7 @@ class OSD : public Dispatcher,
19451945
void ms_handle_connect(Connection *con) override;
19461946
void ms_handle_fast_connect(Connection *con) override;
19471947
void ms_handle_fast_accept(Connection *con) override;
1948-
int ms_handle_fast_authentication(Connection *con) override;
1948+
bool ms_handle_fast_authentication(Connection *con) override;
19491949
bool ms_handle_reset(Connection *con) override;
19501950
void ms_handle_remote_reset(Connection *con) override {}
19511951
bool ms_handle_refused(Connection *con) override;

0 commit comments

Comments
 (0)