gssntlmssp library crashes on anonymous user authentication (usr_name == NULL)

[kvv81]

We are using gssntlmssp in SMB2 server for NTLM authentication (as a plugin for GSSAPI library called for SESSION_SETUP verb). We have this problem in gssntlmssp during null session penetration test (using anonymous session - i.e. empty user with empty password):
net use \\rnd-volodymyr2\root "" /user:

The problem is due to bug of anonymous access validation:
src/gss_sec_ctx.c:838

        if (((usr_name == NULL) || (usr_name[0] == '\0')) &&
            (nt_chal_resp.length == 0) &&
            (((lm_chal_resp.length == 1) && (lm_chal_resp.data[0] == '\0')) ||
             (lm_chal_resp.length == 0))) {
            /* Anonymous auth */
            /* FIXME: not supported for now */
            set_GSSERR(ERR_NOTSUPPORTED);
            goto done;

        } else {
         ...
         ulen = strlen(usr_name);

Compound condition requires all sub-conditions to be met:
empty user_name AND zero NT challenge len AND zero LM challenge len
So if only user_name is empty but challenges are not zero - we go to full processing.
During full processing we don't check user_name for NULL anymore and fails on NULL pointer de-reference.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fe739ffb700 (LWP 726)]
gssntlm_accept_sec_context (minor_status=0x7fe739fb8574, context_handle=0x7fe70c000a40, acceptor_cred_handle=<optimized out>, input_token=0x7fe71c001110, input_chan_bindings=0x0, src_name=0x7fe739fb8218, 
    mech_type=0x7fe739fb8228, output_token=0x7fe739fb83b0, ret_flags=0x7fe739fb8204, time_rec=0x7fe739fb857c, delegated_cred_handle=0x0) at src/gss_sec_ctx.c:866
866	            ulen = strlen(usr_name);
(gdb) bt
#0  gssntlm_accept_sec_context (minor_status=0x7fe739fb8574, context_handle=0x7fe70c000a40, acceptor_cred_handle=<optimized out>, input_token=0x7fe71c001110, input_chan_bindings=0x0, src_name=0x7fe739fb8218, 
    mech_type=0x7fe739fb8228, output_token=0x7fe739fb83b0, ret_flags=0x7fe739fb8204, time_rec=0x7fe739fb857c, delegated_cred_handle=0x0) at src/gss_sec_ctx.c:866
#1  0x00007fecb46b06d6 in gss_accept_sec_context () from /lib64/libgssapi_krb5.so.2
#2  0x00007fecb46dde73 in spnego_gss_accept_sec_context () from /lib64/libgssapi_krb5.so.2
#3  0x00007fecb46b06d6 in gss_accept_sec_context () from /lib64/libgssapi_krb5.so.2
#4  0x00007feca3b0b1e7 in Proto::GssApi::accept_security_context(unsigned short, unsigned char*, Proto::GssapiSecurityContext*, Proto::GssapiBuffer*) (this=0x7feca99eeab0 <Smb2::SmbProto::_gss_api>, 
    req_security_buffer_length=535, req_security_buffer=<optimized out>, context=0x7fe9f1e2c148, output_token=0x7fe517cefe18) at src/proto/common/gss_api.cpp:269
#5  0x00007feca3e24f70 in Smb2::accept_security_context_async_worker(void*) (args=0x7fe517cefda0) at src/proto/smb2/smb2_session.cpp:957
...
(gdb) p usr_name
$1 = 0x0
(gdb) p nt_chal_resp.length
$2 = 316
(gdb) p lm_chal_resp.length
$3 = 24

Proposed fix is to change validation to this:

if (empty user_name OR zero NT challenge len OR zero LM challenge len) {
  set_GSSERR(ERR_NOTSUPPORTED);
} else {
 ...

Patch file is below:

diff --git a/src/gss_sec_ctx.c b/src/gss_sec_ctx.c
index 2989f1a..f1a433c 100644
--- a/src/gss_sec_ctx.c
+++ b/src/gss_sec_ctx.c
@@ -835,8 +835,8 @@ uint32_t gssntlm_accept_sec_context(uint32_t *minor_status,
 goto done;
 }
 
- if (((usr_name == NULL) || (usr_name[0] == '\0')) &&
- (nt_chal_resp.length == 0) &&
+ if (((usr_name == NULL) || (usr_name[0] == '\0')) ||
+ (nt_chal_resp.length == 0) ||
 (((lm_chal_resp.length == 1) && (lm_chal_resp.data[0] == '\0')) ||
 (lm_chal_resp.length == 0))) {
 /* Anonymous auth */

[simo5]

I think an empty LM/zero len LM is allowed, so it can be an OR for that one, probably an AND between NT and LM, ie both need to be empty for it to be considered a bad thing.

[simo5]

@kvv81 What do you think of #61 ?

[kvv81]

I think it's good for us. Yes, ERR_NOUSRFOUND for empty user is better than ERR_NOTSUPPORTED.

For the case when either LM or NT challenge response is empty (but username is still set) we will go to full processing but return validation error later (EINVAL) from gssntlm_srv_auth -> ntlmv2_verify_nt_response/ntlmv2_verify_lm_response so this case is also covered but in other way.
So in general your version of the patch is doing the same checks but returns better errors. Thanks!

[simo5]

Thanks for confirming, merged.