Jim:
1. AuthenticatedData cannot be processed in a single pass for both creation
and writting. The ASN must be changed to
AuthenticatedData ::= SEQUENCE {
version CMSVersion,
originatorInfo [0] IMPLICIT OriginatorInfo OPTIONAL,
recipientInfos RecipientInfos,
macAlgorithm MessageAuthenticationCodeAlgorithm,
digestAlgorithm [1] DigestAlgorithm OPTIONAL,
encapContentInfo EncapsulatedContentInfo,
authenctiatedAttributes [2] IMPLICIT AuthAttributes
OPTIONAL,
mac MessageAuthencticationCode,
unauthenticatedAttributes [3] IMPLICIT UnauthAttributes
OPTIONAL
}
With the current layout, while the verification can be done in one pass the
creation requires two passes. The required steps are 1) compute the hash on
the data, 2) create the authenticated attributes and write, 3) write the
content, 4) compute and write the mac. This means that we had to process
the content twice.
You are correct. The digest algorithm is needed before the content, and the
authenitcated attributes should follow the content. I had bundled the digest
algorithm and authenitcated attributes together since they are related. I can
handle this in words.
2. Section 2.0 Paragraph 3: Should read "each content type permits..." the
s is missing on permits
Agree.
3. Section 3.0 Paragraph "conent is ..." please add "data" to the list of
defined content types defined in the document
Agree.
4. Section 4.0 Last Paragraph: Please delete the last sentence in this
paragraph. I don't understand what is going on in this paragraph. We have
obously defined how to do the encapsulated content in this document (such as
signed data in signed data), we just have not defined any other base content
types
Agree. This was intended to allow arbitrary content types to be encapsulated.
I agree that it does not belong in this section. Does the thought belong
somewhere else?
Also, I added the Data ASN.1 definition:
The data content type shall have ASN.1 type Data:
Data ::= OCTET STRING
5. Section 2.0 Paragraph 1. Change "ContentInfo encapsulates one or more
content types." to "ContentInfo encapsulates an identified content type."
There is no sequence so the one or more is misleading.
I agree with your point. How about:
ContentInfo encapsulates a single identified content type, and
the identified type may provide further encapsulation.
6. Section 3.0 Should we change the title on this section from General
Syntax to Content Info? Since we are no longer restricting the exported
types to ContentInfo then there is no longer a single general syntax
anymore.
This is the title used by PKCS#7 v1.5. My hope is to make it easier for the
reader who is concerned about backward compatibility.
What do other people think about this one?
7. Section 6.1 - I must have missed this one coming through the list.
While I have no objects to including the attribute sequence, I strongly
suggest that the name/type be change to UnprotectedAttributes. Plaintext
originally confused me as a new type of textual attributes.
Unprotected is fine.
8. Section 9.2: Paragraph 2; Please change to "If authAttributeInfo is
absent ..." or add the word "the"
This has to be reworked to implement your first change. The authAttributeInfo
structure is being broken up.
9. Section 9.2: Was there a reason for removing the "rather than the
implcit [0] .." phrase. This exists in the process for signed data and I
think should be here as well.
For authenticated-data, digests are only computed on the content. They are
never computed on the attributes. The IMPLICIT [0] stuff was about the
attribute encoding.
10. Global -- You have some inconsitant labeling Some references are
bracked and some are not. Please go through the document and choose one
method or the other but be consistant. (Examples are 11.0 uses brackets but
10.2.2 does not.)
I changed 10.2.2. I did not see any others like that one.
11. Section 11..2 Para 1 Last sentence. This is a tautalogy. I think you
meant to use the phrase "the authenctor's message ditest algorith."
Agree. It shoudl be the originator's algorithm.
12. Section 11.2 Para 3. Missing "an" before unauthencticate attribute.
Agree.
13. Section 12.3.1.1 Para "keyEncryptionAlgrothm must ..." I have a problem
with the phase a "and contain a", the first 6 or 7 time I read this I
assumed it was a field in the parameters rather than being the parameter
itself. Writing as "The algorihtm identifier parameter field for
id-alg-ESDH is KeyWrapAlgorihtm and must be present." avoids this issue.
Okay. How about:
The algorihtm identifier parameter field for id-alg-ESDH
is KeyWrapAlgorihtm, and this parameter must be present.
14. Section 12.3.3 Last paragarph. What is this paragraph doing here? It
is talking about key agreement in the symetric key -encryption section.
The output of a key agreement algorithm is a key-encryption key, and this
key-encryption key is used to encrypt the content-encryption key. The last
paragraph is a backward pointer telling folks that the same techniques are used
regardless of the source of the key-encryption key. I will add an sentence to
the front of this paragraph that hopefully makes this more clear.
15. Section 12.6.2 - You have not modified the key wrap algorithm to allow
for arbitrary length RC2 key sources.
Are you suggesting an explicit length field or something else?
16. Section 12.3.1.1 - In the KeyWrapAlgorithm is the IV present and is its
value the constant 'A5' repeated n time? The IV was not present in the
previous versions but would appear to be present now. We still have the IV
restriction on the key wrap algorithm however.
There is no field needed to carry the IV as it is always "A5...A5". For 3DES,
the parameter is always NULL, and for RC2 the parameter is the effective key
length. Where do you see an IV?
Russ