9

I am just learning python and have written some code to set iptables using the python-iptables library. The problem I'm running in to is that I had to rewrite a lot of the same lines of code over and over. I understand functions somewhat but not OOP. I'm thinking there is a better OOP way of writing this code but I can't get my head around it. Any pointers will be greatly appreciated. The code is below.

import iptc

def dropAllInbound():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    rule = iptc.Rule()
    rule.in_interface = 'eth+'
    rule.target = iptc.Target(rule, 'DROP')
    chain.insert_rule(rule)

def allowLoopback():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    rule = iptc.Rule()
    rule.in_interface = 'lo'
    rule.target = iptc.Target(rule, 'ACCEPT')
    chain.insert_rule(rule)

def allowEstablishedInbound():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    rule = iptc.Rule()
    match = rule.create_match('state')
    match.state = 'RELATED,ESTABLISHED'
    rule.target = iptc.Target(rule, 'ACCEPT')
    chain.insert_rule(rule)

def allowHTTP():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    rule = iptc.Rule()
    rule.in_interface = 'eth+'
    rule.protocol = 'tcp'
    match = rule.create_match('tcp')
    match.dport = '80'
    rule.target = iptc.Target(rule, 'ACCEPT')
    chain.insert_rule(rule)

def allowHTTPS():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    rule = iptc.Rule()
    rule.in_interface = 'eth+'
    rule.protocol = 'tcp'
    match = rule.create_match('tcp')
    match.dport = '443'
    rule.target = iptc.Target(rule, 'ACCEPT')
    chain.insert_rule(rule)

def allowSSH():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    rule = iptc.Rule()
    rule.in_interface = 'eth+'
    rule.protocol = 'tcp'
    match = rule.create_match('tcp')
    match.dport = '22'
    rule.target = iptc.Target(rule, 'ACCEPT')
    chain.insert_rule(rule)

def allowEstablishedOutbound():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'OUTPUT')
    rule = iptc.Rule()
    match = rule.create_match('state')
    match.state = 'RELATED,ESTABLISHED'
    rule.target = iptc.Target(rule, 'ACCEPT')
    chain.insert_rule(rule)

def dropAllOutbound():
    chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'OUTPUT')
    rule = iptc.Rule()
    rule.in_interface = 'eth+'
    rule.target = iptc.Target(rule, 'DROP')
    chain.insert_rule(rule)

def defaultAction():
    dropAllOutbound()
    dropAllInbound()
    allowLoopback()
    allowEstablishedInbound()
    allowEstablishedOutbound()

def getInput():
        print 'Default action (1) is most secure '
        print 'Default  -  1'
        print 'HTTP     -  2'
        print 'HTTPS    -  3'
        print 'SSH      -  4'
        print 'Exit     -  5'
        choices = raw_input('Enter choices (comma Separated) ').split(',')
        for action in choices:
            if action == "1":
                defaultAction()
                break
            if action == "2":
                allowHTTP()
                break
            if action == "3":
                allowHTTPS()
                break
            if action == "4":
                allowSSH()
                break
            else:
                break
getInput()

Notice how all the rules have similar lines of code. Is there a way to create a rule generator object or something like that to minimize rewriting that code?

I added the following function so and call it every time the script is run so that the rules get flushed.

def startClean():
    chainIn = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')
    chainIn.flush()
    chainOut = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'OUTPUT')
    chainOut.flush()
h33th3n
  • 257
  • 1
  • 3
  • 11
  • 1
    If you add any abstraction here, you make your code harder to maintain for your future self and for someone else, because they have to (re)learn your abstraction. This can be a huge pain when there is a serious and urgent bug in the firewall config. Consider this when evaluating your options. – pts Dec 26 '13 at 02:32
  • Thank you for your insight. So basically you think what I have now is good and makes good sense? – h33th3n Dec 26 '13 at 02:37
  • 1
    Yes, I'd leave it as it is right now if there are no other answers, and I'd reconsider it 1 year from now, with more Python experience and a better understanding of the maintenance costs. – pts Dec 26 '13 at 09:55
  • 2
    One more comment: please flush (clear) all chains and tables in the beginning, so if you run your script multiple times, you always get the same result, and rules don't accumulate. – pts Dec 26 '13 at 09:56
  • 2
    More suitable for [CodeReview](http://codereview.stackexchange.com/) – Mp0int Dec 26 '13 at 13:18

1 Answers1

4

OOP is for maintaining the state of something. OOP is for some object that has both properties and methods to manipulate those properties.

class Chair(object):

    MAX_WEIGHT = 300

    def __init__(self):
        super().__init__()

        self.weight = 5
        self.currentWeight = self.weight
        self.holding = None
        self.broken = False

    def hold(self, item):
        self.holding = item
        self.currentWeight = self.weight + item.weight
        self.checkWeight()

    def checkWeight(self):
        if self.holding.weight > self.MAX_WEIGHT:
            self.broken = True
            ...

Your code seems fine; rewriting the code just for OOP may be more work than it's worth. If you really want to use OOP you may wish to do something like below.

class Table(object):
    def __init__(self):
        self.chain = None
        self.rule = None
        self.match = None

    def setInput(self):
        self.chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT')

    def setOutput(self):
        self.chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'OUTPUT')

    ...

table = Table()
table.setInput()
...
justengel
  • 6,132
  • 4
  • 26
  • 42
  • Thank you for this comment. It looks like the running consensus is that the code is fine as is. I may go ahead and try to build it out OOP style just for learning though. – h33th3n Dec 26 '13 at 16:53