RE: Gen-ART Review of draft-ietf-smime-sha2-03.txt

2008-03-01 Thread Turner, Sean P.
Spencer,

Thanks for taking the time to read the draft. Responses are inline.

spt 

>-Original Message-
>From: Spencer Dawkins [mailto:[EMAIL PROTECTED] 
>Sent: Friday, February 29, 2008 12:27 AM
>To: General Area Review Team
>Cc: Sean Turner; Blake Ramsdell; ietf@ietf.org; [EMAIL PROTECTED]
>Subject: Gen-ART Review of draft-ietf-smime-sha2-03.txt
>
>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.

Will do.

>Document: draft-ietf-smime-sha2-03.txt
>Reviewer: Spencer Dawkins
>Review Date: 2008 02 28
>IETF LC End Date: 2008-03-07
>IESG Telechat date: (if known)
>
>Summary: This document isn't ready for publication as a 
>Proposed Standard - it's got enough cut-and-paste errors and 
>apparently-omitted text that I'd think twice about trying to 
>implement it. And if it has a note that says "normative 
>reference still in progress, should be brought in line with 
>normative reference before publishing as an RFC", I'm not sure 
>why it's being last called now (of course, that decision is 
>above my pay grade).

Wrt the reference, that draft is being worked in PKIX. Hopefully, it will
progress quickly - I'm hoping for this summer (or sooner) for it to complete
WG LC and IETF LC.  Also, the reference is for object identifiers all of
which were assigned years ago and are stable.

>Comments:
>
>Please include any nits listed in
>http://www.ietf.org/mail-archive/web/ietf/current/msg50518.html
> that I may have missed in this review, by reference :-(

Will do.

>When one of the last call comments is "There are obvious 
>errors (intentionnaly left by the editor in order to know how 
>many people read the document)", this does not inspire 
>confidence. I note there is no shepherd writeup in the tracker 
>yet. The "of for" typo below has been in every version since 
>00. Who else has read this draft?

This was, I believe, his attempt at humor. See my response to Denis' email.

>Abstract
>
>   This document describes the conventions for using the message digest
>   algorithms SHA-224, SHA-256, SHA-384, SHA-512, as defined in FIPS
>
>Nit - I'm not sure what passes for normal in security drafts, 
>but I'd expect to see SHA and FIPS expanded on first use in 
>the abstract, and in the introduction of the document. Ditto 
>for DSA, RSA, and ECDSA.

I will expand the acronyms.

>   180-3, with the Cryptographic Message Syntax (CMS). It also 
>describes
>   the conventions for using these algorithms with CMS and the 
>DSA, RSA,
>   and ECDSA signature algorithms.
>
>Conventions used in this document
>
>Nit - it is odd to see this section as part of the abstract...

For some reason the tool picks up this section as part of the abstract. It's
got a heading title so I don't think it's in the abstract.

>1. Introduction
>
>   This document specifies the algorithm identifiers and specifies
>   parameters for the message digest algorithms SHA-224, SHA-256, SHA-
>   384, and SHA-512 for use with the Cryptographic Message Syntax (CMS)
>   [RFC3852].  The message digest algorithms are defined in and [SHS].
>
>Concern: "in and" seems to be missing something.

Denis caught this too. Will fix by removing the "and".

>   If an implementation chooses to support one of the algorithms
>   discussed in this document, then the implementation MUST do so as
>   described in this document.
>
>Concern: this MUST (and the parallel MUST in the next 
>paragraph) seem odd - do you need to say this?

Hmmm ... I guess not.  I'll remove both sentences.

>   Note that [RFC4231] specifies the conventions for use of for the
>
>Concern: "of for" seems to be missing something.

I will remove "for use of" so the sentence will read: Note that [RFC4231]
specifies the conventions for the message authentication code (MAC)
algorithms 

>   message authentication code (MAC) algorithms: HMAC with 
>SHA-224, HMAC
>   with SHA-256, HMAC with SHA-384, and HMAC with SHA-512.
>
>2. Message Digest Algorithms
>
>   The following addresses the parameters field:
>
>Nit - this sentence isn't clear and isn't required. I'd strike it.

Removed.

>   There are two possible encodings for the SHA AlgorithmIdentifier
>   parameters field.  The two alternatives arise from the fact 
>that when
>   the 1988 syntax for AlgorithmIdentifier was translated into the 1997
>   syntax, the OPTIONAL associated with the AlgorithmIdentifier
>   parameters got lost.  Later the OPTIONAL was recovered via a defect
>   report, but by then many people thought that algorithm parameters
>   were mandatory.  Because of this history some implementations encode
>   parameters as a NULL element and others omit them entirely.  The
>   correct encoding is to omit the parameters field; however,
>   implementations MUST also handle a SHA AlgorithmIdentifier 
>parameters
>   

RE: Last Call: draft-ietf-smime-sha2 (Using SHA2 Algorithms withCryptographic Message Syntax) to Proposed Standard

2008-03-01 Thread Turner, Sean P.
Denis,

Thanks for taking the time to review this ID. Responses are inline.

spt 

>-Original Message-
>
>There are obvious errors (intentionnaly left by the editor in 
>order to know how many people read the document).

If I was going to leave something intentionally in the document to see if
you'd read it, then it would have been a lot better ... something like (note
this isn't an actual offer) "if you mention this sentence to me I'll buy you
a beer".  This ID is way too short to sneak in this kind of phrase.

>On page 1:
>
>The message digest algorithms are defined in and [SHS].  
> ^^^ Also in section 2.4:

Will remove the "and".

>2.4. SHA-512 
>
>   The SHA-256 message digest algorithm is defined in [SHS].
>
>whereas it should be:
>
>2.4. SHA-512 
>
>   The SHA-512 message digest algorithm is defined in [SHS].

Will replace SHA-256 with SHA-512 in 2.4.

>It would be valuable to explain why DSA cannot be used with 
>SHA-384 and SHA-512.

I'll add some text.

>In addition, it is not acceptable to reference in the 
>*normative* references "work in progess", i.e.[ECCADD].

I'm pretty sure this is done all the time. There are 17 IDs in the RFC
editor queue with works-in-progress in normative references.

>The same applies for [SHS]. The text states:
>
>   NOTE [to be removed upon publication as an RFC]: NIST has not yet 
>   finalized FIPS 186-3 and there is a chance that the draft may be 
>   changed.  This may result in differences between what is documented 
>   in the current version of this document and what is in the 
>FIPS.  It 
>   is intended to synchronize the final version of this draft with the 
>   FIPS before publication as an RFC. 

The FIPS PUB 186-3 part that this ID needs has very little chance of
changing. The WG wanted this note to be safe.

>There is a more substantive comment on the first paragraph of 
>section 1. 
>The text states:
>
>   If an implementation chooses to support one of the algorithms 
>   discussed in this document, then the implementation MUST do so as 
>   described in this document. 
>
>I believe the text should be:
>
>   If an implementation chooses to support one of the algorithms 
>   discussed in this document, then the implementation MUST do so as 
>   described in [SHS]. 

The algorithms aren't defined in this document only the OIDs and where they
go in CMS ... so I still think it's actually "as described in this
document". But, Spencer suggests removing the sentences because they're not
needed and I tend to agree with him.

>A small discussion in the security considerations section on 
>the advantages (in particular in terms of performances versus 
>security) of using one or another function from the SHA2 
>family would be helpful.

I'll see if I can't get something from NIST (or at least point to it).

>While I welcome this draft, everybody should take into 
>consideration that, if the SHA2 family happens to be broken 
>then we will be at risk. 
>This should be mentioned into the security considerations section.

If an algorithm is cracked then isn't it obvious that we're in trouble?  No
other algorithm document I could find says something like this so I'm
inclined to not include this in the security considerations section.

>The NESSIE program has evaluated with succces the WHIRLPOOL algorithm. 
>WHIRLPOOL would be a good substitute to SHA-512 and I would 
>encourage that "someone" drafts an RFC to specify OIDs for 
>using WHIRLPOOL with CMS.
>
>Denis
>
>>The IESG has received a request from the S/MIME Mail Security WG 
>>(smime) to consider the following document:
>>
>>- 'Using SHA2 Algorithms with Cryptographic Message Syntax '
>>as a Proposed Standard
>>
>>The IESG plans to make a decision in the next few weeks, and solicits 
>>final comments on this action.  Please send substantive 
>comments to the 
>>ietf@ietf.org mailing lists by 2008-03-07. Exceptionally, 
>comments may 
>>be sent to [EMAIL PROTECTED] instead. In either case, please retain the 
>>beginning of the Subject line to allow automated sorting.
>>
>>The file can be obtained via
>>http://www.ietf.org/internet-drafts/draft-ietf-smime-sha2-03.txt
>>
>>
>>IESG discussion can be tracked via
>>https://datatracker.ietf.org/public/pidtracker.cgi?command=vie
w_id&dTag
>>=16033&rfc_flag=0
>>
>>
>
>Regards,
>
>Denis Pinkas
>
>
>

___
IETF mailing list
IETF@ietf.org
https://www.ietf.org/mailman/listinfo/ietf


RE: Last Call: draft-ietf-smime-sha2 (Using SHA2 AlgorithmswithCryptographic Message Syntax) to Proposed Standard

2008-03-04 Thread Turner, Sean P.
Denis,

Responses are in-line.

spt 

>-Original Message-
>From: [EMAIL PROTECTED] 
>[mailto:[EMAIL PROTECTED] On Behalf Of Denis Pinkas
>Sent: Monday, March 03, 2008 9:06 AM
>To: Paul Hoffman; Turner, Sean P.; ietf@ietf.org
>Cc: [EMAIL PROTECTED]
>Subject: Re: Last Call: draft-ietf-smime-sha2 (Using SHA2 
>AlgorithmswithCryptographic Message Syntax) to Proposed Standard
>
>
>Responses are in-line.
>
>(...)
>
>>>>There is a more substantive comment on the first paragraph 
>of section 
>>>>1.
>>>>
>>>>The text states:
>>>>
>>>>If an implementation chooses to support one of the algorithms
>>>>discussed in this document, then the implementation 
>MUST do so as
>>>>described in this document.
>>>>
>>>>I believe the text should be:
>>>>
>>>>If an implementation chooses to support one of the algorithms
>>>>discussed in this document, then the implementation 
>MUST do so as
>>>>described in [SHS].
>>>
>>>The algorithms aren't defined in this document only the OIDs 
>and where 
>>>they go in CMS ... so I still think it's actually "as described in 
>>>this document". But, Spencer suggests removing the sentences because 
>>>they're not needed and I tend to agree with him.
>>
>>Spencer wins on this one. The sentence doesn't make much sense in a 
>>standards-track document.
>
>It is important to know in which document the algorithms are 
>described in detail.
>Deleting the sentence would not solve my concern.

The 2nd sentence in the Intro says (or will say after I delete the and):
"The message digest algorithms are defined in [SHS]."  The 1st sentence in 3
references the DSS, X9.62, and RFC2313 for DSA, ECDSA, and RSA (we're moving
the references to 1st sentence of 2nd para. I think we're covered.

>>>  >A small discussion in the security considerations section on
>>>>the advantages (in particular in terms of performances versus
>>>>security) of using one or another function from the SHA2 
>family would 
>>>>be helpful.
>>>
>>>I'll see if I can't get something from NIST (or at least 
>point to it).
>>
>>I'll disagree with this. These are not security considerations; they 
>>are performance considerations. And pretty obvious ones at that.
>
>It is both performance and security. 
>The larger the hash is, the better is the security, but the 
>performance may be lower.

I don't really have a problem adding this sentence.

>>>  >While I welcome this draft, everybody should take into
>>>>consideration that, if the SHA2 family happens to be broken then we 
>>>>will be at risk.
>>>>This should be mentioned into the security considerations section.
>>>
>>>If an algorithm is cracked then isn't it obvious that we're in 
>>>trouble?  No other algorithm document I could find says 
>something like 
>>>this so I'm inclined to not include this in the security 
>considerations section.
>>
>>... or anywhere else. If any algorithm (hash, encryption, signing,
>>...) is broken, it is broken. Sean's right here.
>
>The message is the following: if the SHA2 family is broken, 
>then you had better to use two hash algorithms from a 
>different family (e.g. use Whirlpool).
>
>We should also reference 
>http://www.ietf.org/internet-drafts/draft-ietf-smime-multisig-04.txt
>which allows to use two different hash functions (from 
>different families, if possible).

I'm not sure we need to add this reference. 

>Denis
>
>
>

___
IETF mailing list
IETF@ietf.org
https://www.ietf.org/mailman/listinfo/ietf


RE: Gen-art review of draft-ietf-smime-multisig-04.txt

2008-03-10 Thread Turner, Sean P.
Elwyn,

Thanks for the review. Responses inline... 

spt

>-Original Message-
>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).

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.

>Editorial:
>idnits reports a clean bill of health.
>
>Abstract: Expand CMS acronym.

fixed

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

fixed

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

fixed

>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.

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

___
IETF mailing list
IETF@ietf.org
https://www.ietf.org/mailman/listinfo/ietf