Skip to content

Commit 73b9009

Browse files
kevinAlbskevinAlbs
authored
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
1 parent b6e91c8 commit 73b9009

12 files changed

+152
-121
lines changed

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,24 +1378,14 @@ _do_spawn (const char *path, char **args, bson_error_t *error)
13781378
NULL /* current directory */,
13791379
&startup_info,
13801380
&process_information)) {
1381-
long lastError = GetLastError ();
1382-
LPSTR message = NULL;
1383-
1384-
FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ARGUMENT_ARRAY | FORMAT_MESSAGE_FROM_SYSTEM |
1385-
FORMAT_MESSAGE_IGNORE_INSERTS,
1386-
NULL,
1387-
lastError,
1388-
0,
1389-
(LPSTR) &message,
1390-
0,
1391-
NULL);
1381+
char *message = mongoc_winerr_to_string (GetLastError ());
13921382

13931383
_mongoc_set_error (error,
13941384
MONGOC_ERROR_CLIENT,
13951385
MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE,
13961386
"failed to spawn mongocryptd: %s",
13971387
message);
1398-
LocalFree (message);
1388+
bson_free (message);
13991389
mcommon_string_from_append_destroy (&command);
14001390
return false;
14011391
}

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

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

204204
if (res) {
205-
DWORD flags = FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
206-
207-
if (FormatMessage (flags, 0, res, MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR) &lpMsgBuf, 0, 0)) {
208-
DNS_ERROR ("Failed to look up %s record \"%s\": %s", rr_type_name, hostname, (char *) lpMsgBuf);
209-
}
210-
211-
DNS_ERROR ("Failed to look up %s record \"%s\": Unknown error", rr_type_name, hostname);
205+
// Cast signed DNS_STATUS to unsigned DWORD. FormatMessage expects DWORD.
206+
char *msg = mongoc_winerr_to_string ((DWORD) res);
207+
DNS_ERROR ("Failed to look up %s record \"%s\": %s", rr_type_name, hostname, msg);
208+
bson_free (msg);
212209
}
213210

214211
if (!pdns) {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ _mongoc_set_error_category (bson_error_t *error, uint8_t category)
129129
error->reserved = category;
130130
}
131131

132+
#ifdef _WIN32
133+
// Call `mongoc_winerr_to_string` on a Windows error code (e.g. a return from GetLastError()).
134+
char *
135+
mongoc_winerr_to_string (DWORD err_code);
136+
#endif
137+
132138
BSON_END_DECLS
133139

134140
#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
@@ -369,3 +369,40 @@ _mongoc_set_error_with_category (
369369
va_end (args);
370370
}
371371
}
372+
373+
#ifdef _WIN32
374+
375+
char *
376+
mongoc_winerr_to_string (DWORD err_code)
377+
{
378+
LPSTR msg = NULL;
379+
if (0 == FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ARGUMENT_ARRAY |
380+
FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
381+
NULL,
382+
err_code,
383+
MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
384+
(LPSTR) &msg,
385+
0,
386+
NULL)) {
387+
LocalFree (msg);
388+
return bson_strdup_printf ("(0x%.8lX) (Failed to get error message)", err_code);
389+
}
390+
391+
// Remove trailing newline.
392+
size_t msglen = strlen (msg);
393+
if (msglen >= 1 && msg[msglen - 1] == '\n') {
394+
if (msglen >= 2 && msg[msglen - 2] == '\r') {
395+
// Remove trailing \r\n.
396+
msg[msglen - 2] = '\0';
397+
} else {
398+
// Just remove trailing \n.
399+
msg[msglen - 1] = '\0';
400+
}
401+
}
402+
403+
char *ret = bson_strdup_printf ("(0x%.8lX) %s", err_code, msg);
404+
LocalFree (msg);
405+
return ret;
406+
}
407+
408+
#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>
@@ -305,7 +306,9 @@ _get_os_version (void)
305306
BSON_ASSERT (req > 0);
306307
found = true;
307308
} else {
308-
MONGOC_WARNING ("Error with GetVersionEx(): %lu", GetLastError ());
309+
char *msg = mongoc_winerr_to_string (GetLastError ());
310+
MONGOC_WARNING ("Error with GetVersionEx(): %s", msg);
311+
bson_free (msg);
309312
}
310313

311314
#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>
@@ -140,16 +141,9 @@ _mongoc_openssl_import_cert_store (LPWSTR store_name, DWORD dwFlags, X509_STORE
140141
store_name); /* system store name. "My" or "Root" */
141142

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

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

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
202202
cert = CertCreateCertificateContext (X509_ASN_ENCODING, encoded_cert, encoded_cert_len);
203203

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

@@ -224,16 +226,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
224226
NULL, /* pvStructInfo */
225227
&blob_private_len); /* pcbStructInfo */
226228
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);
229+
char *msg = mongoc_winerr_to_string (GetLastError ());
230+
MONGOC_ERROR ("Failed to parse private key. %s", msg);
231+
bson_free (msg);
237232
goto fail;
238233
}
239234

@@ -247,7 +242,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
247242
blob_private,
248243
&blob_private_len);
249244
if (!success) {
250-
MONGOC_ERROR ("Failed to parse private key. Error 0x%.8X", (unsigned int) GetLastError ());
245+
char *msg = mongoc_winerr_to_string (GetLastError ());
246+
MONGOC_ERROR ("Failed to parse private key: %s", msg);
247+
bson_free (msg);
251248
goto fail;
252249
}
253250

@@ -259,7 +256,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
259256
PROV_RSA_FULL, /* dwProvType */
260257
CRYPT_VERIFYCONTEXT); /* dwFlags */
261258
if (!success) {
262-
MONGOC_ERROR ("CryptAcquireContext failed with error 0x%.8X", (unsigned int) GetLastError ());
259+
char *msg = mongoc_winerr_to_string (GetLastError ());
260+
MONGOC_ERROR ("CryptAcquireContext failed: %s", msg);
261+
bson_free (msg);
263262
goto fail;
264263
}
265264

@@ -273,7 +272,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
273272
0, /* dwFlags */
274273
&hKey); /* phKey, OUT */
275274
if (!success) {
276-
MONGOC_ERROR ("CryptImportKey for private key failed with error 0x%.8X", (unsigned int) GetLastError ());
275+
char *msg = mongoc_winerr_to_string (GetLastError ());
276+
MONGOC_ERROR ("CryptImportKey for private key failed: %s", msg);
277+
bson_free (msg);
277278
CryptReleaseContext (provider, 0);
278279
goto fail;
279280
}
@@ -287,7 +288,9 @@ mongoc_secure_channel_setup_certificate_from_file (const char *filename)
287288
0, /* dwFlags */
288289
(const void *) provider); /* pvData */
289290
if (!success) {
290-
MONGOC_ERROR ("Can't associate private key with public key: 0x%.8X", (unsigned int) GetLastError ());
291+
char *msg = mongoc_winerr_to_string (GetLastError ());
292+
MONGOC_ERROR ("Can't associate private key with public key: %s", msg);
293+
bson_free (msg);
291294
goto fail;
292295
}
293296

@@ -356,7 +359,9 @@ mongoc_secure_channel_setup_ca (mongoc_ssl_opt_t *opt)
356359

357360
cert = CertCreateCertificateContext (X509_ASN_ENCODING, encoded_cert, encoded_cert_len);
358361
if (!cert) {
359-
MONGOC_WARNING ("Could not convert certificate");
362+
char *msg = mongoc_winerr_to_string (GetLastError ());
363+
MONGOC_WARNING ("Could not convert certificate: %s", msg);
364+
bson_free (msg);
360365
goto fail;
361366
}
362367

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

370375
if (cert_store == NULL) {
371-
MONGOC_ERROR ("Error opening certificate store");
376+
char *msg = mongoc_winerr_to_string (GetLastError ());
377+
MONGOC_ERROR ("Error opening certificate store: %s", msg);
378+
bson_free (msg);
372379
goto fail;
373380
}
374381

375382
if (!CertAddCertificateContextToStore (cert_store, cert, CERT_STORE_ADD_USE_EXISTING, NULL)) {
376-
MONGOC_WARNING ("Failed adding the cert");
383+
char *msg = mongoc_winerr_to_string (GetLastError ());
384+
MONGOC_WARNING ("Failed adding the cert: %s", msg);
385+
bson_free (msg);
377386
goto fail;
378387
}
379388

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

449458
if (cert_store == NULL) {
450-
MONGOC_ERROR ("Error opening certificate store");
459+
char *msg = mongoc_winerr_to_string (GetLastError ());
460+
MONGOC_ERROR ("Error opening certificate store: %s", msg);
461+
bson_free (msg);
451462
goto fail;
452463
}
453464

454465
if (!CertAddCRLContextToStore (cert_store, crl, CERT_STORE_ADD_USE_EXISTING, NULL)) {
455-
MONGOC_WARNING ("Failed adding the CRL");
466+
char *msg = mongoc_winerr_to_string (GetLastError ());
467+
MONGOC_WARNING ("Failed adding the CRL: %s", msg);
468+
bson_free (msg);
456469
goto fail;
457470
}
458471

@@ -614,13 +627,12 @@ mongoc_secure_channel_handshake_step_1 (mongoc_stream_tls_t *tls, char *hostname
614627
&secure_channel->ret_flags, /* pfContextAttr OUT param */
615628
&secure_channel->ctxt->time_stamp /* ptsExpiry OUT param */
616629
);
617-
618630
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);
631+
// Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
632+
char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
633+
MONGOC_LOG_AND_SET_ERROR (
634+
error, MONGOC_ERROR_STREAM, MONGOC_ERROR_STREAM_SOCKET, "initial InitializeSecurityContext failed: %s", msg);
635+
bson_free (msg);
624636
return false;
625637
}
626638

@@ -849,24 +861,14 @@ mongoc_secure_channel_handshake_step_2 (mongoc_stream_tls_t *tls, char *hostname
849861

850862

851863
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);
864+
// Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
865+
char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
861866
MONGOC_LOG_AND_SET_ERROR (error,
862867
MONGOC_ERROR_STREAM,
863868
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,
869+
"Failed to initialize security context: %s",
868870
msg);
869-
LocalFree (msg);
871+
bson_free (msg);
870872
}
871873
}
872874
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
_mongoc_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);

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -954,19 +954,10 @@ mongoc_stream_tls_secure_channel_new (mongoc_stream_t *base_stream, const char *
954954
&secure_channel->cred->time_stamp); /* certificate expiration time */
955955

956956
if (sspi_status != SEC_E_OK) {
957-
LPTSTR msg = NULL;
958-
FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
959-
NULL,
960-
GetLastError (),
961-
LANG_NEUTRAL,
962-
(LPTSTR) &msg,
963-
0,
964-
NULL);
965-
MONGOC_ERROR ("Failed to initialize security context, error code: 0x%04X%04X: '%s'",
966-
(unsigned int) (sspi_status >> 16) & 0xffff,
967-
(unsigned int) sspi_status & 0xffff,
968-
msg);
969-
LocalFree (msg);
957+
// Cast signed SECURITY_STATUS to unsigned DWORD. FormatMessage expects DWORD.
958+
char *msg = mongoc_winerr_to_string ((DWORD) sspi_status);
959+
MONGOC_ERROR ("Failed to initialize security context: %s", msg);
960+
bson_free (msg);
970961
RETURN (NULL);
971962
}
972963

0 commit comments

Comments
 (0)