Subject: Re: [Emu] AD Review: draft-ietf-emu-noob-03

From: Aura Tuomas <[email-masked]>
Date: Tue, 16 Mar 2021 13:16:34 +0000

Hi Roman,

Thank you for your review. We have made the necessary changes and published version -04. I have also explained the changes made in-line below. Hopefully, the draft is now ready for the next steps.

Regards,
Tuomas

-------- Forwarded Message -------- 
Subject: 	[Emu] AD Review: draft-ietf-emu-noob-03
Date: 	Sun, 7 Mar 2021 13:33:59 +0000
From: 	Roman Danyliw <[email-masked]>

To: 	[email-masked] <[email-masked]>

Hi!

I performed an AD review on draft-ietf-emu-noob-03. Thanks for the work on this document -- in particular for providing copious examples in the Appendix; and co-developing this text with the implementations and the proofs.

** Section 3.2.3. Per "The OOB receiver MUST compare the received value of the fingerprint Hoob ...", perhaps overly pedantic, it would be worth mentioning that this is compared relative to the expected PeerId + Hoob.

Tuomas: Added  "The OOB receiver MUST compare the received value of the fingerprint Hoob (see Section 3.3.2) with a value that it computes locally for the PeerID received."

** Section 3.4.2 and Section 6.6. I wanted to talk through the expected implementation logic around the downgrade protection in the check during the cryptosuite upgrade. Specifically:

(a) Section 3.4.2. The server SHOULD NOT offer and the peer MUST NOT accept
protocol versions or cryptosuites that it knows to be weaker than the
one currently in the Cryptosuitep field of the persistent EAP-NOOB
association.

(b) Section 6.6. As long as
the server or peer saves any information about the other endpoint, it
MUST also remember the previously negotiated cryptosuite and MUST NOT
accept renegotiation of any cryptosuite that is known to be weaker
than the previous one, such as a deprecated cryptosuite.

To make sure I understand that right, let's say I registered cryptosuite = 3 as "ECDHE curve Curve25519 + SHA-512". If the peer initially used this new cryptosuite=3 and later tried to negotiate the current cryptosuite=1, this should fail because SHA-256 is weaker than SHA-512? What about the situation of hypothetical cryptosuite = 4 as "fancy new PQ-resistant algo + SHA-256"? 
No issues with the suggested design, but perhaps we should further caveat somewhere in the document by adding language that determining the relative strength of the cryptosuites is out of scope and may be managed through local policy or configuration.

Tuomas: Yes, thank you for raising this. We have added: "Determining the relative strength of the cryptosuites is out of scope and may be managed through local policy or configuration at the peer and server." 

** Section 4. Per "The EAP Method Type number for EAP-NOOB needs to be assigned", can the explicit registry name for this be explicitly named.

Tuomas: We now call out the registry name explicitly. We realized that all the new registries created should also have explicit names. We have made the necessary changes. 

** Section 4.1. Per "public-key format [RFC7517] Section 6.2.1" in both cryptosuites, RFC5717 doesn't have a Section 6.2.1.

Tuomas: Good catch. This is a remanent from when the text was pointing to section 6.2.1 of RFC 7518. Fixed.

** Section 5.4. Editorial. Please add the model URLs as a reference instead of a bare URL

Tuomas: The URLs are now informational references. 

** Section 5.4, 6.1, 6.2, 6.6, Appendix E. In the spirit of inclusive language, please consider: s/man-in-the-middle/on-path/

** Section 6.5. In the spirit of inclusive language, please consider: s/blacklist misbehaving peer devices/add misbehaving peer devices to a deny list/

Tuomas: We have now made the appropriate terminology changes.

** Appendix C. Per "Table 11 lists some suggested data fields for ServerInfo. Further specifications may specify application- specific ServerInfo and PeerInfo contents.":

-- I would recommend tuning the guidance to make it clear that if these fields names are used in any OOB-enabled application their semantics will be as defined here (I stumbled over calling these "suggested data fields").

NEW:
Table 11 defines commonly used data fields for ServerInfo. Further specifications may define additional application-specific ...

-- Is there an EAP reference to describe handle unknown fields?

Tuomas: Error types 5002 and 5004 handle the cases where ServerInfo and PeerInfo have unknown fields. 

-- Did the WG discuss/consider defining an IANA registry to manage the Peer/ServerInfo fields to ensure there are clear pointers to their semantics?

Tuomas: This is was something briefly eluded to by the IoT directorate review of Dave Thaler. Based on Dave's recommendation, we had added a type tag. We consulted RFC 8216 again and like your recommendation of making the PeerInfo and ServerInfo semantics clearer with an IANA registry. We were initially hesitant but 'specification required' is flexible enough to not prevent new applications of EAP-NOOB from defining new data fields. PeerInfo and ServerInfo are now IANA registries. 

** Appendix F and Section 3.3.2. The existing examples in this Appendix are very helpful. One additional place where I was looking for an illustrative example was the JSON input that would get hashed into the Hoob, MACs. Just one of them would have been useful.

Tuomas: The example inputs of Hoob, MACs, MACp, etc. have now been added. 

Regards,
Roman

---

From: Roman Danyliw <[email-masked]>
Date: Tue, 30 Mar 2021 18:26:13 +0000

Hi Tuomas!

Thanks for the inline explanation below and the new text in -04.  This new content addresses my feedback so I advanced the document to IETF LC.

Regards,
Roman

> -----Original Message-----
> From: Aura Tuomas <[email-masked]>
> Sent: Tuesday, March 16, 2021 9:17 AM
> To: Roman Danyliw <[email-masked]>; [email-masked]
> Subject: RE: AD Review: draft-ietf-emu-noob-03
> 
> Hi Roman,
> 
> Thank you for your review. We have made the necessary changes and published
> version -04. I have also explained the changes made in-line below. Hopefully,
> the draft is now ready for the next steps.
> 
> Regards,
> Tuomas
> 
> 
> I performed an AD review on draft-ietf-emu-noob-03. Thanks for the work on
> this document -- in particular for providing copious examples in the Appendix;
> and co-developing this text with the implementations and the proofs.
> 
> ** Section 3.2.3. Per "The OOB receiver MUST compare the received value of
> the fingerprint Hoob ...", perhaps overly pedantic, it would be worth
> mentioning that this is compared relative to the expected PeerId + Hoob.
> 
> Tuomas: Added  "The OOB receiver MUST compare the received value of the
> fingerprint Hoob (see Section 3.3.2) with a value that it computes locally for
> the PeerID received."
> 
> 
> ** Section 3.4.2 and Section 6.6. I wanted to talk through the expected
> implementation logic around the downgrade protection in the check during the
> cryptosuite upgrade. Specifically:
> 
> protocol versions or cryptosuites that it knows to be weaker than the one
> currently in the Cryptosuitep field of the persistent EAP-NOOB association.
> 
> the server or peer saves any information about the other endpoint, it MUST
> also remember the previously negotiated cryptosuite and MUST NOT accept
> renegotiation of any cryptosuite that is known to be weaker than the previous
> one, such as a deprecated cryptosuite.
> 
> To make sure I understand that right, let's say I registered cryptosuite = 3 as
> "ECDHE curve Curve25519 + SHA-512". If the peer initially used this new
> cryptosuite=3 and later tried to negotiate the current cryptosuite=1, this should
> fail because SHA-256 is weaker than SHA-512? What about the situation of
> hypothetical cryptosuite = 4 as "fancy new PQ-resistant algo + SHA-256"?
> No issues with the suggested design, but perhaps we should further caveat
> somewhere in the document by adding language that determining the relative
> strength of the cryptosuites is out of scope and may be managed through local
> policy or configuration.
> 
> Tuomas: Yes, thank you for raising this. We have added: "Determining the
> relative strength of the cryptosuites is out of scope and may be managed
> through local policy or configuration at the peer and server."
> 
> 
> ** Section 4. Per "The EAP Method Type number for EAP-NOOB needs to be
> assigned", can the explicit registry name for this be explicitly named.
> 
> Tuomas: We now call out the registry name explicitly. We realized that all the
> new registries created should also have explicit names. We have made the
> necessary changes.
> 
> 
> ** Section 4.1. Per "public-key format [RFC7517] Section 6.2.1" in both
> cryptosuites, RFC5717 doesn't have a Section 6.2.1.
> 
> ** Section 5.4, 6.1, 6.2, 6.6, Appendix E. In the spirit of inclusive language,
> please consider: s/man-in-the-middle/on-path/
> 
> ** Section 6.5. In the spirit of inclusive language, please consider: s/blacklist
> misbehaving peer devices/add misbehaving peer devices to a deny list/
> 
> ** Appendix C. Per "Table 11 lists some suggested data fields for ServerInfo.
> Further specifications may specify application- specific ServerInfo and PeerInfo
> contents.":
> 
> -- I would recommend tuning the guidance to make it clear that if these fields
> names are used in any OOB-enabled application their semantics will be as
> defined here (I stumbled over calling these "suggested data fields").
> 
> NEW:
> Table 11 defines commonly used data fields for ServerInfo. Further
> specifications may define additional application-specific ...
> 
> -- Did the WG discuss/consider defining an IANA registry to manage the
> Peer/ServerInfo fields to ensure there are clear pointers to their semantics?
> 
> Tuomas: This is was something briefly eluded to by the IoT directorate review
> of Dave Thaler. Based on Dave's recommendation, we had added a type tag.
> We consulted RFC 8216 again and like your recommendation of making the
> PeerInfo and ServerInfo semantics clearer with an IANA registry. We were
> initially hesitant but 'specification required' is flexible enough to not prevent
> new applications of EAP-NOOB from defining new data fields. PeerInfo and
> ServerInfo are now IANA registries.
> 
> 
> ** Appendix F and Section 3.3.2. The existing examples in this Appendix are
> very helpful. One additional place where I was looking for an illustrative
> example was the JSON input that would get hashed into the Hoob, MACs. Just
> one of them would have been useful.
>