Fix metadata signatures mismatch between specs and implementation #98
Assignees
Comments
Merged
Merged
|
Since I just re-fixed this issue, I wanted to document the following thought: Given the two fairly big PRs (#120, #142) related to this issue, one might argue that it would have been a lot easier to just leave the original However, one reason why we used classes in the first place was that they are well suited to present a data model to a human reader, IMHO they do a better job than functions that work on dictionaries, which would have been the alternative. |
lukpueh
added a commit
to theupdateframework/python-tuf
that referenced
this issue
Jul 10, 2020
This commit performs restructuring on the recently added metadata class model architecture, which shall be part of a new simple TUF API. The key change is that the Metadata class is now used as container for inner TUF metadata (Root, Timestamp, Snapshot, Targets) instead of serving as base class for these, that means we use 'composition' instead of 'inheritance'. Still, in order to aggregate common attributes of the inner Metadata (expires, version, spec_version), we use a new baseclass 'Signed', which also corresponds to the signed field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to more closely represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). Note that the proposed changes require us to now access some attributes/methods via the signed attribute of a Metadata object and not directly on the Metadata object, but it would be possible to add short-cuts. (see todo notes in doc header). Further changes include: - Add minimal doc header with TODO notes - Make attributes that correspond to fields in TUF JSON metadata public again. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code. - Generally try to reduce code. - Remove keyring and consistent_snapshot attributes from metadata class. As discussed in #1060 they are a better fit for extra management code (also see #660) - Remove sslib schema checks (see TODO notes about validation in doc header) - Drop usage of build_dict_conforming_to_schema, it seems a lot simpler and more explicit to just code this here. - ... same goes for make_metadata_fileinfo - Adapt tests accordingly TODO: Document!!! Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
added a commit
to theupdateframework/python-tuf
that referenced
this issue
Jul 10, 2020
This commit performs restructuring on the recently added metadata class model architecture, which shall be part of a new simple TUF API. The key change is that the Metadata class is now used as container for inner TUF metadata (Root, Timestamp, Snapshot, Targets) instead of serving as base class for these, that means we use 'composition' instead of 'inheritance'. Still, in order to aggregate common attributes of the inner Metadata (expires, version, spec_version), we use a new baseclass 'Signed', which also corresponds to the signed field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to more closely represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). Note that the proposed changes require us to now access some attributes/methods via the signed attribute of a Metadata object and not directly on the Metadata object, but it would be possible to add short-cuts. (see todo notes in doc header). Further changes include: - Add minimal doc header with TODO notes - Make attributes that correspond to fields in TUF JSON metadata public again. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code. - Generally try to reduce code. - Remove keyring and consistent_snapshot attributes from metadata class. As discussed in #1060 they are a better fit for extra management code (also see #660) - Remove sslib schema checks (see TODO notes about validation in doc header) - Drop usage of build_dict_conforming_to_schema, it seems a lot simpler and more explicit to just code this here. - ... same goes for make_metadata_fileinfo - Adapt tests accordingly TODO: Document!!! Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
added a commit
to lukpueh/tuf
that referenced
this issue
Aug 18, 2020
Add metadata module with container classes for TUF role metadata, including methods to read/serialize/write from and to JSON, perform TUF-compliant metadata updates, and create and verify signatures. The 'Metadata' class provides a container for inner TUF metadata objects (Root, Timestamp, Snapshot, Targets) (i.e. OOP composition) The 'Signed' class provides a base class to aggregate common attributes (i.e. version, expires, spec_version) of the inner metadata classes. (i.e. OOP inheritance). The name of the class also aligns with the 'signed' field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to well represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). This commits also adds tests. **Some more design considerations** (also in regards to prior sketches of this module) - Aims at simplicity, brevity and recognizability of the wireline metadata format. - All attributes that correspond to fields in TUF JSON metadata are public. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code, and impedes recognizability of the wireline metadata format. - Although, it might be convenient to have short-cuts on the Metadata class that point to methods and attributes that are common to all subclasses of the contained Signed class (e.g. Metadata.version instead of Metadata.signed.version, etc.), this also conflicts with goal of recognizability of the wireline metadata. Thus we won't add such short-cuts for now. See: theupdateframework#1060 (comment) - Signing keys and a 'consistent_snapshot' boolean are not on the targets metadata class. They are a better fit for management code. See: theupdateframework#1060 (comment), and theupdateframework#660. - Does not use sslib schema checks (see TODO notes about validation in doc header) - Does not use existing tuf utils, such as make_metadata_fileinfo, build_dict_conforming_to_schema, if it is easy and more explicit to just re-implement the desired behavior on the metadata classes. - All datetime's are treated as UTC. Since timezone info is not captured in the wireline metadata format it should not be captured in the internal representation either. - Does not use 3rd-party dateutil package, in order to minimize dependency footprint, which is especially important for update clients which often have to vendor their dependencies. However, compatibility between the more advanced dateutil.relativedelta (e.g handles leap years automatically) and timedelta is tested. - Uses PEP8 indentation (4 space) and Google-style doc string instead of sslab-style. See secure-systems-lab/code-style-guidelines#20 - Does not support Python2 **TODO:** See doc header TODO list
lukpueh
added a commit
to lukpueh/tuf
that referenced
this issue
Aug 18, 2020
Add metadata module with container classes for TUF role metadata, including methods to read/serialize/write from and to JSON, perform TUF-compliant metadata updates, and create and verify signatures. The 'Metadata' class provides a container for inner TUF metadata objects (Root, Timestamp, Snapshot, Targets) (i.e. OOP composition) The 'Signed' class provides a base class to aggregate common attributes (i.e. version, expires, spec_version) of the inner metadata classes. (i.e. OOP inheritance). The name of the class also aligns with the 'signed' field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to well represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). This commits also adds tests. **Additional design considerations** (also in regards to prior sketches of this module) - Aims at simplicity, brevity and recognizability of the wireline metadata format. - All attributes that correspond to fields in TUF JSON metadata are public. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code, and impedes recognizability of the wireline metadata format. - Although, it might be convenient to have short-cuts on the Metadata class that point to methods and attributes that are common to all subclasses of the contained Signed class (e.g. Metadata.version instead of Metadata.signed.version, etc.), this also conflicts with goal of recognizability of the wireline metadata. Thus we won't add such short-cuts for now. See: theupdateframework#1060 (comment) - Signing keys and a 'consistent_snapshot' boolean are not on the targets metadata class. They are a better fit for management code. See: theupdateframework#1060 (comment), and theupdateframework#660. - Does not use sslib schema checks (see TODO notes about validation in doc header) - Does not use existing tuf utils, such as make_metadata_fileinfo, build_dict_conforming_to_schema, if it is easy and more explicit to just re-implement the desired behavior on the metadata classes. - All datetime's are treated as UTC. Since timezone info is not captured in the wireline metadata format it should not be captured in the internal representation either. - Does not use 3rd-party dateutil package, in order to minimize dependency footprint, which is especially important for update clients which often have to vendor their dependencies. However, compatibility between the more advanced dateutil.relativedelta (e.g handles leap years automatically) and timedelta is tested. - Uses PEP8 indentation (4 space) and Google-style doc string instead of sslab-style. See secure-systems-lab/code-style-guidelines#20 - Does not support Python =< 3.5 **TODO: See doc header TODO list** Co-authored-by: Trishank Karthik Kuppusamy <trishank.kuppusamy@datadoghq.com> Co-authored-by: Joshua Lock <jlock@vmware.com> Co-authored-by: Teodora Sechkova <tsechkova@vmware.com> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
added a commit
to lukpueh/tuf
that referenced
this issue
Aug 18, 2020
Add metadata module with container classes for TUF role metadata, including methods to read/serialize/write from and to JSON, perform TUF-compliant metadata updates, and create and verify signatures. The 'Metadata' class provides a container for inner TUF metadata objects (Root, Timestamp, Snapshot, Targets) (i.e. OOP composition) The 'Signed' class provides a base class to aggregate common attributes (i.e. version, expires, spec_version) of the inner metadata classes. (i.e. OOP inheritance). The name of the class also aligns with the 'signed' field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to well represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). This commits also adds tests. **TODO: See doc header TODO list** **Additional design considerations** (also in regards to prior sketches of this module) - Aims at simplicity, brevity and recognizability of the wireline metadata format. - All attributes that correspond to fields in TUF JSON metadata are public. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code, and impedes recognizability of the wireline metadata format. - Although, it might be convenient to have short-cuts on the Metadata class that point to methods and attributes that are common to all subclasses of the contained Signed class (e.g. Metadata.version instead of Metadata.signed.version, etc.), this also conflicts with goal of recognizability of the wireline metadata. Thus we won't add such short-cuts for now. See: theupdateframework#1060 (comment) - Signing keys and a 'consistent_snapshot' boolean are not on the targets metadata class. They are a better fit for management code. See: theupdateframework#1060 (comment), and theupdateframework#660. - Does not use sslib schema checks (see TODO notes about validation in doc header) - Does not use existing tuf utils, such as make_metadata_fileinfo, build_dict_conforming_to_schema, if it is easy and more explicit to just re-implement the desired behavior on the metadata classes. - All datetime's are treated as UTC. Since timezone info is not captured in the wireline metadata format it should not be captured in the internal representation either. - Does not use 3rd-party dateutil package, in order to minimize dependency footprint, which is especially important for update clients which often have to vendor their dependencies. However, compatibility between the more advanced dateutil.relativedelta (e.g handles leap years automatically) and timedelta is tested. - Uses PEP8 indentation (4 space) and Google-style doc string instead of sslab-style. See secure-systems-lab/code-style-guidelines#20 - Does not support Python =< 3.5 Co-authored-by: Trishank Karthik Kuppusamy <trishank.kuppusamy@datadoghq.com> Co-authored-by: Joshua Lock <jlock@vmware.com> Co-authored-by: Teodora Sechkova <tsechkova@vmware.com> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
added a commit
to lukpueh/tuf
that referenced
this issue
Aug 18, 2020
Add metadata module with container classes for TUF role metadata, including methods to read/serialize/write from and to JSON, perform TUF-compliant metadata updates, and create and verify signatures. The 'Metadata' class provides a container for inner TUF metadata objects (Root, Timestamp, Snapshot, Targets) (i.e. OOP composition) The 'Signed' class provides a base class to aggregate common attributes (i.e. version, expires, spec_version) of the inner metadata classes. (i.e. OOP inheritance). The name of the class also aligns with the 'signed' field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to well represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). This commits also adds tests. **TODO: See doc header TODO list** **Additional design considerations** (also in regards to prior sketches of this module) - Aims at simplicity, brevity and recognizability of the wireline metadata format. - All attributes that correspond to fields in TUF JSON metadata are public. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code, and impedes recognizability of the wireline metadata format. - Although, it might be convenient to have short-cuts on the Metadata class that point to methods and attributes that are common to all subclasses of the contained Signed class (e.g. Metadata.version instead of Metadata.signed.version, etc.), this also conflicts with goal of recognizability of the wireline metadata. Thus we won't add such short-cuts for now. See: theupdateframework#1060 (comment) - Signing keys and a 'consistent_snapshot' boolean are not on the targets metadata class. They are a better fit for management code. See: theupdateframework#1060 (comment), and theupdateframework#660. - Does not use sslib schema checks (see TODO notes about validation in doc header) - Does not use existing tuf utils, such as make_metadata_fileinfo, build_dict_conforming_to_schema, if it is easy and more explicit to just re-implement the desired behavior on the metadata classes. - All datetime's are treated as UTC. Since timezone info is not captured in the wireline metadata format it should not be captured in the internal representation either. - Does not use 3rd-party dateutil package, in order to minimize dependency footprint, which is especially important for update clients which often have to vendor their dependencies. However, compatibility between the more advanced dateutil.relativedelta (e.g handles leap years automatically) and timedelta is tested. - Uses PEP8 indentation (4 space) and Google-style doc string instead of sslab-style. See secure-systems-lab/code-style-guidelines#20 - Does not support Python =< 3.5 Co-authored-by: Trishank Karthik Kuppusamy <trishank.kuppusamy@datadoghq.com> Co-authored-by: Joshua Lock <jlock@vmware.com> Co-authored-by: Teodora Sechkova <tsechkova@vmware.com> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
added a commit
to lukpueh/tuf
that referenced
this issue
Aug 20, 2020
Add metadata module with container classes for TUF role metadata, including methods to read/serialize/write from and to JSON, perform TUF-compliant metadata updates, and create and verify signatures. The 'Metadata' class provides a container for inner TUF metadata objects (Root, Timestamp, Snapshot, Targets) (i.e. OOP composition) The 'Signed' class provides a base class to aggregate common attributes (i.e. version, expires, spec_version) of the inner metadata classes. (i.e. OOP inheritance). The name of the class also aligns with the 'signed' field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to well represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). This commits also adds tests. **TODO: See doc header TODO list** **Additional design considerations** (also in regards to prior sketches of this module) - Aims at simplicity, brevity and recognizability of the wireline metadata format. - All attributes that correspond to fields in TUF JSON metadata are public. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code, and impedes recognizability of the wireline metadata format. - Although, it might be convenient to have short-cuts on the Metadata class that point to methods and attributes that are common to all subclasses of the contained Signed class (e.g. Metadata.version instead of Metadata.signed.version, etc.), this also conflicts with goal of recognizability of the wireline metadata. Thus we won't add such short-cuts for now. See: theupdateframework#1060 (comment) - Signing keys and a 'consistent_snapshot' boolean are not on the targets metadata class. They are a better fit for management code. See: theupdateframework#1060 (comment), and theupdateframework#660. - Does not use sslib schema checks (see TODO notes about validation in doc header) - Does not use existing tuf utils, such as make_metadata_fileinfo, build_dict_conforming_to_schema, if it is easy and more explicit to just re-implement the desired behavior on the metadata classes. - All datetime's are treated as UTC. Since timezone info is not captured in the wireline metadata format it should not be captured in the internal representation either. - Does not use 3rd-party dateutil package, in order to minimize dependency footprint, which is especially important for update clients which often have to vendor their dependencies. However, compatibility between the more advanced dateutil.relativedelta (e.g handles leap years automatically) and timedelta is tested. - Uses PEP8 indentation (4 space) and Google-style doc string instead of sslab-style. See secure-systems-lab/code-style-guidelines#20 - Does not support Python =< 3.5 Co-authored-by: Trishank Karthik Kuppusamy <trishank.kuppusamy@datadoghq.com> Co-authored-by: Joshua Lock <jlock@vmware.com> Co-authored-by: Teodora Sechkova <tsechkova@vmware.com> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
MVrachev
pushed a commit
to MVrachev/tuf
that referenced
this issue
Sep 17, 2020
Add metadata module with container classes for TUF role metadata, including methods to read/serialize/write from and to JSON, perform TUF-compliant metadata updates, and create and verify signatures. The 'Metadata' class provides a container for inner TUF metadata objects (Root, Timestamp, Snapshot, Targets) (i.e. OOP composition) The 'Signed' class provides a base class to aggregate common attributes (i.e. version, expires, spec_version) of the inner metadata classes. (i.e. OOP inheritance). The name of the class also aligns with the 'signed' field of the outer metadata container. Based on prior observations in TUF's sister project in-toto, this architecture seems to well represent the metadata model as it is defined in the specification (see in-toto/in-toto#98 and in-toto/in-toto#142 for related discussions). This commits also adds tests. **TODO: See doc header TODO list** **Additional design considerations** (also in regards to prior sketches of this module) - Aims at simplicity, brevity and recognizability of the wireline metadata format. - All attributes that correspond to fields in TUF JSON metadata are public. There doesn't seem to be a good reason to protect them with leading underscores and use setters/getters instead, it just adds more code, and impedes recognizability of the wireline metadata format. - Although, it might be convenient to have short-cuts on the Metadata class that point to methods and attributes that are common to all subclasses of the contained Signed class (e.g. Metadata.version instead of Metadata.signed.version, etc.), this also conflicts with goal of recognizability of the wireline metadata. Thus we won't add such short-cuts for now. See: theupdateframework#1060 (comment) - Signing keys and a 'consistent_snapshot' boolean are not on the targets metadata class. They are a better fit for management code. See: theupdateframework#1060 (comment), and theupdateframework#660. - Does not use sslib schema checks (see TODO notes about validation in doc header) - Does not use existing tuf utils, such as make_metadata_fileinfo, build_dict_conforming_to_schema, if it is easy and more explicit to just re-implement the desired behavior on the metadata classes. - All datetime's are treated as UTC. Since timezone info is not captured in the wireline metadata format it should not be captured in the internal representation either. - Does not use 3rd-party dateutil package, in order to minimize dependency footprint, which is especially important for update clients which often have to vendor their dependencies. However, compatibility between the more advanced dateutil.relativedelta (e.g handles leap years automatically) and timedelta is tested. - Uses PEP8 indentation (4 space) and Google-style doc string instead of sslab-style. See secure-systems-lab/code-style-guidelines#20 - Does not support Python =< 3.5 Co-authored-by: Trishank Karthik Kuppusamy <trishank.kuppusamy@datadoghq.com> Co-authored-by: Joshua Lock <jlock@vmware.com> Co-authored-by: Teodora Sechkova <tsechkova@vmware.com> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We have been silently accepting a minor metadata format mismatch between specs and implementation. I think it is time to fix that by choosing one or the other.
Reason for the format as specified: Adopted from TUF
Reason for the format as implemented:
is-arelationship seemed (from an OO perspective) more intuitive and easier to handle thanhas-arealtionship.Implementation
{ "_type": "layout" or "link", ... # rest of the respective properties, "signatures" : [ { "keyid" : KEYID, "method" : METHOD, "sig" : SIGNATURE }, , ... ] }Specification
{ "signed" : ROLE, # layout or link metadata object [**] "signatures" : [ { "keyid" : KEYID, "method" : METHOD, "sig" : SIGNATURE }, , ... ] } # [**] Btw. I find the term `ROLE` for the metadata type really confusing. # While it makes sense for TUF, # where each role corresponds to # an individual metadata format, it does not for in-toto, which does # not have the some notion of roles.The text was updated successfully, but these errors were encountered: