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.

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

authtool: Enhance argument combinations validation #9704

Merged
merged 2 commits into from Jul 7, 2016

Conversation

badone
Copy link
Contributor

@badone badone commented Jun 14, 2016

Under certain circumstances ceph-authtool can create invalid key entries in
the specified keyring and behave less than intuitively. What's worse these
keys can currently cause ceph daemons to crash. This patch attempts to tighten
the behaviour and eliminate the possibility an invalid key can be created.

http://tracker.ceph.com/issues/2904

Fixes: #2904
Signed-off-by: Brad Hubbard bhubbard@redhat.com

@badone
Copy link
Contributor Author

badone commented Jun 15, 2016

make[5]: *** Deleting file `test/cephtool-test-mds.sh.log'

Known issue.

@tchaikov tchaikov self-assigned this Jun 16, 2016
@@ -86,6 +86,10 @@ int main(int argc, const char **argv)
} else if (ceph_argparse_flag(args, i, "--gen-print-key", (char*)NULL)) {
gen_print_key = true;
} else if (ceph_argparse_witharg(args, i, &val, "-a", "--add-key", (char*)NULL)) {
if (val.empty()) {
cerr << "Option --add-key requires an argument" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe "an non-empty argument".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, this matches an existing error message from ceph_argparse for consistency.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it!

@badone badone added the tests label Jun 19, 2016

Test --add-key with empty argument

$ /home/brad/working/src/ceph/src/ceph-authtool kring -C --name=mon.* --add-key= --cap mon 'allow *'
Copy link
Contributor

Choose a reason for hiding this comment

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

might want to use a path also applies under other circumstances.

@badone
Copy link
Contributor Author

badone commented Jul 5, 2016

OOPs! Sure, let me fix that...

Under certain circumstances ceph-authtool can create invalid key entries in
the specified keyring and behave less than intuitively. What's worse these
keys can currently cause ceph daemons to crash. This patch attempts to tighten
the behaviour and eliminate the possibility an invalid key can be created.

Fixes: http://tracker.ceph.com/issues/2904
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@tchaikov
Copy link
Contributor

tchaikov commented Jul 5, 2016

lgtm, and tested locally, will merge once jenkins finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants