Another workaround that doesn't require making your keytab world readable:

Using krb5_ccache_type=KEYRING instead of FILE in your pam_winbind config.

And using "kerberos method = secrets only" seems to (I haven't fully tested it) now work well enough in Samba 4.x for me where it didn't in Samba 3.x.

Seem like a partial reversal of symptoms from here:
https://bugzilla.samba.org/show_bug.cgi?id=6833

This workaround only applies to mit kerberos, but does not work with heimal kerberos (unknown ccache type KEYRING:user:..).

The reason I use keytab is nfs (machine account is used for mounting), so secrets only is not a solution for me.

Created attachment 9867 [details]
close keytab explicitly if krb5_kt_start_seq_get fails

Now i digged the source and I discovered the following:

source3/librpc/crypto/gse_krb5.c:
400: smb_krb5_open_keytab(krbctx, NULL, false, &keytab); // succeeds "always", even if the keytab file is missing

413: ret = krb5_kt_start_seq_get(krbctx, keytab, &kt_cursor); //this fails, go to out

478: krb5_kt_end_seq_get(krbctx, keytab, &kt_cursor); //here the crash seems to be triggered

(In reply to comment #3)
> Created attachment 9867 [details]
> close keytab explicitly if krb5_kt_start_seq_get fails
> 
> Now i digged the source and I discovered the following:
> 
> source3/librpc/crypto/gse_krb5.c:
> 400: smb_krb5_open_keytab(krbctx, NULL, false, &keytab); // succeeds "always",
> even if the keytab file is missing
> 
> 413: ret = krb5_kt_start_seq_get(krbctx, keytab, &kt_cursor); //this fails, go
> to out
> 
> 478: krb5_kt_end_seq_get(krbctx, keytab, &kt_cursor); //here the crash seems to
> be triggered

I have digged the function krb5_kt_start_seq_get and saw the following (on Ubuntu 14.04)

samba is linked against libkrb5-26-heimdal. Looking at the source of heimdal in the file "keytab_file.c" the function krb5_kt_start_seq_get is implemented (as fkt_start_seq_get_int)

in this function the file gets opened, in this case (as it is not running as root, see winbindd.c:641) it fails and fd is written somewhere in the cursor, so we will see the memcmp call zero_csr failing later.

But the thing I wanted to show is that if krb5_kt_start_seq_get fails no resources are reserved that have to be cleaned up by krb5_kt_end_seq_get, so this patch does not introduce a memory leak.

Created attachment 9886 [details]
new version of the previous patch

Created attachment 9887 [details]
new version of the previous patch

typo

Created attachment 9888 [details]
new version of the previous patch, state why STRUCT_ZERO is called

state why STRUCT_ZERO is called

Bug remains at least with a Debian Jessie Setup using Samba 4.1.13+dfsg-2 being a domain member authenticating against a Win 2003 R2 Server. In my case Kerberos logins via GDM and at console fail:

Nov  8 11:54:48 myHost gdm-password]: pam_unix(gdm-password:auth): authentication failure; logname= uid=0 euid=0 tty= ruser= rhost=  user=JohnDoe
Nov  8 11:54:48 myHost gdm-password]: pam_winbind(gdm-password:auth): getting password (0x00000388)
Nov  8 11:54:48 myHost gdm-password]: pam_winbind(gdm-password:auth): pam_get_item returned a password
Nov  8 11:54:48 myHost gdm-password]: pam_winbind(gdm-password:auth): request wbcLogonUser failed: WBC_ERR_AUTH_ERROR, PAM error: PAM_SYSTEM_ERR (4), NTSTATUS: NT_STATUS_CONNECTION_DISCONNECTED, Error message was: NT_STATUS_CONNECTION_DISCONNECTED
Nov  8 11:54:48 myHost gdm-password]: pam_winbind(gdm-password:auth): internal module error (retval = PAM_SYSTEM_ERR(4), user = 'JohnDoe')

I did a chmod g+r /etc/krb5.keytab
It seems to be sufficient that the krb5.keytab file is accessible by the root group. Sounds silly but worked for me as a temporary fix wihtout making the file readable to the world and without using krb5_ccache_type=KEYRING.

Any news on this? It seems I am now running into the issue on Debian 8/Jessie and Samba 2:4.1.17+dfsg-1.

P.S: The `chmod g+r /etc/krb5.keytab` workaround still works.

I think the chmod g+r /etc/krb5.keytab is not that bad.

What I think happens is the following:
pam connects to winbindd pipe

winbindd forks, changes to the uid of the user that is logging in.
It seems to change only uid. The gid, or at least groups seems to contain gid 0. The chmod g+r would not work.

in source3/librpc/crypto/gse_krb5.c the system keytab will be opened.
if krb5_kt_start_seq_get failes (happens if the current process is unable to open the system keytab), the "cleaning" is too agressive (See comments and patch why).

My patch does only handle that specific case.

Sorry, in my previous comment I did some mistake (on why chmod g+r works)

In winbindd_pam.c there is the following:

        /************************ ENTERING NON-ROOT **********************/

        if (user_ccache_file != NULL) {
                set_effective_uid(uid);
                DEBUG(10,("winbindd_raw_kerberos_login: uid is %d\n", uid));
        }

        result = kerberos_return_pac(mem_ctx,
                                     principal_s,
                                     pass,
                                     time_offset,
                                     &ticket_lifetime,
                                     &renewal_until,
                                     cc,
                                     true,
                                     true,
                                     WINBINDD_PAM_AUTH_KRB5_RENEW_TIME,
                                     NULL,
                                     &logon_info);
        if (user_ccache_file != NULL) {
                gain_root_privilege();
        }

        /************************ RETURNED TO ROOT **********************/

here you can see that gid is not touched, only set_effective_uid (this is system dependend. I think on linux it does use setresuid by default)

Ok, two questions:

1) If gid was to be changed, too, then the system keytab could not be accessed at all anymore? Which would mean the whole thing would fail for sure?
2) How does this relate to the zeroing of the struct (your patch)?

> 1) If gid was to be changed, too, then the system keytab could not be accessed at all anymore? Which would mean the whole thing would fail for sure?

yes, it would fail

> 2) How does this relate to the zeroing of the struct (your patch)?

The root cause is that there is an assumtpion that krb5_kt_start_seq_get does not touch kt_cursor if it fails.

Usually the kt_cursor struct contains the fd of the keytab (in the case of FILE:...). What the heimdal developers do is try to open the keytab file and use kt_cursor as storage for the fd.
if open fails, this fd is -1, so the whole struct is not 0 anymore. It is true in this case that kt_cursor does not reference some resources (so no krb5_kt_end_seq_get is needed, in fact, calling this will crash the application).

winbindd calls this function some lines late (after the out)
                ZERO_STRUCT(zero_csr);
                if ((memcmp(&kt_cursor, &zero_csr,
                            sizeof(krb5_kt_cursor)) != 0) && keytab) {
                        krb5_kt_end_seq_get(krbctx, keytab, &kt_cursor);
                }

So there are two possibilities:
1) say to heimdal developers they must not change the kt_cursor if krb5_kt_start_seq_get fails (But again, I see no documentation that would say the passed kt_cursor keeps the same if the whole function fails)
2) handle this case in samba. Doing this like I propose is only one possibility, but its clearly wrong to assume that if krb5_kt_start_seq_get fails kt_cursor is 0

Just one note
technically secrets and keytab means that samba uses both the internal secrets and system keytab file for keytab storage.

secrets is in memory (so this works even if changing uid).
keytab on the other hand is only opened when needed.

This patch does not solve the fact that only secrets will be used when not running as root. I theory it will also crash if someone removes the system keytab, regardless of the uid.

The major reason someone uses secrets and keytab is (or at least I use it that way) that net ads keytab is quite handy, especially if you use nfs and ssh sso.

I think you might have problems* if you use "kerberos method = system keytab", since in that case winbindd will not have access to any keytab.

* I have not tested. Interrestingly kinit does not need the system keytab at all. I do not know why winbindd needs it in that case

Hi,

This patch is currently applied in Ubuntu. Anything preventing it to be merged in samba master?

Created attachment 13308 [details]
Possible patch for master

Hi Michael,

would the attached patch be ok for you?

Sorry that it took so long...

Thanks!
metze

(In reply to Stefan Metzmacher from comment #17)
looks good. Comment describes the reason why ZERO_STRUCT is used good enough

Created attachment 13325 [details]
Patch for v4-6-test

Created attachment 13326 [details]
Patch for v4-5-test

Karolin, please apply the patches to the relevant branches. Thanks!

(In reply to Andreas Schneider from comment #21)
Pushed to autobuild-v4-{6,5}-test.

(In reply to Karolin Seeger from comment #22)
Pushed to both branches.
Closing out bug report.

Thanks!

*** Bug 12992 has been marked as a duplicate of this bug. ***

the patch works for me on 4.5.8, thx.