Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mgr: fix crash on set_config from python module with insufficient caps #14706

Merged
merged 1 commit into from Apr 28, 2017
Merged

mgr: fix crash on set_config from python module with insufficient caps #14706

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/mgr/PyModules.cc
View file
Open in desktop
Expand Up @@ -544,8 +544,14 @@ void PyModules::set_config(const std::string &handle,
}
set_cmd.wait();

// FIXME: is config-key put ever allowed to fail?
assert(set_cmd.r == 0);
if (set_cmd.r != 0) {
// config-key put will fail if mgr's auth key has insufficient
// permission to set config keys
// FIXME: should this somehow raise an exception back into Python land?
dout(0) << "`config-key put " << global_key << " " << val << "` failed: "
<< cpp_strerror(set_cmd.r) << dendl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can print out the set_cmd.outs also to help debug the failure.

Copy link
Contributor Author

@tserong tserong Apr 21, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried adding dout(0) << set_cmd.outs << dendl; but it doesn't seem to add much useful. The existing code prints:

2017-04-21 22:06:30.807527 7f1fdfd60700  0 mgr set_config `config-key put mgr.rest.enable_auth false` failed: (13) Permission denied

If I add set_cmd.outs we also get:

2017-04-21 22:06:30.807538 7f1fdfd60700  0 mgr set_config access denied

Copy link
Contributor

@tchaikov tchaikov Apr 24, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tserong i don't think we can assume that monitor does not provide any information more helpful than "access denied". so i think it would helpful if we can have all available error message in mgr's log file even it sounds redundant: error 13 versus `error 13 + "access denied". BTW, i suggested we print the errorno along with the error message. not just the error message from mon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, seems reasonable, updated.

dout(0) << "mon returned " << set_cmd.r << ": " << set_cmd.outs << dendl;
}
}

std::vector<ModuleCommand> PyModules::get_commands()
Expand Down