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 <rzarzynski@mirantis.com>

diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h
index f47edf417fc2dd0b4c7124a186cfca8e01d0e8c5..d93b0cad921b707b6aef5a57850461a703cc4f04 100644 (file)
--- 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 5f87d0031344ddd871a2cd2aa3720218dbb92e96..cd64a82c8d69dc83b889378fda52c612075715d8 100644 (file)
--- 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<string, bufferlist>::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<string, bufferlist>::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<string, bufferlist>::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 b2092a160ac09f2da1997f5ed4175ed597092b29..0672083f2cf75e3414f08e11fe817644737240a3 100644 (file)
--- 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 23ac325566bdcb108fbc0677b6a2928556bb9214..ec91e2d9147afd4f5639169cbda118db2f9b0a12 100644 (file)
--- 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 00ed83b3582ffdbf431461626421009fd98c479b..a73af280f7954d74f7581a1060267ffdd0f4772c 100644 (file)
--- 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);