[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[KEYPROV] comments on dskpp-05



The following are my comments on dskpp-05.  I would categorize all of these
as editorial/consistency comments/questions. In the comments r = replace and
suggested new text is provided (r/old text/new text). Since I didn't do an
exhaustive search in the archives, if I re-open something that was closed in
the past then "been there done that (BTDT)," in my mind, is an appropriate
response.

0. Wow - this is way easier to read and understand!

1. (wordsmithing): r/variations/variants and r/variation/variant?

2. Introduction, 3rd para: r/RFC4785/[RFC4785]

3. 1.1.7, 2nd para: r/last/previous

4. 1.5.2: Should we add a blurb about the XMLENC namespace? <xmlenc:> is
used right?

5. 2.2: Add "Mutually Authenticated Key Agreement", "Key Agreement", and
"Key Confirmation".  These terms are used a lot and ought to be included.

6. 2.3 DSKPP-PRF: In 2.3 DSKPP-PRF has (k,x,l) as parameters in 3.5/C.2.2 it
has (k,s,dsLen). I assume you meant to use what's in 3.5/C.2.2 in 2.3.

7. 1.3/1.4/2.2/elsewhere: Para 1 describes protocol "exchange" and "pass"
but not "protocol run". To be clear we should add a definition in para 1 or
2.2 for protocol run. Suggested def: protocol run is either exchange
(2-pass) or two exchanges (4-pass).

8. 3.1.1 and elsewhere: r/[RFC3280]/[RFC5280]

9. 3.1.1: K_SERVER is described in 2.2 (and elsewhere) as a public key, but
3.1.1 says if K = K_SERVER then the server's certificate SHOULD be verified.
Is it a "should" because it might not be certificate? If it's always a
certificate should we say that?

10. 3.1.2.2: r/a secret random value R_C/a secret random value k, R_C, -
Just to be sure K isn't confused with k.

11. 3.1.3: r/parameter K/parameter k - should refer to the parameter?

12. 3.1.4.1: r/and K MUST/and k MUST - should refer to the parameter?

13. 3.2.1: r/the list of Key Protection Methods (KPML) it supports/its Key
Protection Method List (KPML)

14. 3.1.1/3.2.1: r:/terminate the DSKPP session/terminate the DSKPP protocol
run. Is session meant to imply more than a run?

15. 3.1.1/3.2.1: When the status is not continue and the client aborts the
protocol, should it need to delete any nonces, keys, and/or secrets
associated with the failed run?

16. 3.1.1/3.2.1: When the server aborts because authentication fails, should
it need to delete any nonces, keys, and/or secrets associated with the
failed run?

17. 3.2.1: r/Section 1.1.1/Section 1.1.1) - need a ")" somewhere after
"(Alternatively, ..."

18. 3.2/3.2.1: last sentence of 3.2 says PSKC/SKPC fit the bill and then
later PKCS12/PKCS5 get added, I'd say add PKCS12/5 to 3.2 for consistency.

19. 3.2.2.1/3.2.2.2/3.2.2.3: (total of six times) r/type
xenc:EncryptedKeyType/type <xenc:EncryptedKeyType> 

20. 3.2.2.1/3.2.2.2/3.2.2.3: The following appears three times: "In the
2-pass version of DSKPP, the client MUST send a payload associated with this
key protection method." I think you're trying to say when you do this one,
you MUST send a payload with this KPM. Without a change it sounds like I
have to send all 3 when I do 2-pass.

21. 3.1.4/3.2.3: does it matter that for server authentication the string in
3.1.4 is "MAC 1 computation" and in 3.2.3.1 it's "MAX 2 computation"?
Likewise for key confirmation the string in 3.1.4.2 is  "MAC 1 computation"
and in 3.2.3.2 it's "MAC 2 computation"?

22. 3.1.4.1/3.1.1/Figure 3: R, which is sent by the client, isn't always
R_TRIGGER (right?) - shouldn't there be an [R] in the figure in 3.1.1 that
shows SAL, [R_TRIGGER], [DeviceID], [KeyID]? Otherwise how does [R] get from
the client to the server? It's also not shown in Figure 3 (not sure it needs
to be in this one though).

23.  3.2.1/3.2.3.2: R isn't R_C or R_TRIGGER (right?) so shouldn't [R] be
added to the figure in 3.2.1 that shows R_C, SAL, KPML, [AD], [R_TRIGGER],
[DeviceID], [KeyID] being sent to the server?

24. 3.1.1./3.2.1: Should add a sentence to explain the [URL_S]: The server
MAY also include a URL to its location.

25. 3.4.2: So what gets sent isn't bits it's the int/hex? Just confused by
the bit map. Are the {s and ,s also sent as per 3.4.2.1, 3.4.2.2, and
3.4.2.3?

26. 4.3, <Extensions>: r/A sequence of extensions/A sequence of OPTIONAL
extensions.

27. 4.3, last para: Should we add a paragraphs that explains
ClientNonce/TriggerNonce and
SupportKeyTypes/SupportEncrytpionAlgorithms/SupportMACAlgorirthms?

28. 4.1/4.3.2: 4.1 says DSKPP MUST NOT depend on specific sorting order for
values but 4.3.2 <SupportedKeyProtectionMethod> says methods MUST be listed
in order of precedence. Is this a problem?

29. 4.5: Add period to end of 1st para.

30. 4.5: r/<AuthenticationData>: IThe/<AuthenticationData>: The

31. 4.5: Add period to end of <Extensions> bullet.

32. 8: r/DSKPP clients need to support either the two-pass or the four-pass
variant of the protocol/DSKPP clients MUST support either the two-pass or
the four-pass variant of the protocol

33. 9.3.3: r/<DSKPPTrigger>/<KeyProvTrigger>

34. 9.3: Also add to the list that any value present in an <extension>
that's part of <KeyProvClientHello> is also exposed.  This will make sure
that folks that define their own know the values placed are not normally
protected.

35. 9.3: If an attacker send a <KeyProvTrigger> won't they also learn the
values listed in 9.3? I guess my point it's not just a passive attack.

36. 9.4: We should add something like the following (from RFC 3370):
Implementers should be aware that cryptographic algorithms become weaker
with time.  As new cryptanalysis techniques are developed and computing
performance improves, the work factor to break a particular cryptographic
algorithm will reduce.  Therefore, cryptographic algorithm implementations
should be modular allowing new algorithms to be readily inserted.  That is,
implementers should be prepared to regularly update the set of algorithms in
their implementations.

37. 9.6.1: r/to K_TOKEN ,/to K_TOKEN,

38. 9.6.4: Sometimes it's "AuthenticationCode" sometimes it's
"Authentication Code". Same for Authentication Data.

39. 12: Should "SecureId" be added to this list?

40. 15.2: If you MUST support AES-CBC-128 as defined in [FIPS197-AES]
(likewise for [PKCS-1], [FIPS180-SHA], [FIPS197-AES], [PSKC], and [RFC2104]
references) isn't it a normative reference?

41. Appendices: I think the appendices should clearly state whether they are
informative or normative.  Appendix A is clearly an informative because it's
examples.  I think Appendix B is also informative because of the MAY but it
might be clearer to add a 1st sentence saying it's informative? Appendix C
is normative, according to 8, so I think we should add "This Appendix forms
a normative part of the document."  Only confusing because the Appendix is
titled "Example of DSKPP-PRF Realizations."

42. C.2.1: Is there another way to specify the DSKPP-PRF-AES and
DSKPP-PRF-SHA256 algorithms?  I think the MAYs in the first sentences should
be a MUSTs.

43. C.2.1/C.2.2: r/When this URL is used to identify the encryption
algorithm to use,/When this URL is used to identify the encryption
algorithm,

_______________________________________________
KEYPROV mailing list
KEYPROV at ietf.org
https://www.ietf.org/mailman/listinfo/keyprov