calc_hmac_sha: fix access-past-end-of-buffer

Fix a place where we access a buffer past the end of its length. Clean
up the function a bit.

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>

diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index 60bf7ac58ecb604c3ebeab7884c584f2eadd7581..d7dacc22c98bcf7783398a8aa423f16f57da998d 100644 (file)
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -25,29 +25,22 @@ int parse_time(const char *time_str, time_t *time)
 /*
  * calculate the sha1 value of a given msg and key
  */
-int calc_hmac_sha1(const char *key, int key_len,
-                   const char *msg, int msg_len,
-                   char *dest, int *len) /* dest should be large enough to hold result */
+void calc_hmac_sha1(const char *key, int key_len,
+                    const char *msg, int msg_len, char *dest)
+/* destination should be CEPH_CRYPTO_HMACSHA1_DIGESTSIZE bytes long */
 {
-  if (*len < CEPH_CRYPTO_HMACSHA1_DIGESTSIZE)
-    return -EINVAL;
-
-  char hex_str[CEPH_CRYPTO_HMACSHA1_DIGESTSIZE * 2 + 1];
   char key_buf[CEPH_CRYPTO_HMACSHA1_DIGESTSIZE];
-  key_len = max(key_len, CEPH_CRYPTO_HMACSHA1_DIGESTSIZE);
+  memset(key_buf, 0, CEPH_CRYPTO_HMACSHA1_DIGESTSIZE);
   memcpy(key_buf, key, key_len);
-  memset(key_buf + key_len, 0, CEPH_CRYPTO_HMACSHA1_DIGESTSIZE - key_len);
 
   HMACSHA1 hmac((const unsigned char *)key, key_len);
   hmac.Update((const unsigned char *)msg, msg_len);
   hmac.Final((unsigned char *)dest);
-  *len = CEPH_CRYPTO_HMACSHA1_DIGESTSIZE;
   
-  buf_to_hex((unsigned char *)dest, *len, hex_str);
+  char hex_str[(CEPH_CRYPTO_HMACSHA1_DIGESTSIZE * 2) + 1];
+  buf_to_hex((unsigned char *)dest, CEPH_CRYPTO_HMACSHA1_DIGESTSIZE, hex_str);
 
   RGW_LOG(15) << "hmac=" << hex_str << endl;
-
-  return 0;
 }
 
 int NameVal::parse()

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 4ff8637e03e59315180145fddbb2b6774413d175..1c26bddc4cc680f65484ed2dee2207b70fba27f5 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -358,9 +358,10 @@ extern bool verify_permission(struct req_state *s, int perm);
  * by converting %-escaped strings into characters, etc*/
 extern bool url_decode(string& src_str, string& dest_str);
 
-extern int calc_hmac_sha1(const char *key, int key_len,
-                          const char *msg, int msg_len,
-                          char *dest, int *len); /* dest should be large enough to hold result */
+extern void calc_hmac_sha1(const char *key, int key_len,
+                          const char *msg, int msg_len, char *dest);
+/* destination should be CEPH_CRYPTO_HMACSHA1_DIGESTSIZE bytes long */
+
 /* loglevel of the gateway */
 extern int rgw_log_level;
 

diff --git a/src/rgw/rgw_os_auth.cc b/src/rgw/rgw_os_auth.cc
index fe2e4d94ec5ec1a45bbd84b7cef56c5e0603f6ec..a9fb6114dc1b57183987c2adb1ae3132608b5b8e 100644 (file)
--- a/src/rgw/rgw_os_auth.cc
+++ b/src/rgw/rgw_os_auth.cc
@@ -17,7 +17,6 @@ static int build_token(string& os_user, string& key, uint64_t nonce, utime_t& ex
   ::encode(expiration, bl);
 
   bufferptr p(CEPH_CRYPTO_HMACSHA1_DIGESTSIZE);
-  int len = p.length();
 
   char buf[bl.length() * 2 + 1];
   buf_to_hex((const unsigned char *)bl.c_str(), bl.length(), buf);
@@ -29,13 +28,7 @@ static int build_token(string& os_user, string& key, uint64_t nonce, utime_t& ex
   for (int i = 0; i < (int)key.length(); i++, s++) {
     k[i % CEPH_CRYPTO_HMACSHA1_DIGESTSIZE] |= *s;
   }
-  int ret = calc_hmac_sha1(k, sizeof(k), bl.c_str(), bl.length(),
-                            p.c_str(), &len);
-  if (ret < 0)
-     return ret;
-
-  if (len != CEPH_CRYPTO_HMACSHA1_DIGESTSIZE)
-    return -EINVAL;
+  calc_hmac_sha1(k, sizeof(k), bl.c_str(), bl.length(), p.c_str());
 
   bl.append(p);
 

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index e9c2a07559aaa426f7b59f767afcaa96145790cc..a6d5fab454a8519274e1adf3dc133d4a20553708 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -415,12 +415,11 @@ bool RGWHandler_REST_S3::authorize(struct req_state *s)
   int key_len = strlen(key);
 
   char hmac_sha1[CEPH_CRYPTO_HMACSHA1_DIGESTSIZE];
-  int len = sizeof(hmac_sha1);
-  if (calc_hmac_sha1(key, key_len, auth_hdr.c_str(), auth_hdr.size(), hmac_sha1, &len) < 0)
-    return false;
+  calc_hmac_sha1(key, key_len, auth_hdr.c_str(), auth_hdr.size(), hmac_sha1);
 
   char b64[64]; /* 64 is really enough */
-  int ret = ceph_armor(b64, &b64[sizeof(b64)], hmac_sha1, &hmac_sha1[len]);
+  int ret = ceph_armor(b64, b64 + 64, hmac_sha1,
+                      hmac_sha1 + CEPH_CRYPTO_HMACSHA1_DIGESTSIZE);
   if (ret < 0) {
     RGW_LOG(10) << "ceph_armor failed" << endl;
     return false;