diff --git a/CHANGES b/CHANGES index ca42081834..1b9233d1ab 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +4183. [cleanup] Use timing-safe memory comparisons in cryptographic + code. Also, the timing-safe comparison functions have + been renamed to avoid possible confusion with + memcmp(). [RT #40148] + 4182. [cleanup] Use mnemonics for RR class and type comparisons. [RT #40297] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index a29de87cb2..0753095299 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -82,6 +82,7 @@ #include #include #include +#include #include #include #include @@ -3460,7 +3461,7 @@ process_cookie(dig_lookup_t *l, dns_message_t *msg, INSIST(msg->cc_ok == 0 && msg->cc_bad == 0); if (optlen >= len && optlen >= 8U) { - if (memcmp(isc_buffer_current(optbuf), sent, 8) == 0) { + if (isc_safe_memequal(isc_buffer_current(optbuf), sent, 8)) { msg->cc_ok = 1; } else { printf(";; Warning: Client COOKIE mismatch\n"); diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 8190fc081a..980a7c0708 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include @@ -767,7 +768,7 @@ hashlist_add_dns_name(hashlist_t *l, /*const*/ dns_name_t *name, static int hashlist_comp(const void *a, const void *b) { - return (memcmp(a, b, hash_length + 1)); + return (isc_safe_memcompare(a, b, hash_length + 1)); } static void @@ -794,7 +795,7 @@ hashlist_hasdup(hashlist_t *l) { next += l->length; if (next[l->length-1] != 0) continue; - if (memcmp(current, next, l->length - 1) == 0) + if (isc_safe_memequal(current, next, l->length - 1)) return (ISC_TRUE); current = next; } @@ -2025,7 +2026,7 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node, if (exists && nsec3.hash == hashalg && nsec3.iterations == iterations && nsec3.salt_length == salt_len && - !memcmp(nsec3.salt, salt, salt_len)) + isc_safe_memequal(nsec3.salt, salt, salt_len)) continue; dns_rdatalist_init(&rdatalist); rdatalist.rdclass = rdata.rdclass; @@ -2713,7 +2714,7 @@ set_nsec3params(isc_boolean_t update, isc_boolean_t set_salt, if (!update && set_salt) { if (salt_length != orig_saltlen || - memcmp(saltbuf, orig_salt, salt_length) != 0) + !isc_safe_memequal(saltbuf, orig_salt, salt_length)) fatal("An NSEC3 chain exists with a different salt. " "Use -u to update it."); } else if (!set_salt) { diff --git a/bin/named/client.c b/bin/named/client.c index 4df385f78b..64c6180b21 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -1815,7 +1816,7 @@ process_cookie(ns_client_t *client, isc_buffer_t *buf, size_t optlen) { isc_buffer_init(&db, dbuf, sizeof(dbuf)); compute_cookie(client, when, nonce, &db); - if (memcmp(old, dbuf, COOKIE_SIZE) != 0) { + if (!isc_safe_memequal(old, dbuf, COOKIE_SIZE)) { isc_stats_increment(ns_g_server->nsstats, dns_nsstatscounter_cookienomatch); return; diff --git a/lib/dns/client.c b/lib/dns/client.c index 1e4c9ae376..c150fd19e8 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -14,8 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: client.c,v 1.14 2011/03/12 04:59:47 tbox Exp $ */ - #include #include @@ -24,6 +22,7 @@ #include #include #include +#include #include #include #include diff --git a/lib/dns/hmac_link.c b/lib/dns/hmac_link.c index fad20a02d3..794ff86617 100644 --- a/lib/dns/hmac_link.c +++ b/lib/dns/hmac_link.c @@ -139,7 +139,7 @@ hmacmd5_compare(const dst_key_t *key1, const dst_key_t *key2) { else if (hkey1 == NULL || hkey2 == NULL) return (ISC_FALSE); - if (isc_safe_memcmp(hkey1->key, hkey2->key, ISC_MD5_BLOCK_LENGTH)) + if (isc_safe_memequal(hkey1->key, hkey2->key, ISC_MD5_BLOCK_LENGTH)) return (ISC_TRUE); else return (ISC_FALSE); @@ -424,7 +424,7 @@ hmacsha1_compare(const dst_key_t *key1, const dst_key_t *key2) { else if (hkey1 == NULL || hkey2 == NULL) return (ISC_FALSE); - if (isc_safe_memcmp(hkey1->key, hkey2->key, ISC_SHA1_BLOCK_LENGTH)) + if (isc_safe_memequal(hkey1->key, hkey2->key, ISC_SHA1_BLOCK_LENGTH)) return (ISC_TRUE); else return (ISC_FALSE); @@ -709,7 +709,7 @@ hmacsha224_compare(const dst_key_t *key1, const dst_key_t *key2) { else if (hkey1 == NULL || hkey2 == NULL) return (ISC_FALSE); - if (isc_safe_memcmp(hkey1->key, hkey2->key, ISC_SHA224_BLOCK_LENGTH)) + if (isc_safe_memequal(hkey1->key, hkey2->key, ISC_SHA224_BLOCK_LENGTH)) return (ISC_TRUE); else return (ISC_FALSE); @@ -996,7 +996,7 @@ hmacsha256_compare(const dst_key_t *key1, const dst_key_t *key2) { else if (hkey1 == NULL || hkey2 == NULL) return (ISC_FALSE); - if (isc_safe_memcmp(hkey1->key, hkey2->key, ISC_SHA256_BLOCK_LENGTH)) + if (isc_safe_memequal(hkey1->key, hkey2->key, ISC_SHA256_BLOCK_LENGTH)) return (ISC_TRUE); else return (ISC_FALSE); @@ -1283,7 +1283,7 @@ hmacsha384_compare(const dst_key_t *key1, const dst_key_t *key2) { else if (hkey1 == NULL || hkey2 == NULL) return (ISC_FALSE); - if (isc_safe_memcmp(hkey1->key, hkey2->key, ISC_SHA384_BLOCK_LENGTH)) + if (isc_safe_memequal(hkey1->key, hkey2->key, ISC_SHA384_BLOCK_LENGTH)) return (ISC_TRUE); else return (ISC_FALSE); @@ -1570,7 +1570,7 @@ hmacsha512_compare(const dst_key_t *key1, const dst_key_t *key2) { else if (hkey1 == NULL || hkey2 == NULL) return (ISC_FALSE); - if (isc_safe_memcmp(hkey1->key, hkey2->key, ISC_SHA512_BLOCK_LENGTH)) + if (isc_safe_memequal(hkey1->key, hkey2->key, ISC_SHA512_BLOCK_LENGTH)) return (ISC_TRUE); else return (ISC_FALSE); diff --git a/lib/dns/nsec3.c b/lib/dns/nsec3.c index ff1282b9be..de49f25e1c 100644 --- a/lib/dns/nsec3.c +++ b/lib/dns/nsec3.c @@ -25,6 +25,7 @@ #include #include #include +#include #include @@ -1925,7 +1926,7 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, dns_name_t* name, * Work out what this NSEC3 covers. * Inside (<0) or outside (>=0). */ - scope = memcmp(owner, nsec3.next, nsec3.next_length); + scope = isc_safe_memcompare(owner, nsec3.next, nsec3.next_length); /* * Prepare to compute all the hashes. @@ -1950,7 +1951,7 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, dns_name_t* name, return (ISC_R_IGNORE); } - order = memcmp(hash, owner, length); + order = isc_safe_memcompare(hash, owner, length); if (first && order == 0) { /* * The hashes are the same. diff --git a/lib/dns/opensslgost_link.c b/lib/dns/opensslgost_link.c index 2399648736..8fcefe4878 100644 --- a/lib/dns/opensslgost_link.c +++ b/lib/dns/opensslgost_link.c @@ -14,14 +14,13 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: opensslgost_link.c,v 1.5 2011/01/19 23:47:12 tbox Exp $ */ - #include #if defined(OPENSSL) && defined(HAVE_OPENSSL_GOST) #include #include +#include #include #include @@ -308,7 +307,7 @@ opensslgost_todns(const dst_key_t *key, isc_buffer_t *data) { p = der; len = i2d_PUBKEY(pkey, &p); INSIST(len == sizeof(der)); - INSIST(memcmp(gost_prefix, der, 37) == 0); + INSIST(isc_safe_memequal(gost_prefix, der, 37)); memmove(r.base, der + 37, 64); isc_buffer_add(data, 64); diff --git a/lib/dns/opensslrsa_link.c b/lib/dns/opensslrsa_link.c index e34cdd823d..96433e84b1 100644 --- a/lib/dns/opensslrsa_link.c +++ b/lib/dns/opensslrsa_link.c @@ -650,9 +650,10 @@ opensslrsa_verify2(dst_context_t *dctx, int maxbits, const isc_region_t *sig) { DST_R_VERIFYFAILURE)); if (status != (int)(prefixlen + digestlen)) return (DST_R_VERIFYFAILURE); - if (memcmp(original, prefix, prefixlen)) + if (!isc_safe_memequal(original, prefix, prefixlen)) return (DST_R_VERIFYFAILURE); - if (memcmp(original + prefixlen, digest, digestlen)) + if (!isc_safe_memequal(original + prefixlen, + digest, digestlen)) return (DST_R_VERIFYFAILURE); status = 1; } diff --git a/lib/dns/pkcs11dh_link.c b/lib/dns/pkcs11dh_link.c index 12e98ca948..30ca031398 100644 --- a/lib/dns/pkcs11dh_link.c +++ b/lib/dns/pkcs11dh_link.c @@ -276,7 +276,8 @@ pkcs11dh_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dh1, CKA_BASE); @@ -285,7 +286,8 @@ pkcs11dh_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dh1, CKA_VALUE); @@ -294,7 +296,8 @@ pkcs11dh_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dh1, CKA_VALUE2); @@ -302,7 +305,8 @@ pkcs11dh_compare(const dst_key_t *key1, const dst_key_t *key2) { if (((attr1 != NULL) || (attr2 != NULL)) && ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen))) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen))) return (ISC_FALSE); if (!dh1->ontoken && !dh2->ontoken) @@ -333,7 +337,8 @@ pkcs11dh_paramcompare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dh1, CKA_BASE); @@ -342,7 +347,8 @@ pkcs11dh_paramcompare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); return (ISC_TRUE); @@ -682,13 +688,13 @@ pkcs11dh_todns(const dst_key_t *key, isc_buffer_t *data) { isc_buffer_availableregion(data, &r); - if ((glen == 1) && (memcmp(pk11_dh_bn2, base, glen) == 0) && + if ((glen == 1) && isc_safe_memequal(pk11_dh_bn2, base, glen) && (((plen == sizeof(pk11_dh_bn768)) && - (memcmp(pk11_dh_bn768, prime, plen) == 0)) || + isc_safe_memequal(pk11_dh_bn768, prime, plen)) || ((plen == sizeof(pk11_dh_bn1024)) && - (memcmp(pk11_dh_bn1024, prime, plen) == 0)) || + isc_safe_memequal(pk11_dh_bn1024, prime, plen)) || ((plen == sizeof(pk11_dh_bn1536)) && - (memcmp(pk11_dh_bn1536, prime, plen) == 0)))) { + isc_safe_memequal(pk11_dh_bn1536, prime, plen)))) { plen = 1; glen = 0; } @@ -699,10 +705,11 @@ pkcs11dh_todns(const dst_key_t *key, isc_buffer_t *data) { uint16_toregion(plen, &r); if (plen == 1) { - if (memcmp(pk11_dh_bn768, prime, sizeof(pk11_dh_bn768)) == 0) + if (isc_safe_memequal(pk11_dh_bn768, prime, + sizeof(pk11_dh_bn768))) *r.base = 1; - else if (memcmp(pk11_dh_bn1024, prime, - sizeof(pk11_dh_bn1024)) == 0) + else if (isc_safe_memequal(pk11_dh_bn1024, prime, + sizeof(pk11_dh_bn1024))) *r.base = 2; else *r.base = 3; @@ -819,7 +826,7 @@ pkcs11dh_fromdns(dst_key_t *key, isc_buffer_t *data) { } else { base = r.base; - if (memcmp(base, pk11_dh_bn2, glen) == 0) { + if (isc_safe_memequal(base, pk11_dh_bn2, glen)) { base = pk11_dh_bn2; glen_ = sizeof(pk11_dh_bn2); } diff --git a/lib/dns/pkcs11dsa_link.c b/lib/dns/pkcs11dsa_link.c index 6b7d5f6752..7a0787c48d 100644 --- a/lib/dns/pkcs11dsa_link.c +++ b/lib/dns/pkcs11dsa_link.c @@ -14,8 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - #ifdef PKCS11CRYPTO #include @@ -443,7 +441,8 @@ pkcs11dsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dsa1, CKA_SUBPRIME); @@ -452,7 +451,8 @@ pkcs11dsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dsa1, CKA_BASE); @@ -461,7 +461,8 @@ pkcs11dsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dsa1, CKA_VALUE); @@ -470,7 +471,8 @@ pkcs11dsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(dsa1, CKA_VALUE2); @@ -478,7 +480,8 @@ pkcs11dsa_compare(const dst_key_t *key1, const dst_key_t *key2) { if (((attr1 != NULL) || (attr2 != NULL)) && ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen))) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen))) return (ISC_FALSE); if (!dsa1->ontoken && !dsa2->ontoken) diff --git a/lib/dns/pkcs11ecdsa_link.c b/lib/dns/pkcs11ecdsa_link.c index d57be91970..bca32b1cba 100644 --- a/lib/dns/pkcs11ecdsa_link.c +++ b/lib/dns/pkcs11ecdsa_link.c @@ -14,8 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - #include #if defined(PKCS11CRYPTO) && defined(HAVE_PKCS11_ECDSA) @@ -398,7 +396,8 @@ pkcs11ecdsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(ec1, CKA_EC_POINT); @@ -407,7 +406,8 @@ pkcs11ecdsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(ec1, CKA_VALUE); @@ -415,7 +415,8 @@ pkcs11ecdsa_compare(const dst_key_t *key1, const dst_key_t *key2) { if (((attr1 != NULL) || (attr2 != NULL)) && ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen))) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen))) return (ISC_FALSE); if (!ec1->ontoken && !ec2->ontoken) diff --git a/lib/dns/pkcs11gost_link.c b/lib/dns/pkcs11gost_link.c index 1816d734de..ff728005f1 100644 --- a/lib/dns/pkcs11gost_link.c +++ b/lib/dns/pkcs11gost_link.c @@ -14,8 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - #include #if defined(PKCS11CRYPTO) && defined(HAVE_PKCS11_GOST) @@ -444,7 +442,8 @@ pkcs11gost_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(gost1, CKA_VALUE2); @@ -452,7 +451,8 @@ pkcs11gost_compare(const dst_key_t *key1, const dst_key_t *key2) { if (((attr1 != NULL) || (attr2 != NULL)) && ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen))) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen))) return (ISC_FALSE); if (!gost1->ontoken && !gost2->ontoken) @@ -856,7 +856,7 @@ pkcs11gost_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { buf[36] += adj; buf[38] += adj; } - if (memcmp(priv.elements[0].data, buf, 39) != 0) + if (!isc_safe_memequal(priv.elements[0].data, buf, 39)) DST_RET(DST_R_INVALIDPRIVATEKEY); priv.elements[0].tag = TAG_GOST_PRIVRAW; priv.elements[0].length -= 39; diff --git a/lib/dns/pkcs11rsa_link.c b/lib/dns/pkcs11rsa_link.c index bca1392ab8..edcddbfcc1 100644 --- a/lib/dns/pkcs11rsa_link.c +++ b/lib/dns/pkcs11rsa_link.c @@ -506,7 +506,8 @@ pkcs11rsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(rsa1, CKA_PUBLIC_EXPONENT); @@ -515,7 +516,8 @@ pkcs11rsa_compare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_TRUE); else if ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen)) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen)) return (ISC_FALSE); attr1 = pk11_attribute_bytype(rsa1, CKA_PRIVATE_EXPONENT); @@ -523,7 +525,8 @@ pkcs11rsa_compare(const dst_key_t *key1, const dst_key_t *key2) { if (((attr1 != NULL) || (attr2 != NULL)) && ((attr1 == NULL) || (attr2 == NULL) || (attr1->ulValueLen != attr2->ulValueLen) || - memcmp(attr1->pValue, attr2->pValue, attr1->ulValueLen))) + !isc_safe_memequal(attr1->pValue, attr2->pValue, + attr1->ulValueLen))) return (ISC_FALSE); if (!rsa1->ontoken && !rsa2->ontoken) @@ -1179,7 +1182,7 @@ rsa_check(pk11_object_t *rsa, pk11_object_t *pubrsa) { if (priv_exp != NULL) { if (priv_explen != pub_explen) return (DST_R_INVALIDPRIVATEKEY); - if (memcmp(priv_exp, pub_exp, pub_explen) != 0) + if (!isc_safe_memequal(priv_exp, pub_exp, pub_explen)) return (DST_R_INVALIDPRIVATEKEY); } else { privattr->pValue = pub_exp; @@ -1204,7 +1207,7 @@ rsa_check(pk11_object_t *rsa, pk11_object_t *pubrsa) { if (priv_mod != NULL) { if (priv_modlen != pub_modlen) return (DST_R_INVALIDPRIVATEKEY); - if (memcmp(priv_mod, pub_mod, pub_modlen) != 0) + if (!isc_safe_memequal(priv_mod, pub_mod, pub_modlen)) return (DST_R_INVALIDPRIVATEKEY); } else { privattr->pValue = pub_mod; diff --git a/lib/dns/spnego.c b/lib/dns/spnego.c index 6ae1123018..0de332fc6a 100644 --- a/lib/dns/spnego.c +++ b/lib/dns/spnego.c @@ -14,8 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - /*! \file * \brief * Portable SPNEGO implementation. @@ -146,6 +144,7 @@ #include #include #include +#include #include #include #include @@ -395,7 +394,7 @@ cmp_gss_type(gss_buffer_t token, gss_OID gssoid) if (((OM_uint32) *p++) != gssoid->length) return (GSS_S_DEFECTIVE_TOKEN); - return (memcmp(p, gssoid->elements, gssoid->length)); + return (isc_safe_memcompare(p, gssoid->elements, gssoid->length)); } /* accept_sec_context.c */ @@ -635,16 +634,18 @@ gss_accept_sec_context_spnego(OM_uint32 *minor_status, return (GSS_S_DEFECTIVE_TOKEN); } if (mech_len == GSS_KRB5_MECH->length && - memcmp(GSS_KRB5_MECH->elements, - mechbuf + sizeof(mechbuf) - mech_len, - mech_len) == 0) { + isc_safe_memequal(GSS_KRB5_MECH->elements, + mechbuf + sizeof(mechbuf) - mech_len, + mech_len)) + { found = 1; break; } if (mech_len == GSS_MSKRB5_MECH->length && - memcmp(GSS_MSKRB5_MECH->elements, - mechbuf + sizeof(mechbuf) - mech_len, - mech_len) == 0) { + isc_safe_memequal(GSS_MSKRB5_MECH->elements, + mechbuf + sizeof(mechbuf) - mech_len, + mech_len)) + { found = 1; if (i == 0) pref = GSS_MSKRB5_MECH; @@ -715,7 +716,7 @@ gssapi_verify_mech_header(u_char ** str, p += foo; if (mech_len != mech->length) return (GSS_S_BAD_MECH); - if (memcmp(p, mech->elements, mech->length) != 0) + if (!isc_safe_memequal(p, mech->elements, mech->length)) return (GSS_S_BAD_MECH); p += mech_len; *str = p; @@ -1668,7 +1669,7 @@ spnego_reply(OM_uint32 *minor_status, buf = input_token->value; buf_size = input_token->length; } else if ((size_t)mech_len == GSS_KRB5_MECH->length && - memcmp(GSS_KRB5_MECH->elements, p, mech_len) == 0) + isc_safe_memequal(GSS_KRB5_MECH->elements, p, mech_len)) return (gss_init_sec_context(minor_status, initiator_cred_handle, context_handle, @@ -1683,7 +1684,7 @@ spnego_reply(OM_uint32 *minor_status, ret_flags, time_rec)); else if ((size_t)mech_len == GSS_SPNEGO_MECH->length && - memcmp(GSS_SPNEGO_MECH->elements, p, mech_len) == 0) { + isc_safe_memequal(GSS_SPNEGO_MECH->elements, p, mech_len)) { ret = gssapi_spnego_decapsulate(minor_status, input_token, &buf, @@ -1721,9 +1722,9 @@ spnego_reply(OM_uint32 *minor_status, resp.supportedMech, &oidlen); if (ret || oidlen != GSS_KRB5_MECH->length || - memcmp(oidbuf + sizeof(oidbuf) - oidlen, - GSS_KRB5_MECH->elements, - oidlen) != 0) { + !isc_safe_memequal(oidbuf + sizeof(oidbuf) - oidlen, + GSS_KRB5_MECH->elements, oidlen)) + { free_NegTokenResp(&resp); return GSS_S_BAD_MECH; } diff --git a/lib/isc/hmacmd5.c b/lib/isc/hmacmd5.c index 5f77323e6c..f2a0b11d9d 100644 --- a/lib/isc/hmacmd5.c +++ b/lib/isc/hmacmd5.c @@ -326,5 +326,5 @@ isc_hmacmd5_verify2(isc_hmacmd5_t *ctx, unsigned char *digest, size_t len) { REQUIRE(len <= ISC_MD5_DIGESTLENGTH); isc_hmacmd5_sign(ctx, newdigest); - return (isc_safe_memcmp(digest, newdigest, len)); + return (isc_safe_memequal(digest, newdigest, len)); } diff --git a/lib/isc/hmacsha.c b/lib/isc/hmacsha.c index c03a27ac77..db42cda295 100644 --- a/lib/isc/hmacsha.c +++ b/lib/isc/hmacsha.c @@ -978,7 +978,7 @@ isc_hmacsha1_verify(isc_hmacsha1_t *ctx, unsigned char *digest, size_t len) { REQUIRE(len <= ISC_SHA1_DIGESTLENGTH); isc_hmacsha1_sign(ctx, newdigest, ISC_SHA1_DIGESTLENGTH); - return (isc_safe_memcmp(digest, newdigest, len)); + return (isc_safe_memequal(digest, newdigest, len)); } /* @@ -991,7 +991,7 @@ isc_hmacsha224_verify(isc_hmacsha224_t *ctx, unsigned char *digest, size_t len) REQUIRE(len <= ISC_SHA224_DIGESTLENGTH); isc_hmacsha224_sign(ctx, newdigest, ISC_SHA224_DIGESTLENGTH); - return (isc_safe_memcmp(digest, newdigest, len)); + return (isc_safe_memequal(digest, newdigest, len)); } /* @@ -1004,7 +1004,7 @@ isc_hmacsha256_verify(isc_hmacsha256_t *ctx, unsigned char *digest, size_t len) REQUIRE(len <= ISC_SHA256_DIGESTLENGTH); isc_hmacsha256_sign(ctx, newdigest, ISC_SHA256_DIGESTLENGTH); - return (isc_safe_memcmp(digest, newdigest, len)); + return (isc_safe_memequal(digest, newdigest, len)); } /* @@ -1017,7 +1017,7 @@ isc_hmacsha384_verify(isc_hmacsha384_t *ctx, unsigned char *digest, size_t len) REQUIRE(len <= ISC_SHA384_DIGESTLENGTH); isc_hmacsha384_sign(ctx, newdigest, ISC_SHA384_DIGESTLENGTH); - return (isc_safe_memcmp(digest, newdigest, len)); + return (isc_safe_memequal(digest, newdigest, len)); } /* @@ -1030,5 +1030,5 @@ isc_hmacsha512_verify(isc_hmacsha512_t *ctx, unsigned char *digest, size_t len) REQUIRE(len <= ISC_SHA512_DIGESTLENGTH); isc_hmacsha512_sign(ctx, newdigest, ISC_SHA512_DIGESTLENGTH); - return (isc_safe_memcmp(digest, newdigest, len)); + return (isc_safe_memequal(digest, newdigest, len)); } diff --git a/lib/isc/include/isc/safe.h b/lib/isc/include/isc/safe.h index 89d56def73..b2495e962f 100644 --- a/lib/isc/include/isc/safe.h +++ b/lib/isc/include/isc/safe.h @@ -26,9 +26,17 @@ ISC_LANG_BEGINDECLS isc_boolean_t -isc_safe_memcmp(const void *s1, const void *s2, size_t n); +isc_safe_memequal(const void *s1, const void *s2, size_t n); /*%< - * Clone of libc memcmp() safe to differential timing attacks. + * Returns ISC_TRUE iff. two blocks of memory are equal, otherwise + * ISC_FALSE. + * + */ + +int +isc_safe_memcompare(const void *b1, const void *b2, size_t len); +/*%< + * Clone of libc memcmp() which is safe to differential timing attacks. */ ISC_LANG_ENDDECLS diff --git a/lib/isc/safe.c b/lib/isc/safe.c index fd27687188..a6d010ff64 100644 --- a/lib/isc/safe.c +++ b/lib/isc/safe.c @@ -1,5 +1,6 @@ /* - * Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") + * Copyright (C) 2013, 2015 Internet Systems Consortium, Inc. ("ISC") + * Copyright (c) 2014 Google Inc. * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -14,8 +15,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - /*! \file */ #include @@ -28,7 +27,7 @@ #endif isc_boolean_t -isc_safe_memcmp(const void *s1, const void *s2, size_t n) { +isc_safe_memequal(const void *s1, const void *s2, size_t n) { isc_uint8_t acc = 0; if (n != 0U) { @@ -40,3 +39,30 @@ isc_safe_memcmp(const void *s1, const void *s2, size_t n) { } return (ISC_TF(acc == 0)); } + + +int +isc_safe_memcompare(const void *b1, const void *b2, size_t len) { + const unsigned char *p1 = b1, *p2 = b2; + size_t i; + int res = 0, done = 0; + + for (i = 0; i < len; i++) { + /* lt is -1 if p1[i] < p2[i]; else 0. */ + int lt = (p1[i] - p2[i]) >> CHAR_BIT; + + /* gt is -1 if p1[i] > p2[i]; else 0. */ + int gt = (p2[i] - p1[i]) >> CHAR_BIT; + + /* cmp is 1 if p1[i] > p2[i]; -1 if p1[i] < p2[i]; else 0. */ + int cmp = lt - gt; + + /* set res = cmp if !done. */ + res |= cmp & ~done; + + /* set done if p1[i] != p2[i]. */ + done |= lt | gt; + } + + return (res); +} diff --git a/lib/isc/tests/safe_test.c b/lib/isc/tests/safe_test.c index 7b7ac39926..113ec8efc5 100644 --- a/lib/isc/tests/safe_test.c +++ b/lib/isc/tests/safe_test.c @@ -28,25 +28,46 @@ #include #include -ATF_TC(isc_safe_memcmp); -ATF_TC_HEAD(isc_safe_memcmp, tc) { - atf_tc_set_md_var(tc, "descr", "safe memcmp()"); +ATF_TC(isc_safe_memequal); +ATF_TC_HEAD(isc_safe_memequal, tc) { + atf_tc_set_md_var(tc, "descr", "safe memequal()"); } -ATF_TC_BODY(isc_safe_memcmp, tc) { +ATF_TC_BODY(isc_safe_memequal, tc) { UNUSED(tc); - ATF_CHECK(isc_safe_memcmp("test", "test", 4)); - ATF_CHECK(!isc_safe_memcmp("test", "tesc", 4)); - ATF_CHECK(isc_safe_memcmp("\x00\x00\x00\x00", "\x00\x00\x00\x00", 4)); - ATF_CHECK(!isc_safe_memcmp("\x00\x00\x00\x00", "\x00\x00\x00\x01", 4)); - ATF_CHECK(!isc_safe_memcmp("\x00\x00\x00\x02", "\x00\x00\x00\x00", 4)); + ATF_CHECK(isc_safe_memequal("test", "test", 4)); + ATF_CHECK(!isc_safe_memequal("test", "tesc", 4)); + ATF_CHECK(isc_safe_memequal("\x00\x00\x00\x00", + "\x00\x00\x00\x00", 4)); + ATF_CHECK(!isc_safe_memequal("\x00\x00\x00\x00", + "\x00\x00\x00\x01", 4)); + ATF_CHECK(!isc_safe_memequal("\x00\x00\x00\x02", + "\x00\x00\x00\x00", 4)); +} + +ATF_TC(isc_safe_memcompare); +ATF_TC_HEAD(isc_safe_memcompare, tc) { + atf_tc_set_md_var(tc, "descr", "safe memcompare()"); +} +ATF_TC_BODY(isc_safe_memcompare, tc) { + UNUSED(tc); + + ATF_CHECK(isc_safe_memcompare("test", "test", 4) == 0); + ATF_CHECK(isc_safe_memcompare("test", "tesc", 4) > 0); + ATF_CHECK(isc_safe_memcompare("test", "tesy", 4) < 0); + ATF_CHECK(isc_safe_memcompare("\x00\x00\x00\x00", + "\x00\x00\x00\x00", 4) == 0); + ATF_CHECK(isc_safe_memcompare("\x00\x00\x00\x00", + "\x00\x00\x00\x01", 4) < 0); + ATF_CHECK(isc_safe_memcompare("\x00\x00\x00\x02", + "\x00\x00\x00\x00", 4) > 0); } /* * Main */ ATF_TP_ADD_TCS(tp) { - ATF_TP_ADD_TC(tp, isc_safe_memcmp); + ATF_TP_ADD_TC(tp, isc_safe_memequal); + ATF_TP_ADD_TC(tp, isc_safe_memcompare); return (atf_no_error()); } - diff --git a/lib/isccc/cc.c b/lib/isccc/cc.c index 400108e3a4..83027643d5 100644 --- a/lib/isccc/cc.c +++ b/lib/isccc/cc.c @@ -509,7 +509,7 @@ verify(isccc_sexpr_t *alist, unsigned char *data, unsigned int length, unsigned char *value; value = (unsigned char *) isccc_sexpr_tostring(hmac); - if (!isc_safe_memcmp(value, digestb64, HMD5_LENGTH)) + if (!isc_safe_memequal(value, digestb64, HMD5_LENGTH)) return (ISCCC_R_BADAUTH); } else { unsigned char *value; @@ -518,7 +518,7 @@ verify(isccc_sexpr_t *alist, unsigned char *data, unsigned int length, value = (unsigned char *) isccc_sexpr_tostring(hmac); GET8(valalg, value); if ((valalg != algorithm) || - (!isc_safe_memcmp(value, digestb64, HSHA_LENGTH))) + !isc_safe_memequal(value, digestb64, HSHA_LENGTH)) return (ISCCC_R_BADAUTH); }