Add an ERR_equals function
Checking for an error on the error queue is very tedious. You have to
check both library and reason code separately, which often means you
need to save the error code to a local variable.
Make an ERR_equals function. I've gone ahead and just made it public API
because even within BoringSSL, it seems to come up everywhere.
Change-Id: Ia963c9ae743ac5c6939846f8f64bbc98b495ce0b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81627
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/cipher/aead_test.cc b/crypto/cipher/aead_test.cc
index 5eb0c39..5aa22a8 100644
--- a/crypto/cipher/aead_test.cc
+++ b/crypto/cipher/aead_test.cc
@@ -386,13 +386,10 @@
out.size(), out_tag.data(), out_tag.size(), ad.data(), ad.size());
// Skip decryption for AEADs that don't implement open_gather().
- if (!ret) {
- uint32_t err = ERR_peek_error();
- if (ERR_GET_LIB(err) == ERR_LIB_CIPHER &&
- ERR_GET_REASON(err) == CIPHER_R_CTRL_NOT_IMPLEMENTED) {
- t->SkipCurrent();
- return;
- }
+ if (!ret && ERR_equals(ERR_peek_error(), ERR_LIB_CIPHER,
+ CIPHER_R_CTRL_NOT_IMPLEMENTED)) {
+ t->SkipCurrent();
+ return;
}
if (t->HasAttribute("FAILS")) {
diff --git a/crypto/fipsmodule/ec/oct.cc.inc b/crypto/fipsmodule/ec/oct.cc.inc
index 69323dc..ac34f5e 100644
--- a/crypto/fipsmodule/ec/oct.cc.inc
+++ b/crypto/fipsmodule/ec/oct.cc.inc
@@ -262,9 +262,7 @@
}
if (!BN_mod_sqrt(y, tmp1, field, ctx)) {
- uint32_t err = ERR_peek_last_error();
- if (ERR_GET_LIB(err) == ERR_LIB_BN &&
- ERR_GET_REASON(err) == BN_R_NOT_A_SQUARE) {
+ if (ERR_equals(ERR_peek_last_error(), ERR_LIB_BN, BN_R_NOT_A_SQUARE)) {
ERR_clear_error();
OPENSSL_PUT_ERROR(EC, EC_R_INVALID_COMPRESSED_POINT);
} else {
diff --git a/crypto/fipsmodule/rsa/rsa_impl.cc.inc b/crypto/fipsmodule/rsa/rsa_impl.cc.inc
index d4c9bf8..3fad8e0 100644
--- a/crypto/fipsmodule/rsa/rsa_impl.cc.inc
+++ b/crypto/fipsmodule/rsa/rsa_impl.cc.inc
@@ -1183,9 +1183,7 @@
return 0;
}
- RSA *tmp = NULL;
- uint32_t err;
- int ret = 0;
+ bssl::UniquePtr<RSA> tmp;
// |rsa_generate_key_impl|'s 2^-20 failure probability is too high at scale,
// so we run the FIPS algorithm four times, bringing it down to 2^-80. We
@@ -1195,27 +1193,25 @@
do {
ERR_clear_error();
// Generate into scratch space, to avoid leaving partial work on failure.
- tmp = RSA_new();
- if (tmp == NULL) {
- goto out;
+ tmp.reset(RSA_new());
+ if (tmp == nullptr) {
+ return 0;
}
- if (rsa_generate_key_impl(tmp, bits, e_value, cb)) {
+ if (rsa_generate_key_impl(tmp.get(), bits, e_value, cb)) {
break;
}
- err = ERR_peek_error();
- RSA_free(tmp);
- tmp = NULL;
+ tmp = nullptr;
failures++;
// Only retry on |RSA_R_TOO_MANY_ITERATIONS|. This is so a caller-induced
// failure in |BN_GENCB_call| is still fatal.
- } while (failures < 4 && ERR_GET_LIB(err) == ERR_LIB_RSA &&
- ERR_GET_REASON(err) == RSA_R_TOO_MANY_ITERATIONS);
+ } while (failures < 4 && ERR_equals(ERR_peek_error(), ERR_LIB_RSA,
+ RSA_R_TOO_MANY_ITERATIONS));
- if (tmp == NULL || (check_fips && !RSA_check_fips(tmp))) {
- goto out;
+ if (tmp == nullptr || (check_fips && !RSA_check_fips(tmp.get()))) {
+ return 0;
}
rsa_invalidate_key(rsa);
@@ -1235,11 +1231,7 @@
replace_bignum(&rsa->dmq1_fixed, &tmp->dmq1_fixed);
replace_bignum(&rsa->iqmp_mont, &tmp->iqmp_mont);
rsa->private_key_frozen = tmp->private_key_frozen;
- ret = 1;
-
-out:
- RSA_free(tmp);
- return ret;
+ return 1;
}
int RSA_generate_key_ex(RSA *rsa, int bits, const BIGNUM *e_value,
diff --git a/crypto/pem/pem_info.cc b/crypto/pem/pem_info.cc
index edc97f4..acf035c 100644
--- a/crypto/pem/pem_info.cc
+++ b/crypto/pem/pem_info.cc
@@ -143,9 +143,7 @@
for (;;) {
if (!PEM_read_bio(bp, &name, &header, &data, &len)) {
- uint32_t error = ERR_peek_last_error();
- if (ERR_GET_LIB(error) == ERR_LIB_PEM &&
- ERR_GET_REASON(error) == PEM_R_NO_START_LINE) {
+ if (ERR_equals(ERR_peek_last_error(), ERR_LIB_PEM, PEM_R_NO_START_LINE)) {
ERR_clear_error();
break;
}
diff --git a/crypto/pem/pem_lib.cc b/crypto/pem/pem_lib.cc
index e7c4e0b..d8872fd 100644
--- a/crypto/pem/pem_lib.cc
+++ b/crypto/pem/pem_lib.cc
@@ -182,9 +182,7 @@
for (;;) {
if (!PEM_read_bio(bp, &nm, &header, &data, &len)) {
- uint32_t error = ERR_peek_error();
- if (ERR_GET_LIB(error) == ERR_LIB_PEM &&
- ERR_GET_REASON(error) == PEM_R_NO_START_LINE) {
+ if (ERR_equals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_NO_START_LINE)) {
ERR_add_error_data(2, "Expecting: ", name);
}
return 0;
diff --git a/crypto/test/test_util.cc b/crypto/test/test_util.cc
index df3c9ed..75a9820 100644
--- a/crypto/test/test_util.cc
+++ b/crypto/test/test_util.cc
@@ -71,7 +71,7 @@
}
testing::AssertionResult ErrorEquals(uint32_t err, int lib, int reason) {
- if (ERR_GET_LIB(err) == lib && ERR_GET_REASON(err) == reason) {
+ if (ERR_equals(err, lib, reason)) {
return testing::AssertionSuccess();
}
diff --git a/crypto/x509/by_file.cc b/crypto/x509/by_file.cc
index d9e0778..511f488 100644
--- a/crypto/x509/by_file.cc
+++ b/crypto/x509/by_file.cc
@@ -70,9 +70,9 @@
for (;;) {
x = PEM_read_bio_X509_AUX(in, NULL, NULL, NULL);
if (x == NULL) {
- uint32_t error = ERR_peek_last_error();
- if (ERR_GET_LIB(error) == ERR_LIB_PEM &&
- ERR_GET_REASON(error) == PEM_R_NO_START_LINE && count > 0) {
+ if (ERR_equals(ERR_peek_last_error(), ERR_LIB_PEM,
+ PEM_R_NO_START_LINE) &&
+ count > 0) {
ERR_clear_error();
break;
}
@@ -131,9 +131,9 @@
for (;;) {
x = PEM_read_bio_X509_CRL(in, NULL, NULL, NULL);
if (x == NULL) {
- uint32_t error = ERR_peek_last_error();
- if (ERR_GET_LIB(error) == ERR_LIB_PEM &&
- ERR_GET_REASON(error) == PEM_R_NO_START_LINE && count > 0) {
+ if (ERR_equals(ERR_peek_last_error(), ERR_LIB_PEM,
+ PEM_R_NO_START_LINE) &&
+ count > 0) {
ERR_clear_error();
break;
}
diff --git a/include/openssl/err.h b/include/openssl/err.h
index 0263505..31a2b86 100644
--- a/include/openssl/err.h
+++ b/include/openssl/err.h
@@ -80,6 +80,13 @@
return (int)(packed_error & 0xfff);
}
+// ERR_equals returns one if |packed_error|'s library and reason code are |lib|
+// and |reason|, respectively, and zero otherwise.
+OPENSSL_INLINE int ERR_equals(uint32_t packed_error, int lib, int reason) {
+ return ERR_GET_LIB(packed_error) == lib &&
+ ERR_GET_REASON(packed_error) == reason;
+}
+
// ERR_get_error gets the packed error code for the least recent error and
// removes that error from the queue. If there are no errors in the queue then
// it returns zero.
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 1b1a9b5..f63f0d2 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -548,9 +548,8 @@
}
if (ret == ssl_open_record_error &&
hs->wait == ssl_hs_read_server_hello) {
- uint32_t err = ERR_peek_error();
- if (ERR_GET_LIB(err) == ERR_LIB_SSL &&
- ERR_GET_REASON(err) == SSL_R_SSLV3_ALERT_HANDSHAKE_FAILURE) {
+ if (ERR_equals(ERR_peek_error(), ERR_LIB_SSL,
+ SSL_R_SSLV3_ALERT_HANDSHAKE_FAILURE)) {
// Add a dedicated error code to the queue for a handshake_failure
// alert in response to ClientHello. This matches NSS's client
// behavior and gives a better error on a (probable) failure to
diff --git a/ssl/ssl_file.cc b/ssl/ssl_file.cc
index c284d90..5360566 100644
--- a/ssl/ssl_file.cc
+++ b/ssl/ssl_file.cc
@@ -343,9 +343,7 @@
}
// When the while loop ends, it's usually just EOF.
- uint32_t err = ERR_peek_last_error();
- if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
- ERR_GET_REASON(err) == PEM_R_NO_START_LINE) {
+ if (ERR_equals(ERR_peek_last_error(), ERR_LIB_PEM, PEM_R_NO_START_LINE)) {
ERR_clear_error();
return 1;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 975b14a..4c1bd55 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -4447,8 +4447,8 @@
static bool ExpectSingleError(int lib, int reason) {
const char *expected = ERR_reason_error_string(ERR_PACK(lib, reason));
- int err = ERR_get_error();
- if (ERR_GET_LIB(err) != lib || ERR_GET_REASON(err) != reason) {
+ uint32_t err = ERR_get_error();
+ if (!ERR_equals(err, lib, reason)) {
char buf[ERR_ERROR_STRING_BUF_LEN];
ERR_error_string_n(err, buf, sizeof(buf));
fprintf(stderr, "Wanted %s, got: %s.\n", expected, buf);
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index c0a30f8..ff93490 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -922,10 +922,8 @@
fprintf(stderr, "Expected ECH rejection, but connection succeeded.\n");
return false;
}
- uint32_t err = ERR_peek_error();
if (SSL_get_error(ssl.get(), -1) != SSL_ERROR_SSL ||
- ERR_GET_LIB(err) != ERR_LIB_SSL ||
- ERR_GET_REASON(err) != SSL_R_ECH_REJECTED) {
+ !ERR_equals(ERR_peek_error(), ERR_LIB_SSL, SSL_R_ECH_REJECTED)) {
fprintf(stderr, "Expected ECH rejection, but connection succeeded.\n");
return false;
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 7312d74..f973d9e 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -1173,9 +1173,7 @@
}
}
- uint32_t err = ERR_peek_last_error();
- if (ERR_GET_LIB(err) != ERR_LIB_PEM ||
- ERR_GET_REASON(err) != PEM_R_NO_START_LINE) {
+ if (!ERR_equals(ERR_peek_last_error(), ERR_LIB_PEM, PEM_R_NO_START_LINE)) {
return false;
}
