Re: [Last-Call] Secdir telechat review of draft-ietf-ipsecme-ikev2-multiple-ke-10

[Sean Turner <[e-mail]>]

Thanks for these, I think we’re all resolved!

spt

> On Nov 29, 2022, at 08:31, CJ Tjhai <[e-mail]> wrote:
> 
> Hi Sean,
> 
> Many thanks for the review.
> 
> Please see the comments inlined below.
> 
> Best regards,
> CJ
> 
> PS. Hi @Valery Smyslov, you've beat me to it again. I've added my changes to your PR. 
> 
> 
> 
> On Tue, 29 Nov 2022 at 12:18, Valery Smyslov <[e-mail]> wrote:
> Hi Sean,
> 
> thank you for your review. Please, see inline.
> 
> > Reviewer: Sean Turner
> > Review result: Has Nits
> > 
> > Hi! Thanks for the well written draft. I really liked Appendix B that includes
> > the tried but discarded designs.
> 
> Thank you.
> 
> > Issue worth discussing (and it might be a short discussion):
> > 
> > Are there any instructions that the DEs needs to make sure that this registry
> > is not populated with PQ-wanna-be Transforms? E.g., I show up my shiny new (and
> > supposedly) PQ resistant alg and the DE says ....
> 
> I'm not sure the DEs have enough qualification to judge whether the proposed 
> algorithm is good or bad with its cryptographic properties. I believe it is the CFRG's task 
> to bless algorithms and the DEs should only pay attention to is whether 
> the proposed algorithm meets the protocol restrictions (and those are 
> listed in Section 4.1 for the DEs).
> 
> 
> I agree with Valery on this. Besides, the current draft is a generic one, it doesn't specify any specific algorithms. So I would expect that there will be specific documents specifying how to use a particular algorithm with this draft. That document will specify amongst other thing the wire format, key generation, encapsulation, decapsulation, etc. Once that document is approved, then only entries will be added into the registry.  
>  
> > Nits:
> > 
> > The use of “we” is a style thing that I would change, but if the WG/IESG are
> > good with it I can get on board too.
> 
> I'll rely on my co-authors on this :-)
> 
> Thanks, the use of "we" and "ours" have now been removed. The changes can be found in the same PR: https://github.com/post-quantum/ietf-pq-ikev2/pull/22 
>  
> 
> > s1.2, last para: “require such a requirement” is a bit awkward. How about “have
> > such a requirement” or “levy such a requirement”?
> 
> Changed to "have such a requirement".
> 
> > s2, hybrid: I think you might want to include some words by what you mean by
> > “hybrid”? Maybe as simple as copy some of the text from the 1st para of s3.1
> > forward, “when multiple key exchanges are performed and the calculated shared
> > key depends on all of them”.
> > 
> > s3.1, s/Note that with this semantics,/Note that with these semantics,
> 
> Fixed, thank you.
> 
> > s4.1:
> > 
> > s/must/MUST in the DE instructions?
> 
> Hm, I may be wrong, but in my understanding RFC2119 words have their meaning
> only in the context of an RFC/I-D (to which the DE instructions don't belong to)...
> 
> > s/addition,any/addition, any
> 
> Fixed.
> 
> > s5:
> > 
> > s/dwarfed/ with thwart or mitigate
> 
> Changed to mitigate.
> 
> > s/the data need to remain/the data needs to remain
> 
> Fixed.
> 
> > A.1:
> > 
> > s/as follows/as follows.
> 
> OK.
> 
> > s/SKEYSEED(1)  …. )./SKEYSEED(1) … )
> 
> Done.
> 
> > s/{SK_d(1) … SPIr)./{SK_d(1) … SPIr)
> 
> Ditto.
> 
> > Is this missing:
> > 
> >  The updated SKEYSEED value is then used to derive the following
> >  keying materials
> > 
> > between these two lines:
> > 
> >  SKEYSEED(2) = prf(SK_d(1), SK(2) | Ni | Nr)
> >  {SK_d(2) | SK_ai(2) | SK_ar(2) | SK_ei(2) | SK_er(2) | SK_pi(2) |
> >     SK_pr(2)} = prf+ (SKEYSEED(2), Ni | Nr | SPIi | SPIr)
> 
> Well, I think it must be clear enough from the formulas - 
> we first calculate new SKEYSEED (SKEYSEED(2)) and then
> use it to calculate new SK_* keys (SK_*(2)).
> We purposely added indexes in round braces to make it easier 
> for readers to figure out "generations" of the keys.
> Do you think it is not clear enough?
> 
> > A.4:s/a security association/an IKE SA
> 
> OK.
> 
> The changes can be reviewed in the PR:
> https://github.com/post-quantum/ietf-pq-ikev2/pull/22
> 
> Regards,
> Valery.
> 
> 
> 
> PQ Solutions Limited (trading as ‘Post-Quantum’) is a private limited company incorporated in England and Wales with registered number 06808505.
>  
> This email is meant only for the intended recipient. If you have received this email in error, any review, use, dissemination, distribution, or copying of this email is strictly prohibited. Please notify us immediately of the error by return email and please delete this message from your system. Thank you in advance for your cooperation.
> 
> For more information about Post-Quantum, please visit www.post-quantum.com.
> 
> In the course of our business relationship, we may collect, store and transfer information about you. Please see our privacy notice at www.post-quantum.com/privacy-policy/ to learn about how we use this information.