Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

win_lgpo: Support managing arbitrary registry key/value/data actions in Registry.pol #56013

Closed
lorengordon opened this issue Jan 28, 2020 · 7 comments · Fixed by #62888
Closed
Assignees
Labels
Feature new functionality including changes to functionality and code refactors, etc. help-wanted Community help is needed to resolve this lgpo Windows

Comments

@lorengordon
Copy link
Contributor

Description of Issue

At the moment, the lgpo module uses the policy name or id (plus path) to identify a distinct item in the ADML/ADMX files. We would like to use the key and valueName if we can. This is convenient because it directly indicates where in the registry the setting is configured (ultimately), and interfacing with the registry is rather easier than gpedit.msc. Also, DISA SCAP benchmarks reference the registry key/valueName in scan findings, so supporting key/valueName would make it very easy to take a scan and create/update a policy.

To pull an example from the docs:

PS>Get-ChildItem -Path C:\Windows\PolicyDefinitions -Recurse -Filter *.admx | Select-String "ShellRemoveOrderPrints"

C:\windows\PolicyDefinitions\ICM.admx:661:    <policy name="ShellRemoveOrderPrints_1" class="User" displayName="$(string.ShellRemoveOrderPrints)" explainText="$(string.ShellRemoveOrderPrints_Help)" key="Software\Microsoft\Windows\CurrentVersion\Policies\Explorer" valueName="NoOnlinePrintsWizard">
C:\windows\PolicyDefinitions\ICM.admx:671:    <policy name="ShellRemoveOrderPrints_2" class="Machine" displayName="$(string.ShellRemoveOrderPrints)" explainText="$(string.ShellRemoveOrderPrints_Help)" key="Software\Microsoft\Windows\CurrentVersion\Policies\Explorer" valueName="NoOnlinePrintsWizard">

This entry has 3 policy aliases today:

    policy_aliases:
        - Turn off the "Order Prints" picture task
        - ShellRemoveOrderPrints_2
        - System\Internet Communication Management\Internet Communication settings\Turn off the "Order Prints" picture task

I'm basically asking that the key and valueName from the xml entry also be supported, key="Software\Microsoft\Windows\CurrentVersion\Policies\Explorer" valueName="NoOnlinePrintsWizard"

    policy_aliases:
        - Turn off the "Order Prints" picture task
        - ShellRemoveOrderPrints_2
        - System\Internet Communication Management\Internet Communication settings\Turn off the "Order Prints" picture task
        - Software\Microsoft\Windows\CurrentVersion\Policies\Explorer\NoOnlinePrintsWizard
@lorengordon
Copy link
Contributor Author

hiya @lomeroe, any thoughts on this?

@lorengordon
Copy link
Contributor Author

lorengordon commented Jan 29, 2020

Another use case is when working with an existing registry.pol file, such as from a group policy backup or the Microsoft Security Compliance Toolkit (which is mostly a group policy backup). We want to convert these registry.pol files from the binary format to text so it's diff-able and appropriate for source control. Using the LGPO.exe tool that Microsoft publishes with the SCT, it can read the registry.pol and output the contents to a basic ini format, looking something like this for a single entry:

Computer
Software\Microsoft\Windows\CurrentVersion\Policies\Explorer
NoDriveTypeAutoRun
DWORD:255

Trying to convert that to something this win_lgpo.py module supports is rather confusing. It's not NoDriveTypeAutoRun.

If you search the admx you just get this, and it's not Autorun_Box either.

C:\WINDOWS\system32> Get-ChildItem -Path C:\Windows\PolicyDefinitions -Recurse -Filter *.admx | Select-String "NoDriveTypeAutoRun"

C:\Windows\PolicyDefinitions\AutoPlay.admx:47:        <enum id="Autorun_Box" valueName="NoDriveTypeAutoRun" required="true">

If you open up the admx you can see the whole policy, which then gives you the "name" Autorun, and that does actually work. But doing this for hundreds of settings, repeated for every SCT baseline? It would be so much easier if the module just worked with the registry key and valueName!

@lomeroe
Copy link
Contributor

lomeroe commented Feb 4, 2020

seems reasonable, I don't think it would be too difficult for someone to add this feature

@lorengordon
Copy link
Contributor Author

I didn't think it would be hard, but then I started digging into it, and this code is... dense. Turning out to be rather hard for me to figure out where exactly to fit it in.

I was actually thinking it would be easier to just ignore the adml/admx stuff if a user provides the registry paths. Just write out whatever they provide to registry.pol. The comparison and change detection might need some extra thought with that approach though. So I'm kinda taking this route on my own for now, just updating our custom execution module to re-use some of the private functions in salt's lgpo module...

@Akm0d Akm0d added Feature new functionality including changes to functionality and code refactors, etc. help-wanted Community help is needed to resolve this team-windows Windows labels Feb 7, 2020
@Akm0d Akm0d added this to the Approved milestone Feb 7, 2020
@lorengordon
Copy link
Contributor Author

While re-implementing our custom execution module to utilize the built-in lgpo functionality as much as possible, I realized that my original ask here is not really what we need...

We want to be able to set any, arbitrary registry key/value/data via the lgpo module and write it to the applicable user/machine Registry.pol file. It does not matter whether there is a corresponding ADMX/ADML policy, so it would not be sufficient to lookup the key and valueName from the ADMX. This ends up being a couple orders of magnitude easier than the ADMX lookups.

Here is where we implemented our wrapping custom execution module...

We also discovered a number of bugs in various lgpo functions, and I opened PRs on salt to address a couple of them. For now, we've had to vendor a patched win_lgpo.py since the patches will not be backported and some bugs just could not be monkey-patched.

There's also a bug around DELETE policies, which @twangboy beat me to fixing in master, albeit a much different way than I did it.

There were also some, we'll say, differences in stylistic preferences. Like, comparing policy names in a case-insensitive way. And allowing the policy name from a secedit ini file instead of the policy name used by the NetUserModal functions. Those are not necessarily bugs, but could be considered separate feature requests.

@lorengordon lorengordon changed the title win_lgpo: Use key and valueName to specify ADML/ADMX paths win_lgpo: Support managing arbitrary registry key/value/data actions in Registry.pol Mar 26, 2020
@lomeroe
Copy link
Contributor

lomeroe commented Apr 7, 2020

@lorengordon I've put very little testing into this other than a quick "does it work" (0 regression testing/etc), hopefully it captures what you're thinking...I only took a quick glance at your custom module code...

This function should add data to the registry.pol file, but requires the _writeAdminTemplateRegPolFile function to be edited. The "existing_data" declaration needs to be after the policy_data definition and updated to read the file (otherwise a subsequent run of "lgpo.set" will drop the non-ADMX reg keys -- do you see that today w/arbitrary data added via imports?):

def _writeAdminTemplateRegPolFile(admtemplate_data,
<SNIP>
    policy_data = _policy_info()
    existing_data = _read_regpol_file(
        policy_data.admx_registry_classes[registry_class]['policy_path'])
    policySearchXpath = '//ns1:*[@id = "{0}" or @name = "{0}"]'
<SNIP>
def set_policy_regkey(policy_class,
                      reg_key,
                      reg_valuename=None,
                      reg_valuetype=None,
                      value=None,
                      delete=False,
                      delete_all=False):
    '''
    Add arbitrary registry info to the registry.pol file

    policy_class
        The policy class to add i.e. machine/computer or user

    reg_key
        The registry key path

    reg_valuename
        The registry valuename within key

    reg_valuetype
        The registry value type i.e. REG_DWORD, REG_SZ, etc

    value
        The registry value

    delete
        The specified registry valuename should be deleted

    delete_all
        All registry values in the specified key should be deleted
    '''
    registry = Registry()
    module_policy_data = _policy_info()
    if policy_class.lower() in  ['computer', 'machine']:
        policy_class = 'Machine'
    elif policy_class.lower() in ['user']:
        policy_class = 'User'
    else:
        raise CommandExecutionError('An invalid policy class was specified')
    policy_file_data = _read_regpol_file(module_policy_data.admx_registry_classes[policy_class]['policy_path'])
    encoded_semicolon = ';'.encode('utf-16-le')
    encoded_null = chr(0).encode('utf-16-le')
    if delete_all:
        reg_key = reg_key.encode('utf-16-le')
        search_string = b''.join(['['.encode('utf-16-le'),
                                  reg_key,
                                  encoded_null,
                                  encoded_semicolon,
                                  '**delvals.'.encode('utf-16-le'),
                                  encoded_null,
                                  encoded_semicolon,
                                  chr(registry.vtype['REG_SZ']).encode('utf-32-le'),
                                  encoded_semicolon,
                                  six.unichr(len(' {0}'.format(chr(0)).encode('utf-16-le'))).encode('utf-32-le'),
                                  encoded_semicolon,
                                  ' '.encode('utf-16-le'),
                                  encoded_null,
                                  ']'.encode('utf-16-le')])
    elif delete:
        if reg_valuename:
            search_string = _buildKnownDataSearchString(reg_key=reg_key,
                                                        reg_valueName=reg_valuename,
                                                        reg_vtype=reg_valuetype,
                                                        reg_data=value,
                                                        check_deleted=True)
        else:
            raise CommandExecutionError('A reg valuename must be specified when delete=True')
    else:
        if reg_valuename and reg_valutetype:
            search_string = _buildKnownDataSearchString(reg_key=reg_key,
                                                        reg_valueName=reg_valuename,
                                                        reg_vtype=reg_valuetype,
                                                        reg_data=value,
                                                        check_deleted=False)
        else:
            raise CommandExecutionError('No valuename or valuetype specified')
    if search_string:
        policy_file_data = _policyFileReplaceOrAppend(this_string=search_string,
                                                      policy_data=policy_file_data,
                                                      append_only=False)
        try:
            _write_regpol_data(policy_file_data,
                               module_policy_data.admx_registry_classes[policy_class]['policy_path'],
                               module_policy_data.gpt_ini_path,
                               module_policy_data.admx_registry_classes[policy_class]['gpt_extension_location'],
                               module_policy_data.admx_registry_classes[policy_class]['gpt_extension_guid'])
        # TODO: This needs to be more specific or removed
        except CommandExecutionError as exc:  # pylint: disable=broad-except
            log.exception('Unhandled exception occurred while attempting to '
                          'write Adm Template Policy File.\nException: %s', exc)
            return False
    else:
        log.error('No data to put in registry.pol file')
        return False
    return True

Adding an arbitrary registry entry:

salt-call --local lgpo.set_policy_regkey "machine" "Software\MyKey" "MyValue" "REG_DWORD" 1

Deleting an arbitrary value

salt-call --local lgpo.set_policy_regkey "machine" "Software\MyOtherKey" "some_val" delete=True

Delete all values under a key

salt-call --local lgpo.set_policy_regkey "machine" "Software\MyKey" delete_all=True

I seriously spent about 30 minutes on this, so it could totally bork things up :) but maybe gets you in the right direction

@twangboy twangboy added the lgpo label May 5, 2022
@twangboy twangboy modified the milestones: Approved, Sulphur v3006.0 May 5, 2022
@twangboy twangboy self-assigned this May 5, 2022
@twangboy
Copy link
Contributor

twangboy commented Oct 4, 2022

I think this approach could help with some of the localization issues we're having as the registry isn't localized. I think I'm going to take a stab at this. Probably steal your code @lomeroe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. help-wanted Community help is needed to resolve this lgpo Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants