From ef3b30ddb6cff80c1ead60cb63940e80d0fb9ec5 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 24 Aug 2016 02:08:43 +0900 Subject: Avoid unnecessary memory allocation in string2hex() Remove string2hex() and replace with newly added ossl_bin2hex(). Since the output hex string is always returned to users as a String, we can avoid the memory allocation by writing directly to the String buffer. This also reduces some lines of code. --- ext/openssl/ossl.c | 49 +++++++++++------------------------ ext/openssl/ossl.h | 10 ++++---- ext/openssl/ossl_hmac.c | 68 ++++++++++++++++++++----------------------------- ext/openssl/ossl_ocsp.c | 16 ++++++------ 4 files changed, 56 insertions(+), 87 deletions(-) (limited to 'ext/openssl') diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 5cd0fe21..a9000f25 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -11,40 +11,6 @@ #include /* for ossl_raise */ #include /* for OpenSSL < 1.1.0 locks */ -/* - * String to HEXString conversion - */ -int -string2hex(const unsigned char *buf, int buf_len, char **hexbuf, int *hexbuf_len) -{ - static const char hex[]="0123456789abcdef"; - int i, len; - - if (buf_len < 0 || buf_len > INT_MAX / 2) { /* PARANOIA? */ - return -1; - } - len = 2 * buf_len; - if (!hexbuf) { /* if no buf, return calculated len */ - if (hexbuf_len) { - *hexbuf_len = len; - } - return len; - } - if (!(*hexbuf = OPENSSL_malloc(len + 1))) { - return -1; - } - for (i = 0; i < buf_len; i++) { - (*hexbuf)[2 * i] = hex[((unsigned char)buf[i]) >> 4]; - (*hexbuf)[2 * i + 1] = hex[buf[i] & 0x0f]; - } - (*hexbuf)[2 * i] = '\0'; - - if (hexbuf_len) { - *hexbuf_len = len; - } - return len; -} - /* * Data Conversion */ @@ -145,6 +111,21 @@ ossl_buf2str(char *buf, int len) return str; } +void +ossl_bin2hex(unsigned char *in, char *out, size_t inlen) +{ + const char *hex = "0123456789abcdef"; + size_t i; + + assert(inlen <= LONG_MAX / 2); + for (i = 0; i < inlen; i++) { + unsigned char p = in[i]; + + out[i * 2 + 0] = hex[p >> 4]; + out[i * 2 + 1] = hex[p & 0x0f]; + } +} + /* * our default PEM callback */ diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 0ba64e65..864d0683 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -96,11 +96,6 @@ extern VALUE eOSSLError; }\ } while (0) -/* - * String to HEXString conversion - */ -int string2hex(const unsigned char *, int, char **, int *); - /* * Data Conversion */ @@ -118,6 +113,11 @@ do{\ assert(newlen <= len);\ rb_str_set_len((str), newlen);\ }while(0) +/* + * Convert binary string to hex string. The caller is responsible for + * ensuring out has (2 * len) bytes of capacity. + */ +void ossl_bin2hex(unsigned char *in, char *out, size_t len); /* * Our default PEM callback diff --git a/ext/openssl/ossl_hmac.c b/ext/openssl/ossl_hmac.c index ec3f9de7..270979ed 100644 --- a/ext/openssl/ossl_hmac.c +++ b/ext/openssl/ossl_hmac.c @@ -162,7 +162,7 @@ ossl_hmac_update(VALUE self, VALUE data) } static void -hmac_final(HMAC_CTX *ctx, unsigned char **buf, unsigned int *buf_len) +hmac_final(HMAC_CTX *ctx, unsigned char *buf, unsigned int *buf_len) { HMAC_CTX *final; @@ -175,12 +175,7 @@ hmac_final(HMAC_CTX *ctx, unsigned char **buf, unsigned int *buf_len) ossl_raise(eHMACError, "HMAC_CTX_copy"); } - if (!(*buf = OPENSSL_malloc(HMAC_size(final)))) { - HMAC_CTX_free(final); - OSSL_Debug("Allocating %d mem", (int)HMAC_size(final)); - ossl_raise(eHMACError, "Cannot allocate memory for hmac"); - } - HMAC_Final(final, *buf, buf_len); + HMAC_Final(final, buf, buf_len); HMAC_CTX_free(final); } @@ -191,26 +186,25 @@ hmac_final(HMAC_CTX *ctx, unsigned char **buf, unsigned int *buf_len) * Returns the authentication code an instance represents as a binary string. * * === Example - * - * instance = OpenSSL::HMAC.new('key', OpenSSL::Digest.new('sha1')) - * #=> f42bb0eeb018ebbd4597ae7213711ec60760843f - * instance.digest - * #=> "\xF4+\xB0\xEE\xB0\x18\xEB\xBDE\x97\xAEr\x13q\x1E\xC6\a`\x84?" - * + * instance = OpenSSL::HMAC.new('key', OpenSSL::Digest.new('sha1')) + * #=> f42bb0eeb018ebbd4597ae7213711ec60760843f + * instance.digest + * #=> "\xF4+\xB0\xEE\xB0\x18\xEB\xBDE\x97\xAEr\x13q\x1E\xC6\a`\x84?" */ static VALUE ossl_hmac_digest(VALUE self) { HMAC_CTX *ctx; - unsigned char *buf; unsigned int buf_len; - VALUE digest; + VALUE ret; GetHMAC(self, ctx); - hmac_final(ctx, &buf, &buf_len); - digest = ossl_buf2str((char *)buf, buf_len); + ret = rb_str_new(NULL, EVP_MAX_MD_SIZE); + hmac_final(ctx, (unsigned char *)RSTRING_PTR(ret), &buf_len); + assert(buf_len <= EVP_MAX_MD_SIZE); + rb_str_set_len(ret, buf_len); - return digest; + return ret; } /* @@ -219,27 +213,21 @@ ossl_hmac_digest(VALUE self) * * Returns the authentication code an instance represents as a hex-encoded * string. - * */ static VALUE ossl_hmac_hexdigest(VALUE self) { HMAC_CTX *ctx; - unsigned char *buf; - char *hexbuf; + unsigned char buf[EVP_MAX_MD_SIZE]; unsigned int buf_len; - VALUE hexdigest; + VALUE ret; GetHMAC(self, ctx); - hmac_final(ctx, &buf, &buf_len); - if (string2hex(buf, buf_len, &hexbuf, NULL) != 2 * (int)buf_len) { - OPENSSL_free(buf); - ossl_raise(eHMACError, "Memory alloc error"); - } - OPENSSL_free(buf); - hexdigest = ossl_buf2str(hexbuf, 2 * buf_len); + hmac_final(ctx, buf, &buf_len); + ret = rb_str_new(NULL, buf_len * 2); + ossl_bin2hex(buf, RSTRING_PTR(ret), buf_len); - return hexdigest; + return ret; } /* @@ -323,22 +311,22 @@ ossl_hmac_s_digest(VALUE klass, VALUE digest, VALUE key, VALUE data) static VALUE ossl_hmac_s_hexdigest(VALUE klass, VALUE digest, VALUE key, VALUE data) { - unsigned char *buf; - char *hexbuf; + unsigned char buf[EVP_MAX_MD_SIZE]; unsigned int buf_len; - VALUE hexdigest; + VALUE ret; StringValue(key); StringValue(data); - buf = HMAC(GetDigestPtr(digest), RSTRING_PTR(key), RSTRING_LENINT(key), - (unsigned char *)RSTRING_PTR(data), RSTRING_LEN(data), NULL, &buf_len); - if (string2hex(buf, buf_len, &hexbuf, NULL) != 2 * (int)buf_len) { - ossl_raise(eHMACError, "Cannot convert buf to hexbuf"); - } - hexdigest = ossl_buf2str(hexbuf, 2 * buf_len); + if (!HMAC(GetDigestPtr(digest), RSTRING_PTR(key), RSTRING_LENINT(key), + (unsigned char *)RSTRING_PTR(data), RSTRING_LEN(data), + buf, &buf_len)) + ossl_raise(eHMACError, "HMAC"); + + ret = rb_str_new(NULL, buf_len * 2); + ossl_bin2hex(buf, RSTRING_PTR(ret), buf_len); - return hexdigest; + return ret; } /* diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c index d9ee51cd..a8b3503d 100644 --- a/ext/openssl/ossl_ocsp.c +++ b/ext/openssl/ossl_ocsp.c @@ -1602,15 +1602,15 @@ ossl_ocspcid_get_issuer_name_hash(VALUE self) { OCSP_CERTID *id; ASN1_OCTET_STRING *name_hash; - char *hexbuf; + VALUE ret; GetOCSPCertId(self, id); OCSP_id_get0_info(&name_hash, NULL, NULL, NULL, id); - if (string2hex(name_hash->data, name_hash->length, &hexbuf, NULL) < 0) - ossl_raise(eOCSPError, "string2hex"); + ret = rb_str_new(NULL, name_hash->length * 2); + ossl_bin2hex(name_hash->data, RSTRING_PTR(ret), name_hash->length); - return ossl_buf2str(hexbuf, name_hash->length * 2); + return ret; } /* @@ -1625,15 +1625,15 @@ ossl_ocspcid_get_issuer_key_hash(VALUE self) { OCSP_CERTID *id; ASN1_OCTET_STRING *key_hash; - char *hexbuf; + VALUE ret; GetOCSPCertId(self, id); OCSP_id_get0_info(NULL, NULL, &key_hash, NULL, id); - if (string2hex(key_hash->data, key_hash->length, &hexbuf, NULL) < 0) - ossl_raise(eOCSPError, "string2hex"); + ret = rb_str_new(NULL, key_hash->length * 2); + ossl_bin2hex(key_hash->data, RSTRING_PTR(ret), key_hash->length); - return ossl_buf2str(hexbuf, key_hash->length * 2); + return ret; } /* -- cgit v1.2.3