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>

diff --git a/src/rgw/rgw_asio_client.cc b/src/rgw/rgw_asio_client.cc
index fd36719e477685093de29f91f080bb2c96885c18..57dbd6b189bf2eb40b312d5b9021dfde9b521dab 100644 (file)
--- a/src/rgw/rgw_asio_client.cc
+++ b/src/rgw/rgw_asio_client.cc
@@ -20,7 +20,7 @@ ClientIO::ClientIO(tcp::socket& socket,
 
 ClientIO::~ClientIO() = default;
 
-void ClientIO::init_env(CephContext *cct)
+int ClientIO::init_env(CephContext *cct)
 {
   env.init(cct);
 
@@ -79,6 +79,7 @@ void ClientIO::init_env(CephContext *cct)
   env.set("REMOTE_ADDR", socket.remote_endpoint().address().to_string());
   // TODO: set SERVER_PORT_SECURE if using ssl
   // TODO: set REMOTE_USER if authenticated
+  return 0;
 }
 
 size_t ClientIO::write_data(const char* buf, size_t len)

diff --git a/src/rgw/rgw_asio_client.h b/src/rgw/rgw_asio_client.h
index 038be51411c6a443bece5459fe878ab0e69880cf..eeed4ee8051e28fa9e5dac1a066ec37e2c4f0d50 100644 (file)
--- a/src/rgw/rgw_asio_client.h
+++ b/src/rgw/rgw_asio_client.h
@@ -36,7 +36,7 @@ class ClientIO : public io::RestfulClient,
            beast::flat_buffer& buffer);
   ~ClientIO() override;
 
-  void init_env(CephContext *cct) override;
+  int init_env(CephContext *cct) override;
   size_t complete_request() override;
   void flush() override;
   size_t send_status(int status, const char *status_name) override;

diff --git a/src/rgw/rgw_civetweb.cc b/src/rgw/rgw_civetweb.cc
index f4d17d721c18d5308504f21540d16727dbe93891..098153c10596c35a3ba74f2db1cdd9a958fe1297 100644 (file)
--- a/src/rgw/rgw_civetweb.cc
+++ b/src/rgw/rgw_civetweb.cc
@@ -67,17 +67,24 @@ size_t RGWCivetWeb::complete_request()
   return 0;
 }
 
-void RGWCivetWeb::init_env(CephContext *cct)
+int RGWCivetWeb::init_env(CephContext *cct)
 {
   env.init(cct);
   const struct mg_request_info* info = mg_get_request_info(conn);
 
   if (! info) {
-    return;
+    // request info is NULL; we have no info about the connection
+    return -EINVAL;
   }
 
   for (int i = 0; i < info->num_headers; i++) {
     const struct mg_request_info::mg_header* header = &info->http_headers[i];
+
+    if (header->name == nullptr || header->value==nullptr) {
+      lderr(cct) << "client supplied malformatted headers" << dendl;
+      return -EINVAL;
+    }
+
     const boost::string_ref name(header->name);
     const auto& value = header->value;
 
@@ -130,6 +137,7 @@ void RGWCivetWeb::init_env(CephContext *cct)
   if (info->is_ssl) {
     env.set("SERVER_PORT_SECURE", port_buf);
   }
+  return 0;
 }
 
 size_t RGWCivetWeb::send_status(int status, const char *status_name)

diff --git a/src/rgw/rgw_civetweb.h b/src/rgw/rgw_civetweb.h
index 0160c03a42d981d97df6e79ee563e060c21b2910..4df4d19e0a494a9702884e9e642e429e2d9a26a7 100644 (file)
--- a/src/rgw/rgw_civetweb.h
+++ b/src/rgw/rgw_civetweb.h
@@ -27,7 +27,7 @@ class RGWCivetWeb : public rgw::io::RestfulClient,
   size_t dump_date_header();
 
 public:
-  void init_env(CephContext *cct) override;
+  [[nodiscard]] int init_env(CephContext *cct) override;
 
   size_t send_status(int status, const char *status_name) override;
   size_t send_100_continue() override;

diff --git a/src/rgw/rgw_client_io.cc b/src/rgw/rgw_client_io.cc
index dd116c427a8e3a3ccb95db09f5b8075a077285fc..49811a9d201f0980f1131514f5b73accd23e00a6 100644 (file)
--- a/src/rgw/rgw_client_io.cc
+++ b/src/rgw/rgw_client_io.cc
@@ -13,8 +13,11 @@
 namespace rgw {
 namespace io {
 
-void BasicClient::init(CephContext *cct) {
-  init_env(cct);
+[[nodiscard]] int BasicClient::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)) {
     const auto& env_map = get_env().get_map();
@@ -24,6 +27,7 @@ void BasicClient::init(CephContext *cct) {
       ldout(cct, 20) << iter.first << "=" << (x) << dendl;
     }
   }
+  return init_error;
 }
 
 } /* namespace io */

diff --git a/src/rgw/rgw_client_io.h b/src/rgw/rgw_client_io.h
index 1881192acac890d4efdfcf00212e62c0696419c2..8ee423b2ef51d32e25b27f800f16c373235ca87b 100644 (file)
--- a/src/rgw/rgw_client_io.h
+++ b/src/rgw/rgw_client_io.h
@@ -28,13 +28,13 @@ using Exception = std::system_error;
  * interacted with. */
 class BasicClient {
 protected:
-  virtual void init_env(CephContext *cct) = 0;
+  virtual int init_env(CephContext *cct) = 0;
 
 public:
   virtual ~BasicClient() = default;
 
   /* Initialize the BasicClient and inject CephContext. */
-  void init(CephContext *cct);
+  int init(CephContext *cct);
 
   /* Return the RGWEnv describing the environment that a given request lives in.
    * The method does not throw exceptions. */
@@ -197,7 +197,7 @@ protected:
     decoratee = &new_dec;
   }
 
-  void init_env(CephContext *cct) override {
+  int init_env(CephContext *cct) override {
     return get_decoratee().init_env(cct);
   }
 

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

diff --git a/src/rgw/rgw_fcgi.h b/src/rgw/rgw_fcgi.h
index 52d7cacabcfa1f043375f7f3f381347b7b79c35f..7f8e61a3996c2009f7df8afac589c8eb72132849 100644 (file)
--- a/src/rgw/rgw_fcgi.h
+++ b/src/rgw/rgw_fcgi.h
@@ -27,7 +27,7 @@ public:
       txbuf(*this) {
   }
 
-  void init_env(CephContext* cct) override;
+  int init_env(CephContext* cct) override;
   size_t send_status(int status, const char* status_name) override;
   size_t send_100_continue() override;
   size_t send_header(const boost::string_ref& name,

diff --git a/src/rgw/rgw_lib.h b/src/rgw/rgw_lib.h
index 9ec3f35b6d9742bc0e375a48f0e894147b19e23e..7ab037f5934a2fe1647ae9d72da7d251bbb03e27 100644 (file)
--- a/src/rgw/rgw_lib.h
+++ b/src/rgw/rgw_lib.h
@@ -63,8 +63,9 @@ namespace rgw {
     RGWLibIO(const RGWUserInfo &_user_info)
       : user_info(_user_info) {}
 
-    void init_env(CephContext *cct) override {
+    int init_env(CephContext *cct) override {
       env.init(cct);
+      return 0;
     }
 
     const RGWUserInfo& get_user() {

diff --git a/src/rgw/rgw_loadgen.cc b/src/rgw/rgw_loadgen.cc
index 0badc78e84beb7ff252fdf73766ac755f8041ce1..fd66469e4d7d99116879c8b77b1e3e56df927de5 100644 (file)
--- a/src/rgw/rgw_loadgen.cc
+++ b/src/rgw/rgw_loadgen.cc
@@ -71,7 +71,7 @@ size_t RGWLoadGenIO::complete_request()
   return 0;
 }
 
-void RGWLoadGenIO::init_env(CephContext *cct)
+int RGWLoadGenIO::init_env(CephContext *cct)
 {
   env.init(cct);
 
@@ -96,6 +96,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;
 }
 
 size_t RGWLoadGenIO::send_status(const int status,

diff --git a/src/rgw/rgw_loadgen.h b/src/rgw/rgw_loadgen.h
index dd9e2d8f6afc18f69772c57087b45e1341f25897..72aace76c3e86237492ab9e788c3db88db24b547 100644 (file)
--- a/src/rgw/rgw_loadgen.h
+++ b/src/rgw/rgw_loadgen.h
@@ -38,7 +38,7 @@ class RGWLoadGenIO : public rgw::io::RestfulClient
   RGWLoadGenRequestEnv* req;
   RGWEnv env;
 
-  void init_env(CephContext *cct) override;
+  int init_env(CephContext *cct) override;
   size_t read_data(char *buf, size_t len);
   size_t write_data(const char *buf, size_t len);
 

diff --git a/src/rgw/rgw_process.cc b/src/rgw/rgw_process.cc
index ecea70c055cb8b6bfd621f941474bdf648159e74..dd4ca9b6922bd5a6a057f77dd46874869aa5648a 100644 (file)
--- a/src/rgw/rgw_process.cc
+++ b/src/rgw/rgw_process.cc
@@ -118,9 +118,7 @@ int process_request(RGWRados* const store,
                     OpsLogSocket* const olog,
                     int* http_ret)
 {
-  int ret = 0;
-
-  client_io->init(g_ceph_context);
+  int ret = client_io->init(g_ceph_context);
 
   req->log_init();
 
@@ -138,13 +136,19 @@ int process_request(RGWRados* const store,
   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;
@@ -153,7 +157,7 @@ int process_request(RGWRados* const store,
                                                frontend_prefix,
                                                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;