Subject: [Gen-art] Gen-art review of draft-ietf-smime-multisig-04.txt

From: Elwyn Davies <[email-masked]>
Date: Fri, 07 Mar 2008 18:11:19 +0000

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
_http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html_).

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-smime-multisig-04.txt
Reviewer: Elwyn Davies
Review Date: 7 March 2008
IETF LC End Date: 7 March 2008
IESG Telechat date: (if known)

Summary:
Mostly fine except for a piece of unclear specification noted below and 
a few editorial nits.
Caveat: I am not a security expert and this should not be taken as an 
endorsement of the security competence of the proposal.

Comments:
s3:  The first part of the specification for MultipleSignatures is :

>    The fields in MultipleSignatures have the following meaning:
>
>      - bodyHashAlg includes the digest algorithmIdentifier for the
>      referenced multiple-signatures attribute.
>
>      - signAlg includes the signature algorithmIdentifier for the
>      referenced multiple-signatures attribute.
>
I am confused by the use of 'includes' here: Do these specs imply that 
the values of these fields are comma separated lists of all relevant alg 
identifiers for the signatures?  An example with three signatures might 
clarify what is going on, but the spec should be clarified in any case, 
I think (but I may just not be sufficiently knowledgable about this sort 
of spec).

Editorial:
idnits reports a clean bill of health.

Abstract: Expand CMS acronym.

s5: s/in a singled/in a single/

s5.2: s/the rquire application/the required application/

s5.3, para 5: The first sentence
>
> If signatures are added for the support of [ESS] features, then the
>    fact that an outer layer signature can be treated as a non-
>    significant failure.
>
does not parse.  Probably missing 'is invalid' or some such relating to 
outer layer signature.

Appendix B: 'hashes CMS'??? Does not parse!

B.1: s/is needed/are needed/

B.2 1/a/ii: s/Reistance/Resistance/

B.2 1/c/iii: s/success/successful/

B.2 2: Expand DER acronym.

B.2: is not normative but uses SHOULD NOT.

B.2 (2nd para on p18): s/that the attack/than the attack/

_______________________________________________
Gen-art mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/gen-art

---

From: "Turner, Sean P." <[email-masked]>
Date: Mon, 10 Mar 2008 10:58:16 -0400

Elwyn,

Thanks for the review. Responses inline... 

spt

>-----Original Message-----
>Comments:
>I am confused by the use of 'includes' here: Do these specs 
>imply that the values of these fields are comma separated 
>lists of all relevant alg identifiers for the signatures?  An 
>example with three signatures might clarify what is going on, 
>but the spec should be clarified in any case, I think (but I 
>may just not be sufficiently knowledgable about this sort of spec).

The attribute is multivalued (discussed before the ASN.1) so there is a set
of values for each signature applied. The reason for only using two in the
example was purely based on page real estate.

>Abstract: Expand CMS acronym.

fixed

>s5: s/in a singled/in a single/

fixed

>s5.2: s/the rquire application/the required application/

fixed

>does not parse.  Probably missing 'is invalid' or some such 
>relating to outer layer signature.

fixed

>Appendix B: 'hashes CMS'??? Does not parse!

fixed (reword)

>B.1: s/is needed/are needed/

fixed (reword)

>B.2 1/a/ii: s/Reistance/Resistance/

fixed

>B.2 1/c/iii: s/success/successful/

fixed

>B.2 2: Expand DER acronym.

fixed

>B.2: is not normative but uses SHOULD NOT.

fixed

>B.2 (2nd para on p18): s/that the attack/than the attack/

fixed