From 1e37143c7d03a27141a7de68091237ee9f163833 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli <5175948+kinkie@users.noreply.github.com> Date: Sat, 30 Dec 2023 16:34:18 +0000 Subject: [PATCH] Cleanup: move NTLM status from defines to enum (#676) NTLM status codes are defines and overlap somewhat. Turn them into a C++ enum class. --- lib/ntlmauth/ntlmauth.cc | 38 +++++++-------- lib/ntlmauth/ntlmauth.h | 48 ++++++++---------- src/auth/ntlm/SMB_LM/ntlm_smb_lm_auth.cc | 62 ++++++++++++++++-------- src/auth/ntlm/SSPI/ntlm_sspi_auth.cc | 55 ++++++++++----------- src/auth/ntlm/fake/ntlm_fake_auth.cc | 4 +- 5 files changed, 112 insertions(+), 95 deletions(-) diff --git a/lib/ntlmauth/ntlmauth.cc b/lib/ntlmauth/ntlmauth.cc index c578eb2081..eb6cb45804 100644 --- a/lib/ntlmauth/ntlmauth.cc +++ b/lib/ntlmauth/ntlmauth.cc @@ -59,12 +59,12 @@ ntlm_dump_ntlmssp_flags(uint32_t flags) /** * Check the validity of a decoded NTLM packet. * - * \retval NTLM_ERR_NONE Packet is okay - * \retval NTLM_ERR_BLOB Packet is not even an NTLMSSP packet at all. - * \retval NTLM_ERR_PROTOCOL Packet is not the expected type. + * \retval NtlmError::None Packet is okay + * \retval NtlmError::BlobError Packet is not even an NTLMSSP packet at all. + * \retval NtlmError::ProtocolError Packet is not the expected type. */ -int -ntlm_validate_packet(const ntlmhdr * hdr, const int32_t type) +NtlmError +ntlm_validate_packet(const ntlmhdr *hdr, const int32_t type) { /* * Must be the correct security package and request type. @@ -72,17 +72,17 @@ ntlm_validate_packet(const ntlmhdr * hdr, const int32_t type) */ if (memcmp(hdr->signature, "NTLMSSP", 8) != 0) { fprintf(stderr, "ntlmCheckHeader: bad header signature\n"); - return NTLM_ERR_BLOB; + return NtlmError::BlobError; } if (type == NTLM_ANY) - return NTLM_ERR_NONE; + return NtlmError::None; if ((int32_t)le32toh(hdr->type) != type) { /* don't report this error - it's ok as we do a if() around this function */ debug("ntlm_validate_packet: type is %d, wanted %d\n", le32toh(hdr->type), type); - return NTLM_ERR_PROTOCOL; + return NtlmError::ProtocolError; } - return NTLM_ERR_NONE; + return NtlmError::None; } /** @@ -237,19 +237,19 @@ ntlm_make_challenge(ntlm_challenge *ch, * this function will only insert data if the packet contains any. Otherwise * the buffers will be left untouched. * - * \retval NTLM_ERR_NONE username present, maybe also domain. - * \retval NTLM_ERR_PROTOCOL packet type is not an authentication packet. - * \retval NTLM_ERR_LOGON no username. - * \retval NTLM_ERR_BLOB domain field is apparently larger than the packet. + * \retval NtlmError::None username present, maybe also domain. + * \retval NtlmError::ProtocolError packet type is not an authentication packet. + * \retval NtlmError::LoginEror no username. + * \retval NtlmError::BlobError domain field is apparently larger than the packet. */ -int +NtlmError ntlm_unpack_auth(const ntlm_authenticate *auth, char *user, char *domain, const int32_t size) { lstring rv; - if (ntlm_validate_packet(&auth->hdr, NTLM_AUTHENTICATE)) { + if (ntlm_validate_packet(&auth->hdr, NTLM_AUTHENTICATE) != NtlmError::None) { fprintf(stderr, "ntlm_unpack_auth: header check fails\n"); - return NTLM_ERR_PROTOCOL; + return NtlmError::ProtocolError; } debug("ntlm_unpack_auth: size of %d\n", size); debug("ntlm_unpack_auth: flg %08x\n", auth->flags); @@ -268,7 +268,7 @@ ntlm_unpack_auth(const ntlm_authenticate *auth, char *user, char *domain, const } if (rv.l >= size) { debug("ntlm_unpack_auth: Domain length %d too big for %d byte packet.\n", rv.l, size); - return NTLM_ERR_BLOB; + return NtlmError::BlobError; } rv = ntlm_fetch_string(&auth->hdr, size, &auth->user, auth->flags); @@ -277,8 +277,8 @@ ntlm_unpack_auth(const ntlm_authenticate *auth, char *user, char *domain, const user[rv.l] = '\0'; debug("ntlm_unpack_auth: Username '%s' (len=%d).\n", user, rv.l); } else - return NTLM_ERR_LOGON; + return NtlmError::LoginEror; - return NTLM_ERR_NONE; + return NtlmError::None; } diff --git a/lib/ntlmauth/ntlmauth.h b/lib/ntlmauth/ntlmauth.h index 278daef00a..e454847ae6 100644 --- a/lib/ntlmauth/ntlmauth.h +++ b/lib/ntlmauth/ntlmauth.h @@ -13,10 +13,6 @@ /* Endian functions are usually handled by the OS but not always. */ #include "ntlmauth/support_endian.h" -#ifdef __cplusplus -extern "C" { -#endif - /* Used internally. Microsoft seems to think this is right, I believe them. * Right. */ #define NTLM_MAX_FIELD_LENGTH 300 /* max length of an NTLMSSP field */ @@ -32,21 +28,21 @@ extern "C" { #define NTLM_REQUEST_NON_NT_SESSION_KEY 0x400000 /* NTLM error codes */ -#define NTLM_ERR_INTERNAL -3 -#define NTLM_ERR_BLOB -2 -#define NTLM_ERR_BAD_PROTOCOL -1 -#define NTLM_ERR_NONE 0 /* aka. SMBLM_ERR_NONE */ -/* codes used by smb_lm helper */ -#define NTLM_ERR_SERVER 1 /* aka. SMBLM_ERR_SERVER */ -#define NTLM_ERR_PROTOCOL 2 /* aka. SMBLM_ERR_PROTOCOL */ -#define NTLM_ERR_LOGON 3 /* aka. SMBLM_ERR_LOGON */ -#define NTLM_ERR_UNTRUSTED_DOMAIN 4 -#define NTLM_ERR_NOT_CONNECTED 10 -/* codes used by mswin_ntlmsspi helper */ -#define NTLM_SSPI_ERROR 1 -#define NTLM_BAD_NTGROUP 2 -#define NTLM_BAD_REQUEST 3 -/* TODO: reduce the above codes down to one set non-overlapping. */ +enum class NtlmError +{ + None = 0, + ServerError, + ProtocolError, + LoginEror, + UntrustedDomain, + NotConnected, + SspiError, + BadNtGroup, + BadRequest, + InternalError, + BlobError, + BadProtocol +}; /** String header. String data resides at the end of the request */ typedef struct _strhdr { @@ -83,7 +79,7 @@ typedef struct _ntlmhdr { } ntlmhdr; /** Validate the packet type matches one we want. */ -int ntlm_validate_packet(const ntlmhdr *packet, const int32_t type); +NtlmError ntlm_validate_packet(const ntlmhdr *packet, const int32_t type); /** Retrieve a string from the NTLM packet payload. */ lstring ntlm_fetch_string(const ntlmhdr *packet, @@ -183,14 +179,10 @@ typedef struct _ntlm_authenticate { } ntlm_authenticate; /** Unpack username and domain out of a packet payload. */ -int ntlm_unpack_auth(const ntlm_authenticate *auth, - char *user, - char *domain, - const int32_t size); - -#if __cplusplus -} -#endif +NtlmError ntlm_unpack_auth(const ntlm_authenticate *auth, + char *user, + char *domain, + const int32_t size); #endif /* SQUID_NTLMAUTH_H */ diff --git a/src/auth/ntlm/SMB_LM/ntlm_smb_lm_auth.cc b/src/auth/ntlm/SMB_LM/ntlm_smb_lm_auth.cc index 78aee1b42c..cdf9c6bcec 100644 --- a/src/auth/ntlm/SMB_LM/ntlm_smb_lm_auth.cc +++ b/src/auth/ntlm/SMB_LM/ntlm_smb_lm_auth.cc @@ -93,7 +93,7 @@ static unsigned char challenge[NTLM_NONCE_LEN]; static unsigned char lmencoded_empty_pass[ENCODED_PASS_LEN], ntencoded_empty_pass[ENCODED_PASS_LEN]; SMB_Handle_Type handle = nullptr; -int ntlm_errno; +static NtlmError ntlm_errno; static char credentials[MAX_USERNAME_LEN+MAX_DOMAIN_LEN+2]; /* we can afford to waste */ static char my_domain[100], my_domain_controller[100]; static char errstr[1001]; @@ -218,7 +218,6 @@ make_challenge(char *domain, char *domain_controller) char * ntlm_check_auth(ntlm_authenticate * auth, int auth_length) { - int rv; char pass[MAX_PASSWD_LEN+1]; char *domain = credentials; char *user; @@ -226,7 +225,7 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) if (handle == NULL) { /*if null we aren't connected, but it shouldn't happen */ debug("Weird, we've been disconnected\n"); - ntlm_errno = NTLM_ERR_NOT_CONNECTED; + ntlm_errno = NtlmError::NotConnected; return nullptr; } @@ -234,12 +233,12 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) tmp = ntlm_fetch_string(&(auth->hdr), auth_length, &auth->domain, auth->flags); if (tmp.str == NULL || tmp.l == 0) { debug("No domain supplied. Returning no-auth\n"); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } if (tmp.l > MAX_DOMAIN_LEN) { debug("Domain string exceeds %d bytes, rejecting\n", MAX_DOMAIN_LEN); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } memcpy(domain, tmp.str, tmp.l); @@ -251,12 +250,12 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) tmp = ntlm_fetch_string(&(auth->hdr), auth_length, &auth->user, auth->flags); if (tmp.str == NULL || tmp.l == 0) { debug("No username supplied. Returning no-auth\n"); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } if (tmp.l > MAX_USERNAME_LEN) { debug("Username string exceeds %d bytes, rejecting\n", MAX_USERNAME_LEN); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } memcpy(user, tmp.str, tmp.l); @@ -272,7 +271,7 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) if (len != ENCODED_PASS_LEN || offset + len > auth_length || offset == 0) { debug("LM response: insane data (pkt-sz: %d, fetch len: %d, offset: %d)\n", auth_length, len, offset); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } tmp.str = (char *)packet + offset; @@ -280,7 +279,7 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) } if (tmp.l > MAX_PASSWD_LEN) { debug("Password string exceeds %d bytes, rejecting\n", MAX_PASSWD_LEN); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } @@ -293,7 +292,7 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) if (memcmp(tmp.str,lmencoded_empty_pass,ENCODED_PASS_LEN)==0) { fprintf(stderr,"Empty LM password supplied for user %s\\%s. " "No-auth\n",domain,user); - ntlm_errno=NTLM_ERR_LOGON; + ntlm_errno=NtlmError::LoginEror; return nullptr; } @@ -307,7 +306,7 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) if (len != ENCODED_PASS_LEN || offset + len > auth_length || offset == 0) { debug("NT response: insane data (pkt-sz: %d, fetch len: %d, offset: %d)\n", auth_length, len, offset); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } tmp.str = (char *)packet + offset; @@ -317,7 +316,7 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) user,ntencoded_empty_pass,tmp.str,tmp.l); if (memcmp(tmp.str,lmencoded_empty_pass,ENCODED_PASS_LEN)==0) { fprintf(stderr,"ERROR: Empty NT password supplied for user %s\\%s. No-auth\n", domain, user); - ntlm_errno = NTLM_ERR_LOGON; + ntlm_errno = NtlmError::LoginEror; return nullptr; } } @@ -325,25 +324,50 @@ ntlm_check_auth(ntlm_authenticate * auth, int auth_length) debug("checking domain: '%s', user: '%s', pass='%s'\n", domain, user, pass); - rv = SMB_Logon_Server(handle, user, pass, domain, 1); + const auto rv = SMB_Logon_Server(handle, user, pass, domain, 1); debug("Login attempt had result %d\n", rv); - if (rv != NTLM_ERR_NONE) { /* failed */ - ntlm_errno = rv; + switch (rv) { + case SMBlibE_Success: + ntlm_errno = NtlmError::None; + break; + case SMBlibE_BAD: + ntlm_errno = NtlmError::BlobError; + return nullptr; + case SMBlibE_ProtLow: + case SMBlibE_NoSpace: + case SMBlibE_BadParam: + case SMBlibE_NegNoProt: + case SMBlibE_LowerLayer: + case SMBlibE_SendFailed: + case SMBlibE_RecvFailed: + case SMBlibE_ProtUnknown: + case SMBlibE_NoSuchMsg: + ntlm_errno = NtlmError::ProtocolError; + return nullptr; + case SMBlibE_NotImpl: + case SMBlibE_CallFailed: + case SMBlibE_Remote: + ntlm_errno = NtlmError::ServerError; + return nullptr; + case SMBlibE_GuestOnly: + ntlm_errno = NtlmError::LoginEror; + return nullptr; + default: + ntlm_errno = NtlmError::ServerError; return nullptr; } + *(user - 1) = '\\'; /* hack. Performing, but ugly. */ debug("credentials: %s\n", credentials); return credentials; } -extern "C" void timeout_during_auth(int signum); - static char got_timeout = 0; /** signal handler to be invoked when the authentication operation * times out */ -void +extern "C" void timeout_during_auth(int) { dc_disconnect(); @@ -561,7 +585,7 @@ manage_request() } if (cred == NULL) { int smblib_err, smb_errorclass, smb_errorcode, nb_error; - if (ntlm_errno == NTLM_ERR_LOGON) { /* hackish */ + if (ntlm_errno == NtlmError::LoginEror) { /* hackish */ SEND("NA Logon Failure"); return; } diff --git a/src/auth/ntlm/SSPI/ntlm_sspi_auth.cc b/src/auth/ntlm/SSPI/ntlm_sspi_auth.cc index f9f44d1d0a..fea8557c29 100644 --- a/src/auth/ntlm/SSPI/ntlm_sspi_auth.cc +++ b/src/auth/ntlm/SSPI/ntlm_sspi_auth.cc @@ -97,7 +97,7 @@ int fail_debug_enabled = 0; #endif /* returns 1 on success, 0 on failure */ -int +static int Valid_Group(char *UserName, char *Group) { int result = FALSE; @@ -170,63 +170,62 @@ Valid_Group(char *UserName, char *Group) * In case of problem returns one of the * codes defined in libntlmauth/ntlmauth.h */ -int +static NtlmError ntlm_check_auth(ntlm_authenticate * auth, char *user, char *domain, int auth_length) { - int x; - int rv; char credentials[DNLEN+UNLEN+2]; /* we can afford to waste */ if (!NTLM_LocalCall) { user[0] = '\0'; domain[0] = '\0'; - x = ntlm_unpack_auth(auth, user, domain, auth_length); + const auto x = ntlm_unpack_auth(auth, user, domain, auth_length); - if (x != NTLM_ERR_NONE) + if (x != NtlmError::None) return x; if (domain[0] == '\0') { debug("No domain supplied. Returning no-auth\n"); - return NTLM_BAD_REQUEST; + return NtlmError::BadRequest; } if (user[0] == '\0') { debug("No username supplied. Returning no-auth\n"); - return NTLM_BAD_REQUEST; + return NtlmError::BadRequest; } debug("checking domain: '%s', user: '%s'\n", domain, user); - } else + } else { debug("checking local user\n"); + } snprintf(credentials, DNLEN+UNLEN+2, "%s\\%s", domain, user); - rv = SSP_ValidateNTLMCredentials(auth, auth_length, credentials); + const auto rv = SSP_ValidateNTLMCredentials(auth, auth_length, credentials); debug("Login attempt had result %d\n", rv); if (!rv) { /* failed */ - return NTLM_SSPI_ERROR; + return NtlmError::SspiError; } if (UseAllowedGroup) { if (!Valid_Group(credentials, NTAllowedGroup)) { debug("User %s not in allowed Group %s\n", credentials, NTAllowedGroup); - return NTLM_BAD_NTGROUP; + return NtlmError::BadNtGroup; } } if (UseDisallowedGroup) { if (Valid_Group(credentials, NTDisAllowedGroup)) { debug("User %s is in denied Group %s\n", credentials, NTDisAllowedGroup); - return NTLM_BAD_NTGROUP; + return NtlmError::BadNtGroup; } } debug("credentials: %s\n", credentials); - return NTLM_ERR_NONE; + return NtlmError::None; } -void +static void helperfail(const char *reason) { #if FAIL_DEBUG @@ -244,7 +243,7 @@ helperfail(const char *reason) */ char *my_program_name = nullptr; -void +static void usage() { fprintf(stderr, @@ -257,7 +256,7 @@ usage() my_program_name); } -void +static void process_options(int argc, char *argv[]) { int opt, had_error = 0; @@ -312,7 +311,7 @@ token_decode(size_t *decodedLen, uint8_t decoded[], const char *buf) return true; } -int +static int manage_request() { ntlmhdr *fast_header; @@ -360,7 +359,9 @@ manage_request() if ((strlen(buf) > 3) && NTLM_packet_debug_enabled) { if (!token_decode(&decodedLen, decoded, buf+3)) return 1; - strncpy(helper_command, buf, 2); + helper_command[0] = buf[0]; + helper_command[1] = buf[1]; + helper_command[2] = '\0'; debug("Got '%s' from Squid with data:\n", helper_command); hex_dump(reinterpret_cast(decoded), decodedLen); } else @@ -383,7 +384,7 @@ manage_request() fast_header = (struct _ntlmhdr *) decoded; /* sanity-check: it IS a NTLMSSP packet, isn't it? */ - if (ntlm_validate_packet(fast_header, NTLM_ANY) != NTLM_ERR_NONE) { + if (ntlm_validate_packet(fast_header, NTLM_ANY) != NtlmError::None) { SEND_ERR("message=\"Broken authentication packet\""); return 1; } @@ -441,7 +442,7 @@ manage_request() fast_header = (struct _ntlmhdr *) decoded; /* sanity-check: it IS a NTLMSSP packet, isn't it? */ - if (ntlm_validate_packet(fast_header, NTLM_ANY) != NTLM_ERR_NONE) { + if (ntlm_validate_packet(fast_header, NTLM_ANY) != NtlmError::None) { SEND_ERR("message=\"Broken authentication packet\""); return 1; } @@ -456,22 +457,22 @@ manage_request() /* notreached */ case NTLM_AUTHENTICATE: { /* check against SSPI */ - int err = ntlm_check_auth((ntlm_authenticate *) decoded, user, domain, decodedLen); + const auto err = ntlm_check_auth((ntlm_authenticate *) decoded, user, domain, decodedLen); have_challenge = 0; - if (err != NTLM_ERR_NONE) { + if (err != NtlmError::None) { #if FAIL_DEBUG fail_debug_enabled =1; #endif switch (err) { - case NTLM_ERR_NONE: + case NtlmError::None: break; - case NTLM_BAD_NTGROUP: + case NtlmError::BadNtGroup: SEND_ERR("message=\"Incorrect Group Membership\""); return 1; - case NTLM_BAD_REQUEST: + case NtlmError::BadRequest: SEND_ERR("message=\"Incorrect Request Format\""); return 1; - case NTLM_SSPI_ERROR: + case NtlmError::SspiError: FormatMessage( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | diff --git a/src/auth/ntlm/fake/ntlm_fake_auth.cc b/src/auth/ntlm/fake/ntlm_fake_auth.cc index f61842de6b..851651807f 100644 --- a/src/auth/ntlm/fake/ntlm_fake_auth.cc +++ b/src/auth/ntlm/fake/ntlm_fake_auth.cc @@ -218,8 +218,8 @@ main(int argc, char *argv[]) } else if (strncmp(buf, "KK ", 3) == 0) { if (!packet) { SEND("BH received KK with no data! user="); - } else if (ntlm_validate_packet(packet, NTLM_AUTHENTICATE) == NTLM_ERR_NONE) { - if (ntlm_unpack_auth((ntlm_authenticate *)packet, user, domain, decodedLen) == NTLM_ERR_NONE) { + } else if (ntlm_validate_packet(packet, NTLM_AUTHENTICATE) == NtlmError::None) { + if (ntlm_unpack_auth((ntlm_authenticate *)packet, user, domain, decodedLen) == NtlmError::None) { lc(user); if (strip_domain_enabled) { SEND2("AF %s", user); -- 2.47.2