You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by pdube <gi...@git.apache.org> on 2016/06/02 17:29:04 UTC
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
GitHub user pdube opened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by kishankavala <gi...@git.apache.org>.
Github user kishankavala commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube Current fix is good enough. I'm only concerned about more such issues in VR.
can you please create a tracking bug for VR fix and make a note that this is a regression?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the issue:
https://github.com/apache/cloudstack/pull/1581
I just kicked off a build that includes this PR. I'm going to push this to our lab for testing and report back.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by kishankavala <gi...@git.apache.org>.
Github user kishankavala commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@leprechau ACL rule ordering was introduced in 2013 in ACS 4.2. Since 4.2 release, the order was never changed.
Older commit your are referring to, was during development phase of 4.2.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by kishankavala <gi...@git.apache.org>.
Github user kishankavala commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@swill This is a regression caused by VR refactor. There could be more such issues. I would prefer a fix in the VR script. Current fix is more like reversing the order twice to make it correct.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube Thanks, will give it a try soon
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by kishankavala <gi...@git.apache.org>.
Github user kishankavala commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube though the fix looks good, the root of the issue is different.
Earlier when bash scripts were used to configure rules on VR, iptable rules for ACLs were inserted (-I option).
This changed to add (-A option) after VR refactor, resulting in rules being applied in the reverse order.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by leprechau <gi...@git.apache.org>.
Github user leprechau commented on the issue:
https://github.com/apache/cloudstack/pull/1581
Looking through the ```git blame``` for this section of code it appears the operator and the ordering of the list was the same as this patch prior to 2013 when the operator was switched.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
GitHub user pdube reopened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
commit caf4a48075e0f59b5d101efdd3ac6b1bee8f4f39
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:15:38Z
Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
commit 4c97a3981dc0d543e02f62f2bb4fc2eb805545c6
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:44:39Z
Added unit test to verify ordering
commit 9cdd23fdc77e643d886c3af8cb0a60f9c4ddf84f
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-03T12:48:47Z
Added ASF license to unit test file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by spellgen <gi...@git.apache.org>.
Github user spellgen commented on the issue:
https://github.com/apache/cloudstack/pull/1581
the changes look good to me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1581#discussion_r65701961
--- Diff: core/test/com/cloud/agent/api/routing/SetNetworkACLCommandTest.java ---
@@ -0,0 +1,34 @@
+package com.cloud.agent.api.routing;
--- End diff --
Thanks, updated it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@remibergsma Yes, the rules should appear in the right order now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1581
The only known issue is that it this periodically happens. I have not had a chance to dig deeper...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the issue:
https://github.com/apache/cloudstack/pull/1581
### ACS CI BVT Run
**Sumarry:**
Build Number 207
Hypervisor xenserver
NetworkType Advanced
Passed=68
Failed=5
Skipped=3
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_vpc_vpn.py
* ContextSuite context=TestRVPCSite2SiteVpn>:setup Failing since 24 runs
* ContextSuite context=TestVpcRemoteAccessVpn>:setup Failing since 23 runs
* ContextSuite context=TestVpcSite2SiteVpn>:setup Failing since 22 runs
* test_volumes.py
* test_06_download_detached_volume Failed
* test_vm_life_cycle.py
* test_10_attachAndDetach_iso Failed
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_login.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
GitHub user pdube reopened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
commit caf4a48075e0f59b5d101efdd3ac6b1bee8f4f39
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:15:38Z
Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
commit 4c97a3981dc0d543e02f62f2bb4fc2eb805545c6
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:44:39Z
Added unit test to verify ordering
commit 9cdd23fdc77e643d886c3af8cb0a60f9c4ddf84f
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-03T12:48:47Z
Added ASF license to unit test file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
GitHub user pdube reopened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
commit caf4a48075e0f59b5d101efdd3ac6b1bee8f4f39
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:15:38Z
Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
commit 4c97a3981dc0d543e02f62f2bb4fc2eb805545c6
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:44:39Z
Added unit test to verify ordering
commit 9cdd23fdc77e643d886c3af8cb0a60f9c4ddf84f
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-03T12:48:47Z
Added ASF license to unit test file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube Travis timed out, you may want to try the Travis Lottery again ;-)
Is this ready for testing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1581#discussion_r65601112
--- Diff: core/test/com/cloud/agent/api/routing/SetNetworkACLCommandTest.java ---
@@ -0,0 +1,34 @@
+package com.cloud.agent.api.routing;
--- End diff --
@pdube Jenkins fails because this file has no license.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by leprechau <gi...@git.apache.org>.
Github user leprechau commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@kishankavala Understood, thank you for the explanation. The long history of the project makes digging into some of these things difficult.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1581
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 0
Errors: 0
Duration: 4h 01m 52s
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__Jun_13_2016_17_06_42_SFRV4J:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/DeployDataCenter__Jun_13_2016_17_06_42_SFRV4J/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/DeployDataCenter__Jun_13_2016_17_06_42_SFRV4J/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/DeployDataCenter__Jun_13_2016_17_06_42_SFRV4J/runinfo.txt)
**`/tmp/MarvinLogs/test_network_8S1TLA:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/test_network_8S1TLA/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/test_network_8S1TLA/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/test_network_8S1TLA/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_SYNGX8:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/test_vpc_routers_SYNGX8/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/test_vpc_routers_SYNGX8/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1581/tmp/MarvinLogs/test_vpc_routers_SYNGX8/runinfo.txt)
Uploads will be available until `2016-08-14 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@kishankavala I think that the ultimate fix will be in the VR. However, the inversion of the list is fixed with this patch, and does not require a VR update. This is a good enough fix for now, as the ordering inversion is a critical security bug, since the rule numbers you are giving are not being applied as expected.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by leprechau <gi...@git.apache.org>.
Github user leprechau commented on the issue:
https://github.com/apache/cloudstack/pull/1581
Looks clean and the added test case should prevent this from happening again. According to the "git blame" the comparator was switched back in 2013 and has been broken since.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
GitHub user pdube reopened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
commit caf4a48075e0f59b5d101efdd3ac6b1bee8f4f39
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:15:38Z
Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
commit 4c97a3981dc0d543e02f62f2bb4fc2eb805545c6
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:44:39Z
Added unit test to verify ordering
commit 9cdd23fdc77e643d886c3af8cb0a60f9c4ddf84f
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-03T12:48:47Z
Added ASF license to unit test file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube can you review @kishankavala's comments. I want to cut an RC soon, but we have other pending issues in the VR right now which @kiwiflyer, @dmabry and team are working on...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the issue:
https://github.com/apache/cloudstack/pull/1581
Thanks @dmabry
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube can you close and reopen this one? thx...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1581
I'm not sure what to make of this comment @kishankavala. Is that a LGTM?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
GitHub user pdube reopened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
commit caf4a48075e0f59b5d101efdd3ac6b1bee8f4f39
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:15:38Z
Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
commit 4c97a3981dc0d543e02f62f2bb4fc2eb805545c6
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:44:39Z
Added unit test to verify ordering
commit 9cdd23fdc77e643d886c3af8cb0a60f9c4ddf84f
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-03T12:48:47Z
Added ASF license to unit test file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
GitHub user pdube reopened a pull request:
https://github.com/apache/cloudstack/pull/1581
CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR.
The comparator was inverted.
Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404
In this example, I created rules with the port numbers the same as the rule numbers.
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:2
DROP all -- anywhere anywhere
We can see above that the rules are inverted.
After the fix:
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- anywhere 225.0.0.50
ACCEPT all -- anywhere vrrp.mcast.net
DROP tcp -- anywhere anywhere tcp dpt:2
DROP tcp -- anywhere anywhere tcp dpt:3
DROP tcp -- anywhere anywhere tcp dpt:5
DROP tcp -- anywhere anywhere tcp dpt:10
DROP tcp -- anywhere anywhere tcp dpt:netstat
DROP all -- anywhere anywhere
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pdube/cloudstack network-acl-rules-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1581.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1581
----
commit caf4a48075e0f59b5d101efdd3ac6b1bee8f4f39
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:15:38Z
Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
commit 4c97a3981dc0d543e02f62f2bb4fc2eb805545c6
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-02T17:44:39Z
Added unit test to verify ordering
commit 9cdd23fdc77e643d886c3af8cb0a60f9c4ddf84f
Author: Patrick Dube <pd...@cloudops.com>
Date: 2016-06-03T12:48:47Z
Added ASF license to unit test file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the issue:
https://github.com/apache/cloudstack/pull/1581
I tested this in our lab with advanced networking verified the patch is working as expected. I used the following test process.
1. Created an acl and applied it to 1 VPC Network Tier.
```
10 192.168.10.0/24 Allow ALL Ingress
20 192.168.20.0/24 Allow ALL Ingress
30 192.168.30.0/24 Allow ALL Ingress
```
2. iptables looked like the following on the VPC VR
```
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- 0.0.0.0/0 225.0.0.50
ACCEPT all -- 0.0.0.0/0 224.0.0.18
ACCEPT all -- 192.168.10.0/24 0.0.0.0/0
ACCEPT all -- 192.168.20.0/24 0.0.0.0/0
ACCEPT all -- 192.168.30.0/24 0.0.0.0/0
DROP all -- 0.0.0.0/0 0.0.0.0/0
```
3. I added an additional rule of:
```
40 192.168.40.0/24 Allow TCP 80 80 Ingress
```
4. iptables looked like the following on the VPC VR
```
Chain ACL_INBOUND_eth2 (1 references)
target prot opt source destination
ACCEPT all -- 0.0.0.0/0 225.0.0.50
ACCEPT all -- 0.0.0.0/0 224.0.0.18
ACCEPT all -- 192.168.10.0/24 0.0.0.0/0
ACCEPT all -- 192.168.20.0/24 0.0.0.0/0
ACCEPT all -- 192.168.30.0/24 0.0.0.0/0
ACCEPT tcp -- 192.168.40.0/24 0.0.0.0/0 tcp dpt:80
DROP all -- 0.0.0.0/0 0.0.0.0/0
```
In summary, it looks like this patch works verified by manual testing in my lab.
In short, LGTM based on testing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the issue:
https://github.com/apache/cloudstack/pull/1581
@pdube No problem. Think you could kick this off again and see if Travis comes back green?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1581: CLOUDSTACK-9404 Fixed ordering of network ACL rules ...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the issue:
https://github.com/apache/cloudstack/pull/1581
The Travis build has timed out 3 times now. Is there any known issue with Travis right now @swill ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1581: CLOUDSTACK-9404 Fixed ordering of network ACL...
Posted by pdube <gi...@git.apache.org>.
Github user pdube closed the pull request at:
https://github.com/apache/cloudstack/pull/1581
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---