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

From: "Philip Hoyer" <[email-masked]>
Date: Fri, 28 Aug 2009 15:59:10 +0200

Andrea,
Thanks so much for the review.
Please see my comments and what I have changed for final submission:

Philip

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On
Behalf Of [email-masked]
Sent: Monday, June 15, 2009 6:23 AM
To: [email-masked]; [email-masked]
Subject: Re: [KEYPROV] WGLC: draft-ietf-keyprov-pskc-03.txt

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"
[PH] changed
 
Introduction, P1: ", for example^^,^^ vendors"
[PH] changed 

S3, #6: "wether" is misspelled
[PH] could not find this, must have been corrected earlier 

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.
[PH] Changed the example to use HOTP algorithm
 
*** 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?
[PH] this was deemed good practice for XML so maybe if it is not DSKPP
but other larger xml documents the potentially could contain multiple
PSKC containers and hence having an Id would be very helpful to refer to
them
 
*** S4.1, 'Algorithm': [PSKC-ALGORITHM-PROFILES] is going to expire on
June 27, 2009.
[PH] I need to update the spec and update, I know.... not enough time,
too much to do.....
 
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.
[PH]: changed
 
*** 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?
[PH] can't follow UID. It seems to be ok in my version. The UID for a
key could be a group of people (sort of a role certificate). 
 
*** 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."
[PH] changed
 
S4.2.3, P3: "indicates XXthatXX the entity"
[PH] changed
 
*** 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.
[PH] I changed this to 'it indicates the user the device is associated
with.'
 
S4.3, P1: "sending and receiving partiesXX,XX might"
[PH] changed
 
S4.3, P2: check wording for editorial problems
[PH] slightly changed wording
 
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,^^
[PH] changed
 
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".
[PH] good spot, changed the example and description
 
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.
[PH] changed
 
S5, <Verify>: "(000 is the vice versa of Integrity)"
[PH] changed
 
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".
[PH] changed but not used receiving party since in this case it is the
party that validates the algorithm response
 
S5, <Append>: Same comment as above.
[PH] changed
 
S5, <MaxLength>: "lenght" --- misspelled
[PH] changed
 
*** 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.

[PH] not sure what I should add here could Ming help?
 
S6.3, last sentence: confusing sentence
[PH] changed

S6.4: The wording is difficult to understand, as there are lots of
misspellings, etc.
[PH] completely changed
 
S10.1, Profiling: "meta-data" --- everywhere else in the doc "meta data"
is used.
[PH] changed
 
S11: Is the annotation regarding line break within SchemaLocation
necessary?  The DSKPP schema has lots of line breaks, but no annotations
included.
[PH] removed

S12.5 and S12.6, "depreciate":  The word "deprecate" is typically used
instead of "depreciate".
[PH] changed
 
S13.2: "provides a mean^^s^^"
[PH] already changed
 
S15: "Veath"->"Vaeth"
[PH] D'oh, changed
 
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.
[PH] sadly yes
 
A.1.4: "Over-The-Aire" -> "Over-The-Air" (Aire is a river in N England)
[PH] changed

________________________________

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,

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

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

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

---

From: "Philip Hoyer" <[email-masked]>
Date: Fri, 28 Aug 2009 16:29:31 +0200

Sean,
Thanks for the reviewed please find below comments on what I have
changed for final submission.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On
Behalf Of Sean Turner
Sent: Monday, June 22, 2009 1:31 AM
To: KEYPROV
Subject: Re: [KEYPROV] WGLC: draft-ietf-keyprov-pskc-03.txt

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
{PH] changed

Intro: r/of a IANA/of an IANA
[PH] changed

Sec 1.3: r/semantics../semantics.
[PH] changed

Sec 3: r/upmost/at most?
[PH] changed

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

Sec 4: r/This attribute is mandatory./This attribute MUST be included.
[PH] changed

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.
[PH] sorry left it as is

Sec 4.1, <Time>: r/dependent)/dependent).
[PH] changed

Sec 4.1, Encrypted: r/value/value.
[PH] changed

Sec 4.2.2: r/ token)/ token).
[PH] changed

Sec 4.2.2: r/has a single mandatory child element/has a single child 
element that MUST be included:
[PH] changed

Sec 4.2.3: r/,DC=net/,DC=net.
[PH] changed

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
[PH] changed

Sec 4.2.4 (x2): r/'CheckDigit':  This optional attribute 
indicates/'CheckDigit':  This attribute indicates
[PH][ changed

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.
[PH] changed

r/'Encoding':  This mandatory attribute defines/'Encoding':  This 
attribute, which MUST be included in <ChallengeFormat>, defines
[PH] changed slightly differently

r/'Min':  This mandatory attribute defines/'Min':  This attribute, which

MUST be included in <ChallengeFormat>, defines
[PH] changed slightly differently

r/'Max':  This mandatory attribute defines/'Max':  This attribute, which

MUST be included in <ChallengeFormat>, defines
[PH] changed slightly differently

r/'Encoding':  This mandatory attribute defines/'Encoding':  This 
attribute, which MUST be included in <ResponseFormat>, defines
[PH] changed slightly differently

r/'Min':  This mandatory attribute defines/'Min':  This attribute, which

MUST be included in <ResponseFormat>, defines
[PH] changed slightly differently

r/'Max':  This mandatory attribute defines/'Max':  This attribute, which

MUST be included in <ResponseFormat>, defines
[PH] changed slightly differently

Sec 4.3 (2nd para): r/sending and receiving party/sending and receiving 
Parties
[PH] changed

Fig 3/5: Should <UserId> be in Fig 3.  It's not in Fig 5?
[PH] yes has been added

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
[PH] changed

Sec 6.1: r/is use as the algorithm/is used as the algorithm
[PH] changed

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
[PH] changed to ' AES-128-CBC Encrypted Pre-Shared Secret Key with
HMAC-SHA1'

Sec 6.1: Is there a reason:

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

wasn't included?
[PH] added

Sec 6.3 (last para): r/asymmetric key encryption algorithm/asymmetric 
key encryption algorithm.
[PH] changed completely

Sec 9 (Algorithm URIs): r/The usage of new/The use of new
[PH] changed

Sec 9 (Policy): r/the usage of the key/the use of the key
[PH] changed

spt

Symmetric
> As always, please remember to send a note in if you've read the
document and
much