Hardened OpenSSL digest/HMAC calls [RT #37944]

CHANGES

@@ -1,3 +1,8 @@
+4012.	[bug]		Check returned status of OpenSSL digest and HMAC
+			functions when they return one. Note this applies
+			only to FIPS capable OpenSSL libraries put in
+			FIPS mode and MD5. [RT #37944]
+
 4011.	[bug]		master's list port and dscp inheritance was not
 			properly implemented. [RT #37792]
 

config.h.in

@@ -446,6 +446,9 @@ int sigwait(const unsigned int *set, int *sig);
 /* Define to 1 if you have the `usleep' function. */
 #undef HAVE_USLEEP
 
+/* HMAC_*() return ints */
+#undef HMAC_RETURN_INT
+
 /* Use HMAC-SHA1 for Source Identity Token generation */
 #undef HMAC_SHA1_SIT
 

config.h.win32

@@ -354,6 +354,9 @@ typedef __int64 off_t;
 /* Define if your OpenSSL version supports AES */
 @HAVE_OPENSSL_AES@
 
+/* HMAC_*() return ints */
+@HMAC_RETURN_INT@
+
 /* Use AES for Source Identity Token generation */
 @AES_SIT@
 

configure

@@ -16167,6 +16167,43 @@ $as_echo "yes" >&6; }
 		ISC_PLATFORM_OPENSSLHASH="#define ISC_PLATFORM_OPENSSLHASH 1"
 		ISC_OPENSSL_INC="$DST_OPENSSL_INC"
 		ISC_OPENSSL_LIBS="$DST_OPENSSL_LIBS"
+		saved_cflags="$CFLAGS"
+		save_libs="$LIBS"
+		CFLAGS="$CFLAGS $ISC_OPENSSL_INC"
+		LIBS="$LIBS $ISC_OPENSSL_LIBS"
+		{ $as_echo "$as_me:${as_lineno-$LINENO}: checking HMAC_Init() return type" >&5
+$as_echo_n "checking HMAC_Init() return type... " >&6; }
+		cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+		#include <openssl/hmac.h>
+int
+main ()
+{
+
+		HMAC_CTX ctx;
+		int n = HMAC_Init(&ctx, NULL, 0, NULL);
+		n += HMAC_Update(&ctx, NULL, 0);
+		n += HMAC_Final(&ctx, NULL, NULL);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+		{ $as_echo "$as_me:${as_lineno-$LINENO}: result: int" >&5
+$as_echo "int" >&6; }
+
+$as_echo "#define HMAC_RETURN_INT 1" >>confdefs.h
+
+else
+
+		{ $as_echo "$as_me:${as_lineno-$LINENO}: result: void" >&5
+$as_echo "void" >&6; }
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+		CFLAGS="$saved_cflags"
+		LIBS="$save_libs"
 		;;
 	no)
 		{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5

configure.in

@@ -1876,6 +1876,22 @@ case $want_openssl_hash in
 		ISC_PLATFORM_OPENSSLHASH="#define ISC_PLATFORM_OPENSSLHASH 1"
 		ISC_OPENSSL_INC="$DST_OPENSSL_INC"
 		ISC_OPENSSL_LIBS="$DST_OPENSSL_LIBS"
+		saved_cflags="$CFLAGS"
+		save_libs="$LIBS"
+		CFLAGS="$CFLAGS $ISC_OPENSSL_INC"
+		LIBS="$LIBS $ISC_OPENSSL_LIBS"
+		AC_MSG_CHECKING([HMAC_Init() return type])
+		AC_TRY_COMPILE([
+		#include <openssl/hmac.h>],[
+		HMAC_CTX ctx;
+		int n = HMAC_Init(&ctx, NULL, 0, NULL);
+		n += HMAC_Update(&ctx, NULL, 0);
+		n += HMAC_Final(&ctx, NULL, NULL);],[
+		AC_MSG_RESULT(int)
+		AC_DEFINE(HMAC_RETURN_INT, 1, [HMAC_*() return ints])],[
+		AC_MSG_RESULT(void)])
+		CFLAGS="$saved_cflags"
+		LIBS="$save_libs"
 		;;
 	no)
 		AC_MSG_RESULT(no)

lib/isc/hmacmd5.c

@@ -44,7 +44,12 @@ void
 isc_hmacmd5_init(isc_hmacmd5_t *ctx, const unsigned char *key,
 		 unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Init(ctx, (const void *) key,
+				(int) len, EVP_md5()) == 1);
+#else
 	HMAC_Init(ctx, (const void *) key, (int) len, EVP_md5());
+#endif
 }
 
 void
@@ -56,12 +61,20 @@ void
 isc_hmacmd5_update(isc_hmacmd5_t *ctx, const unsigned char *buf,
 		   unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Update(ctx, buf, (int) len) == 1);
+#else
 	HMAC_Update(ctx, buf, (int) len);
+#endif
 }
 
 void
 isc_hmacmd5_sign(isc_hmacmd5_t *ctx, unsigned char *digest) {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Final(ctx, digest, NULL) == 1);
+#else
 	HMAC_Final(ctx, digest, NULL);
+#endif
 	HMAC_CTX_cleanup(ctx);
 }
 

lib/isc/hmacsha.c

@@ -44,7 +44,12 @@ void
 isc_hmacsha1_init(isc_hmacsha1_t *ctx, const unsigned char *key,
 		  unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Init(ctx, (const void *) key,
+				(int) len, EVP_sha1()) == 1);
+#else
 	HMAC_Init(ctx, (const void *) key, (int) len, EVP_sha1());
+#endif
 }
 
 void
@@ -56,7 +61,11 @@ void
 isc_hmacsha1_update(isc_hmacsha1_t *ctx, const unsigned char *buf,
 		   unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Update(ctx, buf, (int) len) == 1);
+#else
 	HMAC_Update(ctx, buf, (int) len);
+#endif
 }
 
 void
@@ -65,7 +74,11 @@ isc_hmacsha1_sign(isc_hmacsha1_t *ctx, unsigned char *digest, size_t len) {
 
 	REQUIRE(len <= ISC_SHA1_DIGESTLENGTH);
 
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Final(ctx, newdigest, NULL) == 1);
+#else
 	HMAC_Final(ctx, newdigest, NULL);
+#endif
 	HMAC_CTX_cleanup(ctx);
 	memmove(digest, newdigest, len);
 	memset(newdigest, 0, sizeof(newdigest));
@@ -75,7 +88,12 @@ void
 isc_hmacsha224_init(isc_hmacsha224_t *ctx, const unsigned char *key,
 		    unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Init(ctx, (const void *) key,
+				(int) len, EVP_sha224()) == 1);
+#else
 	HMAC_Init(ctx, (const void *) key, (int) len, EVP_sha224());
+#endif
 }
 
 void
@@ -87,7 +105,11 @@ void
 isc_hmacsha224_update(isc_hmacsha224_t *ctx, const unsigned char *buf,
 		   unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Update(ctx, buf, (int) len) == 1);
+#else
 	HMAC_Update(ctx, buf, (int) len);
+#endif
 }
 
 void
@@ -96,7 +118,11 @@ isc_hmacsha224_sign(isc_hmacsha224_t *ctx, unsigned char *digest, size_t len) {
 
 	REQUIRE(len <= ISC_SHA224_DIGESTLENGTH);
 
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Final(ctx, newdigest, NULL) == 1);
+#else
 	HMAC_Final(ctx, newdigest, NULL);
+#endif
 	HMAC_CTX_cleanup(ctx);
 	memmove(digest, newdigest, len);
 	memset(newdigest, 0, sizeof(newdigest));
@@ -106,7 +132,12 @@ void
 isc_hmacsha256_init(isc_hmacsha256_t *ctx, const unsigned char *key,
 		    unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Init(ctx, (const void *) key,
+				(int) len, EVP_sha256()) == 1);
+#else
 	HMAC_Init(ctx, (const void *) key, (int) len, EVP_sha256());
+#endif
 }
 
 void
@@ -118,7 +149,11 @@ void
 isc_hmacsha256_update(isc_hmacsha256_t *ctx, const unsigned char *buf,
 		   unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Update(ctx, buf, (int) len) == 1);
+#else
 	HMAC_Update(ctx, buf, (int) len);
+#endif
 }
 
 void
@@ -127,7 +162,11 @@ isc_hmacsha256_sign(isc_hmacsha256_t *ctx, unsigned char *digest, size_t len) {
 
 	REQUIRE(len <= ISC_SHA256_DIGESTLENGTH);
 
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Final(ctx, newdigest, NULL) == 1);
+#else
 	HMAC_Final(ctx, newdigest, NULL);
+#endif
 	HMAC_CTX_cleanup(ctx);
 	memmove(digest, newdigest, len);
 	memset(newdigest, 0, sizeof(newdigest));
@@ -137,7 +176,12 @@ void
 isc_hmacsha384_init(isc_hmacsha384_t *ctx, const unsigned char *key,
 		    unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Init(ctx, (const void *) key,
+				(int) len, EVP_sha384()) == 1);
+#else
 	HMAC_Init(ctx, (const void *) key, (int) len, EVP_sha384());
+#endif
 }
 
 void
@@ -149,7 +193,11 @@ void
 isc_hmacsha384_update(isc_hmacsha384_t *ctx, const unsigned char *buf,
 		   unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Update(ctx, buf, (int) len) == 1);
+#else
 	HMAC_Update(ctx, buf, (int) len);
+#endif
 }
 
 void
@@ -158,7 +206,11 @@ isc_hmacsha384_sign(isc_hmacsha384_t *ctx, unsigned char *digest, size_t len) {
 
 	REQUIRE(len <= ISC_SHA384_DIGESTLENGTH);
 
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Final(ctx, newdigest, NULL) == 1);
+#else
 	HMAC_Final(ctx, newdigest, NULL);
+#endif
 	HMAC_CTX_cleanup(ctx);
 	memmove(digest, newdigest, len);
 	memset(newdigest, 0, sizeof(newdigest));
@@ -168,7 +220,12 @@ void
 isc_hmacsha512_init(isc_hmacsha512_t *ctx, const unsigned char *key,
 		    unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Init(ctx, (const void *) key,
+				(int) len, EVP_sha512()) == 1);
+#else
 	HMAC_Init(ctx, (const void *) key, (int) len, EVP_sha512());
+#endif
 }
 
 void
@@ -180,7 +237,11 @@ void
 isc_hmacsha512_update(isc_hmacsha512_t *ctx, const unsigned char *buf,
 		   unsigned int len)
 {
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Update(ctx, buf, (int) len) == 1);
+#else
 	HMAC_Update(ctx, buf, (int) len);
+#endif
 }
 
 void
@@ -189,7 +250,11 @@ isc_hmacsha512_sign(isc_hmacsha512_t *ctx, unsigned char *digest, size_t len) {
 
 	REQUIRE(len <= ISC_SHA512_DIGESTLENGTH);
 
+#ifdef HMAC_RETURN_INT
+	RUNTIME_CHECK(HMAC_Final(ctx, newdigest, NULL) == 1);
+#else
 	HMAC_Final(ctx, newdigest, NULL);
+#endif
 	HMAC_CTX_cleanup(ctx);
 	memmove(digest, newdigest, len);
 	memset(newdigest, 0, sizeof(newdigest));

lib/isc/md5.c

@@ -52,7 +52,7 @@
 #ifdef ISC_PLATFORM_OPENSSLHASH
 void
 isc_md5_init(isc_md5_t *ctx) {
-	EVP_DigestInit(ctx, EVP_md5());
+	RUNTIME_CHECK(EVP_DigestInit(ctx, EVP_md5()) == 1);
 }
 
 void
@@ -62,12 +62,14 @@ isc_md5_invalidate(isc_md5_t *ctx) {
 
 void
 isc_md5_update(isc_md5_t *ctx, const unsigned char *buf, unsigned int len) {
-	EVP_DigestUpdate(ctx, (const void *) buf, (size_t) len);
+	RUNTIME_CHECK(EVP_DigestUpdate(ctx,
+				       (const void *) buf,
+				       (size_t) len) == 1);
 }
 
 void
 isc_md5_final(isc_md5_t *ctx, unsigned char *digest) {
-	EVP_DigestFinal(ctx, digest, NULL);
+	RUNTIME_CHECK(EVP_DigestFinal(ctx, digest, NULL) == 1);
 }
 
 #elif PKCS11CRYPTO

lib/isc/sha1.c

@@ -55,7 +55,7 @@ isc_sha1_init(isc_sha1_t *context)
 {
 	INSIST(context != NULL);
 
-	EVP_DigestInit(context, EVP_sha1());
+	RUNTIME_CHECK(EVP_DigestInit(context, EVP_sha1()) == 1);
 }
 
 void
@@ -70,7 +70,9 @@ isc_sha1_update(isc_sha1_t *context, const unsigned char *data,
 	INSIST(context != 0);
 	INSIST(data != 0);
 
-	EVP_DigestUpdate(context, (const void *) data, (size_t) len);
+	RUNTIME_CHECK(EVP_DigestUpdate(context,
+				       (const void *) data,
+				       (size_t) len) == 1);
 }
 
 void
@@ -78,7 +80,7 @@ isc_sha1_final(isc_sha1_t *context, unsigned char *digest) {
 	INSIST(digest != 0);
 	INSIST(context != 0);
 
-	EVP_DigestFinal(context, digest, NULL);
+	RUNTIME_CHECK(EVP_DigestFinal(context, digest, NULL) == 1);
 }
 
 #elif PKCS11CRYPTO

lib/isc/sha2.c

@@ -75,7 +75,7 @@ isc_sha224_init(isc_sha224_t *context) {
 	if (context == (isc_sha224_t *)0) {
 		return;
 	}
-	EVP_DigestInit(context, EVP_sha224());
+	RUNTIME_CHECK(EVP_DigestInit(context, EVP_sha224()) == 1);
 }
 
 void
@@ -93,7 +93,8 @@ isc_sha224_update(isc_sha224_t *context, const isc_uint8_t* data, size_t len) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha224_t *)0 && data != (isc_uint8_t*)0);
 
-	EVP_DigestUpdate(context, (const void *) data, len);
+	RUNTIME_CHECK(EVP_DigestUpdate(context,
+				       (const void *) data, len) == 1);
 }
 
 void
@@ -103,7 +104,7 @@ isc_sha224_final(isc_uint8_t digest[], isc_sha224_t *context) {
 
 	/* If no digest buffer is passed, we don't bother doing this: */
 	if (digest != (isc_uint8_t*)0) {
-		EVP_DigestFinal(context, digest, NULL);
+		RUNTIME_CHECK(EVP_DigestFinal(context, digest, NULL) == 1);
 	} else {
 		EVP_MD_CTX_cleanup(context);
 	}
@@ -114,7 +115,7 @@ isc_sha256_init(isc_sha256_t *context) {
 	if (context == (isc_sha256_t *)0) {
 		return;
 	}
-	EVP_DigestInit(context, EVP_sha256());
+	RUNTIME_CHECK(EVP_DigestInit(context, EVP_sha256()) == 1);
 }
 
 void
@@ -132,7 +133,8 @@ isc_sha256_update(isc_sha256_t *context, const isc_uint8_t *data, size_t len) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha256_t *)0 && data != (isc_uint8_t*)0);
 
-	EVP_DigestUpdate(context, (const void *) data, len);
+	RUNTIME_CHECK(EVP_DigestUpdate(context,
+				       (const void *) data, len) == 1);
 }
 
 void
@@ -142,7 +144,7 @@ isc_sha256_final(isc_uint8_t digest[], isc_sha256_t *context) {
 
 	/* If no digest buffer is passed, we don't bother doing this: */
 	if (digest != (isc_uint8_t*)0) {
-		EVP_DigestFinal(context, digest, NULL);
+		RUNTIME_CHECK(EVP_DigestFinal(context, digest, NULL) == 1);
 	} else {
 		EVP_MD_CTX_cleanup(context);
 	}
@@ -153,7 +155,7 @@ isc_sha512_init(isc_sha512_t *context) {
 	if (context == (isc_sha512_t *)0) {
 		return;
 	}
-	EVP_DigestInit(context, EVP_sha512());
+	RUNTIME_CHECK(EVP_DigestInit(context, EVP_sha512()) == 1);
 }
 
 void
@@ -170,7 +172,8 @@ void isc_sha512_update(isc_sha512_t *context, const isc_uint8_t *data, size_t le
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha512_t *)0 && data != (isc_uint8_t*)0);
 
-	EVP_DigestUpdate(context, (const void *) data, len);
+	RUNTIME_CHECK(EVP_DigestUpdate(context,
+				       (const void *) data, len) == 1);
 }
 
 void isc_sha512_final(isc_uint8_t digest[], isc_sha512_t *context) {
@@ -179,7 +182,7 @@ void isc_sha512_final(isc_uint8_t digest[], isc_sha512_t *context) {
 
 	/* If no digest buffer is passed, we don't bother doing this: */
 	if (digest != (isc_uint8_t*)0) {
-		EVP_DigestFinal(context, digest, NULL);
+		RUNTIME_CHECK(EVP_DigestFinal(context, digest, NULL) == 1);
 	} else {
 		EVP_MD_CTX_cleanup(context);
 	}
@@ -190,7 +193,7 @@ isc_sha384_init(isc_sha384_t *context) {
 	if (context == (isc_sha384_t *)0) {
 		return;
 	}
-	EVP_DigestInit(context, EVP_sha384());
+	RUNTIME_CHECK(EVP_DigestInit(context, EVP_sha384()) == 1);
 }
 
 void
@@ -208,7 +211,8 @@ isc_sha384_update(isc_sha384_t *context, const isc_uint8_t* data, size_t len) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha512_t *)0 && data != (isc_uint8_t*)0);
 
-	EVP_DigestUpdate(context, (const void *) data, len);
+	RUNTIME_CHECK(EVP_DigestUpdate(context,
+				       (const void *) data, len) == 1);
 }
 
 void
@@ -218,7 +222,7 @@ isc_sha384_final(isc_uint8_t digest[], isc_sha384_t *context) {
 
 	/* If no digest buffer is passed, we don't bother doing this: */
 	if (digest != (isc_uint8_t*)0) {
-		EVP_DigestFinal(context, digest, NULL);
+		RUNTIME_CHECK(EVP_DigestFinal(context, digest, NULL) == 1);
 	} else {
 		EVP_MD_CTX_cleanup(context);
 	}

win32utils/Configure

@@ -365,6 +365,7 @@ my @substdefh = ("AES_SIT",
                  "HAVE_PKCS11_ECDSA",
                  "HAVE_PKCS11_GOST",
                  "HAVE_READLINE",
+                 "HMAC_RETURN_INT",
                  "HMAC_SHA1_SIT",
                  "HMAC_SHA256_SIT",
                  "ISC_LIST_CHECKINIT",
@@ -1876,6 +1877,30 @@ if ($enable_openssl_hash eq "yes") {
         die "No OpenSSL for hash functions\n";
     }
     $configdefp{"ISC_PLATFORM_OPENSSLHASH"} = 1;
+    if ($verbose) {
+        print "checking HMAC_Init() return type\n";
+    }
+    open F, ">testhmac.c" || die $!;
+    print F << 'EOF';
+#include <openssl/hmac.h>
+
+int
+main(void)
+{
+        HMAC_CTX ctx;
+        int n = HMAC_Init(&ctx, NULL, 0, NULL);
+        n += HMAC_Update(&ctx, NULL, 0);
+        n += HMAC_Final(&ctx, NULL, NULL);
+        return(n);
+}
+EOF
+    close F;
+    my $include = $configinc{"OPENSSL_INC"};
+    my $library = $configlib{"OPENSSL_LIB"};
+    $compret = `cl /nologo /MD /I "$include" testhmac.c "$library"`;
+    if (grep { -f and -x } ".\\testhmac.exe") {
+        $configdefh{"HMAC_RETURN_INT"} = 1;
+    }
 }
 
 # with-pkcs11