-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Feature Request: Manage Windows Local Security Policy Settings #21485
Comments
Great idea. |
Was thinking about something like this for quite a while - thanks for providing these great resources, this should be a good start for anyone who finds time to implement this. |
@lorengordon great idea! Could you provide some samples of what you think a cli execution module and also a state should look like? Including some actual settings you'd like to configure? So on the command line the execution module would look something like this:
And the state (sls file) would look something like this:
Does that make sense? That would help make sure we build it in a way that makes sense if we had some examples. |
Yes, of course. I apologize in advance for the length of this comment... I've been thinking about this a little bit more, as well. I'll discuss the module first, then the state when I get a bit more time to think about it. It probably will help to study the secedit command line usage. One approach for a secedit module would be to mirror the command line usage to whatever degree makes sense. To configure a setting, secedit reads it from an .inf file. I'm not aware of any way to pass in a setting as a string on the command line. The .inf file has a fairly specific syntax. A yaml-to-inf parser may come in handy for templating and managing settings in bulk. A sample .inf is at the bottom of this comment. Notice the 'sections' in the .inf... The
As it is important for the setting to be located within the correct 'section' of the .inf, I think that mapping of setting to section will need to be exposed in the interface to a All that said, I could see cli usage something like this (notice how the 'Registry Values' section requires a 'type' parameter):
(Alternatively, each 'section' could be implemented as a separate function. Or that could be an internal implementation detail, with Each of those cli commands would have to create a temp.inf file with the corresponding information in the correct format, including the
Notes on that command:
That would work for configuring a single setting at a time, but performance could be much improved by aggregating all the settings into a single .inf for a single call to secedit.exe. I don't really know how to do that with a clean data model... (or would this best be handled in the state structure?) While messy, one possible implementation would be for
sample .inf:
|
Oh, and for whoever starts down this path, it's probably worth warning: as valuable as secedit is, being pretty much the only tool available to do the job, it's also a horrible mess of brokenness. See this rant for many of the gotchas. |
I've got a module started on this...mainly worked the "get" portion, though I've tested out the basics of the "put" function in my first test functions (set_password_policy/set_lockout_policy) https://github.com/lomeroe/salt/blob/add-windows-pol-mgmt/salt/modules/win_secpol.py Wanted to see what others thought while I was working on finishing it up.... example output: |
Awesome! Testing it now... |
Works like a charm. I think it would be great to get it in the develop branch. One thing I might change first is to set |
Sounds good - I wasn't even sold on the "secpol" name and wondered if I could come up with something better...I will make that mod though. Once I have the 'set' working, I'll do a PR into develop. |
I am a little curious about the reasoning and differences behind the 'How' mechanisms, Registry vs NetUserModal vs Secedit. I had really only envisioned secedit-based policies, specifically because they are applied to the local security policy and enforced at a lower level than normal registry edits. It seems like all the |
Sure - Anytime there is an API to do something, I will likely use it over calling out to another application (like secedit), that's just personal preference. With this module, I started off needing to set the lockout/password policies on some non-domain members. I was able to do everything with the API (NetUserModalGet/Set) until you get the the reversible encryption and password complexity settings (the API for these is apparently in SAM lib which pywin32 doesn't have hooks into, and even documentation for C/.NET is very limited), so I couldn't find any other way than secedit. Once I had that basic code for secedit, and I came upon your feature request, I thought I could augment it to do just about everything you could do w/gpedit.msc. As for the registry "how" - when secedit is just going to make a registry modification for me, I didn't see the purpose it calling out to it, when I can cut out the middle man and make the same modification myself (like using the API). For example, setting the "Network Access: Do not allow anonymous enumeration of SAM accounts and shares" policy in the GUI just sets the "RestrictAnonymous" registry key. Just like secedit will if it is in the [Registry Values] section of an INI file... To directly answer your questions: Different mechanisms because that's just how I chose to do it (right/wrong is rightfully debatable). :) NetUserModalsSet does modify the policy (likely secedit calls the same underlying windows API). You can try the set_password_policy and set_lockout_policy functions to see NetUserModalSet change the policy... |
Gotcha. I'll give those functions a try. But, I don't think that's right about registry entries and secedit. My understanding is that secedit does not modify the registry directly. Secedit modifies the local security policy, which in turn modifies the registry. I believe registry-based local security policy is actually stored in registry.pol files with a binary format, not just the registry. So I think registry entries supported by secedit need to be applied by secedit. Otherwise, the registry entries will be overridden when the policy refreshes (assuming that entry has been set in policy). To see what I mean, use gpedit.msc to modify the security policy. Then use regedit.exe to manually modify the corresponding registry entry. Then run |
Hrm....If I set the 'restirctanonymous' reg key it doesn't change after a gpupdate /force The gpedit.msc also shows the policy change from Disabled to Enabled if I just flip the registry key only... Same goes for the 'dontdisplaylastusername' registry key There could be other keys that behave differently...which policy setting did you try? |
Ahh, yeah, Windows is so annoying about these things. They call it a 'registry policy' but they're specifically referring to settings that fall under the Administrative Templates section of the policy in gpedit.msc. |
The link you provided does say that they 'get imported on update' from the .pol files, so it is possible that upon reboot something else could happen... Looking at the registry.pol file on the system I'm working on, it looks like it only has "administrative template" settings in it. That makes sense to me, b/c I have salt states that set some of those values, but gpedit.msc doesn't show them as configured (though the reg changes are in effect) If administrative template configuration is added to this module (which I'd like to do), then it appears to make the gui/registry line up, the registry.pol file would need to be updated as well as the registry itself... |
I really hate trying to configure Windows. It's so inconsistent and opaque. |
That is the truth... I committed some more changes, so the "set" function should be functional (added a couple more policies, but not many). Give it a try and see what you think... I'll try to finish adding other policies to it next week and build a state module. |
Ok, I withdraw my concern about secedit vs direct registry modifications. I did some more testing and it turns out I was remembering how another tool behaves, rather than secedit: the LGPO utility I linked in the first post. I'm not really sure how it does it, but policies applied with LGPO override changes I make directly to the registry or via gpedit.msc. Any changes revert to what was set via LGPO, once policy refreshes (or forcing a refresh with Anyway, tested the |
Also, as far as the administrative template configuration, it can be applied with secedit. The [Registry] section of a secedit inf may contain entries for settings within the administrative template sections. Or perhaps it could be handled by modifying the registry directly, as well. I'll have to do a little more testing to see whether changes to the registry directly will be reflected in the administrative template section of the security policy, or if configuring the security policy will override changes made to the registry. |
Ok, I see now what you meant about the registry and administrative template settings. Did some testing and can confirm that modifying the registry entry corresponding to an administrative template setting does not update what's displayed in gpedit.msc. Worse, I tried using secedit to configure the administrative template setting from an inf, and while it did change the registry entry it did not change what was displayed in gpedit.msc. When I ran So, however LGPO is doing it may be the only way. I'm not really great at reading cpp, but they do provide the source. I'll try to struggle through it when I get some time (may not get to it for a couple weeks). |
@lorengordon - got sidetracked for a few days, but was able to spend a little time on it again today...I added User Rights Assignments to the 'set' and 'get', I also renamed to 'gpedit', just seemed appropriate to me (since I kept running 'gpedit' to double-check output), not married to it though.... Give it a try and let me know what you think.... https://github.com/lomeroe/salt/blob/add-windows-pol-mgmt/salt/modules/win_gpedit.py |
I'm out for the next couple days, but can test it early next week. I'm relatively unconcerned about the name, as long as it works. But if I were forced to choose between gpedit, secpol, or secedit, I think I like secpol the best. Especially if it manages to implement functionality equivalent to LGPO, eventually, which seems to work at a lower level than gpedit or secedit. |
@lomeroe: I never did get around to test this any further. Are you still working on it? Since we were already using Apply_LGPO_Delta.exe, I just wrote a custom execution module wrapper around it. First one I've written, turned out to be pretty easy. I'd appreciate any feedback, pointers, etc, if anyone has time to take a look: https://gist.github.com/lorengordon/a1d1c08e8587d92d3612. |
No, I never finished up the module/state I was working on, the things I needed it for got back-burner'd and I unfortunately had to put off finishing it up. It is on my list of things to accomplish and I anticipate it to bubble back towards the top of my to-do list soon, may be moot with the module you've done. |
Ok, no worries. I've added support for secedit-based inf-style policies ("System Access" and "Privilege Rights"), in addition to the registry-based policies. I'd like to do something like you did with the policy_info class to define the data model and help validate values, but it works. https://gist.github.com/lorengordon/a1d1c08e8587d92d3612 |
Well, it turned out to be fairly easy to use a class to define most of the data structure and various helper functions. That's likely about as far as I can take this myself at the moment. Is it worth trying to convert or import or otherwise utilize the .cpp source from Apply_LGPO_Delta so it can be used as a library in python, to eliminate the external dependency on the .exe? I have no idea how to go about that, and my Google searches are not proving particularly enlightening. |
@Trouble123, the virtual name for the module is just |
Their is a lot of code in this. Was it auto generated, i.e. how will it be maintain ?
|
Thanks, I guess copying the .py file was OK? |
@Trouble123, try And yes, you should be able to just copy the file like that into (site|dist)-packages (amzn linux?), though perhaps more typically for salt you would place it in a |
@damon-atkins it is unfortunately statically defined/manually maintained, I was previously not able to find it defined elsewhere. I'm sure some windows dll has the similar data that the mmc snap-in is using, but I couldn't find it or how to get at it. @lorengordon asked for documentation on the class to be added, which I will work up and put in while I'm working on the state module. It did turn into quite a behemoth to make things work how I envisioned the module functioning. Luckily (maybe?), I think most of the items in that class have not changed (or changed much) since Windows 2000. edited to add: Not all of the security settings policies have been added to the class either... |
Hi I have yet to get this to work. I tried to now run it locally on a Windows 2012R2 DC that i have synced the module to and i get this error: `
Any ideas? thanks |
@Trouble123 not exactly sure what is happening, but when sys.doc is called or when "-d" is passed to the module (salt-call lgpo -d) to get the doc string, powershell is executed and the ServerManager module is imported (the lgpo module doesn't use any powershell): ''' That seems to be what is throwing the error you are seeing. What is the output of "c:\salt\salt-call --versions-report"? I don't believe the 32-bit version of powershell has the ServerManager module, so it is possible that somehow a 32-bit version of powershell.exe is being loaded (if you are on a 64-bit OS)... |
Hi, sorry for the delay. Here is what i get when running it on the W2012R2 server: Salt Version: Dependency Versions: System Versions: |
@Trouble123 that looks like the output from the master, not the minion itself. However, I can duplicate your error message by running the 32-bit minion on a 64-bit 2012 R2 server. If you do have a 64-bit minion installed, then I'm not sure what is causing the error. I'd suggest uninstalling the minion and re-download the minion installer, ensuring you are getting the 64-bit version and re-install it. |
@lomeroe, it appears that the |
ServerManager for PowerShell is only valuable if you install extra Microsoft software. I recently fix some other powershell salt modules in Carbon. It's not something Salt should install. |
@damon-atkins, what is it that you are responding to? Are you saying that |
My comments are about this error.
|
Yeah, still pretty sure that error is not relevant to the Not sure what is trying to import the ServerManager powershell module in salt 2016.3...haven't dug into that. |
It runs all the time, its in def virtual it how it determines if it is valid module. If the import failed it returns False. Unfortunately cmd.run_all outputs the error to the log. |
What module are you talking about? I don't see any call here, https://github.com/saltstack/salt/blob/develop/salt/modules/win_lgpo.py#L2634-L2640. Is it in the
|
Not the |
Ahh, I wonder if it happens when a module cannot be found... I'm guessing in that case the LazyLoader iterates through every module attempting to find the requested module, hits In which case, still, this particular error message is not related directly to the Non-existent module
Real module
|
This should do the trick. If you are behind a proxy, add the proxy="" option
The 3.6.0 version of lxml must be specified as 3.6.4 (latest lxml in pypi today) does not have a binary windows package currently. @twangboy thoughts on adding lxml to the windows salt package? If you don't think that should be done, then I'll add documentation to the module similar to the above note... |
@lomeroe, yep, that did it. Still seeing the ServerManager import error, but I think that's more related to the ordering of the modules when the loader is iterating through them all to find the |
Submitted a new issue for the issue with the ServerManager PowerShell import... |
@lorengordon ServerManager its not in your code. It was a response to @Trouble123 It will always show in in the logs if its not installed, it can be ignored. Sorry if this was not clear. |
@lorengordon C:\salt>salt-call --versions-report Dependency Versions: System Versions: I looked at the exe that i installed it from, and its definitely the 64bit: Salt-Minion-2016.3.1-AMD64-Setup.exe I ran the command on another Win 2012R2 DC, and it gave me the result without the error. I then copied the win_lgpo.py & pyc file back into the _modules folder, and did a salt 'MINION' saltutil.sync_all. I can see it has sync'd: MINION: But, still have not been able to run any commands against it. salt 'MINION' sys.doc lgpo - gave me nothing So i looked at the posts on this issue from 12 months ago and ran the following; still no joy C:\salt>salt-call lgpo.get return_full_policy_names=True -l debug |
@Trouble123 sorry I sent you down the x86/x64 path -- as @lorengordon pointed out, the server-manager module is also part of RSAT (I have that installed everywhere, so that point skipped my mind). lxml is the likely missing module keeping it from loading (the lgpo module doesn't log an error that it cannot import it, it simply returns False if any of the imports fail) |
@lorengordon Since there is now the |
I'm good with that yes, thanks! |
I would like to see salt implement a module and a state to manage local security policy objects. I know salt can manage Windows registry settings, but that can be overridden when the local security policy refreshes (which in turn can be overridden by group policy, but that's a feature of Active Directory Domain Services; I'm only referring to the local policy feature). A wrapper around secedit.exe would probably be sufficient. (I don't think there is another mechanism to programatically interface with the local security policy.)
I haven't seen this capability in many configuration management tools, though Puppet seems to have modules for it now. It's interesting that they use the plain English description from the GUI to specify a configuration setting, rather than the name used in the INF for the configuration setting (i.e. 'Password must meet complexity requirements' vs 'PasswordComplexity'). This is a little friendlier for admins and makes it easier for them to find the right syntax. It can be a rather convoluted process to figure out the corresponding INF setting just from what's displayed in the gpedit.msc GUI, so this is rather convenient... I'm betting the Puppet module has a hard-coded lookup table that does the translation. It should be noted that Microsoft has a habit of changing the description in the GUI between versions, which may cause confusion over time with that approach. Personally, I'd like to be able to specify a setting either way.
A Microsoft programmer has also written a wrapper around secedit.exe, called Apply_LGPO_Delta.exe, and they provide the source. It might be useful to study their approach.
The text was updated successfully, but these errors were encountered: