Subject: RE: WG Last Call: draft-ietf-smime-3278bis-03

From: "Turner, Sean P." <[email-masked]>
Date: Thu, 6 Nov 2008 08:07:39 -0500

The WG LC closes today.  Below is a list of comments I've received so far.
-------
Section 3.1 (E-S ECDH), in the third paragraph, it states that the
originatorInfo field MAY include the certificate(s) for the EC public key(s)
used in the formation of the pairwise key.  The originatorInfo field only
contain certs and CRLs related to the originator (and not the recipient).
With ephemeral-static, the originaltor does not have a certified public key.

spt> I deleted the last two sentences from the paragraph.
	
Section 3.1.1 says that the permitted digest algorithms for use with ECDH
were expanded to SHA-1, SHA-224  etc. Note that this is not described in
3.1.1 (which deals with enveloped data).  Also, "NULL to absent or ECPoint"
should be "NULL to absent or ECParameters". 

spt> I moved the discussion about the algorithm changes to the bullet
describing the Section 5 changes.  That line should refer to the hash
algorithm used in the KDF and then it would apply to EnvelopedData.
	
In that same area (comments for section 3.1.1) the last word is ECPoint.
Shouldn't this be ECParameters? 

spt> Yes in Section 1.2 the bullet for 3.1.1 should refer to ECParameters
not ECPoint.

Section 3.1.1 the section on keyEncryption Algorithm says that it must
contain the "key encryption algorithm" object identifier (see section 7.1).
Section 7.1 does not include anything with that designator.

spt> This actually made me go and make sure all the pointers to 7.1 to make
sure they pointed to the new sub-sections.  It should point to 7.1.4 which
is the key agreement information section.  I change the 1st sentence to say
"keyEncryptionAlgorithm MUST contain the key encryption algorithm, which in
this case is a key agreement algorithm, object identifier (see Section
7.1.4)."  I also added a pointer to 7.1.5 in the sentence that describes the
KeyWrapAlgorithm field.

Section 3.1.2 at the end "a shared secret bit string 'K' which is used as
the pairwise key-encryption key"  is misleading.  The shared secret is not
the KEK.  The shared secret is sent with other info through a KDF to form
the KEK.  This sentence is replicated in other places in this draft, for
example, Section 3.1.3, section 3.2.2, 3.2.3.

spt> The key deployment and key agreement operations result in the ephemeral
and shared secret.  The "Z", which is used to derive the "K", is generated
during the key agreement operation.  In NIST SP800-56A it's Section 6.2.2
Step 4 (same is true of SEC1 process).

Section 3.2.2 - last paragraph talking about multiple layers.  You have one
message with one recipient and nested layers (eg, encrypted signed encrypted
message).  This says that you can use the same originator ephemeral key for
all of these nested layers?  If so, don't you have to ensure that an
addedukm field is added at each layer and they are different or does it not
matter if  you have the same KEK at each layer?  Further, NIST SP800-56A
says "Each call to the KDF requires a freshly computed shared secret, and
this shared secret shall be zeroized imediately following its use".  So
doesn't that mean you need a new ephmeral for each layer?

spt> I'll address this in a seperate email.

Section 3.2.3 "....checks that the domain parameters are the same as the
recipient's domain parameters"  Should recipient be originator instead?

spt> I modified the sentence as follows so it's clear the recipient is check
that the originator's and recipient's domain parameters are the same: "The
receiving agent then retrieves the static and ephemeral EC public keys of
the originator, from the originator and ukm fields as described in Section
3.2.1, and its static EC public key identified in the rid field and checks
that the ***originator's*** domain parameters are the same as the
recipient's domain parameters."

Section 4.2 "originatorInfo field MAY include the cert(s) for the EC public
key(s)"  If originatorInfo only applies to the originator and this is C(1,2,
ECC MQV) then don't we have just one static key (ie, you would state EC
public key -- not plural)

spt> I'll remove the "s".

Section 5:  "The 1st paragraph was modified to require both SignedData...."
Should this be recommend?  Also, this comment is for Section 6 not section
5. 

spt> I changed it to recommended and I merge the two bullets.

Section 6:  talks about ecdsa-with-SHA* having NULL parameters (to be
consistent with RFC3278, section 7).  However, this seems to contradict
3851bis, section 2.5.2 , last paragraph.  Same thing in the Note in section
6.

spt> Yes I ought to change these. I changed the note so that it only applies
to SHA-1 and changed NULL to absent in the two ASN.1 modules.

Comments for Section 7.1   Should you include "key wrap" in the part about
adding subsections? 

spt> Added it in Section 1.2.

Section 7.2 last para listing the KDF in SP800-56A.  This KDF is
dramatically different from the one in RFC3278.  The "otherinfo" in
SP800-56A is much more complex and I don't know what values the AlgorithmId,
PartyUInfo, PartyVInfo would take by reading this draft.  Also, this one
does hash(counter || Z || otherinfo); the old kdf did hash(Z || counter ||
otherinfo)

spt> I will address this is in a separate email.

Reference: FIPS 180-3 is no longer a draft  - it's official as of October
2008. 

spt> Fixed.

Section E.8 of X9.62 says that CMS SignedData represents signatures as a bit
string.  Section 2.1.1 says "signature MUST contain the DER encoding (as an
octet string) "  These two are in conflict, but I think that Section 2.1.1
is correct.

spt> Glad we got it right ;) Any chance somebody can take this to ANSI?

TYPOS
	
Section 1.2   change FIPP to FIPS 
Section 1.2  -- Section 5. Need an "in" in this sentence:  "...to be present
in certificates". 
Section 3 :  need a "to" in "security due to the originator's" 
section 7.1.2: last para.  need a " ' " in recipient's ECParameters. 
section 7.1.3: need to make field plural in first sentence section 7.2:
should there be a "." in ephemeralPublicKey.publicKey  (wasn't sure) section
8: need to change RECOMMEND to RECOMMENDED Section 9: need to make random
number plural 
	
spt> all fixed except 7.2 where we left the period.  A subfield of
originatorPublicKey is publicKey and that's where the key goes.