From 70b021d4f926d20f7478851cb44ff9609420ad4e Mon Sep 17 00:00:00 2001 From: Colin Patrick McCabe Date: Wed, 23 Mar 2011 22:42:02 -0700 Subject: [PATCH] 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 --- src/rgw/rgw_common.cc | 19 ++++++------------- src/rgw/rgw_common.h | 7 ++++--- src/rgw/rgw_os_auth.cc | 9 +-------- src/rgw/rgw_rest_s3.cc | 7 +++---- 4 files changed, 14 insertions(+), 28 deletions(-) diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 60bf7ac58ecb6..d7dacc22c98bc 100644 --- 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 4ff8637e03e59..1c26bddc4cc68 100644 --- 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 fe2e4d94ec5ec..a9fb6114dc1b5 100644 --- 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 e9c2a07559aaa..a6d5fab454a85 100644 --- 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; -- 2.39.5