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: ASSIGNED
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: 2021-04-29 18:34 UTC (History)
1 user (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

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).