Bug 14901 - The CVE-2020-25717 username map [script] advice has undesired side effects for the local nt token
Summary: The CVE-2020-25717 username map [script] advice has undesired side effects fo...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.15.2
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL: https://gitlab.com/samba-team/samba/-...
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-11 20:11 UTC by Stefan Metzmacher
Modified: 2021-11-22 16:22 UTC (History)
8 users (show)

See Also:


Attachments
Patches for v4-15-test (28.02 KB, text/plain)
2021-11-16 14:46 UTC, Stefan Metzmacher
slow: review+
Details
Patches for v4-14-test (28.02 KB, text/plain)
2021-11-16 14:47 UTC, Stefan Metzmacher
slow: review+
Details
Patches for v4-13-test (28.02 KB, text/plain)
2021-11-16 14:47 UTC, Stefan Metzmacher
slow: review+
Details
patch from 4.13 backported to 4.10 (29.18 KB, patch)
2021-11-22 08:49 UTC, Andrew Bartlett
abartlet: ci-passed? (abartlet)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Metzmacher 2021-11-11 20:11:33 UTC
The advice from https://www.samba.org/samba/security/CVE-2020-25717.html:

> However there are setups which are joined to an active directory
> domain just for authentication, but the authorization is handled
> without nss_winbind by mapping the domain account to a local user
> provided by nss_file, nss_ldap or something similar. NOTE: These
> setups won't work anymore without explicitly mapping the users!
>
> For these setups administrators need to use the 'username map' or
> 'username map script' option in order to map domain users explicitly
> to local users, e.g.
> 
>  user = DOMAIN\user
>
> Please consult 'man 5 smb.conf' for further details on 'username
> map' or 'username map script'. Also note that in the above example '\'
> refers to the default value of the 'winbind separator' option.

has the side effect that the nt token, which we got from the DC
either via netr_LogonSamLogon* for NTLMSSP or the PAC in the kerberos ticket,
is discarded and a new nt token is constructed based on the mapped local
username via nss modules (e.g. nss_files, nss_ldap).

This is not what admins expect as result of a security updated,
as things like the primary user sid in the domain won't
be added to the token, which will cause problems with
existing file permissions.

So I created patches which allow the behavior to be controlled
by smb.conf options, as a start I added the following to
disable it:

local nt token from nss:* = no

Or you do it for a whole domain called 'DOMAIN':

local nt token from nss:DOMAIN = no

Or for individual users like this:

local nt token from nss:DOMAIN\user = no

Lets assume you only have a single domain with a netbios domain
name 'SAMBA' and you want all users of that domain to be
mapped from 'SAMBA\user' to just 'user', except for the cases
where the username is 'root' or 'ubuntu'.

Then you could use this options:

  username map script = /etc/samba/username_map_script.sh
  local nt token from nss:SAMBA = no

The /etc/samba/username_map_script.sh script would have this content:

#!/bin/bash
  
ACCOUNTNAME="$1"

DOMAINNAME="SAMBA"

case "${ACCOUNTNAME}" in
${DOMAINNAME}\\root)
        # skip
        ;;
${DOMAINNAME}\\ubuntu)
        # skip
        ;;

${DOMAINNAME}\\*)
        echo "${ACCOUNTNAME}" | sed -e 's/[^\\]*\\//'
        ;;
esac

exit 0


This would basically restore the fallback behavior of
'DOMAIN\user' to 'user' in a controlled way and only for
a single domain and not any trusted domains.

https://gitlab.com/samba-team/samba/-/merge_requests/2251
has the initial patch for new options.
Comment 1 Stefan Metzmacher 2021-11-12 08:50:59 UTC
https://gitlab.com/samba-team/samba/-/merge_requests/2253 has some more
improvements under discussion in order to avoid the username map [script]
workaround completely.
Comment 2 Samba QA Contact 2021-11-15 19:02:04 UTC
This bug was referenced in samba master:

bfd093648b4af51d104096c0cb3535e8706671e5
5ea347d3673e35891613c90ca837d1ce4833c1b0
fdbee5e074ebd76d659613b8b7114d70f938c38a
8a9f2aa2c1cdfa72ad50d7c4f879220fe37654cd
494bf7de6ff3e9abeb3753df0635737b80ce5bb7
0a546be05295a7e4a552f9f4f0c74aeb2e9a0d6e
Comment 3 Stefan Metzmacher 2021-11-16 14:46:21 UTC
Created attachment 16996 [details]
Patches for v4-15-test
Comment 4 Stefan Metzmacher 2021-11-16 14:47:00 UTC
Created attachment 16997 [details]
Patches for v4-14-test
Comment 5 Stefan Metzmacher 2021-11-16 14:47:25 UTC
Created attachment 16998 [details]
Patches for v4-13-test
Comment 6 Stefan Metzmacher 2021-11-16 14:53:02 UTC
(In reply to Stefan Metzmacher from comment #0)

The advice from https://www.samba.org/samba/security/CVE-2020-25717.html:

> However there are setups which are joined to an active directory
> domain just for authentication, but the authorization is handled
> without nss_winbind by mapping the domain account to a local user
> provided by nss_file, nss_ldap or something similar. NOTE: These
> setups won't work anymore without explicitly mapping the users!
>
> For these setups administrators need to use the 'username map' or
> 'username map script' option in order to map domain users explicitly
> to local users, e.g.
> 
>  user = DOMAIN\user
>
> Please consult 'man 5 smb.conf' for further details on 'username
> map' or 'username map script'. Also note that in the above example '\'
> refers to the default value of the 'winbind separator' option.

has the side effect that the nt token, which we got from the DC
either via netr_LogonSamLogon* for NTLMSSP or the PAC in the kerberos ticket,
is discarded and a new nt token is constructed based on the mapped local
username via nss modules (e.g. nss_files, nss_ldap).

This is not what admins expect as result of a security updated,
as things like the primary user sid in the domain won't
be added to the token, which will cause problems with
existing file permissions.

Admins typically use this in combination with idmap_nss.

The patches on this bug reports (from 
https://gitlab.com/samba-team/samba/-/merge_requests/2253)
should make it possible to revert any 'username map [script]'
changes, which were added as reaction to the original CVE-2020-25717
advisory! Please note you still need to keep 'min domain uid' options.
Comment 7 Ralph Böhme 2021-11-17 09:14:30 UTC
Reassigning to Jule for inclusion in 4.13, 4.14 and 4.15.
Comment 8 Jule Anger 2021-11-17 14:36:01 UTC
Pushed to autobuild-v4-{15,14,13}-test.
Comment 9 Samba QA Contact 2021-11-17 15:51:07 UTC
This bug was referenced in samba v4-13-test:

a6eddc3bd7a032e1fd3921cd7ea213b5c48f2eab
302bb70ebc9b47d9f1d46212deac17470e64740d
0a56d233bfdb48bb2222891f7abfe054769b2ef2
a40c007fb5574cc781b60ab948477dcd9dd05aab
32ba258cd753301504bdb4a00624053f08373b95
105c6a15effd118d7cfe9dfa7b1ad4faab9fe224
Comment 10 Samba QA Contact 2021-11-17 16:25:12 UTC
This bug was referenced in samba v4-15-test:

38ddd41e9c6314ab37d727ff20ca09426a6d7e89
ebe18e23ba62e99295661584ce72942ce214aa4c
3f009a620a32fc02e26f7056b2c53cb940b7bbd4
ae21fe9c01b50232ca3223cca0096f8001786395
5d5e5a1f3558b52776ada0c1fabfa87c0adafd2d
9bcba58e4d42f6107ad8f9fa3faf892f9426a0ec
Comment 11 Samba QA Contact 2021-11-18 07:55:13 UTC
This bug was referenced in samba v4-14-test:

1bd06f8cb357df0c3f3f25899cda38b6f842c659
8bed2c3f7a970dc8933a5215e2d9ba041c9a8759
f00c993f0c74de38d58766b1050bb13f78b42c9a
9bef6bc6cf027c3b61498b4944388940e23e7a1c
ff3798418e8a77492d50dfd32deed4f11f7ba7ce
8ccb26c679ba0b909cbba654d00797f99580679f
Comment 12 Jule Anger 2021-11-18 08:11:15 UTC
Closing out bug report.

Thanks!
Comment 13 Andrew Bartlett 2021-11-22 08:47:12 UTC
CVE-2020-25727 is incorrectly referenced in the first patch (that is an Alfresco CVE).  It should be CVE-2020-25717.
Comment 14 Andrew Bartlett 2021-11-22 08:49:30 UTC
Created attachment 17013 [details]
patch from 4.13 backported to 4.10

I've backported the patch and run the test locally.  I'm also running a full CI on gitlab on top of the 2021-11 security patches and v4-10-test.