From 4e114f8ed653700f411f29bf9e87392d8f4ff9db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Thu, 12 Mar 2020 10:20:37 +0100
Subject: [PATCH] Stop leaking OpenSSL types and defines in the isc/md.h

The <isc/md.h> header directly included <openssl/evp.h> header which
enforced all users of the libisc library to explicitly list the include
path to OpenSSL and link with -lcrypto.  By hiding the specific
implementation into the private namespace, we no longer enforce this.
In the long run, this might also allow us to switch cryptographic
library implementation without affecting the downstream users.

While making the isc_md_type_t type opaque, the API using the data type
was changed to use the pointer to isc_md_type_t instead of using the
type directly.
---
 lib/dns/ds.c                |  2 +-
 lib/dns/hmac_link.c         | 26 +++++++++++-----------
 lib/isc/hmac.c              |  6 +++---
 lib/isc/include/isc/hmac.h  |  6 +++---
 lib/isc/include/isc/md.h    | 43 +++++++++++++++++++++++--------------
 lib/isc/md.c                | 26 +++++++++++++++++-----
 lib/isc/tests/hmac_test.c   |  2 +-
 lib/isc/tests/md_test.c     |  4 ++--
 lib/isc/win32/libisc.def.in |  6 ++++++
 lib/isccc/cc.c              |  5 +++--
 10 files changed, 81 insertions(+), 45 deletions(-)

diff --git a/lib/dns/ds.c b/lib/dns/ds.c
index 93ce59f3f9..7f6c8ba3c4 100644
--- a/lib/dns/ds.c
+++ b/lib/dns/ds.c
@@ -37,7 +37,7 @@ dns_ds_fromkeyrdata(const dns_name_t *owner, dns_rdata_t *key,
 	unsigned int digestlen;
 	isc_region_t r;
 	isc_md_t *md;
-	isc_md_type_t md_type = 0;
+	const isc_md_type_t *md_type = NULL;
 
 	REQUIRE(key != NULL);
 	REQUIRE(key->type == dns_rdatatype_dnskey ||
diff --git a/lib/dns/hmac_link.c b/lib/dns/hmac_link.c
index 7524fcf062..0dfe82ded0 100644
--- a/lib/dns/hmac_link.c
+++ b/lib/dns/hmac_link.c
@@ -137,7 +137,7 @@
 	}
 
 static isc_result_t
-hmac_fromdns(isc_md_type_t type, dst_key_t *key, isc_buffer_t *data);
+hmac_fromdns(const isc_md_type_t *type, dst_key_t *key, isc_buffer_t *data);
 
 struct dst_hmac_key {
 	uint8_t key[ISC_MAX_BLOCK_SIZE];
@@ -157,7 +157,8 @@ getkeybits(dst_key_t *key, struct dst_private_element *element) {
 }
 
 static inline isc_result_t
-hmac_createctx(isc_md_type_t type, const dst_key_t *key, dst_context_t *dctx) {
+hmac_createctx(const isc_md_type_t *type, const dst_key_t *key,
+	       dst_context_t *dctx) {
 	isc_result_t result;
 	const dst_hmac_key_t *hkey = key->keydata.hmac_key;
 	isc_hmac_t *ctx = isc_hmac_new(); /* Either returns or abort()s */
@@ -246,7 +247,8 @@ hmac_verify(const dst_context_t *dctx, const isc_region_t *sig) {
 }
 
 static inline bool
-hmac_compare(isc_md_type_t type, const dst_key_t *key1, const dst_key_t *key2) {
+hmac_compare(const isc_md_type_t *type, const dst_key_t *key1,
+	     const dst_key_t *key2) {
 	dst_hmac_key_t *hkey1, *hkey2;
 
 	hkey1 = key1->keydata.hmac_key;
@@ -263,7 +265,7 @@ hmac_compare(isc_md_type_t type, const dst_key_t *key1, const dst_key_t *key2) {
 }
 
 static inline isc_result_t
-hmac_generate(isc_md_type_t type, dst_key_t *key) {
+hmac_generate(const isc_md_type_t *type, dst_key_t *key) {
 	isc_buffer_t b;
 	isc_result_t ret;
 	unsigned int bytes, len;
@@ -306,11 +308,10 @@ hmac_destroy(dst_key_t *key) {
 
 static inline isc_result_t
 hmac_todns(const dst_key_t *key, isc_buffer_t *data) {
+	REQUIRE(key != NULL && key->keydata.hmac_key != NULL);
 	dst_hmac_key_t *hkey = key->keydata.hmac_key;
 	unsigned int bytes;
 
-	REQUIRE(hkey != NULL);
-
 	bytes = (key->key_size + 7) / 8;
 	if (isc_buffer_availablelength(data) < bytes) {
 		return (ISC_R_NOSPACE);
@@ -321,7 +322,7 @@ hmac_todns(const dst_key_t *key, isc_buffer_t *data) {
 }
 
 static inline isc_result_t
-hmac_fromdns(isc_md_type_t type, dst_key_t *key, isc_buffer_t *data) {
+hmac_fromdns(const isc_md_type_t *type, dst_key_t *key, isc_buffer_t *data) {
 	dst_hmac_key_t *hkey;
 	unsigned int keylen;
 	isc_region_t r;
@@ -356,7 +357,7 @@ hmac_fromdns(isc_md_type_t type, dst_key_t *key, isc_buffer_t *data) {
 }
 
 static inline int
-hmac__get_tag_key(isc_md_type_t type) {
+hmac__get_tag_key(const isc_md_type_t *type) {
 	if (type == ISC_MD_MD5) {
 		return (TAG_HMACMD5_KEY);
 	} else if (type == ISC_MD_SHA1) {
@@ -376,7 +377,7 @@ hmac__get_tag_key(isc_md_type_t type) {
 }
 
 static inline int
-hmac__get_tag_bits(isc_md_type_t type) {
+hmac__get_tag_bits(const isc_md_type_t *type) {
 	if (type == ISC_MD_MD5) {
 		return (TAG_HMACMD5_BITS);
 	} else if (type == ISC_MD_SHA1) {
@@ -396,7 +397,8 @@ hmac__get_tag_bits(isc_md_type_t type) {
 }
 
 static inline isc_result_t
-hmac_tofile(isc_md_type_t type, const dst_key_t *key, const char *directory) {
+hmac_tofile(const isc_md_type_t *type, const dst_key_t *key,
+	    const char *directory) {
 	dst_hmac_key_t *hkey;
 	dst_private_t priv;
 	int bytes = (key->key_size + 7) / 8;
@@ -428,7 +430,7 @@ hmac_tofile(isc_md_type_t type, const dst_key_t *key, const char *directory) {
 }
 
 static inline int
-hmac__to_dst_alg(isc_md_type_t type) {
+hmac__to_dst_alg(const isc_md_type_t *type) {
 	if (type == ISC_MD_MD5) {
 		return (DST_ALG_HMACMD5);
 	} else if (type == ISC_MD_SHA1) {
@@ -448,7 +450,7 @@ hmac__to_dst_alg(isc_md_type_t type) {
 }
 
 static inline isc_result_t
-hmac_parse(isc_md_type_t type, dst_key_t *key, isc_lex_t *lexer,
+hmac_parse(const isc_md_type_t *type, dst_key_t *key, isc_lex_t *lexer,
 	   dst_key_t *pub) {
 	dst_private_t priv;
 	isc_result_t result, tresult;
diff --git a/lib/isc/hmac.c b/lib/isc/hmac.c
index 2f186f8df7..7399792eba 100644
--- a/lib/isc/hmac.c
+++ b/lib/isc/hmac.c
@@ -41,7 +41,7 @@ isc_hmac_free(isc_hmac_t *hmac) {
 
 isc_result_t
 isc_hmac_init(isc_hmac_t *hmac, const void *key, size_t keylen,
-	      isc_md_type_t md_type) {
+	      const isc_md_type_t *md_type) {
 	REQUIRE(hmac != NULL);
 	REQUIRE(key != NULL);
 
@@ -95,7 +95,7 @@ isc_hmac_final(isc_hmac_t *hmac, unsigned char *digest,
 	return (ISC_R_SUCCESS);
 }
 
-isc_md_type_t
+const isc_md_type_t *
 isc_hmac_get_md_type(isc_hmac_t *hmac) {
 	REQUIRE(hmac != NULL);
 
@@ -117,7 +117,7 @@ isc_hmac_get_block_size(isc_hmac_t *hmac) {
 }
 
 isc_result_t
-isc_hmac(isc_md_type_t type, const void *key, const int keylen,
+isc_hmac(const isc_md_type_t *type, const void *key, const int keylen,
 	 const unsigned char *buf, const size_t len, unsigned char *digest,
 	 unsigned int *digestlen) {
 	isc_result_t res;
diff --git a/lib/isc/include/isc/hmac.h b/lib/isc/include/isc/hmac.h
index ab349c10f1..5dcbcf71de 100644
--- a/lib/isc/include/isc/hmac.h
+++ b/lib/isc/include/isc/hmac.h
@@ -42,7 +42,7 @@ typedef void isc_hmac_t;
  * (i.e. the length of the digest) will be written to the @digestlen.
  */
 isc_result_t
-isc_hmac(isc_md_type_t type, const void *key, const int keylen,
+isc_hmac(const isc_md_type_t *type, const void *key, const int keylen,
 	 const unsigned char *buf, const size_t len, unsigned char *digest,
 	 unsigned int *digestlen);
 
@@ -76,7 +76,7 @@ isc_hmac_free(isc_hmac_t *hmac);
 
 isc_result_t
 isc_hmac_init(isc_hmac_t *hmac, const void *key, size_t keylen,
-	      isc_md_type_t type);
+	      const isc_md_type_t *type);
 
 /**
  * isc_hmac_reset:
@@ -123,7 +123,7 @@ isc_hmac_final(isc_hmac_t *hmac, unsigned char *digest,
  * This function return the isc_md_type_t previously set for the supplied
  * HMAC context or NULL if no isc_md_type_t has been set.
  */
-isc_md_type_t
+const isc_md_type_t *
 isc_hmac_get_md_type(isc_hmac_t *hmac);
 
 /**
diff --git a/lib/isc/include/isc/md.h b/lib/isc/include/isc/md.h
index 27720fe70a..80cd340673 100644
--- a/lib/isc/include/isc/md.h
+++ b/lib/isc/include/isc/md.h
@@ -21,9 +21,7 @@
 #include <isc/result.h>
 #include <isc/types.h>
 
-#include <openssl/evp.h>
-
-typedef EVP_MD_CTX isc_md_t;
+typedef void isc_md_t;
 
 /**
  * isc_md_type_t:
@@ -36,14 +34,27 @@ typedef EVP_MD_CTX isc_md_t;
  *
  * Enumeration of supported message digest algorithms.
  */
-typedef const EVP_MD *isc_md_type_t;
+typedef void isc_md_type_t;
 
-#define ISC_MD_MD5    EVP_md5()
-#define ISC_MD_SHA1   EVP_sha1()
-#define ISC_MD_SHA224 EVP_sha224()
-#define ISC_MD_SHA256 EVP_sha256()
-#define ISC_MD_SHA384 EVP_sha384()
-#define ISC_MD_SHA512 EVP_sha512()
+#define ISC_MD_MD5    isc__md_md5()
+#define ISC_MD_SHA1   isc__md_sha1()
+#define ISC_MD_SHA224 isc__md_sha224()
+#define ISC_MD_SHA256 isc__md_sha256()
+#define ISC_MD_SHA384 isc__md_sha384()
+#define ISC_MD_SHA512 isc__md_sha512()
+
+const isc_md_type_t *
+isc__md_md5(void);
+const isc_md_type_t *
+isc__md_sha1(void);
+const isc_md_type_t *
+isc__md_sha224(void);
+const isc_md_type_t *
+isc__md_sha256(void);
+const isc_md_type_t *
+isc__md_sha384(void);
+const isc_md_type_t *
+isc__md_sha512(void);
 
 #define ISC_MD5_DIGESTLENGTH	isc_md_type_get_size(ISC_MD_MD5)
 #define ISC_MD5_BLOCK_LENGTH	isc_md_type_get_block_size(ISC_MD_MD5)
@@ -58,7 +69,7 @@ typedef const EVP_MD *isc_md_type_t;
 #define ISC_SHA512_DIGESTLENGTH isc_md_type_get_size(ISC_MD_SHA512)
 #define ISC_SHA512_BLOCK_LENGTH isc_md_type_get_block_size(ISC_MD_SHA512)
 
-#define ISC_MAX_MD_SIZE	   EVP_MAX_MD_SIZE
+#define ISC_MAX_MD_SIZE	   64U	/* EVP_MAX_MD_SIZE */
 #define ISC_MAX_BLOCK_SIZE 128U /* ISC_SHA512_BLOCK_LENGTH */
 
 /**
@@ -75,7 +86,7 @@ typedef const EVP_MD *isc_md_type_t;
  * at @digestlen, at most ISC_MAX_MD_SIZE bytes will be written.
  */
 isc_result_t
-isc_md(isc_md_type_t type, const unsigned char *buf, const size_t len,
+isc_md(const isc_md_type_t *type, const unsigned char *buf, const size_t len,
        unsigned char *digest, unsigned int *digestlen);
 
 /**
@@ -105,7 +116,7 @@ isc_md_free(isc_md_t *);
  * initialized before calling this function.
  */
 isc_result_t
-isc_md_init(isc_md_t *, const isc_md_type_t md_type);
+isc_md_init(isc_md_t *, const isc_md_type_t *md_type);
 
 /**
  * isc_md_reset:
@@ -152,7 +163,7 @@ isc_md_final(isc_md_t *md, unsigned char *digest, unsigned int *digestlen);
  * This function return the isc_md_type_t previously set for the supplied
  * message digest context or NULL if no isc_md_type_t has been set.
  */
-isc_md_type_t
+const isc_md_type_t *
 isc_md_get_md_type(isc_md_t *md);
 
 /**
@@ -180,7 +191,7 @@ isc_md_get_block_size(isc_md_t *md);
  * isc_md_type_t , i.e. the size of the hash.
  */
 size_t
-isc_md_type_get_size(isc_md_type_t md_type);
+isc_md_type_get_size(const isc_md_type_t *md_type);
 
 /**
  * isc_md_block_size:
@@ -189,4 +200,4 @@ isc_md_type_get_size(isc_md_type_t md_type);
  * isc_md_type_t.
  */
 size_t
-isc_md_type_get_block_size(isc_md_type_t md_type);
+isc_md_type_get_block_size(const isc_md_type_t *md_type);
diff --git a/lib/isc/md.c b/lib/isc/md.c
index 0c572cc755..dd503ed51d 100644
--- a/lib/isc/md.c
+++ b/lib/isc/md.c
@@ -37,7 +37,7 @@ isc_md_free(isc_md_t *md) {
 }
 
 isc_result_t
-isc_md_init(isc_md_t *md, const isc_md_type_t md_type) {
+isc_md_init(isc_md_t *md, const isc_md_type_t *md_type) {
 	REQUIRE(md != NULL);
 
 	if (md_type == NULL) {
@@ -89,7 +89,7 @@ isc_md_final(isc_md_t *md, unsigned char *digest, unsigned int *digestlen) {
 	return (ISC_R_SUCCESS);
 }
 
-isc_md_type_t
+const isc_md_type_t *
 isc_md_get_md_type(isc_md_t *md) {
 	REQUIRE(md != NULL);
 
@@ -111,7 +111,10 @@ isc_md_get_block_size(isc_md_t *md) {
 }
 
 size_t
-isc_md_type_get_size(isc_md_type_t md_type) {
+isc_md_type_get_size(const isc_md_type_t *md_type) {
+	STATIC_ASSERT(ISC_MAX_MD_SIZE >= EVP_MAX_MD_SIZE,
+		      "Change ISC_MAX_MD_SIZE to be greater than or equal to "
+		      "EVP_MAX_MD_SIZE");
 	if (md_type != NULL) {
 		return ((size_t)EVP_MD_size(md_type));
 	}
@@ -120,7 +123,10 @@ isc_md_type_get_size(isc_md_type_t md_type) {
 }
 
 size_t
-isc_md_type_get_block_size(isc_md_type_t md_type) {
+isc_md_type_get_block_size(const isc_md_type_t *md_type) {
+	STATIC_ASSERT(ISC_MAX_MD_SIZE >= EVP_MAX_MD_SIZE,
+		      "Change ISC_MAX_MD_SIZE to be greater than or equal to "
+		      "EVP_MAX_MD_SIZE");
 	if (md_type != NULL) {
 		return ((size_t)EVP_MD_block_size(md_type));
 	}
@@ -129,7 +135,7 @@ isc_md_type_get_block_size(isc_md_type_t md_type) {
 }
 
 isc_result_t
-isc_md(isc_md_type_t md_type, const unsigned char *buf, const size_t len,
+isc_md(const isc_md_type_t *md_type, const unsigned char *buf, const size_t len,
        unsigned char *digest, unsigned int *digestlen) {
 	isc_md_t *md;
 	isc_result_t res;
@@ -155,3 +161,13 @@ end:
 
 	return (res);
 }
+
+#define md_register_algorithm(alg) \
+	const isc_md_type_t *isc__md_##alg(void) { return (EVP_##alg()); }
+
+md_register_algorithm(md5);
+md_register_algorithm(sha1);
+md_register_algorithm(sha224);
+md_register_algorithm(sha256);
+md_register_algorithm(sha384);
+md_register_algorithm(sha512);
diff --git a/lib/isc/tests/hmac_test.c b/lib/isc/tests/hmac_test.c
index 78ffd013cc..69f68e055e 100644
--- a/lib/isc/tests/hmac_test.c
+++ b/lib/isc/tests/hmac_test.c
@@ -84,7 +84,7 @@ isc_hmac_free_test(void **state) {
 
 static void
 isc_hmac_test(isc_hmac_t *hmac, const void *key, size_t keylen,
-	      isc_md_type_t type, const char *buf, size_t buflen,
+	      const isc_md_type_t *type, const char *buf, size_t buflen,
 	      const char *result, const int repeats) {
 	assert_non_null(hmac);
 	assert_int_equal(isc_hmac_init(hmac, key, keylen, type), ISC_R_SUCCESS);
diff --git a/lib/isc/tests/md_test.c b/lib/isc/tests/md_test.c
index 62d6cd163b..074defa467 100644
--- a/lib/isc/tests/md_test.c
+++ b/lib/isc/tests/md_test.c
@@ -82,8 +82,8 @@ isc_md_free_test(void **state) {
 }
 
 static void
-isc_md_test(isc_md_t *md, isc_md_type_t type, const char *buf, size_t buflen,
-	    const char *result, const int repeats) {
+isc_md_test(isc_md_t *md, const isc_md_type_t *type, const char *buf,
+	    size_t buflen, const char *result, const int repeats) {
 	assert_non_null(md);
 	assert_int_equal(isc_md_init(md, type), ISC_R_SUCCESS);
 
diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in
index 1559661777..c0cbfc164a 100644
--- a/lib/isc/win32/libisc.def.in
+++ b/lib/isc/win32/libisc.def.in
@@ -61,6 +61,12 @@ isc__mem_reallocate
 isc__mem_strdup
 isc__mempool_get
 isc__mempool_put
+isc__md_md5
+isc__md_sha1
+isc__md_sha224
+isc__md_sha256
+isc__md_sha384
+isc__md_sha512
 isc_socket_accept
 isc_socket_attach
 isc_socket_bind
diff --git a/lib/isccc/cc.c b/lib/isccc/cc.c
index e781e92e2d..84397a9d58 100644
--- a/lib/isccc/cc.c
+++ b/lib/isccc/cc.c
@@ -29,6 +29,7 @@
 #include <inttypes.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <isc/assertions.h>
@@ -249,7 +250,7 @@ list_towire(isccc_sexpr_t *list, isc_buffer_t **buffer) {
 static isc_result_t
 sign(unsigned char *data, unsigned int length, unsigned char *hmac,
      uint32_t algorithm, isccc_region_t *secret) {
-	isc_md_type_t md_type;
+	const isc_md_type_t *md_type;
 	isc_result_t result;
 	isccc_region_t source, target;
 	unsigned char digest[ISC_MAX_MD_SIZE];
@@ -370,7 +371,7 @@ isccc_cc_towire(isccc_sexpr_t *alist, isc_buffer_t **buffer, uint32_t algorithm,
 static isc_result_t
 verify(isccc_sexpr_t *alist, unsigned char *data, unsigned int length,
        uint32_t algorithm, isccc_region_t *secret) {
-	isc_md_type_t md_type;
+	const isc_md_type_t *md_type;
 	isccc_region_t source;
 	isccc_region_t target;
 	isc_result_t result;