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

2021-03-30 Thread Roman Danyliw
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 
> Sent: Tuesday, March 16, 2021 9:17 AM
> To: Roman Danyliw ; emu@ietf.org
> 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
> 
> 
> -------- Forwarded Message --------
> Subject:  [Emu] AD Review: draft-ietf-emu-noob-03
> Date: Sun, 7 Mar 2021 13:33:59 +
> From: Roman Danyliw 
> 
> To:   emu@ietf.org 
> 
> 
> 
> 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 "

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

2021-03-16 Thread Aura Tuomas
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 +
From:   Roman Danyliw 

To: emu@ietf.org 



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 enoug

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

2021-03-07 Thread Roman Danyliw
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.

** 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.

** 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.

** 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.  Editorial.  Please add the model URLs as a reference instead 
of a bare URL

** 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 ...

-- Is there an EAP reference to describe handle 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?

** 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.

Regards,
Roman

___
Emu mailing list
Emu@ietf.org
https://www.ietf.org/mailman/listinfo/emu