nasa / utm-apis

The collection of APIs for NASA's UTM project in the form of OpenAPI documents.
55 stars 35 forks source link

discuss tcl4valtest's validation order assumptions #159

Closed issmith1 closed 4 years ago

issmith1 commented 5 years ago

Tcl4valtests assume this Validation order:

  1. Check for valid access_token before processing the http request body. 403 if scope is wrong, 401 if invalid token.
  2. Validate JSON body (syntactic check… strings, doubles, lengths, non-nulls, etc.) 400 on failure.
  3. If uss_name != sub claim, return 403
  4. if other business logic failure, return 400
  5. otherwise return 20x (edited)
issmith1 commented 5 years ago

The policy is, if a USS can reasonably meet this ordering we ask that be so. If a USS decides they cannot recode to this order, contact NASA so we change test(s) to accept more than one valid reply.

esparano commented 5 years ago

There are a few extra validations we do, and I'm not sure where they fall into this validation order.

a) We throw a 400 when referenced operations cannot be found. Is this part of step 2 or 4?

b) We throw a 403 when the referenced operation's uss_name does not match that of the requestor. This check is for Positions, UtmMessages, and NegotiationMessages that reference operations, and excludes UtmMessage with message_type=PERIODIC_POSITION_REPORTS_START/END

c) We have "data consistency" checks that throw a 403 if a USS attempts to update another USS's data models. If the Operation, for example, is being updated, not created, we check to make sure the uss_name matches the previous value, so that USSs cannot update an operation they don't own. #3 allows this type of hijacking.

d) We throw a 403 if an Operation request is PUBLIC_SAFETY and using /operations, or is NOT PUBLIC_SAFETY and is using /enhanced/operations

issmith1 commented 5 years ago

a) We throw a 400 when referenced operations cannot be found. Is this part of step 2 or 4? USS Spec does not ask for this validation to be made, the tcl4valtests does not consider it.

THe USS Spcs allows the USS to determine data referential integrity between data from other USSs. Example is, a USS gets an gufi123 from another USS followed by a Position Report for gufi123. USS Spec does not say what happens if Position is received before the Op.

Another case is, a USS never got the first Operation announcement, however a subsequent UtmMessage of type alert comes in for that gufi. Should the USS accept? NUSS does accept the alert. (NUSS stores in a db w/out foreign keys.)

issmith1 commented 5 years ago

b) is #3 c) is #3 but I consider this up for discussion. I can see where it is reasonable if some 400s are checked before this referential integrity check; both are in the webapp. d) #3

mcelhennyi commented 5 years ago

We have a conflict with the current approach to the combos of priority elements testing.

When we are testing for priority_elements combos we are tested on PUBLIC_SAFETY combos at our /operations endpoint. We believe this should be tested at our /enhanced/operations endpoint. This is because it is much simpler to first check a single field (priority_status) before checking both fields for combination correctness (priority_status & priority_level). This is somewhat an optimization, because we can check only a single item before spending time checking another item (or all fields) in a potentially already failed message. If we do our general structure validation before checking for a PUBLIC_SAFETY operation at a non pubsafe endpoint, we then waste resource on a process that is ultimately unneeded.

I realize that some people may not have this ability or this order of implementation so I suggest that we change the tests for all PUBLIC_SAFETY status combination tests to the enhanced endpoint to minimize the impact of this optimization. Currently it seems only the valid PUBLIC_SAFETY tests go to the enhanced endpoint, but the ones that expect validation failure go to the normal endpoint. This will allow people who test the combinations first to fail with a 400 for combo and allow people who do the simple check then the combo check (us) to also fail with a 400 on bad combinations. All while making sure the USS's understand the combinations AND the use of an endpoint for each type of operation (normal vs enhanced)

Otherwise, I would think a 403 or a 400 would be a valid return for a message that sends in a bad combo or a bad status at an endpoint that requires a specific status/combo.

nasajoey commented 4 years ago

We address this as a known open issue in the forthcoming auth tech memo. For NASA UTM research purposes, we have successfully revealed this as an issue and will likely leave it here since flight testing is complete. other projects or standards groups may want to reference this issue in developing the details to follow on to our Project's work.