uptane / uptane-standard

standard for Uptane
https://uptane.github.io
Other
37 stars 31 forks source link

Clarify that partial verification is a minimum #165

Closed tkfu closed 4 years ago

tkfu commented 4 years ago

Add some language clarifying that PV ECUs can do more verification if desired, and that this can make the system more flexible.

trishankatdatadog commented 4 years ago

@tkfu Are you done? Was planning to review...

tkfu commented 4 years ago

Done now--just caught a couple proofing errors.

jhdalek55 commented 4 years ago

I think we still have some unresolved comments here. @tkfu when you get a chance can you address these.

iramcdonald commented 4 years ago

Hi,

I strongly agree with @jhdalek55 that "minimum" is the wrong word. I prefer his suggested change.

In standards specs, well-written conformance sections never use the term "minimum". There are simply mandatory requirements (e.g., MUST at least perform partial verification) and recommendations (SHOULD perform full verification).

Cheers,

Ira McDonald (Musician / Software Architect)Co-Chair - TCG Trusted Mobility Solutions WG

Co-Chair - TCG Metadata Access Protocol SG

Chair - Linux Foundation Open Printing WGSecretary - IEEE-ISTO Printer Working GroupCo-Chair - IEEE-ISTO PWG Internet Printing Protocol WGIETF Designated Expert - IPP & Printer MIBBlue Roof Music / High North Inchttp://sites.google.com/site/blueroofmusic http://sites.google.com/site/blueroofmusichttp://sites.google.com/site/highnorthinc http://sites.google.com/site/highnorthincmailto: blueroofmusic@gmail.com blueroofmusic@gmail.com(permanent) PO Box 221 Grand Marais, MI 49839 906-494-2434

On Tue, Jun 23, 2020 at 8:48 AM Lois Anne DeLong notifications@github.com wrote:

@jhdalek55 commented on this pull request.

In uptane-standard.md https://github.com/uptane/uptane-standard/pull/165#discussion_r444196079 :

@@ -818,7 +818,7 @@ The ECU SHALL create a version report as described in {{version_report}}, and se

Metadata verification procedures {#metadata_verification}

-A Primary ECU MUST perform full verification of metadata. A Secondary ECU SHOULD perform full verification of metadata. If a Secondary cannot perform full verification, it SHALL perform partial verification instead. +A Primary ECU MUST perform full verification of metadata. A Secondary ECU SHOULD perform full verification of metadata. If a Secondary cannot perform full verification, it SHALL perform at least partial verification instead.

See explanation above. We are trying to establish that there is a "minimum" that must be performed. If that point is not clear, perhaps it needs to be rephrased.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uptane/uptane-standard/pull/165#discussion_r444196079, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE33UOZX4HXXWB4PNJL2NXTRYCQCBANCNFSM4NZPJTTQ .

trishankatdatadog commented 4 years ago

Agree with @jhdalek55 and @iramcdonald, let's avoid the word "minimum" and be as clear as possible

jhdalek55 commented 4 years ago

@tkfu I know you have a full plate, but is there any chance you can review the suggested changes (including a few wording changes I just proposed today, so we could close this out? Thanks.

iramcdonald commented 4 years ago

Hi,

This thread is still using "minimum" - please don't. Just say that if a Secondary cannot perform full verification, then it MUST perform partial verification. (full stop)

Then nothing stops that Secondary from performing selective full verification steps, as a security enhancement.

BTW - MUST or SHALL - but one or the other has to be removed entirely from the Standard and Deployment Considerations. IETF RFCs use MUST, ISO and ITU-T standards use SHALL. Take your choice please.

Cheers,

Ira McDonald (Musician / Software Architect)Co-Chair - TCG Trusted Mobility Solutions WG

Co-Chair - TCG Metadata Access Protocol SG

Chair - Linux Foundation Open Printing WGSecretary - IEEE-ISTO Printer Working GroupCo-Chair - IEEE-ISTO PWG Internet Printing Protocol WGIETF Designated Expert - IPP & Printer MIBBlue Roof Music / High North Inchttp://sites.google.com/site/blueroofmusic http://sites.google.com/site/blueroofmusichttp://sites.google.com/site/highnorthinc http://sites.google.com/site/highnorthincmailto: blueroofmusic@gmail.com blueroofmusic@gmail.com(permanent) PO Box 221 Grand Marais, MI 49839 906-494-2434

On Fri, Jul 17, 2020 at 3:58 PM Lois Anne DeLong notifications@github.com wrote:

@jhdalek55 commented on this pull request.

In uptane-standard.md https://github.com/uptane/uptane-standard/pull/165#discussion_r456647052 :

@@ -829,6 +829,12 @@ In order to perform partial verification, an ECU SHALL perform the following ste

  1. Load and verify the current time or the most recent securely attested time.
  2. Download and check the Targets metadata file from the Director repository, following the procedure in {{check_targets}}.

+Note that this verification procedure is intended to be a minimum. A partial verification ECU MAY implement more metadata checks. + +For example, in many cases it is desirable to also fetch and verify Root metadata from the Director (following the procedure in {{check_root}}) before checking Targets metadata. Without this check, an ECU will not be able to verify any metadata from the Director if the Director's Targets key is rotated, possible necessitating a recall.

I can't insert the suggestion in the right place for some reason. But, I would re-word line 832 as:

Note that this verification procedure outline the minimum number of actions that MUST be performed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/uptane/uptane-standard/pull/165#discussion_r456647052, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE33UO37F4S6TAG6Y5CCCXLR4CUPDANCNFSM4NZPJTTQ .

jhdalek55 commented 4 years ago

@iramcdonald --we will fix all the wording issues per your suggestion. I just want to be sure that is all that is needed before we can close this out. But, thanks for a better way of phrasing what we need to say.

jhdalek55 commented 4 years ago

Can we try to resolve this or does it require more thought or discussion? It's been up here awhile.

trishankatdatadog commented 4 years ago

Let me know when this PR is ready to review, I'll help

tkfu commented 4 years ago

@trishankatdatadog Ready for review.

iramcdonald commented 4 years ago

Hi,

I agree with Lois's clarification.

I also note that we can add a SHOULD in v1.1.0 (while the MUST has to wait for v2.0.0).

Cheers,

Ira McDonald (Musician / Software Architect)Co-Chair - TCG Trusted Mobility Solutions WG

Co-Chair - TCG Metadata Access Protocol SG

Chair - Linux Foundation Open Printing WGSecretary - IEEE-ISTO Printer Working GroupCo-Chair - IEEE-ISTO PWG Internet Printing Protocol WGIETF Designated Expert - IPP & Printer MIBBlue Roof Music / High North Inchttp://sites.google.com/site/blueroofmusic http://sites.google.com/site/blueroofmusichttp://sites.google.com/site/highnorthinc http://sites.google.com/site/highnorthincmailto: blueroofmusic@gmail.com blueroofmusic@gmail.com(permanent) PO Box 221 Grand Marais, MI 49839 906-494-2434

On Tue, Sep 1, 2020 at 11:27 AM Lois Anne DeLong notifications@github.com wrote:

@jhdalek55 requested changes on this pull request.

Looks good except for one suggested tweak

In uptane-standard.md https://github.com/uptane/uptane-standard/pull/165#discussion_r481228387 :

  1. all Primaries perform full verification;

-1. all Secondaries that are updated via OTA MUST at least perform partial verification; and

+1. all Secondaries that are updated via OTA at least perform partial verification; and

  1. all other ECUs that do not perform verification cannot be updated via OTA.

⬇️ Suggested change

-1. all other ECUs that do not perform verification cannot be updated via OTA.

+1. all other ECUs that do not perform any type of verification cannot be updated via OTA.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/uptane/uptane-standard/pull/165#pullrequestreview-479858851, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE33UO24OOEYFHAVPOACPRLSDUHELANCNFSM4NZPJTTQ .

jhdalek55 commented 4 years ago

What is missing to complete this PR? Can we close it out this week?

tkfu commented 4 years ago

What is missing to complete this PR? Can we close it out this week?

As far as I'm concerned, nothing's missing--I believe I've addressed all comments. We just need one person to actually click the approve button, because that's the process we agreed on. @JustinCappos @mnm678 @trishankatdatadog @iramcdonald