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

[Valery Smyslov <[e-mail]>]

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

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

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