diff --git a/.gitignore b/.gitignore index c5219b2e..959510e4 100644 --- a/.gitignore +++ b/.gitignore @@ -160,6 +160,7 @@ typings/ !.vscode/tasks.json !.vscode/launch.json !.vscode/extensions.json +*.code-workspace # Provided default Pycharm Run/Debug Configurations should be tracked by git diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f52a7080..435a4812 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,7 +7,6 @@ include: stages: - lint - test -- cleanup variables: POSTGRES_USER: scram @@ -40,12 +39,18 @@ pytest: - make migrate - make run script: + - export COMPOSE_PROJECT_NAME=$CI_PIPELINE_ID - make coverage.xml artifacts: reports: coverage_report: coverage_format: cobertura path: coverage.xml + after_script: + - export COMPOSE_PROJECT_NAME=$CI_PIPELINE_ID + - make stop + - make clean + gemnasium-dependency_scanning: variables: @@ -61,17 +66,3 @@ code_quality_html: sast: stage: test - -final_clean: - image: docker:24.0.6-dind - services: - - docker:dind - before_script: - - apk add make - - export COMPOSE_PROJECT_NAME=$CI_PIPELINE_ID - stage: cleanup - rules: - - when: always # run even if something failed - script: - - make stop - - make clean diff --git a/Makefile b/Makefile index 4a83c796..74556490 100644 --- a/Makefile +++ b/Makefile @@ -22,17 +22,17 @@ compose.override.yml: ## behave-all: runs behave inside the containers against all of your features .Phony: behave-all behave-all: compose.override.yml - @docker compose run django coverage run -a manage.py behave --no-input --simple + @docker compose run --rm django coverage run -a manage.py behave --no-input --simple ## behave: runs behave inside the containers against a specific feature (append FEATURE=feature_name_here) .Phony: behave behave: compose.override.yml - @docker compose run django python manage.py behave --no-input --simple -i $(FEATURE) + @docker compose run --rm django python manage.py behave --no-input --simple -i $(FEATURE) ## behave-translator .Phony: behave-translator behave-translator: compose.override.yml - @docker compose exec -T translator /usr/local/bin/behave /app/acceptance/features + @docker compose exec -T translator /usr/local/bin/behave /app/tests/acceptance/features ## build: rebuilds all your containers or a single one if CONTAINER is specified .Phony: build @@ -42,8 +42,8 @@ build: compose.override.yml ## coverage.xml: generate coverage from test runs coverage.xml: pytest behave-all behave-translator - @docker compose run django coverage report - @docker compose run django coverage xml + @docker compose run --rm django coverage report + @docker compose run --rm django coverage xml ## ci-test: runs all tests just like Gitlab CI does .Phony: ci-test @@ -58,7 +58,7 @@ clean: compose.override.yml ## collect-static: run collect static admin command .Phony: collectstatic collectstatic: compose.override.yml - @docker compose run django python manage.py collectstatic + @docker compose run --rm django python manage.py collectstatic ## django-addr: get the IP and ephemeral port assigned to docker:8000 .Phony: django-addr @@ -101,18 +101,18 @@ list-routes: compose.override.yml ## migrate: makemigrations and then migrate .Phony: migrate migrate: compose.override.yml - @docker compose run django python manage.py makemigrations - @docker compose run django python manage.py migrate + @docker compose run --rm django python manage.py makemigrations + @docker compose run --rm django python manage.py migrate ## pass-reset: change admin's password .Phony: pass-reset pass-reset: compose.override.yml - @docker compose run django python manage.py changepassword admin + @docker compose run --rm django python manage.py changepassword admin ## pytest: runs pytest inside the containers .Phony: pytest pytest: compose.override.yml - @docker compose run django coverage run -m pytest + @docker compose run --rm django coverage run -m pytest ## run: brings up the containers as described in compose.override.yml .Phony: run @@ -132,4 +132,4 @@ tail-log: compose.override.yml ## type-check: static type checking .Phony: type-check type-check: compose.override.yml - @docker compose run django mypy scram + @docker compose run --rm django mypy scram diff --git a/compose/local/translator/Dockerfile b/compose/local/translator/Dockerfile index 90681331..21ec884a 100644 --- a/compose/local/translator/Dockerfile +++ b/compose/local/translator/Dockerfile @@ -22,7 +22,9 @@ RUN mkdir /app \ COPY ./translator/translator.py /app COPY ./translator/gobgp.py /app -COPY ./translator/tests /app +COPY ./translator/exceptions.py /app +COPY ./translator/shared.py /app + RUN chmod +x /app/translator.py WORKDIR /app diff --git a/scram/route_manager/tests/test_websockets.py b/scram/route_manager/tests/test_websockets.py index dcda2f53..367359b8 100644 --- a/scram/route_manager/tests/test_websockets.py +++ b/scram/route_manager/tests/test_websockets.py @@ -183,5 +183,9 @@ def local_setUp(self): lambda ip, mask: { "type": "translator_add", "message": {"asn": 65550, "community": 100, "route": f"{ip}/{mask}"}, - } + }, + lambda ip, mask: { + "type": "translator_add", + "message": {"asn": 64496, "community": 4294967295, "route": f"{ip}/{mask}"}, + }, ] diff --git a/translator/exceptions.py b/translator/exceptions.py new file mode 100644 index 00000000..f1a54299 --- /dev/null +++ b/translator/exceptions.py @@ -0,0 +1,9 @@ +""" +This module holds all of the exceptions we want to raise in our translators. +""" + + +class ASNError(TypeError): + """ + ASNError provides an error class to use when there is an issue with an Autonomous System Number. + """ diff --git a/translator/gobgp.py b/translator/gobgp.py index fe0b679b..23ae2572 100644 --- a/translator/gobgp.py +++ b/translator/gobgp.py @@ -4,7 +4,9 @@ import gobgp_pb2 import gobgp_pb2_grpc import grpc +from exceptions import ASNError from google.protobuf.any_pb2 import Any +from shared import asn_is_valid _TIMEOUT_SECONDS = 1000 DEFAULT_ASN = 65400 @@ -76,8 +78,8 @@ def _build_path(self, ip, event_data={}): as_path = Any() as_segment = None - # Make sure our asn is an acceptable number. This is the max as stated in rfc6996 - assert 0 < asn < 4294967295 + # Make sure our asn is an acceptable value. + asn_is_valid(asn) as_segment = [attribute_pb2.AsSegment(numbers=[asn])] as_segments = attribute_pb2.AsPathAttribute(segments=as_segment) as_path.Pack(as_segments) @@ -88,6 +90,8 @@ def _build_path(self, ip, event_data={}): # Standard community # Since we pack both into the community string we need to make sure they will both fit if asn < 65536 and community < 65536: + # We bitshift ASN left by 16 so that there is room to add the community on the end of it. This is because + # GoBGP wants the community sent as a single integer. comm_id = (asn << 16) + community communities.Pack(attribute_pb2.CommunitiesAttribute(communities=[comm_id])) else: @@ -120,8 +124,8 @@ def add_path(self, ip, event_data): gobgp_pb2.AddPathRequest(table_type=gobgp_pb2.GLOBAL, path=path), _TIMEOUT_SECONDS, ) - except AssertionError: - logging.warning("ASN assertion failed") + except ASNError as e: + logging.warning(f"ASN assertion failed with error: {e}") def del_all_paths(self): logging.warning("Withdrawing ALL routes") @@ -136,8 +140,8 @@ def del_path(self, ip, event_data): gobgp_pb2.DeletePathRequest(table_type=gobgp_pb2.GLOBAL, path=path), _TIMEOUT_SECONDS, ) - except AssertionError: - logging.warning("ASN assertion failed") + except ASNError as e: + logging.warning(f"ASN assertion failed with error: {e}") def get_prefixes(self, ip): prefixes = [gobgp_pb2.TableLookupPrefix(prefix=str(ip.ip))] diff --git a/translator/shared.py b/translator/shared.py new file mode 100644 index 00000000..1fe048bd --- /dev/null +++ b/translator/shared.py @@ -0,0 +1,27 @@ +""" +This module provides a location for code that we want to share between all translators. +""" + +from exceptions import ASNError + + +def asn_is_valid(asn: int) -> bool: + """ + asn_is_valid makes sure that an ASN passed in is a valid 2 or 4 Byte ASN. + + Args: + asn (int): The Autonomous System Number that we want to validate + + Raises: + ASNError: If the ASN is not between 0 and 4294967295 or is not an integer. + + Returns: + bool: _description_ + """ + if not isinstance(asn, int): + raise ASNError(f"ASN {asn} is not an Integer, has type {type(asn)}") + if not 0 < asn < 4294967295: + # This is the max as stated in rfc6996 + raise ASNError(f"ASN {asn} is out of range. Must be between 0 and 4294967295") + + return True diff --git a/translator/tests/acceptance/environment.py b/translator/tests/acceptance/environment.py index f6b95746..1f06b08d 100644 --- a/translator/tests/acceptance/environment.py +++ b/translator/tests/acceptance/environment.py @@ -3,3 +3,4 @@ def before_all(context): context.gobgp = GoBGP("gobgp:50051") + context.config.setup_logging() diff --git a/translator/tests/acceptance/features/bgp.feature b/translator/tests/acceptance/features/bgp.feature index 2779e55f..8a27651d 100644 --- a/translator/tests/acceptance/features/bgp.feature +++ b/translator/tests/acceptance/features/bgp.feature @@ -4,23 +4,24 @@ Feature: block with BGP Scenario Outline: We can block an IP When we add with and to the block list Then is blocked + Then we delete with and from the block list And is unblocked Examples: data - | route | asn | community | unblock_ip | - | 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 | 54321 | 444 | baba:: | - | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | + | route | asn | community | unblock_ip | + | 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 | 54321 | 444 | baba:: | + | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | + | 192.0.2.20/32 | 4200000000 | 4200000000 | 192.0.2.11 | + | 2001:DB8:C::/64 | 4200000000 | 4200000000 | 2001:DB8::4 | + | 2001:DB8:C::1/64 | 4200000000 | 4200000000 | 2001:DB8::5 | - Scenario Outline: We can block an IP - When we add with and to the block list - And we delete with and from the block list - Then is unblocked + Scenario Outline: Invalid ASNs fail + When and with invalid is sent Examples: data - | route | asn | community | unblock_ip | - | 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 | 54321 | 444 | 2001:DB8::1 | - | 2001:DB8:B::/64 | 4200000000 | 321 | 2001:DB8::4 | + | route | asn | community | + | 2001:DB8:C::2/64 | 4242424242424242424242 | 100 | + | 2001:DB8:C::2/64 | -1 | 100 | + | 2001:DB8:C::2/64 | 0 | 100 | diff --git a/translator/tests/acceptance/steps/actions.py b/translator/tests/acceptance/steps/actions.py index e3e7f7c7..ce340de1 100644 --- a/translator/tests/acceptance/steps/actions.py +++ b/translator/tests/acceptance/steps/actions.py @@ -1,7 +1,11 @@ import ipaddress +import logging import time from behave import then, when +from behave.log_capture import capture + +logging.basicConfig(level=logging.DEBUG) @when("we add {route} with {asn} and {community} to the block list") @@ -11,7 +15,7 @@ def add_block(context, route, asn, community): context.gobgp.add_path(ip, event_data) -@when("we delete {route} with {asn} and {community} from the block list") +@then("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": int(asn), "community": int(community)} @@ -31,6 +35,13 @@ def get_block_status(context, ip): return False +@capture +@when("{route} and {community} with invalid {asn} is sent") +def asn_validation_fails(context, route, asn, community): + add_block(context, route, asn, community) + assert context.log_capture.find_event("ASN assertion failed") + + @then("{ip} is blocked") def check_block(context, ip): assert get_block_status(context, ip)