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

Correct more issues in lgpo #50006

Merged
merged 13 commits into from Nov 2, 2018

Conversation

Projects
None yet
6 participants
@lomeroe
Copy link
Contributor

commented Oct 11, 2018

What does this PR do?

Corrects multiple issues in lgpo:

admx search results were getting clobbered by variable name re-use
a byte version of the string ']' was not being used in a indexof search, which caused exceptions on PY3
detected location of the end of a policy element was not being calculated properly (due to reading the Registry.pol file in a raw format) -- this would cause corrupt Registry.pol files, primarily when policy element data was replaced
Some logging elements were not correctly reporting the exceptions.
Some logging elements were reporting exceptions when a warning may be more appropriate

What issues does this PR fix or reference?

Fixes #49225
Fixes #48782

Previous Behavior

ADMX policies failed to apply properly in some cases

New Behavior

ADMX policies apply properly

Tests written?

Yes

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@salt-jenkins salt-jenkins requested a review from saltstack/team-windows Oct 11, 2018

@lomeroe

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2018

I wrote an attempt at a module test, I couldn't figure out how to get the test suite to run properly on windows to actually verify that my test was 100% functional. Is the additional magic needed to make the test suite run on windows documented somewhere, or am I just that daft?

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

@lomeroe you need to install a few additional modules:

pip install pytest-salt mock

Then to run the tests... assuming you're inside the tests directory:

python runtests.py --run-destructive -n integration.modules.test_win_lgpo
@twangboy

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

Also, I have a PR in to speed up the LGPO module... Could you take a look at it...?

#49919

'''
ret = self.run_function('archive.unzip',
'https://www.microsoft.com/en-us/download/confirmation.aspx?id=55319&6B49FDFB-8E5B-4B07-BC31-15695C5A2143=1',
dest='c:\\windows\\system32')

This comment has been minimized.

Copy link
@twangboy

twangboy Oct 11, 2018

Contributor

The setUp function runs before each individual test... you'll probably want to use the LoaderModuleMockMixin

In your imports:

from tests.support.mixins import LoaderModuleMockMixin

Then in the class add the mixin:

class WinLgpoTest(ModuleCase, LoaderModuleMockMixin):

Then rename the setUp function to setup_loader_modules:

    def setup_loader_modules(self):
        '''
        setup function
         downloads and extracts the lgpo.exe tool into c:\windows\system32
        for use in validating the registry.pol files
        '''
        ret = self.run_function('archive.unzip',
                                'https://www.microsoft.com/en-us/download/confirmation.aspx?id=55319&6B49FDFB-8E5B-4B07-BC31-15695C5A2143=1',
                                dest='c:\\windows\\system32')

@lomeroe lomeroe force-pushed the lomeroe:issue48782_2018.3 branch from ef9cc6c to 83ef726 Oct 17, 2018

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Oct 18, 2018

@twangboy Please merge this when you feel it's ready.

@lomeroe lomeroe force-pushed the lomeroe:issue48782_2018.3 branch from 83ef726 to 748e3cf Oct 18, 2018

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

@lomeroe The setUpClass if failing on Py2:
Command:

python runtests.py --run-destructive -n integration.modules.test_win_lgpo

Py2 results:

*** integration.modules.test_win_lgpo Tests  *************************
 --------  Tests with Errors  ----------------------------------------
   -> setUpClass (integration.modules.test_win_lgpo.WinLgpoTest)  ....
       Traceback (most recent call last):
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 160, in setUpClass
           osrelease_grains = cls().run_function('grains.item', ['osrelease'])
         File "C:\Python27\lib\unittest\case.py", line 189, in __init__
           (self.__class__, methodName))
       ValueError: no such test method in <class 'integration.modules.test_win_lgpo.WinLgpoTest'>: runTest

Errors on Py2

@lomeroe lomeroe requested a review from saltstack/team-core as a code owner Oct 24, 2018

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

@lomeroe Looks like you've got some extra commits in here. Can you rebase this so it's only your commits included?

@lomeroe lomeroe force-pushed the lomeroe:issue48782_2018.3 branch from ad0decb to 7bfebc7 Oct 24, 2018

@lomeroe

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2018

@rallytime hrmm, sorry, not sure how that happend...think I have it fixed now

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

@lomeroe yes, looks good now. Thank you!

@@ -3094,9 +3094,15 @@ def _getAdmlPresentationRefId(adml_data, ref_id):
else:
if etree.QName(p_item.tag).localname == 'text':
if prepended_text:

This comment has been minimized.

Copy link
@DmitryKuzmenko

DmitryKuzmenko Oct 25, 2018

Contributor

I'd better suggest to rewrite this in the following way:

if etree.QName(p_item.tag).localname == 'text':
    prepended_text = ' '.join((text for text in (prepended_text, getattr(p_item, 'text', '').rstrip()) if text))
else:
    prepended_text = ''
@@ -4179,7 +4185,7 @@ def _regexSearchKeyValueCombo(policy_data, policy_regpath, policy_regkey):
b'\00;'])
match = re.search(_thisSearch, policy_data, re.IGNORECASE)
if match:
return policy_data[match.start():(policy_data.index(']', match.end())) + 1]
return policy_data[match.start():(policy_data.index(b']', match.end())) + 2]

This comment has been minimized.

Copy link
@DmitryKuzmenko

DmitryKuzmenko Oct 25, 2018

Contributor

I'm sorry, I'm not aware of this but it would be cool to have an example or just a comment here explaining what's +2.

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

@lomeroe Is there any way to get these tests running in Py2?

lomeroe added some commits Oct 4, 2018

fix clobbering of admx_search_results which was keeping some policies
from being properly detected

add a handful of debug statements to help with debugging admx/adml
policy discovery

validate text attribute exists on item before joining it with prepended
text
capture and print exception information
in _regexSearchKeyValueCombo use byte ']' when searching for end of
policy configuration in registry.pol file (py3 fix), additionally move 2
characters from ']' location to skip ']' and the '\x00' due to the
utf-16-le encoding to find the end of the element

this was causing _regexSearchKeyValueCombo to return the element without
the ending ']\x00' and if/when the element was replaced two ']\x00'
existed at the end of the element
Log a warning message instead of an exception when a SID cannot be
converted to a username (for user rights assignments)

Add a module test file
update test to actually work
update lgpo module to catch another possible case of multiple ADML names

lomeroe added some commits Oct 17, 2018

add fix/test for #50079
ADML display names that have a single match in each Computer/User policy
would not work with the "short" display name, requiring either the
"long" name or the explicit policy name

@lomeroe lomeroe force-pushed the lomeroe:issue48782_2018.3 branch from 7bfebc7 to dc58252 Nov 2, 2018

@lomeroe

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

@twangboy This commit should have added PY2 support for the tests. Is it still failing for you?

@DmitryKuzmenko changes you requested have been made

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Awesome! The tests are running now, but 8 of them are failing... Are you seeing this?

=======================  Overall Tests Report  =======================
*** integration.modules.test_win_lgpo Tests  *************************
 --------  Failed Tests  ---------------------------------------------
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_Access_data_sources_across_domains
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 449, in test_set_computer_policy_Access_data_sources_across_domains
           [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\CurrentVersion\\Internet Settings\\Zones\\3[\s]*1406[\s]*DWORD:1'])
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "Windows Components\Internet Explorer\Internet Control Panel\Security Page\Internet Zone\Access data sources across domains" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\Windows\\CurrentVersion\\Internet Settings\\Zones\\3[\s]*1406[\s]*DWORD:1" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_ActiveHours
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 479, in test_set_computer_policy_ActiveHours
           r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursEnd[\s]*DWORD:19'
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "ActiveHours" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetActiveHours[\s]*DWORD:1" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_ClipboardRedirection
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 287, in test_set_computer_policy_ClipboardRedirection
           [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:1'])
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:1" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_DisableUXWUAccess
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 430, in test_set_computer_policy_DisableUXWUAccess
           [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetDisableUXWUAccess[\s]*DWORD:1'])
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "DisableUXWUAccess" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetDisableUXWUAccess[\s]*DWORD:1" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_NTP_Client
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 196, in test_set_computer_policy_NTP_Client
           r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DELETE'
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "System\Windows Time Service\Time Providers\Configure Windows NTP Client" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*DELETE" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_RA_Unsolicit
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 236, in test_set_computer_policy_RA_Unsolicit
           r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*\*[\s]*DELETEALLVALUES',
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "RA_Unsolicit" configuration, regex "Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:0" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_WindowsUpdate
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 272, in test_set_computer_policy_WindowsUpdate
           r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AllowMUUpdateService[\s]*DELETE'
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "Windows Components\Windows Update\Configure Automatic Updates" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*NoAutoUpdate[\s]*DWORD:1" not found in lgpo output
   ...................................................................
   -> integration.modules.test_win_lgpo.WinLgpoTest.test_set_computer_policy_multipleAdmxPolicies
       Traceback (most recent call last):
         File "C:\\dev\\salt\tests\support\helpers.py", line 129, in wrap
           return caller(cls)
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 346, in test_set_computer_policy_multipleAdmxPolicies
           [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0'])
         File "c:\dev\salt\tests\integration\modules\test_win_lgpo.py", line 144, in _testComputerAdmxPolicy
           self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
       AssertionError: Failed validating policy "Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection" configuration, regex "Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0" not found in lgpo output
   ...................................................................
 ---------------------------------------------------------------------
======================================================================
FAILED (total=13, skipped=0, passed=5, failures=8, errors=0)
=======================  Overall Tests Report  =======================
@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

@lomeroe Nevermind, I reset my environment. All is well.

@twangboy twangboy closed this Nov 2, 2018

@twangboy twangboy reopened this Nov 2, 2018

@twangboy twangboy merged commit 3ffa392 into saltstack:2018.3 Nov 2, 2018

9 of 10 checks passed

jenkins/pr/py2-windows-2016 The py2-windows-2016 job has failed
Details
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
@lomeroe

This comment has been minimized.

Copy link
Contributor Author

commented Nov 3, 2018

@lomeroe

This comment has been minimized.

Copy link
Contributor Author

commented Nov 5, 2018

@twangboy I'm not seeing failures on my test system when running the tests in PY2 or PY3 (this is a win2016 server)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Python Version: 2.7.14 (v2.7.14:84471935ed, Sep 16 2017, 20:25:58) [MSC v.1500 64 bit (AMD64)]
 * Transplanting configuration files to 'c:\users\admini~1\appdata\local\temp\2\salt-tests-tmpdir\config'
 * Current Directory: c:\users\administrator\downloads\salt-issue48782_2018.3\salt-issue48782_2018.3
 * Test suite is running under PID 3888
 * Logging tests on c:\users\admini~1\appdata\local\temp\2\salt-runtests.log
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Setting up Salt daemons to execute tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Starting salt-master ... STARTED!
 * Starting salt-minion ... STARTED!
 * Starting sub salt-minion ... STARTED!
 * Starting syndic salt-master ... STARTED!
 * Starting salt-syndic ... STARTED!
======================================================================
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Starting integration.modules.test_win_lgpo Tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.............
----------------------------------------------------------------------
Ran 13 tests in 400.444s

OK

=======================  Overall Tests Report  =======================
***  No Problems Found While Running Tests  **************************
======================================================================
OK (total=13, skipped=0, passed=13, failures=0, errors=0)
=======================  Overall Tests Report  =======================
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Python Version: 3.5.3 (v3.5.3:1880cb95a742, Jan 16 2017, 16:02:32) [MSC v.1900 64 bit (AMD64)]
 * Transplanting configuration files to 'C:\Users\ADMINI~1\AppData\Local\Temp\2\salt-tests-tmpdir\config'
 * Current Directory: c:\users\administrator\downloads\salt-issue48782_2018.3\salt-issue48782_2018.3
 * Test suite is running under PID 4428
 * Logging tests on C:\Users\ADMINI~1\AppData\Local\Temp\2\salt-runtests.log
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Setting up Salt daemons to execute tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Starting salt-master ... STARTED!
 * Starting salt-minion ... STARTED!
 * Starting sub salt-minion ... STARTED!
 * Starting syndic salt-master ... STARTED!
 * Starting salt-syndic ... STARTED!
======================================================================
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Starting integration.modules.test_win_lgpo Tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.............
----------------------------------------------------------------------
Ran 13 tests in 840.968s

OK

=======================  Overall Tests Report  =======================
***  No Problems Found While Running Tests  **************************
======================================================================
OK (total=13, skipped=0, passed=13, failures=0, errors=0)
=======================  Overall Tests Report  =======================
@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2018

I had something messed up in my envrironment. I reset and they all passed on both Py2 and Py3. I merged this shortly after. Sorry for the misunderstanding and thanks for getting tests for this. It is sorely needed.

@lomeroe

This comment has been minimized.

Copy link
Contributor Author

commented Nov 5, 2018

@twangboy good deal, I see that now in the thread, but missed that update over the weekend...glad we got it all merged in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.