Skip to content

Commit 3888ac8

Browse files
batrickbatrick
committed
Merge PR ceph#52939 into main
* refs/pull/52939/head: mon/MonClient: handle ms_handle_fast_authentication return Reviewed-by: Radoslaw Zarzynski <[email protected]>
2 parents b18b1ef + bfbfbbf commit 3888ac8

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
@@ -6424,7 +6424,9 @@ int Monitor::handle_auth_request(
64246424
&auth_meta->connection_secret,
64256425
&auth_meta->authorizer_challenge);
64266426
if (isvalid) {
6427-
ms_handle_fast_authentication(con);
6427+
if (!ms_handle_fast_authentication(con)) {
6428+
return -EACCES;
6429+
}
64286430
return 1;
64296431
}
64306432
if (!more && !was_challenge && auth_meta->authorizer_challenge) {
@@ -6545,7 +6547,9 @@ int Monitor::handle_auth_request(
65456547
}
65466548
if (r > 0 &&
65476549
!s->authenticated) {
6548-
ms_handle_fast_authentication(con);
6550+
if (!ms_handle_fast_authentication(con)) {
6551+
return -EACCES;
6552+
}
65496553
}
65506554

65516555
dout(30) << " r " << r << " reply:\n";
@@ -6583,12 +6587,12 @@ void Monitor::ms_handle_accept(Connection *con)
65836587
}
65846588
}
65856589

6586-
int Monitor::ms_handle_fast_authentication(Connection *con)
6590+
bool Monitor::ms_handle_fast_authentication(Connection *con)
65876591
{
65886592
if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) {
65896593
// mon <-> mon connections need no Session, and setting one up
65906594
// creates an awkward ref cycle between Session and Connection.
6591-
return 1;
6595+
return true;
65926596
}
65936597

65946598
auto priv = con->get_priv();
@@ -6598,7 +6602,7 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
65986602
if (state == STATE_SHUTDOWN) {
65996603
dout(10) << __func__ << " ignoring new con " << con << " (shutdown)" << dendl;
66006604
con->mark_down();
6601-
return -EACCES;
6605+
return false;
66026606
}
66036607
s = session_map.new_session(
66046608
entity_name_t(con->get_peer_type(), -1), // we don't know yet
@@ -6616,11 +6620,10 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
66166620
<< " " << *s << dendl;
66176621

66186622
AuthCapsInfo &caps_info = con->get_peer_caps_info();
6619-
int ret = 0;
66206623
if (caps_info.allow_all) {
66216624
s->caps.set_allow_all();
66226625
s->authenticated = true;
6623-
ret = 1;
6626+
return true;
66246627
} else if (caps_info.caps.length()) {
66256628
bufferlist::const_iterator p = caps_info.caps.cbegin();
66266629
string str;
@@ -6629,22 +6632,19 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
66296632
} catch (const ceph::buffer::error &err) {
66306633
derr << __func__ << " corrupt cap data for " << con->get_peer_entity_name()
66316634
<< " in auth db" << dendl;
6632-
str.clear();
6633-
ret = -EACCES;
6635+
return false;
66346636
}
6635-
if (ret >= 0) {
6636-
if (s->caps.parse(str, NULL)) {
6637-
s->authenticated = true;
6638-
ret = 1;
6639-
} else {
6640-
derr << __func__ << " unparseable caps '" << str << "' for "
6641-
<< con->get_peer_entity_name() << dendl;
6642-
ret = -EACCES;
6643-
}
6637+
if (s->caps.parse(str, NULL)) {
6638+
s->authenticated = true;
6639+
return true;
6640+
} else {
6641+
derr << __func__ << " unparseable caps '" << str << "' for "
6642+
<< con->get_peer_entity_name() << dendl;
6643+
return false;
66446644
}
6645+
} else {
6646+
return false;
66456647
}
6646-
6647-
return ret;
66486648
}
66496649

66506650
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
@@ -969,7 +969,7 @@ class Monitor : public Dispatcher,
969969
MonCap mon_caps;
970970
bool get_authorizer(int dest_type, AuthAuthorizer **authorizer);
971971
public: // for AuthMonitor msgr1:
972-
int ms_handle_fast_authentication(Connection *con) override;
972+
bool ms_handle_fast_authentication(Connection *con) override;
973973
private:
974974
void ms_handle_accept(Connection *con) override;
975975
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
@@ -7650,9 +7650,8 @@ void OSD::ms_fast_dispatch(Message *m)
76507650
OID_EVENT_TRACE_WITH_MSG(m, "MS_FAST_DISPATCH_END", false);
76517651
}
76527652

7653-
int OSD::ms_handle_fast_authentication(Connection *con)
7653+
bool OSD::ms_handle_fast_authentication(Connection *con)
76547654
{
7655-
int ret = 0;
76567655
auto s = ceph::ref_cast<Session>(con->get_priv());
76577656
if (!s) {
76587657
s = ceph::make_ref<Session>(cct, con);
@@ -7670,6 +7669,7 @@ int OSD::ms_handle_fast_authentication(Connection *con)
76707669
AuthCapsInfo &caps_info = con->get_peer_caps_info();
76717670
if (caps_info.allow_all) {
76727671
s->caps.set_allow_all();
7672+
return true;
76737673
} else if (caps_info.caps.length() > 0) {
76747674
bufferlist::const_iterator p = caps_info.caps.cbegin();
76757675
string str;
@@ -7679,23 +7679,22 @@ int OSD::ms_handle_fast_authentication(Connection *con)
76797679
catch (ceph::buffer::error& e) {
76807680
dout(10) << __func__ << " session " << s << " " << s->entity_name
76817681
<< " failed to decode caps string" << dendl;
7682-
ret = -EACCES;
7683-
}
7684-
if (!ret) {
7685-
bool success = s->caps.parse(str);
7686-
if (success) {
7687-
dout(10) << __func__ << " session " << s
7688-
<< " " << s->entity_name
7689-
<< " has caps " << s->caps << " '" << str << "'" << dendl;
7690-
ret = 1;
7691-
} else {
7692-
dout(10) << __func__ << " session " << s << " " << s->entity_name
7693-
<< " failed to parse caps '" << str << "'" << dendl;
7694-
ret = -EACCES;
7695-
}
7682+
return false;
76967683
}
7684+
bool success = s->caps.parse(str);
7685+
if (success) {
7686+
dout(10) << __func__ << " session " << s
7687+
<< " " << s->entity_name
7688+
<< " has caps " << s->caps << " '" << str << "'" << dendl;
7689+
return true;
7690+
} else {
7691+
dout(10) << __func__ << " session " << s << " " << s->entity_name
7692+
<< " failed to parse caps '" << str << "'" << dendl;
7693+
return false;
7694+
}
7695+
} else {
7696+
return false;
76977697
}
7698-
return ret;
76997698
}
77007699

77017700
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;
@@ -1957,7 +1957,7 @@ class OSD : public Dispatcher,
19571957
void ms_handle_connect(Connection *con) override;
19581958
void ms_handle_fast_connect(Connection *con) override;
19591959
void ms_handle_fast_accept(Connection *con) override;
1960-
int ms_handle_fast_authentication(Connection *con) override;
1960+
bool ms_handle_fast_authentication(Connection *con) override;
19611961
bool ms_handle_reset(Connection *con) override;
19621962
void ms_handle_remote_reset(Connection *con) override {}
19631963
bool ms_handle_refused(Connection *con) override;

0 commit comments

Comments
 (0)