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.

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

Problem with target "NOTRACK" #204

Closed
jllorente opened this issue Feb 7, 2017 · 9 comments
Closed

Problem with target "NOTRACK" #204

jllorente opened this issue Feb 7, 2017 · 9 comments

Comments

@jllorente
Copy link
Collaborator

jllorente commented Feb 7, 2017

Hi,

I think I've found a bug regarding target "NOTRACK"
I'm attempting to produce a test rule to untrack SCTP protocol

# iptables -t raw -A PREROUTING -p 132 -j NOTRACK

The problem is that the target is not handled properly and yields the following:

# iptables -t raw -S PREROUTING
-P PREROUTING ACCEPT
-P OUTPUT ACCEPT
-A PREROUTING -p sctp -j CT

Steps to reproduce:

import iptc

rule = iptc.Rule()
rule.protocol = "132"
rule.target = iptc.Target(rule, "NOTRACK")
chain = iptc.Chain(iptc.Table(iptc.Table.RAW), "PREROUTING")
chain.insert_rule(rule)

Any help would be appreciated!
Thanks!

@jllorente
Copy link
Collaborator Author

jllorente commented Feb 7, 2017

I have done bit more testing. Turns out that I can properly retrieve a NOTRACK rule via iptc, but I cannot insert it back.

Steps to reproduce:

### First insert the rule via iptables
# iptables -t raw -A PREROUTING -p 132 -j NOTRACK


### Then fire up python(3)
import iptc

chain = iptc.Chain(iptc.Table(iptc.Table.RAW), "PREROUTING")
rule = chain.rules[0]

rule.target.name
>> NOTRACK

rule.target.standard_target
>>> CT
 
rule.target.get_all_parameters()
>>> {}

rule.target.__dict__
>>>
{'_revision': 2,
 '_ptr': <iptc.xtables.LP_xt_entry_target at 0x7f4c562b1ae8>,
 '_orig_parse': <CFunctionType object at 0x7f4c57169818>,
 '_orig_options': <iptc.xtables.LP_xt_option_entry at 0x7f4c562b1268>,
 '_target_buf': <iptc.ip4tc.LP_c_ubyte at 0x7f4c562b1488>,
 '_name': 'CT',
 '_buffer': <iptc.ip4tc._Buffer at 0x7f4c5627acc0>,
 '_xt': XTables for protocol 2,
 '_ptrptr': <iptc.xtables.LP_LP_xt_entry_target at 0x7f4c562b1d90>,
 '_module': <iptc.xtables._xtables_target_v10 at 0x7f4c562b18c8>,
 '_alias_module': <iptc.xtables._xtables_target_v10 at 0x7f4c562b1f28>,
 '_rule': <iptc.ip4tc.Rule at 0x7f4c5627aa20>}

Hope this sheds some light :)

@pepoluan
Copy link
Contributor

I think the cause to this issue is that NOTRACK is actually an 'alias' to CT --notrack.

But as the Target was created without any parameters, the conversion to CT does not have space to record the notrack parameter.

Try targetting CT --notrack instead, and check if the rule is again malformed.

@jllorente
Copy link
Collaborator Author

Thanks for pointing that out @pepoluan, you are absolutely right, NOTRACK is indeed an alias for the CT target.

I would like to point out that to perform that "NOTRACK" action, one could insert the rule in 2 different ways, however only the former seems to be properly processed by iptc.

# iptables -t raw -A PREROUTING -p 132 -j CT --notrack
# iptables -t raw -A PREROUTING -p 132 -j NOTRACK

The supported way then to handle the NOTRACK is as follows:

import iptc

rule = iptc.Rule()
rule.protocol = "132"
rule.target = iptc.Target(rule, "CT")
rule.target.notrack = ""
chain = iptc.Chain(iptc.Table(iptc.Table.RAW), "PREROUTING")
chain.insert_rule(rule)

Shall we then close this issue or is there any way of adding support for the alias processing?

Cheers!

@pepoluan
Copy link
Contributor

pepoluan commented Feb 27, 2017

Just a thought: Should we augment the Target class' __init__ to raise an exception for this case?

(IIRC, NOTRACK is deprecated anyways, so we can extend this behavior to all deprecated targets.)

@ldx
Copy link
Owner

ldx commented Mar 2, 2017

@jllorente there might be a way to handle this better, but I'm kinda pressed on time right now, so not sure when I'll be able to look into this.

@pepoluan I usually don't really like adding target/match specific logic to the classes, but if you can elaborate or create a PR to show what you mean I'll be happy to review it.

@pepoluan
Copy link
Contributor

pepoluan commented Mar 2, 2017

Busy doing other things, so rather than submitting a half-baked PR, I'll just illustrate it in a diff against latest master:


diff --git a/iptc/ip4tc.py b/iptc/ip4tc.py
index 3a5791a..3fe4e0d 100644
--- a/iptc/ip4tc.py
+++ b/iptc/ip4tc.py
@@ -15,6 +15,8 @@ from .xtables import (XT_INV_PROTO, NFPROTO_IPV4, XTablesError, xtables,

 __all__ = ["Table", "Chain", "Rule", "Match", "Target", "Policy", "IPTCError"]

+DEPRECATED_TARGETS = {"NOTRACK"}
+
 try:
     load_kernel("ip_tables")
 except:
@@ -687,6 +689,9 @@ class Target(IPTCModule):
         self._orig_parse = None
         self._orig_options = None

+        if name in DEPRECATED_TARGETS:
+            raise DeprecationWarning('Target "{0}" is deprecated!'.format(name))
+
         # NOTE:
         # get_ip() returns the 'ip' structure that contains (1)the 'flags' field, and
         # (2)the value for the GOTO flag.

So, we have a global constant DEPRECATED_TARGETS, and during Target instantiation, a simple check would raise DeprecationWarning exception if the Target name is in that set.

@ldx
Copy link
Owner

ldx commented Mar 7, 2017

Oh I see. Unfortunately there's more here than meets the eye: whether a target/match should be marked as "deprecated" also depends on the version. I'm also not a fan of baking this into class.

OTOH, it might be a good idea to update docs about this.

@pepoluan
Copy link
Contributor

pepoluan commented Mar 9, 2017

Fair enough. I too am not really too keen on enforcement of DeprecatedWarning like this.

Another idea would be to perform an (optional) auto-retrieval post-commit, and warn the user if the retrieved Target changes (e.g., from "NOTRACK" to "CT"). Not sure how much work to implement this kind of self-verification, though.

@jllorente
Copy link
Collaborator Author

I will close this issue. IMO the better way is using CT target anyway.
I hope this serves at least for future reference :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants