Secondary partial verification #143

shikhar394 · 2017-11-08T22:46:19Z

Purpose of the PR:

The PR implements partial verification for secondary ECUs. Addresses issue #48

Summary of Changes:

Working partial verification for secondaries.
Imports director's public key if partial_verifying = True
If partial_verifying = True, secondaries accept the target's data from the primaries and write them to director_targets.der (Demo_secondary)
Secondaries (secondary.py) reads the targets file and checks it using tuf.updater.singlerepo._verify_uncompressed_metadata_file
If the check passes, it formats the data to right the metadata json format and assigns it to validated targets for the secondary

Further Requirements:

Removing print/debug statements
Refactoring some code to improve clarity and reduce redundancy

coveralls · 2017-11-08T23:00:26Z

Coverage decreased (-1.8%) to 94.366% when pulling 09d416ca7e7e68e680cd3aea7af445925e9b4c06 on shikhar394:Secondary_PartialVerification into c252f59 on uptane:develop.

awwad

In trying to run this, I've run into some errors that don't make sense to me. I'll continue to debug. Meanwhile, however, I thought I'd give you the rest of the feedback on the PR.

There are some style, documentation, and behavior issues that are readily fixed - see comments. Also, there aren't instructions on how to use the new feature, and the added code is untested and drops coverage by 2% or so.

awwad · 2017-12-13T18:29:01Z

uptane/clients/secondary.py




+  def process_partial_metadata(self, metadata_archive_fname):


We would need to get coverage back up above 95 or 96% before merging this. If it stays, process_partial_metadata() would need a test written for it in test_secondary, for example.

awwad · 2017-12-14T16:28:30Z

demo/demo_primary.py

@@ -592,7 +591,6 @@ def get_metadata_for_ecu(ecu_serial, force_partial_verification=False):
  # being created outside of the target extraction directory.


-


You remove 4 blank lines from module demo_primary.py (and make no other changes to it). These removals don't serve a purpose and caused a conflict when other changes were merged into develop in this section of the code. This is a good reason to avoid unneeded/unrelated changes in a PR. (:

I'll push a commit that removes them to resolve the conflict (though I'll leave this here for a bit first to make sure you see it before it collapses).

awwad · 2017-12-14T16:29:27Z

demo/demo_primary.py

@@ -609,15 +607,13 @@ def get_metadata_for_ecu(ecu_serial, force_partial_verification=False):
      #     repr(ecu_serial) + ') does not appear in the mapping of ECU Serials '
      #     'to CAN IDs.')
      fname = primary_ecu.get_full_metadata_archive_fname()
-


As with the previous comment, the removal of this blank line resulted in a conflict.

awwad · 2017-12-14T16:29:55Z

demo/demo_primary.py

    if not os.path.exists(fname):
      raise uptane.Error('This Primary does not have a collection of metadata '
          'to distribute to Secondaries.')

    print('Distributing metadata to ECU ' + repr(ecu_serial))

    binary_data = xmlrpc_client.Binary(open(fname, 'rb').read())
-


As with the previous comment, the removal of this blank line resulted in a conflict.

awwad · 2017-12-14T16:35:13Z

demo/demo_secondary.py

@@ -85,12 +86,13 @@ def clean_slate(
  global _ecu_serial
  global _primary_host
  global _primary_port
+  global _partial_verifying


What is the purpose of the global _partial_verifying? The only place you use it is in the same function where it's set, clean_slate, and it's set by the value passed in. You should just be checking partial_verifying, no?

The other globals here are a bit different in that they are module defaults that are used in multiple functions, and must be changed in all those functions if someone calls clean_slate with a different argument (hence the module global). I admit that it's not a great construct to begin with 😃 , but in the case of _partial_verifying, I don't think there's a point.

awwad · 2017-12-14T17:06:27Z

uptane/clients/secondary.py

+  def process_partial_metadata(self, metadata_archive_fname):
+    """
+    Implementation for partial verification in secondaries. 
+    Accepts the archive fname. Processes it to only get a tuf.util.tempfile


Not an archive

awwad · 2017-12-14T17:13:03Z

uptane/clients/secondary.py

+    for Director's targets metadata from the archive. 
+    Checks the targets metadata for the different conditions and attacks as
+    specified in the implementation requirements. 
+    If it passes all checks, the update will be installed.


This is misleading, as this function doesn't perform the "installation". The image file need not even have been retrieved from the Primary yet. This function, a sister function to fully_validate_metadata, performs partial validation and saves the target info for a validated target (hash, length, etc.).

awwad · 2017-12-14T17:14:17Z

uptane/clients/secondary.py

+
+    #z = zipfile.ZipFile(metadata_archive_fname)
+    #z.extract('director/metadata/targets.der', self.full_client_dir)
+    #director_target_file = os.path.join(self.full_client_dir, 'director', 'metadata', 'targets.der')


Did you mean to add these commented-out lines?

awwad · 2017-12-14T17:15:47Z

uptane/clients/secondary.py

    z.extractall(os.path.join(self.full_client_dir, 'unverified'))





+
+


NBD, but no need for the extra two lines.

awwad · 2017-12-14T17:17:41Z

uptane/clients/secondary.py

+    data = tuf.util.load_file(metadata_archive_fname)
+    targets = data['signed']['targets']
+    for file in targets.keys():
+      target_metadata['filepath'] = file                


There's line-ending whitespace in this function and its docstring. Please watch that. It's a source of future changes that may be hard to spot, and conflicts.

awwad · 2017-12-15T16:12:37Z

While a Director key is provided to the PV secondary in initialization, it isn't actually used at all.

awwad · 2017-12-15T16:21:55Z

Using tuf.client.updater._verify_uncompressed_metadata() this way might be using too much of the TUF stack. That ... might be okay if some things are changed, but either way the Director key provided on initialization is quite misleading, as it's never used. What is used instead is a root file the partial verification secondary starts with.

Note that one of the issues that arises from trying to do this this way is that the root file cannot be replaced and will eventually expire. If we want to use this much of the TUF stack, instead of just using a Director Targets key provided on initialization, we have to think through it more carefully (no expiration, how to replace the root file in certain circumstances, how much code we're expecting partial verification secondaries to support...).

shikhar394 · 2017-12-15T19:31:11Z

Thanks for the reviews, Sebastien!! I will carefully fix all of them on this branch. I will make sure that these issues never come up in my future pushes and also look through my other PRs to fix these issues so you don’t have to review the same mistakes. I should have a fix for them all after finals get over!! Thanks again for all the help!

awwad · 2018-01-05T18:35:37Z

WRT the Director Targets signature verification on a partial verification secondary, the correct way to do this is much lower level, using only tuf.keys (or the lowest level function in tuf.sig) rather than tuf.client.updater.

While it might be nice to be able to use the root file in the partial verification design on rare occasion, this isn't the design we ended with, so there's no call for the use of so much of the TUF stack (employing role_db and key_db and looking up what keys are expected for the Targets role based on what root says doesn't make sense.).

There are several places in the TUF and Uptane code where you can find signature verification given just a signable and a public key -- or data, signature, and public key.

coveralls · 2018-01-05T18:59:47Z

Coverage decreased (-1.4%) to 94.811% when pulling 03fa6ca2a8efee62126577f74e8db55f5116196e on shikhar394:Secondary_PartialVerification into 6ec703a on uptane:develop.

coveralls · 2018-01-05T19:00:42Z

Coverage decreased (-1.4%) to 94.811% when pulling 03fa6ca2a8efee62126577f74e8db55f5116196e on shikhar394:Secondary_PartialVerification into 6ec703a on uptane:develop.

coveralls · 2018-01-06T16:55:07Z

Coverage decreased (-1.6%) to 94.655% when pulling 69f83e6a794ca0351b9d4da278332276a62b0483 on shikhar394:Secondary_PartialVerification into 6ec703a on uptane:develop.

coveralls · 2018-01-11T07:27:51Z

Coverage decreased (-0.3%) to 95.912% when pulling baee8b9982341a3295475c3c1056331531a9cb86 on shikhar394:Secondary_PartialVerification into 6ec703a on uptane:develop.

coveralls · 2018-01-16T21:56:14Z

Coverage decreased (-0.2%) to 96.069% when pulling db8cb706e9abbcb295ad9224c4c8e53a3488cda9 on shikhar394:Secondary_PartialVerification into 6ec703a on uptane:develop.

coveralls · 2018-01-17T02:16:37Z

Coverage decreased (-0.2%) to 96.069% when pulling d526f65e8565145bf361b5ddacf019feb3859354 on shikhar394:Secondary_PartialVerification into 6ec703a on uptane:develop.

coveralls · 2018-01-17T21:11:29Z

Coverage decreased (-0.3%) to 95.915% when pulling 869f44fadf660b859cc71e1171936e9c2503e7ca on shikhar394:Secondary_PartialVerification into 56622b6 on uptane:develop.

coveralls · 2018-01-17T22:00:39Z

Coverage increased (+0.3%) to 96.661% when pulling 43c1262 on shikhar394:Secondary_PartialVerification into 68e49e1 on uptane:develop.

awwad · 2018-01-22T15:53:41Z

uptane/clients/secondary.py

    #Combs through the director's targets metadata to find the one assigned to the
    #current ECU. 
    for target in targets: 
      if targets[target]['custom']['ecu_serial'] == self.ecu_serial:
        target_metadata['filepath'] = target
        target_metadata['fileinfo'] = targets[target]
        validated_targets_for_this_ecu.append(target_metadata)
+


I'm confused: how did these whitespace changes "stop Travis CI from working"? (Was there a conflict or something?)

Oh, I believe GitHub covered up the changes. I added some old code I had deleted in a previous commit. The deletion was causing the issue, hence, I put the code back and it worked!

I'm looking at one commit, not a combination of commits. How would it be covered up?

This is strange, I can't find the commit where I deleted lines 597 through 607 in order to have less code, since I thought that should work. On closer inspection I realized that it wouldn't and basically reverted the changes to what it was originally.
I could rename the commit if it is an issue?
Thanks!

awwad · 2018-01-23T22:07:22Z

Rebased onto the most recent develop commit, removed a few empty commits, and squashed some others together to tidy up. Mostly left the commit messages alone.

Still reviewing.

awwad

There are some problems here. See the comments. Due to the proximity of the next demo, I'll push the changes myself. I've already written them and will push after I've given you a chance to see this, since some of the comments will be hidden afterwards.

Meanwhile, I'll do some testing.

awwad · 2018-01-23T20:24:23Z

tests/test_secondary.py

-secondary_instances = [None, None, None]
+# Initializes the number of secondary instances to the number of 
+# TEMP_CLIENT_DIRS
+secondary_instances = [None] * len(TEMP_CLIENT_DIRS)


Minor:
While scaling this list automatically if the tests change in the future is nicer in general, in a few lines we go right back to manually setting values for each secondary below, so we haven't really gained anything by doing it two different ways. But good instinct.

awwad · 2018-01-23T20:28:54Z

tests/test_secondary.py

+      #Checks the number of secondaries for PV 
+      if i == PV_SECONDARY1_INDICE: 
+        self.assertTrue(instance.partial_verifying)
+        self.assertFalse(None is instance.director_public_key)


Minor:
There's an assertIsNotNone, so assertIsNotNone(instance.director_public_key)

awwad · 2018-01-23T20:29:16Z

tests/test_secondary.py

+        self.assertFalse(None is instance.director_public_key)
+      else:
+        self.assertFalse(instance.partial_verifying)
+        self.assertTrue(None is instance.director_public_key)


Minor:
assertIsNone(instance.director_public_key)

awwad · 2018-01-23T22:25:18Z

uptane/clients/secondary.py

@@ -553,11 +554,81 @@ def process_metadata(self, metadata_archive_fname):
    Fully validate the metadata for the target file(s)
    """
    tuf.formats.RELPATH_SCHEMA.check_match(metadata_archive_fname)
+    if self.partial_verifying:


docstring above here is no longer accurate due to changes. Needs to be fixed.

metadata_archive_fname is now also not an accurate representation, since it could be an archive of all metadata or a single metadata file.

awwad · 2018-01-29T16:42:02Z

uptane/clients/secondary.py

+  def process_partial_metadata(self, director_targets_metadata):
+    """
+    Implementation for partial verification in secondaries.
+    Accepts the director targets metadata.


When writing something like this, you should take care to make clear what the arguments are. In this case, you should clarify that argument director_targets_metadata is a filename (rather than a dictionary of role metadata or a file object). I like to try to include fname in filename variables, usually. See the lab code style guidelines' naming conventions, which emphasize distinguishing filename, file object, etc.

While I don't think it's required for this function, it might be nonetheless be wise to have a docstring like those of the public functions here (including the standard sections, like arguments, purpose, returns, side-effects, and exceptions), since this function is somewhat complex in its side-effects.

awwad · 2018-01-30T16:44:15Z

uptane/clients/secondary.py

+    Implementation for partial verification in secondaries.
+    Accepts the director targets metadata.
+    Checks the targets metadata for the different conditions and attacks as
+    specified in the implementation requirements.


Major:

It seems not to perform half of these checks, actually. There's no check for metadata expiration (time < expiration time in validated metadata) and no check for rollback (version in metadata >= previously saved Director Targets metadata version number).

Previously, the updater.target() calls stack would have done those for you, but we can't use that for partial verification, as noted.

Because of the proximity to a demo, I'll update it myself (if I haven't already by the time this review posts).

awwad · 2018-01-30T16:49:44Z

uptane/clients/secondary.py

+    if self.director_public_key is None:
+      raise uptane.Error("Director public key not found for partial"
+          " verification of secondary.")
+    validated_targets_for_this_ecu = []


Since this is the only place that the Secondary retains the fileinfo for the firmware the Director last approved for it, you should probably not empty this until you're sure the new metadata has been validated and contains a new value. Otherwise, anytime you receive bad metadata, you'll lose the record of what the Director last approved. (You'll still know what is currently installed, but that's not the same thing: installations can fail, etc.)

It may also be wise to gather up the fields that represent the Director's last valid, received instructions to this ECU, and make it clear that that's what they are with some comments or naming. This is a reference implementation, so it should be helpful in that way to its consumers, who will often be implementers.

awwad · 2018-01-30T17:41:57Z

uptane/clients/secondary.py

+    metadata_file_object = tuf.util.load_file(director_targets_metadata)
+
+    data = metadata_file_object['signed']
+    signature = metadata_file_object['signatures'][0]


It would be preferable to loop over the signatures over the metadata, and if any of them are a valid signature by the required key, to deem the metadata valid. When doing this, one should check the key IDs before doing the signature checks to save a little time by skipping signatures that report that they were made by key IDs you're not interested in (i.e. that don't match self.director_public_key's key id).

There are several reasons the Director Targets metadata would be signed both with the key that partial verifiers expect and also by other keys, so we should support this in this implementation -- especially since it's easy.

awwad · 2018-02-08T20:30:24Z

tests/test_secondary.py

+    # PV Secondary 1 with valid director public key and update JSON
+    shutil.copy(sample_working_metadata_path, director_targets_metadata_path_json)
+    if not os.path.exists(director_targets_metadata_path_json):
+      raise("Director's targets not available")


First, there are typos here: you mean something like raise Exception(), not raise() -- raise('somestring') doesn't work.

Further, test code shouldn't generally be raising its own exceptions. You should perform a test using self.assertXYZ(something) and allow the test to succeed or fail. In special circumstances, if something is wrong with the test code itself and it's not that you want to indicate that the code being tested is failing, but that the test code itself is broken (like if shutil.copy somehow fails), you might want to have normal assert statements to raise assertion errors (debatable) instead of failing the test. Here, though, I probably wouldn't do either of those things: AFAIK, there's no reason to suppose that shutil.copy(X,Y) will fail to copy X to Y AND not raise an exception on its own. And if someone is toying with the test files immediately after that copy occurs in a randomly named temporary directory, you have larger problems.

So I'll just chop these six lines (if: raise; if: raise; if: raise).

awwad · 2018-02-08T20:44:19Z

uptane/clients/secondary.py

+    elif director_targets_metadata_fname.endswith('der'):
+      data_signed = tuf_asn1_codec.convert_signed_metadata_to_der(
+          {'signed': data, 'signatures': []}, only_signed=True)
+      data_signed = hashlib.sha256(data_signed).digest()


I think code in the reference implementation elsewhere always depends on tuf.conf.METADATA_FORMAT. The Secondary should be using that to determine what kind of file it's dealing with, not basing it on the file ending, I think.

awwad · 2018-02-08T20:54:57Z

tests/test_secondary.py

+     - secondary_instances[3]: Director's targets metadata available with 
+       invalid signatures in JSON mode
+     - secondary_instances[3]: Director's targets metadata available with 
+       valid signatures in DER mode


We should test the same scenarios in JSON and DER modes. The test suites run twice, once with tuf.conf.METADATA_FORMAT set to 'json' and once with tuf.conf.METADATA_FORMAT set to 'der'. As written, this code doesn't test an invalid signature in DER mode, ignores tuf.conf.METADATA_FORMAT, and runs the three tests twice the exact same way (when the full test suite is run by runtests.py in JSON mode, and once when the full test suite is run by runtests.py in DER mode).

vladimir-v-diaz · 2018-02-28T16:12:00Z

The pull request summary might need some updating. I confirmed that tuf.updater.singlerepo._verify_uncompressed_metadata_file is no longer used in this pull request.

awwad · 2018-02-28T16:12:52Z

Notes for reviewer:

The Implementation Specification defines the steps a partial verification Secondary should perform here
Some of the original description is no longer accurate. For example, tuf.client.updater._verify_uncompressed_metadata() is not used. The comments in this PR should cover the changes. (I know it's a bit long.)

- Working partial verification for secondaries - secondary.process_metadata calls partial verification if partial_verifying=True - secondary.process_partial_metadata accepts the directors targets from the primary - Verifies it using updater._verify_uncompressed_metadata_file - Reorganizes the metadata from the targets file to match that of the metadata files used by the ECU to install without any issue

Deal correctly with the expiration time format (importing iso8601 to convert it). Rename signed_data to data_signed with modest hope that it will be somewhat less confusing given TUF's nomenclature. Expand on explanations about data conversion in code comments. Signed-off-by: Sebastien Awwad <[email protected]>

Secondary.validated_targets_for_this_ecu will no longer be reset before new targets have been validated to replace the old values. Previously, if new metadata was encountered, but no target info was provided for this ECU, valiated_targets_for_this_ecu would be emptied. Instead, for both partial and full verification Secondaries, the most recent target info (hash, length, etc) for firmware a Director instructed this particular ECU to install is retained until it is replaced by new target info that is successfully validated. Signed-off-by: Sebastien Awwad <[email protected]>

The demo instructions are already pretty busy, so I'm trying to avoid making them more complicated. This way, they're in a separate section. I also add some detail to the explanation of steps that a demo partial-verification Secondary's update_cycle() call performs. Signed-off-by: Sebastien Awwad <[email protected]>

Signed-off-by: Sebastien Awwad <[email protected]>

Previously, the partial verification Secondary code (same PR) would determine whether to analyze Director Targets metadata provided to it as JSON or DER based on the file extension. This is not consistent with the rest of the code in the reference implementation, so it is corrected here. Signed-off-by: Sebastien Awwad <[email protected]>

Since the partial-verification Secondary now respects tuf.conf.METADATA_FORMAT, the test code can now stop trying to test both formats in both testing modes and instead just test based on content and let tuf.conf.METADATA_FORMAT determine which mode to work in. Added sample bad sig Director Targets metadata in DER format for use in the test. Signed-off-by: Sebastien Awwad <[email protected]>

Signed-off-by: Sebastien Awwad <[email protected]>

in a comment in test_secondary Signed-off-by: Sebastien Awwad <[email protected]>

Signed-off-by: Sebastien Awwad <[email protected]>

tuf.ReplayedMetadataError expects specific arguments rather than just taking an error string - the metadata type, previously validated version, and currently received (replayed) version. Signed-off-by: Sebastien Awwad <[email protected]>

Includes the addition of several sample metadata files for testing. These are now used in test_secondary. Signed-off-by: Sebastien Awwad <[email protected]>

and trim some trailing whitespace in test_secondary (from this same PR) Signed-off-by: Sebastien Awwad <[email protected]>

Previous behavior was to return from the time attestation validation function without updating the time or raising an error. Now, the behavior is to raise an error, which is consistent with the behavior if the Secondary's nonce is missing from the received time attestation. Signed-off-by: Sebastien Awwad <[email protected]>

Expect error similar to time attestation validation when the nonce the Secondary expects is not included. Signed-off-by: Sebastien Awwad <[email protected]>

Test result if signature is by key ID that is not expected (but sig is valid) and if signature is by key ID that is expected but the key type is not the type expected. (Increases code coverage) Signed-off-by: Sebastien Awwad <[email protected]>

Signed-off-by: Sebastien Awwad <[email protected]>

Also slightly rewords the beginning of an error message to make it clearer. Signed-off-by: Sebastien Awwad <[email protected]>

to have shorter names that express what the files are (rather than one example usage). Also updates test_secondary to point to the new names. Signed-off-by: Sebastien Awwad <[email protected]>

Fixes two changes in this PR: uses '.' instead of C++ style '::' class member notation (my bad habit) in test_secondary and removes the single whitespace change to demo_primary made in this PR, for PR cleanliness and code readability. Signed-off-by: Sebastien Awwad <[email protected]>

Will spare someone the effort of generating it, for future tests. Signed-off-by: Sebastien Awwad <[email protected]>

awwad · 2018-02-28T16:24:48Z

Rebased to remove conflicts.

vladimir-v-diaz

Here is my review of README.md.

vladimir-v-diaz · 2018-02-28T15:55:26Z

README.md

+repository. A mismatch results in that installation instruction being ignored.
+- fetches the target images indicated and compares them to the trusted info
+(hash, length, etc.). Any images fetched
+from the repositories that do not match validated metadata are discarded.


How does Uptane deal with clients that are unable to successfully fetch a required image? For instance, a malicious attack where a crucial firmware instructed by the manufacturer is denied to a car.

At the level of our code, the same way as TUF: if a target can't be obtained, an error is raised indicating such.

If you're asking about a higher level, it would be up to firmware developers and car manufacturers to decide what behavior should result based an inability to update in a given incident.

vladimir-v-diaz · 2018-02-28T16:27:06Z

README.md

+Briefly, partial verification entails one signature check to validate metadata,
+and one signature check whenever the minimum time is ratcheted forward (for
+metadata expiration purposes). Partial-verification Secondaries need to know
+the Director's Targets role public key and the Timeserver's public key.


Is there a mechanism for acquiring the public key, or is it left to the implementer? Does the reference implementation provide a way for a PVS to acquire the Director's targets public key?

These are only questions that I ask myself while reading these standalone README edits.

vladimir-v-diaz · 2018-02-28T16:39:50Z

README.md

+(ECU id/serial, vehicle id/VIN, Primary port, etc.).
+
+
+The Secondary's update_cycle() call does the following:


Does it help readability if the parts of the update_cycle() that differ between full-verification and partial-verification stand out (perhaps in bold)?

to use a more sensible data structure. The next two commits will address the remainder of the code (test_50_validate_image). Signed-off-by: Sebastien Awwad <[email protected]>

instead of to an absent file. This way, we can also test the validation of that firmware when we present it to the partial verification Secondary. (Previously, the metadata was validated and firmware itself was not checked against this validated metadat during the testing of the partial verification client.) Signed-off-by: Sebastien Awwad <[email protected]>

as just the f-v Sec's validation of images. Also closes a potential gap in testing whereby the second and third tests for full verification client validation of images were pre-empted by missing files leading to similar errors. Signed-off-by: Sebastien Awwad <[email protected]>

vladimir-v-diaz · 2018-02-28T18:54:25Z

uptane/clients/secondary.py

-      return
+          'Version Manifest to the Primary.')
+      raise uptane.BadTimeAttestation('This Secondary has been have been '
+          'provided a time attestation, but there is no record of this '


This error message needs a revision.

vladimir-v-diaz · 2018-02-28T18:58:42Z

uptane/clients/secondary.py

+      file is validated against the Director's public key
+      (self.director_public_key). If the signature is valid, the new Targets
+      role file is trusted, else it is discarded and we TUF raises a
+      signature exception.


There is a typo in this sentence.

I would also specify the exception raised instead of only "signature exception."

awwad self-requested a review December 13, 2017 18:26

awwad suggested changes Dec 15, 2017

View reviewed changes

awwad force-pushed the Secondary_PartialVerification branch from cc02ba6 to 03fa6ca Compare January 5, 2018 18:47

shikhar394 force-pushed the Secondary_PartialVerification branch from d526f65 to 869f44f Compare January 17, 2018 20:57

awwad reviewed Jan 22, 2018

View reviewed changes

awwad force-pushed the Secondary_PartialVerification branch from 9c7da9e to aa92dff Compare January 23, 2018 22:06

awwad reviewed Jan 30, 2018

View reviewed changes

awwad force-pushed the Secondary_PartialVerification branch from 4147e0f to e3724a0 Compare February 7, 2018 22:56

awwad reviewed Feb 8, 2018

View reviewed changes

awwad self-assigned this Feb 22, 2018

awwad requested a review from vladimir-v-diaz February 22, 2018 15:37

awwad added 21 commits February 28, 2018 11:23

DOC: Expand explanation in README of demo clients' update_cycle()

70aea1e

Signed-off-by: Sebastien Awwad <[email protected]>

Add test to p-v Secondary tests: missing Director pub key

593e48e

Signed-off-by: Sebastien Awwad <[email protected]>

DOC: Note state of test and sample files for secondary

9f07341

in a comment in test_secondary Signed-off-by: Sebastien Awwad <[email protected]>

Simplify some pv secondary test variables in test_secondary

ce97644

Signed-off-by: Sebastien Awwad <[email protected]>

Test partial verification Secondary for expired and replayed metadata

e26b12f

Includes the addition of several sample metadata files for testing. These are now used in test_secondary. Signed-off-by: Sebastien Awwad <[email protected]>

DOC: Add clarifying comment to temp vars in test_secondary

47b292c

and trim some trailing whitespace in test_secondary (from this same PR) Signed-off-by: Sebastien Awwad <[email protected]>

Test time attestation validation by Secondary that never sent a nonce

c7299b7

Expect error similar to time attestation validation when the nonce the Secondary expects is not included. Signed-off-by: Sebastien Awwad <[email protected]>

Test p-v Secondary behavior if Director Targets file is missing

700b4f0

Signed-off-by: Sebastien Awwad <[email protected]>

DOC: Tidy up some demo comments in demo_secondary

8535118

Signed-off-by: Sebastien Awwad <[email protected]>

DOC: Clarify error conditions in process_partial_metadata docstring

9755886

Also slightly rewords the beginning of an error message to make it clearer. Signed-off-by: Sebastien Awwad <[email protected]>

Rename four sample / test data files used by test_secondary

69310a8

to have shorter names that express what the files are (rather than one example usage). Also updates test_secondary to point to the new names. Signed-off-by: Sebastien Awwad <[email protected]>

Add some more sample Director Targets metadata for future use

eb77939

Will spare someone the effort of generating it, for future tests. Signed-off-by: Sebastien Awwad <[email protected]>

awwad force-pushed the Secondary_PartialVerification branch from 2b8aeb2 to 5cc18bc Compare February 28, 2018 16:24

awwad approved these changes Feb 28, 2018

View reviewed changes

vladimir-v-diaz reviewed Feb 28, 2018

View reviewed changes

awwad added 3 commits February 28, 2018 12:20

Reorganize test data use in test_secondary

0f84590

to use a more sensible data structure. The next two commits will address the remainder of the code (test_50_validate_image). Signed-off-by: Sebastien Awwad <[email protected]>

awwad force-pushed the Secondary_PartialVerification branch from 5cc18bc to 43c1262 Compare February 28, 2018 17:58

vladimir-v-diaz reviewed Feb 28, 2018

View reviewed changes

		@@ -592,7 +591,6 @@ def get_metadata_for_ecu(ecu_serial, force_partial_verification=False):
		# being created outside of the target extraction directory.

		z.extractall(os.path.join(self.full_client_dir, 'unverified'))

		(ECU id/serial, vehicle id/VIN, Primary port, etc.).


		The Secondary's update_cycle() call does the following:

Secondary partial verification #143

Are you sure you want to change the base?

Secondary partial verification #143

Conversation

shikhar394 commented Nov 8, 2017 • edited Loading

Purpose of the PR:

Summary of Changes:

Further Requirements:

coveralls commented Nov 8, 2017

awwad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awwad Dec 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awwad Dec 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awwad commented Dec 15, 2017

awwad commented Dec 15, 2017 • edited Loading

shikhar394 commented Dec 15, 2017

awwad commented Jan 5, 2018 • edited Loading

coveralls commented Jan 5, 2018

coveralls commented Jan 5, 2018

coveralls commented Jan 6, 2018

coveralls commented Jan 11, 2018

coveralls commented Jan 16, 2018 • edited Loading

coveralls commented Jan 17, 2018

coveralls commented Jan 17, 2018

coveralls commented Jan 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awwad commented Jan 23, 2018 • edited Loading

awwad left a comment

Choose a reason for hiding this comment

awwad Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

awwad Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

awwad Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awwad Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awwad Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladimir-v-diaz commented Feb 28, 2018

awwad commented Feb 28, 2018

awwad commented Feb 28, 2018

vladimir-v-diaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shikhar394 commented Nov 8, 2017 •

edited

Loading

awwad Dec 14, 2017 •

edited

Loading

awwad Dec 14, 2017 •

edited

Loading

awwad commented Dec 15, 2017 •

edited

Loading

awwad commented Jan 5, 2018 •

edited

Loading

coveralls commented Jan 16, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading

awwad commented Jan 23, 2018 •

edited

Loading

awwad Jan 23, 2018 •

edited

Loading

awwad Jan 23, 2018 •

edited

Loading

awwad Jan 23, 2018 •

edited

Loading

awwad Jan 30, 2018 •

edited

Loading

awwad Feb 8, 2018 •

edited

Loading