Bug #54558: malformed json in a Ceph RESTful API call can stop all ceph-mon services - RADOS - Ceph

Actions

Copy link

Bug #54558

closed

malformed json in a Ceph RESTful API call can stop all ceph-mon services

Added by nikhil kshirsagar about 2 years ago. Updated about 1 year ago.

Status:

Resolved

Priority:

Normal

Assignee:

Category:

Target version:

% Done:

Source:

Tags:

backport_processed

Backport:

quincy,pacific,octopus

Regression:

Severity:

3 - minor

Reviewed:

Affected Versions:

Ceph - v17.0.0

ceph-qa-suite:

Component(RADOS):

Pull request ID:

45547

Crash signature (v1):

Crash signature (v2):

Description

When curl from cli, an HTTP request containing malformed json data, for creating user and defining capabilities, caused every ceph-mon service to receive abort signal and to get stuck in restart loop.

The malformed request looks like -

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'\''"}'

The request status shows it is still in the queue.

[
    {
        "failed": [],
        "finished": [],
        "has_failed": false,
        "id": "140576245092648",
        "is_finished": false,
        "is_waiting": false,
        "running": [
            {
                "command": "auth add entity=client.testuser02 caps=mon 'allow r' osd 'allow rw pool=testpool01'",
                "outb": "",
                "outs": "" 
            }
        ],
        "state": "pending",
        "waiting": []
    }
]

But this works fine, (using list type in the caps dict value)

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"]}'

Ceph API should be resilient to bad formatting in HTTP requests. As of now, when ceph aborts the thread, even ceph -s hangs for a while. ceph keeps trying to fulfill the request and fails until the request is manually removed. rbd uploads time out.

The mon logs show,

-1> 2022-03-14T11:01:37.789+0000 7fbe63d09700  0 mon.juju-8c5f4a-sts-stein-bionic-0@0(leader) e3 handle_command mon_command({"prefix": "auth add", "entity": "client.testuser02", "caps": "mon 'allow r' osd 'allow rw pool=testpool01'"} v 0) v1
     0> 2022-03-14T11:01:37.797+0000 7fbe63d09700 -1 *** Caught signal (Aborted) **
 in thread 7fbe63d09700 thread_name:ms_dispatch

 ceph version 15.2.14 (cd3bb7e87a2f62c1b862ff3fd8b1eec13391a5be) octopus (stable)
 1: (()+0x12980) [0x7fbe6ec02980]
 2: (gsignal()+0xc7) [0x7fbe6e83de87]
 3: (abort()+0x141) [0x7fbe6e83f7f1]
 4: (()+0x8c957) [0x7fbe6f451957]
 5: (()+0x92ae6) [0x7fbe6f457ae6]
 6: (()+0x92b21) [0x7fbe6f457b21]
 7: (()+0x92d54) [0x7fbe6f457d54]
 8: (bool ceph::common::cmd_getval<std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >(std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::variant<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, double, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<long, std::allocator<long> >, std::vector<double, std::allocator<double> > >, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, boost::variant<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, double, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<long, std::allocator<long> >, std::vector<double, std::allocator<double> > > > > > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&)+0x108) [0x561b5882d218]
 9: (Monitor::_generate_command_map(std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::variant<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, double, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<long, std::allocator<long> >, std::vector<double, std::allocator<double> > >, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, boost::variant<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, double, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<long, std::allocator<long> >, std::vector<double, std::allocator<double> > > > > >&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&)+0x10d) [0x561b587d4add]
 10: (Monitor::handle_command(boost::intrusive_ptr<MonOpRequest>)+0x11fe) [0x561b58807cee]
 11: (Monitor::dispatch_op(boost::intrusive_ptr<MonOpRequest>)+0xa5a) [0x561b5880e0ea]
 12: (Monitor::_ms_dispatch(Message*)+0x51a) [0x561b5880f2ca]
 13: (Dispatcher::ms_dispatch2(boost::intrusive_ptr<Message> const&)+0x58) [0x561b5883dab8]
 14: (DispatchQueue::entry()+0x11c2) [0x7fbe70ac6d62]
 15: (DispatchQueue::DispatchThread::entry()+0xd) [0x7fbe70b6625d]
 16: (()+0x76db) [0x7fbe6ebf76db]
 17: (clone()+0x3f) [0x7fbe6e92061f]
 NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.

--- logging levels ---
   0/ 5 none
   0/ 1 lockdep
   0/ 1 context
   1/ 1 crush
   1/ 5 mds
   1/ 5 mds_balancer
   1/ 5 mds_locker
   1/ 5 mds_log
   1/ 5 mds_log_expire
   1/ 5 mds_migrator
   0/ 1 buffer
   0/ 1 timer
   0/ 1 filer
   0/ 1 striper
   0/ 1 objecter
   0/ 5 rados
   0/ 5 rbd
   0/ 5 rbd_mirror
   0/ 5 rbd_replay
   0/ 5 rbd_rwl
   0/ 5 journaler
   0/ 5 objectcacher
   0/ 5 immutable_obj_cache
   0/ 5 client
   1/ 5 osd
   0/ 5 optracker
   0/ 5 objclass
   1/ 3 filestore
   1/ 3 journal
   0/ 0 ms
   1/ 5 mon
   0/10 monc
   1/ 5 paxos
   0/ 5 tp
   1/ 5 auth
   1/ 5 crypto
   1/ 1 finisher
   1/ 1 reserver
   1/ 5 heartbeatmap
   1/ 5 perfcounter
   1/ 5 rgw
   1/ 5 rgw_sync
   1/10 civetweb
   1/ 5 javaclient
   1/ 5 asok
   1/ 1 throttle
   0/ 0 refs
   1/ 5 compressor
   1/ 5 bluestore
   1/ 5 bluefs
   1/ 3 bdev
   1/ 5 kstore
   4/ 5 rocksdb
   4/ 5 leveldb
   4/ 5 memdb
   1/ 5 fuse
   1/ 5 mgr
   1/ 5 mgrc
   1/ 5 dpdk
   1/ 5 eventtrace
   1/ 5 prioritycache
   0/ 5 test
  -2/-2 (syslog threshold)
  -1/-1 (stderr threshold)
--- pthread ID / name mapping for recent threads ---
  7fbe60502700 / ms_dispatch
  7fbe61504700 / rocksdb:dump_st
  7fbe62506700 / fn_monstore
  7fbe63d09700 / ms_dispatch
  7fbe6650e700 / safe_timer
  7fbe69d15700 / rocksdb:low0
  7fbe6b55e700 / admin_socket
  7fbe79673540 / ceph-mon
  max_recent     10000
  max_new         1000
  log_file /var/log/ceph/ceph-mon.juju-8c5f4a-sts-stein-bionic-0.log
--- end dump of recent events ---

This is the code that leads to the issue, in cmdparse.cc,

git branch

(HEAD detached at v15.2.14)

bool cmd_getval(const cmdmap_t& cmdmap,
                const std::string& k, bool& val)
{
  /*
   * Specialized getval for booleans.  CephBool didn't exist before Nautilus,
   * so earlier clients are sent a CephChoices argdesc instead, and will
   * send us a "--foo-bar" value string for boolean arguments.
   */
  if (cmdmap.count(k)) {
    try {
      val = boost::get<bool>(cmdmap.find(k)->second);
      return true;
    } catch (boost::bad_get&) {
      try {
        std::string expected = "--" + k;
        std::replace(expected.begin(), expected.end(), '_', '-');

        std::string v_str = boost::get<std::string>(cmdmap.find(k)->second);
        if (v_str == expected) {
          val = true;
          return true;
        } else {
          throw bad_cmd_get(k, cmdmap);
        }
      } catch (boost::bad_get&) {
        throw bad_cmd_get(k, cmdmap);
      }
    }
  }
  return false;
}

}

Files

mon_abort (377 KB) mon_abort

nikhil kshirsagar, 03/16/2022 05:27 AM

Related issues 3 (0 open — 3 closed)

Actions

Copy link

Updated by nikhil kshirsagar about 2 years ago

The bad json data that crashed the mon is pasted below..


curl -k "$endpoint/request" -X POST -d '{"prefix":"auth add","entity":"client.testuser02","caps":"mon '\''allow r'\'' osd '\''allow rw pool=testpool01'\''"}'

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/bin/ceph-mon -f --cluster ceph --id juju-8c5f4a-sts-stein-bionic-0 --setus'.
Program terminated with signal SIGABRT, Aborted.
#0  raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51    ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
[Current thread is 1 (Thread 0x7f40c85fd700 (LWP 71986))]
(gdb) bt
#0  raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00005570a1df13a0 in reraise_fatal (signum=6) at ./src/global/signal_handler.cc:81
#2  handle_fatal_signal (signum=6) at ./src/global/signal_handler.cc:326
#3  <signal handler called>
#4  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#5  0x00007f40d31337f1 in __GI_abort () at abort.c:79
#6  0x00007f40d3d45957 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x00007f40d3d4bae6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#8  0x00007f40d3d4bb21 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#9  0x00007f40d3d4bd54 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#10 0x00005570a1bea218 in ceph::common::cmd_getval<std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > (cmdmap=std::map with 3 elements = {...}, k="caps", val=std::vector of length 0, capacity 0) at ./src/common/cmdparse.h:71
#11 0x00005570a1b91add in Monitor::_generate_command_map (cmdmap=std::map with 3 elements = {...}, param_str_map=std::map with 0 elements) at ./src/mon/Monitor.cc:3031
#12 0x00005570a1bc4cee in Monitor::handle_command (this=this@entry=0x5570a452b000, op=...) at ./src/mon/Monitor.cc:3335
#13 0x00005570a1bcb0ea in Monitor::dispatch_op (this=this@entry=0x5570a452b000, op=...) at ./src/mon/Monitor.cc:4494
#14 0x00005570a1bcc2ca in Monitor::_ms_dispatch (this=this@entry=0x5570a452b000, m=m@entry=0x5570b7284280) at ./src/mon/Monitor.cc:4356
#15 0x00005570a1bfaab8 in Monitor::ms_dispatch (m=0x5570b7284280, this=0x5570a452b000) at ./src/mon/Monitor.h:885
#16 Dispatcher::ms_dispatch2 (this=0x5570a452b000, m=...) at ./src/msg/Dispatcher.h:124
#17 0x00007f40d53bad62 in Messenger::ms_deliver_dispatch (m=..., this=0x5570a452f800) at ./src/msg/Messenger.h:703
#18 DispatchQueue::entry (this=0x5570a452fb20) at ./src/msg/DispatchQueue.cc:199
#19 0x00007f40d545a25d in DispatchQueue::DispatchThread::entry (this=<optimized out>) at ./src/msg/DispatchQueue.h:101
#20 0x00007f40d34eb6db in start_thread (arg=0x7f40c85fd700) at pthread_create.c:463
#21 0x00007f40d321461f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

 (gdb) p cmdmap
$1 = std::map with 3 elements = {["caps"] = {which_ = 0, storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {
          buf = "\300\211\343\246pU\000\000,\000\000\000\000\000\000\000,", '\000' <repeats 14 times>, align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}, 
  ["entity"] = {which_ = 0, storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {buf = " \234̩pU\000\000\021\000\000\000\000\000\000\000\036", '\000' <repeats 14 times>, 
          align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}, ["prefix"] = {which_ = 0, 
    storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {buf = "hC\227\253pU\000\000\b\000\000\000\000\000\000\000auth add\000\000\000\000\000\000\000", 
          align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}}
(gdb) p param_str_map
$2 = std::map with 0 elements
(gdb) 

(gdb) info locals
cv = std::vector of length 0, capacity 0
p = 
      {first = "caps", second = {which_ = 0, storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {buf = "\300\211\343\246pU\000\000,\000\000\000\000\000\000\000,", '\000' <repeats 14 times>, align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}}
(gdb) 

I think we should validate the string in 

void Monitor::handle_command(MonOpRequestRef op)
{
  ceph_assert(op->is_type_command());
  auto m = op->get_req<MMonCommand>();
  if (m->fsid != monmap->fsid) {
    dout(0) << "handle_command on fsid " << m->fsid << " != " << monmap->fsid
            << dendl;
    reply_command(op, -EPERM, "wrong fsid", 0);
    return;
  }

(gdb) f 12
#12 0x00005570a1bc4cee in Monitor::handle_command (this=this@entry=0x5570a452b000, op=...) at ./src/mon/Monitor.cc:3335
3335    in ./src/mon/Monitor.cc

(gdb) info locals
__PRETTY_FUNCTION__ = "void Monitor::handle_command(MonOpRequestRef)" 
m = <optimized out>
session = <optimized out>
__func__ = "handle_command" 
prefix = "auth add" 
fullcmd = std::vector of length 2, capacity 2 = {"auth", "add"}
cmdmap = std::map with 3 elements = {["caps"] = {which_ = 0, storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {
          buf = "\300\211\343\246pU\000\000,\000\000\000\000\000\000\000,", '\000' <repeats 14 times>, align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}, 
  ["entity"] = {which_ = 0, storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {buf = " \234̩pU\000\000\021\000\000\000\000\000\000\000\036", '\000' <repeats 14 times>, 
          align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}, ["prefix"] = {which_ = 0, 
    storage_ = {<boost::detail::aligned_storage::aligned_storage_imp<32, 8>> = {data_ = {buf = "hC\227\253pU\000\000\b\000\000\000\000\000\000\000auth add\000\000\000\000\000\000\000", 
          align_ = {<No data fields>}}}, static size = <optimized out>, static alignment = <optimized out>}}}
ss = <incomplete type>
ds = <incomplete type>
rdata = {_buffers = {_root = {next = 0x7f40c85f93e0}, _tail = 0x7f40c85f93e0}, _carriage = 0x5570a23bb9c0 <ceph::buffer::v15_2_0::list::always_empty_bptr>, _len = 0, _num = 0, static always_empty_bptr = {
    _raw = 0x0, _off = 0, _len = 0}}
rs = "unrecognized command" 
r = -22
module = "auth" 
err = "" 
format = "plain" 
f = {px = 0x0}
leader_cmd = <optimized out>
mgr_cmds = <optimized out>
mgr_cmd = <optimized out>
mon_cmd = <optimized out>
service = "auth" 
cmd_is_rw = true
param_str_map = std::map with 0 elements
(gdb)

Actions

Copy link

Updated by Brad Hubbard about 2 years ago

Project changed from Ceph to mgr
Category set to restful module

Moving to restful module so it an be triaged appropriately.

Actions

Copy link

Updated by Brad Hubbard about 2 years ago

Project changed from mgr to RADOS
Category deleted (~~restful module~~)

Moving to RADOS since it can be reproduced from a python script which takes the restful API out of the picture.

Actions

Copy link

Updated by nikhil kshirsagar about 2 years ago

File mon_abort mon_abort added

Reproduces without restful api too,

#!/usr/bin/env python3
import json
import rados
c = rados.Rados(conffile='/etc/ceph/ceph.conf')
c.connect()
cmd = json.dumps({"prefix":"auth add","entity":"client.testuser02","caps":"mon '\''allow r'\'' osd '\''allow rw pool=testpool01'\''"})
print(c.mon_command(cmd, b''))

mon logs are in the attached file mon_abort. The crash is clearly seen when this script is run.

ceph -s hung for a while then seemed to recover when i ctrl-c'd it and tried again,

root@juju-8c5f4a-sts-stein-bionic-0:/var/log/ceph# ceph -s

^CInterrupted
root@juju-8c5f4a-sts-stein-bionic-0:/var/log/ceph# ceph -s
cluster:
id: 6123c916-a12a-11ec-bc02-fa163e9f86e0
health: HEALTH_WARN
mon is allowing insecure global_id reclaim
1 monitors have not enabled msgr2
Reduced data availability: 69 pgs inactive
1921 daemons have recently crashed

services:
    mon: 1 daemons, quorum juju-8c5f4a-sts-stein-bionic-0 (age 0.814748s)
    mgr: juju-8c5f4a-sts-stein-bionic-0(active, since 26m)
    osd: 3 osds: 3 up (since 22h), 3 in

data:
    pools:   4 pools, 69 pgs
    objects: 0 objects, 0 B
    usage:   0 B used, 0 B / 0 B avail
    pgs:     100.000% pgs unknown
             69 unknown

Actions

Copy link

Updated by nikhil kshirsagar about 2 years ago

Discussed with Brad Hubbard. It seems the caps should be a list. For eg, this json works fine,

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"]}'

but this does not,

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'\''"}'

A patch of this sort may work to avoid the abort, since we will catch the exception if we fail to populate cv from the caps and call reply_command() with the appropriate message.

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index b32af4ba809..f3e771034bc 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -3028,13 +3028,20 @@ void Monitor::_generate_command_map(cmdmap_t& cmdmap,
       continue;
     if (p->first == "caps") {
       vector<string> cv;
-      if (cmd_getval(cmdmap, "caps", cv) &&
-         cv.size() % 2 == 0) {
-       for (unsigned i = 0; i < cv.size(); i += 2) {
-         string k = string("caps_") + cv[i];
-         param_str_map[k] = cv[i + 1];
-       }
-       continue;
+      try
+      {
+        if (cmd_getval(cmdmap, "caps", cv) &&
+           cv.size() % 2 == 0) {
+         for (unsigned i = 0; i < cv.size(); i += 2) {
+           string k = string("caps_") + cv[i];
+           param_str_map[k] = cv[i + 1];
+         }
+         continue;
+        }
+      }
+      catch(...)
+      {
+        reply_command(op, -EINVAL, "Could not populate caps", 0);
       }
     }
     param_str_map[p->first] = cmd_vartype_stringify(p->second);

Actions

Copy link

Updated by nikhil kshirsagar about 2 years ago

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index b32af4ba809..175ffa51ada 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -3028,13 +3028,20 @@ void Monitor::_generate_command_map(cmdmap_t& cmdmap,
       continue;
     if (p->first == "caps") {
       vector<string> cv;
-      if (cmd_getval(cmdmap, "caps", cv) &&
-         cv.size() % 2 == 0) {
-       for (unsigned i = 0; i < cv.size(); i += 2) {
-         string k = string("caps_") + cv[i];
-         param_str_map[k] = cv[i + 1];
-       }
-       continue;
+      try
+      {
+        if (cmd_getval(cmdmap, "caps", cv) &&
+           cv.size() % 2 == 0) {
+         for (unsigned i = 0; i < cv.size(); i += 2) {
+           string k = string("caps_") + cv[i];
+           param_str_map[k] = cv[i + 1];
+         }
+         continue;
+        }
+      }
+      catch(bad_cmd_get e)
+      {
+        reply_command(op, -EINVAL, e.what(), 0);
       }
     }

Actions

Copy link

Updated by nikhil kshirsagar about 2 years ago

The earlier patch fails to compile, so I have changed it to the following, which does build.

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index b32af4ba809..73106494f68 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -3332,7 +3332,14 @@ void Monitor::handle_command(MonOpRequestRef op)

   // validate user's permissions for requested command
   map<string,string> param_str_map;
-  _generate_command_map(cmdmap, param_str_map);
+  try {
+    _generate_command_map(cmdmap, param_str_map);
+  }
+  catch(bad_cmd_get e)
+  {
+    reply_command(op, -EINVAL, e.what(), 0);
+  }
+
   if (!_allowed_command(session, service, prefix, cmdmap,
                         param_str_map, mon_cmd)) {
     dout(1) << __func__ << " access denied" << dendl;

Actions

Copy link

Updated by nikhil kshirsagar about 2 years ago

I've tested the above patch, and it seems to be working as intended.


root@focal-testing:/home/nikhil/Downloads/ceph_upstream/ceph/build# ceph -s
2022-03-22T04:46:26.671+0000 7f32384c2700 -1 WARNING: all dangerous and experimental features are enabled.
2022-03-22T04:46:26.763+0000 7f32384c2700 -1 WARNING: all dangerous and experimental features are enabled.
  cluster:
    id:     cc29048a-b441-4d8c-9c35-2ede3188c292
    health: HEALTH_OK

  services:
    mon: 3 daemons, quorum a,b,c (age 21m)
    mgr: x(active, since 20m)
    mds: a:1 {0=a=up:active}
    osd: 3 osds: 3 up (since 19m), 3 in (since 19m)

  data:
    pools:   3 pools, 65 pgs
    objects: 22 objects, 2.2 KiB
    usage:   6.0 GiB used, 297 GiB / 303 GiB avail
    pgs:     65 active+clean

---

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 
(0, b'', 'added key for client.testuser02')

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'")

root@focal-testing:/home/nikhil/Downloads/ceph_upstream/ceph/build# ceph -s
2022-03-22T04:50:52.789+0000 7f56fef4f700 -1 WARNING: all dangerous and experimental features are enabled.
2022-03-22T04:50:52.801+0000 7f56fef4f700 -1 WARNING: all dangerous and experimental features are enabled.
  cluster:
    id:     cc29048a-b441-4d8c-9c35-2ede3188c292
    health: HEALTH_OK

  services:
    mon: 3 daemons, quorum a,b,c (age 25m)
    mgr: x(active, since 25m)
    mds: a:1 {0=a=up:active}
    osd: 3 osds: 3 up (since 23m), 3 in (since 23m)

  data:
    pools:   3 pools, 65 pgs
    objects: 22 objects, 2.2 KiB
    usage:   6.0 GiB used, 297 GiB / 303 GiB avail
    pgs:     65 active+clean

Actions

Copy link

Updated by Radoslaw Zarzynski about 2 years ago

Status changed from New to In Progress

Actions

Copy link

#10

Updated by Radoslaw Zarzynski about 2 years ago

Could you please send a PR for this?

Actions

Copy link

#11

Updated by nikhil kshirsagar about 2 years ago

Thanks for the comment Radoslaw!

This reproduces on master as well. Unfortunately I do not seem to have permissions to "edit" the bug once created, so I cannot change it from Ceph - v15.2.14 to master.

I've sent the PR - https://github.com/ceph/ceph/pull/45547

Actions

Copy link

#12

Updated by Brad Hubbard about 2 years ago

Affected Versions v17.0.0 added
Affected Versions deleted (~~v15.2.14~~)

Actions

Copy link

#13

Updated by Brad Hubbard about 2 years ago

Description updated (diff)

Actions

Copy link

#14

Updated by Brad Hubbard about 2 years ago

Pull request ID set to 45547

Actions

Copy link

#15

Updated by nikhil kshirsagar about 2 years ago

Reproduces on master.

Actions

Copy link

#16

Updated by Dan Mick about 2 years ago

Description updated (diff)

Actions

Copy link

#17

Updated by Neha Ojha about 2 years ago

Status changed from In Progress to Fix Under Review

Actions

Copy link

#18

Updated by Yuri Weinstein about 2 years ago

https://github.com/ceph/ceph/pull/45547 merged

Actions

Copy link

#19

Updated by Ponnuvel P about 2 years ago

Backport set to pacific,octopus

Actions

Copy link

#20

Updated by Ponnuvel P about 2 years ago

Status changed from Fix Under Review to Pending Backport
Backport changed from pacific,octopus to quincy,pacific,octopus

Actions

Copy link

#21

Updated by Backport Bot about 2 years ago

Copied to Backport #55296: pacific: malformed json in a Ceph RESTful API call can stop all ceph-mon services added

Actions

Copy link

#22

Updated by Backport Bot about 2 years ago

Copied to Backport #55297: quincy: malformed json in a Ceph RESTful API call can stop all ceph-mon services added

Actions

Copy link

#23

Updated by Backport Bot about 2 years ago

Copied to Backport #55298: octopus: malformed json in a Ceph RESTful API call can stop all ceph-mon services added

Actions

Copy link

#24

Updated by Backport Bot over 1 year ago

Tags set to backport_processed

Actions

Copy link

#25

Updated by Ilya Dryomov over 1 year ago

I don't think https://github.com/ceph/ceph/pull/45547 is a complete fix, see my comment in the PR.

Actions

Copy link

#26

Updated by nikhil kshirsagar over 1 year ago

Ilya Dryomov wrote:

I don't think https://github.com/ceph/ceph/pull/45547 is a complete fix, see my comment in the PR.

I submitted https://github.com/ceph/ceph/pull/48044 to bail out of handle_command if an exception is thrown.

Actions

Copy link

#27

Updated by nikhil kshirsagar over 1 year ago

nikhil kshirsagar wrote:

Ilya Dryomov wrote:

I don't think https://github.com/ceph/ceph/pull/45547 is a complete fix, see my comment in the PR.

I submitted https://github.com/ceph/ceph/pull/48044 to bail out of handle_command if an exception is thrown.

Created https://tracker.ceph.com/issues/57859 for the cleanup, and linked it in https://github.com/ceph/ceph/pull/48044

Actions

Copy link

#28

Updated by Radoslaw Zarzynski about 1 year ago

Status changed from Pending Backport to Resolved

Actions

Copy link

Also available in: Atom PDF

Project

General

Profile

Ceph » RADOS

Custom queries

Bug #54558

malformed json in a Ceph RESTful API call can stop all ceph-mon services

Updated by nikhil kshirsagar about 2 years ago

Updated by Brad Hubbard about 2 years ago

Updated by Brad Hubbard about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by Radoslaw Zarzynski about 2 years ago

Updated by Radoslaw Zarzynski about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by Brad Hubbard about 2 years ago

Updated by Brad Hubbard about 2 years ago

Updated by Brad Hubbard about 2 years ago

Updated by nikhil kshirsagar about 2 years ago

Updated by Dan Mick about 2 years ago

Updated by Neha Ojha about 2 years ago

Updated by Yuri Weinstein about 2 years ago

Updated by Ponnuvel P about 2 years ago

Updated by Ponnuvel P about 2 years ago

Updated by Backport Bot about 2 years ago

Updated by Backport Bot about 2 years ago

Updated by Backport Bot about 2 years ago

Updated by Backport Bot over 1 year ago

Updated by Ilya Dryomov over 1 year ago

Updated by nikhil kshirsagar over 1 year ago

Updated by nikhil kshirsagar over 1 year ago

Updated by Radoslaw Zarzynski about 1 year ago