Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tap 7: Conformance testing #19

Merged
merged 59 commits into from
May 10, 2017
Merged

Tap 7: Conformance testing #19

merged 59 commits into from
May 10, 2017

Conversation

vladimir-v-diaz
Copy link
Contributor

Conformance testing can determine whether an implementation meets the requirements set by a specification. A tool that helps developers and users test that an implementation bahaves according to the TUF specification does not presently exist. Although the reference implementation contains unit tests that verify correct behavior, such as updating metadata in the expected order and blocking known updater attacks, these unit tests only work with the reference implementation. Conformance testing should instead work across different languages and platforms. In other words, the specification should endorse an official tool, compatible with any implementation, and cover how an implementation can be set up for conformance testing.

The implementation of TAP 7 is a work in progress and is available for testing here

@vladimir-v-diaz
Copy link
Contributor Author

vladimir-v-diaz commented Apr 27, 2017

@awwad @JustinCappos Can you please review TAP 7 when you get the chance? I will focus on adding some of the more important attacks and unit tests that remain, and verify that compliant_updater.py (see README) behaves as outlined in TAP 7. Once the attacks are implemented, I'll move on to implementing the actual conformance testing tool that is used by adopters and users.

tap7.md Outdated
[Conformance testing](https://en.wikipedia.org/wiki/Conformance_testing) can
determine whether an implementation meets the requirements set by a
specification. A tool that helps developers and users test that an
implementation bahaves according to the TUF specification does not presently
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo - behaves

tap7.md Outdated
# Motivation

An adopter of the framework, say a developer that has written an implementation
in language X, can currently test for conformance by (1) verifying that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"can currently test for conformance by [doing 1 and/or 2]" is misleading. 1 would not test for conformance. I suggest instead:
"Adopters of the framework who have written implementations have tried testing for conformance by (1) ... "

tap7.md Outdated
behavior when the generated metadata is updated. In the second case, the
implementation is said to be conformant depending on how thoroughly the unit
tests are reproduced in X. There are bound to be inconsistencies between both
sets of unit tests, so a single tool is preferable. A single tool for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider:
"There are bound to be inconsistencies between both sets of unit tests, and improvements in TUF testing or changes to TUF would result in a need for implemeters to add test code in parallel, so a single tool is preferable."

tap7.md Outdated
# Rationale

Developers of an implementation who wish to undergo conformance testing are
required to provide a program, or script, that accepts a specific set of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that we will be doing the conformance testing for them, which is not how we expect to do this (though we're happy to help when possible). Instead, consider:
"Developers of an implementation can conformance test a program or script intended to perform secure updates that accepts a specific set of command-line arguments."

tap7.md Outdated
Developers of an implementation who wish to undergo conformance testing are
required to provide a program, or script, that accepts a specific set of
command-line arguments. The program should be able to perform a secure update.
A fixed set of arguments are needed so that conformance testing is consistent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are -> is

tap7.md Outdated
A fixed set of arguments are needed so that conformance testing is consistent
across different programs. The conformance tester also requires a minimum
number of arguments so that it can thoroughly cover all potential outcomes that
it wishes to test. It should be noted, however, that this program does not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of the sentence that begins with "It should be noted...", consider:
"The implementers' updater may operate in a different mode than it would in production so as to generate interpretable error codes and accept command line arguments that the conformance tester would provide."

tap7.md Outdated
necessarily have to be the updater used in production, only that it should
function as defined in this TAP for conformance testing.

The program itself accepts a command-line argument that indicates the target
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"An implementation to be tested will need to accept ..."

tap7.md Outdated
file to download when the program initiates an update, the location of a TUF
repository that satisfies requests for metadata and targets, and where
downloaded metadata files and the target file are saved when the program
initiates an update. Specifically, the program can be called as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically, the implementation needs to accept arguments as follows:

tap7.md Outdated
(6) malicious mirrors
(7) mix-and-match
(8) rollback
(9) key compromise.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "key compromise" attack and what does it mean to block it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An attacker tries to use a compromised key that has since been revoked. Do you have a suggestion for saying that in a concise way?
"Recover from key compromise?"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an attack, so maybe I should expand the description of the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SECURITY.md page uses "Vulnerability to key compromises." I should use that and say "attacks and weaknesses" in the description of the list.

https://github.com/theupdateframework/tuf/blob/develop/SECURITY.md

tap7.md Outdated
attacks on the updater are present, as defined later in the
*Specification* section of this TAP. Note that the conformance tester provides
the remote files (i.e., repository files) specified on the command-line, and so
it can test for various conditions and input metadata.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second clause doesn't seem to follow, or at least I don't follow the connection. Can you restate this sentence?

@jhdalek55
Copy link
Contributor

jhdalek55 commented May 9, 2017 via email

tap7.md Outdated
populates the directory containing the remote repository files, and executes
the update command. The updater should load *tmp/metadata/root.json* (or the
appropriate path), refresh metadata accordingly, and lastly fetch the
requested update file .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an unclear description. It might help to refer to verified and unverified metadata directories (or perhaps client and repository)? The root file goes into the client's verified metadata directory, is a file the client is taken to have started off with. Repository files sit in another directory.

tap7.md Outdated
4
```

As before, the conformance tool is able to use this excution of the script to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excution -> execution

tap7.md Outdated
framework where metadata or update files are not saved to a file system on the
device. In this case, the developer or user running the conformance tests
would need to arrange for the files that are requested and stored by the device
to be saved to directories that *can* be accessed by the conformance tests.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding "unmodified' -- "to be saved unmodified". The checks in the plan involve hashing the files to establish equality with the metadata files provided by the repository. (I still don't like this, but if we're doing it rather than only relying on return codes and the existence of target files, we should make it clear what the constraints are. For example, in the TUF reference implementation, you couldn't just write roledb contents to file, as those have had signatures stripped from them. The data written to disk has to be identical to the data drawn from the repository and validated in order for the hash check to work as a means of determining whether or not the correct metadata has been obtained and validated.)

"Unmodified" or similar additions may need to be added elsewhere, too; I'm not sure.

tap7.md Outdated
file(s) that the updater eventually trusts, and not that these files are
"installed" in some particular manner. Additionally, the conformance tests must
confirm the updater's ability to detect the attacks covered in the
specification (with the exlusion of the slow retrieval attack).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exlusion -> exclusion

tap7.md Outdated
and then manipulating the rate of transfer. Regardless of the transport
mechanism used, developers should take care to prevent their updaters from
being vulnerable to such attacks, which can happen before any data is
transferred, or after the it has begun.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo - "after the it has begun"

@awwad
Copy link
Contributor

awwad commented May 9, 2017

Up to you, Lois - the comments I left aren't that extensive.

@jhdalek55
Copy link
Contributor

jhdalek55 commented May 9, 2017 via email

vladimir-v-diaz and others added 6 commits May 9, 2017 14:30
Fix description of attacks protected against
Fix the second clause of the paragraph that explains how metadata is generated by the conformance tool
I made my first step in terms of re-organizing the document. I moved up the "Specification Heading" and then added some tentative subheads to this section. I'm not 100 percent sure I made these subhead breaks correctly, and I believe there might be some repetition that could be removed.

I am by no means finished...I haven't even begun to think about the example section. But if you see things that you feel are flat out wrong, by all means correct them. Tomorrow, I'll take a first crack at the example section.
@jhdalek55 jhdalek55 merged commit cf73ec8 into master May 10, 2017
@vladimir-v-diaz
Copy link
Contributor Author

@jhdalek55
Did you accidentally merge this pull request?

@jhdalek55
Copy link
Contributor

jhdalek55 commented May 15, 2017 via email

@vladimir-v-diaz
Copy link
Contributor Author

It's not a problem, but it appears that you accidentally merged the TAP 7 pull request into the "master" branch here, and subsequent commits related to TAP 7 were then committed to the "master" branch:
https://github.com/theupdateframework/taps/commits/master

I've changed it so that PRs cannot be accidentally merged until they've been fully reviewed.

You can continue making edits to TAP 7 on the "master" branch. We'll just update the status of the TAP 7 file once it's accepted.

@jhdalek55
Copy link
Contributor

jhdalek55 commented May 15, 2017 via email

@jhdalek55
Copy link
Contributor

jhdalek55 commented May 16, 2017 via email

@vladimir-v-diaz
Copy link
Contributor Author

vladimir-v-diaz commented May 17, 2017 via email

@jhdalek55
Copy link
Contributor

jhdalek55 commented May 17, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants