PowerShell Code Cleanup

Obsolesce

I wrote a PowerShell script to run as a task every hour, or as part of another task. I wrote it to work, and it works great and gets the job done. But looking at it, it seems like there's a much cleaner and future-proof way to do it.

In addition to getting this done, I also want to take the opportunity to learn better PowerShell methods where I can as I go, which is the reason for this post.

Imagine if I had to add like 20 or more Firewall rules to this script (which could be possible in the near future)... it will become messy, and the last line of the script may or may not work with so many "-and" statements.

The script:

$FWRule1 = Get-NetFireWallRule -DisplayName "!Allow Sync Fileserv 445 "
$FWRule2 = Get-NetFireWallRule -DisplayName "!Allow Outbound UDP svchost.exe "
$FWRule3 = Get-NetFireWallRule -DisplayName "!Allow Outbound TCP TeamViewer.exe "
$FWRule4 = Get-NetFireWallRule -DisplayName "!Allow Outbound 5938 UDP TeamViewer.exe Local Port "
if ($FWRule1 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Sync Fileserv 445 " -Enabled True -Direction Inbound -Profile Private -LocalPort 445 -RemotePort ANY -RemoteAddress 192.168.4.7 -Protocol TCP -Action Allow -Description "Allows file sync from Fileserv via SMB 445. (FWRule1)"}
if ($FWRule2 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Outbound UDP svchost.exe " -Enabled True -Direction Outbound -Profile ANY -Protocol UDP -Program "C:\WINDOWS\system32\svchost.exe" -Action Allow -Description "Allows Outbound UDP svchost.exe. (FWRule2)"}
if ($FWRule3 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Outbound TCP TeamViewer.exe " -Enabled True -Direction Outbound -Profile ANY -Protocol TCP -Program "C:\Program Files (x86)\TeamViewer\TeamViewer.exe" -Action Allow -Description "Allows Outbound TCP TeamViewer.exe. (FWRule3)"}
if ($FWRule4 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Outbound 5938 UDP TeamViewer.exe Local Port " -Enabled True -Direction Outbound -Profile ANY -LocalPort 5938 -RemotePort ANY -Protocol UDP -Program "C:\Program Files (x86)\TeamViewer\TeamViewer.exe" -Action Allow -Description "Allows Outbound TeamViewer.exe communication via 5938 UDP. (FWRule4)"}
Remove-Variable FWRule1
Remove-Variable FWRule2
Remove-Variable FWRule3
Remove-Variable FWRule4
Get-NetFirewallRule | Where-Object {($_.DisplayName -ne "!Allow Sync Fileserv 445 ") -and ($_.DisplayName -ne "!Allow Outbound UDP svchost.exe ") -and ($_.DisplayName -ne "!Allow Outbound TCP TeamViewer.exe ") -and ($_.DisplayName -ne "!Allow Outbound 5938 UDP TeamViewer.exe Local Port ")} | Remove-NetFirewallRule

dafyre

You can fix the ugilness of long single lines by using the backtick (`). That makes the code more readable to some folks.

Be consistent with your ordering as well, and I don't think you'd have a problem.

matteo nunziati

@Tim_G never used PS seriousltùy, but as acoder I would you suggest to make your FW rules instances of a class and put them into an array. then for-looping them

this will fix the growing number of rules.

about last line... don't get it. it was better to say: remove ALL firewall rules FIRST and THEN ADD only those required?

in linux a simple ip-tables script always start wiping everything and then adding a deny/allow on basic traffic chains, like in/out/forward. then you add specific rules.

just my 2 cents

Obsolesce

@matteo-nunziati said in PowerShell Code Cleanup:

about last line... don't get it. it was better to say: remove ALL firewall rules FIRST and THEN ADD only those required?

I thought about that too, but then if someone/something is connected, wiping the firewall rules first will disconnect the current user or process. I don't want that to happen. So I set it to wipe everything that shouldn't be there.

Obsolesce

I ended up changing the script a little bit because it seems like TeamViewer made a change, and I also wanted to allow LAN connections to connect to TeamViewer in addition from the internet... but I at least made it a little bit easier to manage the -and statements:

# $FWRule1 = Get-NetFireWallRule -DisplayName "!Allow Sync Fileserv 445 "
$FWRule2 = Get-NetFireWallRule -DisplayName "!Allow Outbound UDP svchost.exe "
$FWRule3 = Get-NetFireWallRule -DisplayName "!Allow Outbound TCP TeamViewer_Service.exe "
$FWRule4 = Get-NetFireWallRule -DisplayName "!Allow Outbound 5938 UDP TeamViewer_Service.exe Local Port "
$FWRule5 = Get-NetFireWallRule -DisplayName "!Allow Inbound TCP LAN TeamViewer_Service.exe "
$FWRule6 = Get-NetFireWallRule -DisplayName "!Allow Inbound UDP LAN TeamViewer_Service.exe "
# if ($FWRule1 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Sync Fileserv 445 " -Enabled True -Direction Inbound -Profile Private -LocalPort 445 -RemotePort ANY -RemoteAddress 192.168.4.7 -Protocol TCP -Action Allow -Description "Allows file sync from Fileserv via SMB 445. (FWRule1)"}
if ($FWRule2 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Outbound UDP svchost.exe " -Enabled True -Direction Outbound -Profile ANY -Protocol UDP -Program "C:\WINDOWS\system32\svchost.exe" -Action Allow -Description "Allows Outbound UDP svchost.exe. (FWRule2)"}
if ($FWRule3 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Outbound TCP TeamViewer_Service.exe " -Enabled True -Direction Outbound -Profile ANY -Protocol TCP -Program "%ProgramFiles% (x86)\TeamViewer\TeamViewer_Service.exe" -Action Allow -Description "Allows Outbound TCP TeamViewer_Service.exe. (FWRule3)"}
if ($FWRule4 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Outbound 5938 UDP TeamViewer_Service.exe Local Port " -Enabled True -Direction Outbound -Profile ANY -LocalPort 5938 -RemotePort ANY -Protocol UDP -Program "%ProgramFiles% (x86)\TeamViewer\TeamViewer_Service.exe" -Action Allow -Description "Allows Outbound TeamViewer_Service.exe communication via 5938 UDP. (FWRule4)"}
if ($FWRule5 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Inbound TCP LAN TeamViewer_Service.exe " -Enabled True -Direction Inbound -Profile Private -Protocol TCP -Program "%ProgramFiles% (x86)\TeamViewer\TeamViewer_Service.exe" -Action Allow -Description "Allows Inbound TCP LAN TeamViewer_Service.exe. (FWRule5)"}
if ($FWRule6 -eq $null) {New-NetFirewallRule -DisplayName "!Allow Inbound UDP LAN TeamViewer_Service.exe " -Enabled True -Direction Inbound -Profile Private -Protocol UDP -Program "%ProgramFiles% (x86)\TeamViewer\TeamViewer_Service.exe" -Action Allow -Description "Allows Inbound UDP LAN TeamViewer_Service.exe. (FWRule6)"}
Get-NetFirewallRule | Where-Object {
    ($_.DisplayName -ne "!Allow Outbound UDP svchost.exe ") -and
    ($_.DisplayName -ne "!Allow Outbound TCP TeamViewer_Service.exe ") -and
    ($_.DisplayName -ne "!Allow Outbound 5938 UDP TeamViewer_Service.exe Local Port ") -and
    ($_.DisplayName -ne "!Allow Inbound TCP LAN TeamViewer_Service.exe ") -and
    ($_.DisplayName -ne "!Allow Inbound UDP LAN TeamViewer_Service.exe ")
} | Remove-NetFirewallRule
# Remove-Variable FWRule1
Remove-Variable FWRule2
Remove-Variable FWRule3
Remove-Variable FWRule4
Remove-Variable FWRule5
Remove-Variable FWRule6

Obsolesce

@matteo-nunziati said in PowerShell Code Cleanup:

@Tim_G never used PS seriousltùy, but as acoder I would you suggest to make your FW rules instances of a class and put them into an array. then for-looping them
this will fix the growing number of rules.

Thanks, I will look into this.