From 8ba2ad10e88cd0ac04898d68247a43a149d9fe1f Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Thu, 21 Mar 2024 16:27:52 -0600 Subject: [PATCH 01/19] style(black): ran black --- .idea/misc.xml | 5 ++++- translator/gobgp.py | 28 +++++++++++++++++++++------- translator/translator.py | 4 ++-- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/.idea/misc.xml b/.idea/misc.xml index 1570102f..fe7aed4e 100644 --- a/.idea/misc.xml +++ b/.idea/misc.xml @@ -1,7 +1,10 @@ + + - \ No newline at end of file + diff --git a/translator/gobgp.py b/translator/gobgp.py index 4bec099e..80acafd3 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -22,7 +22,19 @@ def _get_family(self, ip_version): else: return gobgp_pb2.Family.AFI_IP - def _build_path(self, ip): + def _build_path(self, ip, event_data): + logging.info(f"ip: {ip}, event_data: {event_data}") + if "asn" in event_data: + asn = event_data["asn"] + else: + asn = 293 + if "community" in event_data: + community = event_data["community"] + else: + community = 666 + + logging.info(f"{ip} {asn} {community}") + origin = Any() origin.Pack( attribute_pb2.OriginAttribute( @@ -58,7 +70,7 @@ def _build_path(self, ip): ) communities = Any() - comm_id = (293 << 16) + 666 + comm_id = (asn << 16) + community communities.Pack(attribute_pb2.CommunitiesAttribute(communities=[comm_id])) attributes = [origin, next_hop, communities] @@ -69,10 +81,12 @@ def _build_path(self, ip): family=gobgp_pb2.Family(afi=family, safi=gobgp_pb2.Family.SAFI_UNICAST), ) - def add_path(self, ip): - path = self._build_path(ip) + def add_path(self, ip, event_data): + logging.info(f"ip: {ip}, event_data: {event_data}") + + path = self._build_path(ip, event_data) - logging.info(f"Blocking {ip}") + logging.info(f"Blocking {event_data}") self.stub.AddPath( gobgp_pb2.AddPathRequest(table_type=gobgp_pb2.GLOBAL, path=path), @@ -84,8 +98,8 @@ def del_all_paths(self): self.stub.DeletePath(gobgp_pb2.DeletePathRequest(table_type=gobgp_pb2.GLOBAL), _TIMEOUT_SECONDS) - def del_path(self, ip): - path = self._build_path(ip) + def del_path(self, ip, event_data): + path = self._build_path(ip, event_data) logging.info(f"Unblocking {ip}") diff --git a/translator/translator.py b/translator/translator.py index 45641734..c9367fae 100644 --- a/translator/translator.py +++ b/translator/translator.py @@ -40,9 +40,9 @@ async def main(): continue if event_type == "translator_add": - g.add_path(ip) + g.add_path(ip, event_data) elif event_type == "translator_remove": - g.del_path(ip) + g.del_path(ip, event_data) elif event_type == "translator_check": json_message["type"] = "translator_check_resp" json_message["message"]["is_blocked"] = g.is_blocked(ip) From cbfb55553e44f128910a6bcf9fffc968b6938eaa Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 22 Mar 2024 10:03:01 -0600 Subject: [PATCH 02/19] docs(decisions): update decisions to explain how the configurable paylods portion works. --- docs/decisions.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index 3de1112f..9891c6e4 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -53,12 +53,19 @@ Any API clients will connect to the SCRAM REST API, which is over HTTPS. We can information being sent to the API from the client. We wanted to centralize the administration of the clients, so it seemed natural to use the built-in django admin site. The client is allowed to start up and hit a POST only endpoint on first connection where it registers itself with SCRAM. We only allow the client to tell us its fqdn and a unique -identifier. The SCRAM admin is expected to go to the admin site to toggle the boolean for setting the client as +identifier. The SCRAM administrator is expected to go to the admin site to toggle the boolean for setting the client as authorized, as well as choose from any of the list of available actiontypes. During an entry's creation, we verify this -client is allowed to create an entry using the actiontype it is giving. +client is allowed to create an entry using the actiontype it is providing. This model does allow for anyone to "register" a client, but until someone with admin level permissions goes to the admin site and accepts the registration and sets authorized actiontypes, this client cannot create any entries and therefore, not affect any sort of change. This endpoint is POST only, so nobody should be allowed to see what clients exist unless they can access the admin site. Our main security concern after all this is a DoS by constantly POSTing to this endpoint, which can be handled the same way any other DoS would be dealth with (ie likely blocked via SCRAM). + +#### Configurable Payloads +Configurable payloads allows you to override certain data sent to the translator. Currently, this is ASN and BGP community. +In order to do so, you update the JSON dictionary inside the Web Socket Message entry in the admin page. One thing to note +is that you must include a "route" key in that dictionary with a string value of any kind. If you decide to change the +key in the JSON payload whose value will contain the route being acted on field, you must change the route key in the +dictionary to match that field. From f3e4af16ddbc141b2010a8dc666161b9c5414135 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 22 Mar 2024 10:04:20 -0600 Subject: [PATCH 03/19] test(fix): who is a required field for API creates --- scram/route_manager/tests/test_websockets.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scram/route_manager/tests/test_websockets.py b/scram/route_manager/tests/test_websockets.py index 8733ebf9..dcda2f53 100644 --- a/scram/route_manager/tests/test_websockets.py +++ b/scram/route_manager/tests/test_websockets.py @@ -125,6 +125,7 @@ def api_create_entry(self, route): "route": route, "comment": "test", "uuid": self.uuid, + "who": "Test User", }, format="json", ) From 0e83c60d49ef9f30e6c942bdabbc84ba55136be1 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 22 Mar 2024 14:52:39 -0600 Subject: [PATCH 04/19] test(UNDO-AFTER-TESTING): comment out steps/translator.py calls translator.py calls are erroring out in CI but not locally. just curious what happens if we skip those --- .../acceptance/features/add_automated_block_entry.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature b/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature index 83af7dc5..62a048f6 100644 --- a/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature +++ b/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature @@ -15,7 +15,7 @@ Feature: an automated source adds a block entry Then the number of entrys is 1 And is one of our list of entrys And is contained in our list of entrys - And is announced by block translators +# And is announced by block translators Examples: v4 IPs | ip | cidr | @@ -78,7 +78,7 @@ Feature: an automated source adds a block entry And we list the entrys Then the number of entrys is 1 - And is announced by block translators +# And is announced by block translators Examples: | ip | From 498588d66b95d78830ff8e6c9bb31a813a5f74eb Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 22 Mar 2024 15:03:49 -0600 Subject: [PATCH 05/19] test(UNDO-AFTER-TESTING): get ALL the block translator calls --- .../acceptance/features/expiration.feature | 34 +++++++++---------- .../features/restrict_changes.feature | 4 +-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/scram/route_manager/tests/acceptance/features/expiration.feature b/scram/route_manager/tests/acceptance/features/expiration.feature index 27b1684f..ce850abd 100644 --- a/scram/route_manager/tests/acceptance/features/expiration.feature +++ b/scram/route_manager/tests/acceptance/features/expiration.feature @@ -10,14 +10,14 @@ Feature: entries auto-expire And we add the entry 192.0.2.3/32 with expiration 2030-01-01T00:00:00Z And we add the entry 192.0.2.4/32 with expiration in 12 seconds Then the number of entrys is 4 - And 192.0.2.1/32 is announced by block translators - And 192.0.2.2/32 is announced by block translators - And 192.0.2.3/32 is announced by block translators - And 192.0.2.4/32 is announced by block translators +# And 192.0.2.1/32 is announced by block translators +# And 192.0.2.2/32 is announced by block translators +# And 192.0.2.3/32 is announced by block translators +# And 192.0.2.4/32 is announced by block translators And we remove expired entries And the number of entrys is 2 - And 192.0.2.1/32 is not announced by block translators - And 192.0.2.3/32 is announced by block translators +# And 192.0.2.1/32 is not announced by block translators +# And 192.0.2.3/32 is announced by block translators And we wait 12 seconds And we remove expired entries And the number of entrys is 1 @@ -30,12 +30,12 @@ Feature: entries auto-expire And we add the entry 192.0.2.2/32 with expiration 2000-01-01T00:00:00Z And we add the entry 192.0.2.3/32 with expiration 2030-01-01T00:00:00Z Then the number of entrys is 3 - And 192.0.2.1/32 is announced by block translators - And 192.0.2.3/32 is announced by block translators +# And 192.0.2.1/32 is announced by block translators +# And 192.0.2.3/32 is announced by block translators And we remove expired entries And the number of entrys is 1 - And 192.0.2.1/32 is not announced by block translators - And 192.0.2.3/32 is announced by block translators +# And 192.0.2.1/32 is not announced by block translators +# And 192.0.2.3/32 is announced by block translators Scenario: Adding an IP twice gets expired after the last entry expires @@ -44,9 +44,9 @@ Feature: entries auto-expire When we're logged in And we add the entry 192.0.2.1/32 with expiration 1970-01-01T00:00:00Z And we add the entry 192.0.2.1/32 with expiration 2030-01-01T00:00:00Z - Then 192.0.2.1/32 is announced by block translators - And we remove expired entries - And 192.0.2.1/32 is announced by block translators +# Then 192.0.2.1/32 is announced by block translators + Then we remove expired entries +# And 192.0.2.1/32 is announced by block translators Scenario: Adding an IP twice gets expired after the last entry expires in 2 seconds Given a block actiontype is defined @@ -54,10 +54,10 @@ Feature: entries auto-expire When we're logged in And we add the entry 192.0.2.1/32 with expiration 1970-01-01T00:00:00Z And we add the entry 192.0.2.1/32 with expiration in 12 seconds - Then 192.0.2.1/32 is announced by block translators - And we remove expired entries +# Then 192.0.2.1/32 is announced by block translators + Then we remove expired entries And we wait 1 seconds - And 192.0.2.1/32 is announced by block translators +# And 192.0.2.1/32 is announced by block translators And we wait 13 seconds And we remove expired entries - And 192.0.2.1/32 is not announced by block translators +# And 192.0.2.1/32 is not announced by block translators diff --git a/scram/route_manager/tests/acceptance/features/restrict_changes.feature b/scram/route_manager/tests/acceptance/features/restrict_changes.feature index bb24eddc..32c050de 100644 --- a/scram/route_manager/tests/acceptance/features/restrict_changes.feature +++ b/scram/route_manager/tests/acceptance/features/restrict_changes.feature @@ -9,5 +9,5 @@ Feature: restrict changing entries And we update the entry 192.0.2.208 to 192.0.2.209 Then we get a 405 status code And the number of entrys is 1 - And 192.0.2.208 is announced by block translators - And 192.0.2.209 is not announced by block translators +# And 192.0.2.208 is announced by block translators +# And 192.0.2.209 is not announced by block translators From 5745f3ff49dfea248733d44825fddf75acff2829 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Mon, 25 Mar 2024 09:07:49 -0600 Subject: [PATCH 06/19] refactor(asn/community): clean up the asn/community variables code to be cleaner and less ESnet specific related to some comments in https://github.com/esnet-security/SCRAM/pull/33 --- translator/gobgp.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index 80acafd3..f2c9825e 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -23,17 +23,9 @@ def _get_family(self, ip_version): return gobgp_pb2.Family.AFI_IP def _build_path(self, ip, event_data): - logging.info(f"ip: {ip}, event_data: {event_data}") - if "asn" in event_data: - asn = event_data["asn"] - else: - asn = 293 - if "community" in event_data: - community = event_data["community"] - else: - community = 666 - - logging.info(f"{ip} {asn} {community}") + logging.debug(f"ip: {ip}, event_data: {event_data}") + asn = event_data.get("asn", 64500) + community = event_data.get("community", 666) origin = Any() origin.Pack( From b06c1c0e4ea01a794a62e8e4a34f3dc41351bf3b Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 26 Apr 2024 17:19:31 -0500 Subject: [PATCH 07/19] style(precommit): ran hooks --- translator/gobgp.py | 50 +++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index f2c9825e..95a3006e 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -23,17 +23,30 @@ def _get_family(self, ip_version): return gobgp_pb2.Family.AFI_IP def _build_path(self, ip, event_data): - logging.debug(f"ip: {ip}, event_data: {event_data}") - asn = event_data.get("asn", 64500) - community = event_data.get("community", 666) - + # defaults + asn = 64500 + community = 666 + + logging.debug(event_data) + # Pull asn and community from event_data if we have any + if event_data: + if "asn" in event_data: + asn = event_data["asn"] + if "community" in event_data: + community = event_data["community"] + + # Set the origin to incomplete (options are IGP, EGP, incomplete) + # Incomplete means that BGP is unsure of exactly how the prefix was injected into the topology. + # The most common scenario here is that the prefix was redistributed into Border Gateway Protocol + # from some other protocol, typically an IGP. - https://www.kwtrain.com/blog/bgp-pt2 origin = Any() origin.Pack( attribute_pb2.OriginAttribute( - origin=2, # INCOMPLETE + origin=2, ) ) + # IP prefix and its associated length nlri = Any() nlri.Pack( attribute_pb2.IPAddressPrefix( @@ -42,10 +55,10 @@ def _build_path(self, ip, event_data): ) ) + # Set the next hop to the correct value depending on IP family next_hop = Any() family = self._get_family(ip.ip.version) - - if ip.ip.version == 6: + if family == 6: next_hop.Pack( attribute_pb2.MpReachNLRIAttribute( family=gobgp_pb2.Family(afi=family, safi=gobgp_pb2.Family.SAFI_UNICAST), @@ -53,7 +66,6 @@ def _build_path(self, ip, event_data): nlris=[nlri], ) ) - else: next_hop.Pack( attribute_pb2.NextHopAttribute( @@ -61,11 +73,22 @@ def _build_path(self, ip, event_data): ) ) + # Set our AS Path + as_path = Any() + as_segment = None + + # Make sure our asn is an acceptable number. This is the max as stated in rfc6996 + assert asn < 4294967295 + as_segment = [attribute_pb2.AsSegment(numbers=[asn])] + as_segments = attribute_pb2.AsPathAttribute(segments=as_segment) + as_path.Pack(as_segments) + + # Set our community number + # TODO: We currently only accept one community number, we may want to accept more than one in the future communities = Any() - comm_id = (asn << 16) + community - communities.Pack(attribute_pb2.CommunitiesAttribute(communities=[comm_id])) + communities.Pack(attribute_pb2.CommunitiesAttribute(communities=[community])) - attributes = [origin, next_hop, communities] + attributes = [origin, next_hop, as_path, communities] return gobgp_pb2.Path( nlri=nlri, @@ -74,12 +97,9 @@ def _build_path(self, ip, event_data): ) def add_path(self, ip, event_data): - logging.info(f"ip: {ip}, event_data: {event_data}") - + logging.info(f"Blocking {ip}") path = self._build_path(ip, event_data) - logging.info(f"Blocking {event_data}") - self.stub.AddPath( gobgp_pb2.AddPathRequest(table_type=gobgp_pb2.GLOBAL, path=path), _TIMEOUT_SECONDS, From af7a206e8c587415f5847575c055ffc811e959ba Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 26 Apr 2024 23:30:05 -0500 Subject: [PATCH 08/19] test(translator): update translator tests to handle passing event_data use scenario outlines so we can test different ASNs and Communities; though that's outside the scope of this set of changes --- .../tests/acceptance/features/bgp.feature | 32 +++++++++---------- translator/tests/acceptance/steps/actions.py | 10 +++--- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/translator/tests/acceptance/features/bgp.feature b/translator/tests/acceptance/features/bgp.feature index e8c8f626..de0a0987 100644 --- a/translator/tests/acceptance/features/bgp.feature +++ b/translator/tests/acceptance/features/bgp.feature @@ -1,22 +1,22 @@ Feature: block with BGP Users can block routes via BGP - Scenario: We can block a v4 IP - When we add 192.0.2.4/32 to the block list - Then 192.0.2.4 is blocked - And 192.0.2.5 is unblocked + Scenario Outline: We can block an IP + When we add with and to the block list + Then is blocked + And is unblocked - Scenario: We can block a v6 IP - When we add 2001:DB8:A::/64 to the block list - Then 2001:DB8:A:: is blocked - And baba:: is unblocked + Examples: data + | route | asn | community | UnblockIP | + | 192.0.2.4/32 | 64500 | 666 | 192.0.2.5 | + | 2001:DB8:A::/64 | 64500 | 666 | baba:: | - Scenario: We can block, then unblock a v4 IP - When we add 192.0.2.4/32 to the block list - And we delete 192.0.2.4/32 from the block list - Then 192.0.2.4 is unblocked + Scenario Outline: We can block an IP + When we add with and to the block list + And we delete from the block list + Then is unblocked - Scenario: We can block, then unblock a v6 IP - When we add 2001:DB8:B::/64 to the block list - And we delete 2001:DB8:B::/64 from the block list - Then 2001:DB8:B:: is unblocked + Examples: data + | route | asn | community | UnblockIP | + | 192.0.2.4/32 | 64500 | 666 | 192.0.2.4 | + | 2001:DB8:A::/64 | 64500 | 666 | 2001:DB8:B:: | diff --git a/translator/tests/acceptance/steps/actions.py b/translator/tests/acceptance/steps/actions.py index 1db2e62b..44b4e634 100644 --- a/translator/tests/acceptance/steps/actions.py +++ b/translator/tests/acceptance/steps/actions.py @@ -4,16 +4,18 @@ from behave import then, when -@when("we add {route} to the block list") -def add_block(context, route): +@when("we add {route} with {asn} and {community} to the block list") +def add_block(context, route, asn, community): ip = ipaddress.ip_interface(route) - context.gobgp.add_path(ip) + event_data = {"asn": int(asn), "community": int(community)} + context.gobgp.add_path(ip, event_data) @when("we delete {route} from the block list") def del_block(context, route): ip = ipaddress.ip_interface(route) - context.gobgp.del_path(ip) + event_data = {"asn": 64500, "community": 666} + context.gobgp.del_path(ip, event_data) def get_block_status(context, ip): From adc68d2c158d0619a82fbe7d5b0a3ad64e325c1e Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 26 Apr 2024 23:42:42 -0500 Subject: [PATCH 09/19] test(announcer-checks): turn back on the steps checking for announced or not announced routes we turned this off in the thoughts this might be behind the broken asyncio tests, but it appears to not be the case --- .../add_automated_block_entry.feature | 4 +-- .../acceptance/features/expiration.feature | 30 +++++++++---------- .../features/restrict_changes.feature | 4 +-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature b/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature index 62a048f6..83af7dc5 100644 --- a/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature +++ b/scram/route_manager/tests/acceptance/features/add_automated_block_entry.feature @@ -15,7 +15,7 @@ Feature: an automated source adds a block entry Then the number of entrys is 1 And is one of our list of entrys And is contained in our list of entrys -# And is announced by block translators + And is announced by block translators Examples: v4 IPs | ip | cidr | @@ -78,7 +78,7 @@ Feature: an automated source adds a block entry And we list the entrys Then the number of entrys is 1 -# And is announced by block translators + And is announced by block translators Examples: | ip | diff --git a/scram/route_manager/tests/acceptance/features/expiration.feature b/scram/route_manager/tests/acceptance/features/expiration.feature index ce850abd..e3dec5fa 100644 --- a/scram/route_manager/tests/acceptance/features/expiration.feature +++ b/scram/route_manager/tests/acceptance/features/expiration.feature @@ -10,14 +10,14 @@ Feature: entries auto-expire And we add the entry 192.0.2.3/32 with expiration 2030-01-01T00:00:00Z And we add the entry 192.0.2.4/32 with expiration in 12 seconds Then the number of entrys is 4 -# And 192.0.2.1/32 is announced by block translators -# And 192.0.2.2/32 is announced by block translators -# And 192.0.2.3/32 is announced by block translators -# And 192.0.2.4/32 is announced by block translators + And 192.0.2.1/32 is announced by block translators + And 192.0.2.2/32 is announced by block translators + And 192.0.2.3/32 is announced by block translators + And 192.0.2.4/32 is announced by block translators And we remove expired entries And the number of entrys is 2 -# And 192.0.2.1/32 is not announced by block translators -# And 192.0.2.3/32 is announced by block translators + And 192.0.2.1/32 is not announced by block translators + And 192.0.2.3/32 is announced by block translators And we wait 12 seconds And we remove expired entries And the number of entrys is 1 @@ -30,12 +30,12 @@ Feature: entries auto-expire And we add the entry 192.0.2.2/32 with expiration 2000-01-01T00:00:00Z And we add the entry 192.0.2.3/32 with expiration 2030-01-01T00:00:00Z Then the number of entrys is 3 -# And 192.0.2.1/32 is announced by block translators -# And 192.0.2.3/32 is announced by block translators + And 192.0.2.1/32 is announced by block translators + And 192.0.2.3/32 is announced by block translators And we remove expired entries And the number of entrys is 1 -# And 192.0.2.1/32 is not announced by block translators -# And 192.0.2.3/32 is announced by block translators + And 192.0.2.1/32 is not announced by block translators + And 192.0.2.3/32 is announced by block translators Scenario: Adding an IP twice gets expired after the last entry expires @@ -44,9 +44,9 @@ Feature: entries auto-expire When we're logged in And we add the entry 192.0.2.1/32 with expiration 1970-01-01T00:00:00Z And we add the entry 192.0.2.1/32 with expiration 2030-01-01T00:00:00Z -# Then 192.0.2.1/32 is announced by block translators + Then 192.0.2.1/32 is announced by block translators Then we remove expired entries -# And 192.0.2.1/32 is announced by block translators + And 192.0.2.1/32 is announced by block translators Scenario: Adding an IP twice gets expired after the last entry expires in 2 seconds Given a block actiontype is defined @@ -54,10 +54,10 @@ Feature: entries auto-expire When we're logged in And we add the entry 192.0.2.1/32 with expiration 1970-01-01T00:00:00Z And we add the entry 192.0.2.1/32 with expiration in 12 seconds -# Then 192.0.2.1/32 is announced by block translators + Then 192.0.2.1/32 is announced by block translators Then we remove expired entries And we wait 1 seconds -# And 192.0.2.1/32 is announced by block translators + And 192.0.2.1/32 is announced by block translators And we wait 13 seconds And we remove expired entries -# And 192.0.2.1/32 is not announced by block translators + And 192.0.2.1/32 is not announced by block translators diff --git a/scram/route_manager/tests/acceptance/features/restrict_changes.feature b/scram/route_manager/tests/acceptance/features/restrict_changes.feature index 32c050de..bb24eddc 100644 --- a/scram/route_manager/tests/acceptance/features/restrict_changes.feature +++ b/scram/route_manager/tests/acceptance/features/restrict_changes.feature @@ -9,5 +9,5 @@ Feature: restrict changing entries And we update the entry 192.0.2.208 to 192.0.2.209 Then we get a 405 status code And the number of entrys is 1 -# And 192.0.2.208 is announced by block translators -# And 192.0.2.209 is not announced by block translators + And 192.0.2.208 is announced by block translators + And 192.0.2.209 is not announced by block translators From 53530c3e1799cf8f24983379f99e04cb74b48bd8 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Sat, 27 Apr 2024 18:33:05 -0500 Subject: [PATCH 10/19] fix(debug): pull out debug message to avoid plain text logging something we shouldn't --- translator/gobgp.py | 1 - 1 file changed, 1 deletion(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index 95a3006e..6655bbf4 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -27,7 +27,6 @@ def _build_path(self, ip, event_data): asn = 64500 community = 666 - logging.debug(event_data) # Pull asn and community from event_data if we have any if event_data: if "asn" in event_data: From 0ddff86546dbcf12026d2933a56bd77d142398d6 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Sat, 27 Apr 2024 18:38:51 -0500 Subject: [PATCH 11/19] refactor(asn/community): use a cleaner method to grab from event data or set defaults --- translator/gobgp.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index 6655bbf4..e8949dcf 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -22,17 +22,10 @@ def _get_family(self, ip_version): else: return gobgp_pb2.Family.AFI_IP - def _build_path(self, ip, event_data): - # defaults - asn = 64500 - community = 666 - - # Pull asn and community from event_data if we have any - if event_data: - if "asn" in event_data: - asn = event_data["asn"] - if "community" in event_data: - community = event_data["community"] + def _build_path(self, ip, event_data={}): + # Grab ASN and Community from our event_data, or use the defaults + asn = event_data.get("asn", 65400) + community = event_data.get("community", 666) # Set the origin to incomplete (options are IGP, EGP, incomplete) # Incomplete means that BGP is unsure of exactly how the prefix was injected into the topology. From 66fb4f47ca41fb080333ce80b4a059e92e27c005 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Sat, 27 Apr 2024 18:48:30 -0500 Subject: [PATCH 12/19] style(variable-naming): change variable naming to match others --- translator/tests/acceptance/features/bgp.feature | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/translator/tests/acceptance/features/bgp.feature b/translator/tests/acceptance/features/bgp.feature index de0a0987..c6787b42 100644 --- a/translator/tests/acceptance/features/bgp.feature +++ b/translator/tests/acceptance/features/bgp.feature @@ -4,19 +4,19 @@ Feature: block with BGP Scenario Outline: We can block an IP When we add with and to the block list Then is blocked - And is unblocked + And is unblocked Examples: data - | route | asn | community | UnblockIP | + | route | asn | community | unblock_ip | | 192.0.2.4/32 | 64500 | 666 | 192.0.2.5 | | 2001:DB8:A::/64 | 64500 | 666 | baba:: | Scenario Outline: We can block an IP When we add with and to the block list And we delete from the block list - Then is unblocked + Then is unblocked Examples: data - | route | asn | community | UnblockIP | + | route | asn | community | unblock_ip | | 192.0.2.4/32 | 64500 | 666 | 192.0.2.4 | | 2001:DB8:A::/64 | 64500 | 666 | 2001:DB8:B:: | From b999fb89ec39a26e74fb23736f93e56c448c7009 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Sat, 27 Apr 2024 19:13:17 -0500 Subject: [PATCH 13/19] refactor(4byte-ASN): test a v4 and v6 version with 4 byte ASNs using the private range per https://datatracker.ietf.org/doc/rfc6996/ --- translator/tests/acceptance/features/bgp.feature | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/translator/tests/acceptance/features/bgp.feature b/translator/tests/acceptance/features/bgp.feature index c6787b42..5ee21253 100644 --- a/translator/tests/acceptance/features/bgp.feature +++ b/translator/tests/acceptance/features/bgp.feature @@ -7,9 +7,11 @@ Feature: block with BGP And is unblocked Examples: data - | route | asn | community | unblock_ip | - | 192.0.2.4/32 | 64500 | 666 | 192.0.2.5 | - | 2001:DB8:A::/64 | 64500 | 666 | baba:: | + | route | asn | community | unblock_ip | + | 192.0.2.4/32 | 64500 | 666 | 192.0.2.5 | + | 192.0.2.10/32 | 4200000000 | 321 | 192.0.2.11 | + | 2001:DB8:A::/64 | 64500 | 666 | baba:: | + | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | Scenario Outline: We can block an IP When we add with and to the block list @@ -17,6 +19,8 @@ Feature: block with BGP Then is unblocked Examples: data - | route | asn | community | unblock_ip | - | 192.0.2.4/32 | 64500 | 666 | 192.0.2.4 | - | 2001:DB8:A::/64 | 64500 | 666 | 2001:DB8:B:: | + | route | asn | community | unblock_ip | + | 192.0.2.4/32 | 64500 | 666 | 192.0.2.4 | + | 192.0.2.10/32 | 4200000000 | 321 | 192.0.2.11 | + | 2001:DB8:A::/64 | 64500 | 666 | 2001:DB8::1 | + | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | From ac71b25be7c7a422453c1f6929081091e02878c5 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Sat, 27 Apr 2024 19:22:08 -0500 Subject: [PATCH 14/19] test(variables): pass asn/community as vars to the del_block function --- translator/tests/acceptance/features/bgp.feature | 2 +- translator/tests/acceptance/steps/actions.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/translator/tests/acceptance/features/bgp.feature b/translator/tests/acceptance/features/bgp.feature index 5ee21253..218803af 100644 --- a/translator/tests/acceptance/features/bgp.feature +++ b/translator/tests/acceptance/features/bgp.feature @@ -15,7 +15,7 @@ Feature: block with BGP Scenario Outline: We can block an IP When we add with and to the block list - And we delete from the block list + And we delete with and from the block list Then is unblocked Examples: data diff --git a/translator/tests/acceptance/steps/actions.py b/translator/tests/acceptance/steps/actions.py index 44b4e634..e3e7f7c7 100644 --- a/translator/tests/acceptance/steps/actions.py +++ b/translator/tests/acceptance/steps/actions.py @@ -11,10 +11,10 @@ def add_block(context, route, asn, community): context.gobgp.add_path(ip, event_data) -@when("we delete {route} from the block list") -def del_block(context, route): +@when("we delete {route} with {asn} and {community} from the block list") +def del_block(context, route, asn, community): ip = ipaddress.ip_interface(route) - event_data = {"asn": 64500, "community": 666} + event_data = {"asn": int(asn), "community": int(community)} context.gobgp.del_path(ip, event_data) From 8c878023010a7eaf92e4a97732ca89026d5b2f8b Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Sat, 27 Apr 2024 19:31:30 -0500 Subject: [PATCH 15/19] test(asn/community): use nondefaults in the future we can start testing that our nondefaults are being set properly --- translator/tests/acceptance/features/bgp.feature | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/translator/tests/acceptance/features/bgp.feature b/translator/tests/acceptance/features/bgp.feature index 218803af..2779e55f 100644 --- a/translator/tests/acceptance/features/bgp.feature +++ b/translator/tests/acceptance/features/bgp.feature @@ -8,9 +8,9 @@ Feature: block with BGP Examples: data | route | asn | community | unblock_ip | - | 192.0.2.4/32 | 64500 | 666 | 192.0.2.5 | + | 192.0.2.4/32 | 54321 | 444 | 192.0.2.5 | | 192.0.2.10/32 | 4200000000 | 321 | 192.0.2.11 | - | 2001:DB8:A::/64 | 64500 | 666 | baba:: | + | 2001:DB8:A::/64 | 54321 | 444 | baba:: | | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | Scenario Outline: We can block an IP @@ -20,7 +20,7 @@ Feature: block with BGP Examples: data | route | asn | community | unblock_ip | - | 192.0.2.4/32 | 64500 | 666 | 192.0.2.4 | + | 192.0.2.4/32 | 54321 | 444 | 192.0.2.4 | | 192.0.2.10/32 | 4200000000 | 321 | 192.0.2.11 | - | 2001:DB8:A::/64 | 64500 | 666 | 2001:DB8::1 | + | 2001:DB8:A::/64 | 54321 | 444 | 2001:DB8::1 | | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | From 0058530451b420b641cb1c19081e3fa5460fcbda Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Wed, 1 May 2024 14:26:56 -0500 Subject: [PATCH 16/19] refactor(defaults): move defaults up top as globals to make it easier to find also grab next hop from event_data if its there and shorthand the ip.version a bit --- translator/gobgp.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index e8949dcf..eaacbb36 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -7,6 +7,10 @@ from google.protobuf.any_pb2 import Any _TIMEOUT_SECONDS = 1000 +DEFAULT_ASN = 65400 +DEFAULT_COMMUNITY = 666 +DEFAULT_V4_NEXTHOP = "192.0.2.199" +DEFAULT_V6_NEXTHOP = "100::1" logging.basicConfig(level=logging.DEBUG) @@ -16,7 +20,7 @@ def __init__(self, url): channel = grpc.insecure_channel(url) self.stub = gobgp_pb2_grpc.GobgpApiStub(channel) - def _get_family(self, ip_version): + def _get_family_AFI(self, ip_version): if ip_version == 6: return gobgp_pb2.Family.AFI_IP6 else: @@ -24,8 +28,9 @@ def _get_family(self, ip_version): def _build_path(self, ip, event_data={}): # Grab ASN and Community from our event_data, or use the defaults - asn = event_data.get("asn", 65400) - community = event_data.get("community", 666) + asn = event_data.get("asn", DEFAULT_ASN) + community = event_data.get("community", DEFAULT_COMMUNITY) + family = ip.ip.version # Set the origin to incomplete (options are IGP, EGP, incomplete) # Incomplete means that BGP is unsure of exactly how the prefix was injected into the topology. @@ -49,19 +54,21 @@ def _build_path(self, ip, event_data={}): # Set the next hop to the correct value depending on IP family next_hop = Any() - family = self._get_family(ip.ip.version) + family_AFI = self._get_family_AFI(family) if family == 6: + next_hops = list(event_data.get("next_hop", DEFAULT_V6_NEXTHOP)) next_hop.Pack( attribute_pb2.MpReachNLRIAttribute( - family=gobgp_pb2.Family(afi=family, safi=gobgp_pb2.Family.SAFI_UNICAST), - next_hops=["100::1"], + family=gobgp_pb2.Family(afi=family_AFI, safi=gobgp_pb2.Family.SAFI_UNICAST), + next_hop=next_hops, nlris=[nlri], ) ) else: + next_hops = list(event_data.get("next_hop", DEFAULT_V4_NEXTHOP)) next_hop.Pack( attribute_pb2.NextHopAttribute( - next_hop="192.0.2.199", + next_hop=next_hops, ) ) @@ -76,7 +83,6 @@ def _build_path(self, ip, event_data={}): as_path.Pack(as_segments) # Set our community number - # TODO: We currently only accept one community number, we may want to accept more than one in the future communities = Any() communities.Pack(attribute_pb2.CommunitiesAttribute(communities=[community])) From b66a24a53045cd318f9e7685eee70853f46ce900 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Thu, 2 May 2024 15:55:34 -0500 Subject: [PATCH 17/19] style(log): dont use f-strings without a placeholder --- translator/gobgp.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index eaacbb36..a16394cf 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -77,7 +77,7 @@ def _build_path(self, ip, event_data={}): as_segment = None # Make sure our asn is an acceptable number. This is the max as stated in rfc6996 - assert asn < 4294967295 + assert 0 < asn < 4294967295 as_segment = [attribute_pb2.AsSegment(numbers=[asn])] as_segments = attribute_pb2.AsPathAttribute(segments=as_segment) as_path.Pack(as_segments) @@ -96,7 +96,10 @@ def _build_path(self, ip, event_data={}): def add_path(self, ip, event_data): logging.info(f"Blocking {ip}") - path = self._build_path(ip, event_data) + try: + path = self._build_path(ip, event_data) + except AssertionError: + logging.warning("ASN assertion failed") self.stub.AddPath( gobgp_pb2.AddPathRequest(table_type=gobgp_pb2.GLOBAL, path=path), @@ -109,9 +112,11 @@ def del_all_paths(self): self.stub.DeletePath(gobgp_pb2.DeletePathRequest(table_type=gobgp_pb2.GLOBAL), _TIMEOUT_SECONDS) def del_path(self, ip, event_data): - path = self._build_path(ip, event_data) - logging.info(f"Unblocking {ip}") + try: + path = self._build_path(ip, event_data) + except AssertionError: + logging.warning("ASN assertion failed") self.stub.DeletePath( gobgp_pb2.DeletePathRequest(table_type=gobgp_pb2.GLOBAL, path=path), From bb22538a8e1dea2ffb9b388eb714963f47308b81 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 3 May 2024 14:49:10 -0500 Subject: [PATCH 18/19] style(black): ran black --- translator/gobgp.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index a16394cf..7a109a21 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -30,7 +30,7 @@ def _build_path(self, ip, event_data={}): # Grab ASN and Community from our event_data, or use the defaults asn = event_data.get("asn", DEFAULT_ASN) community = event_data.get("community", DEFAULT_COMMUNITY) - family = ip.ip.version + ip_version = ip.ip.version # Set the origin to incomplete (options are IGP, EGP, incomplete) # Incomplete means that BGP is unsure of exactly how the prefix was injected into the topology. @@ -54,18 +54,18 @@ def _build_path(self, ip, event_data={}): # Set the next hop to the correct value depending on IP family next_hop = Any() - family_AFI = self._get_family_AFI(family) - if family == 6: - next_hops = list(event_data.get("next_hop", DEFAULT_V6_NEXTHOP)) + family_afi = self._get_family_AFI(ip_version) + if ip_version == 6: + next_hops = event_data.get("next_hop", DEFAULT_V6_NEXTHOP) next_hop.Pack( attribute_pb2.MpReachNLRIAttribute( - family=gobgp_pb2.Family(afi=family_AFI, safi=gobgp_pb2.Family.SAFI_UNICAST), - next_hop=next_hops, + family=gobgp_pb2.Family(afi=family_afi, safi=gobgp_pb2.Family.SAFI_UNICAST), + next_hops=[next_hops], nlris=[nlri], ) ) else: - next_hops = list(event_data.get("next_hop", DEFAULT_V4_NEXTHOP)) + next_hops = event_data.get("next_hop", DEFAULT_V4_NEXTHOP) next_hop.Pack( attribute_pb2.NextHopAttribute( next_hop=next_hops, @@ -91,21 +91,21 @@ def _build_path(self, ip, event_data={}): return gobgp_pb2.Path( nlri=nlri, pattrs=attributes, - family=gobgp_pb2.Family(afi=family, safi=gobgp_pb2.Family.SAFI_UNICAST), + family=gobgp_pb2.Family(afi=family_afi, safi=gobgp_pb2.Family.SAFI_UNICAST), ) def add_path(self, ip, event_data): logging.info(f"Blocking {ip}") try: path = self._build_path(ip, event_data) + + self.stub.AddPath( + gobgp_pb2.AddPathRequest(table_type=gobgp_pb2.GLOBAL, path=path), + _TIMEOUT_SECONDS, + ) except AssertionError: logging.warning("ASN assertion failed") - self.stub.AddPath( - gobgp_pb2.AddPathRequest(table_type=gobgp_pb2.GLOBAL, path=path), - _TIMEOUT_SECONDS, - ) - def del_all_paths(self): logging.warning("Withdrawing ALL routes") @@ -115,17 +115,16 @@ def del_path(self, ip, event_data): logging.info(f"Unblocking {ip}") try: path = self._build_path(ip, event_data) + self.stub.DeletePath( + gobgp_pb2.DeletePathRequest(table_type=gobgp_pb2.GLOBAL, path=path), + _TIMEOUT_SECONDS, + ) except AssertionError: logging.warning("ASN assertion failed") - self.stub.DeletePath( - gobgp_pb2.DeletePathRequest(table_type=gobgp_pb2.GLOBAL, path=path), - _TIMEOUT_SECONDS, - ) - def get_prefixes(self, ip): prefixes = [gobgp_pb2.TableLookupPrefix(prefix=str(ip.ip))] - family = self._get_family(ip.ip.version) + family = self._get_family_AFI(ip.ip.version) result = self.stub.ListPath( gobgp_pb2.ListPathRequest( table_type=gobgp_pb2.GLOBAL, From 821e4269db51afd5060423664f8fe2e61e13e790 Mon Sep 17 00:00:00 2001 From: Sam Oehlert Date: Fri, 3 May 2024 15:37:27 -0500 Subject: [PATCH 19/19] refactor(afi-variable): update to match naming scheme from other methods for family_afi variable --- translator/gobgp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/translator/gobgp.py b/translator/gobgp.py index 7a109a21..cb547ae6 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -124,12 +124,12 @@ def del_path(self, ip, event_data): def get_prefixes(self, ip): prefixes = [gobgp_pb2.TableLookupPrefix(prefix=str(ip.ip))] - family = self._get_family_AFI(ip.ip.version) + family_afi = self._get_family_AFI(ip.ip.version) result = self.stub.ListPath( gobgp_pb2.ListPathRequest( table_type=gobgp_pb2.GLOBAL, prefixes=prefixes, - family=gobgp_pb2.Family(afi=family, safi=gobgp_pb2.Family.SAFI_UNICAST), + family=gobgp_pb2.Family(afi=family_afi, safi=gobgp_pb2.Family.SAFI_UNICAST), ), _TIMEOUT_SECONDS, )