You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Jayapal Reddy <ja...@citrix.com> on 2013/05/17 20:53:25 UTC
Review Request: Fixed SRX icmp firewall rule configuration issue
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/
-----------------------------------------------------------
Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
Description
-------
1. Updated to configure the firewall filter for the icmp protocol
2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
This addresses bug CLOUDSTACK-2386.
Diffs
-----
plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
utils/src/com/cloud/utils/net/NetUtils.java 9551c26
Diff: https://reviews.apache.org/r/11224/diff/
Testing
-------
1. Added icmp firewall rule and tested ping to public ip from the public subnet
2. Tested configuring Static NAT and PF
Thanks,
Jayapal Reddy
Re: Review Request: Fixed SRX icmp firewall rule configuration issue
Posted by Sheng Yang <sh...@yasker.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/#review20857
-----------------------------------------------------------
I am fine with "Ship it".
- Sheng Yang
On May 20, 2013, 5:55 a.m., Jayapal Reddy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11224/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 5:55 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
>
>
> Description
> -------
>
> 1. Updated to configure the firewall filter for the icmp protocol
> 2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
>
>
> This addresses bug CLOUDSTACK-2386.
>
>
> Diffs
> -----
>
> plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 4a90a77
> utils/src/com/cloud/utils/net/NetUtils.java 9551c26
>
> Diff: https://reviews.apache.org/r/11224/diff/
>
>
> Testing
> -------
>
> 1. Added icmp firewall rule and tested ping to public ip from the public subnet
> 2. Tested configuring Static NAT and PF
>
>
> Thanks,
>
> Jayapal Reddy
>
>
Re: Review Request: Fixed SRX icmp firewall rule configuration issue
Posted by Abhinandan Prateek <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/#review20937
-----------------------------------------------------------
Ship it!
Ship It!
- Abhinandan Prateek
On May 20, 2013, 5:55 a.m., Jayapal Reddy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11224/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 5:55 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
>
>
> Description
> -------
>
> 1. Updated to configure the firewall filter for the icmp protocol
> 2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
>
>
> This addresses bug CLOUDSTACK-2386.
>
>
> Diffs
> -----
>
> plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 4a90a77
> utils/src/com/cloud/utils/net/NetUtils.java 9551c26
>
> Diff: https://reviews.apache.org/r/11224/diff/
>
>
> Testing
> -------
>
> 1. Added icmp firewall rule and tested ping to public ip from the public subnet
> 2. Tested configuring Static NAT and PF
>
>
> Thanks,
>
> Jayapal Reddy
>
>
Re: Review Request: Fixed SRX icmp firewall rule configuration issue
Posted by Sheng Yang <sh...@yasker.org>.
> On May 20, 2013, 7 p.m., Sheng Yang wrote:
> > plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java, line 854
> > <https://reviews.apache.org/r/11224/diff/2/?file=293966#file293966line854>
> >
> > I think it's wrong here. Firewall is only firewall, it won't and shouldn't response to the ICMP or other request, unless port forwarding or static nat rule configured.
>
> Jayapal Reddy wrote:
> In firewall when ICMP rule is configured and there is no static NAT or PF
> - In this case public interface IP will give the response.
> Static NAT + Firewall ICMP
> - In this case VM will give the response.
>
> This behaviour is also same in VR
In the past, VR didn't associate IP address when apply firewall rules. But it's complex to deal with firewall rules differently from static nat and port forwarding/load balancing rules, so we change it to today's behavior. But it's still uncommon to have different entity response to the same IP. We need document this behavior.
> On May 20, 2013, 7 p.m., Sheng Yang wrote:
> > plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java, line 845
> > <https://reviews.apache.org/r/11224/diff/2/?file=293966#file293966line845>
> >
> > Why ICMP's getSrcPortRange is null(then need the min/max default value)?
>
> Jayapal Reddy wrote:
> For ICMP protocol we are not passing the port range. We are only passing the icml type and code. So for ICMP protocol we are getting getSrcPortRange() as null because there are ports passed for icml.
Yes, seems you also assume TCP/UDP port can have null port range, in order to get 0~65535 port all open.
- Sheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/#review20784
-----------------------------------------------------------
On May 20, 2013, 5:55 a.m., Jayapal Reddy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11224/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 5:55 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
>
>
> Description
> -------
>
> 1. Updated to configure the firewall filter for the icmp protocol
> 2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
>
>
> This addresses bug CLOUDSTACK-2386.
>
>
> Diffs
> -----
>
> plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 4a90a77
> utils/src/com/cloud/utils/net/NetUtils.java 9551c26
>
> Diff: https://reviews.apache.org/r/11224/diff/
>
>
> Testing
> -------
>
> 1. Added icmp firewall rule and tested ping to public ip from the public subnet
> 2. Tested configuring Static NAT and PF
>
>
> Thanks,
>
> Jayapal Reddy
>
>
Re: Review Request: Fixed SRX icmp firewall rule configuration issue
Posted by Jayapal Reddy <ja...@citrix.com>.
> On May 20, 2013, 7 p.m., Sheng Yang wrote:
> > plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java, line 845
> > <https://reviews.apache.org/r/11224/diff/2/?file=293966#file293966line845>
> >
> > Why ICMP's getSrcPortRange is null(then need the min/max default value)?
For ICMP protocol we are not passing the port range. We are only passing the icml type and code. So for ICMP protocol we are getting getSrcPortRange() as null because there are ports passed for icml.
> On May 20, 2013, 7 p.m., Sheng Yang wrote:
> > plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java, line 854
> > <https://reviews.apache.org/r/11224/diff/2/?file=293966#file293966line854>
> >
> > I think it's wrong here. Firewall is only firewall, it won't and shouldn't response to the ICMP or other request, unless port forwarding or static nat rule configured.
In firewall when ICMP rule is configured and there is no static NAT or PF
- In this case public interface IP will give the response.
Static NAT + Firewall ICMP
- In this case VM will give the response.
This behaviour is also same in VR
- Jayapal
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/#review20784
-----------------------------------------------------------
On May 20, 2013, 5:55 a.m., Jayapal Reddy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11224/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 5:55 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
>
>
> Description
> -------
>
> 1. Updated to configure the firewall filter for the icmp protocol
> 2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
>
>
> This addresses bug CLOUDSTACK-2386.
>
>
> Diffs
> -----
>
> plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 4a90a77
> utils/src/com/cloud/utils/net/NetUtils.java 9551c26
>
> Diff: https://reviews.apache.org/r/11224/diff/
>
>
> Testing
> -------
>
> 1. Added icmp firewall rule and tested ping to public ip from the public subnet
> 2. Tested configuring Static NAT and PF
>
>
> Thanks,
>
> Jayapal Reddy
>
>
Re: Review Request: Fixed SRX icmp firewall rule configuration issue
Posted by Sheng Yang <sh...@yasker.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/#review20784
-----------------------------------------------------------
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
<https://reviews.apache.org/r/11224/#comment42875>
Why ICMP's getSrcPortRange is null(then need the min/max default value)?
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
<https://reviews.apache.org/r/11224/#comment42876>
I think it's wrong here. Firewall is only firewall, it won't and shouldn't response to the ICMP or other request, unless port forwarding or static nat rule configured.
- Sheng Yang
On May 20, 2013, 5:55 a.m., Jayapal Reddy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11224/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 5:55 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
>
>
> Description
> -------
>
> 1. Updated to configure the firewall filter for the icmp protocol
> 2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
>
>
> This addresses bug CLOUDSTACK-2386.
>
>
> Diffs
> -----
>
> plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 4a90a77
> utils/src/com/cloud/utils/net/NetUtils.java 9551c26
>
> Diff: https://reviews.apache.org/r/11224/diff/
>
>
> Testing
> -------
>
> 1. Added icmp firewall rule and tested ping to public ip from the public subnet
> 2. Tested configuring Static NAT and PF
>
>
> Thanks,
>
> Jayapal Reddy
>
>
Re: Review Request: Fixed SRX icmp firewall rule configuration issue
Posted by Jayapal Reddy <ja...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11224/
-----------------------------------------------------------
(Updated May 20, 2013, 5:55 a.m.)
Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Murali Reddy.
Changes
-------
Updated patch.
Description
-------
1. Updated to configure the firewall filter for the icmp protocol
2. Proxy arp is required for icmp response on SRX public IP. So adding proxy arp along with firewall rules
This addresses bug CLOUDSTACK-2386.
Diffs (updated)
-----
plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java a429306
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java a0068c3
server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 4a90a77
utils/src/com/cloud/utils/net/NetUtils.java 9551c26
Diff: https://reviews.apache.org/r/11224/diff/
Testing
-------
1. Added icmp firewall rule and tested ping to public ip from the public subnet
2. Tested configuring Static NAT and PF
Thanks,
Jayapal Reddy