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

[Sean Turner <[e-mail]>]

> On Nov 29, 2022, at 07: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).

Valery you’re not giving yourself and Tero enough credit ;) But, you did say exactly what I hoped you would say, in that the CFRG is going to evaluate the alg. Note sure if this needs to be documented.

>> 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 :-)
> 
>> 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)...

Yeah that’s what the “?” was about. I think you’re right here that 2119 shouldn’t be applied.

>> 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?

My OCD was going off with the perious presentation included those words. If it was purposely dropped that’s okay.

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