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

[nikhil kshirsagar]

[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

[nikhil kshirsagar (nkshirsagar)]

// deleted comment //

[nikhil kshirsagar (nkshirsagar)]

// deleted

[nikhil kshirsagar (nkshirsagar)]

// deleted

[nikhil kshirsagar (nkshirsagar)]

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)]

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.

[nikhil kshirsagar (nkshirsagar)]

uploading debdiff for focal here.

[nikhil kshirsagar (nkshirsagar)]

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)]

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;

[Ponnuvel Palaniyappan (pponnuvel)]

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).

[James Page (james-page)]

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

[Andreas Hasenack (ahasenack)]

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.

[Corey Bryant (corey.bryant)]

Hello nikhil, or anyone else affected,

Accepted ceph into ussuri-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:ussuri-proposed
  sudo apt-get update

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, and change the tag from verification-ussuri-needed to verification-ussuri-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-ussuri-failed. In either case, details of your testing will help us make a better decision.

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

[nikhil kshirsagar (nkshirsagar)]

I have tested the SRU proposed packages for focal, the testing results look good and as expected.

[nikhil kshirsagar (nkshirsagar)]

I have tested the SRU proposed packages on ussuri for the cloud archive, the testing results look good and as expected.

[Andreas Hasenack (ahasenack)]

This verification is ok, but the SRU is blocked on the ambiguous verification of #1996010.

[Launchpad Janitor (janitor)]

This bug was fixed in the package ceph - 15.2.17-0ubuntu0.20.04.5

---------------
ceph (15.2.17-0ubuntu0.20.04.5) focal; urgency=medium

   * d/p/bluestore-leak-fix.patch: Fix leak in bluestore cache (LP: #1996010).
   * d/p/bail-after-error.patch: Bail after exception in mon (LP: #1969000).
   * d/p/relax-epoch.patch: Relax epoch-based assertions (LP: #2019293).

 -- Luciano Lo Giudice <email address hidden> Fri, 22 Sep 2023 09:21:41 +0100

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for ceph has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for ceph has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package ceph - 15.2.17-0ubuntu0.20.04.5~cloud0
---------------

 ceph (15.2.17-0ubuntu0.20.04.5~cloud0) bionic-ussuri; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 ceph (15.2.17-0ubuntu0.20.04.5) focal; urgency=medium
 .
    * d/p/bluestore-leak-fix.patch: Fix leak in bluestore cache (LP: #1996010).
    * d/p/bail-after-error.patch: Bail after exception in mon (LP: #1969000).
    * d/p/relax-epoch.patch: Relax epoch-based assertions (LP: #2019293).