Skip to content

Commit

Permalink
mon: MonCap: take EntityName instead when expanding profiles
Browse files Browse the repository at this point in the history
entity_name_t is tightly coupled to the messenger, while EntityName is
tied to auth.  When expanding profiles we want to tie the profile
expansion to the entity that was authenticated.  Otherwise we may incur
in weird behavior such as having caps validation failing because a given
client messenger inst does not match the auth entity it used.

e.g., running

ceph --name osd.0 config-key exists foo daemon-private/osd.X/foo

has entity_name_t 'client.12345' and EntityName 'osd.0'.  Using
entity_name_t during profile expansion would not allow the client access
to daemon-private/osd.X/foo (client.12345 != osd.X).

Fixes: #10844
Backport: firefly,giant

Signed-off-by: Joao Eduardo Luis <joao@redhat.com>
(cherry picked from commit 87544f6)
  • Loading branch information
Joao Eduardo Luis authored and ldachary committed Mar 18, 2015
1 parent 8902279 commit 4427358
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 11 deletions.
6 changes: 3 additions & 3 deletions src/mon/MonCap.cc
Expand Up @@ -114,7 +114,7 @@ BOOST_FUSION_ADAPT_STRUCT(StringConstraint,

// </magic>

void MonCapGrant::expand_profile(entity_name_t name) const
void MonCapGrant::expand_profile(EntityName name) const
{
// only generate this list once
if (!profile_grants.empty())
Expand Down Expand Up @@ -196,7 +196,7 @@ void MonCapGrant::expand_profile(entity_name_t name) const
}

mon_rwxa_t MonCapGrant::get_allowed(CephContext *cct,
entity_name_t name,
EntityName name,
const std::string& s, const std::string& c,
const map<string,string>& c_args) const
{
Expand Down Expand Up @@ -262,7 +262,7 @@ void MonCap::set_allow_all()
}

bool MonCap::is_capable(CephContext *cct,
entity_name_t name,
EntityName name,
const string& service,
const string& command, const map<string,string>& command_args,
bool op_may_read, bool op_may_write, bool op_may_exec) const
Expand Down
8 changes: 4 additions & 4 deletions src/mon/MonCap.h
Expand Up @@ -8,7 +8,7 @@
using std::ostream;

#include "include/types.h"
#include "msg/msg_types.h"
#include "common/entity_name.h"

class CephContext;

Expand Down Expand Up @@ -76,7 +76,7 @@ struct MonCapGrant {
// needed by expand_profile() (via is_match()) and cached here.
mutable list<MonCapGrant> profile_grants;

void expand_profile(entity_name_t name) const;
void expand_profile(EntityName name) const;

MonCapGrant() : allow(0) {}
MonCapGrant(mon_rwxa_t a) : allow(a) {}
Expand All @@ -97,7 +97,7 @@ struct MonCapGrant {
* @return bits we allow
*/
mon_rwxa_t get_allowed(CephContext *cct,
entity_name_t name,
EntityName name,
const std::string& service,
const std::string& command,
const map<string,string>& command_args) const;
Expand Down Expand Up @@ -143,7 +143,7 @@ struct MonCap {
* @return true if the operation is allowed, false otherwise
*/
bool is_capable(CephContext *cct,
entity_name_t name,
EntityName name,
const string& service,
const string& command, const map<string,string>& command_args,
bool op_may_read, bool op_may_write, bool op_may_exec) const;
Expand Down
2 changes: 1 addition & 1 deletion src/mon/Monitor.cc
Expand Up @@ -2317,7 +2317,7 @@ bool Monitor::_allowed_command(MonSession *s, string &module, string &prefix,
bool cmd_w = this_cmd->requires_perm('w');
bool cmd_x = this_cmd->requires_perm('x');

bool capable = s->caps.is_capable(g_ceph_context, s->inst.name,
bool capable = s->caps.is_capable(g_ceph_context, s->entity_name,
module, prefix, param_str_map,
cmd_r, cmd_w, cmd_x);

Expand Down
2 changes: 1 addition & 1 deletion src/mon/Session.h
Expand Up @@ -75,7 +75,7 @@ struct MonSession : public RefCountedObject {
bool is_capable(string service, int mask) {
map<string,string> args;
return caps.is_capable(g_ceph_context,
inst.name,
entity_name,
service, "", args,
mask & MON_CAP_R, mask & MON_CAP_W, mask & MON_CAP_X);
}
Expand Down
5 changes: 3 additions & 2 deletions src/test/mon/moncap.cc
Expand Up @@ -177,7 +177,7 @@ TEST(MonCap, AllowAll) {

ASSERT_TRUE(cap.parse("allow *", NULL));
ASSERT_TRUE(cap.is_allow_all());
ASSERT_TRUE(cap.is_capable(NULL, entity_name_t::CLIENT(0),
ASSERT_TRUE(cap.is_capable(NULL, EntityName(),
"foo", "asdf", map<string,string>(), true, true, true));

MonCap cap2;
Expand All @@ -191,7 +191,8 @@ TEST(MonCap, ProfileOSD) {
bool r = cap.parse("allow profile osd", NULL);
ASSERT_TRUE(r);

entity_name_t name = entity_name_t::OSD(123);
EntityName name;
name.from_str("osd.123");
map<string,string> ca;

ASSERT_TRUE(cap.is_capable(NULL, name, "osd", "", ca, true, false, false));
Expand Down

0 comments on commit 4427358

Please sign in to comment.