Bug 12695 - net ads gpo list doesn't cope with missing attributes.
Summary: net ads gpo list doesn't cope with missing attributes.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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!