Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[uss_qualifier] Validate that some queries to service_providers are properly authenticated for NET0210 #187

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

Shastick
Copy link
Contributor

@Shastick Shastick commented Sep 7, 2023

Add check for NET0210, which is about checking that service providers correctly authenticate requests.

This is done via a new misbehavior scenario that will inject flight data and query the SP's endpoint for that data without providing credentials.

It behaves very similarly to the nominal_behavior scenario, with these two tweaks:

  • a DSS instance is required instead of being optional
  • the polling stops as soon as we have validated that the SP properly authenticates the flights and flight_details requests.

@Shastick Shastick force-pushed the net-0210-sp-auth-checks branch 2 times, most recently from a591c99 to 39b0727 Compare September 11, 2023 12:25
@Shastick Shastick marked this pull request as ready for review September 11, 2023 14:31
@Shastick Shastick force-pushed the net-0210-sp-auth-checks branch 3 times, most recently from 00fc3ba to a325673 Compare September 11, 2023 15:22
"GET",
fq.query.request.url,
)
server_id = fq.query.get("server_id", "unknown")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that server_id should be set here once #188 is merged

@Shastick Shastick force-pushed the net-0210-sp-auth-checks branch 5 times, most recently from 7e792c5 to 3b659d8 Compare September 12, 2023 13:06
Copy link
Member

@BenjaminPelletier BenjaminPelletier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great -- just the propagated pre-existing issue regarding RIDVersion and I expect this PR to be ready to merge

@Shastick Shastick force-pushed the net-0210-sp-auth-checks branch 6 times, most recently from 70c8e5c to 490163e Compare September 12, 2023 20:11
@Shastick Shastick force-pushed the net-0210-sp-auth-checks branch from 490163e to f065c41 Compare September 12, 2023 20:26
@Shastick
Copy link
Contributor Author

Unsure if this is flaky or due to interfering tests, but I ran into multiple of these failures on the CI:

2023-09-12 20:15:38.239 | WARNING  | monitoring.uss_qualifier.suites.suite:_print_failed_check:50 - New failed check:
  details: 'Flight be73f2ee-528f-4d8d-8725-eb0de98d544a: has no telemetry position that
    belong to a cluster.
  
    Severity Medium upgraded to Critical because USS_QUALIFIER_STOP_FAST environment
    variable indicates true'
  documentation_url: https://github.com/interuss/monitoring/blob/889863e731c55d197ddb13c9d2d7faf3779a3cf0/monitoring/uss_qualifier/scenarios/astm/netrid/v19/nominal_behavior.md#individual-flights-obfuscation-check
  name: Individual flights obfuscation
  participants:
  - uss3
  requirements:
  - astm.f3411.v19.NET0490
  severity: Critical
  summary: Error while evaluating obfuscation of individual flights. Flight was outside
    of all clusters.
  timestamp: '2023-09-12T20:15:38.237596Z'

Will have a look tomorrow.

@BenjaminPelletier
Copy link
Member

Unsure if this is flaky or due to interfering tests, but I ran into multiple of these failures on the CI:
...
Will have a look tomorrow.

I think this is likely to be #144 and if so should be transient -- I'd poke @mickmis next week. It looks like the CI is passing now.

flights_data: FlightDataResource,
service_providers: NetRIDServiceProviders,
evaluation_configuration: EvaluationConfigurationResource,
dss_pool: DSSInstancesResource = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since dss_pool requires at least one instance below, this argument shouldn't default to None. But we can clean that up later.

@BenjaminPelletier BenjaminPelletier changed the title [uss_qualifier] validate that some queries to service_providers are properly authenticated for NET0210 [uss_qualifier] Validate that some queries to service_providers are properly authenticated for NET0210 Sep 12, 2023
@BenjaminPelletier BenjaminPelletier merged commit fbf372f into interuss:main Sep 12, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants