843e5f
From 76594cba9a1e910bb36160d96fc3872349341799 Mon Sep 17 00:00:00 2001
aeea22
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@sury.org>
aeea22
Date: Wed, 25 Apr 2018 14:04:31 +0200
aeea22
Subject: [PATCH] Replace isc_safe routines with their OpenSSL counter parts
aeea22
aeea22
(cherry picked from commit 66ba2fdad583d962a1f4971c85d58381f0849e4d)
aeea22
aeea22
Remove isc_safe_memcompare, it's not needed anywhere and can't be replaced with CRYPTO_memcmp()
aeea22
aeea22
(cherry picked from commit b105ccee68ccc3c18e6ea530063b3c8e5a42571c)
aeea22
aeea22
Fix the isc_safe_memwipe() usage with (NULL, >0)
aeea22
aeea22
(cherry picked from commit 083461d3329ff6f2410745848a926090586a9846)
aeea22
---
aeea22
 bin/dnssec/dnssec-signzone.c |  2 +-
1e4169
 lib/dns/nsec3.c              |  4 +-
1e4169
 lib/dns/spnego.c             |  4 +-
1e4169
 lib/isc/Makefile.in          |  8 +---
1e4169
 lib/isc/include/isc/safe.h   | 18 ++------
1e4169
 lib/isc/safe.c               | 83 ------------------------------------
1e4169
 lib/isc/tests/safe_test.c    | 18 --------
1e4169
 7 files changed, 11 insertions(+), 126 deletions(-)
aeea22
 delete mode 100644 lib/isc/safe.c
aeea22
aeea22
diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
ad7b3b
index 6ddaebe..d921870 100644
aeea22
--- a/bin/dnssec/dnssec-signzone.c
aeea22
+++ b/bin/dnssec/dnssec-signzone.c
ad7b3b
@@ -787,7 +787,7 @@ hashlist_add_dns_name(hashlist_t *l, /*const*/ dns_name_t *name,
aeea22
 
aeea22
 static int
aeea22
 hashlist_comp(const void *a, const void *b) {
aeea22
-	return (isc_safe_memcompare(a, b, hash_length + 1));
aeea22
+	return (memcmp(a, b, hash_length + 1));
aeea22
 }
aeea22
 
aeea22
 static void
aeea22
diff --git a/lib/dns/nsec3.c b/lib/dns/nsec3.c
1e4169
index 6ae7ca8..01426d6 100644
aeea22
--- a/lib/dns/nsec3.c
aeea22
+++ b/lib/dns/nsec3.c
1e4169
@@ -1963,7 +1963,7 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, dns_name_t* name,
aeea22
 	 * Work out what this NSEC3 covers.
aeea22
 	 * Inside (<0) or outside (>=0).
aeea22
 	 */
aeea22
-	scope = isc_safe_memcompare(owner, nsec3.next, nsec3.next_length);
aeea22
+	scope = memcmp(owner, nsec3.next, nsec3.next_length);
aeea22
 
aeea22
 	/*
aeea22
 	 * Prepare to compute all the hashes.
1e4169
@@ -1987,7 +1987,7 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, dns_name_t* name,
aeea22
 			return (ISC_R_IGNORE);
aeea22
 		}
aeea22
 
aeea22
-		order = isc_safe_memcompare(hash, owner, length);
aeea22
+		order = memcmp(hash, owner, length);
aeea22
 		if (first && order == 0) {
aeea22
 			/*
aeea22
 			 * The hashes are the same.
aeea22
diff --git a/lib/dns/spnego.c b/lib/dns/spnego.c
ad7b3b
index ad77f24..670982a 100644
aeea22
--- a/lib/dns/spnego.c
aeea22
+++ b/lib/dns/spnego.c
ad7b3b
@@ -371,7 +371,7 @@ gssapi_spnego_decapsulate(OM_uint32 *,
aeea22
 
aeea22
 /* mod_auth_kerb.c */
aeea22
 
aeea22
-static int
aeea22
+static isc_boolean_t
aeea22
 cmp_gss_type(gss_buffer_t token, gss_OID gssoid)
aeea22
 {
aeea22
 	unsigned char *p;
ad7b3b
@@ -395,7 +395,7 @@ cmp_gss_type(gss_buffer_t token, gss_OID gssoid)
aeea22
 	if (((OM_uint32) *p++) != gssoid->length)
aeea22
 		return (GSS_S_DEFECTIVE_TOKEN);
aeea22
 
aeea22
-	return (isc_safe_memcompare(p, gssoid->elements, gssoid->length));
aeea22
+	return (!isc_safe_memequal(p, gssoid->elements, gssoid->length));
aeea22
 }
aeea22
 
aeea22
 /* accept_sec_context.c */
aeea22
diff --git a/lib/isc/Makefile.in b/lib/isc/Makefile.in
843e5f
index 0fd0837..8ad54bb 100644
aeea22
--- a/lib/isc/Makefile.in
aeea22
+++ b/lib/isc/Makefile.in
aeea22
@@ -60,7 +60,7 @@ OBJS =		@ISC_EXTRA_OBJS@ @ISC_PK11_O@ @ISC_PK11_RESULT_O@ \
aeea22
 		parseint.@O@ portset.@O@ quota.@O@ radix.@O@ random.@O@ \
aeea22
 		ratelimiter.@O@ refcount.@O@ region.@O@ regex.@O@ result.@O@ \
aeea22
 		rwlock.@O@ \
843e5f
-		safe.@O@ serial.@O@ siphash.@O@ sha1.@O@ sha2.@O@ sockaddr.@O@ stats.@O@ \
843e5f
+		serial.@O@ siphash.@O@ sha1.@O@ sha2.@O@ sockaddr.@O@ stats.@O@ \
aeea22
 		string.@O@ strtoul.@O@ symtab.@O@ task.@O@ taskpool.@O@ \
aeea22
 		tm.@O@ timer.@O@ version.@O@ \
aeea22
 		${UNIXOBJS} ${NLSOBJS} ${THREADOBJS}
aeea22
@@ -79,7 +79,7 @@ SRCS =		@ISC_EXTRA_SRCS@ @ISC_PK11_C@ @ISC_PK11_RESULT_C@ \
aeea22
 		netaddr.c netscope.c pool.c ondestroy.c \
aeea22
 		parseint.c portset.c quota.c radix.c random.c ${CHACHASRCS} \
aeea22
 		ratelimiter.c refcount.c region.c regex.c result.c rwlock.c \
843e5f
-		safe.c serial.c siphash.c sha1.c sha2.c sockaddr.c stats.c string.c \
843e5f
+		serial.c siphash.c sha1.c sha2.c sockaddr.c stats.c string.c \
aeea22
 		strtoul.c symtab.c task.c taskpool.c timer.c \
aeea22
 		tm.c version.c
aeea22
 
aeea22
@@ -95,10 +95,6 @@ TESTDIRS =	@UNITTESTS@
aeea22
 
aeea22
 @BIND9_MAKE_RULES@
aeea22
 
aeea22
-safe.@O@: safe.c
aeea22
-	${LIBTOOL_MODE_COMPILE} ${CC} ${ALL_CFLAGS} @CCNOOPT@ \
aeea22
-		-c ${srcdir}/safe.c
aeea22
-
aeea22
 version.@O@: version.c
aeea22
 	${LIBTOOL_MODE_COMPILE} ${CC} ${ALL_CFLAGS} \
aeea22
 		-DVERSION=\"${VERSION}\" \
aeea22
diff --git a/lib/isc/include/isc/safe.h b/lib/isc/include/isc/safe.h
ad7b3b
index 66ed08b..88b8f47 100644
aeea22
--- a/lib/isc/include/isc/safe.h
aeea22
+++ b/lib/isc/include/isc/safe.h
ad7b3b
@@ -15,29 +15,19 @@
aeea22
 
aeea22
 /*! \file isc/safe.h */
aeea22
 
ad7b3b
-#include <stdbool.h>
ad7b3b
-
aeea22
-#include <isc/types.h>
aeea22
-#include <stdlib.h>
aeea22
+#include <isc/lang.h>
aeea22
+#include <openssl/crypto.h>
aeea22
 
aeea22
 ISC_LANG_BEGINDECLS
aeea22
 
ad7b3b
-bool
aeea22
-isc_safe_memequal(const void *s1, const void *s2, size_t n);
ad7b3b
+#define isc_safe_memequal(s1, s2, n) !CRYPTO_memcmp(s1, s2, n)
aeea22
 /*%<
ad7b3b
  * Returns true iff. two blocks of memory are equal, otherwise
ad7b3b
  * false.
aeea22
  *
aeea22
  */
aeea22
 
aeea22
-int
aeea22
-isc_safe_memcompare(const void *b1, const void *b2, size_t len);
aeea22
-/*%<
aeea22
- * Clone of libc memcmp() which is safe to differential timing attacks.
aeea22
- */
aeea22
-
aeea22
-void
aeea22
-isc_safe_memwipe(void *ptr, size_t len);
aeea22
+#define isc_safe_memwipe(ptr, len) OPENSSL_cleanse(ptr, len)
aeea22
 /*%<
aeea22
  * Clear the memory of length `len` pointed to by `ptr`.
aeea22
  *
aeea22
diff --git a/lib/isc/safe.c b/lib/isc/safe.c
aeea22
deleted file mode 100644
ad7b3b
index 7a464b6..0000000
aeea22
--- a/lib/isc/safe.c
aeea22
+++ /dev/null
ad7b3b
@@ -1,83 +0,0 @@
aeea22
-/*
aeea22
- * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
aeea22
- *
aeea22
- * This Source Code Form is subject to the terms of the Mozilla Public
aeea22
- * License, v. 2.0. If a copy of the MPL was not distributed with this
aeea22
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
aeea22
- *
aeea22
- * See the COPYRIGHT file distributed with this work for additional
aeea22
- * information regarding copyright ownership.
aeea22
- */
aeea22
-
aeea22
-/*! \file */
aeea22
-
aeea22
-#include <config.h>
aeea22
-
ad7b3b
-#include <stdbool.h>
ad7b3b
-
aeea22
-#include <isc/safe.h>
aeea22
-#include <isc/string.h>
aeea22
-#include <isc/util.h>
aeea22
-
aeea22
-#ifdef WIN32
aeea22
-#include <windows.h>
aeea22
-#endif
aeea22
-
aeea22
-#ifdef _MSC_VER
aeea22
-#pragma optimize("", off)
aeea22
-#endif
aeea22
-
ad7b3b
-bool
aeea22
-isc_safe_memequal(const void *s1, const void *s2, size_t n) {
ad7b3b
-	uint8_t acc = 0;
aeea22
-
aeea22
-	if (n != 0U) {
ad7b3b
-		const uint8_t *p1 = s1, *p2 = s2;
aeea22
-
aeea22
-		do {
aeea22
-			acc |= *p1++ ^ *p2++;
aeea22
-		} while (--n != 0U);
aeea22
-	}
ad7b3b
-	return (acc == 0);
aeea22
-}
aeea22
-
aeea22
-
aeea22
-int
aeea22
-isc_safe_memcompare(const void *b1, const void *b2, size_t len) {
aeea22
-	const unsigned char *p1 = b1, *p2 = b2;
aeea22
-	size_t i;
aeea22
-	int res = 0, done = 0;
aeea22
-
aeea22
-	for (i = 0; i < len; i++) {
aeea22
-		/* lt is -1 if p1[i] < p2[i]; else 0. */
aeea22
-		int lt = (p1[i] - p2[i]) >> CHAR_BIT;
aeea22
-
aeea22
-		/* gt is -1 if p1[i] > p2[i]; else 0. */
aeea22
-		int gt = (p2[i] - p1[i]) >> CHAR_BIT;
aeea22
-
aeea22
-		/* cmp is 1 if p1[i] > p2[i]; -1 if p1[i] < p2[i]; else 0. */
aeea22
-		int cmp = lt - gt;
aeea22
-
aeea22
-		/* set res = cmp if !done. */
aeea22
-		res |= cmp & ~done;
aeea22
-
aeea22
-		/* set done if p1[i] != p2[i]. */
aeea22
-		done |= lt | gt;
aeea22
-	}
aeea22
-
aeea22
-	return (res);
aeea22
-}
aeea22
-
aeea22
-void
aeea22
-isc_safe_memwipe(void *ptr, size_t len) {
aeea22
-	if (ISC_UNLIKELY(ptr == NULL || len == 0))
aeea22
-		return;
aeea22
-
aeea22
-#ifdef WIN32
aeea22
-	SecureZeroMemory(ptr, len);
aeea22
-#elif HAVE_EXPLICIT_BZERO
aeea22
-	explicit_bzero(ptr, len);
aeea22
-#else
aeea22
-	memset(ptr, 0, len);
aeea22
-#endif
aeea22
-}
aeea22
diff --git a/lib/isc/tests/safe_test.c b/lib/isc/tests/safe_test.c
843e5f
index 266ac75..60e9181 100644
aeea22
--- a/lib/isc/tests/safe_test.c
aeea22
+++ b/lib/isc/tests/safe_test.c
843e5f
@@ -45,22 +45,6 @@ isc_safe_memequal_test(void **state) {
1e4169
 				       "\x00\x00\x00\x00", 4));
aeea22
 }
aeea22
 
1e4169
-/* test isc_safe_memcompare() */
1e4169
-static void
1e4169
-isc_safe_memcompare_test(void **state) {
1e4169
-	UNUSED(state);
aeea22
-
1e4169
-	assert_int_equal(isc_safe_memcompare("test", "test", 4), 0);
1e4169
-	assert_true(isc_safe_memcompare("test", "tesc", 4) > 0);
1e4169
-	assert_true(isc_safe_memcompare("test", "tesy", 4) < 0);
1e4169
-	assert_int_equal(isc_safe_memcompare("\x00\x00\x00\x00",
1e4169
-					     "\x00\x00\x00\x00", 4), 0);
1e4169
-	assert_true(isc_safe_memcompare("\x00\x00\x00\x00",
1e4169
-					"\x00\x00\x00\x01", 4) < 0);
1e4169
-	assert_true(isc_safe_memcompare("\x00\x00\x00\x02",
1e4169
-					"\x00\x00\x00\x00", 4) > 0);
aeea22
-}
aeea22
-
1e4169
 /* test isc_safe_memwipe() */
1e4169
 static void
1e4169
 isc_safe_memwipe_test(void **state) {
843e5f
@@ -69,7 +53,6 @@ isc_safe_memwipe_test(void **state) {
aeea22
 	/* These should pass. */
aeea22
 	isc_safe_memwipe(NULL, 0);
aeea22
 	isc_safe_memwipe((void *) -1, 0);
aeea22
-	isc_safe_memwipe(NULL, 42);
aeea22
 
aeea22
 	/*
aeea22
 	 * isc_safe_memwipe(ptr, size) should function same as
843e5f
@@ -108,7 +91,6 @@ main(void) {
1e4169
 	const struct CMUnitTest tests[] = {
1e4169
 		cmocka_unit_test(isc_safe_memequal_test),
1e4169
 		cmocka_unit_test(isc_safe_memwipe_test),
1e4169
-		cmocka_unit_test(isc_safe_memcompare_test),
1e4169
 	};
1e4169
 
1e4169
 	return (cmocka_run_group_tests(tests, NULL, NULL));
aeea22
-- 
1e4169
2.20.1
aeea22