rgw: make init env methods return an error

Since web frontends may signal an error when requests are malformed or so, let
us double check this and raise errors early. The current user of this is
civetweb frontend; which can potentially return null from `parse_http_headers`
when a HTTP header without a ":" is supplied at which point headers.value is
null which can lead to undefined behaviour later in RGW.

Fixes: http://tracker.ceph.com/issues/23039
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
(cherry picked from commit 7872a831783e17dcc4d0aa70cffc256afb664f7a)

src/rgw/rgw_lib.h:
  Added https://github.com/ceph/ceph/pull/19065 to the backport as it is harmless

diff --git a/src/rgw/rgw_civetweb.cc b/src/rgw/rgw_civetweb.cc
index 372c7fc99fc0a20b15b6fe70c243f6421208300f..e20f074795bbb7f7009493c5d8bcae6c8bf5a21d 100644 (file)
--- a/src/rgw/rgw_civetweb.cc
+++ b/src/rgw/rgw_civetweb.cc
@@ -103,17 +103,23 @@ int RGWMongoose::complete_request()
   return 0;
 }
 
-void RGWMongoose::init_env(CephContext *cct)
+int RGWMongoose::init_env(CephContext *cct)
 {
   env.init(cct);
   struct mg_request_info *info = mg_get_request_info(conn);
 
-  if (!info)
-    return;
+  if (!info) {
+    return -EINVAL;
+  }
 
   for (int i = 0; i < info->num_headers; i++) {
     struct mg_request_info::mg_header *header = &info->http_headers[i];
 
+    if (header->name == nullptr || header->value == nullptr) {
+      lderr(cct) << "client supplied malformed headers" << dendl;
+      return -EINVAL;
+    }
+
     if (strcasecmp(header->name, "content-length") == 0) {
       env.set("CONTENT_LENGTH", header->value);
       continue;
@@ -165,6 +171,7 @@ void RGWMongoose::init_env(CephContext *cct)
   if (info->is_ssl) {
     env.set("SERVER_PORT_SECURE", port_buf);
   }
+  return 0;
 }
 
 int RGWMongoose::send_status(int status, const char *status_name)

diff --git a/src/rgw/rgw_civetweb.h b/src/rgw/rgw_civetweb.h
index 8b553928c01a9a56e23121aa2296d96c521229a3..f8a7cb5756e0cbfb1a0aee1346f4c02ec0b790b9 100644 (file)
--- a/src/rgw/rgw_civetweb.h
+++ b/src/rgw/rgw_civetweb.h
@@ -27,7 +27,7 @@ class RGWMongoose : public RGWStreamIO
   bool explicit_conn_close;
 
 public:
-  void init_env(CephContext *cct);
+  int init_env(CephContext *cct) override;
 
   int write_data(const char *buf, int len);
   int read_data(char *buf, int len);

diff --git a/src/rgw/rgw_client_io.cc b/src/rgw/rgw_client_io.cc
index b326a61b7faf0517ef7518ea7f310ad328f365bf..571b1409f5f9eb20485567a91b262cf16c85930c 100644 (file)
--- a/src/rgw/rgw_client_io.cc
+++ b/src/rgw/rgw_client_io.cc
@@ -9,8 +9,11 @@
 
 #define dout_subsys ceph_subsys_rgw
 
-void RGWClientIO::init(CephContext *cct) {
-  init_env(cct);
+int RGWClientIO::init(CephContext *cct) {
+  int init_error = init_env(cct);
+
+  if (init_error != 0)
+    return init_error;
 
   if (cct->_conf->subsys.should_gather(ceph_subsys_rgw, 20)) {
     std::map<string, string, ltstr_nocase>& env_map = env.get_map();
@@ -20,6 +23,8 @@ void RGWClientIO::init(CephContext *cct) {
       ldout(cct, 20) << iter->first << "=" << iter->second << dendl;
     }
   }
+
+  return init_error;
 }
 
 int RGWStreamIO::print(const char *format, ...)
@@ -85,7 +90,6 @@ int RGWStreamIO::read(char *buf, int max, int *actual, bool hash /* = false */)
     }
     calc_hash_sha256_update_stream(sha256_hash, buf, *actual);
   }
-
   return 0;
 }
 

diff --git a/src/rgw/rgw_client_io.h b/src/rgw/rgw_client_io.h
index 09cf332b1ecae46fbd24c1eb481f84f279517eb0..e2c22604c035a88923a7c414474f8e7ebffeb355 100644 (file)
--- a/src/rgw/rgw_client_io.h
+++ b/src/rgw/rgw_client_io.h
@@ -17,14 +17,14 @@ class RGWClientIO {
 
 protected:
   RGWEnv env;
-
-  virtual void init_env(CephContext *cct) = 0;
+  virtual int init_env(CephContext *cct) = 0;
 
 public:
   virtual ~RGWClientIO() {}
   RGWClientIO() : _account(false) {}
 
-  void init(CephContext *cct);
+  /* Initialize the BasicClient and inject CephContext. */
+  int init(CephContext *cct);
   RGWEnv& get_env() { return env; }
 
   bool account() { return _account; }

diff --git a/src/rgw/rgw_fcgi.cc b/src/rgw/rgw_fcgi.cc
index 4a1788f03f1c6cbd38220776d839fb8be3ac0c4e..9afaeb6c00caca4f8334694a64cd1ed7334b4c92 100644 (file)
--- a/src/rgw/rgw_fcgi.cc
+++ b/src/rgw/rgw_fcgi.cc
@@ -26,9 +26,10 @@ void RGWFCGX::flush()
   FCGX_FFlush(fcgx->out);
 }
 
-void RGWFCGX::init_env(CephContext *cct)
+int RGWFCGX::init_env(CephContext* const cct)
 {
   env.init(cct, (char **)fcgx->envp);
+  return 0;
 }
 
 int RGWFCGX::send_status(int status, const char *status_name)

diff --git a/src/rgw/rgw_fcgi.h b/src/rgw/rgw_fcgi.h
index d83e03f5e6372194101a39bf81d9bebf33dcfdbb..e895017406c98166be3bd2322ed7520cfb1b40ba 100644 (file)
--- a/src/rgw/rgw_fcgi.h
+++ b/src/rgw/rgw_fcgi.h
@@ -22,7 +22,7 @@ class RGWFCGX : public RGWStreamIO
   int status_num;
 
 protected:
-  void init_env(CephContext *cct);
+  int init_env(CephContext *cct) override;
   int write_data(const char *buf, int len);
   int read_data(char *buf, int len);
 

diff --git a/src/rgw/rgw_lib.h b/src/rgw/rgw_lib.h
index 46014477bea59149b53802ba3d6726327a94ec38..6da048aea664301db6d13e5be35fddd2b74cd593 100644 (file)
--- a/src/rgw/rgw_lib.h
+++ b/src/rgw/rgw_lib.h
@@ -62,7 +62,11 @@ namespace rgw {
     RGWLibIO(const RGWUserInfo &_user_info)
       : user_info(_user_info) {}
 
-    virtual void init_env(CephContext *cct) {}
+
+    int init_env(CephContext *cct) override {
+      env.init(cct);
+      return 0;
+    }
 
     const RGWUserInfo& get_user() {
       return user_info;

diff --git a/src/rgw/rgw_loadgen.cc b/src/rgw/rgw_loadgen.cc
index 14f63a195832979919f733c2daf7e79dc54f7da6..20657fe46ffebba92b60d7621a4789333e45cd0d 100644 (file)
--- a/src/rgw/rgw_loadgen.cc
+++ b/src/rgw/rgw_loadgen.cc
@@ -65,7 +65,7 @@ int RGWLoadGenIO::complete_request()
   return 0;
 }
 
-void RGWLoadGenIO::init_env(CephContext *cct)
+int RGWLoadGenIO::init_env(CephContext *cct)
 {
   env.init(cct);
 
@@ -90,6 +90,7 @@ void RGWLoadGenIO::init_env(CephContext *cct)
   char port_buf[16];
   snprintf(port_buf, sizeof(port_buf), "%d", req->port);
   env.set("SERVER_PORT", port_buf);
+  return 0;
 }
 
 int RGWLoadGenIO::send_status(int status, const char *status_name)

diff --git a/src/rgw/rgw_loadgen.h b/src/rgw/rgw_loadgen.h
index 01b2161606ffbd5eaec467b51c90e708ea05e1ec..8ce613dc294a61a75277433308901af71744d72b 100644 (file)
--- a/src/rgw/rgw_loadgen.h
+++ b/src/rgw/rgw_loadgen.h
@@ -31,7 +31,7 @@ class RGWLoadGenIO : public RGWStreamIO
   uint64_t left_to_read;
   RGWLoadGenRequestEnv *req;
 public:
-  void init_env(CephContext *cct);
+  int init_env(CephContext *cct) override;
 
   int write_data(const char *buf, int len);
   int read_data(char *buf, int len);

diff --git a/src/rgw/rgw_process.cc b/src/rgw/rgw_process.cc
index 97b4d13814a7599ec4ef1f9249e0881b7f7bd74d..6bb8b91f3fcf97f2686b1a35185b97122df72258 100644 (file)
--- a/src/rgw/rgw_process.cc
+++ b/src/rgw/rgw_process.cc
@@ -35,9 +35,7 @@ void RGWProcess::RGWWQ::_dump_queue()
 int process_request(RGWRados* store, RGWREST* rest, RGWRequest* req,
                    RGWStreamIO* client_io, OpsLogSocket* olog)
 {
-  int ret = 0;
-
-  client_io->init(g_ceph_context);
+  int ret = client_io->init(g_ceph_context);
 
   req->log_init();
 
@@ -55,20 +53,26 @@ int process_request(RGWRados* store, RGWREST* rest, RGWRequest* req,
   RGWObjectCtx rados_ctx(store, s);
   s->obj_ctx = &rados_ctx;
 
+  if (ret < 0) {
+    s->cio = client_io;
+    abort_early(s, nullptr, ret, nullptr);
+    return ret;
+  }
+
   s->req_id = store->unique_id(req->id);
   s->trans_id = store->unique_trans_id(req->id);
   s->host_id = store->host_id;
 
   req->log_format(s, "initializing for trans_id = %s", s->trans_id.c_str());
 
-  RGWOp* op = NULL;
+  RGWOp* op = nullptr;
   int init_error = 0;
   bool should_log = false;
   RGWRESTMgr *mgr;
   RGWHandler_REST *handler = rest->get_handler(store, s, client_io, &mgr,
                                              &init_error);
   if (init_error != 0) {
-    abort_early(s, NULL, init_error, NULL);
+    abort_early(s, nullptr, init_error, nullptr);
     goto done;
   }
   dout(10) << "handler=" << typeid(*handler).name() << dendl;