Skip to content

Commit e206584

Browse files
kevinAlbskevinAlbs
authored
[r1.30] CDRIVER-6019 Improve error messages from Windows APIs (#2020) (#2023)
* CDRIVER-6019 Improve error messages from Windows APIs (#2020) * 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
1 parent 0dc5a96 commit e206584

12 files changed

+158
-123
lines changed

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <mongoc/mongoc.h>
2525
#include <mongoc/mongoc-client-private.h>
2626
#include <mongoc/mongoc-client-side-encryption-private.h>
27+
#include <mongoc/mongoc-error-private.h>
2728
#include <mongoc/mongoc-host-list-private.h>
2829
#include <mongoc/mongoc-stream-private.h>
2930
#include <mongoc/mongoc-topology-private.h>
@@ -1377,24 +1378,14 @@ _do_spawn (const char *path, char **args, bson_error_t *error)
13771378
NULL /* current directory */,
13781379
&startup_info,
13791380
&process_information)) {
1380-
long lastError = GetLastError ();
1381-
LPSTR message = NULL;
1382-
1383-
FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ARGUMENT_ARRAY | FORMAT_MESSAGE_FROM_SYSTEM |
1384-
FORMAT_MESSAGE_IGNORE_INSERTS,
1385-
NULL,
1386-
lastError,
1387-
0,
1388-
(LPSTR) &message,
1389-
0,
1390-
NULL);
1381+
char *message = mongoc_winerr_to_string (GetLastError ());
13911382

13921383
bson_set_error (error,
13931384
MONGOC_ERROR_CLIENT,
13941385
MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE,
13951386
"failed to spawn mongocryptd: %s",
13961387
message);
1397-
LocalFree (message);
1388+
bson_free (message);
13981389
mcommon_string_from_append_destroy (&command);
13991390
return false;
14001391
}
@@ -3006,7 +2997,7 @@ _mongoc_encryptedFields_fill_auto_datakeys (
30062997
BSON_ASSERT_PARAM (factory);
30072998

30082999
if (error) {
3009-
*error = (bson_error_t){0};
3000+
*error = (bson_error_t) {0};
30103001
}
30113002
bson_init (out_fields);
30123003

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,10 @@ _mongoc_get_rr_dnsapi (
211211
res = DnsQuery_UTF8 (hostname, nst, options, NULL /* IP Address */, &pdns, 0 /* reserved */);
212212

213213
if (res) {
214-
DWORD flags = FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
215-
216-
if (FormatMessage (flags, 0, res, MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR) &lpMsgBuf, 0, 0)) {
217-
DNS_ERROR ("Failed to look up %s record \"%s\": %s", rr_type_name, hostname, (char *) lpMsgBuf);
218-
}
219-
220-
DNS_ERROR ("Failed to look up %s record \"%s\": Unknown error", rr_type_name, hostname);
214+
// Cast signed DNS_STATUS to unsigned DWORD. FormatMessage expects DWORD.
215+
char *msg = mongoc_winerr_to_string ((DWORD) res);
216+
DNS_ERROR ("Failed to look up %s record \"%s\": %s", rr_type_name, hostname, msg);
217+
bson_free (msg);
221218
}
222219

223220
if (!pdns) {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,12 @@ typedef enum {
105105
bool
106106
mongoc_error_append_contents_to_bson (const bson_error_t *error, bson_t *bson, mongoc_error_content_flags_t flags);
107107

108+
#ifdef _WIN32
109+
// Call `mongoc_winerr_to_string` on a Windows error code (e.g. a return from GetLastError()).
110+
char *
111+
mongoc_winerr_to_string (DWORD err_code);
112+
#endif
113+
108114
BSON_END_DECLS
109115

110116
#endif /* MONGOC_ERROR_PRIVATE_H */

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,3 +338,40 @@ mongoc_error_append_contents_to_bson (const bson_error_t *error, bson_t *bson, m
338338
}
339339
return true;
340340
}
341+
342+
#ifdef _WIN32
343+
344+
char *
345+
mongoc_winerr_to_string (DWORD err_code)
346+
{
347+
LPSTR msg = NULL;
348+
if (0 == FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ARGUMENT_ARRAY |
349+
FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
350+
NULL,
351+
err_code,
352+
MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
353+
(LPSTR) &msg,
354+
0,
355+
NULL)) {
356+
LocalFree (msg);
357+
return bson_strdup_printf ("(0x%.8lX) (Failed to get error message)", err_code);
358+
}
359+
360+
// Remove trailing newline.
361+
size_t msglen = strlen (msg);
362+
if (msglen >= 1 && msg[msglen - 1] == '\n') {
363+
if (msglen >= 2 && msg[msglen - 2] == '\r') {
364+
// Remove trailing \r\n.
365+
msg[msglen - 2] = '\0';
366+
} else {
367+
// Just remove trailing \n.
368+
msg[msglen - 1] = '\0';
369+
}
370+
}
371+
372+
char *ret = bson_strdup_printf ("(0x%.8lX) %s", err_code, msg);
373+
LocalFree (msg);
374+
return ret;
375+
}
376+
377+
#endif // _WIN32

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include <mongoc/mongoc-client.h>
3333
#include <mongoc/mongoc-client-private.h>
3434
#include <mongoc/mongoc-error.h>
35+
#include <mongoc/mongoc-error-private.h>
3536
#include <mongoc/mongoc-log.h>
3637
#include <mongoc/mongoc-version.h>
3738
#include <mongoc/mongoc-util-private.h>
@@ -313,7 +314,9 @@ _get_os_version (void)
313314
BSON_ASSERT (req > 0);
314315
found = true;
315316
} else {
316-
MONGOC_WARNING ("Error with GetVersionEx(): %lu", GetLastError ());
317+
char *msg = mongoc_winerr_to_string (GetLastError ());
318+
MONGOC_WARNING ("Error with GetVersionEx(): %s", msg);
319+
bson_free (msg);
317320
}
318321

319322
#elif defined(_POSIX_VERSION)

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
#include <string.h>
3131

32+
#include <mongoc/mongoc-error-private.h>
3233
#include <mongoc/mongoc-http-private.h>
3334
#include <mongoc/mongoc-init.h>
3435
#include <mongoc/mongoc-openssl-private.h>
@@ -139,16 +140,9 @@ _mongoc_openssl_import_cert_store (LPWSTR store_name, DWORD dwFlags, X509_STORE
139140
store_name); /* system store name. "My" or "Root" */
140141

141142
if (cert_store == NULL) {
142-
LPTSTR msg = NULL;
143-
FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
144-
NULL,
145-
GetLastError (),
146-
LANG_NEUTRAL,
147-
(LPTSTR) &msg,
148-
0,
149-
NULL);
150-
MONGOC_ERROR ("Can't open CA store: 0x%.8X: '%s'", GetLastError (), msg);
151-
LocalFree (msg);
143+
char *msg = mongoc_winerr_to_string (GetLastError ());
144+
MONGOC_ERROR ("Can't open CA store: %s", msg);
145+
bson_free (msg);
152146
return false;
153147
}
154148

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

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <mongoc/mongoc-stream-tls-secure-channel-private.h>
3030
#include <mongoc/mongoc-errno-private.h>
3131
#include <mongoc/mongoc-error.h>
32+
#include <mongoc/mongoc-error-private.h>
3233
#include <common-string-private.h>
3334
#include <common-cmp-private.h>
3435

@@ -202,7 +203,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
202203
cert = CertCreateCertificateContext (X509_ASN_ENCODING, encoded_cert, encoded_cert_len);
203204

204205
if (!cert) {
205-
MONGOC_ERROR ("Failed to extract public key from '%s'. Error 0x%.8X", filename, (unsigned int) GetLastError ());
206+
char *msg = mongoc_winerr_to_string (GetLastError ());
207+
MONGOC_ERROR ("Failed to extract public key from '%s': %s", filename, msg);
208+
bson_free (msg);
206209
goto fail;
207210
}
208211

@@ -224,16 +227,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
224227
NULL, /* pvStructInfo */
225228
&blob_private_len); /* pcbStructInfo */
226229
if (!success) {
227-
LPTSTR msg = NULL;
228-
FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
229-
NULL,
230-
GetLastError (),
231-
LANG_NEUTRAL,
232-
(LPTSTR) &msg,
233-
0,
234-
NULL);
235-
MONGOC_ERROR ("Failed to parse private key. %s (0x%.8X)", msg, (unsigned int) GetLastError ());
236-
LocalFree (msg);
230+
char *msg = mongoc_winerr_to_string (GetLastError ());
231+
MONGOC_ERROR ("Failed to parse private key. %s", msg);
232+
bson_free (msg);
237233
goto fail;
238234
}
239235

@@ -247,7 +243,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
247243
blob_private,
248244
&blob_private_len);
249245
if (!success) {
250-
MONGOC_ERROR ("Failed to parse private key. Error 0x%.8X", (unsigned int) GetLastError ());
246+
char *msg = mongoc_winerr_to_string (GetLastError ());
247+
MONGOC_ERROR ("Failed to parse private key: %s", msg);
248+
bson_free (msg);
251249
goto fail;
252250
}
253251

@@ -259,7 +257,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
259257
PROV_RSA_FULL, /* dwProvType */
260258
CRYPT_VERIFYCONTEXT); /* dwFlags */
261259
if (!success) {
262-
MONGOC_ERROR ("CryptAcquireContext failed with error 0x%.8X", (unsigned int) GetLastError ());
260+
char *msg = mongoc_winerr_to_string (GetLastError ());
261+
MONGOC_ERROR ("CryptAcquireContext failed: %s", msg);
262+
bson_free (msg);
263263
goto fail;
264264
}
265265

@@ -273,7 +273,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
273273
0, /* dwFlags */
274274
&hKey); /* phKey, OUT */
275275
if (!success) {
276-
MONGOC_ERROR ("CryptImportKey for private key failed with error 0x%.8X", (unsigned int) GetLastError ());
276+
char *msg = mongoc_winerr_to_string (GetLastError ());
277+
MONGOC_ERROR ("CryptImportKey for private key failed: %s", msg);
278+
bson_free (msg);
277279
CryptReleaseContext (provider, 0);
278280
goto fail;
279281
}
@@ -287,7 +289,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
287289
0, /* dwFlags */
288290
(const void *) provider); /* pvData */
289291
if (!success) {
290-
MONGOC_ERROR ("Can't associate private key with public key: 0x%.8X", (unsigned int) GetLastError ());
292+
char *msg = mongoc_winerr_to_string (GetLastError ());
293+
MONGOC_ERROR ("Can't associate private key with public key: %s", msg);
294+
bson_free (msg);
291295
goto fail;
292296
}
293297

@@ -356,7 +360,9 @@ mongoc_secure_channel_setup_ca (mongoc_ssl_opt_t *opt)
356360

357361
cert = CertCreateCertificateContext (X509_ASN_ENCODING, encoded_cert, encoded_cert_len);
358362
if (!cert) {
359-
MONGOC_WARNING ("Could not convert certificate");
363+
char *msg = mongoc_winerr_to_string (GetLastError ());
364+
MONGOC_WARNING ("Could not convert certificate: %s", msg);
365+
bson_free (msg);
360366
goto fail;
361367
}
362368

@@ -368,12 +374,16 @@ mongoc_secure_channel_setup_ca (mongoc_ssl_opt_t *opt)
368374
L"Root"); /* system store name. "My" or "Root" */
369375

370376
if (cert_store == NULL) {
371-
MONGOC_ERROR ("Error opening certificate store");
377+
char *msg = mongoc_winerr_to_string (GetLastError ());
378+
MONGOC_ERROR ("Error opening certificate store: %s", msg);
379+
bson_free (msg);
372380
goto fail;
373381
}
374382

375383
if (!CertAddCertificateContextToStore (cert_store, cert, CERT_STORE_ADD_USE_EXISTING, NULL)) {
376-
MONGOC_WARNING ("Failed adding the cert");
384+
char *msg = mongoc_winerr_to_string (GetLastError ());
385+
MONGOC_WARNING ("Failed adding the cert: %s", msg);
386+
bson_free (msg);
377387
goto fail;
378388
}
379389

@@ -447,12 +457,16 @@ mongoc_secure_channel_setup_crl (mongoc_ssl_opt_t *opt)
447457
L"Root"); /* system store name. "My" or "Root" */
448458

449459
if (cert_store == NULL) {
450-
MONGOC_ERROR ("Error opening certificate store");
460+
char *msg = mongoc_winerr_to_string (GetLastError ());
461+
MONGOC_ERROR ("Error opening certificate store: %s", msg);
462+
bson_free (msg);
451463
goto fail;
452464
}
453465

454466
if (!CertAddCRLContextToStore (cert_store, crl, CERT_STORE_ADD_USE_EXISTING, NULL)) {
455-
MONGOC_WARNING ("Failed adding the CRL");
467+
char *msg = mongoc_winerr_to_string (GetLastError ());
468+
MONGOC_WARNING ("Failed adding the CRL: %s", msg);
469+
bson_free (msg);
456470
goto fail;
457471
}
458472

@@ -614,13 +628,12 @@ mongoc_secure_channel_handshake_step_1 (mongoc_stream_tls_t *tls, char *hostname
614628
&secure_channel->ret_flags, /* pfContextAttr OUT param */
615629
&secure_channel->ctxt->time_stamp /* ptsExpiry OUT param */
616630
);
617-
618631
if (sspi_status != SEC_I_CONTINUE_NEEDED) {
619-
MONGOC_LOG_AND_SET_ERROR (error,
620-
MONGOC_ERROR_STREAM,
621-
MONGOC_ERROR_STREAM_SOCKET,
622-
"initial InitializeSecurityContext failed: %ld",
623-
sspi_status);
632+
// Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
633+
char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
634+
MONGOC_LOG_AND_SET_ERROR (
635+
error, MONGOC_ERROR_STREAM, MONGOC_ERROR_STREAM_SOCKET, "initial InitializeSecurityContext failed: %s", msg);
636+
bson_free (msg);
624637
return false;
625638
}
626639

@@ -849,24 +862,14 @@ mongoc_secure_channel_handshake_step_2 (mongoc_stream_tls_t *tls, char *hostname
849862

850863

851864
default: {
852-
LPTSTR msg = NULL;
853-
854-
FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
855-
NULL,
856-
GetLastError (),
857-
LANG_NEUTRAL,
858-
(LPTSTR) &msg,
859-
0,
860-
NULL);
865+
// Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
866+
char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
861867
MONGOC_LOG_AND_SET_ERROR (error,
862868
MONGOC_ERROR_STREAM,
863869
MONGOC_ERROR_STREAM_SOCKET,
864-
"Failed to initialize security context, error code: "
865-
"0x%04X%04X: %s",
866-
(unsigned int) (sspi_status >> 16) & 0xffff,
867-
(unsigned int) sspi_status & 0xffff,
870+
"Failed to initialize security context: %s",
868871
msg);
869-
LocalFree (msg);
872+
bson_free (msg);
870873
}
871874
}
872875
return false;

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#define CRYPT_STRING_NOCRLF 0x40000000
2626
#endif
2727

28+
#include <mongoc/mongoc-error-private.h>
2829
#include <mongoc/mongoc-util-private.h>
2930
#include <mongoc/mongoc-sspi-private.h>
3031

@@ -56,16 +57,9 @@ _mongoc_sspi_destroy_sspi_client_state (mongoc_sspi_client_state_t *state)
5657
void
5758
_mongoc_sspi_set_gsserror (DWORD errCode, const SEC_CHAR *msg)
5859
{
59-
SEC_CHAR *err;
60-
DWORD status;
61-
DWORD flags = FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
62-
status = FormatMessageA (flags, NULL, errCode, MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR) &err, 0, NULL);
63-
if (status) {
64-
MONGOC_ERROR ("SSPI: %s: %s", msg, err);
65-
LocalFree (err);
66-
} else {
67-
MONGOC_ERROR ("SSPI: %s", msg);
68-
}
60+
char *err = mongoc_winerr_to_string (errCode);
61+
MONGOC_ERROR ("SSPI: %s: %s", msg, err);
62+
bson_free (err);
6963
}
7064

7165
static SEC_CHAR *

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

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -628,24 +628,9 @@ _mongoc_stream_tls_openssl_handshake (mongoc_stream_t *stream, const char *host,
628628

629629
/* Otherwise, use simple error info. */
630630
{
631-
#ifdef _WIN32
632-
LPTSTR msg = NULL;
633-
FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
634-
NULL,
635-
errno, /* WSAETIMEDOUT */
636-
LANG_NEUTRAL,
637-
(LPTSTR) &msg,
638-
0,
639-
NULL);
640-
#else
641-
const char *msg = strerror (errno); /* ETIMEDOUT */
642-
#endif
643-
631+
char errmsg_buf[BSON_ERROR_BUFFER_SIZE];
632+
char *msg = bson_strerror_r (errno, errmsg_buf, sizeof errmsg_buf);
644633
bson_set_error (error, MONGOC_ERROR_STREAM, MONGOC_ERROR_STREAM_SOCKET, "TLS handshake failed: %s", msg);
645-
646-
#ifdef _WIN32
647-
LocalFree (msg);
648-
#endif
649634
}
650635

651636
RETURN (false);

0 commit comments

Comments
 (0)