Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Problem with "hashlimit" match without hashlimit_htable_expire value #201

Closed
jllorente opened this issue Dec 16, 2016 · 12 comments
Closed

Problem with "hashlimit" match without hashlimit_htable_expire value #201

jllorente opened this issue Dec 16, 2016 · 12 comments

Comments

@jllorente
Copy link
Collaborator

Hi,
I tried to replicate the example described here (

python-iptables/tests/test_matches.py

Line 431 in edd1623

class TestHashlimitMatch(unittest.TestCase):
)

Turns out if you do not set hashlimit_htable_expire, the iptables chain is not populated but no error is shown either, fails silently. I did this, not setting a value because I was a bit unsure since I hadn't used it either in my other iptables scripts.

Steps to reproduce:

rule = iptc.Rule()
rule.src = "127.0.0.1"
rule.protocol = "udp"
rule.target = iptc.Target(rule, "ACCEPT")
match = iptc.Match(rule, "hashlimit")
chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), "FORWARD")
table = iptc.Table(iptc.Table.FILTER)
match.hashlimit_name = 'foo'
match.hashlimit_mode = 'srcip'
match.hashlimit_upto = '200/sec'
#match.hashlimit = '200' # This seems not to be necessary
#match.hashlimit_htable_expire = '100'
rule.add_match(match)
chain.insert_rule(rule)
@pepoluan
Copy link
Contributor

I can verify this bug.

I'm using my patched version (see #200 ), on Linux Mint 18 "Sarah". Uncommenting the line

#match.hashlimit_htable_expire = '100'

Successfully added the rule to iptables.

Furthermore, if I disable autocommit by modifying the code as such:

table = iptc.Table(iptc.Table.FILTER)
table.autocommit = False
rule = iptc.Rule()
rule.src = "127.0.0.1"
rule.protocol = "udp"
rule.target = iptc.Target(rule, "ACCEPT")
match = iptc.Match(rule, "hashlimit")
chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), "FORWARD")
match.hashlimit_name = 'foo'
match.hashlimit_mode = 'srcip'
match.hashlimit_upto = '200/sec'
#match.hashlimit = '200' # This seems not to be necessary
#match.hashlimit_htable_expire = '100'
rule.add_match(match)
chain.insert_rule(rule)
table.commit()
table.autocommit = True

Now it emits an error at table.commit():

Traceback (most recent call last):
  File "/home/pepoluan/PycharmProjects/python-iptables/tests/try_it.py", line 20, in <module>
    table.commit()
  File "/home/pepoluan/PycharmProjects/python-iptables/iptc/ip4tc.py", line 1591, in commit
    raise IPTCError("can't commit: %s" % (self.strerror()))
iptc.ip4tc.IPTCError: can't commit: Invalid argument

Trying to emulate same using CLI:

iptables -A FORWARD -s 127.0.0.1 -p udp -j ACCEPT \
  -m hashlimit --hashlimit-name foo \
  --hashlimit-mode srcip --hashlimit-upto 200/sec

Does not exhibit the same behavior.

@pepoluan
Copy link
Contributor

pepoluan commented Dec 16, 2016 •
edited
Loading

Additional info:

If I edit the code as such:

table = iptc.Table(iptc.Table.FILTER)
table.autocommit = False
rule = iptc.Rule()
rule.src = "127.0.0.1"
rule.protocol = "udp"
rule.target = iptc.Target(rule, "ACCEPT")
match = iptc.Match(rule, "hashlimit")
print('Params just after match creation:\n  ', match.parameters)
chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), "FORWARD")
match.hashlimit_name = 'foo'
match.hashlimit_mode = 'srcip'
match.hashlimit_upto = '200/sec'
print('Params after partial set:\n  ', match.parameters)
#match.hashlimit = '200' # This seems not to be necessary
#match.hashlimit_htable_expire = '100'
rule.add_match(match)
chain.insert_rule(rule)
table.commit()
table.autocommit = True

I get this:

Params just after match creation:
   {u'hashlimit_burst': u'5', u'hashlimit_name': u'', u'hashlimit_upto': u'inf'}
Params after partial set:
   {u'hashlimit_burst': u'5', u'hashlimit_mode': u'srcip', u'hashlimit_name': u'foo', u'hashlimit_upto': u'200/sec', u'hashlimit_htable_expire': u'0'}
Traceback (most recent call last):
  File "/home/pepoluan/PycharmProjects/python-iptables/tests/try_it.py", line 25, in <module>
    table.commit()
  File "/home/pepoluan/PycharmProjects/python-iptables/iptc/ip4tc.py", line 1591, in commit
    raise IPTCError("can't commit: %s" % (self.strerror()))
iptc.ip4tc.IPTCError: can't commit: Invalid argument

So, apparently, hashlimit_htable_expire got set to '0'. Trying this on the CLI:

# iptables -A FORWARD -s 127.0.0.1 -p udp -j ACCEPT -m hashlimit --hashlimit-name foo --hashlimit-mode srcip --hashlimit-upto 200/sec --hashlimit-htable-expire 0
iptables: Invalid argument. Run `dmesg' for more information.

iptables puked on --hashlimit-htable-expire 0, it seems. Because changing that to --hashlimit-htable-expire 100 removes the error.

Not sure where this u'hashlimit_htable_expire': u'0' comes from, though.

@ldx
Copy link
Owner

ldx commented Jan 3, 2017

Yes, this is a known issue. Setting it explicitly works around the problem, thought.

@ldx ldx closed this as completed Jan 3, 2017
@jllorente
Copy link
Collaborator Author

Hei!
I kept playing around with this issue and I have found something interesting, yet fairly simple.
Add 3 rules via iptables CLI, where the hashlimit-name carries the expiration time configured:

#iptables -A INPUT -m hashlimit --hashlimit-above 20/sec --hashlimit-name test_100 --hashlimit-burst 30 --hashlimit-htable-expire 100
#iptables -A INPUT -m hashlimit --hashlimit-above 20/sec --hashlimit-name test_1000 --hashlimit-burst 30 --hashlimit-htable-expire 1000
#iptables -A INPUT -m hashlimit --hashlimit-above 20/sec --hashlimit-name test_10000 --hashlimit-burst 30 --hashlimit-htable-expire 10000

Then dump the rules

#iptables -S INPUT
-A INPUT -m hashlimit --hashlimit-above 20/sec --hashlimit-burst 30 --hashlimit-name test_100 --hashlimit-htable-expire 100
-A INPUT -m hashlimit --hashlimit-above 20/sec --hashlimit-burst 30 --hashlimit-name test_1000
-A INPUT -m hashlimit --hashlimit-above 20/sec --hashlimit-burst 30 --hashlimit-name test_10000 --hashlimit-htable-expire 10000

The default value for hashlimit-htable-expire is therefore 1000 ms (1 sec).
As @pepoluan mentioned before, the culprit is setting either match.hashlimit_upto or match.hashlimit_above, which generates hashlimit-htable-expire=0

It would be great if we could somehow set the default value to 1000 upon creation of a hashlimit match.

@jllorente
Copy link
Collaborator Author

Hi,

More information to this regard. There seems to be a larger issue than initially expected.

The following examples creates a hashlimit match and sets a target value for the rule

# Example setting a target value - ACCEPT
>>> chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), "FORWARD")
>>> rule = iptc.Rule()
>>> rule.src = '1.2.3.4'
>>> target = rule.create_target('ACCEPT')
>>> match  = rule.create_match('hashlimit')
>>> match.hashlimit_name  = 'hl_test1'
>>> match.hashlimit_above = '100/sec'
>>> match.hashlimit_burst = '150/sec'

# This rule is not inserted as hashlimit_htable_expire = 0
>>> print(match.parameters)
{'hashlimit_burst': '150', 'hashlimit_name': 'hl_test1', 'hashlimit_above': '100/sec', 'hashlimit_htable_expire': '0'}
>>> chain.insert_rule(rule)

# This rule is inserted
>>> match.hashlimit_htable_expire = '100'
>>> print(match.parameters)
{'hashlimit_htable_expire': '100', 'hashlimit_above': '100/sec', 'hashlimit_name': 'hl_test1', 'hashlimit_burst': '150'}
>>> chain.insert_rule(rule)

# This rule is inserted
>>> match.hashlimit_htable_expire = '2000'
>>> print(match.parameters)
{'hashlimit_burst': '150', 'hashlimit_name': 'hl_test1', 'hashlimit_above': '100/sec', 'hashlimit_htable_expire': '2000'}
>>> chain.insert_rule(rule)

# This rule is not inserted, furthermore there is no hashlimit_htable_expire parameter in match
>>> match.hashlimit_htable_expire = '1000'
>>> print(match.parameters)
{'hashlimit_burst': '150', 'hashlimit_name': 'hl_test1', 'hashlimit_above': '100/sec'}
>>> chain.insert_rule(rule)

Following the examples of above create a hashlimit match but without setting a target value for the rule gives the following error for all the variations of the hashlimit_htable_expire parameter.

/home/ubuntu/.local/lib/python3.5/site-packages/iptc/ip4tc.py in insert_rule(self, rule, position)
   1434         rbuf = rule.rule                                                                  
   1435         if not rbuf:                                                                      
-> 1436             raise ValueError("invalid rule")                                              
   1437         self.table.insert_entry(self.name, rbuf, position)                                
   1438                                                                                           
                                                                                                  
ValueError: invalid rule

However, the same rule without the target can be added via iptables CLI, then with iptc I can read it and reinsert it again and again... Is the Rule() created differently when reading from the netlink interface?

iptables -A FORWARD -s 1.2.3.4/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test1 --hashlimit-htable-expire 100

In addition, I have also conducted more tests with the htable_expire = 1000 and there seems to be another bug somewhere...

chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), "FORWARD")
rule = iptc.Rule()
rule.src = '5.6.7.8'
target = rule.create_target('ACCEPT')
match  = rule.create_match('hashlimit')
match.hashlimit_name  = 'hl_test2'
match.hashlimit_above = '100/sec'
match.hashlimit_burst = '150/sec'
match.hashlimit_htable_expire = '1000'
print(match.parameters)
chain.insert_rule(rule)
# Sometimes, if I repeat this insert_rule() several times from the python interpreter, it inserts junk as can be seen from the iptables output. The value is random and it changed for every batch of tests I've run.
# iptables -S FORWARD
-A FORWARD -s 5.6.7.8/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test2 --hashlimit-htable-expire 4031034112 -j ACCEPT
-A FORWARD -s 5.6.7.8/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test2 --hashlimit-htable-expire 4031034112 -j ACCEPT
-A FORWARD -s 5.6.7.8/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test2 --hashlimit-htable-expire 4031034112 -j ACCEPT
-A FORWARD -s 5.6.7.8/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test2 --hashlimit-htable-expire 4031034112 -j ACCEPT
-A FORWARD -s 5.6.7.8/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test2 --hashlimit-htable-expire 4031034112 -j ACCEPT
-A FORWARD -s 5.6.7.8/32 -m hashlimit --hashlimit-above 100/sec --hashlimit-burst 150 --hashlimit-name hl_test2 --hashlimit-htable-expire 4031034112 -j ACCEPT

Cheers!
Jesus

@pepoluan
Copy link
Contributor

pepoluan commented Jan 11, 2017 via email

@jllorente
Copy link
Collaborator Author

I cannot tell where the problem lies, but there is certainly a bug somewhere... I would suggest to reopen the issue until it is resolved, as it may help other people running into the same problem.

As a temporary solution I am using this in my rules:

match.hashlimit_htable_expire = '1100'

Cheers!

@ldx
Copy link
Owner

ldx commented Jan 18, 2017

Looking into this a bit more

  1. As for the expire parameter, in iptables, in extensions/libxt_hashlimit.c info->cfg.expire gets set when burst is set:

    static void hashlimit_mt_check(struct xt_fcheck_call *cb)
    {
    const struct hashlimit_mt_udata *udata = cb->udata;
    struct xt_hashlimit_mtinfo1 *info = cb->data;

     if (!(cb->xflags & (F_UPTO | F_ABOVE)))
 	xtables_error(PARAMETER_PROBLEM,
 			"You have to specify --hashlimit");
 if (!(cb->xflags & F_HTABLE_EXPIRE))
 	info->cfg.expire = udata->mult * 1000; /* from s to msec */

 if (info->cfg.mode & XT_HASHLIMIT_BYTES) {
 	uint32_t burst = 0;
 	if (cb->xflags & F_BURST) {
 		if (info->cfg.burst < cost_to_bytes(info->cfg.avg))
 			xtables_error(PARAMETER_PROBLEM,
 				"burst cannot be smaller than %ub", cost_to_bytes(info->cfg.avg));

 		burst = info->cfg.burst;
 		burst /= cost_to_bytes(info->cfg.avg);
 		if (info->cfg.burst % cost_to_bytes(info->cfg.avg))
 			burst++;
 		if (!(cb->xflags & F_HTABLE_EXPIRE))
 			info->cfg.expire = XT_HASHLIMIT_BYTE_EXPIRE_BURST * 1000;
 	}
 	info->cfg.burst = burst;
 } else if (info->cfg.burst > XT_HASHLIMIT_BURST_MAX)
 	burst_error();

    }

and then in hashlimit_mt_save():

	if (info->cfg.expire != quantum)
		printf(" --hashlimit-htable-expire %u", info->cfg.expire);

so it's ambigous in the extension itself - --hashlimit-htable-expire is set implicitly. Python-iptables uses the save() callback to retrieve currently set options/values. There's not much we can do here.

  1. There is a special target with an "empty" name. You can use target = rule.create_target('') to instantiate it.

Does this help?

@pepoluan
Copy link
Contributor

It is an upstream bug, then :-)

@ldx
Copy link
Owner

ldx commented Jan 18, 2017

I wouldn't call it a bug, but unfortunately I see no easy way to fix it in python-iptables without adding extension-specific workarounds. :)

Let me know if there's anything else I can help with!

@jllorente
Copy link
Collaborator Author

Thanks @ldx for the digging!
For now, I think I will stick to the match.hashlimit_htable_expire='1001' for this particular extension.

@pepoluan
Copy link
Contributor

pepoluan commented Jun 5, 2017

Another thought came to me: If the lib requires hashlimit_htable_expire, we ought to take a look at how the iptables program handle the case when hashlimit_htable_expire was not specified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ldx @pepoluan @jllorente