Bug 14687 - GPO restore fails if an empty REG_MULTI_SZ is encountered in registry.pol.xml
Summary: GPO restore fails if an empty REG_MULTI_SZ is encountered in registry.pol.xml
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.11.1
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-14 13:50 UTC by Kacper
Modified: 2024-02-19 10:30 UTC (History)
2 users (show)

See Also:

Attachments
Proposed patch (648 bytes, patch)
2021-04-14 18:31 UTC, Kacper 		no flags Details
Added Registry.pol.xml for testing (2.11 KB, patch)
2021-04-27 22:04 UTC, Kacper 		no flags Details
Patch passing test suite (3.56 KB, patch)
2021-04-29 18:31 UTC, Kacper 		no flags 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 Kacper 2021-04-14 13:50:25 UTC 
Entries in registry.pol.xml can have empty value entries after backup. This leads to an exception during restore and subsequent failure of restoring a GPO.

Sample entry:
<Entry type="7" type_name="REG_MULTI_SZ">
  <Key>SOFTWARE\Policies\Microsoft\Windows\AppPrivacy</Key>
  <ValueName>LetAppsAccessCamera_UserInControlOfTheseApps</ValueName>
  <Value/>
</Entry>

There is a check for empty REG_SZ and REG_EXPAND_SZ but not for REG_MULTI_SZ.
Comment 1 Kacper 2021-04-14 18:31:35 UTC 
Created attachment 16589 [details]
Proposed patch
Comment 2 Jeremy Allison 2021-04-26 19:15:47 UTC 
This looks good to me - but is there a specific registry.pol.xml we can add a test empty value to in order to regression test the fix is good ?

Can we add one to:

source4/selftest/provisions/generalized-gpo-backup/policy/{1E1DC8EA-390C-4800-B327-98B56A0AEA5D}/Machine/Registry.pol.xml

or:

source4/selftest/provisions/generalized-gpo-backup/policy/{1E1DC8EA-390C-4800-B327-98B56A0AEA5D}/User/Registry.pol.xml

for a test ?
Comment 3 Kacper 2021-04-27 22:04:15 UTC 
Created attachment 16598 [details]
Added Registry.pol.xml for testing
Comment 4 Kacper 2021-04-27 22:06:36 UTC 
(In reply to Jeremy Allison from comment #2)
I've updated the patch to include the GPO "Let Windows apps access the camera" which has multiple empty REG_MULTI_SZ values. Should I also update the binary Registry.pol.SAMBABACKUP file?
Comment 5 Jeremy Allison 2021-04-28 16:36:09 UTC 
I think that covers the testing part, yes ? Once it's in that file I think the selftests will pack/unpack it - correct ? If that's so I think this patch is good to go.
Comment 6 Kacper 2021-04-29 18:31:59 UTC 
Created attachment 16601 [details]
Patch passing test suite
Comment 7 Kacper 2021-04-29 18:34:10 UTC 
(In reply to Jeremy Allison from comment #5)

Ran the test suite and the previous patch didn't pass. I've updated the patch so it passes the selftest (Registry.pol.SAMBABACKUP need updating too).
Comment 8 Björn Jacke 2021-12-13 18:01:45 UTC 
hm, selftest still seems to succeed with the extended test but without the fix
Comment 9 Kacper 2024-02-19 10:29:00 UTC 
This bug is fixed in commit d5d96bed02fab78386fad908e4dd18c1adcd4795 (gp_pol: Allow null data for REG_MULTI_SZ). We can go ahead and close this issue.