uptane/uptane-standard

Unaddressed freeze attack (or denial of service) based on Director replay

tkfu opened this issue · 19 comments

tkfu commented

This issue relies on the following properties of director metadata and verification procedure:

  • 5.2.3.1, "Each Targets role MAY provide a list of some images on the repository." Targets metadata in general can be empty.
  • 5.4.4.6 (targets metadata verification procedure), there is no requirement that top-level targets metadata from the director contain an image.

Some implementations might choose to always have director targets metadata include the full list of images that the inventory database believes should be installed on the vehicle. However, as an efficiency measure, some implementations might choose to only have the director targets metadata include only the images that need to be updated. With this implementation choice, it would possible (it would be common in fact) for vehicles to receive targets metadata from the director with no targets included. If there are no targets, this metadata can be maliciously replayed.

For example, if Alice has a MitM against both Bob and Carol, she can choose to allow updates to Carol's vehicle until it is up to date, then replay the now-empty director targets metadata to Bob. Thus, Bob will always have valid, signed metadata directing him not to install any updates.

Another potential scenario is a denial of service: Bob's vehicle has been part of the fleet a long time, and thus gets director targets metadata with a very high version number. Alice replays this metadata to Carol; it is accepted. Carol will now reject future valid metadata from the Director as being a potential rollback attack.

For scenario 1, one might argue that it doesn't breach Uptane guarantees because the freeze attack would still be detectable on the server side (because Bob's vehicle manifest will continue to report that he is running old versions). I'm not sure if I'd fully accept this argument, though, because there is no explicit error raised. You would have to add an extra automated check on the server side, because Bob never has enough information to know he's compromised. I could easily imagine this slipping through the cracks of typical OEM aftersales systems.

In scenario 2 there's no breach of any Uptane guarantees, but it's still clearly undesirable.

tkfu commented

We could mitigate this by adding the VIN (or whatever vehicle identifier) as a required top-level field in director targets metadata, but if I understand correctly that would have to wait for 2.0 (please correct me if I'm wrong there, @iramcdonald).

In the near term, we can (and should) add something to deployment considerations warning about the risk of using empty director targets metadata. If you really want to minimize the size of director metadata, the minimum safe thing you can do under the current standard is to ensure that all targets metadata issued by the director includes at least one target.

tkfu commented

the minimum safe thing you can do under the current standard is to ensure that all targets metadata issued by the director includes at least one target.

Sorry for arguing with myself here, but I think it may be possible to read the current standard to permit adding extra fields to the metadata. I don't see anywhere where we say "the targets metadata MUST NOT contain any fields not explicitly mentioned in the standard". So if it's possible to add top-level fields to the targets metadata, we could potentially also recommend (as a mitigation in deployment considerations) simply adding a top-level VIN field, as I suggested for 2.0.

The argument against that is that the standard does specifically say that "Targets metadata files MAY contain extra metadata for images on the repository", and then explicitly says where that metadata must be, i.e. in a "custom" field underneath the image.

So, should we take that to mean that the only place where implementations are permitted to add extra information is in the custom field we talk about? Or can we have a baseline assumption that anything not explicitly forbidden is permitted?

tkfu commented

I opened a PR for this (uptane/deployment-considerations#90), and I elected to describe both possible mitigations, because adding a VIN field would require an in-vehicle client change whereas simply ensuring director targets metadata isn't empty can be implemented entirely on the server side.

@JustinCappos @tkfu let me know if I should add an agenda item to discuss the custom fields described above.

tkfu commented

Please do @jhdalek55, thank you!

@tkfu @JustinCappos according to the discussion notes from the 2/2/21 call, this issue was partially addressed by PR #90 on the Deployment pages, which was merged on 2/13. Can someone reframe the larger issue discussed here that would affect the Standard and then change the milestone label to 2.0.0. My notes state this would be "the larger issue of deciding whether to allow custom metadata anywhere vs. restricting it to a 'custom' field." On the 2/2call there was consensusthat "we will need to add a MUST or SHALL on the larger issue of permitting custom metadata for V.2.0.0."

Can someone act on the request above of better and frame the 2.0.0 question of this issue?

The interim step was taken. I would still like to see the larger issue clarified somewhat better.

At our Uptane Standards Meeting on 7/20, it was affirmed that this was just awaiting the wording change and that this change can be made now rather than waiting till right before the 2.0.0 deadline. However, we must approve PR #213 (which adds a statement that changes are made in real-time on the repository, but are not considered "official" till the next version is released) and resolve the must vs. shall issue, which will be discussed at the 8/3/21 meeting.

PR#213 was merged. Wording changes for 2.0.0. can now be made in agreement with our SHALL vs MUST decision.

We need a volunteer to make the necessary wording change.

@tkfu or anyone. This is not an issue I feel comfortable drafting.

This is still adrift. Can we make the wording change. @tkfu as you raised this issue, can you address this?

@marinamoore offered to do a first pass to resolve this issue.

solved in #224