Subject: [Gen-art] Gen-ART Review of draft-ietf-smime-sha2-03.txt

From: "Spencer Dawkins" <[email-masked]>
Date: Thu, 28 Feb 2008 23:27:05 -0600

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

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 :-(

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?

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.

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

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.

   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?

   Note that [RFC4231] specifies the conventions for use of for the

Concern: "of for" seems to be missing something.

   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.

   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.

Concern - the MUST in this paragraph duplicates clearer normative text in 
the next paragraph. I'd remove the last sentence.

   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?

   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?

   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?

3.1. DSA

   The algorithm identifier for DSA with SHA-224 signature values is:

Concern: SHA-256, right?

     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)" 

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

---

From: "Turner, Sean P." <[email-masked]>
Date: Fri, 29 Feb 2008 16:24:09 -0500

Spencer,

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

spt 

>-----Original Message-----
>From: Spencer Dawkins [mailto:[email-masked]] 
>Sent: Friday, February 29, 2008 12:27 AM
>To: General Area Review Team
>Cc: Sean Turner; Blake Ramsdell; [email-masked]; [email-masked]
>Subject: Gen-ART Review of draft-ietf-smime-sha2-03.txt
>
>Please resolve these comments along with any other Last Call 
>comments you may receive.

Will do.

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

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

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

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

>Concern: "in and" seems to be missing something.

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

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

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

>Nit - this sentence isn't clear and isn't required. I'd strike it.

Removed.

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

>Concern - SHA-512, right?

Denis caught this one too. It will be fixed.

>The algorithms have been mentioned a bunch of times without 
>them. Move them to the Introduction?

Will move them to the introduction.

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

>   The algorithm identifier for DSA with SHA-224 signature values is:

Yes.

>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

---

From: "Spencer Dawkins" <[email-masked]>
Date: Fri, 29 Feb 2008 23:04:21 -0600

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