Bug 12695 - net ads gpo list doesn't cope with missing attributes.
net ads gpo list doesn't cope with missing attributes.
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Tools
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-15 22:41 UTC by Jeremy Allison
Modified: 2017-03-28 10:13 UTC (History)
1 user (show)

See Also:


Attachments
git-am fix for master. (2.67 KB, patch)
2017-03-15 22:45 UTC, Jeremy Allison
no flags Details
Updated commit message with bugID. (2.72 KB, patch)
2017-03-15 23:01 UTC, Jeremy Allison
gd: review+
Details
git-am fix for 4.6.next, 4.5.next, 4.4.next. (2.84 KB, patch)
2017-03-17 00:01 UTC, Jeremy Allison
gd: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2017-03-15 22:41:24 UTC
When parsing an LDAP GPO object we insist on the following attributes existing:

displayName
flags
gPCFileSysPath
name
ntSecurityDescriptor
versionNumber

If any are not present we fail listing all GPO objects with an "Out of memory" error (which is obviously incorrect).

Patch to follow.
Comment 1 Jeremy Allison 2017-03-15 22:45:46 UTC
Created attachment 13069 [details]
git-am fix for master.

This patch fixes the problem by causing any missing attributes in ads_parse_gpo() to return ADS_ERROR(LDAP_NO_SUCH_ATTRIBUTE).

The interfaces to ads_pull_string() and ads_pull_sd() are broken in that they return NULL for both a talloc fail (out of memory) and also for a missing attribute in the LDAPMessage * pointer, so there's no way to tell the difference between these error cases. This patch causes ads_parse_gpo() to follow the same convention as other uses of ads_pull_string(), ads_pull_sd(), which is to assume a NULL return means missing attribute, not out of memory. Fixing this is a patch for another day though.

Jeremy.
Comment 2 Jeremy Allison 2017-03-15 23:01:14 UTC
Created attachment 13071 [details]
Updated commit message with bugID.
Comment 3 Guenther Deschner 2017-03-16 09:37:22 UTC
Comment on attachment 13071 [details]
Updated commit message with bugID.

LGTM, thanks!
Comment 4 Jeremy Allison 2017-03-17 00:01:31 UTC
Created attachment 13077 [details]
git-am fix for 4.6.next, 4.5.next, 4.4.next.

Cherry-picked from master for 4.6.next, 4.5.next, 4.4.next.
Comment 5 Guenther Deschner 2017-03-17 13:14:10 UTC
Comment on attachment 13077 [details]
git-am fix for 4.6.next, 4.5.next, 4.4.next.

LGTM
Comment 6 Guenther Deschner 2017-03-17 13:15:44 UTC
Karolin, please add to the appropriate branches. Thanks!
Comment 7 Karolin Seeger 2017-03-27 09:42:02 UTC
Pushed to autobuild-v4-{6,5}-test.
Comment 8 Karolin Seeger 2017-03-28 10:13:37 UTC
(In reply to Karolin Seeger from comment #7)
Pushed to both branches.
Closing out bug report.

Thanks!