fix system library error messages

previously, we'd log a lot of "unknown" error messages. this should fix
that.

Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
Signed-off-by: Ted Poole <tpoole@redhat.com>

Author: dgn

bssl-compat/patch/include/openssl/err.h.sh

@@ -28,4 +28,4 @@ uncomment.sh "$1" --comment -h \
   --uncomment-macro OPENSSL_PUT_ERROR \
   --uncomment-func-decl ERR_put_error \
   --uncomment-macro-redef ERR_NUM_ERRORS \
-  --sed '/#define ERR_PACK/i#define ERR_PACK(lib, reason) ossl_ERR_PACK(lib, 0, reason)'
+  --sed '/#define ERR_PACK/i#define ERR_PACK(lib, reason) (lib == ossl_ERR_LIB_SYS) ? (ossl_ERR_SYSTEM_FLAG | reason) : ossl_ERR_PACK(lib, 0, reason)'

bssl-compat/source/err.cc

@@ -2,6 +2,7 @@
 #include <ossl.h>
 #include <map>
 #include <string>
+#include <cstring>
 
 
 uint64_t o2b(uint64_t e) {
@@ -71,6 +72,12 @@ extern "C" uint32_t ERR_get_error(void) {
 
 
 extern "C" const char *ERR_lib_error_string(uint32_t packed_error) {
+  // Handle system library errors like BoringSSL does
+  // OpenSSL 3.x returns NULL for system errors, but BoringSSL returns "system library"
+  if (ossl_ERR_SYSTEM_ERROR(packed_error)) {
+    return "system library";
+  }
+
   const char *ret = ossl.ossl_ERR_lib_error_string(b2o(packed_error));
   return (ret ? ret : "unknown library");
 }
@@ -92,6 +99,21 @@ extern "C" uint32_t ERR_peek_last_error(void) {
 
 
 extern "C" const char *ERR_reason_error_string(uint32_t packed_error) {
+  // Handle system library errors like BoringSSL does
+  // For system errors, return strerror(errno) like BoringSSL does
+  if (ossl_ERR_SYSTEM_ERROR(packed_error)) {
+    uint32_t reason = ossl_ERR_GET_REASON(packed_error);
+    // For some reason BoringSSL only calls strerror() for errno < 127, and just
+    // returns the string "unknown error" for errno >= 127. This excludes some
+    // valid errno values (on Linux at least). We delibrately differ from
+    // BoringSSL here, and up the limit to 256 to ensure we don't hide any valid
+    // error strings.
+    if (reason > 0 && reason < 256) {
+      return strerror(reason);
+    }
+    return "unknown error";
+  }
+
   /**
    * This is not an exhaustive list of errors; rather it is just the ones that
    * need to be translated for the Envoy tests to pass (yes some of the tests do

bssl-compat/test/test_err.cc

@@ -2,6 +2,8 @@
 #include <openssl/err.h>
 #include <openssl/ssl.h>
 #include <limits>
+#include <cstring>
+#include <cerrno>
 
 
 TEST(ErrTest, test_ERR_func_error_string) {
@@ -50,3 +52,140 @@ TEST(ErrTest, test_SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM) {
   EXPECT_STREQ("NO_COMMON_SIGNATURE_ALGORITHMS", ERR_reason_error_string(e));
   EXPECT_STREQ("error:100000fd:SSL routines:OPENSSL_internal:NO_COMMON_SIGNATURE_ALGORITHMS", ERR_error_string_n(e, buf, sizeof(buf)));
 }
+
+
+/**
+ * OpenSSL and BoringSSL pack errors (consisting of library & reason codes) into
+ * a 32 bit integer using similar mechanisms, but with slightly different mask
+ * and shift values, giving different values between the libraries. This is
+ * mostly hidden by the use of macros like ERR_PACK(library, reason),
+ * ERR_GET_LIB(error), ERR_GET_REASON(error), and library codes like
+ * ERR_LIB_SSL, ERR_LIB_X509 etc.
+ *
+ * However, OpenSSL and BoringSSL pack _system errors_ very differently....
+ *
+ * BoringSSL packs system errors the same as all its other errors, by passing
+ * ERR_LIB_SYS as the library code, and errno as the reason, to the
+ * ERR_PACK(lib,reason) macro i.e. ERR_PACK(ERR_LIB_SYS, errno).
+ *
+ * OpenSSL _does not_ pack system errors with ERR_PACK(ERR_LIB_SYS, errno) as
+ * one might expect. Instead, it uses a different special case mechanism of
+ * OR-ing the special ERR_SYSTEM_FLAG (0x80000000) with the errno value i.e.
+ * (ERR_SYSTEM_FLAG | errno).
+ *
+ * For example, to represent a EPIPE system error (0x20 on Linux):
+ * - BoringSSL : ERR_PACK(ERR_LIB_SYS,EPIPE)) = 0x02000020
+ * - OpenSSL   : (ERR_SYSTEM_FLAG | EPIPE) = 0x80000020
+ *
+ * Since bssl-compat is trying to emulate BoringSSL's behavior, we need to
+ * ensure that packing system errors using ERR_PACK() appears to work the same
+ * way as BoringSSL (even though the resulting numeric values will be
+ * different).
+ */
+TEST(ErrTest, test_ERR_PACK_system_error) {
+  uint32_t packed = ERR_PACK(ERR_LIB_SYS, EPIPE);
+
+  EXPECT_EQ(EPIPE, 0x20); // Sanity check
+
+#ifdef BSSL_COMPAT
+  EXPECT_EQ(ossl_ERR_SYSTEM_FLAG, 0x80000000); // Sanity check
+  // These checks will fail if we haven't modified bssl-compat's definition of
+  // ERR_PACK(lib,reason) to handle OpenSSL's special case for system errors.
+  EXPECT_EQ(packed, ossl_ERR_SYSTEM_FLAG | EPIPE);
+  EXPECT_EQ(packed, 0x80000020);
+#else
+  EXPECT_EQ(ERR_LIB_SYS, 2); // Sanity check
+  EXPECT_EQ(packed, 0x02000020);
+#endif
+
+  // These should work idenically on BoringSSL and OpenSSL
+  EXPECT_EQ(ERR_GET_LIB(packed), ERR_LIB_SYS);
+  EXPECT_EQ(ERR_GET_REASON(packed), EPIPE);
+}
+
+
+TEST(ErrTest, test_system_error_ECONNRESET) {
+  uint32_t err = ERR_PACK(ERR_LIB_SYS, ECONNRESET);
+
+  ASSERT_EQ(ECONNRESET, 0x68); // Sanity check
+
+#ifdef BSSL_COMPAT
+  EXPECT_EQ(err, 0x80000068); // OpenSSL packed value
+#else
+  EXPECT_EQ(err, 0x02000068); // BoringSSL packed value
+#endif
+
+  EXPECT_EQ(ERR_LIB_SYS, ERR_GET_LIB(err));
+  EXPECT_EQ(ECONNRESET, ERR_GET_REASON(err));
+
+  const char *lib = ERR_lib_error_string(err);
+  ASSERT_NE(nullptr, lib);
+  EXPECT_STREQ("system library", lib);
+
+  const char *reason = ERR_reason_error_string(err);
+  ASSERT_NE(nullptr, reason);
+  EXPECT_STREQ(strerror(ECONNRESET), reason);
+}
+
+
+TEST(ErrTest, test_system_error_EPIPE) {
+  uint32_t err = ERR_PACK(ERR_LIB_SYS, EPIPE);
+
+  ASSERT_EQ(EPIPE, 0x20); // Sanity check
+
+#ifdef BSSL_COMPAT
+  EXPECT_EQ(err, 0x80000020); // OpenSSL packed value
+#else
+  EXPECT_EQ(err, 0x02000020); // BoringSSL packed value
+#endif
+
+  EXPECT_EQ(ERR_LIB_SYS, ERR_GET_LIB(err));
+  EXPECT_EQ(EPIPE, ERR_GET_REASON(err));
+
+  const char *lib = ERR_lib_error_string(err);
+  ASSERT_NE(nullptr, lib);
+  EXPECT_STREQ("system library", lib);
+
+  const char *reason = ERR_reason_error_string(err);
+  ASSERT_NE(nullptr, reason);
+  EXPECT_STREQ(strerror(EPIPE), reason);
+}
+
+
+TEST(ErrTest, test_system_error_ETIMEDOUT) {
+  uint32_t err = ERR_PACK(ERR_LIB_SYS, ETIMEDOUT);
+
+  ASSERT_EQ(ETIMEDOUT, 0x6E); // Sanity check
+
+#ifdef BSSL_COMPAT
+  EXPECT_EQ(err, 0x8000006E); // OpenSSL packed value
+#else
+  EXPECT_EQ(err, 0x0200006E); // BoringSSL packed value
+#endif
+
+  EXPECT_EQ(ERR_LIB_SYS, ERR_GET_LIB(err));
+  EXPECT_EQ(ETIMEDOUT, ERR_GET_REASON(err));
+
+  const char *lib = ERR_lib_error_string(err);
+  ASSERT_NE(nullptr, lib);
+  EXPECT_STREQ("system library", lib);
+
+  const char *reason = ERR_reason_error_string(err);
+  ASSERT_NE(nullptr, reason);
+  EXPECT_STREQ(strerror(ETIMEDOUT), reason);
+}
+
+
+TEST(ErrTest, test_system_error_invalid_errno) {
+  uint32_t err = ERR_PACK(ERR_LIB_SYS, 0xFFF);
+
+  EXPECT_EQ(ERR_LIB_SYS, ERR_GET_LIB(err));
+
+  const char *lib = ERR_lib_error_string(err);
+  ASSERT_NE(nullptr, lib);
+  EXPECT_STREQ("system library", lib);
+
+  const char *reason = ERR_reason_error_string(err);
+  ASSERT_NE(nullptr, reason);
+  EXPECT_STREQ("unknown error", reason);
+}