"established" option should allow "all" as a protocol parameter, at least for iptables definitions about capirca HOT 5 CLOSED

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

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.

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?

>> 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).