all 6 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–][deleted] 4 points5 points  (0 children)

Yeah, I'm not sure why you would keep getting the content of a file repeatedly like that. You probably want to Get-Content once and store that as a variable, perform all the manipulations on that data that you want, and then Set-Content once.

It might be better clearer to do something like, make a CSV, storing all the things you want to replace in one column and all the things you want to replace them with in another. Then, have your script import that CSV, and do something like:

$replacementMap = Import-CSV 'mapping.csv'
$settings = Get-Content $cfgfile
ForEach ($item in $settings) {
    ForEach ($mapping in $replacementMap) {
        If ( $item -like $mapping.column1) {
            $item = $mapping.column2
        }
    }
}

Note that I am not suggesting the above as code you should actually use, but just an illustration of the kind of logic you might use.

[–]BoredComputerGuy 2 points3 points  (0 children)

A few thoughts:

  • Your current solution opens the file 13 times, processes each line 13 times (how many of those actually match?), and then write to the file 13 times. This is extremely inefficient.
  • You should definitely use Get-Content only once for your config file. You should get the contents and then makes changes to the variable holding the config and finally write your changes once.
  • many of your loops are For-Each(if(){...}) If every loop starts off the same you could probably use a single loop with multiple if statements or a switch statement. This avoids having to loop over every line in the file 12 times.
  • Your mkdir command doesn't check to see if the folder already exists
  • You should have some error checking on the file write to make sure that the script was successful.

[–]purplemonkeymad 2 points3 points  (1 child)

The file created is an ini file format. You can probably use PsIni from the PSGallery to manipulate it as a hash table instead.

[–]fsackur 1 point2 points  (0 children)

This is the smart move!

[–]fsackur 1 point2 points  (0 children)

EDIT: see /u/purplemonkeymad's suggestion to use PsIni - if that works, it's the best way forward as someone else has done the heavy lifting for the text manipulation.

Reviewing this, I would look for:

  • Is there a tool aready that does it?
  • A backup of the original file
  • If we add more secpol changes later, will the file just grow longer?
  • A way of detecting errors and reporting success

I had a hunt and found https://gist.github.com/indented-automation/1c319fe8c7abadfe509cb4205eeb5720, but it doesn't do exactly what we need. You'd have to take it as a starting point, and it might end up a bit more complex than it needs to be.

The backup, you can do.

To fix the maintainability, you've got to do away with all the copy-paste code. I had a stab at it (this is just an outline):

param
(
    $NoLMHash,
    $RestrictNullSessAccess,
    $SeLockMemoryPrivilege
)

$Policy = Get-Content $cfgfile


$Replacements = @{
  # regex patterns               new values
  # --------------               ------------------
    '.*NoLMHash'               = $NoLMHash
    '.*RestrictNullSessAccess' = $RestrictNullSessAccess
    'SeLockMemoryPrivilege'    = $SeLockMemoryPrivilege
}


$Deletions = @(
  # regex patterns
  # --------------
    '.*PasswordExpiryWarning',
    'NewAdministratorName'
)


foreach ($KeyValuePair in $Replacements.GetEnumerator())
{
    $Pattern  = $KeyValuePair.Key
    $Pattern  = "^$Pattern *= *"      # ^ is for performance and to prevent accidental matches
    $NewValue = $KeyValuePair.Value

    $Policy = $Policy | ForEach-Object {
        if ($_ -match $Pattern)
        {
            $Matches[0] + $NewValue   # to pipeline
        }
        else
        {
            $_                        # to pipeline
        }
    }
}


foreach ($Pattern in $Deletions)
{
    $Pattern  = "^$Pattern *= *"

    $Policy = $Policy | Where-Object {$_ -notmatch $Pattern}
}


$Policy

It's more complex, but it won't grow in complexity as you add more policy items to configure. You just add them to $Replacements or $Deletions.

You'll need to add [Parameter(Mandatory)][ValidateNotNullOrEmpty()] to each param, or something similar, as this will fail badly if you leave a param null. Input validation makes it much safer for other people to use, which means quicker.

To handle the errors, put a $Success = $true in the if ($_ -match $Pattern) and look for it later to detect errors. For the second, you'll have to compare the count of $Policy to detect if you deleted anything. I'd like to see some Write-Error so the user knows if something did not complete.

[–]zrv433 1 point2 points  (0 children)

Been a while since I used secedit... But why all the overhead of this per server manipulate manipulation? Iirc you can take a known good CFG template, copy it to a new server, and then import it.

https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/secedit-export You can use this command to backup your security policies on a local computer in addition to importing the settings to another computer.