[uss_qualifier] Inspect all collected queries for the usage of https (NET0220)

[Shastick]

Checking for NET0220 by ensuring all queries to USSes are relying on https.

In order to validate that queries are not sent in cleartext while not breaking local non-https setups, this:

• tags all/most queries with the server_id it is intended to
• adds a local_debug flag to service providers, observers and dss'es
• adds a check to the aggregate checks that validates all recorded queries are using https when they target an USS that isn't in local_debug mode.

Open questions/notes:

• We may want to add the participant_id/server_id to the UTM session object? That might make this PR a bit simple.

[BenjaminPelletier]

When possible, we should attribute failed checks to the specific participant failing them -- multiple participants for a single check means any one (or multiple) of the specified participants may have failed the check. So, instead, we should iterate over participants and, for each participant, check whether all their queries use https. If yes, pass; if no, fail; if no queries available, don't perform check. (yes, in that case there will be multiple instances of the same check: one instance for each relevant participant)

[Shastick]

Makes sense. This brings us back to the question of attributing queries to participants, and whether to do so by looking at the host-name or URL, or by tagging the requests where they are made:

We can probably set the server_id field for all queries. Though, what should we do if we run into a query that is not associated with an SP? I guess failing hard so that we uncover these when scenarios are developed?

[BenjaminPelletier]

Yes, I think server_id should be the means we can use to identify the participants. I think we would want to use all data we have available to us to make the check, but I think we can actually simply ignore queries where we don't have a server ID. For instance, if we added a test portion that interacted with a third entity other than a Service Provider or Display Provider and we happened not to populated server_id in our queries to them, I don't think that would justify failing hard.

That said, we may not be populating server_id in all the places that we should, so failing hard temporarily to make sure we've identified all those places seems fine.

[Shastick]

I guess we could add a dev flag somewhere that will have more stringent internal requirements?

Or is leaving the code as-is fine and we remove the check at a later stage?

[Shastick]

As discussed with @BenjaminPelletier offline: we'll leave this "debug" flag hardcoded for now. Once things are stable we can simply remove it.

[Shastick]

Modulo the dev option, this is ready for another round of review.

[BenjaminPelletier]

It would be better to attach this information to the failed check itself as notes do not obviously attach to any particular check. record_failed has an optional parameter for relevant queries that we should use as much as possible -- it should be populated with the timestamps for this set of queries in this case.

[Shastick]

Looks like the "strict dev mode" is working as intended: freshly merged tests cause this failure:

2023-09-13 06:09:37.699 | WARNING  | monitoring.uss_qualifier.suites.suite:_print_failed_check:50 - New failed check:
  details: '
  
    Severity Medium upgraded to Critical because USS_QUALIFIER_STOP_FAST environment
    variable indicates true'
  documentation_url: https://github.com/interuss/monitoring/blob/3b806aabb95cd436c072eba9ac22deae51d3936a/monitoring/uss_qualifier/scenarios/astm/netrid/v22a/aggregate_checks.md#no-unattributed-queries-check
  name: No unattributed queries
  participants: []
  requirements: []
  severity: Critical
  summary: 'found unattributed queries: [''http://v22a.ridsp.uss1.localutm/mock/ridsp/v2/uss/flights?view=37.17008162687164%2C-80.60282480412947%2C37.22330680061591%2C-80.5495996303852&recent_positions_duration=60'',
    ''http://v22a.ridsp.uss1.localutm/mock/ridsp/v2/uss/flights?view=37.172210633821415%2C-80.6006957971797%2C37.221177793666136%2C-80.55172863733499&recent_positions_duration=60'',
    ''http://v22a.ridsp.uss1.localutm/mock/ridsp/v2/uss/flights?view=37.189952358402834%2C-80.58295407259827%2C37.203436069084724%2C-80.5694703619164&recent_positions_duration=60'']'
  timestamp: '2023-09-13T06:09:37.696861Z'

I'll tweak these tests ASAP

[Shastick]

PR ready for final reviw

[BenjaminPelletier]

Looks great, thanks! Just 2 copy/paste cleanup items, and one optional note

[Shastick]

Deduped test-case doc and rebased, we should be ready to merge this.