Skip to content

Commit 149fb98

Browse files
addaleaxaddaleax
authored
chore(NODE-3500): clean up various bugs and memory leaks (#135)
1 parent 677df23 commit 149fb98

File tree

9 files changed

+185
-186
lines changed

9 files changed

+185
-186
lines changed

‎src/kerberos_common.h‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
#ifndef COMMON_H
2-
#define COMMON_H
1+
#ifndef KERBEROS_COMMON_H
2+
#define KERBEROS_COMMON_H
33

44
#include <nan.h>
55

@@ -17,11 +17,6 @@ typedef sspi_server_state krb_server_state;
1717
typedef sspi_result krb_result;
1818
#endif
1919

20-
// Provide a default custom delter for the `gss_result` type
21-
inline void ResultDeleter(krb_result* result) {
22-
free(result);
23-
}
24-
2520
// Useful methods for optional value handling
2621
NAN_INLINE std::string StringOptionValue(v8::Local<v8::Object> options, const char* _key) {
2722
Nan::HandleScope scope;

‎src/kerberos_worker.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
#ifndef ASYNC_WORKER_H
2-
#define ASYNC_WORKER_H
1+
#ifndef KERBEROS_ASYNC_WORKER_H
2+
#define KERBEROS_ASYNC_WORKER_H
33

44
#include <functional>
55
#include <nan.h>

‎src/unix/base64.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
#include <string.h>
2121

2222
// base64 tables
23-
static char basis_64[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
24-
static signed char index_64[128] = {
23+
static const char basis_64[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
24+
static const signed char index_64[128] = {
2525
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
2626
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62,
2727
-1, -1, -1, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, -1, -1, -1, -1, -1, 0,

‎src/unix/kerberos_gss.cc‎

Lines changed: 51 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
2929
#endif
3030

31-
static gss_result* gss_success_result(int ret);
32-
static gss_result* gss_error_result(OM_uint32 err_maj, OM_uint32 err_min);
33-
static gss_result* gss_error_result_with_message(const char* message);
34-
static gss_result* gss_error_result_with_message_and_code(const char* mesage, int code);
31+
static gss_result gss_success_result(int ret);
32+
static gss_result gss_error_result(OM_uint32 err_maj, OM_uint32 err_min);
33+
static gss_result gss_error_result_with_message(const char* message);
34+
static gss_result gss_error_result_with_message_and_code(const char* mesage, int code);
3535

3636
gss_client_state* gss_client_state_new() {
3737
gss_client_state* state = (gss_client_state*)malloc(sizeof(gss_client_state));
@@ -53,11 +53,11 @@ gss_server_state* gss_server_state_new() {
5353
return state;
5454
}
5555

56-
gss_result* server_principal_details(const char* service, const char* hostname) {
56+
gss_result server_principal_details(const char* service, const char* hostname) {
5757
char match[1024];
5858
size_t match_len = 0;
59-
char* details = NULL;
60-
gss_result* result = NULL;
59+
std::string details;
60+
gss_result result {};
6161

6262
int code;
6363
krb5_context kcontext;
@@ -96,8 +96,7 @@ gss_result* server_principal_details(const char* service, const char* hostname)
9696
}
9797

9898
if (strncmp(pname, match, match_len) == 0) {
99-
details = (char*)malloc(strlen(pname) + 1);
100-
strcpy(details, pname);
99+
details = pname;
101100
krb5_free_unparsed_name(kcontext, pname);
102101
krb5_free_keytab_entry_contents(kcontext, &entry);
103102
break;
@@ -107,11 +106,11 @@ gss_result* server_principal_details(const char* service, const char* hostname)
107106
krb5_free_keytab_entry_contents(kcontext, &entry);
108107
}
109108

110-
if (details == NULL) {
109+
if (details.empty()) {
111110
result = gss_error_result_with_message_and_code("Principal not found in keytab", -1);
112111
} else {
113112
result = gss_success_result(AUTH_GSS_COMPLETE);
114-
result->data = details;
113+
result.data = std::move(details);
115114
}
116115
end:
117116
if (cursor)
@@ -123,7 +122,7 @@ gss_result* server_principal_details(const char* service, const char* hostname)
123122
return result;
124123
}
125124

126-
gss_result* authenticate_gss_client_init(const char* service,
125+
gss_result authenticate_gss_client_init(const char* service,
127126
const char* principal,
128127
long int gss_flags,
129128
gss_server_state* delegatestate,
@@ -133,7 +132,7 @@ gss_result* authenticate_gss_client_init(const char* service,
133132
OM_uint32 min_stat;
134133
gss_buffer_desc name_token = GSS_C_EMPTY_BUFFER;
135134
gss_buffer_desc principal_token = GSS_C_EMPTY_BUFFER;
136-
gss_result* ret = NULL;
135+
gss_result ret {};
137136

138137
state->server_name = GSS_C_NO_NAME;
139138
state->mech_oid = mech_oid;
@@ -218,14 +217,14 @@ int authenticate_gss_client_clean(gss_client_state* state) {
218217
return ret;
219218
}
220219

221-
gss_result* authenticate_gss_client_step(gss_client_state* state,
220+
gss_result authenticate_gss_client_step(gss_client_state* state,
222221
const char* challenge,
223222
struct gss_channel_bindings_struct* channel_bindings) {
224223
OM_uint32 maj_stat;
225224
OM_uint32 min_stat;
226225
gss_buffer_desc input_token = GSS_C_EMPTY_BUFFER;
227226
gss_buffer_desc output_token = GSS_C_EMPTY_BUFFER;
228-
gss_result* ret = NULL;
227+
gss_result ret {};
229228
int temp_ret = AUTH_GSS_CONTINUE;
230229

231230
// Always clear out the old response
@@ -330,13 +329,13 @@ gss_result* authenticate_gss_client_step(gss_client_state* state,
330329
return ret;
331330
}
332331

333-
gss_result* authenticate_gss_client_unwrap(gss_client_state* state, const char* challenge) {
332+
gss_result authenticate_gss_client_unwrap(gss_client_state* state, const char* challenge) {
334333
OM_uint32 maj_stat;
335334
OM_uint32 min_stat;
336335
gss_buffer_desc input_token = GSS_C_EMPTY_BUFFER;
337336
gss_buffer_desc output_token = GSS_C_EMPTY_BUFFER;
338337
int conf = 0;
339-
gss_result* ret = NULL;
338+
gss_result ret {};
340339

341340
// Always clear out the old response
342341
if (state->response != NULL) {
@@ -378,7 +377,7 @@ gss_result* authenticate_gss_client_unwrap(gss_client_state* state, const char*
378377
return ret;
379378
}
380379

381-
gss_result* authenticate_gss_client_wrap(gss_client_state* state,
380+
gss_result authenticate_gss_client_wrap(gss_client_state* state,
382381
const char* challenge,
383382
const char* user,
384383
int protect) {
@@ -388,7 +387,7 @@ gss_result* authenticate_gss_client_wrap(gss_client_state* state,
388387
gss_buffer_desc output_token = GSS_C_EMPTY_BUFFER;
389388
char buf[4096];
390389
unsigned long buf_size;
391-
gss_result* ret = NULL;
390+
gss_result ret {};
392391

393392
// Always clear out the old response
394393
if (state->response != NULL) {
@@ -422,9 +421,10 @@ gss_result* authenticate_gss_client_wrap(gss_client_state* state,
422421
memcpy(buf, &buf_size, 4);
423422
buf[0] = GSS_AUTH_P_NONE;
424423
// server decides if principal can log in as user
425-
strncpy(buf + 4, user, sizeof(buf) - 4);
424+
strncpy(buf + 4, user, sizeof(buf) - 5);
425+
buf[sizeof(buf) - 1] = '\0';
426426
input_token.value = buf;
427-
input_token.length = 4 + strlen(user);
427+
input_token.length = 4 + strlen(buf + 4);
428428
}
429429

430430
// Do GSSAPI wrap
@@ -455,12 +455,12 @@ gss_result* authenticate_gss_client_wrap(gss_client_state* state,
455455
return ret;
456456
}
457457

458-
gss_result* authenticate_gss_server_init(const char* service, gss_server_state* state) {
458+
gss_result authenticate_gss_server_init(const char* service, gss_server_state* state) {
459459
OM_uint32 maj_stat;
460460
OM_uint32 min_stat;
461461
size_t service_len;
462462
gss_buffer_desc name_token = GSS_C_EMPTY_BUFFER;
463-
gss_result* ret = NULL;
463+
gss_result ret {};
464464

465465
state->context = GSS_C_NO_CONTEXT;
466466
state->server_name = GSS_C_NO_NAME;
@@ -537,14 +537,14 @@ int authenticate_gss_server_clean(gss_server_state* state) {
537537
return ret;
538538
}
539539

540-
gss_result* authenticate_gss_server_step(gss_server_state* state, const char* challenge) {
540+
gss_result authenticate_gss_server_step(gss_server_state* state, const char* challenge) {
541541
OM_uint32 maj_stat;
542542
OM_uint32 min_stat;
543543
gss_buffer_desc input_token = GSS_C_EMPTY_BUFFER;
544544
gss_buffer_desc output_token = GSS_C_EMPTY_BUFFER;
545545
gss_name_t target_name = GSS_C_NO_NAME;
546546
// int ret = AUTH_GSS_CONTINUE;
547-
gss_result* ret = NULL;
547+
gss_result ret {};
548548

549549
// Always clear out the old response
550550
if (state->response != NULL) {
@@ -632,15 +632,15 @@ gss_result* authenticate_gss_server_step(gss_server_state* state, const char* ch
632632
return ret;
633633
}
634634

635-
gss_result* authenticate_user_krb5pwd(const char* user,
635+
gss_result authenticate_user_krb5pwd(const char* user,
636636
const char* pswd,
637637
const char* service,
638638
const char* default_realm) {
639639
krb5_context kcontext = NULL;
640640
krb5_error_code code;
641641
krb5_principal client = NULL;
642642
krb5_principal server = NULL;
643-
gss_result* result = NULL;
643+
gss_result result {};
644644
int ret = 0;
645645
char* name = NULL;
646646
char* p = NULL;
@@ -727,58 +727,53 @@ gss_result* authenticate_user_krb5pwd(const char* user,
727727
return result;
728728
}
729729

730-
static gss_result* gss_success_result(int ret) {
731-
gss_result* result = (gss_result*)malloc(sizeof(gss_result));
732-
result->code = ret;
733-
result->message = NULL;
734-
return result;
730+
static gss_result gss_success_result(int ret) {
731+
return { ret, "", "" };
735732
}
736733

737-
static gss_result* gss_error_result(OM_uint32 err_maj, OM_uint32 err_min) {
734+
static gss_result gss_error_result(OM_uint32 err_maj, OM_uint32 err_min) {
738735
OM_uint32 maj_stat, min_stat;
739736
OM_uint32 msg_ctx = 0;
740737
gss_buffer_desc status_string;
741-
char buf_maj[512];
742-
char buf_min[512];
743-
gss_result* result;
738+
std::string buf_maj;
739+
std::string buf_min;
744740

745741
do {
746742
maj_stat = gss_display_status(
747743
&min_stat, err_maj, GSS_C_GSS_CODE, GSS_C_NO_OID, &msg_ctx, &status_string);
748744
if (GSS_ERROR(maj_stat))
749745
break;
750-
strncpy(buf_maj, (char*)status_string.value, sizeof(buf_maj));
746+
buf_maj = (char*)status_string.value;
751747
gss_release_buffer(&min_stat, &status_string);
752748

753749
maj_stat = gss_display_status(
754750
&min_stat, err_min, GSS_C_MECH_CODE, GSS_C_NULL_OID, &msg_ctx, &status_string);
755751
if (!GSS_ERROR(maj_stat)) {
756-
strncpy(buf_min, (char*)status_string.value, sizeof(buf_min));
757-
gss_release_buffer(&min_stat, &status_string);
752+
buf_min = (char*)status_string.value;
758753
}
759754
} while (!GSS_ERROR(maj_stat) && msg_ctx != 0);
760755

761-
result = (gss_result*)malloc(sizeof(gss_result));
762-
result->code = AUTH_GSS_ERROR;
763-
result->message = (char*)malloc(sizeof(char) * 1024 + 2);
764-
sprintf(result->message, "%s: %s", buf_maj, buf_min);
765-
766-
return result;
756+
return {
757+
AUTH_GSS_ERROR,
758+
buf_maj + ": " + buf_min,
759+
""
760+
};
767761
}
768762

769-
static gss_result* gss_error_result_with_message(const char* message) {
770-
gss_result* result = (gss_result*)malloc(sizeof(gss_result));
771-
result->code = AUTH_GSS_ERROR;
772-
result->message = strdup(message);
773-
return result;
763+
static gss_result gss_error_result_with_message(const char* message) {
764+
return {
765+
AUTH_GSS_ERROR,
766+
message,
767+
""
768+
};
774769
}
775770

776-
static gss_result* gss_error_result_with_message_and_code(const char* message, int code) {
777-
gss_result* result = (gss_result*)malloc(sizeof(gss_result));
778-
result->code = AUTH_GSS_ERROR;
779-
result->message = (char*)malloc(strlen(message) + 20);
780-
sprintf(result->message, "%s (%d)", message, code);
781-
return result;
771+
static gss_result gss_error_result_with_message_and_code(const char* message, int code) {
772+
return {
773+
AUTH_GSS_ERROR,
774+
std::string(message) + " (" + std::to_string(code) + ")",
775+
""
776+
};
782777
}
783778

784779
#if defined(__clang__)

‎src/unix/kerberos_gss.h‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,20 @@
1414
* limitations under the License.
1515
**/
1616

17+
#ifndef KERBEROS_GSS_H
18+
#define KERBEROS_GSS_H
19+
1720
extern "C" {
1821
#include <gssapi/gssapi.h>
1922
#include <gssapi/gssapi_generic.h>
2023
#include <gssapi/gssapi_krb5.h>
2124
}
2225

23-
#define krb5_get_err_text(context, code) error_message(code)
26+
#include <string>
27+
28+
inline const char* krb5_get_err_text(const krb5_context&, krb5_error_code code) {
29+
return error_message(code);
30+
}
2431

2532
#define AUTH_GSS_ERROR -1
2633
#define AUTH_GSS_COMPLETE 1
@@ -32,8 +39,8 @@ extern "C" {
3239

3340
typedef struct {
3441
int code;
35-
char* message;
36-
char* data;
42+
std::string message;
43+
std::string data;
3744
} gss_result;
3845

3946
typedef struct {
@@ -63,29 +70,31 @@ typedef struct {
6370
gss_client_state* gss_client_state_new();
6471
gss_server_state* gss_server_state_new();
6572

66-
gss_result* server_principal_details(const char* service, const char* hostname);
73+
gss_result server_principal_details(const char* service, const char* hostname);
6774

68-
gss_result* authenticate_gss_client_init(const char* service,
69-
const char* principal,
70-
long int gss_flags,
71-
gss_server_state* delegatestate,
72-
gss_OID mech_oid,
73-
gss_client_state* state);
75+
gss_result authenticate_gss_client_init(const char* service,
76+
const char* principal,
77+
long int gss_flags,
78+
gss_server_state* delegatestate,
79+
gss_OID mech_oid,
80+
gss_client_state* state);
7481

7582
int authenticate_gss_client_clean(gss_client_state* state);
76-
gss_result* authenticate_gss_client_step(gss_client_state* state,
77-
const char* challenge,
78-
struct gss_channel_bindings_struct* channel_bindings);
79-
gss_result* authenticate_gss_client_unwrap(gss_client_state* state, const char* challenge);
80-
gss_result* authenticate_gss_client_wrap(gss_client_state* state,
81-
const char* challenge,
82-
const char* user,
83-
int protect);
84-
gss_result* authenticate_gss_server_init(const char* service, gss_server_state* state);
83+
gss_result authenticate_gss_client_step(gss_client_state* state,
84+
const char* challenge,
85+
struct gss_channel_bindings_struct* channel_bindings);
86+
gss_result authenticate_gss_client_unwrap(gss_client_state* state, const char* challenge);
87+
gss_result authenticate_gss_client_wrap(gss_client_state* state,
88+
const char* challenge,
89+
const char* user,
90+
int protect);
91+
gss_result authenticate_gss_server_init(const char* service, gss_server_state* state);
8592
int authenticate_gss_server_clean(gss_server_state* state);
86-
gss_result* authenticate_gss_server_step(gss_server_state* state, const char* challenge);
93+
gss_result authenticate_gss_server_step(gss_server_state* state, const char* challenge);
94+
95+
gss_result authenticate_user_krb5pwd(const char* user,
96+
const char* pswd,
97+
const char* service,
98+
const char* default_realm);
8799

88-
gss_result* authenticate_user_krb5pwd(const char* user,
89-
const char* pswd,
90-
const char* service,
91-
const char* default_realm);
100+
#endif

0 commit comments

Comments
 (0)