Bug 10490 - pam_winbind fails with kerberos method = secrets and keytab
Summary: pam_winbind fails with kerberos method = secrets and keytab
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Winbind (show other bugs)
Version: 4.1.11
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 12992 (view as bug list)
Depends on:
Blocks: 12505
  Show dependency treegraph
 
Reported: 2014-03-07 21:51 UTC by Michael Saxl
Modified: 2020-12-11 09:34 UTC (History)
9 users (show)

See Also:


Attachments
close keytab explicitly if krb5_kt_start_seq_get fails (538 bytes, patch)
2014-04-23 07:02 UTC, Michael Saxl
no flags Details
new version of the previous patch (620 bytes, patch)
2014-05-02 14:04 UTC, Michael Saxl
no flags Details
new version of the previous patch (620 bytes, patch)
2014-05-02 14:05 UTC, Michael Saxl
no flags Details
new version of the previous patch, state why STRUCT_ZERO is called (699 bytes, patch)
2014-05-02 14:12 UTC, Michael Saxl
no flags Details
Possible patch for master (1.23 KB, patch)
2017-06-24 11:50 UTC, Stefan Metzmacher
no flags Details
Patch for v4-6-test (1.34 KB, patch)
2017-06-29 13:10 UTC, Stefan Metzmacher
slow: review+
asn: review+
Details
Patch for v4-5-test (1.34 KB, patch)
2017-06-29 13:13 UTC, Stefan Metzmacher
slow: review+
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saxl 2014-03-07 21:51:44 UTC
with samba 4.1 (also 4.0) logins do not work if kerberso method is set to

kerberos method = secrets and keytab

it seems to be a permission problem of /etc/krb5.keytab, since doing a chmod 644 /etc/krb5.keytab fixes the problem
Comment 1 Anton D 2014-04-23 05:43:02 UTC
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
Comment 2 Michael Saxl 2014-04-23 06:05:05 UTC
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.
Comment 3 Michael Saxl 2014-04-23 07:02:20 UTC
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
Comment 4 Michael Saxl 2014-05-02 14:02:42 UTC
(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.
Comment 5 Michael Saxl 2014-05-02 14:04:19 UTC
Created attachment 9886 [details]
new version of the previous patch
Comment 6 Michael Saxl 2014-05-02 14:05:26 UTC
Created attachment 9887 [details]
new version of the previous patch

typo
Comment 7 Michael Saxl 2014-05-02 14:12:31 UTC
Created attachment 9888 [details]
new version of the previous patch, state why STRUCT_ZERO is called

state why STRUCT_ZERO is called
Comment 8 3ronco 2014-11-08 16:16:26 UTC
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.
Comment 9 devurandom 2015-03-10 14:40:23 UTC
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.
Comment 10 devurandom 2015-03-10 14:42:19 UTC
P.S: The `chmod g+r /etc/krb5.keytab` workaround still works.
Comment 11 Michael Saxl 2015-03-10 14:55:49 UTC
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.
Comment 12 Michael Saxl 2015-03-10 16:45:49 UTC
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)
Comment 13 devurandom 2015-03-11 08:18:42 UTC
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)?
Comment 14 Michael Saxl 2015-03-11 12:21:11 UTC
> 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
Comment 15 Michael Saxl 2015-03-11 16:39:30 UTC
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
Comment 16 Mathieu Parent 2016-04-03 05:24:36 UTC
Hi,

This patch is currently applied in Ubuntu. Anything preventing it to be merged in samba master?
Comment 17 Stefan Metzmacher 2017-06-24 11:50:16 UTC
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
Comment 18 Michael Saxl 2017-06-25 07:54:12 UTC
(In reply to Stefan Metzmacher from comment #17)
looks good. Comment describes the reason why ZERO_STRUCT is used good enough
Comment 19 Stefan Metzmacher 2017-06-29 13:10:51 UTC
Created attachment 13325 [details]
Patch for v4-6-test
Comment 20 Stefan Metzmacher 2017-06-29 13:13:17 UTC
Created attachment 13326 [details]
Patch for v4-5-test
Comment 21 Andreas Schneider 2017-07-03 07:49:04 UTC
Karolin, please apply the patches to the relevant branches. Thanks!
Comment 22 Karolin Seeger 2017-07-03 09:04:38 UTC
(In reply to Andreas Schneider from comment #21)
Pushed to autobuild-v4-{6,5}-test.
Comment 23 Karolin Seeger 2017-07-05 06:58:11 UTC
(In reply to Karolin Seeger from comment #22)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 24 Stefan Metzmacher 2017-08-28 11:56:49 UTC
*** Bug 12992 has been marked as a duplicate of this bug. ***
Comment 25 forward 2017-08-29 03:16:55 UTC
the patch works for me on 4.5.8, thx.