Merge pull request #38620 from smithfarm/wip-48615-octopus

octopus: Do not add sensitive information in Ceph log files

Reviewed-by: Neha Ojha <nojha@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>

src/messages/MMonCommand.h

@@ -20,6 +20,9 @@
 #include <vector>
 #include <string>
 
+using TOPNSPC::common::cmdmap_from_json;
+using TOPNSPC::common::cmd_getval;
+
 class MMonCommand : public PaxosServiceMessage {
 public:
   // weird note: prior to octopus, MgrClient would leave fsid blank when
@@ -41,10 +44,26 @@ class MMonCommand : public PaxosServiceMessage {
 public:
   std::string_view get_type_name() const override { return "mon_command"; }
   void print(std::ostream& o) const override {
+    cmdmap_t cmdmap;
+    stringstream ss;
+    string prefix;
+    cmdmap_from_json(cmd, &cmdmap, ss);
+    cmd_getval(cmdmap, "prefix", prefix);
+    // Some config values contain sensitive data, so don't log them
     o << "mon_command(";
-    for (unsigned i=0; i<cmd.size(); i++) {
-      if (i) o << ' ';
-      o << cmd[i];
+    if (prefix == "config set") {
+      string name;
+      cmd_getval(cmdmap, "name", name);
+      o << "[{prefix=" << prefix << ", name=" << name << "}]";
+    } else if (prefix == "config-key set") {
+      string key;
+      cmd_getval(cmdmap, "key", key);
+      o << "[{prefix=" << prefix << ", key=" << key << "}]";
+    } else {
+      for (unsigned i=0; i<cmd.size(); i++) {
+        if (i) o << ' ';
+        o << cmd[i];
+      }
     }
     o << " v " << version << ")";
   }

src/messages/MMonCommandAck.h

@@ -17,6 +17,9 @@
 
 #include "messages/PaxosServiceMessage.h"
 
+using TOPNSPC::common::cmdmap_from_json;
+using TOPNSPC::common::cmd_getval;
+
 class MMonCommandAck : public PaxosServiceMessage {
 public:
   std::vector<std::string> cmd;
@@ -33,7 +36,28 @@ class MMonCommandAck : public PaxosServiceMessage {
 public:
   std::string_view get_type_name() const override { return "mon_command"; }
   void print(std::ostream& o) const override {
-    o << "mon_command_ack(" << cmd << "=" << r << " " << rs << " v" << version << ")";
+    cmdmap_t cmdmap;
+    stringstream ss;
+    string prefix;
+    cmdmap_from_json(cmd, &cmdmap, ss);
+    cmd_getval(cmdmap, "prefix", prefix);
+    // Some config values contain sensitive data, so don't log them
+    o << "mon_command_ack(";
+    if (prefix == "config set") {
+      string name;
+      cmd_getval(cmdmap, "name", name);
+      o << "[{prefix=" << prefix
+        << ", name=" << name << "}]"
+        << "=" << r << " " << rs << " v" << version << ")";
+    } else if (prefix == "config-key set") {
+      string key;
+      cmd_getval(cmdmap, "key", key);
+      o << "[{prefix=" << prefix << ", key=" << key << "}]"
+        << "=" << r << " " << rs << " v" << version << ")";
+    } else {
+      o << cmd;
+    }
+    o << "=" << r << " " << rs << " v" << version << ")";
   }
 
   void encode_payload(uint64_t features) override {

src/mgr/ActivePyModules.cc

@@ -612,7 +612,10 @@ PyObject *ActivePyModules::get_typed_config(
         derr << "Module '" << module_name << "' is not available" << dendl;
         Py_RETURN_NONE;
     }
-    dout(10) << __func__ << " " << final_key << " found: " << value << dendl;
+    // removing value to hide sensitive data going into mgr logs
+    // leaving this for debugging purposes
+    // dout(10) << __func__ << " " << final_key << " found: " << value << dendl;
+    dout(10) << __func__ << " " << final_key << " found" << dendl;
     return module->get_typed_option_value(key, value);
   }
   PyEval_RestoreThread(tstate);

src/mgr/MgrStandby.cc

@@ -131,7 +131,10 @@ int MgrStandby::init()
   // We must register our config callback before calling init(), so
   // that we see the initial configuration message
   monc.register_config_callback([this](const std::string &k, const std::string &v){
-      dout(10) << "config_callback: " << k << " : " << v << dendl;
+      // removing value to hide sensitive data going into mgr logs
+      // leaving this for debugging purposes
+      // dout(10) << "config_callback: " << k << " : " << v << dendl;
+      dout(10) << "config_callback: " << k << " : " << dendl;
       if (k.substr(0, 4) == "mgr/") {
 	const std::string global_key = PyModule::config_prefix + k.substr(4);
         py_module_registry.handle_config(global_key, v);

src/mgr/PyModuleRegistry.cc

@@ -424,7 +424,10 @@ void PyModuleRegistry::handle_config(const std::string &k, const std::string &v)
   std::lock_guard l(module_config.lock);
 
   if (!v.empty()) {
-    dout(10) << "Loaded module_config entry " << k << ":" << v << dendl;
+    // removing value to hide sensitive data going into mgr logs
+    // leaving this for debugging purposes
+    // dout(10) << "Loaded module_config entry " << k << ":" << v << dendl;
+    dout(10) << "Loaded module_config entry " << k << ":" << dendl;
     module_config.config[k] = v;
   } else {
     module_config.config.erase(k);

src/mon/ConfigMonitor.cc

@@ -801,13 +801,6 @@ void ConfigMonitor::load_config()
     it->next();
   }
   dout(10) << __func__ << " got " << num << " keys" << dendl;
-  dout(20) << __func__ << " config map:\n";
-  JSONFormatter jf(true);
-  jf.open_object_section("config_map");
-  config_map.dump(&jf);
-  jf.close_section();
-  jf.flush(*_dout);
-  *_dout << dendl;
 
   // refresh our own config
   {
@@ -884,8 +877,9 @@ bool ConfigMonitor::refresh_config(MonSession *s)
     dout(20) << __func__ << " no change, " << out << dendl;
     return false;
   }
-
-  dout(20) << __func__ << " " << out << dendl;
+  // removing this to hide sensitive data going into logs
+  // leaving this for debugging purposes
+ //  dout(20) << __func__ << " " << out << dendl;
   s->last_config = std::move(out);
   s->any_config = true;
   return true;

src/mon/Monitor.cc

@@ -3336,18 +3336,20 @@ void Monitor::handle_command(MonOpRequestRef op)
   if (!_allowed_command(session, service, prefix, cmdmap,
                         param_str_map, mon_cmd)) {
     dout(1) << __func__ << " access denied" << dendl;
-    (cmd_is_rw ? audit_clog->info() : audit_clog->debug())
-      << "from='" << session->name << " " << session->addrs << "' "
-      << "entity='" << session->entity_name << "' "
-      << "cmd=" << m->cmd << ":  access denied";
+    if (prefix != "config set" && prefix != "config-key set")
+      (cmd_is_rw ? audit_clog->info() : audit_clog->debug())
+        << "from='" << session->name << " " << session->addrs << "' "
+        << "entity='" << session->entity_name << "' "
+        << "cmd=" << m->cmd << ":  access denied";
     reply_command(op, -EACCES, "access denied", 0);
     return;
   }
 
-  (cmd_is_rw ? audit_clog->info() : audit_clog->debug())
-      << "from='" << session->name << " " << session->addrs << "' "
-      << "entity='" << session->entity_name << "' "
-      << "cmd=" << m->cmd << ": dispatch";
+  if (prefix != "config set" && prefix != "config-key set")
+    (cmd_is_rw ? audit_clog->info() : audit_clog->debug())
+        << "from='" << session->name << " " << session->addrs << "' "
+        << "entity='" << session->entity_name << "' "
+        << "cmd=" << m->cmd << ": dispatch";
 
   // compat kludge for legacy clients trying to tell commands that are
   // new.  see bottom of MonCommands.h.  we need to handle both (1)

src/mon/Monitor.h

@@ -56,6 +56,7 @@
 #include "mon/MonOpRequest.h"
 #include "common/WorkQueue.h"
 
+using namespace TOPNSPC::common;
 
 #define CEPH_MON_PROTOCOL     13 /* cluster internal */
 
@@ -839,7 +840,13 @@ class Monitor : public Dispatcher,
             ss << "session dropped for command ";
           }
         }
-        ss << "cmd='" << m->cmd << "': finished";
+        cmdmap_t cmdmap;
+        stringstream ds;
+        string prefix;
+        cmdmap_from_json(m->cmd, &cmdmap, ds);
+        cmd_getval(cmdmap, "prefix", prefix);
+        if (prefix != "config set" && prefix != "config-key set")
+          ss << "cmd='" << m->cmd << "': finished";
 
         mon->audit_clog->info() << ss.str();
 	mon->reply_command(op, rc, rs, rdata, version);