Subject: [KEYPROV] WGLC: draft-ietf-keyprov-pskc-03.txt

From: "Hannes Tschofenig" <[email-masked]>
Date: Tue, 9 Jun 2009 21:59:24 +0300

Hi all, 

This is the 2nd Working Group Last Call for comments on "Portable Symmetric
Key Container (PSKC)":
http://www.ietf.org/internet-drafts/draft-ietf-keyprov-pskc-03.txt

Please have comments to the list by Sunday, 27 June.

As always, please remember to send a note in if you've read the document and
have no other comments other than "its ready to go" - we need those as much
as we need "I found a problem".

Ciao
Hannes & Phillip

---

From: <[email-masked]>
Date: Mon, 15 Jun 2009 01:22:36 -0400

Philip, etal:
 
Great work on the document!
 
Please find my comments below.
 
Items representing things that I think are "issues" are prepended with three asterisks ('***').  Note, however, that I think these are minor in nature.
 
Andrea
==============================================
Key:
   S - Section
   P - Paragraph
   F - Figure

Abstract: Insert a comma after "For example"
 
Introduction, P1: ", for example^^,^^ vendors"
 
S3, #6: "wether" is misspelled
 
S4, P2: when describing the example, would help to mention the use of PIN algorithm since PIN's might not automatically be thought of as symmetric keys.
 
*** S4, 'Id': The reference to multiple containers threw me.  According to our terminology discussion, and the  discussion on Bulk Provisioning in Section 8, PSKC only defines one Key Container.  Under what circumstances would "multiple key containers" be embedded in larger xml documents?
 
*** S4.1, 'Algorithm': [PSKC-ALGORITHM-PROFILES] is going to expire on June 27, 2009.
 
S4.1, last paragraph re <ValueMac>: This paragraph is difficult to understand.  Rather than "MUST be possible to carry" you are not inferring that <ValueMac> is mandatory, but the wording implies this.  <ValueMac> is optional.  Because <ValueMac> is defined in the schema, it must be possible to carry the value --- this doesn't need explicit mention here.  Would be clearer to just have <ValueMac> as another item in the list (after 'Encrypted:") and discuss it from there.
 
*** S4.2, sample schema, <UserID>: The <UserID> value is missing UID.  Under what situations would the Device have a different UserID than the Key that the device holds?  Does Device really refer to Issuer?
 
*** S4.2.1, <DeviceBinding>:  Rob Philpott suggested the following re-wording (I mentioned this re-wording in my comments on the previous version of the doc):
    "This element allows a provisioning server to ensure that the key is
    going to be loaded into the device for which the key provisioning
    request was approved. The device is bound to the request using a device
    identifier, e.g., an IMEI for the phone, or an identifier for a class of
    identifiers, e.g., those for which the keys are protected by a TPM."
 
S4.2.3, P3: "indicates XXthatXX the entity"
 
*** S4.2.3, P3: By "entity" what is being referenced?  Issuer?  The distinction between UserID associated with key and UserID associated with Device is unclear.
 
S4.3, P1: "sending and receiving partiesXX,XX might"
 
S4.3, P2: check wording for editorial problems
 
S4.3, P2: Would provide better context if you stated that this example is based on MasterCard CAP, e.g., "For example, ^^in the case of MasterCard CAP,^^
 
S4.3, last paragraph: The text after F4 mentioned an external key called "MasterKeyLabel", but this doesn't match the example, which instead uses <KeyProfileId> called "keyProfile1".
 
S5, <NumberOfTransactions>: "key carried within the PSKC document can be used" - clarify what "used" means.  I think it means the number of times the key may be used by an application (after the key was imported by the receiving party).  But, "used" could also mean the number of times the key can be imported.
 
S5, <Verify>: "(000 is the vice versa of Integrity)"
 
S5, <Prepend>: This description refers to "validation server".  Would be more consistent with the rest of the document to refer to this as the "receiving party".
 
S5, <Append>: Same comment as above.
 
S5, <MaxLength>: "lenght" --- misspelled
 
*** S6.2: Alignment issue with DSKPP.
 
   Note that according to DSKPP (-07, section 5.2.2), for Passphrase-Based Key Wrap, 
   "the <ds:KeyName> option MUST be used and the key name MUST identify the 
   passphrase that will be used by the server to generate the key wrapping key.  
   The identifier and passphrase components of <ds:KeyName> MUST be set to the 
   Client ID and authentication code components of AD (same AD as contained in this 
   message)."
 
   However, in S6.2 of this version of PSKC, <ds:KeyName> is not mentioned.
 
S6.3, last sentence: confusing sentence
 
S6.4: The wording is difficult to understand, as there are lots of misspellings, etc.
 
S10.1, Profiling: "meta-data" --- everywhere else in the doc "meta data" is used.
 
S11: Is the annotation regarding line break within SchemaLocation necessary?  The DSKPP schema has lots of line breaks, but no annotations included.
 
S12.5 and S12.6, "depreciate":  The word "deprecate" is typically used instead of "depreciate".
 
S13.2: "provides a mean^^s^^"
 
S15: "Veath"->"Vaeth"
 
S16.2: The [DSKPP] is out-of-date.  The latest version was released in Feb 2009 and was version -07.
 
A.1.3: "floppy" - I don't think floppy disks are used any more.
 
A.1.4: "Over-The-Aire" -> "Over-The-Air" (Aire is a river in N England)

________________________________

From: [email-masked] on behalf of Hannes Tschofenig
Sent: Tue 6/9/2009 2:59 PM
To: KEYPROV
Subject: [KEYPROV] WGLC: draft-ietf-keyprov-pskc-03.txt

Hi all,

Ciao
Hannes & Phillip

_______________________________________________
KEYPROV mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/keyprov

---

From: "Hannes Tschofenig" <[email-masked]>
Date: Tue, 16 Jun 2009 20:01:54 +0300

PLEASE read the document!

>-----Original Message-----
>From: [email-masked] 
>[mailto:[email-masked]] On Behalf Of Hannes Tschofenig
>Sent: 09 June, 2009 21:59
>To: KEYPROV
>This is the 2nd Working Group Last Call for comments on 
>"Portable Symmetric Key Container (PSKC)":
>As always, please remember to send a note in if you've read 
>the document and have no other comments other than "its ready 
>to go" - we need those as much as we need "I found a problem".
>
>Ciao
>Hannes & Phillip
>
>
>
>

---

From: Sean Turner <[email-masked]>
Date: Sun, 21 Jun 2009 20:30:40 -0400

Here are my comments.  I'd say these were all nits and that the document 
is ready to proceed.  I hope to not duplicate Andrea's comments.

Abstract: r/The standard/A standard

Intro: r/of a IANA/of an IANA

Sec 1.3: r/semantics../semantics.

Sec 3: r/upmost/at most?

Sec 4.1, 'Algorithm': r/[PSKC-ALGORITHM-PROFILES]/[PSKC-ALGORITHM-PROFILES].

Sec 4: r/This attribute is mandatory./This attribute MUST be included.

Sec 4/4.1: (I waffled on whether or not to include this as a comment) 
Should the values used in Figure 2 be referenced in 4/4.1?  e.g., 'Id:' 
  The 'Id' attribute carries a unique identifier for the container.  For 
this example, the value is "exampleID1".  It ties the examples to the 
text, and might help some readers.

Sec 4.1, <Time>: r/dependent)/dependent).

Sec 4.1, Encrypted: r/value/value.

Sec 4.2.2: r/ token)/ token).

Sec 4.2.2: r/has a single mandatory child element/has a single child 
element that MUST be included:

Sec 4.2.3: r/,DC=net/,DC=net.

Sec 4.2.3: r/Although the syntax of the user identifier is defined there 
/Although the syntax of the user identifier is defined, there

Sec 4.2.4 (x2): r/'CheckDigit':  This optional attribute 
indicates/'CheckDigit':  This attribute indicates

Sec 4.2.4: Mandatory isn't an RFC 2119 keyword.  I think we need to 
change where ever 'mandatory' is to work in a MUST.

r/'Encoding':  This mandatory attribute defines/'Encoding':  This 
attribute, which MUST be included in <ChallengeFormat>, defines

r/'Min':  This mandatory attribute defines/'Min':  This attribute, which 
MUST be included in <ChallengeFormat>, defines

r/'Max':  This mandatory attribute defines/'Max':  This attribute, which 
MUST be included in <ChallengeFormat>, defines

r/'Encoding':  This mandatory attribute defines/'Encoding':  This 
attribute, which MUST be included in <ResponseFormat>, defines

r/'Min':  This mandatory attribute defines/'Min':  This attribute, which 
MUST be included in <ResponseFormat>, defines

r/'Max':  This mandatory attribute defines/'Max':  This attribute, which 
MUST be included in <ResponseFormat>, defines

Sec 4.3 (2nd para): r/sending and receiving party/sending and receiving 
parties

Fig 3/5: Should <UserId> be in Fig 3.  It's not in Fig 5?

Sec 6.1: r/Since AES128-CBC does not provide integrity checks a keyed 
MAC/As AES128-CBC does not provide integrity checks, a keyed MAC

Sec 6.1: r/is use as the algorithm/is used as the algorithm

Fig 6: r/ Figure 6: AES-128-CBC Encrypted Pre-Shared Secret Key/Figure 
6: AES-128-CBC Encrypted Pre-Shared Secret Key with SHA1 as HMAC

Sec 6.1: Is there a reason:

HMAC-SHA224    | http://www.w3.org/2001/04/xmldsig-more#hmac-sha224

wasn't included?

Sec 6.3 (last para): r/asymmetric key encryption algorithm/asymmetric 
key encryption algorithm.

Sec 9 (Algorithm URIs): r/The usage of new/The use of new

Sec 9 (Policy): r/the usage of the key/the use of the key

spt