Skip to content

Fix private gateway ACLs#3316

Closed
DennisKonrad wants to merge 1 commit into
apache:masterfrom
DennisKonrad:staticroute-acl
Closed

Fix private gateway ACLs#3316
DennisKonrad wants to merge 1 commit into
apache:masterfrom
DennisKonrad:staticroute-acl

Conversation

@DennisKonrad

@DennisKonrad DennisKonrad commented May 7, 2019

Copy link
Copy Markdown
Contributor

Description

Problem
At the moment ACLs attached to a private gateway are not working. The rules are created on the virtual router but multiple small errors prevent their evaluation.

Fix # 1
This commit changes the ordering of rules in the FORWARD table so the custom chains created are evaluated.

Fix # 2
Also ACLs on private gateways should not only be valid for the VPC CIDR but also for networks that are reached via static routes. This is achieved by considering all traffic that goes through the relevant network interface.

Fix # 3
This commit sets the right interface for the ACL_INBOUND_ethX chains. Before the interface was set on "out" even though the rules states it's inbound. TODO push commit
( self.fw.append(["filter", "", "-A FORWARD -i %s -j ACL_INBOUND_%s" % (self.dev, self.dev)]) )
-> CsAddress.py:444

Fix # 4
ACL_OUTBOUND rules should also be filtered -> Move them to filter

Fix # 5
To allow for correct private gateway ACL evaluation the argument already generated by the java code needed to be evaluated.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

We tested this by manually applying the changes to the .py files. Also one has to move or delete the .pyc files to experience the changes.

used

Also the order of FORWARD rules is fixed so custom chaines are checked.
This wasn't the case before. They were created but never reached (like
dead code but in iptables)
@yadvr

yadvr commented May 27, 2019

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2800

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-3595)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30425 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3316-t3595-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 70 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@DennisKonrad

Copy link
Copy Markdown
Contributor Author

This is still in development.
Ingoing rules are respected now. Outgoing are not working right now.

@yadvr

yadvr commented Jun 29, 2019

Copy link
Copy Markdown
Member

@DennisKonrad please ping me when you think the PR is ready for testing and review, I'll help kick tests.

@yadvr yadvr modified the milestones: 4.13.0.0, 4.13.1.0 Jul 4, 2019
@yadvr yadvr modified the milestones: 4.13.1.0, 4.14.0.0 Dec 7, 2019
@andrijapanicsb

Copy link
Copy Markdown
Contributor

@DennisKonrad any progress on this one, or should we move it to 4.15 milestone?

@andrijapanicsb

Copy link
Copy Markdown
Contributor

@DennisKonrad this is very sensitive stuff.
Can I kindly ask for the exact description of the issue - I would like to see the issue / reproduce / test the fix

@yadvr yadvr closed this Jan 28, 2020
@yadvr yadvr reopened this Jan 28, 2020
@yadvr

yadvr commented Jan 28, 2020

Copy link
Copy Markdown
Member

@DennisKonrad I've moved it to 4.15, let me know otherwise if this is ready for review/testing.

@yadvr yadvr modified the milestones: 4.14.0.0, 4.15.0.0 Jan 28, 2020
@yadvr yadvr self-assigned this Jan 28, 2020
@DennisKonrad DennisKonrad changed the base branch from master to 4.13 January 28, 2020 09:16
@DennisKonrad DennisKonrad changed the base branch from 4.13 to master January 28, 2020 09:17
@yadvr

yadvr commented Feb 6, 2020

Copy link
Copy Markdown
Member

Courtesy ping - @DennisKonrad

@DaanHoogland DaanHoogland marked this pull request as draft May 7, 2020 09:02
@DaanHoogland DaanHoogland changed the title [WIP DO NOT MERGE] Fix private gateway ACLs Fix private gateway ACLs May 7, 2020
@DennisKonrad

Copy link
Copy Markdown
Contributor Author

Currently I can make no time to push this PR to a point where we could move on. I will open another PR when this is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants