"established" option should allow "all" as a protocol parameter, at least for iptables definitions

[GoogleCodeExporter]

What steps will reproduce the problem?
1. This is a valid command "iptables -A INPUT -p all -m state --state
ESTABLISHED,RELATED -j ACCEPT"

2. capirca will not allow us to generate this though.

term allow-established-all {
  option:: established
  protocol:: all
  action:: accept
}

3. $ ./aclgen.py
Traceback (most recent call last):
  File "./aclgen.py", line 139, in <module>
    main()
  File "./aclgen.py", line 135, in main
    parse_policies(policies_to_render, defs)
  File "./aclgen.py", line 100, in parse_policies
    p = policy.ParsePolicy(open(pol).read(), defs)
  File "/home/khermansen/projects/capirca-read-only.mt.2/lib/policy.py",
line 1240, in ParsePolicy
    return yacc.parse(data)
  File
"/home/khermansen/projects/capirca-read-only.mt.2/third_party/ply/yacc.py",
line 265, in parse
    return self.parseopt_notrack(input,lexer,debug,tracking,tokenfunc)
  File
"/home/khermansen/projects/capirca-read-only.mt.2/third_party/ply/yacc.py",
line 971, in parseopt_notrack
    p.callable(pslice)
  File "/home/khermansen/projects/capirca-read-only.mt.2/lib/policy.py",
line 987, in p_target
    p[0] = Policy(p[2], p[3])
  File "/home/khermansen/projects/capirca-read-only.mt.2/lib/policy.py",
line 123, in __init__
    self.AddFilter(header, terms)
  File "/home/khermansen/projects/capirca-read-only.mt.2/lib/policy.py",
line 128, in AddFilter
    self._TranslateTerms(terms)
  File "/home/khermansen/projects/capirca-read-only.mt.2/lib/policy.py",
line 155, in _TranslateTerms
    term.name))
lib.policy.TermPortProtocolError: no destination ports of the correct
protocol for term allow-established

What is the expected output? What do you see instead?

Expected output should be a correct .ipt file with the proper definitions
including a line such as "-A INPUT -p all -m state --state
ESTABLISHED,RELATED -j ACCEPT"

Please use labels and text to provide additional information.

At this time, capirca's current limitation allows protocols such as tcp/udp
to work with established connection, but not with protocols such as icmp. 
So, a simple scenario is that there is a default DROP policy on INPUT. 
However, when user attempts ICMP ECHO REQUEST from local machine, the ICMP
ECHO REPLY will be dropped.  capirca should allow all protocols with the
"established" option so that such issues like this are easily resolved.

Original issue reported on code.google.com by kristian...@gmail.com on 26 Apr 2010 at 1:03

[GoogleCodeExporter]

If not allowing "all" as a parameter, at least "icmp" should not fail...

Original comment by kristian...@gmail.com on 26 Apr 2010 at 1:29

[GoogleCodeExporter]

This issue is known, and is problematic to fix due to the multiple platforms 
needed 
to be supported and the fact some support stateful filtering (iptables), while 
others 
do not (cisco/juniper).  

If you only need Iptables generation, this can be fixed with the following 
change.
Insert the following diff to lib/policy.py in function _TranslateTerms() and it 
will 
work correctly for iptables output.

--- lib/policy.py       (revision 80)
+++ lib/policy.py       (working copy)
@@ -134,6 +134,12 @@
     for term in terms:
       # todo(pmoody): this probably belongs in Term.SanityCheck(),
       # or at the very least, in some method under class Term()
+
+      # while parsing, we include HIGH_PORTS when 'established' is specified,
+      # but this should only apply to protocols with ports such as tcp/udp.
+      if 'established' in term.option:
+        if 'tcp' not in term.protocol and 'udp' not in term.protocol:
+          term.destination_port = []
       if term.port:
         term.port = TranslatePorts(term.port, term.protocol)
         if not term.port:

However, the drawback to doing this is non-stateful platforms like Cisco and 
Juniper 
output will produce some undesirable and dangerous output. For example, a term 
permitting all protocols with 'established' option will result in output such 
as the 
following for target cisco:

permit all any any

Note that this results in a default allow rule, instead of a rule permitting 
only 
established connections.  This is due to the fact that Cisco and Juniper 
filters do 
not support 'stateful' firewalling, while Iptables does.  Outputing 'permit all 
any 
any established' would result in an error while trying to enter it on a Cisco.

I need to think about how to resolve this for future releases.  Perhaps we can 
tag 
each target platform as supporting stateful or non-stateful filtering, and do 
the 
right thing where possible, and erroring only when the combination of target 
and 
options will result in an insecure output.

Original comment by watson on 26 Apr 2010 at 4:22

• Added labels: Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

This patch alone does not seem to resolve the issue.  It appears to me that 
after
applying the suggested patch, capirca still omits the "-m state --state
ESTABLISHED,RELATED" portion.  Additionally, I still believe that only allowing 
high
ports is flawed.  For instance, take the example of an ntpdate invocation that
originates on source port udp/123 destined for port udp/123.  Using the current
implementation in capirca, the reply packet would be dropped :)  So, I think 
just
removing the HIGH_PORTS designation for iptables rules (at least) is more 
accurate. 
There is no need to specify the ports since the ESTABLISHED,RELATED terms will 
take
care of whatever the initiated ports are automatically.

Thoughts?

Original comment by kristian...@gmail.com on 26 Apr 2010 at 8:18

[GoogleCodeExporter]

>> So, I think just removing the HIGH_PORTS designation for iptables rules (at 
least) is more accurate. There is no need to specify the ports since the 
ESTABLISHED,RELATED terms will take care of whatever the initiated ports are 
automatically.

The ESTABLISHED,RELATED are only used with stateful filtering.  The iptables 
generator allows the creation of both stateful (conn_track) as well as 
stateless (specify 'nostate' argument in the target:: line).

Original comment by watson@google.com on 12 Jul 2011 at 7:03

[GoogleCodeExporter]

Original comment by watson@google.com on 14 Jul 2011 at 7:57

• Changed state: WontFix