Skip to content

Commit c8eb92a

Browse files
Make OPENSSL_memcmp constant-time while preserving ordering
Rewrite OPENSSL_memcmp to iterate the full length and capture the lowest-index differing byte pair via constant_time_select, so it returns memcmp's negative/zero/positive ordering without leaking timing information about where bytes differ. Replace all OPENSSL_memcmp_ordered callers (which used plain memcmp) with the new OPENSSL_memcmp and remove the helper.
1 parent 3e81966 commit c8eb92a

7 files changed

Lines changed: 25 additions & 22 deletions

File tree

crypto/asn1/asn1_lib.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ int ASN1_STRING_cmp(const ASN1_STRING *a, const ASN1_STRING *b) {
350350
return 1;
351351
}
352352

353-
int ret = OPENSSL_memcmp_ordered(a->data, b->data, a_length);
353+
int ret = OPENSSL_memcmp(a->data, b->data, a_length);
354354
if (ret != 0) {
355355
return ret;
356356
}

crypto/asn1/tasn_enc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ static int der_cmp(const void *a, const void *b) {
370370
const DER_ENC *d1 = a, *d2 = b;
371371
int cmplen, i;
372372
cmplen = (d1->length < d2->length) ? d1->length : d2->length;
373-
i = OPENSSL_memcmp_ordered(d1->data, d2->data, cmplen);
373+
i = OPENSSL_memcmp(d1->data, d2->data, cmplen);
374374
if (i) {
375375
return i;
376376
}

crypto/bytestring/cbb.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ static int compare_set_of_element(const void *a_ptr, const void *b_ptr) {
655655
const CBS *a = a_ptr, *b = b_ptr;
656656
size_t a_len = CBS_len(a), b_len = CBS_len(b);
657657
size_t min_len = a_len < b_len ? a_len : b_len;
658-
int ret = OPENSSL_memcmp_ordered(CBS_data(a), CBS_data(b), min_len);
658+
int ret = OPENSSL_memcmp(CBS_data(a), CBS_data(b), min_len);
659659
if (ret != 0) {
660660
return ret;
661661
}

crypto/internal.h

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -803,8 +803,10 @@ static inline uint64_t CRYPTO_bswap8(uint64_t x) {
803803
// These wrapper functions behave the same as the corresponding C standard
804804
// functions, but behave as expected when passed NULL if the length is zero.
805805
//
806-
// |OPENSSL_memcmp| delegates to |CRYPTO_memcmp| to provide constant-time
807-
// comparison, preventing timing side-channel attacks.
806+
// |OPENSSL_memcmp| performs a constant-time comparison that preserves the
807+
// ordering semantics of |memcmp| (negative, zero, or positive). It iterates
808+
// over the full length regardless of where the first differing byte is, so it
809+
// can be used safely with secret data without leaking timing information.
808810

809811
// C++ defines |memchr| as a const-correct overload.
810812
#if defined(__cplusplus)
@@ -843,20 +845,21 @@ static inline void *OPENSSL_memchr(const void *s, int c, size_t n) {
843845
#endif // __cplusplus
844846

845847
static inline int OPENSSL_memcmp(const void *s1, const void *s2, size_t n) {
846-
return CRYPTO_memcmp(s1, s2, n);
847-
}
848-
849-
// OPENSSL_memcmp_ordered wraps memcmp with NULL-safe behavior for n == 0.
850-
// Unlike |OPENSSL_memcmp|, it preserves ordering semantics (negative, zero,
851-
// positive return values) and is NOT constant-time. Use this only for
852-
// comparisons of non-secret data that require ordering.
853-
static inline int OPENSSL_memcmp_ordered(const void *s1, const void *s2,
854-
size_t n) {
855-
if (n == 0) {
856-
return 0;
848+
const uint8_t *a = (const uint8_t *)s1;
849+
const uint8_t *b = (const uint8_t *)s2;
850+
// Walk from the end so the lowest-index differing byte (the one that decides
851+
// memcmp's sign) is the last write to |result|. Equal-byte iterations leave
852+
// |result| unchanged. The loop always runs |n| iterations, so timing depends
853+
// only on |n|.
854+
int result = 0;
855+
for (size_t i = n; i > 0; i--) {
856+
uint8_t ai = a[i - 1];
857+
uint8_t bi = b[i - 1];
858+
crypto_word_t diff_mask =
859+
~constant_time_is_zero_w((crypto_word_t)(ai ^ bi));
860+
result = constant_time_select_int(diff_mask, (int)ai - (int)bi, result);
857861
}
858-
859-
return memcmp(s1, s2, n);
862+
return result;
860863
}
861864

862865
static inline void *OPENSSL_memcpy(void *dst, const void *src, size_t n) {

crypto/obj/obj.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ int OBJ_cmp(const ASN1_OBJECT *a, const ASN1_OBJECT *b) {
115115
} else if (a->length > b->length) {
116116
return 1;
117117
}
118-
return OPENSSL_memcmp_ordered(a->data, b->data, a->length);
118+
return OPENSSL_memcmp(a->data, b->data, a->length);
119119
}
120120

121121
const uint8_t *OBJ_get0_data(const ASN1_OBJECT *obj) {

crypto/pool/pool.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ static int CRYPTO_BUFFER_cmp(const CRYPTO_BUFFER *a, const CRYPTO_BUFFER *b) {
2727
if (a->len != b->len) {
2828
return 1;
2929
}
30-
return OPENSSL_memcmp_ordered(a->data, b->data, a->len);
30+
return OPENSSL_memcmp(a->data, b->data, a->len);
3131
}
3232

3333
CRYPTO_BUFFER_POOL* CRYPTO_BUFFER_POOL_new(void) {

crypto/x509/x509_cmp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ int X509_cmp(const X509 *a, const X509 *b) {
8383
x509v3_cache_extensions((X509 *)a);
8484
x509v3_cache_extensions((X509 *)b);
8585

86-
return OPENSSL_memcmp_ordered(a->cert_hash, b->cert_hash, SHA256_DIGEST_LENGTH);
86+
return OPENSSL_memcmp(a->cert_hash, b->cert_hash, SHA256_DIGEST_LENGTH);
8787
}
8888

8989
int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b) {
@@ -111,7 +111,7 @@ int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b) {
111111
return ret;
112112
}
113113

114-
return OPENSSL_memcmp_ordered(a->canon_enc, b->canon_enc, a->canon_enclen);
114+
return OPENSSL_memcmp(a->canon_enc, b->canon_enc, a->canon_enclen);
115115
}
116116

117117
uint32_t X509_NAME_hash(X509_NAME *x) {

0 commit comments

Comments
 (0)