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

Must all private keys be unique? #174

Closed
allen-cain-tm opened this issue Jul 28, 2020 · 28 comments
Closed

Must all private keys be unique? #174

allen-cain-tm opened this issue Jul 28, 2020 · 28 comments
Milestone

Comments

@allen-cain-tm
Copy link
Contributor

allen-cain-tm commented Jul 28, 2020

  1. Uniqueness of ECU Private Key
    When discussing the ECU key in the Standard, Section 5.4.1 states this is a private key unique to the ECU....
    On this topic, the Deployment Considerations states These keys should be unique to the ECU.

    Proposal: Ensure consistency by allowing the ability for these keys to not be unique per ECU in the Standard.

2) Multiple Primary Scenarios
The Standard and Deployment Considerations have both been expanded to allow for a vehicle to have multiple Primaries.
The Standard Section 5.4.2 states If multiple such Primaries are included within a vehicle, each Secondary ECU SHALL have a single Primary responsible for providing its updates.
The Deployment Considerations states If multiple Primaries are active in the vehicle at the same time, then each Primary SHOULD control a mutually exclusive set of Secondaries, so that each Secondary is controlled only by one Primary.

Proposal: Modify the Standard to allow for multiple Primaries to interact with a Secondary.
Proposed wording: If multiple such Primaries are included within a vehicle, each Secondary ECU **SHOULD** have a single Primary responsible for providing its updates

@JustinCappos
Copy link
Contributor

Uniqueness of ECU Private Key
When discussing the ECU key in the Standard, Section 5.4.1 states this is a private key unique to the ECU....
On this topic, the Deployment Considerations states These keys should be unique to the ECU.

Proposal: Ensure consistency by allowing the ability for these keys to not be unique per ECU in the Standard.

Is there a reason we wouldn't just update the deployment considerations? The standard is messier to change and I as far as I know MUST is the right term here.

@allen-cain-tm
Copy link
Contributor Author

Uniqueness of ECU Private Key

Is there a reason we wouldn't just update the deployment considerations? The standard is messier to change and I as far as I know MUST is the right term here.

I believe there is benefit in allowing flexibility for an implementer/adopter to design their key management system.
As such, believe it is beneficial for both Standard and Deployment Considerations to allow for this flexibility.

@mnm678
Copy link
Collaborator

mnm678 commented Jul 30, 2020

2\. **Proposal:** Modify the Standard to allow for multiple Primaries to interact with a Secondary.
    Proposed wording: `If multiple such Primaries are included within a vehicle, each Secondary ECU **SHOULD** have a single Primary responsible for providing its updates`

I think the choice to make this a SHALL in the standard was intentional to ensure that each Secondary ECU does not get conflicting updates from different Primaries, and to ensure that the Director has a consistent view of the ECUs in a vehicle. If we changed it to a SHOULD, I think we would need to add some clarification (possibly in the deployment considerations) about how to handle these events if an ECU communicates with multiple primaries. That being said, we should certainly resolve the conflict between deployment considerations and the standard (using either the SHALL or the SHOULD).

@iramcdonald
Copy link

iramcdonald commented Jul 30, 2020 via email

@jhdalek55
Copy link
Contributor

In our meeting on 12/8, it was agreed that this should be split into two issues: one would focus on the uniqueness of private keys, while the other suggests a wording change to better frame the issues of working with multiple primaries. I can open a new issue, but I can't seem to edit this one to change the title. Can someone assist in breaking these up. They both can be set for V.2.0.0./

@pattivacek
Copy link
Collaborator

@jhdalek55 Looks like I can edit, but what should I edit it to? If you make the new issue, I can rename this one.

@jhdalek55
Copy link
Contributor

jhdalek55 commented Jan 19, 2021

@pattivacek OK, this took me longer than expected, but I have opened Issue #201 to address the multiple Primary issue. Can you change this one now to be just about ECU private keys?

@pattivacek pattivacek changed the title Incosistency Between Standard & Deployment Considerations ECU Private Key Uniqueness Incosistency Jan 28, 2021
@pattivacek pattivacek changed the title ECU Private Key Uniqueness Incosistency ECU Private Key Uniqueness Inconsistency Jan 28, 2021
@pattivacek
Copy link
Collaborator

@pattivacek OK, this took me longer than expected, but I have opened Issue #201 to address the multiple Primary issue. Can you change this one now to be just about ECU private keys?

Done.

@jhdalek55
Copy link
Contributor

@pattivacek Thank you.

@mnm678
Copy link
Collaborator

mnm678 commented Apr 1, 2021

Back to the key uniqueness issue: When would an ECU key not be unique to the ECU? And would a non-unique key mess up the signing of the ECU version report?

For the image encryption case, I don't see any problem with allowing non-unique ECU keys at the OEM's discretion, but I think duplicate keys for signing may introduce a replay attack for version reports.

@jhdalek55
Copy link
Contributor

As of 4/13/21, This issue remains unresolved as the group has not developed a consensus on whether we want to have non-unique keys for signing. Therefore, this is deferred for the moment

@iramcdonald
Copy link

iramcdonald commented Apr 21, 2021 via email

@allen-cain-tm
Copy link
Contributor Author

Hi @iramcdonald , @mnm678 , @jhdalek55

Thank you for your comments and valuable feedback.
Completely understand that this request/issue must be deferred to v2.0 to ensure we do not impact backwards compatibility.

My intent of changing "SHALL" --> "SHOULD" is to allow OEMs to architect their key management infrastructure in a way that is not restricted by the standard.
This allows an OEM to perform their risk analysis and determine appropriate countermeasures based on corresponding risk levels.

To aid in this ticket progressing, please allow me to ask.
Does this proposed change "fundamentally break" anything in the Standard, or is this only a question of risk?

Thank you -

@mnm678
Copy link
Collaborator

mnm678 commented Apr 23, 2021

To aid in this ticket progressing, please allow me to ask.
Does this proposed change "fundamentally break" anything in the Standard, or is this only a question of risk?

The Uptane uses ECU key to refer to a key performing 2 main functions:

  • Signing the ECU version report
  • Decrypting images

In the second case, the uniqueness can certainly be left up to the OEM. Encrypted images are supported by Uptane, but don't really affect the authenticity checks.

In the first case, it looks like a replay attack would be prevented by the use of the "ECU's unique identifier (e.g., the serial number)" in the version report, so the only question is whether the duplicate key could be used to impersonate a vehicle. This would depend on the attack's ability to get the private key and serial number used in a particular vehicle. What do others think here?

@jhdalek55
Copy link
Contributor

This issue was discussed at our meeting on 7/20/21. @mnm678 @iramcdonald, correct this summary if it is not correct. Part of the problem seems to be that the term "private key" can be interpreted in different ways, one of them being a generic term for an identifier. Private keys must be unique. "Secret key" might be a better choice in some instances. What is needed is some clarifying text and perhaps some renaming within the text. We could use a volunteer to submit a PR to provide the necessary clarification.

@jhdalek55
Copy link
Contributor

Still seeking a volunteer....

@jhdalek55
Copy link
Contributor

Can someone please step in here and draft some new text that changes "private key" to "secret key" when used as a generic terms for an identifier?

@jhdalek55
Copy link
Contributor

jhdalek55 commented Sep 17, 2021

@JustinCappos @iramcdonald @tkfu @trishankatdatadog or anyone else we cares to comment.

I just did a quick search in the Standard and the instance cited at the beginning of this issue thread (is the only place where the phrase "private key" is used. (and the term does not seem to appear in the Deployment Best Practices. Given that, do we need to make this wording change in this one instance? Wouldn't private key be the correct term in this instance?

As for the second phrase specifically mentioned in the original post from @allen-cain-tm ....
"ECU key(s), to sign the ECU's version reports, and optionally to decrypt images. These keys should be unique to the ECU, and the public keys will need to be stored in the Director repository's inventory database"....
It seems we can make the distinction between "private key" and "secret key" here by editing the second sentence as follows "While the signing keys, also known as 'private keys' are required to be unique to the ECU to avoid replay attacks, the keys used to decrypt images need not be unique."

Of course I believe this is still side-stepping the original issue of whether signing keys should be unique, but please let me know if I should clarify this one point in the Deployment document.

@iramcdonald
Copy link

iramcdonald commented Sep 17, 2021 via email

jhdalek55 added a commit to jhdalek55/uptane-standard that referenced this issue Sep 27, 2021
Issue uptane#174 addresses the broad issue of whether private keys must be unique. This PR addresses just one instance singled out by @allen-cain-tm  in Section 5.4.1. It adds just one sentence to separate the idea of private keys used for decryption from private keys used for signing. This does NOT address the broader issue and so it does not close out uptane#174
pattivacek added a commit that referenced this issue Sep 28, 2021
@jhdalek55
Copy link
Contributor

Note: This should probably be "re-flagged" for Version 3.0.0, as we only partially addressed the issue with PR#218. I would recommend we change the title to "Must all private keys be unique?"

@jhdalek55
Copy link
Contributor

@mnm678 Can you either close this issue and re-open a new one, or just re-name it as "Must all private keys be unique?" PR #218 addressed only a small wording issue. We have yet to resolve the bigger issue about non-unique private keys.

@mnm678 mnm678 changed the title ECU Private Key Uniqueness Inconsistency Must all private keys be unique? Nov 23, 2021
@jhdalek55
Copy link
Contributor

@mnm678 can you change the milestone label to Future?

@mnm678 mnm678 modified the milestones: Uptane 2.0.0, Future Dec 6, 2021
@jhdalek55
Copy link
Contributor

After a good deal of discussion, it was decided this question should be rewritten. In addition there needs to be a separate issue added to the Deployment pages to make sure all understand the distinct difference between the types of keys.

@jhdalek55
Copy link
Contributor

jhdalek55 commented Oct 11, 2022

See note above and resolved as directed. Then close this issue.

@jhdalek55
Copy link
Contributor

This is being closed so the core question can be more explicitly stated. I am making one more semantic change that relates to this issue and then I will be closing it.

@jhdalek55
Copy link
Contributor

Closing #174 in favor of #241

@jhdalek55
Copy link
Contributor

I guess I don't have the ability to close this issue. Can someone help me with this?

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

No branches or pull requests

6 participants