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

Refactor of Section 3, 4.1, 4.2 #41

Merged
merged 22 commits into from Dec 17, 2020
Merged

Refactor of Section 3, 4.1, 4.2 #41

merged 22 commits into from Dec 17, 2020

Conversation

gselander
Copy link
Collaborator

Targeting #40

Copy link
Collaborator

@fpalombini fpalombini left a comment

Choose a reason for hiding this comment

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

Great improvement in my opinion (I am thinking from the point of view of implementers/new readers)! Thanks @gselander !

I haven't reviewed all the reference (those that have not been modified in the message formatting sections) to make sure they all point to the right thing.

The changes I suggest can be done in the next update, I'm ok with merging first.

~~~~~~~~~~~
{: #fig-flow title="EDHOC message flow"}
{: #fig-flow title="EDHOC Message Flow"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it helps readability to add all these details at this point in the document... the reader does not know what all the parameters above stand for, and without that I don't know what it adds to the understanding of the protocol.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Starting reading the next section, I see that it refers to this figure. Maybe all I am missing is a sentence saying "The details of the parameters are specified in the following sub-sections".

Cryptographically, EDHOC does not put requirements on the lower layers. EDHOC is not bound to a particular transport layer, and can be used in environments without IP. The transport is responsible to handle message loss, reordering, message duplication, fragmentation, and denial of service protection, where necessary. The Initiator and the Responder need to have agreed on a transport to be used for EDHOC. It is recommended to transport EDHOC in CoAP payloads, see {{transfer}}.
## Method

EDHOC supports authentication with signature or static Diffie-Hellman keys, as defined in the four authentication methods: 0, 1, 2, and 3, see {{method-types}}. (Method 0 corresponds to the case outlined in {{background}} where both Initiator and Responder authenticate with signature keys.)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see {{method-types}}

This points to the IANA registration... I'd rather have the important text here and the IANA registration section point here.


EDHOC supports authentication with signature or static Diffie-Hellman keys, as defined in the four authentication methods: 0, 1, 2, and 3, see {{method-types}}. (Method 0 corresponds to the case outlined in {{background}} where both Initiator and Responder authenticate with signature keys.)

The first data item of message_1, METHOD_CORR (see {{asym-msg1-form}}), is an integer specifying the method and the correlation properties of the transport used, see Sections {{transport}}{: format="counter"}-{{corr}}{: format="counter"}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

General comment: I have reviewed 2 sections and I have been jumping around non-stop already, I think there is too many pointers to other sections in general. For example, we don't need to point to message_1 formatting, I think... let's see if we can slim down the text (possibly for next update).


The Initiator and the Responder need to have agreed on a transport to be used for EDHOC, see {{applicability}}. It is recommended to transport EDHOC in CoAP payloads, see {{transfer}}.

## Connection Identifiers {#ci}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would put this section after the Message Correlation section

}
~~~~~~~~~~~

## Encoding of Public Authentication Key Identifiers {#id_cred}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not calling this section "Authentication Credentials' Identifiers"?

One byte credential identifiers are realistic in many scenarios as most constrained devices only have a few keys. In cases where a node only has one key, the identifier may even be the empty byte string.


## Identities {#auth-key-id}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels somewhat out of place here... I don't have a good proposal on where to put this text, but as of now it does not read to me as a "protocol element", but either as considerations, or maybe should be under the "Authentication Credentials" as a subsection?

## Identifiers

One byte connection and credential identifiers are realistic in many scenarios as most constrained devices only have a few keys and connections. In cases where a node only has one connection or key, the identifiers may even be the empty byte string.
* When public keys are used but not with a PKI (RPK, self-signed certificate), the trust anchor is the public authentication key of the other party. In this case, the identity is typically directly associated to the public authentication key of the other party. For example, the name of the subject may be a canonical representation of the public key. Alternatively, if identities can be expressed in the form of unique subject names assigned to public keys, then a binding to identity can be achieved by including both public key and associated subject name in the protocol message computation: CRED_I or CRED_R may be a self-signed certificate or COSE_Key containing the public authentication key and the subject name, see {{auth-cred}}. Before running EDHOC, each endpoint needs a specific public authentication key/unique associated subject name, or a set of public authentication keys/unique associated subject names, which it is allowed to communicate with. EDHOC provides proof that the other party possesses the private authentication key corresponding to the public authentication key.

## Cipher Suites {#cs}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This section does not introduce the term SUITES_I present in the figure, which would make it compliant with the other sections

@@ -416,12 +511,14 @@ EDHOC allows the communication or negotiation of various protocol features durin

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Adding it here because I can't comment on l.508 - Communication/Negotiation of Protocol Features )

Again this subsection does not seem to fit at the same level as the other "Protocol Elements". I would say this is either "Features" or an appendix, or maybe even as the first subsection of "Message Formatting and Processing"

@@ -528,73 +625,14 @@ To provide forward secrecy in an even more efficient way than re-running EDHOC,
PRK_4x3m = Extract( [ "TH_4", nonce ], PRK_4x3m )
~~~~~~~~~~~

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can add some high level general sentence about how ephemeral keys used in "Ephemeral Public Keys" section.

The key derivation section also belongs better in the "Processing" part of "Message Formatting and Processing" in my opinion (as a subsection)

@gselander gselander merged commit 9264c5d into master Dec 17, 2020
@emanjon emanjon deleted the refactor-overview branch August 19, 2021 14:55
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

3 participants