Bug 11961 - idmap_autorid allocates ids for unknown SIDs from other backends
Summary: idmap_autorid allocates ids for unknown SIDs from other backends
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Winbind (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 12597 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-06-10 14:50 UTC by Ralph Böhme
Modified: 2017-04-21 07:03 UTC (History)
4 users (show)

See Also:


Attachments
Patch for master (3.90 KB, patch)
2016-06-10 16:15 UTC, Ralph Böhme
no flags Details
Patch for master (4.80 KB, patch)
2016-06-12 04:41 UTC, Ralph Böhme
no flags Details
Patch for 4.4 cherry-picked from master (10.53 KB, patch)
2016-06-28 10:19 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.3 cherry-picked from master (10.53 KB, patch)
2016-06-28 10:20 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.5 and 4.6 cherry-picked from master (12.87 KB, patch)
2017-04-07 09:28 UTC, Ralph Böhme
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2016-06-10 14:50:23 UTC
Given a config of

  idmap config * : backend = autorid
  idmap config * : range = 1000000-19999999
  idmap config * : rangesize = 100000

  idmap config HILLHOUSE : backend = ad
  idmap config HILLHOUSE : schema_mode = rfc2307
  idmap config HILLHOUSE : range = 100000-200000

$ ./bin/wbinfo -D HILLHOUSE | grep SID
SID: S-1-5-21-3152989960-574718769-2188965058

Running wbinfo -S with a SID from the HILLHOUSE domain that doesn't exist...

$ ./bin/wbinfo -s S-1-5-21-3152989960-574718769-2188965058-66666
failed to call wbcLookupSid: WBC_ERR_DOMAIN_NOT_FOUND
Could not lookup sid S-1-5-21-3152989960-574718769-2188965058-66666

...ends up in idmap_autorid, allocating a uid from the default autorid range:

$ ./bin/wbinfo -S S-1-5-21-3152989960-574718769-2188965058-66666
1036666

Have patch, need bugnumber. :)
Comment 1 Ralph Böhme 2016-06-10 14:51:38 UTC
Hold on, this was found in 4.3, still need to check whether the problem is still there in 4.4 and/or master.
Comment 2 Ralph Böhme 2016-06-10 16:15:53 UTC
Created attachment 12175 [details]
Patch for master
Comment 3 Ralph Böhme 2016-06-12 04:41:52 UTC
Created attachment 12176 [details]
Patch for master

Took me some time, but I think this patch uses a better approach: instead of trying to fix the damage done, just filter out unknown SIDs.

I guess it would be nice to have a test for this, so I'm going to take a stab at it.
Comment 4 Stefan Metzmacher 2016-06-20 05:13:47 UTC
(In reply to Ralph Böhme from comment #3)

I'm not sure relying on LsaLookupNames() is a good thing.

I wouldn't be surprised that we get SID_TYPE_UNKNOWN from our
DC if it is (temporary) unabled to contact a DC of a trusted domain.
Comment 5 Ralph Böhme 2016-06-20 06:01:29 UTC
s/LsaLookupNames/LsaLookupSids/

We depend on the correct results of LsaLookupSids to determine the right winbind domain. If it fails, like in this case for an unknown SID in an existing domain, we use the default domain which can have unexpected results like in the config described above.
Comment 6 Ralph Böhme 2016-06-28 10:19:38 UTC
Created attachment 12228 [details]
Patch for 4.4 cherry-picked from master
Comment 7 Ralph Böhme 2016-06-28 10:20:08 UTC
Created attachment 12229 [details]
Patch for 4.3 cherry-picked from master
Comment 8 Karolin Seeger 2016-08-03 07:53:29 UTC
Pushed to autobuild-v4-[3|4]-test.
Comment 9 Karolin Seeger 2016-08-05 07:37:47 UTC
(In reply to Karolin Seeger from comment #8)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 10 Ralph Böhme 2017-04-05 08:31:29 UTC
*** Bug 12597 has been marked as a duplicate of this bug. ***
Comment 11 Ralph Böhme 2017-04-07 09:28:35 UTC
Created attachment 13145 [details]
Patch for 4.5 and 4.6 cherry-picked from master
Comment 12 Karolin Seeger 2017-04-20 10:25:29 UTC
(In reply to Ralph Böhme from comment #11)
Pushed to autobuild-v4-{6,5}-test.
Comment 13 Karolin Seeger 2017-04-21 07:03:08 UTC
(In reply to Karolin Seeger from comment #12)
Pushed to both branches.
Closing out bug report.

Thanks!