9
\$\begingroup\$

I have this following module using for adding and enabling/disabling Windows Firewall rules using Python.

I currently use subprocess.call to execute the netsh command inside Python. I'm wondering if there is any better method to do this? Executing the cmd command inside Python seems to be impractical to me.

import subprocess, ctypes, os, sys
from subprocess import Popen, DEVNULL

def chkAdmin():
    """ Force to start application with admin rights """
    try:
        isAdmin = ctypes.windll.shell32.IsUserAnAdmin()
    except AttributeError:
        isAdmin = False
    if not isAdmin:
        ctypes.windll.shell32.ShellExecuteW(None, "runas", sys.executable, __file__, None, 1)

def addRule(rule_name, file_path):
    """ Add rule to Windows Firewall """
    subprocess.call("netsh advfirewall firewall add rule name="+ rule_name +" dir=out action=block enable=no program=" + file_path, shell=True, stdout=DEVNULL, stderr=DEVNULL)
    print("Rule", rule_name, "for", file_path, "added")

def modifyRule(rule_name, state):
    """ Enable/Disable specific rule, 0 = Disable / 1 = Enable """
    if state:
        subprocess.call("netsh advfirewall firewall set rule name="+ rule_name +" new enable=yes", shell=True, stdout=DEVNULL, stderr=DEVNULL)
        print("Rule", rule_name, "Enabled")
    else:
        subprocess.call("netsh advfirewall firewall set rule name="+ rule_name +" new enable=no", shell=True, stdout=DEVNULL, stderr=DEVNULL)
        print("Rule", rule_name, "Disabled")

chkAdmin()
addRule("RULE_NAME", "PATH_TO_FILE")
modifyRule("RULE_NAME", 1)
\$\endgroup\$
3
  • 1
    \$\begingroup\$ could it be this : stackoverflow.com/a/5486837/6212957 \$\endgroup\$ Commented Jan 9, 2019 at 7:23
  • 1
    \$\begingroup\$ I would at least give PowerShell a look. It supports remote execution and is as close as you can get to a tool intended to manage Windows via script. \$\endgroup\$
    – jpmc26
    Commented Jan 9, 2019 at 10:37
  • \$\begingroup\$ Is it possible for these commands to fail? If netsh returns an error for any reason, it will go undetected (correct me if I'm wrong). At least assert things went smoothly even if you don't want to handle errors. \$\endgroup\$ Commented Jan 9, 2019 at 17:35

2 Answers 2

10
\$\begingroup\$

This seems OK

We can add a little flavor to it:

  1. Don't use string concatenation, but use f"{strings}" or "{}".format(strings)

  2. Your modify rule, can be simplified

    The if else don't differ that much, you can use a (Python)ternary to calculate the variables beforehand

  3. Consider to chop up the lines, to make it a little more readable

  4. Functions and variables should be snake_case according to PEP8

  5. Use a if __name__ == '__main__' guard

  6. As mentioned, you could use os.system("command") instead of subprocess

    But honestly I would stick with subprocess, since it will give greater control over how commands are executed

Code

import subprocess, ctypes, os, sys
from subprocess import Popen, DEVNULL

def check_admin():
    """ Force to start application with admin rights """
    try:
        isAdmin = ctypes.windll.shell32.IsUserAnAdmin()
    except AttributeError:
        isAdmin = False
    if not isAdmin:
        ctypes.windll.shell32.ShellExecuteW(None, "runas", sys.executable, __file__, None, 1)

def add_rule(rule_name, file_path):
    """ Add rule to Windows Firewall """
    subprocess.call(
        f"netsh advfirewall firewall add rule name={rule_name} dir=out action=block enable=no program={file_path}", 
        shell=True, 
        stdout=DEVNULL, 
        stderr=DEVNULL
    )
    print(f"Rule {rule_name} for {file_path} added")

def modify_rule(rule_name, state):
    """ Enable/Disable specific rule, 0 = Disable / 1 = Enable """
    state, message = ("yes", "Enabled") if state else ("no", "Disabled")
    subprocess.call(
        f"netsh advfirewall firewall set rule name={rule_name} new enable={state}", 
        shell=True, 
        stdout=DEVNULL, 
        stderr=DEVNULL
    )
    print(f"Rule {rule_name} {message}")

if __name__ == '__main__':
    check_admin()
    add_rule("RULE_NAME", "PATH_TO_FILE")
    modify_rule("RULE_NAME", 1)
\$\endgroup\$
2
  • \$\begingroup\$ Thanks! and may I ask another question? Why you recommend against using string concatenation? \$\endgroup\$
    – phwt
    Commented Jan 9, 2019 at 8:55
  • 4
    \$\begingroup\$ Sure,.. it has no significant influence, but it does read a lot smoother, especially with the introduction of f"{strings}". (Python 3.6+) \$\endgroup\$
    – Ludisposed
    Commented Jan 9, 2019 at 9:02
11
\$\begingroup\$

I agree with @Ludisposed answer, but you have a few subprocess gotchas:

  • You don't need to spawn a shell in order to run the command, simply build your command as a list of arguments and it will be fine. This is especially important if your rules names may contains spaces as the command would be treated entirelly differently in your implementation;
  • Replace the old subprocess.call by subprocess.run;
  • You may be interested to run subprocess.run by specifying check=True in order to generate an exception and be alerted if something does not go according to plan.

Applying these changes to e.g. modify_rule can lead to:

def modify_rule(rule_name, enabled=True):
    """Enable or Disable a specific rule"""
    subprocess.run(
        [
            'netsh', 'advfirewall', 'firewall',
            'set', 'rule', f'name={rule_name}',
            'new', f'enable={"yes" if enabled else "no"}',
        ],
        check=True,
        stdout=DEVNULL,
        stderr=DEVNULL
    )

Also note that I removed the print call from the function as it impairs reusability. If the caller want this kind of messages, it should be responsible for printing them, not this function.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.