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

["Spencer Dawkins" <spencer at wonderhamster.org>]

Hi, Sean,

This is much better than I feared. There were just too many places in -03 
where I was guessing what was intended, for me to say "ready for 
publication", or even "almost ready with nits".

I know you don't make the decisions about when drafts are last-called, but 
when you talk to your shepherd, you might suggest looking at diffs for the 
version posted for last call. that popped up a lot of the concerns i had 
(which were also concerns that denis had).

Best wishes with the draft.

Spencer


> Spencer,
>
> Thanks for taking the time to read the draft. Responses are inline.
>
> spt
>
>>-----Original Message-----
>>From: Spencer Dawkins [mailto:spencer at wonderhamster.org]
>>Sent: Friday, February 29, 2008 12:27 AM
>>To: General Area Review Team
>>Cc: Sean Turner; Blake Ramsdell; ietf at ietf.org; tim.polk at nist.gov
>>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
>>   field which contains a NULL.
>>
>>Nit - I'd describe the normative behavior, and then provide
>>the history (in a separate paragraph), just so implementers
>>don't have to scan history looking for normative behavior.
>
> Since this is an update to RFC3370 and we want the same algorithm 
> parameters
> I copied the text directly from there. I'd like to keep it the same so
> implementers know that there's nothing new.
>
>>Concern - the MUST in this paragraph duplicates clearer
>>normative text in the next paragraph. I'd remove the last sentence.
>
> See above.
>
>>   The AlgorithmIdentifier parameters field is OPTIONAL.  If present,
>>   the parameters field MUST contain a NULL.  Implementations MUST
>>   accept SHA2 AlgorithmIdentifiers with absent parameters.
>>   Implementations MUST accept SHA2 AlgorithmIdentifiers with NULL
>>   parameters.  Implementations SHOULD generate SHA2
>>   AlgorithmIdentifiers with absent parameters.
>>
>>2.4. SHA-512
>>
>>   The SHA-256 message digest algorithm is defined in [SHS].  The
>>
>>Concern - SHA-512, right?
>
> Denis caught this one too. It will be fixed.
>
>>   algorithm identifier for SHA-512 is:
>>
>>     id-sha512 OBJECT IDENTIFIER ::= {
>>       joint-iso-itu-t(2) country(16) us(840) organization(1) gov(101)
>>       csor(3) nistalgorithm(4) hashalgs(2) 3 }
>>
>>   The parameters are as specified in Section 2.
>>
>>3. Signature Algorithms
>>
>>   This section specifies the conventions employed by CMS
>>   implementations that support DSA [DSS], ECDSA [X9.62], and RSA
>>   [RFC2313] with SHA2 algorithms.
>>
>>Nit - why wait until this late in the document to provide
>>these references?
>>The algorithms have been mentioned a bunch of times without
>>them. Move them to the Introduction?
>
> Will move them to the introduction.
>
>>   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.
>>
>>Concern: why is this document being Last Called now, if the
>>expectation is that it's going into REF-WAIT and may change
>>before it's published?
>
> The WG wanted the note to be safe - The parts of FIPS PUB 186-3 that we 
> use
> has little chance of changing.
>
>>3.1. DSA
>>
>>   The algorithm identifier for DSA with SHA-224 signature values is:
>>
>>Concern: SHA-256, right?
>
> Yes.
>
>>     id-dsa-with-sha256 OBJECT IDENTIFIER ::=  { joint-iso-ccitt(2)
>>       country(16) us(840) organization(1) gov(101) csor(3)
>>       algorithms(4) id-dsa-with-sha2(3) 2 }
>>
>>   When either of these algorithm identifiers is used, the
>>   AlgorithmIdentifier parameters field MUST be absent.
>>
>>3.3. ECDSA
>>
>>6. References
>>
>>6.1. Normative References
>>
>>   [RFC2313]   Kaliski, B., "PKCS #1: RSA Encryption Version 1.5", RFC
>>               2313, March 1998.
>>
>>Concern: ID Nits says "** Obsolete normative reference: RFC
>>2313 (Obsoleted by RFC 2437)"
>
> I saw this in the nits tools and decided to not update it (note there's 
> also
> RFC3447 that obsoletes 2437). RFC 2437/3447 include a description of
> RSAPSS/OAEP that in my opinion (and some others) makes it harder to find 
> the
> v1.5 signature scheme. Since we don't use RSAPSS/OAEP I figured it's all
> right to keep the reference to RFC 2313. Since this is an update to RFC 
> 3370
> (which was published 4 years after RFC 2437) and RFC 3370 still references
> RFC 2313, I also figured it would be okay to not update the reference.
>
> spt
>
> 


_______________________________________________
IETF mailing list
IETF at ietf.org
https://www.ietf.org/mailman/listinfo/ietf