From 34e3216ba8369cad05ea6c4f872f681637499d46 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 23 Jun 2021 17:52:52 +0300 Subject: [PATCH] ADR 8: change "Decision outcome" After a discussion with Jussi, we realized that there are a couple of places where we don't want to allow unrecognized fields, because they are sensitive or there are limited acceptable values for them. The places where we don't want to allow unrecognized fields are "keys", "roles", "meta", "hashes" or "targets". If we allow unrecognized fields in them we won't follow the spec or even open the door for possible security vulnerability. Signed-off-by: Martin Vrachev --- docs/adr/0008-accept-unrecognised-fields.md | 27 ++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/adr/0008-accept-unrecognised-fields.md b/docs/adr/0008-accept-unrecognised-fields.md index 9b633a2fc7..1a362cd016 100644 --- a/docs/adr/0008-accept-unrecognised-fields.md +++ b/docs/adr/0008-accept-unrecognised-fields.md @@ -33,15 +33,19 @@ intermediate operations: then, the checksum (the content) of the file must not be changed. - Flexibility to add new fields in the spec without adding breaking changes. +- Attributes that are sensitive or have a strictly defined allowed content +shouldn't allow unrecognized fields at all. ## Considered Options - Ignore and drop unrecognized fields. - Ignore, but store unrecognized fields as an additional attribute. +- Ignore, but store unrecognized fields as an additional attribute with the +exception of a couple of sensitive attributes. ## Decision Outcome Chosen option: "Ignore, but store unrecognized fields as an additional -attribute." +attribute with the exception of a couple of sensitive attributes." The motivation for this decision is that the TUF specification already implies that we should accept unrecognized fields for backward compatibility and easier future extensibility. @@ -49,3 +53,24 @@ future extensibility. Additionally, it seems unacceptable to change a metadata file content just by reading and writing it back. +Still, there are places where we need to make an exception those are: +- `keys` in `root.json` and in `targets.json` `delegations`: allowing an +unrecognized field here will give an attacker the possibility to inject his keys +and thus compromise the trust in the entire system. +See: https://theupdateframework.github.io/specification/latest/index.html#root +- `roles` in `root.json`: according to the spec, `roles` is a dictionary which +allows keys from a limited list: https://theupdateframework.github.io/specification/latest/index.html#root-role +- `meta` in `snapshot.json` and `timestamp.json`: for `timestamp.json`, `meta` +consists of information about only one key - `snapshot.json`, for `snapshot.json` +`meta` contains information about top-level targets metadata and +delegated targets metadata which allows the client to know which metadata files +have been updated also prevents mix-and-match attacks. +See: https://theupdateframework.github.io/specification/latest/index.html#snapshot +- `hashes` in all metadata files besides `root.json`: allowing unrecognized +fields in `hashes` will give the attacker the ability to add a dictionary record +of an algorithm and hash digest of his choice, and thus compromise +the verification of metadata or target files. +- `targets` in `targets.json`: if we allow unrecognized fields in `targets` +we would give the attacker the opportunity to add his vulnerable package to the +list of possible targets for download. This could cause series of attacks. +