Bug 13683 (CVE-2018-16857) - [SECURITY] CVE-2018-16857 Bad password count not effective for default (30min) window
Summary: [SECURITY] CVE-2018-16857 Bad password count not effective for default (30min...
Alias: CVE-2018-16857
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.9.2
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on:
Blocks: 13663
  Show dependency treegraph
Reported: 2018-11-12 22:17 UTC by Andrew Bartlett
Modified: 2018-11-29 07:52 UTC (History)
6 users (show)

See Also:

draft advisory (2.68 KB, text/plain)
2018-11-12 22:51 UTC, Andrew Bartlett
no flags Details
Proposed patch for master (10.70 KB, text/plain)
2018-11-13 01:26 UTC, Tim Beale
abartlet: review+
Proposed changes for v4.9 (55.04 KB, text/plain)
2018-11-13 02:47 UTC, Tim Beale
abartlet: review+
draft advisory (v2) (3.26 KB, text/plain)
2018-11-13 04:37 UTC, Tim Beale
abartlet: review+
advisory with CVE (v3) (3.28 KB, text/plain)
2018-11-13 17:44 UTC, Andrew Bartlett
timbeale: review+
abartlet: review+
patch for master with CVE (v2) (11.02 KB, patch)
2018-11-13 22:20 UTC, Andrew Bartlett
timbeale: review+
abartlet: review+
backported patch for 4.9 with CVE (v2) (55.35 KB, patch)
2018-11-13 23:03 UTC, Andrew Bartlett
abartlet: review+
backported patch for 4.9 with CVE (v3) (55.37 KB, patch)
2018-11-14 04:35 UTC, Tim Beale
timbeale: review+
dbagnall: review+
advisory with CVE (v4) (3.28 KB, text/plain)
2018-11-25 23:38 UTC, Andrew Bartlett
gary: review+
gary: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2018-11-12 22:17:05 UTC
Reporter: Isaac Boukris

By default, Samba will remember bad passwords for 30min:

$ samba-tool domain passwordsettings show
Reset account lockout after (mins): 30

This is also known as the 'bad password observation window' and is configured in the lockOutObservationWindow attribute on the domain DN.

If this value is set to more than 15mins, the effective value to the Samba code counting bad passwords is 0, meaning no bad passwords are counted.
Comment 1 Andrew Bartlett 2018-11-12 22:18:19 UTC
Setting to block the next security release as patches are expected very soon.
Comment 2 Andrew Bartlett 2018-11-12 22:51:00 UTC
Created attachment 14644 [details]
draft advisory
Comment 3 Tim Beale 2018-11-13 01:26:34 UTC
Created attachment 14645 [details]
Proposed patch for master

Attached fix for master.

CI link: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/120567
Comment 4 Tim Beale 2018-11-13 02:47:31 UTC
Created attachment 14646 [details]
Proposed changes for v4.9

Attached changes backport for v4.9. (A partial backport of some PEP8/test-code changes, and then the exact same master patches applied on top).

CI link: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/120593
Comment 5 Tim Beale 2018-11-13 04:37:09 UTC
Created attachment 14647 [details]
draft advisory (v2)

Updated the draft advisory. It's actually any setting above 3 minutes that's incorrect, although the behaviour above 3 minutes is a bit unpredictable.

Initially we thought the main problem was the attribute string was too long (for settings > ~16.5 mins), so ldb_msg_find_attr_as_int() was just returning the default (zero). However, for values between ~3.5 and ~16.5 minutes ldb_msg_find_attr_as_int() also has an integer overflow problem where it tries to convert a 64-bit signed integer to a 32-bit signed integer. 

Below is the --reset-account-lockout-after setting on the left, and the actual value the code tries to use on the right. (Where the negative numbers are as effective as zero).

3 mins = 180 secs (OK)
4 mins = -189 secs
5 mins = -129 secs
6 mins = -69 secs
7 mins = -9 secs
8 mins = 50 secs
9 mins = 110 secs
10 mins = 170 secs
11 mins = -198 secs
12 mins = -138 secs
13 mins = -78 secs
14 mins = -18 secs
15 mins = 41 secs
16 mins = 101 secs
17+ mins = 0 secs
Comment 6 Karolin Seeger 2018-11-13 08:30:38 UTC
This one needs to be addressed in the upcoming sec releases?
We are already late in the process to add vendors. Waiting for patches/advisory here would mean to set a new release date.
Delay or go ahead without this one?
Comment 7 Andrew Bartlett 2018-11-13 08:55:56 UTC
(In reply to Karolin Seeger from comment #6)
Do you want to do two security releases a week apart?

I'm happy with the patches, just need to check the CI links when I'm in office in the morning.

In terms of timeframes, I expect by the end of my work day tomorrow we will have the CI result and the CVE from Red Hat and I'll re-spin all three attachments with those.

Therefore I suggest we wait for this.  Security releases cost our vendors a lot, we shouldn't have two for the AD DC within a fortnight.
Comment 8 Andrew Bartlett 2018-11-13 17:44:07 UTC
Created attachment 14654 [details]
advisory with CVE (v3)
Comment 9 Andrew Bartlett 2018-11-13 17:44:43 UTC
I'll add the CVE to the patches when I get to the office.
Comment 10 Andrew Bartlett 2018-11-13 22:20:03 UTC
Created attachment 14655 [details]
patch for master with CVE (v2)
Comment 11 Andrew Bartlett 2018-11-13 23:03:23 UTC
Created attachment 14656 [details]
backported patch for 4.9 with CVE (v2)
Comment 12 Tim Beale 2018-11-14 04:35:06 UTC
Created attachment 14657 [details]
backported patch for 4.9 with CVE (v3)

For the v4.9 attachment, it had the Reviewed-by tags, but not the CVE number. I've attached an updated patch, with the CVE in the commit headlines.

For the backported dependencies to the test file, I wasn't sure if they should include the CVE or not (I ended up including it, after consulting with Douglas).
Comment 13 Andrew Bartlett 2018-11-14 19:49:36 UTC

This is all good to go.

Comment 14 Karolin Seeger 2018-11-16 10:37:18 UTC
Opening bug report for vendors.
Planned release date is Tuesday, November 27 2018.
Comment 15 Andrew Bartlett 2018-11-25 23:38:31 UTC
Created attachment 14686 [details]
advisory with CVE (v4)

I've updated the CVSS score to include the numerical score and to re-calibrate it to be more in line with similar issues in the past, specifically:


This was a similar issue (on Windows/Samba) where the bad password count was bypassed.  These got scores of 5.4 and 5.0 on the CVS/2.0
Comment 16 Karolin Seeger 2018-11-27 09:47:18 UTC
Samba 4.9.3 has been shipped to address this defect.
Comment 17 Karolin Seeger 2018-11-27 09:48:19 UTC
Pushed to autobuild-master.
Comment 18 Karolin Seeger 2018-11-29 07:52:24 UTC
(In reply to Karolin Seeger from comment #17)
Pushed to master.
Closing out bug report.