Ubuntu
ceph package

[SRU] bail from handle_command() if _generate_command_map() fails

  1. Focal (20.04)
  2. Bug #1969000
Bug #1969000 reported by nikhil kshirsagar
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu Cloud Archive
Invalid
Undecided
Unassigned
Ussuri
In Progress
Undecided
nikhil kshirsagar
ceph (Ubuntu)
Status tracked in Mantic
Focal
Fix Committed
Medium
nikhil kshirsagar
Impish
Won't Fix
Undecided
Unassigned
Jammy
Fix Released
Undecided
Unassigned
Kinetic
Fix Released
Medium
Unassigned
Lunar
Fix Released
Undecided
Unassigned
Mantic
Fix Released
Medium
Unassigned

Bug Description

[Impact]
If improper json data is passed to rados using a manual curl command, or invalid json data through a script like the python eg. shown, it can end up crashing the mon. This is already fixed through https://github.com/ceph/ceph/pull/45891 and already in the Ubuntu octopus point release. This fix is a performance improvement that returns from the function immediately and does not continue executing code anymore in the handle_command() function, since we have caught the exception thrown by _generate_command_map() and dealt with it in Monitor::handle_command().

[Test Plan]
Setup a ceph octopus cluster. A manual run of curl with malformed request like this results in the exception being thrown.

curl -k -H "Authorization: Basic $TOKEN" "https://juju-3b3d82-10-lxd-0:8003/request" -X POST -d '{"prefix":"auth add","entity":"client.testuser02","caps":"mon '\''allow r'\'' osd '\''allow rw pool=testpool01'\''"}'

This reproduces without restful API too.

This python script run on the mon node also will cause the exception to be thrown due to the particular json which is malformed,

root@focal-testing:/home/nikhil/Downloads/ceph_upstream/ceph/build# cat test_ceph.sh
#!/usr/bin/env python3
import json
import rados
c = rados.Rados(conffile='ceph.conf')
c.connect()
cmd = json.dumps({"prefix":"auth add","entity":"client.testuser02","caps":"mon '\''allow r'\'' osd '\''allow rw pool=testpool01'\''"})
#cmd = json.dumps({"prefix":"auth add","entity":"client.testuser02","caps":["mon", "allow r", "osd", "allow rw pool=testpool01"]})
print(c.mon_command(cmd, b''))
root@focal-testing:/home/nikhil/Downloads/ceph_upstream/ceph/build# ./test_ceph.sh
(-22, b'', "bad or missing field 'caps'")

Once this exception is caught correctly as above and the error message printed due to this code, and we bail out of the function due to this SRU, the following code https://github.com/ceph/ceph/blob/6a585618451421f0744745e4dd3636f10f678397/src/mon/Monitor.cc#L3349C1-L3358C6 should never be then further executed because we return as soon as the exception is caught and handled.

So therefore, setting debug level to 10 will validate that the message is never seen from _allowed_command(), i.e at the end of that function,

 dout(10) << __func__ << " " << (capable ? "" : "not ") << "capable" << dendl;

Pasting the function code for reference, (https://github.com/ceph/ceph/blob/octopus/src/mon/Monitor.cc#L3061)

bool Monitor::_allowed_command(MonSession *s, const string &module,
          const string &prefix, const cmdmap_t& cmdmap,
                               const map<string,string>& param_str_map,
                               const MonCommand *this_cmd) {

  bool cmd_r = this_cmd->requires_perm('r');
  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->entity_name,
    module, prefix, param_str_map,
    cmd_r, cmd_w, cmd_x,
    s->get_peer_socket_addr());

  dout(10) << __func__ << " " << (capable ? "" : "not ") << "capable" << dendl;
  return capable;
}

So it would be a reasonable to test the SRU and verify that at loglevel 10, we do not see the https://github.com/ceph/ceph/blob/octopus/src/mon/Monitor.cc#L3077 debug message.

[Where problems could occur]
The only potential problem with this cleanup fix is if
some additional code in the void Monitor::handle_command(MonOpRequestRef op) function is needed to run before exit()'ing out. I have looked for such potential conditions and not found any.

[Other Info]
While the fix to catch the exception is already part of the Octopus 15.2.17 point release, (PR https://github.com/ceph/ceph/pull/45891),
we need this cleanup fix that has now been also merged to master upstream through https://github.com/ceph/ceph/pull/48044

The cleanup fix bails out of the function if the exception is
thrown, therefore avoiding continuing in the function
void Monitor::handle_command(MonOpRequestRef op) in this
error situation.

Upstream tracker - https://tracker.ceph.com/issues/57859
Fixed in ceph main through https://github.com/ceph/ceph/pull/48044

See original description
nikhil kshirsagar (nkshirsagar)
Changed in ceph (Ubuntu):
milestone: none → focal-updates
Changed in ceph (Ubuntu Focal):
importance: Undecided → Medium
Changed in ceph (Ubuntu):
milestone: focal-updates → none
Ponnuvel Palaniyappan (pponnuvel)
tags: added: sts
Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

// deleted comment //

Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

// deleted

Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

// deleted

nikhil kshirsagar (nkshirsagar)
description: updated
nikhil kshirsagar (nkshirsagar)
description: updated
description: updated
Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

This fix https://github.com/ceph/ceph/pull/48044 is what we need to SRU to Octopus because https://github.com/ceph/ceph/pull/45891 is already merged into Octopus and already in the Ubuntu octopus point release also.

nikhil kshirsagar (nkshirsagar)
summary: - [SRU] mon crashes when improper json is passed to rados
+ [SRU] bail from handle_command() if _generate_command_map() fails
nikhil kshirsagar (nkshirsagar)
description: updated
Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

https://github.com/ceph/ceph/pull/48845 and https://github.com/ceph/ceph/pull/48846 are merged upstream. Once they are available in the ceph Ubuntu pacific and quincy point releases this Octopus SRU can be submitted.

Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

uploading debdiff for focal here.

Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote (last edit ):

This has not made it into the ubuntu quincy pt. release as yet, so it's not yet ready for SRU

# pull-lp-source ceph jammy
Found ceph 17.2.5-0ubuntu0.22.04.2 in jammy
Downloading ceph_17.2.5-0ubuntu0.22.04.2.dsc from ports.ubuntu.com (0.010 MiB)
[=====================================================>]100%
Good signature by James Page <email address hidden> (0xBFECAECBA0E7D8C3)
Downloading ceph_17.2.5.orig.tar.xz from ports.ubuntu.com (112.261 MiB)
[=====================================================>]100%
Downloading ceph_17.2.5-0ubuntu0.22.04.2.debian.tar.xz from ports.ubuntu.com (0.121 MiB)
[=====================================================>]100%

Checking if this code has made it (https://bugs.launchpad.net/ubuntu/+source/ceph/+bug/1969000/+attachment/5655825/+files/focal_debdiff_octopus )

I see, in src/mon/Monitor.cc for the quincy sources,

  // Catch bad_cmd_get exception if _generate_command_map() throws it
  try {
    _generate_command_map(cmdmap, param_str_map);
  }
  catch(bad_cmd_get& e) {
    reply_command(op, -EINVAL, e.what(), 0);
  }

---

Neither in pacific as yet,

# pull-uca-source ceph xena
Found ceph 16.2.11-0ubuntu0.21.10.1~cloud0 in focal
Downloading ceph_16.2.11-0ubuntu0.21.10.1~cloud0.dsc from ubuntu-cloud.archive.canonical.com (0.009 MiB)
[=====================================================>]100%
Good signature by James Page <email address hidden> (0xBFECAECBA0E7D8C3)
Downloading ceph_16.2.11.orig.tar.xz from ubuntu-cloud.archive.canonical.com (100.423 MiB)
[=====================================================>]100%
Downloading ceph_16.2.11-0ubuntu0.21.10.1~cloud0.debian.tar.xz from ubuntu-cloud.archive.canonical.com (0.113 MiB)
[=====================================================>]100%

  // Catch bad_cmd_get exception if _generate_command_map() throws it
  try {
    _generate_command_map(cmdmap, param_str_map);
  }
  catch(bad_cmd_get& e) {
    reply_command(op, -EINVAL, e.what(), 0);
  }

nikhil kshirsagar (nkshirsagar)
tags: added: se-sponsors
Revision history for this message
nikhil kshirsagar (nkshirsagar) wrote :

This SRU can now go ahead, as I can see the pacific and quincy Ubuntu point releases have the patch now,

$ pull-uca-source ceph xena
Found ceph 16.2.13-0ubuntu0.21.10.1~cloud1 in focal
Downloading ceph_16.2.13-0ubuntu0.21.10.1~cloud1.dsc from ubuntu-cloud.archive.canonical.com (0.009 MiB)
[=====================================================>]100%
Good signature by James Page <email address hidden> (0xBFECAECBA0E7D8C3)
Downloading ceph_16.2.13.orig.tar.xz from ubuntu-cloud.archive.canonical.com (100.435 MiB)
[=====================================================>]100%
Downloading ceph_16.2.13-0ubuntu0.21.10.1~cloud1.debian.tar.xz from ubuntu-cloud.archive.canonical.com (0.116 MiB)
[=====================================================>]100%
$ ls
ceph-16.2.13 ceph_16.2.13-0ubuntu0.21.10.1~cloud1.debian.tar.xz ceph_16.2.13-0ubuntu0.21.10.1~cloud1.dsc ceph_16.2.13.orig.tar.xz

$ cat src/mon/Monitor.cc |grep -i "catch bad" -A5
  // Catch bad_cmd_get exception if _generate_command_map() throws it
  try {
    _generate_command_map(cmdmap, param_str_map);
  } catch (const bad_cmd_get& e) {
    reply_command(op, -EINVAL, e.what(), 0);
    return;

============

$ pull-lp-source ceph jammy
Found ceph 17.2.6-0ubuntu0.22.04.1 in jammy
Downloading ceph_17.2.6-0ubuntu0.22.04.1.dsc from ports.ubuntu.com (0.010 MiB)
[=====================================================>]100%
Good signature by James Page <email address hidden> (0xBFECAECBA0E7D8C3)
Downloading ceph_17.2.6.orig.tar.xz from ports.ubuntu.com (111.784 MiB)
[=====================================================>]100%
Downloading ceph_17.2.6-0ubuntu0.22.04.1.debian.tar.xz from ports.ubuntu.com (0.121 MiB)
[=====================================================>]100%

$ cd ceph-17.2.6
$ ls
admin ceph.spec.in COPYING-LGPL2.1 etc make-srpm.sh qa SECURITY.md sudoers.d
AUTHORS cmake COPYING-LGPL3 examples man README.aix selinux systemd
bin CMakeLists.txt debian fusetrace mingw_conf.sh README.FreeBSD share udev
ceph-erasure-code-corpus CodingStyle doc install-deps.sh mirroring README.md src win32_build.sh
ceph-menv CONTRIBUTING.rst doc_deps.deb.txt keys monitoring README.solaris SubmittingPatches-backports.rst win32_deps_build.sh
ceph-object-corpus COPYING do_cmake.sh make-debs.sh PendingReleaseNotes README.windows.rst SubmittingPatches-kernel.rst
ceph.spec COPYING-GPL2 do_freebsd.sh make-dist pom.xml run-make-check.sh SubmittingPatches.rst

$ cat src/mon/Monitor.cc |grep -i "catch bad" -A5
  // Catch bad_cmd_get exception if _generate_command_map() throws it
  try {
    _generate_command_map(cmdmap, param_str_map);
  } catch (const bad_cmd_get& e) {
    reply_command(op, -EINVAL, e.what(), 0);
    return;

Changed in ceph (Ubuntu Impish):
status: New → Won't Fix
nikhil kshirsagar (nkshirsagar)
Changed in ceph (Ubuntu Jammy):
status: New → Fix Released
Changed in ceph (Ubuntu Focal):
status: New → In Progress
assignee: nobody → nikhil kshirsagar (nkshirsagar)
Ponnuvel Palaniyappan (pponnuvel)
Changed in ceph (Ubuntu):
status: New → In Progress
Changed in ceph (Ubuntu Kinetic):
status: New → Fix Released
Changed in ceph (Ubuntu Mantic):
status: In Progress → Fix Released
Changed in ceph (Ubuntu Lunar):
status: New → Fix Released
Revision history for this message
Ponnuvel Palaniyappan (pponnuvel) wrote :

The fix is already released for newer Ubuntu and as much marked them as 'Fix released':
Jammy: 17.2.6-0ubuntu0.22.04.1
Kinetic: 17.2.6-0ubuntu0.22.10.1
Lunar: 17.2.6-0ubuntu0.23.04.1
Mantic: 17.2.6-0ubuntu1

This SRU will be needed only for Focal and Ussuri (UCA).

nikhil kshirsagar (nkshirsagar)
tags: added: sts-sru-needed
Mauricio Faria de Oliveira (mfo)
Changed in cloud-archive:
status: New → Invalid
tags: removed: se-sponsors
nikhil kshirsagar (nkshirsagar)
description: updated
Revision history for this message
James Page (james-page) wrote :

Upload made to UNAPPROVED queue for SRU team review in focal.

Revision history for this message
Andreas Hasenack (ahasenack) wrote : Please test proposed package

Hello nikhil, or anyone else affected,

Accepted ceph into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ceph/15.2.17-0ubuntu0.20.04.5 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in ceph (Ubuntu Focal):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-focal
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.