CVE-2020-25717: s3:auth: let auth3_generate_session_info_pac() delegate everything...
commita152f36b0576737e647dbe5f1954039668123c1f
authorStefan Metzmacher <metze@samba.org>
Mon, 4 Oct 2021 17:42:20 +0000 (4 19:42 +0200)
committerJule Anger <janger@samba.org>
Mon, 8 Nov 2021 09:52:11 +0000 (8 10:52 +0100)
tree4713c3a07305cf7b4755c8743bcbf3e161558541
parent131d5ceb9deaaa1d8dd478a9b2e2556133c511aa
CVE-2020-25717: s3:auth: let auth3_generate_session_info_pac() delegate everything to make_server_info_wbcAuthUserInfo()

This consolidates the code paths used for NTLMSSP and Kerberos!

I checked what we were already doing for NTLMSSP, which is this:

a) source3/auth/auth_winbind.c calls wbcAuthenticateUserEx()
b) as a domain member we require a valid response from winbindd,
   otherwise we'll return NT_STATUS_NO_LOGON_SERVERS
c) we call make_server_info_wbcAuthUserInfo(), which internally
   calls make_server_info_info3()
d) auth_check_ntlm_password() calls
   smb_pam_accountcheck(unix_username, rhost), where rhost
   is only an ipv4 or ipv6 address (without reverse dns lookup)
e) from auth3_check_password_send/auth3_check_password_recv()
   server_returned_info will be passed to auth3_generate_session_info(),
   triggered by gensec_session_info(), which means we'll call into
   create_local_token() in order to transform auth_serversupplied_info
   into auth_session_info.

For Kerberos gensec_session_info() will call
auth3_generate_session_info_pac() via the gensec_generate_session_info_pac()
helper function. The current logic is this:

a) gensec_generate_session_info_pac() is the function that
   evaluates the 'gensec:require_pac', which defaulted to 'no'
   before.
b) auth3_generate_session_info_pac() called
   wbcAuthenticateUserEx() in order to pass the PAC blob
   to winbindd, but only to prime its cache, e.g. netsamlogon cache
   and others. Most failures were just ignored.
c) If the PAC blob is available, it extracted the PAC_LOGON_INFO
   from it.
d) Then we called the horrible get_user_from_kerberos_info() function:
   - It uses a first part of the tickets principal name (before the @)
     as username and combines that with the 'logon_info->base.logon_domain'
     if the logon_info (PAC) is present.
   - As a fallback without a PAC it's tries to ask winbindd for a mapping
     from realm to netbios domain name.
   - Finally is falls back to using the realm as netbios domain name
   With this information is builds 'userdomain+winbind_separator+useraccount'
   and calls map_username() followed by smb_getpwnam() with create=true,
   Note this is similar to the make_server_info_info3() => check_account()
   => smb_getpwnam() logic under 3.
   - It also calls smb_pam_accountcheck(), but may pass the reverse DNS lookup name
     instead of the ip address as rhost.
   - It does some MAP_TO_GUEST_ON_BAD_UID logic and auto creates the
     guest account.
e) We called create_info3_from_pac_logon_info()
f) make_session_info_krb5() calls gets called and triggers this:
   - If get_user_from_kerberos_info() mapped to guest, it calls
     make_server_info_guest()
   - If create_info3_from_pac_logon_info() created a info3 from logon_info,
     it calls make_server_info_info3()
   - Without a PAC it tries pdb_getsampwnam()/make_server_info_sam() with
     a fallback to make_server_info_pw()
   From there it calls create_local_token()

I tried to change auth3_generate_session_info_pac() to behave similar
to auth_winbind.c together with auth3_generate_session_info() as
a domain member, as we now rely on a PAC:

a) As domain member we require a PAC and always call wbcAuthenticateUserEx()
   and require a valid response!
b) we call make_server_info_wbcAuthUserInfo(), which internally
   calls make_server_info_info3(). Note make_server_info_info3()
   handles MAP_TO_GUEST_ON_BAD_UID and make_server_info_guest()
   internally.
c) Similar to auth_check_ntlm_password() we now call
   smb_pam_accountcheck(unix_username, rhost), where rhost
   is only an ipv4 or ipv6 address (without reverse dns lookup)
d) From there it calls create_local_token()

As standalone server (in an MIT realm) we continue
with the already existing code logic, which works without a PAC:
a) we keep smb_getpwnam() with create=true logic as it
   also requires an explicit 'add user script' option.
b) In the following commits we assert that there's
   actually no PAC in this mode, which means we can
   remove unused and confusing code.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14646
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
source3/auth/auth_generic.c