rgw: reduce string copies in tempURL processing

Introduce a helper class encapsulating the temp URL HMAC computation,
and also save it for later computation.  Use the underlying HMACSHA1
implementation to compute the HMAC incrementally (saves a copy of all
the parameters).

Also avoid some other string copying in the logic prior to taking the
HMAC, in one case by remembering to use auto& in a std::map range-for
loop, and using boost::string_ref to share ReqState::request_uri and
conditionally suffix of same.

Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>

Conflicts:
	src/rgw/rgw_swift.cc

1 files changed, 56 insertions, 26 deletions

diff --git a/src/rgw/rgw_swift_auth.cc b/src/rgw/rgw_swift_auth.cc
index 43816b7a41a..190e6162475 100644
--- a/src/rgw/rgw_swift_auth.cc
+++ b/src/rgw/rgw_swift_auth.cc
@@ -1,6 +1,8 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 
+#include <boost/utility/string_ref.hpp>
+
 #include "rgw_swift_auth.h"
 #include "rgw_rest.h"
 
@@ -136,29 +138,52 @@ std::string extract_swift_subuser(const std::string& swift_user_name) {
   }
 }
 
-std::string RGWTempURLAuthEngine::generate_signature(const string& key,
-                                                     const string& method,
-                                                     const string& path,
-                                                     const string& expires) const
+class TempURLSig
 {
-  const string str = method + "\n" + expires + "\n" + path;
-  ldout(cct, 20) << "temp url signature (plain text): " << str << dendl;
+private:
+  static constexpr uint32_t output_size =
+    CEPH_CRYPTO_HMACSHA1_DIGESTSIZE * 2 + 1;
 
-  /* unsigned */ char dest[CEPH_CRYPTO_HMACSHA1_DIGESTSIZE];
-  calc_hmac_sha1(key.c_str(), key.size(),
-                 str.c_str(), str.size(),
-                 dest);
+  unsigned char dest[CEPH_CRYPTO_HMACSHA1_DIGESTSIZE]; // 20
+  char dest_str[output_size];
 
-  char dest_str[CEPH_CRYPTO_HMACSHA1_DIGESTSIZE * 2 + 1];
-  buf_to_hex((const unsigned char *)dest, sizeof(dest), dest_str);
+public:
+  TempURLSig() {}
 
-  return dest_str;
-}
+  const char* calc(const string& key,
+		  const string& method,
+		  const boost::string_ref& path,
+		  const string& expires) {
+
+    using ceph::crypto::HMACSHA1;
+    using UCHARPTR = const unsigned char*;
+
+    HMACSHA1 hmac((UCHARPTR) key.c_str(), key.size());
+    hmac.Update((UCHARPTR) method.c_str(), method.size());
+    hmac.Update((UCHARPTR) "\n", 1);
+    hmac.Update((UCHARPTR) expires.c_str(), expires.size());
+    hmac.Update((UCHARPTR) "\n", 1);
+    hmac.Update((UCHARPTR) path.data(), path.size());
+    hmac.Final(dest);
+
+    buf_to_hex((UCHARPTR) dest, sizeof(dest), dest_str);
+
+    return dest_str;
+  }
+
+  int comp(const std::string& rhs) {
+    /* never allow out-of-range exception */
+    if (rhs.size() < output_size)
+      return -1;
+    return rhs.compare(0 /* pos */,  output_size, dest_str);
+  }
+
+}; /* TempURLSig */
 
 RGWAuthApplier::aplptr_t RGWTempURLAuthEngine::authenticate() const
 {
-  const string temp_url_sig = s->info.args.get("temp_url_sig");
-  const string temp_url_expires = s->info.args.get("temp_url_expires");
+  const string& temp_url_sig = s->info.args.get("temp_url_sig");
+  const string& temp_url_expires = s->info.args.get("temp_url_expires");
   if (temp_url_sig.empty() || temp_url_expires.empty()) {
     return nullptr;
   }
@@ -184,13 +209,18 @@ RGWAuthApplier::aplptr_t RGWTempURLAuthEngine::authenticate() const
   /* We need to verify two paths because of compliance with Swift, Tempest
    * and old versions of RadosGW. The second item will have the prefix
    * of Swift API entry point removed. */
+
+  /* XXX can we search this ONCE? */
   const size_t pos = g_conf->rgw_swift_url_prefix.find_last_not_of('/') + 1;
-  const std::vector<std::string> allowed_paths = {
-    s->info.request_uri,
-    s->info.request_uri.substr(pos + 1)
+  boost::string_ref ref_uri = s->info.request_uri;
+  const vector<boost::string_ref> allowed_paths = {
+    ref_uri,
+    ref_uri.substr(pos + 1)
   };
 
   /* Account owner calculates the signature also against a HTTP method. */
+  /* XXX Boost 1.6 small_vector--or maybe, could we select from a few static
+   * vectors? */
   std::vector<std::string> allowed_methods;
   if (strcmp("HEAD", s->info.method) == 0) {
     /* HEAD requests are specially handled. */
@@ -200,7 +230,8 @@ RGWAuthApplier::aplptr_t RGWTempURLAuthEngine::authenticate() const
   }
 
   /* Need to try each combination of keys, allowed path and methods. */
-  for (const auto kv : owner_info.temp_url_keys) {
+  TempURLSig sig_helper;
+  for (const auto& kv : owner_info.temp_url_keys) {
     const int temp_url_key_num = kv.first;
     const string& temp_url_key = kv.second;
 
@@ -208,17 +239,16 @@ RGWAuthApplier::aplptr_t RGWTempURLAuthEngine::authenticate() const
       continue;
     }
 
-    for (const auto path : allowed_paths) {
-      for (const auto method : allowed_methods) {
-        const std::string local_sig = generate_signature(temp_url_key,
-                                                         method, path,
-                                                         temp_url_expires);
+    for (const auto& path : allowed_paths) {
+      for (const auto& method : allowed_methods) {
+        const char* local_sig = sig_helper.calc(temp_url_key, method, path,
+						temp_url_expires);
 
         ldout(s->cct, 20) << "temp url signature [" << temp_url_key_num
                           << "] (calculated): " << local_sig
                           << dendl;
 
-        if (local_sig != temp_url_sig) {
+        if (!!sig_helper.comp(temp_url_sig)) {
           ldout(s->cct,  5) << "temp url signature mismatch: " << local_sig
                             << " != " << temp_url_sig  << dendl;
         } else {