Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gssntlm_acquire_cred_from crashes due SIGSEGV when desired_name=NULL #40

Closed
kvv81 opened this issue Sep 23, 2020 · 5 comments · Fixed by #41
Closed

gssntlm_acquire_cred_from crashes due SIGSEGV when desired_name=NULL #40

kvv81 opened this issue Sep 23, 2020 · 5 comments · Fixed by #41

Comments

@kvv81
Copy link

kvv81 commented Sep 23, 2020

Running samba winbind with gssntlmssp of the latest development version installed (got sources from github + compiled). Immediately after start I get this crash:

Program received signal SIGSEGV, Segmentation fault.
get_creds_from_store (name=name@entry=0x0, cred=cred@entry=0x5555558a6a40, cred_store=cred_store@entry=0x7fffffffd260) at src/gss_creds.c:239
239	src/gss_creds.c: No such file or directory.
(gdb) bt
#0  get_creds_from_store (name=name@entry=0x0, cred=cred@entry=0x5555558a6a40, cred_store=cred_store@entry=0x7fffffffd260) at src/gss_creds.c:239
#1  0x00007fffddb9a487 in gssntlm_acquire_cred_from (minor_status=0x7fffffffce34, desired_name=0x0, time_req=<optimized out>, desired_mechs=<optimized out>, cred_usage=<optimized out>, 
    cred_store=0x7fffffffd260, output_cred_handle=0x7fffffffcd60, actual_mechs=0x0, time_rec=0x0) at src/gss_creds.c:435
#2  0x00007fffede7d160 in gss_add_cred_from () from /lib64/libgssapi_krb5.so.2
#3  0x00007fffede7d7a9 in gss_acquire_cred_from () from /lib64/libgssapi_krb5.so.2
#4  0x00007fffedea622b in get_available_mechs () from /lib64/libgssapi_krb5.so.2
#5  0x00007fffedea643d in spnego_gss_acquire_cred_from () from /lib64/libgssapi_krb5.so.2
#6  0x00007fffede7d160 in gss_add_cred_from () from /lib64/libgssapi_krb5.so.2
#7  0x00007fffede7d7a9 in gss_acquire_cred_from () from /lib64/libgssapi_krb5.so.2
#8  0x00007ffff504fc7f in smb_gss_krb5_import_cred () from /usr/lib64/samba/libkrb5samba-samba4.so
#9  0x00007ffff10b7edf in gensec_gse_client_start () from /usr/lib64/samba/libgse-samba4.so
#10 0x00007ffff18fb91b in gensec_start_mech () from /usr/lib64/samba/libgensec-samba4.so
#11 0x00007ffff18fc71c in gensec_start_mech_by_ops () from /usr/lib64/samba/libgensec-samba4.so
#12 0x00007ffff18eb58d in gensec_spnego_client_negTokenInit_step () from /usr/lib64/samba/libgensec-samba4.so
#13 0x00007ffff18ebb4b in gensec_spnego_client_negTokenInit_start () from /usr/lib64/samba/libgensec-samba4.so
#14 0x00007ffff18ecc61 in gensec_spnego_update_send () from /usr/lib64/samba/libgensec-samba4.so
#15 0x00007ffff18fa940 in gensec_update_send () from /usr/lib64/samba/libgensec-samba4.so
#16 0x00007ffff3015835 in cli_session_setup_gensec_local_next () from /usr/lib64/samba/liblibsmb-samba4.so
#17 0x00007ffff3017287 in cli_session_setup_creds_send () from /usr/lib64/samba/liblibsmb-samba4.so
#18 0x00007ffff30179dd in cli_session_setup_creds () from /usr/lib64/samba/liblibsmb-samba4.so
#19 0x0000555555599727 in init_dc_connection_network ()
#20 0x00007ffff27a4ea4 in messaging_dispatch_classic () from /lib64/libsmbconf.so.0
#21 0x00007ffff27a5f51 in messaging_recv_cb () from /lib64/libsmbconf.so.0
#22 0x00007fffeecff5ba in msg_dgm_ref_recv () from /usr/lib64/samba/libmessages-dgm-samba4.so
#23 0x00007fffeecfd706 in messaging_dgm_read_handler () from /usr/lib64/samba/libmessages-dgm-samba4.so
#24 0x00007ffff5469153 in tevent_common_invoke_fd_handler () from /usr/lib64/samba/libtevent.so.0
#25 0x00007ffff546f587 in epoll_event_loop_once () from /usr/lib64/samba/libtevent.so.0
#26 0x00007ffff546d6a7 in std_event_loop_once () from /usr/lib64/samba/libtevent.so.0
#27 0x00007ffff546889d in _tevent_loop_once () from /usr/lib64/samba/libtevent.so.0
#28 0x000055555557b15c in main ()

As far as I see from backtrace and source code, the problem happens because Kerberos library calls gssntlm_acquire_cred_from with desired_name=NULL and library tries to de-reference it without proper check for NULL.

# 2
status = mech->gss_acquire_cred(minor_status, internal_name, time_req,
                    &target_mechs, cred_usage, &cred, NULL,
                    time_recp);

# 1
 if (cred_usage == GSS_C_INITIATE) {
        if (name != NULL && name->type != GSSNTLM_NAME_USER) {
            set_GSSERRS(ERR_NOUSRNAME, GSS_S_BAD_NAMETYPE);
            goto done;
        }

        if (cred_store != GSS_C_NO_CRED_STORE) {
            retmin = get_creds_from_store(name, cred, cred_store);
        } else {

# 0

static int get_creds_from_store(struct gssntlm_name *name,
                                struct gssntlm_cred *cred,
                                gss_const_key_value_set_t cred_store)
{
    uint32_t i;
    int ret;

    /* special case to let server creds carry a keyfile */
    if (name->type == GSSNTLM_NAME_SERVER) {
@simo5
Copy link
Collaborator

simo5 commented Sep 23, 2020

Uhmm so a cred_store was provided but no name was provided ...
What key/value pairs are actually provided in the cred store ?

We definitely need to resolve the crash, but currently the code assumes that a name needs to be provided if acquire with cred store is provided, so I am trying to figure out what is the right thing to do in this case.

@simo5
Copy link
Collaborator

simo5 commented Sep 23, 2020

i provided a possible patch, please let me know if this will work for you.

@simo5
Copy link
Collaborator

simo5 commented Sep 23, 2020

That said I think there is a bug in samba here.
Samba is looking specifically for krb5 credentials, but is not setting the preferred oid, or checking it, so with gssntlmssp installed it may get back creedntials it is not asking for, I believe.

There is also the chance it will loop back to windbind to obtain NTLM creds where it does not expect so and may lead to a deadlock if this happens within winbindd itself (although I think the windbind client interface prevents it).

@kvv81
Copy link
Author

kvv81 commented Sep 24, 2020

Thank you very much for fast fix, haven't expected it so quick :-)
Will try to test it as soon!

@kvv81
Copy link
Author

kvv81 commented Sep 24, 2020

I have tested the fix (I have done cherry-pick for this commit over the latest version of code because I found that your private branch 'name_crash' misses some recent changes).
I confirm that winbind doesn't crash and it starts properly.

However the basic checks of NTLM functionality have failed due other bug - see #42 .
So I cannot fully verify the changes, but probably we can give green light for this fix.

@simo5 simo5 closed this as completed in #41 Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants