[Bug 1969000] Re: [SRU] bail from handle_command() if _generate_command_map() fails

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


** Changed in: cloud-archive/ussuri
       Status: Fix Committed => Fix Released

-- 
You received this bug notification because you are a member of Ubuntu
OpenStack, which is subscribed to Ubuntu Cloud Archive.
https://bugs.launchpad.net/bugs/1969000

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

Status in Ubuntu Cloud Archive:
  Invalid
Status in Ubuntu Cloud Archive ussuri series:
  Fix Released
Status in ceph package in Ubuntu:
  Fix Released
Status in ceph source package in Focal:
  Fix Released
Status in ceph source package in Impish:
  Won't Fix
Status in ceph source package in Jammy:
  Fix Released
Status in ceph source package in Kinetic:
  Fix Released
Status in ceph source package in Lunar:
  Fix Released
Status in ceph source package in Mantic:
  Fix Released

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 at 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 at 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

To manage notifications about this bug go to:
https://bugs.launchpad.net/cloud-archive/+bug/1969000/+subscriptions