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