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

Stefan Hristozov's review of -12 #194

Closed
emanjon opened this issue Nov 4, 2021 · 1 comment
Closed

Stefan Hristozov's review of -12 #194

emanjon opened this issue Nov 4, 2021 · 1 comment

Comments

@emanjon
Copy link
Collaborator

emanjon commented Nov 4, 2021

https://mailarchive.ietf.org/arch/msg/lake/Od5HG1M9TtOytpJ4Iblsc_2xsE0/

========

  1. Outline
    "ID_CRED_I and ID_CRED_R are credential identifiers enabling the recipient party to retrieve the credential of I and R, respectively."
    I will replace this definition with something like:
    ID_CRED_I and ID_CRED_R are used to identify and optionally transport the authentication keys of the Initiator and the Responder, respectively.

3.8 EAD
Is EAD data generated by the application or data that is pre-provisioned or obtained from somewhere. If the former is true I would like to suggest that the specification allows for implementations where all inputs and output that are generated at run time by the application are provided in non-encoded form to the EDHOC implementation. In this way, the interface to the application will be simpler and CBOR encoding and decoding can be completely hidden from the application developer. This applies especially for EAD, see issue #186. The general question is who is supposed to encode/decode EAD? The application or the EDHOC implementation? As far I understand the specification now only the application knows how to encode and decode EAD.

5.2.1
"If the most preferred cipher suite is selected then SUITES_I is encoded as that cipher suite, i.e., as an int."
Am I understanding that correctly: If other suites are supported in addition they are not sent, e.g., if the initiator supports suites 1,2,3, where 1 is preferred and selected, 1 is sent as int and 2,3 are not sent? If so I will suggest making this sentence more clear.

6 Error Handling
What is the use case for a success error code? Probably it is good to give some example or reference why it is useful to log successes using a predefined error code and encoding. Is logging the only use case for the success error code? For example, my implementation logs many things for debugging purposes. However, I never needed a success error code.

The spec says that success error code must not be sent, therefore the sentence "Error code 0 MAY be used internally.." needs to be "Error code 0 MAY be used only internally.."?

"ERR_INFO can contain any type of CBOR item", see figure 7. Who decides what is the type of the CBOR item? Is this the EDHOC implementation developer?

7 Mandatory-to-Implement Compliance Requirements
"Constrained endpoints SHOULD implement cipher suite 0 or cipher suite 2."
The difference between 0 and 1 and between 2 and 3 is only the size of the tag, i.e. the used algorithms are the same. -> I will suggest changing to "...suite 0/1 or cipher suite 2/3" or similar.

Error messages with which error codes are mandatory to implement? Is only an error message with ERR_CODE 2 mandatory to implement?

8.7 Implementation consideration
"The selection of trusted CAs should be done very carefully and certificate revocation should be supported."

Is OCSP (RFC6960) what should be used for certificate revocation checking? How revocation can be accomplished with C509? How OCSP and EDHOC interact? Can OCSP stapling be used with EDHOC? Can we combine OCSP stapling with EAD?

Additionally, to verify a certificate the device should be aware of the time, which is often problematic on constrained devices, i.e. when certificates are used the device must have a Real-Time Clock (RTC).

@gselander
Copy link
Collaborator

#200 opened to address this review.

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

No branches or pull requests

2 participants