From 7872a831783e17dcc4d0aa70cffc256afb664f7a Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Mon, 12 Feb 2018 18:05:03 +0100 Subject: [PATCH] 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 --- src/rgw/rgw_asio_client.cc | 3 ++- src/rgw/rgw_asio_client.h | 2 +- src/rgw/rgw_civetweb.cc | 12 ++++++++++-- src/rgw/rgw_civetweb.h | 2 +- src/rgw/rgw_client_io.cc | 8 ++++++-- src/rgw/rgw_client_io.h | 6 +++--- src/rgw/rgw_fcgi.cc | 3 ++- src/rgw/rgw_fcgi.h | 2 +- src/rgw/rgw_lib.h | 3 ++- src/rgw/rgw_loadgen.cc | 3 ++- src/rgw/rgw_loadgen.h | 2 +- src/rgw/rgw_process.cc | 14 +++++++++----- 12 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/rgw/rgw_asio_client.cc b/src/rgw/rgw_asio_client.cc index fd36719e477..57dbd6b189b 100644 --- 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 038be51411c..eeed4ee8051 100644 --- 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 f4d17d721c1..098153c1059 100644 --- 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 0160c03a42d..4df4d19e0a4 100644 --- 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 dd116c427a8..49811a9d201 100644 --- 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 1881192acac..8ee423b2ef5 100644 --- 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 68eee476033..a52ea5097bd 100644 --- 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 52d7cacabcf..7f8e61a3996 100644 --- 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 9ec3f35b6d9..7ab037f5934 100644 --- 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 0badc78e84b..fd66469e4d7 100644 --- 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 dd9e2d8f6af..72aace76c3e 100644 --- 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 ecea70c055c..dd4ca9b6922 100644 --- 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; -- 2.39.5