From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 4 Oct 2018 02:44:21 +0000 (+0700)
Subject: Don't leak HMAC_CTXs
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dbdc0a06aac9bbb1db04404afca5941467aef993;p=thirdparty%2Ffreeradius-server.git

Don't leak HMAC_CTXs
---

diff --git a/src/lib/tls/base-h b/src/lib/tls/base-h
index 984fa93baa6..1f578a20f81 100644
--- a/src/lib/tls/base-h
+++ b/src/lib/tls/base-h
@@ -89,7 +89,7 @@ static inline HMAC_CTX *HMAC_CTX_new(void)
 #  define HMAC_CTX_free(_ctx) \
 do {\
 	if (_ctx) {\
-		memset(_ctx, 0, sizeof(*_ctx));\
+		memset(_ctx, 0, sizeof(*((HMAC_CTX *)(_ctx))));\
 		talloc_free(_ctx);\
 	}\
 } while (0)
diff --git a/src/lib/util/all.mk b/src/lib/util/all.mk
index 45433562bc3..2cddc9b1b95 100644
--- a/src/lib/util/all.mk
+++ b/src/lib/util/all.mk
@@ -17,8 +17,8 @@ SOURCES		:= \
 		   getaddrinfo.c \
 		   hash.c \
 		   heap.c \
-		   hmacmd5.c \
-		   hmacsha1.c \
+		   hmac_md5.c \
+		   hmac_sha1.c \
 		   inet.c \
 		   isaac.c \
 		   log.c \
diff --git a/src/lib/util/hmacmd5.c b/src/lib/util/hmac_md5.c
similarity index 86%
rename from src/lib/util/hmacmd5.c
rename to src/lib/util/hmac_md5.c
index 383fdd37bd1..953a8a79774 100644
--- a/src/lib/util/hmacmd5.c
+++ b/src/lib/util/hmac_md5.c
@@ -16,7 +16,7 @@
 
 /** MD5 HMAC not dependent on OpenSSL
  *
- * @file src/lib/util/hmacmd5.c
+ * @file src/lib/util/hmac_md5.c
  *
  * @note New code that needs fast or incremental HMACs should use the OpenSSL EVP_* HMAC
  *	interface instead, as that can take advantage of acceleration instructions provided
@@ -30,13 +30,19 @@
  */
 RCSID("$Id$")
 
-#ifdef HAVE_OPENSSL_EVP_H
-#include <freeradius-devel/tls/base.h>
-#endif
-
 #include <freeradius-devel/util/md5.h>
 
 #ifdef HAVE_OPENSSL_EVP_H
+#  include <freeradius-devel/tls/base.h>
+#  include <openssl/hmac.h>
+
+fr_thread_local_setup(HMAC_CTX *, md5_hmac_ctx)
+
+static void _hmac_md5_ctx_free_on_exit(void *arg)
+{
+	HMAC_CTX_free(arg);
+}
+
 /** Calculate HMAC using OpenSSL's MD5 implementation
  *
  * @param digest Caller digest to be filled in.
@@ -49,7 +55,15 @@ RCSID("$Id$")
 void fr_hmac_md5(uint8_t digest[MD5_DIGEST_LENGTH], uint8_t const *text, size_t text_len,
 		 uint8_t const *key, size_t key_len)
 {
-	HMAC_CTX *ctx  = HMAC_CTX_new();
+	HMAC_CTX *ctx;
+
+	if (unlikely(!md5_hmac_ctx)) {
+		ctx = HMAC_CTX_new();
+		if (unlikely(!ctx)) return;
+		fr_thread_local_set_destructor(md5_hmac_ctx, _hmac_md5_ctx_free_on_exit, ctx);
+	} else {
+		ctx = md5_hmac_ctx;
+	}
 
 #ifdef EVP_MD_CTX_FLAG_NON_FIPS_ALLOW
 	/* Since MD5 is not allowed by FIPS, explicitly allow it. */
@@ -59,10 +73,9 @@ void fr_hmac_md5(uint8_t digest[MD5_DIGEST_LENGTH], uint8_t const *text, size_t
 	HMAC_Init_ex(ctx, key, key_len, EVP_md5(), NULL);
 	HMAC_Update(ctx, text, text_len);
 	HMAC_Final(ctx, digest, NULL);
+	HMAC_CTX_cleanup(ctx);
 }
-
 #else
-
 /** Calculate HMAC using internal MD5 implementation
  *
  * @param digest Caller digest to be filled in.
@@ -170,28 +183,28 @@ Test Vectors (Trailing '\0' of a character string not included in test):
  */
 int main(int argc, char **argv)
 {
-  uint8_t digest[16];
-  char *key;
-  int key_len;
-  char *text;
-  int text_len;
-  int i;
+	uint8_t digest[16];
+	char *key;
+	int key_len;
+	char *text;
+	int text_len;
+	int i;
 
-  key = argv[1];
-  key_len = strlen(key);
+	key = argv[1];
+	key_len = strlen(key);
 
-  text = argv[2];
-  text_len = strlen(text);
+	text = argv[2];
+	text_len = strlen(text);
 
-  fr_hmac_md5(digest, text, text_len, key, key_len);
+	fr_hmac_md5(digest, text, text_len, key, key_len);
 
-  for (i = 0; i < 16; i++) {
-    printf("%02x", digest[i]);
-  }
-  printf("\n");
+	for (i = 0; i < 16; i++) {
+	printf("%02x", digest[i]);
+	}
+	printf("\n");
 
-  exit(0);
-  return 0;
+	exit(0);
+	return 0;
 }
 
 #endif
diff --git a/src/lib/util/hmacsha1.c b/src/lib/util/hmac_sha1.c
similarity index 92%
rename from src/lib/util/hmacsha1.c
rename to src/lib/util/hmac_sha1.c
index 51a72954715..51d2eba1931 100644
--- a/src/lib/util/hmacsha1.c
+++ b/src/lib/util/hmac_sha1.c
@@ -22,7 +22,7 @@
  *
  * Adapted from hmacmd5.c (HMAC-MD5).  Test cases from RFC2202.
  *
- * @file src/lib/util/hmacsha1.c
+ * @file src/lib/util/hmac_sha1.c
  *
  * @author Michael Richardson <mcr@sandelman.ottawa.on.ca>
  *
@@ -31,10 +31,6 @@
  */
 RCSID("$Id$")
 
-#ifdef HAVE_OPENSSL_EVP_H
-#include <freeradius-devel/tls/base.h>
-#endif
-
 #include <freeradius-devel/util/sha1.h>
 
 #ifdef HMAC_SHA1_DATA_PROBLEMS
@@ -42,6 +38,16 @@ unsigned int sha1_data_problems = 0;
 #endif
 
 #ifdef HAVE_OPENSSL_EVP_H
+#  include <freeradius-devel/tls/base.h>
+#  include <openssl/hmac.h>
+
+fr_thread_local_setup(HMAC_CTX *, sha1_hmac_ctx)
+
+static void _hmac_sha1_ctx_free_on_exit(void *arg)
+{
+	HMAC_CTX_free(arg);
+}
+
 /** Calculate HMAC using OpenSSL's SHA1 implementation
  *
  * @param digest Caller digest to be filled in.
@@ -54,10 +60,20 @@ unsigned int sha1_data_problems = 0;
 void fr_hmac_sha1(uint8_t digest[SHA1_DIGEST_LENGTH], uint8_t const *text, size_t text_len,
 		  uint8_t const *key, size_t key_len)
 {
-	HMAC_CTX *ctx  = HMAC_CTX_new();
+	HMAC_CTX *ctx;
+
+	if (unlikely(!sha1_hmac_ctx)) {
+		ctx = HMAC_CTX_new();
+		if (unlikely(!ctx)) return;
+		fr_thread_local_set_destructor(sha1_hmac_ctx, _hmac_sha1_ctx_free_on_exit, ctx);
+	} else {
+		ctx = sha1_hmac_ctx;
+	}
+
 	HMAC_Init_ex(ctx, key, key_len, EVP_sha1(), NULL);
 	HMAC_Update(ctx, text, text_len);
 	HMAC_Final(ctx, digest, NULL);
+	HMAC_CTX_cleanup(ctx);
 }
 
 #else