CDRIVER-6019 Improve error messages from Windows APIs (#2020) (#2022)

* add helper to convert Windows error to string: `mongoc_winerr_to_string`

* use SECURITY_STATUS for error message

Improves observed "Incorrect function" error with a more informative "The client and server cannot communicate, because they do not possess a common algorithm."

* use `bson_strerror_r` for consistency between Windows / non-Windows

Author: kevinAlbs

src/libmongoc/src/mongoc/mongoc-client-side-encryption.c

@@ -1378,24 +1378,14 @@ _do_spawn (const char *path, char **args, bson_error_t *error)
                         NULL /* current directory */,
                         &startup_info,
                         &process_information)) {
-      long lastError = GetLastError ();
-      LPSTR message = NULL;
-
-      FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ARGUMENT_ARRAY | FORMAT_MESSAGE_FROM_SYSTEM |
-                         FORMAT_MESSAGE_IGNORE_INSERTS,
-                      NULL,
-                      lastError,
-                      0,
-                      (LPSTR) &message,
-                      0,
-                      NULL);
+      char *message = mongoc_winerr_to_string (GetLastError ());
 
       _mongoc_set_error (error,
                          MONGOC_ERROR_CLIENT,
                          MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE,
                          "failed to spawn mongocryptd: %s",
                          message);
-      LocalFree (message);
+      bson_free (message);
       mcommon_string_from_append_destroy (&command);
       return false;
    }

src/libmongoc/src/mongoc/mongoc-client.c

@@ -202,13 +202,10 @@ _mongoc_get_rr_dnsapi (
    res = DnsQuery_UTF8 (hostname, nst, options, NULL /* IP Address */, &pdns, 0 /* reserved */);
 
    if (res) {
-      DWORD flags = FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
-
-      if (FormatMessage (flags, 0, res, MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR) &lpMsgBuf, 0, 0)) {
-         DNS_ERROR ("Failed to look up %s record \"%s\": %s", rr_type_name, hostname, (char *) lpMsgBuf);
-      }
-
-      DNS_ERROR ("Failed to look up %s record \"%s\": Unknown error", rr_type_name, hostname);
+      // Cast signed DNS_STATUS to unsigned DWORD. FormatMessage expects DWORD.
+      char *msg = mongoc_winerr_to_string ((DWORD) res);
+      DNS_ERROR ("Failed to look up %s record \"%s\": %s", rr_type_name, hostname, msg);
+      bson_free (msg);
    }
 
    if (!pdns) {

src/libmongoc/src/mongoc/mongoc-error-private.h

@@ -129,6 +129,12 @@ _mongoc_set_error_category (bson_error_t *error, uint8_t category)
    error->reserved = category;
 }
 
+#ifdef _WIN32
+// Call `mongoc_winerr_to_string` on a Windows error code (e.g. a return from GetLastError()).
+char *
+mongoc_winerr_to_string (DWORD err_code);
+#endif
+
 BSON_END_DECLS
 
 #endif /* MONGOC_ERROR_PRIVATE_H */

src/libmongoc/src/mongoc/mongoc-error.c

@@ -369,3 +369,40 @@ _mongoc_set_error_with_category (
       va_end (args);
    }
 }
+
+#ifdef _WIN32
+
+char *
+mongoc_winerr_to_string (DWORD err_code)
+{
+   LPSTR msg = NULL;
+   if (0 == FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ARGUMENT_ARRAY |
+                               FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+                            NULL,
+                            err_code,
+                            MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
+                            (LPSTR) &msg,
+                            0,
+                            NULL)) {
+      LocalFree (msg);
+      return bson_strdup_printf ("(0x%.8lX) (Failed to get error message)", err_code);
+   }
+
+   // Remove trailing newline.
+   size_t msglen = strlen (msg);
+   if (msglen >= 1 && msg[msglen - 1] == '\n') {
+      if (msglen >= 2 && msg[msglen - 2] == '\r') {
+         // Remove trailing \r\n.
+         msg[msglen - 2] = '\0';
+      } else {
+         // Just remove trailing \n.
+         msg[msglen - 1] = '\0';
+      }
+   }
+
+   char *ret = bson_strdup_printf ("(0x%.8lX) %s", err_code, msg);
+   LocalFree (msg);
+   return ret;
+}
+
+#endif // _WIN32

src/libmongoc/src/mongoc/mongoc-handshake.c

@@ -32,6 +32,7 @@
 #include <mongoc/mongoc-client.h>
 #include <mongoc/mongoc-client-private.h>
 #include <mongoc/mongoc-error.h>
+#include <mongoc/mongoc-error-private.h>
 #include <mongoc/mongoc-log.h>
 #include <mongoc/mongoc-version.h>
 #include <mongoc/mongoc-util-private.h>
@@ -305,7 +306,9 @@ _get_os_version (void)
       BSON_ASSERT (req > 0);
       found = true;
    } else {
-      MONGOC_WARNING ("Error with GetVersionEx(): %lu", GetLastError ());
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_WARNING ("Error with GetVersionEx(): %s", msg);
+      bson_free (msg);
    }
 
 #elif defined(_POSIX_VERSION)

src/libmongoc/src/mongoc/mongoc-openssl.c

@@ -29,6 +29,7 @@
 
 #include <string.h>
 
+#include <mongoc/mongoc-error-private.h>
 #include <mongoc/mongoc-http-private.h>
 #include <mongoc/mongoc-init.h>
 #include <mongoc/mongoc-openssl-private.h>
@@ -140,16 +141,9 @@ _mongoc_openssl_import_cert_store (LPWSTR store_name, DWORD dwFlags, X509_STORE
                                store_name);                             /* system store name. "My" or "Root" */
 
    if (cert_store == NULL) {
-      LPTSTR msg = NULL;
-      FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
-                     NULL,
-                     GetLastError (),
-                     LANG_NEUTRAL,
-                     (LPTSTR) &msg,
-                     0,
-                     NULL);
-      MONGOC_ERROR ("Can't open CA store: 0x%.8X: '%s'", GetLastError (), msg);
-      LocalFree (msg);
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Can't open CA store: %s", msg);
+      bson_free (msg);
       return false;
    }
 

src/libmongoc/src/mongoc/mongoc-secure-channel.c

@@ -202,7 +202,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
    cert = CertCreateCertificateContext (X509_ASN_ENCODING, encoded_cert, encoded_cert_len);
 
    if (!cert) {
-      MONGOC_ERROR ("Failed to extract public key from '%s'. Error 0x%.8X", filename, (unsigned int) GetLastError ());
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Failed to extract public key from '%s': %s", filename, msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -224,16 +226,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
                                   NULL,                                    /* pvStructInfo */
                                   &blob_private_len);                      /* pcbStructInfo */
    if (!success) {
-      LPTSTR msg = NULL;
-      FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
-                     NULL,
-                     GetLastError (),
-                     LANG_NEUTRAL,
-                     (LPTSTR) &msg,
-                     0,
-                     NULL);
-      MONGOC_ERROR ("Failed to parse private key. %s (0x%.8X)", msg, (unsigned int) GetLastError ());
-      LocalFree (msg);
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Failed to parse private key. %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -247,7 +242,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
                                   blob_private,
                                   &blob_private_len);
    if (!success) {
-      MONGOC_ERROR ("Failed to parse private key. Error 0x%.8X", (unsigned int) GetLastError ());
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Failed to parse private key: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -259,7 +256,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
                                   PROV_RSA_FULL,        /* dwProvType */
                                   CRYPT_VERIFYCONTEXT); /* dwFlags */
    if (!success) {
-      MONGOC_ERROR ("CryptAcquireContext failed with error 0x%.8X", (unsigned int) GetLastError ());
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("CryptAcquireContext failed: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -273,7 +272,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
                              0,                /* dwFlags */
                              &hKey);           /* phKey, OUT */
    if (!success) {
-      MONGOC_ERROR ("CryptImportKey for private key failed with error 0x%.8X", (unsigned int) GetLastError ());
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("CryptImportKey for private key failed: %s", msg);
+      bson_free (msg);
       CryptReleaseContext (provider, 0);
       goto fail;
    }
@@ -287,7 +288,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
                                                 0,                            /* dwFlags */
                                                 (const void *) provider);     /* pvData */
    if (!success) {
-      MONGOC_ERROR ("Can't associate private key with public key: 0x%.8X", (unsigned int) GetLastError ());
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Can't associate private key with public key: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -356,7 +359,9 @@ mongoc_secure_channel_setup_ca (mongoc_ssl_opt_t *opt)
 
    cert = CertCreateCertificateContext (X509_ASN_ENCODING, encoded_cert, encoded_cert_len);
    if (!cert) {
-      MONGOC_WARNING ("Could not convert certificate");
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_WARNING ("Could not convert certificate: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -368,12 +373,16 @@ mongoc_secure_channel_setup_ca (mongoc_ssl_opt_t *opt)
                                L"Root");                                /* system store name. "My" or "Root" */
 
    if (cert_store == NULL) {
-      MONGOC_ERROR ("Error opening certificate store");
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Error opening certificate store: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
    if (!CertAddCertificateContextToStore (cert_store, cert, CERT_STORE_ADD_USE_EXISTING, NULL)) {
-      MONGOC_WARNING ("Failed adding the cert");
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_WARNING ("Failed adding the cert: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -447,12 +456,16 @@ mongoc_secure_channel_setup_crl (mongoc_ssl_opt_t *opt)
                                L"Root");                                /* system store name. "My" or "Root" */
 
    if (cert_store == NULL) {
-      MONGOC_ERROR ("Error opening certificate store");
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_ERROR ("Error opening certificate store: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
    if (!CertAddCRLContextToStore (cert_store, crl, CERT_STORE_ADD_USE_EXISTING, NULL)) {
-      MONGOC_WARNING ("Failed adding the CRL");
+      char *msg = mongoc_winerr_to_string (GetLastError ());
+      MONGOC_WARNING ("Failed adding the CRL: %s", msg);
+      bson_free (msg);
       goto fail;
    }
 
@@ -614,13 +627,12 @@ mongoc_secure_channel_handshake_step_1 (mongoc_stream_tls_t *tls, char *hostname
                                             &secure_channel->ret_flags,         /* pfContextAttr OUT param */
                                             &secure_channel->ctxt->time_stamp   /* ptsExpiry OUT param */
    );
-
    if (sspi_status != SEC_I_CONTINUE_NEEDED) {
-      MONGOC_LOG_AND_SET_ERROR (error,
-                                MONGOC_ERROR_STREAM,
-                                MONGOC_ERROR_STREAM_SOCKET,
-                                "initial InitializeSecurityContext failed: %ld",
-                                sspi_status);
+      // Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
+      char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
+      MONGOC_LOG_AND_SET_ERROR (
+         error, MONGOC_ERROR_STREAM, MONGOC_ERROR_STREAM_SOCKET, "initial InitializeSecurityContext failed: %s", msg);
+      bson_free (msg);
       return false;
    }
 
@@ -849,24 +861,14 @@ mongoc_secure_channel_handshake_step_2 (mongoc_stream_tls_t *tls, char *hostname
 
 
          default: {
-            LPTSTR msg = NULL;
-
-            FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
-                           NULL,
-                           GetLastError (),
-                           LANG_NEUTRAL,
-                           (LPTSTR) &msg,
-                           0,
-                           NULL);
+            // Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
+            char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
             MONGOC_LOG_AND_SET_ERROR (error,
                                       MONGOC_ERROR_STREAM,
                                       MONGOC_ERROR_STREAM_SOCKET,
-                                      "Failed to initialize security context, error code: "
-                                      "0x%04X%04X: %s",
-                                      (unsigned int) (sspi_status >> 16) & 0xffff,
-                                      (unsigned int) sspi_status & 0xffff,
+                                      "Failed to initialize security context: %s",
                                       msg);
-            LocalFree (msg);
+            bson_free (msg);
          }
          }
          return false;

src/libmongoc/src/mongoc/mongoc-sspi.c

@@ -25,6 +25,7 @@
 #define CRYPT_STRING_NOCRLF 0x40000000
 #endif
 
+#include <mongoc/mongoc-error-private.h>
 #include <mongoc/mongoc-util-private.h>
 #include <mongoc/mongoc-sspi-private.h>
 
@@ -56,16 +57,9 @@ _mongoc_sspi_destroy_sspi_client_state (mongoc_sspi_client_state_t *state)
 void
 _mongoc_sspi_set_gsserror (DWORD errCode, const SEC_CHAR *msg)
 {
-   SEC_CHAR *err;
-   DWORD status;
-   DWORD flags = FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
-   status = FormatMessageA (flags, NULL, errCode, MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR) &err, 0, NULL);
-   if (status) {
-      MONGOC_ERROR ("SSPI: %s: %s", msg, err);
-      LocalFree (err);
-   } else {
-      MONGOC_ERROR ("SSPI: %s", msg);
-   }
+   char *err = mongoc_winerr_to_string (errCode);
+   MONGOC_ERROR ("SSPI: %s: %s", msg, err);
+   bson_free (err);
 }
 
 static SEC_CHAR *

src/libmongoc/src/mongoc/mongoc-stream-tls-openssl.c

@@ -628,24 +628,9 @@ _mongoc_stream_tls_openssl_handshake (mongoc_stream_t *stream, const char *host,
 
    /* Otherwise, use simple error info. */
    {
-#ifdef _WIN32
-      LPTSTR msg = NULL;
-      FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
-                     NULL,
-                     errno, /* WSAETIMEDOUT */
-                     LANG_NEUTRAL,
-                     (LPTSTR) &msg,
-                     0,
-                     NULL);
-#else
-      const char *msg = strerror (errno); /* ETIMEDOUT */
-#endif
-
+      char errmsg_buf[BSON_ERROR_BUFFER_SIZE];
+      char *msg = bson_strerror_r (errno, errmsg_buf, sizeof errmsg_buf);
       _mongoc_set_error (error, MONGOC_ERROR_STREAM, MONGOC_ERROR_STREAM_SOCKET, "TLS handshake failed: %s", msg);
-
-#ifdef _WIN32
-      LocalFree (msg);
-#endif
    }
 
    RETURN (false);

src/libmongoc/src/mongoc/mongoc-stream-tls-secure-channel.c

@@ -954,19 +954,10 @@ mongoc_stream_tls_secure_channel_new (mongoc_stream_t *base_stream, const char *
                                            &secure_channel->cred->time_stamp); /* certificate expiration time */
 
    if (sspi_status != SEC_E_OK) {
-      LPTSTR msg = NULL;
-      FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
-                     NULL,
-                     GetLastError (),
-                     LANG_NEUTRAL,
-                     (LPTSTR) &msg,
-                     0,
-                     NULL);
-      MONGOC_ERROR ("Failed to initialize security context, error code: 0x%04X%04X: '%s'",
-                    (unsigned int) (sspi_status >> 16) & 0xffff,
-                    (unsigned int) sspi_status & 0xffff,
-                    msg);
-      LocalFree (msg);
+      // Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
+      char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
+      MONGOC_ERROR ("Failed to initialize security context: %s", msg);
+      bson_free (msg);
       RETURN (NULL);
    }