From fce89adf352d6624e6c91c3e76a17981f52b28f5 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 23 Feb 2022 18:31:04 -0500 Subject: [PATCH 1/3] Store the full SPN within a server gssntlm_name The SPN is used to fill the Traget Name Attribute in the Target Info array. This means we need to preserver the SPN as passed to us (via conversion from a GSS Name). This patch adds an spn field to the server union part of gssntlm_name structure. Signed-off-by: Simo Sorce --- src/gss_names.c | 102 +++++++++++++++++++++++++++++++++----------- src/gss_ntlmssp.h | 1 + tests/ntlmssptest.c | 95 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 173 insertions(+), 25 deletions(-) diff --git a/src/gss_names.c b/src/gss_names.c index a897989..cd83bbb 100644 --- a/src/gss_names.c +++ b/src/gss_names.c @@ -1,4 +1,4 @@ -/* Copyright 2013 Simo Sorce , see COPYING for license */ +/* Copyright 2013-2022 Simo Sorce , see COPYING for license */ #define _GNU_SOURCE @@ -266,9 +266,6 @@ uint32_t gssntlm_import_name_by_mech(uint32_t *minor_status, gss_OID input_name_type, gss_name_t *output_name) { - char hostname[HOST_NAME_MAX + 1] = { 0 }; - char struid[12] = { 0 }; - uid_t uid; struct gssntlm_name *name = NULL; uint32_t retmaj; uint32_t retmin; @@ -291,30 +288,74 @@ uint32_t gssntlm_import_name_by_mech(uint32_t *minor_status, if (gss_oid_equal(input_name_type, GSS_C_NT_HOSTBASED_SERVICE) || gss_oid_equal(input_name_type, GSS_C_NT_HOSTBASED_SERVICE_X)) { + char *spn = NULL; + char *p = NULL; name->type = GSSNTLM_NAME_SERVER; - retmaj = string_split(&retmin, '@', - input_name_buffer->value, - input_name_buffer->length, - NULL, &name->data.server.name); - if ((retmaj == GSS_S_COMPLETE) || - (retmaj != GSS_S_UNAVAILABLE)) { - goto done; + if (input_name_buffer->length > 0) { + spn = strndup(input_name_buffer->value, input_name_buffer->length); + if (!spn) { + set_GSSERR(ENOMEM); + goto done; + } + p = memchr(spn, '@', input_name_buffer->length); + if (p && input_name_buffer->length == 1) { + free(spn); + spn = p = NULL; + } } - /* no seprator, assume only service is provided and try to source - * the local host name */ - retmin = gethostname(hostname, HOST_NAME_MAX); - if (retmin) { - set_GSSERR(retmin); - goto done; - } - hostname[HOST_NAME_MAX] = '\0'; - name->data.server.name = strdup(hostname); - if (!name->data.server.name) { - set_GSSERR(ENOMEM); + if (p) { + /* Windows expects a SPN not a GSS Name */ + if (p != spn) { + *p = '/'; + name->data.server.spn = spn; + spn = NULL; + } + p += 1; + name->data.server.name = strdup(p); + if (!name->data.server.name) { + free(spn); + set_GSSERR(ENOMEM); + goto done; + } + } else { + char hostname[HOST_NAME_MAX + 1] = { 0 }; + size_t l, r; + /* no seprator, assume only service is provided and try to + * source the local host name */ + retmin = gethostname(hostname, HOST_NAME_MAX); + if (retmin) { + free(spn); + set_GSSERR(retmin); + goto done; + } + hostname[HOST_NAME_MAX] = '\0'; + if (spn != NULL) { + /* spn = + + + <\0> */ + l = strlen(spn) + 1 + strlen(hostname) + 1; + name->data.server.spn = malloc(l); + if (!name->data.server.spn) { + free(spn); + set_GSSERR(ENOMEM); + goto done; + } + r = snprintf(name->data.server.spn, l, "%s/%s", spn, hostname); + if (r != l - 1) { + free(spn); + set_GSSERR(ENOMEM); + goto done; + } + } + name->data.server.name = strdup(hostname); + if (!name->data.server.name) { + free(spn); + set_GSSERR(ENOMEM); + goto done; + } } + free(spn); set_GSSERRS(0, GSS_S_COMPLETE); } else if (gss_oid_equal(input_name_type, GSS_C_NT_USER_NAME)) { @@ -326,6 +367,7 @@ uint32_t gssntlm_import_name_by_mech(uint32_t *minor_status, &name->data.user.domain, &name->data.user.name); } else if (gss_oid_equal(input_name_type, GSS_C_NT_MACHINE_UID_NAME)) { + uid_t uid; name->type = GSSNTLM_NAME_USER; name->data.user.domain = NULL; @@ -333,6 +375,8 @@ uint32_t gssntlm_import_name_by_mech(uint32_t *minor_status, uid = *(uid_t *)input_name_buffer->value; retmaj = uid_to_name(&retmin, uid, &name->data.user.name); } else if (gss_oid_equal(input_name_type, GSS_C_NT_STRING_UID_NAME)) { + char struid[12] = { 0 }; + uid_t uid; name->type = GSSNTLM_NAME_USER; name->data.user.domain = NULL; @@ -454,7 +498,7 @@ void gssntlm_release_attrs(struct gssntlm_name_attribute **attrs) int gssntlm_copy_name(struct gssntlm_name *src, struct gssntlm_name *dst) { - char *dom = NULL, *usr = NULL, *srv = NULL; + char *dom = NULL, *usr = NULL, *spn = NULL, *srv = NULL; int ret; dst->type = src->type; switch (src->type) { @@ -480,6 +524,14 @@ int gssntlm_copy_name(struct gssntlm_name *src, struct gssntlm_name *dst) dst->data.user.name = usr; break; case GSSNTLM_NAME_SERVER: + if (src->data.server.spn) { + spn = strdup(src->data.server.spn); + if (!spn) { + ret = ENOMEM; + goto done; + } + } + dst->data.server.spn = spn; if (src->data.server.name) { srv = strdup(src->data.server.name); if (!srv) { @@ -499,6 +551,7 @@ int gssntlm_copy_name(struct gssntlm_name *src, struct gssntlm_name *dst) if (ret) { safefree(dom); safefree(usr); + safefree(spn); safefree(srv); } return ret; @@ -560,6 +613,7 @@ void gssntlm_int_release_name(struct gssntlm_name *name) safefree(name->data.user.name); break; case GSSNTLM_NAME_SERVER: + safefree(name->data.server.spn); safefree(name->data.server.name); break; } @@ -635,7 +689,7 @@ uint32_t gssntlm_display_name(uint32_t *minor_status, } break; case GSSNTLM_NAME_SERVER: - out->value = strdup(in->data.server.name); + out->value = strdup(in->data.server.spn); if (!out->value) { set_GSSERR(ENOMEM); goto done; diff --git a/src/gss_ntlmssp.h b/src/gss_ntlmssp.h index 85220f1..55f0622 100644 --- a/src/gss_ntlmssp.h +++ b/src/gss_ntlmssp.h @@ -59,6 +59,7 @@ struct gssntlm_name { char *name; } user; struct { + char *spn; char *name; } server; } data; diff --git a/tests/ntlmssptest.c b/tests/ntlmssptest.c index cbef759..8e24cbe 100644 --- a/tests/ntlmssptest.c +++ b/tests/ntlmssptest.c @@ -1,14 +1,21 @@ -/* Copyright 2013 Simo Sorce , see COPYING for license */ +/* Copyright 2013-2022 Simo Sorce , see COPYING for license */ #include #include +#include #include #include #include #include +#include #include "config.h" +#ifndef HOST_NAME_MAX +#include +#define HOST_NAME_MAX MAXHOSTNAMELEN +#endif + #include "../src/gssapi_ntlmssp.h" #include "../src/gss_ntlmssp.h" @@ -2790,6 +2797,87 @@ int test_import_name(void) return ret; } +int test_hostbased_name(void) +{ + char hostname[HOST_NAME_MAX + 1] = { 0 }; + struct { + const char *input; + const char *name; + const char *spn_svc; + const char *spn_name; + size_t spn_svc_len; + } hostbased_test[] = { + { "HTTP", hostname, "HTTP/", hostname, 5 }, + { "HTTP@foo.bar", "foo.bar", "HTTP/", "foo.bar", 5 }, + { "@foo.bar", "foo.bar", NULL, NULL, 0 }, + { "@", hostname, NULL, NULL, 0 }, + { "", hostname, NULL, NULL, 0 }, + { NULL, NULL, NULL, NULL, 0 } + }; + int ret = 0; + + /* get hostname to verify results */ + ret = gethostname(hostname, HOST_NAME_MAX); + if (ret) { + fprintf(stderr, "Test: test_hostbased_name failed to get hostname\n"); + } + + for (int i = 0; hostbased_test[i].input != NULL; i++) { + struct gssntlm_name *gss_host_name = NULL; + gss_buffer_desc host_name; + uint32_t retmin, retmaj; + bool failed = false; + + host_name.value = discard_const(hostbased_test[i].input); + host_name.length = strlen(host_name.value); + + retmaj = gssntlm_import_name(&retmin, &host_name, + GSS_C_NT_HOSTBASED_SERVICE, + (gss_name_t *)&gss_host_name); + if (retmaj == GSS_S_COMPLETE) { + if ((gss_host_name->type != GSSNTLM_NAME_SERVER) || + (strcmp(hostbased_test[i].name, + gss_host_name->data.server.name) != 0)) { + failed = true; + } + if (hostbased_test[i].spn_svc_len != 0) { + if ((strncmp(hostbased_test[i].spn_svc, + gss_host_name->data.server.spn, + hostbased_test[i].spn_svc_len) != 0) || + (strcmp(hostbased_test[i].spn_name, + gss_host_name->data.server.spn + + hostbased_test[i].spn_svc_len) != 0)) { + failed = true; + } + } + } else { + failed = true; + } + + if (failed) { + fprintf(stderr, "gssntlm_import_name(%s) failed!\n", + hostbased_test[i].input); + fprintf(stderr, "Expected: [%s%s]\n", + hostbased_test[i].spn_svc, hostbased_test[i].spn_name); + if (gss_host_name) { + fprintf(stderr, "Obtained: [%s]+[%s]\n", + gss_host_name->data.server.spn, + gss_host_name->data.server.name); + } + if (retmaj != GSS_S_COMPLETE) { + print_gss_error("Function returned error.", retmaj, retmin); + } + fflush(stderr); + + ret++; + } + + gssntlm_release_name(&retmin, (gss_name_t *)&gss_host_name); + } + + return ret; +} + int test_debug(void) { char *test_env; @@ -3059,6 +3147,11 @@ int main(int argc, const char *argv[]) fprintf(stderr, "Test: %s\n", (ret ? "FAIL":"SUCCESS")); if (ret) gret += ret; + fprintf(stderr, "Test importing different hostbased name forms\n"); + ret = test_hostbased_name(); + fprintf(stderr, "Test: %s\n", (ret ? "FAIL":"SUCCESS")); + if (ret) gret += ret; + done: ntlm_free_ctx(&ctx); return gret; From 2c882fc8bfa047df5aecd5b6421de7bd665b4224 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 23 Feb 2022 21:03:47 -0500 Subject: [PATCH 2/3] Use the SPN for Target Info When we encode/decode/process target_info use the new stored SPN. Also mark the SPN as unverified, because we never know if the calling code speaks authoritatively, and may be passing an incorrect name. Signed-off-by: Simo Sorce --- src/gss_auth.c | 2 +- src/gss_sec_ctx.c | 32 +++++++++++--------------------- src/ntlm.c | 14 +++++++------- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/gss_auth.c b/src/gss_auth.c index 3b44533..2670673 100644 --- a/src/gss_auth.c +++ b/src/gss_auth.c @@ -84,7 +84,7 @@ uint32_t gssntlm_cli_auth(uint32_t *minor_status, retmin = ntlm_process_target_info( ctx->ntlm, protect, target_info, - ctx->target_name.data.server.name, + ctx->target_name.data.server.spn, &cb, &client_target_info, &srv_time, add_mic_ptr); if (retmin) { diff --git a/src/gss_sec_ctx.c b/src/gss_sec_ctx.c index 779d87f..3afdd78 100644 --- a/src/gss_sec_ctx.c +++ b/src/gss_sec_ctx.c @@ -26,7 +26,6 @@ uint32_t gssntlm_init_sec_context(uint32_t *minor_status, struct gssntlm_ctx *ctx; struct gssntlm_name *server = NULL; struct gssntlm_cred *cred = NULL; - char *computer_name = NULL; char *nb_computer_name = NULL; char *nb_domain_name = NULL; struct gssntlm_name *client_name = NULL; @@ -165,13 +164,8 @@ uint32_t gssntlm_init_sec_context(uint32_t *minor_status, if (retmaj) goto done; } - computer_name = strdup(client_name->data.server.name); - if (!computer_name) { - set_GSSERR(ENOMEM); - goto done; - } - - retmin = netbios_get_names(ctx->external_context, computer_name, + retmin = netbios_get_names(ctx->external_context, + client_name->data.server.name, &nb_computer_name, &nb_domain_name); if (retmin) { set_GSSERR(retmin); @@ -433,7 +427,6 @@ uint32_t gssntlm_init_sec_context(uint32_t *minor_status, gssntlm_release_cred(&tmpmin, (gss_cred_id_t *)&cred); } gssntlm_release_name(&tmpmin, (gss_name_t *)&client_name); - safefree(computer_name); safefree(nb_computer_name); safefree(nb_domain_name); safefree(trgt_name); @@ -532,7 +525,6 @@ uint32_t gssntlm_accept_sec_context(uint32_t *minor_status, int lm_compat_lvl = -1; struct ntlm_buffer challenge = { 0 }; struct gssntlm_name *server_name = NULL; - char *computer_name = NULL; char *nb_computer_name = NULL; char *nb_domain_name = NULL; char *chal_target_name; @@ -618,13 +610,8 @@ uint32_t gssntlm_accept_sec_context(uint32_t *minor_status, goto done; } - computer_name = strdup(server_name->data.server.name); - if (!computer_name) { - set_GSSERR(ENOMEM); - goto done; - } - - retmin = netbios_get_names(ctx->external_context, computer_name, + retmin = netbios_get_names(ctx->external_context, + server_name->data.server.name, &nb_computer_name, &nb_domain_name); if (retmin) { set_GSSERR(retmin); @@ -731,15 +718,19 @@ uint32_t gssntlm_accept_sec_context(uint32_t *minor_status, goto done; } + av_flags = MSVAVFLAGS_UNVERIFIED_SPN; + timestamp = ntlm_timestamp_now(); retmin = ntlm_encode_target_info(ctx->ntlm, nb_computer_name, nb_domain_name, - computer_name, + server_name->data.server.name, NULL, NULL, - NULL, ×tamp, - NULL, NULL, NULL, + &av_flags, ×tamp, + NULL, + server_name->data.server.spn, + NULL, &target_info); if (retmin) { set_GSSERR(retmin); @@ -1028,7 +1019,6 @@ uint32_t gssntlm_accept_sec_context(uint32_t *minor_status, gssntlm_release_name(&tmpmin, (gss_name_t *)&server_name); gssntlm_release_name(&tmpmin, (gss_name_t *)&gss_usrname); gssntlm_release_cred(&tmpmin, (gss_cred_id_t *)&usr_cred); - safefree(computer_name); safefree(nb_computer_name); safefree(nb_domain_name); safefree(usr_name); diff --git a/src/ntlm.c b/src/ntlm.c index e7661f8..81363d7 100644 --- a/src/ntlm.c +++ b/src/ntlm.c @@ -754,7 +754,7 @@ int ntlm_decode_target_info(struct ntlm_ctx *ctx, struct ntlm_buffer *buffer, int ntlm_process_target_info(struct ntlm_ctx *ctx, bool protect, struct ntlm_buffer *in, - const char *server, + const char *spn, struct ntlm_buffer *unhashed_cb, struct ntlm_buffer *out, uint64_t *out_srv_time, @@ -786,8 +786,9 @@ int ntlm_process_target_info(struct ntlm_ctx *ctx, bool protect, goto done; } - if (server && av_target_name) { - if (strcasecmp(server, av_target_name) != 0) { + if (spn && av_target_name && + ((av_flags & MSVAVFLAGS_UNVERIFIED_SPN) == 0)) { + if (strcasecmp(spn, av_target_name) != 0) { ret = EINVAL; goto done; } @@ -808,15 +809,14 @@ int ntlm_process_target_info(struct ntlm_ctx *ctx, bool protect, if (ret) goto done; } - if (!av_target_name && server) { - av_target_name = strdup(server); + if (!av_target_name && spn) { + av_target_name = strdup(spn); if (!av_target_name) { ret = ENOMEM; goto done; } + av_flags |= MSVAVFLAGS_UNVERIFIED_SPN; } - /* TODO: add way to tell if the target name is verified o not, - * if not set av_flags |= MSVAVFLAGS_UNVERIFIED_SPN; */ ret = ntlm_encode_target_info(ctx, nb_computer_name, nb_domain_name, From 2035e33a10e4d7828278c51435fcd91b053ef181 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 23 Feb 2022 21:14:09 -0500 Subject: [PATCH 3/3] Fix serialization to export also the server spn This ups both context and credentials export versions as the size and content of the serilized structires change. Signed-off-by: Simo Sorce --- src/gss_serialize.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/gss_serialize.c b/src/gss_serialize.c index 4e5f8ea..3640a47 100644 --- a/src/gss_serialize.c +++ b/src/gss_serialize.c @@ -26,7 +26,7 @@ struct export_attrs { struct export_name { uint8_t type; - struct relmem domain; + struct relmem dom_or_spn; struct relmem name; struct export_attrs attrs; }; @@ -38,7 +38,7 @@ struct export_keys { uint32_t seq_num; }; -#define EXPORT_CTX_VER 0x0004 +#define EXPORT_CTX_VER 0x0005 struct export_ctx { uint16_t version; uint8_t role; @@ -211,7 +211,7 @@ static int export_name(struct export_state *state, if (name->data.user.domain) { ret = export_data_buffer(state, name->data.user.domain, strlen(name->data.user.domain), - &exp_name->domain); + &exp_name->dom_or_spn); if (ret) { return ret; } @@ -227,6 +227,14 @@ static int export_name(struct export_state *state, break; case GSSNTLM_NAME_SERVER: exp_name->type = EXP_NAME_SERV; + if (name->data.server.spn) { + ret = export_data_buffer(state, name->data.server.spn, + strlen(name->data.server.spn), + &exp_name->dom_or_spn); + if (ret) { + return ret; + } + } if (name->data.server.name) { ret = export_data_buffer(state, name->data.server.name, strlen(name->data.server.name), @@ -599,10 +607,10 @@ static uint32_t import_name(uint32_t *minor_status, case EXP_NAME_USER: imp_name->type = GSSNTLM_NAME_USER; dest = NULL; - if (name->domain.len > 0) { + if (name->dom_or_spn.len > 0) { retmaj = import_data_buffer(&retmin, state, &dest, NULL, true, - &name->domain, true); + &name->dom_or_spn, true); if (retmaj != GSS_S_COMPLETE) goto done; } imp_name->data.user.domain = (char *)dest; @@ -619,6 +627,14 @@ static uint32_t import_name(uint32_t *minor_status, case EXP_NAME_SERV: imp_name->type = GSSNTLM_NAME_SERVER; dest = NULL; + if (name->dom_or_spn.len > 0) { + retmaj = import_data_buffer(&retmin, state, + &dest, NULL, true, + &name->dom_or_spn, true); + if (retmaj != GSS_S_COMPLETE) goto done; + } + imp_name->data.server.spn = (char *)dest; + dest = NULL; if (name->name.len > 0) { retmaj = import_data_buffer(&retmin, state, &dest, NULL, true, @@ -878,9 +894,11 @@ uint32_t gssntlm_import_sec_context(uint32_t *minor_status, return GSSERR(); } +#define EXPORT_CRED_VER 0x0002 + #pragma pack(push, 1) struct export_cred { - uint16_t version; /* 0x00 0x02 */ + uint16_t version; uint16_t type; struct export_name name; /* user or server name */ @@ -933,7 +951,7 @@ uint32_t gssntlm_export_cred(uint32_t *minor_status, state.exp_data = (uint8_t *)&ecred.data - (uint8_t *)&ecred; state.exp_len = state.exp_data; - ecred.version = htole16(1); + ecred.version = htole16(EXPORT_CRED_VER); switch (cred->type) { case GSSNTLM_CRED_NONE: @@ -1052,7 +1070,7 @@ uint32_t gssntlm_import_cred(uint32_t *minor_status, ecred = (struct export_cred *)state.exp_struct; state.exp_data = (char *)ecred->data - (char *)ecred; - if (ecred->version != le16toh(1)) { + if (ecred->version != le16toh(EXPORT_CRED_VER)) { set_GSSERRS(ERR_BADARG, GSS_S_DEFECTIVE_TOKEN); goto done; }