From: Radoslaw Zarzynski Date: Tue, 25 Oct 2016 11:34:32 +0000 (+0200) Subject: rgw: sanitize length of metadata attributes. X-Git-Tag: v11.1.0~454^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ac4928aa366c2b3b3bf4a71db9c1e82f5b52a8b7;p=ceph.git rgw: sanitize length of metadata attributes. std::string and thus boost::string_ref ARE OBLIGED to carry multiple 0x00 and count them to the length of a string. We need to take that into consideration and sanitize the size of a ceph::buffer::list used to store metadata values (x-amz-meta-*, X-Container-Meta-*, etags). Otherwise we might send 0x00 to clients. Signed-off-by: Radoslaw Zarzynski --- diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index f47edf417fc..d93b0cad921 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -930,7 +930,7 @@ protected: string source_zone; string client_id; string op_id; - string etag; + ceph::buffer::list etag; off_t last_ofs; diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index 5f87d003134..cd64a82c8d6 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -6970,7 +6970,7 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& obj_ctx, real_time delete_at, string *version_id, string *ptag, - string *petag, + ceph::buffer::list *petag, struct rgw_err *err, void (*progress_cb)(off_t, void *), void *progress_data) @@ -7104,10 +7104,9 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& obj_ctx, } if (petag) { - map::iterator iter = src_attrs.find(RGW_ATTR_ETAG); + const auto iter = src_attrs.find(RGW_ATTR_ETAG); if (iter != src_attrs.end()) { - bufferlist& etagbl = iter->second; - *petag = string(etagbl.c_str(), etagbl.length()); + *petag = iter->second; } } @@ -7263,7 +7262,7 @@ int RGWRados::copy_obj(RGWObjectCtx& obj_ctx, real_time delete_at, string *version_id, string *ptag, - string *petag, + ceph::buffer::list *petag, struct rgw_err *err, void (*progress_cb)(off_t, void *), void *progress_data) @@ -7368,10 +7367,9 @@ int RGWRados::copy_obj(RGWObjectCtx& obj_ctx, } if (petag) { - map::iterator iter = attrs.find(RGW_ATTR_ETAG); + const auto iter = attrs.find(RGW_ATTR_ETAG); if (iter != attrs.end()) { - bufferlist& etagbl = iter->second; - *petag = string(etagbl.c_str(), etagbl.length()); + *petag = iter->second; } } @@ -7517,7 +7515,7 @@ int RGWRados::copy_obj_data(RGWObjectCtx& obj_ctx, real_time delete_at, string *version_id, string *ptag, - string *petag, + ceph::buffer::list *petag, struct rgw_err *err) { bufferlist first_chunk; @@ -7563,12 +7561,12 @@ int RGWRados::copy_obj_data(RGWObjectCtx& obj_ctx, } while (ofs <= end); string etag; - map::iterator iter = attrs.find(RGW_ATTR_ETAG); + auto iter = attrs.find(RGW_ATTR_ETAG); if (iter != attrs.end()) { bufferlist& bl = iter->second; etag = string(bl.c_str(), bl.length()); if (petag) { - *petag = etag; + *petag = bl; } } diff --git a/src/rgw/rgw_rados.h b/src/rgw/rgw_rados.h index b2092a160ac..0672083f2cf 100644 --- a/src/rgw/rgw_rados.h +++ b/src/rgw/rgw_rados.h @@ -2627,7 +2627,7 @@ public: ceph::real_time delete_at, string *version_id, string *ptag, - string *petag, + ceph::buffer::list *petag, struct rgw_err *err, void (*progress_cb)(off_t, void *), void *progress_data); @@ -2671,7 +2671,7 @@ public: ceph::real_time delete_at, string *version_id, string *ptag, - string *petag, + ceph::buffer::list *petag, struct rgw_err *err, void (*progress_cb)(off_t, void *), void *progress_data); @@ -2690,7 +2690,7 @@ public: ceph::real_time delete_at, string *version_id, string *ptag, - string *petag, + ceph::buffer::list *petag, struct rgw_err *err); /** diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index 23ac325566b..ec91e2d9147 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -382,11 +382,31 @@ void dump_header(struct req_state* const s, } } +static inline boost::string_ref get_sanitized_hdrval(ceph::buffer::list& raw) +{ + /* std::string and thus boost::string_ref ARE OBLIGED to carry multiple + * 0x00 and count them to the length of a string. We need to take that + * into consideration and sanitize the size of a ceph::buffer::list used + * to store metadata values (x-amz-meta-*, X-Container-Meta-*, etags). + * Otherwise we might send 0x00 to clients. */ + const char* const data = raw.c_str(); + size_t len = raw.length(); + + if (len && data[len - 1] == '\0') { + /* That's the case - the null byte has been included at the last position + * of the bufferlist. We need to restore the proper string length we'll + * pass to string_ref. */ + len--; + } + + return boost::string_ref(data, len); +} + void dump_header(struct req_state* const s, const boost::string_ref& name, ceph::buffer::list& bl) { - return dump_header(s, name, boost::string_ref(bl.c_str(), bl.length())); + return dump_header(s, name, get_sanitized_hdrval(bl)); } void dump_header(struct req_state* const s, @@ -451,8 +471,7 @@ void dump_etag(struct req_state* const s, ceph::buffer::list& bl_etag, const bool quoted) { - return dump_etag(s, boost::string_ref(bl_etag.c_str(), bl_etag.length()), - quoted); + return dump_etag(s, get_sanitized_hdrval(bl_etag), quoted); } void dump_bucket_from_state(struct req_state *s) diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 00ed83b3582..a73af280f79 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -2273,8 +2273,9 @@ void RGWCopyObj_ObjStore_S3::send_response() if (op_ret == 0) { dump_time(s, "LastModified", &mtime); - if (!etag.empty()) { - s->formatter->dump_string("ETag", etag); + std::string etag_str = etag.to_str(); + if (! etag_str.empty()) { + s->formatter->dump_string("ETag", std::move(etag_str)); } s->formatter->close_section(); rgw_flush_formatter_and_reset(s, s->formatter);